Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug in pico-sdk/src/rp2_common/pico_stdio_uart /stdio_uart.c causing lockups #2009

Open
thunter6600 opened this issue Nov 1, 2024 · 1 comment
Assignees

Comments

@thunter6600
Copy link

During a call to “stdio_set_chars_available_callback()” when a receive character is already pending in the UART the system locks up. It continuously re-enters the low-level "on_uart_rx()" function because the callback pointer has not yet been saved before enabling the interrupt.

Note the order of the highlighted lines in file "pico-sdk/src/rp2_common/pico_stdio_uart /stdio_uart.c":

static void stdio_uart_set_chars_available_callback(void (fn)(void), void *param) {
uint irq_num = UART_IRQ_NUM(uart_instance);
if (fn && !chars_available_callback) {
irq_set_exclusive_handler(irq_num, on_uart_rx);
irq_set_enabled(irq_num, true);
uart_set_irqs_enabled(uart_instance, true, false);
} else if (!fn && chars_available_callback) {
uart_set_irqs_enabled(uart_instance, false, false);
irq_set_enabled(irq_num, false);
irq_remove_handler(irq_num, on_uart_rx);
}
chars_available_callback = fn;
chars_available_param = param;
}

Clearly the "chars_available_callback" pointer is set after the RX interrupt has been enabled. I suggest to change the function to look like this:

static void stdio_uart_set_chars_available_callback(void (fn)(void), void *param) {
uint irq_num = UART_IRQ_NUM(uart_instance);
if (fn && !chars_available_callback) {
chars_available_callback = fn;
chars_available_param = param;
irq_set_exclusive_handler(irq_num, on_uart_rx);
irq_set_enabled(irq_num, true);
uart_set_irqs_enabled(uart_instance, true, false);

} else if (!fn && chars_available_callback) {
uart_set_irqs_enabled(uart_instance, false, false);
irq_set_enabled(irq_num, false);
irq_remove_handler(irq_num, on_uart_rx);
chars_available_callback = fn;
chars_available_param = param;

}
}

Note that the order is critical both in the callback installation and de-installation context. You want the callback pointers saved before enabling the interrupt. You also want to clear the callback pointer only after the interrupt is disabled.

There are many scenarios where a UART is connected to a device which spontaneously and regularly sends serial data (e.g. telemetry). If the Pico is powering up while such a data burst is happening, the “stdio_set_chars_available_callback()” will hang unless my suggested fix is implemented.

Please let me know if you need further information.

@peterharperuk peterharperuk self-assigned this Nov 4, 2024
@peterharperuk
Copy link
Contributor

Thanks. I could reproduce a hang by sending random data to the port just before the call stdio_uart_set_chars_available_callback and this seems to fix it.

peterharperuk added a commit to peterharperuk/pico-sdk that referenced this issue Nov 5, 2024
The function is setting the callback after enabing interrupts which
can cause a hang if a receive character is already pending.
Smilarly we also have to clear the callback pointer only after the
interrupt is disabled.

Fixes raspberrypi#2009
peterharperuk added a commit to peterharperuk/pico-sdk that referenced this issue Nov 5, 2024
The function is setting the callback after enabing interrupts which
can cause a hang if a receive character is already pending.
Smilarly we also have to clear the callback pointer only after the
interrupt is disabled.

Fixes raspberrypi#2009
peterharperuk added a commit to peterharperuk/pico-sdk that referenced this issue Nov 5, 2024
The function is setting the callback after enabing the interrupt which
can cause a hang if a receive character is already pending.
Similarly we also have to clear the callback pointer only after the
interrupt is disabled.

Fixes raspberrypi#2009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants