Skip to content

Commit 6fa498c

Browse files
projectgusdpgeorge
authored andcommitted
rp2/mpnetworkport: Fix lost CYW43 WiFi events when using both cores.
There's a very odd but predictable sequence of events that breaks Wi-Fi when using both cores: 1) CPU1 calls pendsv_suspend() - for example sleep() causes a softtimer node to be inserted, which calls pendsv_suspend(). 2) CYW43 sends wakeup IRQ. CPU0 GPIO IRQ handler schedules PendSV and disables the GPIO IRQ on CPU0, to re-enable after cyw43_poll() runs and completes. 3) CPU0 PendSV_Handler runs, sees pendsv is suspended, exits. 4) CPU1 calls pendsv_resume() and pendsv_resume() sees PendSV is pending and triggers it on CPU1. 5) CPU1 runs PendSV_Handler, runs cyw43_poll(), and at the end it re-enables the IRQ *but now on CPU1*. However CPU1 has GPIO IRQs disabled, so the CYW43 interrupt never runs again... The fix in this commit is to always enable/disable the interrupt on CPU0. This isn't supported by the pico-sdk, but it is supported by the hardware. Fixes issue micropython#16779. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <[email protected]>
1 parent dd7a950 commit 6fa498c

File tree

1 file changed

+29
-5
lines changed

1 file changed

+29
-5
lines changed

ports/rp2/mpnetworkport.c

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,36 @@ static soft_timer_entry_t mp_network_soft_timer;
5959

6060
volatile int cyw43_has_pending = 0;
6161

62+
// The Pico SDK only lets us set GPIO wake on the current running CPU, but the
63+
// hardware doesn't have this limit. We need to always enable/disable the pin
64+
// interrupt on CPU0, regardless of which CPU runs PendSV and
65+
// cyw43_post_poll_hook(). See feature request at https://github.com/raspberrypi/pico-sdk/issues/2354
66+
static void gpio_set_cpu0_host_wake_irq_enabled(bool enable) {
67+
// This is a re-implementation of gpio_set_irq_enabled() and _gpio_set_irq_enabled()
68+
// from the pico-sdk, but with the core, gpio, and event type hardcoded to shrink
69+
// code size.
70+
io_bank0_irq_ctrl_hw_t *irq_ctrl_base = &io_bank0_hw->proc0_irq_ctrl;
71+
uint32_t gpio = CYW43_PIN_WL_HOST_WAKE;
72+
uint32_t events = CYW43_IRQ_LEVEL;
73+
io_rw_32 *en_reg = &irq_ctrl_base->inte[gpio / 8];
74+
events <<= 4 * (gpio % 8);
75+
if (enable) {
76+
hw_set_bits(en_reg, events);
77+
} else {
78+
hw_clear_bits(en_reg, events);
79+
}
80+
}
81+
82+
// GPIO IRQ always runs on CPU0
6283
static void gpio_irq_handler(void) {
6384
uint32_t events = gpio_get_irq_event_mask(CYW43_PIN_WL_HOST_WAKE);
6485
if (events & CYW43_IRQ_LEVEL) {
65-
// As we use a high level interrupt, it will go off forever until it's serviced.
66-
// So disable the interrupt until this is done. It's re-enabled again by
67-
// CYW43_POST_POLL_HOOK which is called at the end of cyw43_poll_func.
68-
gpio_set_irq_enabled(CYW43_PIN_WL_HOST_WAKE, CYW43_IRQ_LEVEL, false);
86+
// As we use a level interrupt (and can't use an edge interrupt
87+
// as CYW43_PIN_WL_HOST_WAKE is also a SPI data pin), we need to disable
88+
// the interrupt to stop it re-triggering until after PendSV run
89+
// cyw43_poll(). It is re-enabled in cyw43_post_poll_hook(), implemented
90+
// below.
91+
gpio_set_cpu0_host_wake_irq_enabled(false);
6992
cyw43_has_pending = 1;
7093
__sev();
7194
pendsv_schedule_dispatch(PENDSV_DISPATCH_CYW43, cyw43_poll);
@@ -81,9 +104,10 @@ void cyw43_irq_init(void) {
81104
#endif
82105
}
83106

107+
// This hook will run on whichever CPU serviced the PendSV interrupt
84108
void cyw43_post_poll_hook(void) {
85109
cyw43_has_pending = 0;
86-
gpio_set_irq_enabled(CYW43_PIN_WL_HOST_WAKE, CYW43_IRQ_LEVEL, true);
110+
gpio_set_cpu0_host_wake_irq_enabled(true);
87111
}
88112

89113
#endif

0 commit comments

Comments
 (0)