Skip to content

[bug] Uart::write() deadlocks when Serial.begin() fails (LPUART with unsupported baud/clock) #3071

Description

@t-miura

First of all, I would like to appreciate all contributors on this project so much!
Without this, I(and many others) couldn't get STM32WL supported on Meshtastic(yes, it now runs on STM32WL!)


Describe the bug

When Serial.begin(baud) fails during hardware initialization, uart_init() returns false and _ready is set to false.
However, Uart::write() in cores/arduino/Serial.cpp does not check _ready before entering its transmit path.

After a failed begin(), subsequent write() / print() calls proceed normally until the 63-byte TX ring buffer gets filled up.
At that point, write() enters an infinite spin:

Because the hardware was never successfully initialized, the TX interrupt never fires, tx_tail never advances, and the MCU stalls permanently.


Steps to reproduce

  1. Target an STM32 with LPUART1 (e.g. STM32WLE5 / Wio-E5 mini).
  2. Have LSE selected as the LPUART1 kernel clock source (typical when RTC or low-power wake-up is configured).
  3. Call begin() with a baud rate that LSE cannot satisfy:
    SerialLP1.begin(38400); // fails: LSE (32.768 kHz) < 3 × 38400 = 115,200 Hz
    SerialLP1.println("This will freeze the CPU once the TX buffer fills.");
  4. MCU deadlocks inside Uart::write() once the 63-byte TX buffer is exhausted.

Context / Background

This was observed in Meshtastic , attempts on enhancing variant support for Seeed Wio-E5 mini (STM32WLE5, LPUART1 on PC0/PC1 for GPS/GNSS).
The GPS/GNSS driver probes common baud rates sequentially (9600 -> 38400 -> 115200).
When LSE is active as the LPUART1 kernel clock, the 38400 probe triggers the deadlock immediately on boot sequence.

On a side note, we can't use USART2 on PA_2_ALT1/PA_3_ALT1 that's available on Wio-E5/mini, mainly due to DFU's quirks.
This is also Wio-E5/LoRa-E5 specific context at this moment,
other STM32WL variant in meshtastic uses other pins w/USART for GPS/GNSS.

As this is a physical limitation, I've been testing some naive codes to switch the kernel clock to HSI, and now GPS/GNSS module works in all baud rate above.
(ref: meshtastic/firmware@develop...t-miura:firmware:fix/stm32wl-lpuart-clock-wio-e5-gps , note that currently this will in inf. loop if HSI never becomes ready)

Why is the _ready guard still needed even after switching the kernel clock to HSI?

There are two reasons as far as I concern:

1. What if we failed to switch the kernel clock, and LPUART runs on LSE?

It's very, very unlikely to happen, but there are no guarantee that this won't be happen.
While we're trying to prevent LPUART-on-LSE being used on GPS/GNSS driver, but if _ready guard implemented, it'll be much reliable.

2. uart_config_lowpower() concerns

Note that I haven't actually observed this possible issue, so it's low on my reason list:

First, I've read #2897, and agree with that line:

It is up to end user to properly configure the clock depending on their needs.

Also uart_config_lowpower() function is available to switch LPUART1's kernel clock from LSE to HSI before re-calling begin().

However, there is a code path in that function (under __HAL_RCC_LPUART1_CLK_CONFIG) that actually configures LSE,
not HSI due to a conditional compilation mismatch(uart.c lines 911–914).
If the system compiles through that branch, the "switch to HSI" silently does nothing or configures the wrong source, begin() fails again, and any subsequent write() deadlocks identically.

More broadly, uart_config_lowpower() is a separate opt-in call.
Standard Arduino libraries (TinyGPS, sensor drivers, etc.) call begin() and write() directly.
They have no reason to know that a particular begin() might fail or that a clock switch is needed first.


Expected behavior

If _ready is false, Uart::write() should return 0 immediately rather than spinning on an interrupt that will never fire.

Suggested Fix

Guard both write() overloads in cores/arduino/Serial.cpp with a _ready check:

size_t Uart::write(const uint8_t *buffer, size_t size)
{
  if (!_ready) {
    return 0;
  }
  // ... existing implementation
}

size_t Uart::write(uint8_t c)
{
  uint8_t buff = c;
  return write(&buff, 1);  // delegates — no separate guard needed here
}

Since write(uint8_t c) already delegates to write(const uint8_t*, size_t), only the latter needs the guard.

Thank you and kind regards.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions