From 027c3dca80f4dc87c55125441d3162d06ad2b980 Mon Sep 17 00:00:00 2001 From: Phil Howard Date: Fri, 24 Jan 2025 10:07:20 +0000 Subject: [PATCH 1/2] CI: HACK: Add temporary DMA workaround. Some quirk of the Pico2 W versus the Pico W causes wireless to be very upset by our chained DMA display driver code. See: * https://github.com/micropython/micropython/issues/16627 * https://github.com/raspberrypi/pico-sdk/issues/2206 Backport a Pico SDK patch to fix this. See: * https://github.com/peterharperuk/pico-sdk/commit/2a810a926722b163ea3f7b183053964a2dc57e76 --- boards/pico2_w_cyw43.patch | 57 ++++++++++++++++++++++++++++++++++++++ ci/micropython.sh | 3 ++ 2 files changed, 60 insertions(+) create mode 100644 boards/pico2_w_cyw43.patch diff --git a/boards/pico2_w_cyw43.patch b/boards/pico2_w_cyw43.patch new file mode 100644 index 0000000..dd25e5d --- /dev/null +++ b/boards/pico2_w_cyw43.patch @@ -0,0 +1,57 @@ +From 2a810a926722b163ea3f7b183053964a2dc57e76 Mon Sep 17 00:00:00 2001 +From: Peter Harper +Date: Fri, 24 Jan 2025 18:47:52 +0000 +Subject: [PATCH] Fix unreliable writes to cyw43. + +We use a pio and dma to write to the cyw43 chip using spi. Normally you +write an address and then read the data from that address, so the pio +program does does a write then read. + +If you just want to write data in the case of uploading firmware we +use the fdebug_tx_stall flag to work out if the pio has stalled waiting +to read data which will never arrive. + +The theory is that this flag will also get set if the bus is busy. So +we mistakenly think a write to cyw43 has completed. + +Add a check for the dma irq as well. + +Fixes #2206 +--- + src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +diff --git a/src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c b/src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c +index bcc7284f1..302155ca2 100644 +--- a/src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c ++++ b/src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c +@@ -231,7 +231,7 @@ int cyw43_spi_transfer(cyw43_int_t *self, const uint8_t *tx, size_t tx_length, u + return CYW43_FAIL_FAST_CHECK(-CYW43_EINVAL); + } + +- bus_data_t *bus_data = (bus_data_t *)self->bus_data; ++ volatile bus_data_t *bus_data = (bus_data_t *)self->bus_data; + start_spi_comms(self); + if (rx != NULL) { + if (tx == NULL) { +@@ -306,6 +306,7 @@ int cyw43_spi_transfer(cyw43_int_t *self, const uint8_t *tx, size_t tx_length, u + channel_config_set_bswap(&out_config, true); + channel_config_set_dreq(&out_config, pio_get_dreq(bus_data->pio, bus_data->pio_sm, true)); + ++ dma_hw->ints0 = 1u << bus_data->dma_out; + dma_channel_configure(bus_data->dma_out, &out_config, &bus_data->pio->txf[bus_data->pio_sm], tx, tx_length / 4, true); + + uint32_t fdebug_tx_stall = 1u << (PIO_FDEBUG_TXSTALL_LSB + bus_data->pio_sm); +@@ -315,6 +316,12 @@ int cyw43_spi_transfer(cyw43_int_t *self, const uint8_t *tx, size_t tx_length, u + tight_loop_contents(); // todo timeout + } + __compiler_memory_barrier(); ++ ++ while (!(dma_hw->intr & 1u << bus_data->dma_out)) { ++ tight_loop_contents(); ++ } ++ dma_hw->ints0 = 1u << bus_data->dma_out; ++ + pio_sm_set_enabled(bus_data->pio, bus_data->pio_sm, false); + pio_sm_set_consecutive_pindirs(bus_data->pio, bus_data->pio_sm, CYW43_PIN_WL_DATA_IN, 1, false); + } else if (rx != NULL) { /* currently do one at a time */ diff --git a/ci/micropython.sh b/ci/micropython.sh index 9507499..bd628f8 100644 --- a/ci/micropython.sh +++ b/ci/micropython.sh @@ -52,6 +52,9 @@ function ci_micropython_clone { git submodule update --init lib/micropython-lib git submodule update --init lib/tinyusb git submodule update --init lib/btstack + log_inform "HACK: Patching dma aborts for Pico2 W." + cd "$CI_BUILD_ROOT/micropython/lib/pico-sdk" + git apply "$CI_PROJECT_ROOT/boards/pico2_w_cyw43.patch" cd "$CI_BUILD_ROOT" } From 507a4f87705a7e9c3ed016decb59e4917d2c6003 Mon Sep 17 00:00:00 2001 From: Phil Howard Date: Thu, 30 Jan 2025 10:17:35 +0000 Subject: [PATCH 2/2] HACK: Sync CYW43 patch with upstream. Update the backported cyw43 patch to the version now merged to develop. TODO: Drop this when the next Pico SDK release is incorporated into MicroPython. --- boards/pico2_w_cyw43.patch | 44 +++++++++++++------------------------- 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/boards/pico2_w_cyw43.patch b/boards/pico2_w_cyw43.patch index dd25e5d..5771b38 100644 --- a/boards/pico2_w_cyw43.patch +++ b/boards/pico2_w_cyw43.patch @@ -1,4 +1,4 @@ -From 2a810a926722b163ea3f7b183053964a2dc57e76 Mon Sep 17 00:00:00 2001 +From ea32e6dfb5232ad7d0ebba7da40dda68da05bc76 Mon Sep 17 00:00:00 2001 From: Peter Harper Date: Fri, 24 Jan 2025 18:47:52 +0000 Subject: [PATCH] Fix unreliable writes to cyw43. @@ -9,49 +9,35 @@ program does does a write then read. If you just want to write data in the case of uploading firmware we use the fdebug_tx_stall flag to work out if the pio has stalled waiting -to read data which will never arrive. +to write more data. The theory is that this flag will also get set if the bus is busy. So we mistakenly think a write to cyw43 has completed. -Add a check for the dma irq as well. +Wait for the dma write to complete before waiting for the pio to stall. Fixes #2206 --- - src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c | 9 ++++++++- - 1 file changed, 8 insertions(+), 1 deletion(-) + src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c | 6 ++++-- + 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c b/src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c -index bcc7284f1..302155ca2 100644 +index bcc7284f1..787eeff5d 100644 --- a/src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c +++ b/src/rp2_common/pico_cyw43_driver/cyw43_bus_pio_spi.c -@@ -231,7 +231,7 @@ int cyw43_spi_transfer(cyw43_int_t *self, const uint8_t *tx, size_t tx_length, u - return CYW43_FAIL_FAST_CHECK(-CYW43_EINVAL); - } +@@ -308,11 +308,13 @@ int cyw43_spi_transfer(cyw43_int_t *self, const uint8_t *tx, size_t tx_length, u -- bus_data_t *bus_data = (bus_data_t *)self->bus_data; -+ volatile bus_data_t *bus_data = (bus_data_t *)self->bus_data; - start_spi_comms(self); - if (rx != NULL) { - if (tx == NULL) { -@@ -306,6 +306,7 @@ int cyw43_spi_transfer(cyw43_int_t *self, const uint8_t *tx, size_t tx_length, u - channel_config_set_bswap(&out_config, true); - channel_config_set_dreq(&out_config, pio_get_dreq(bus_data->pio, bus_data->pio_sm, true)); - -+ dma_hw->ints0 = 1u << bus_data->dma_out; dma_channel_configure(bus_data->dma_out, &out_config, &bus_data->pio->txf[bus_data->pio_sm], tx, tx_length / 4, true); ++ pio_sm_set_enabled(bus_data->pio, bus_data->pio_sm, true); ++ dma_channel_wait_for_finish_blocking(bus_data->dma_out); ++ uint32_t fdebug_tx_stall = 1u << (PIO_FDEBUG_TXSTALL_LSB + bus_data->pio_sm); -@@ -315,6 +316,12 @@ int cyw43_spi_transfer(cyw43_int_t *self, const uint8_t *tx, size_t tx_length, u - tight_loop_contents(); // todo timeout + bus_data->pio->fdebug = fdebug_tx_stall; +- pio_sm_set_enabled(bus_data->pio, bus_data->pio_sm, true); + while (!(bus_data->pio->fdebug & fdebug_tx_stall)) { +- tight_loop_contents(); // todo timeout ++ tight_loop_contents(); } __compiler_memory_barrier(); -+ -+ while (!(dma_hw->intr & 1u << bus_data->dma_out)) { -+ tight_loop_contents(); -+ } -+ dma_hw->ints0 = 1u << bus_data->dma_out; -+ pio_sm_set_enabled(bus_data->pio, bus_data->pio_sm, false); - pio_sm_set_consecutive_pindirs(bus_data->pio, bus_data->pio_sm, CYW43_PIN_WL_DATA_IN, 1, false); - } else if (rx != NULL) { /* currently do one at a time */