STM32L4: IOSV not enabled for GPIOG (and thus LPUART)

adri326 commented 5 months ago

On some STM32 MCUs (which includes the STM32L4xx family), the GPIOG ports are powered by an alternative power supply, which is only available when the IOSV bit is set by the user in PWR->CR2, but embassy-stm32 doesn't provide a way to turn it on, nor does it turn it on automatically when trying to use GPIOG.

How I encountered the bug

According to the manual for my testing board, I should be able to use the serial I/O of the ST-Link by sending data over the LPUSART1 peripheral with the PG7 and PG8 pins.

However, when changing the chip name and the pings accordingly, I don't seem to be able to get any output, and the stm32l4/usart example then panics over a Framing error.

Flashing and running the board with an equivalent program using STM32's HAL library does print an output, in contrast.

Changes made

$ git rev-parse HEAD
bd4cb82945112ecb847456a3b0b04163341e44fd

diff --git a/examples/stm32l4/.cargo/config.toml b/examples/stm32l4/.cargo/config.toml
index db3a7cef..6c3e28f0 100644
--- a/examples/stm32l4/.cargo/config.toml
+++ b/examples/stm32l4/.cargo/config.toml
@@ -2,7 +2,7 @@
 # replace STM32F429ZITx with your chip as listed in `probe-rs chip list`
 #runner = "probe-rs run --chip STM32L475VGT6"
 #runner = "probe-rs run --chip STM32L475VG"
-runner = "probe-run --chip STM32L4S5QI"
+runner = "probe-run --chip STM32L4A6ZGTx"

 [build]
 target = "thumbv7em-none-eabi"
diff --git a/examples/stm32l4/Cargo.toml b/examples/stm32l4/Cargo.toml
index d42e6957..50c9893d 100644
--- a/examples/stm32l4/Cargo.toml
+++ b/examples/stm32l4/Cargo.toml
@@ -6,7 +6,7 @@ license = "MIT OR Apache-2.0"

 [dependencies]
 # Change stm32l4s5vi to your chip name, if necessary.
-embassy-stm32 = { version = "0.1.0", path = "../../embassy-stm32", features = [ "defmt", "unstable-pac", "stm32l4s5qi", "memory-x", "time-driver-any", "exti", "chrono"]  }
+embassy-stm32 = { version = "0.1.0", path = "../../embassy-stm32", features = [ "defmt", "unstable-pac", "stm32l4a6zg", "memory-x", "time-driver-any", "exti", "chrono"]  }
 embassy-sync = { version = "0.5.0", path = "../../embassy-sync", features = ["defmt"] }
 embassy-executor = { version = "0.5.0", path = "../../embassy-executor", features = ["task-arena-size-32768", "arch-cortex-m", "executor-thread", "defmt", "integrated-timers"] }
 embassy-time = { version = "0.3.0", path = "../../embassy-time", features = ["defmt", "defmt-timestamp-uptime", "tick-hz-32_768", ] }
diff --git a/examples/stm32l4/src/bin/usart.rs b/examples/stm32l4/src/bin/usart.rs
index 7bab2395..c8026056 100644
--- a/examples/stm32l4/src/bin/usart.rs
+++ b/examples/stm32l4/src/bin/usart.rs
@@ -8,7 +8,8 @@ use embassy_stm32::{bind_interrupts, peripherals, usart};
 use {defmt_rtt as _, panic_probe as _};

 bind_interrupts!(struct Irqs {
-    UART4 => usart::InterruptHandler<peripherals::UART4>;
+    // UART4 => usart::InterruptHandler<peripherals::UART4>;
+    LPUART1 => usart::InterruptHandler<peripherals::LPUART1>;
 });

 #[cortex_m_rt::entry]
@@ -18,7 +19,8 @@ fn main() -> ! {
     let p = embassy_stm32::init(Default::default());

     let config = Config::default();
-    let mut usart = Uart::new(p.UART4, p.PA1, p.PA0, Irqs, NoDma, NoDma, config).unwrap();
+    // let mut usart = Uart::new(p.UART4, p.PA1, p.PA0, Irqs, NoDma, NoDma, config).unwrap();
+    let mut usart = Uart::new(p.LPUART1, p.PG8, p.PG7, Irqs, NoDma, NoDma, config).unwrap();

     unwrap!(usart.blocking_write(b"Hello Embassy World!\r\n"));
     info!("wrote Hello, starting echo");

Output

0.000000 INFO  Hello World!
└─ usart::__cortex_m_rt_main @ src/bin/usart.rs:17
0.000000 TRACE BDCR ok: 00008200
└─ embassy_stm32::rcc::bd::{impl#3}::init @ /home/eburgun/Programs/embassy/embassy-stm32/src/fmt.rs:117
0.000000 DEBUG rcc: Clocks { hclk1: Some(Hertz(4000000)), hclk2: Some(Hertz(4000000)), hclk3: Some(Hertz(4000000)), hsi: None, hsi48: Some(Hertz(48000000)), lse: None, lsi: None, msi: Some(Hertz(4000000)), pclk1: Some(Hertz(4000000)), pclk1_tim: Some(Hertz(4000000)), pclk2: Some(Hertz(4000000)), pclk2_tim: Some(Hertz(4000000)), pll1_p: None, pll1_q: None, pllsai1_p: None, pllsai1_q: None, pllsai2_p: None, rtc: Some(Hertz(32000)), sai1_extclk: None, sai2_extclk: None, sys: Some(Hertz(4000000)) }
└─ embassy_stm32::rcc::set_freqs @ /home/eburgun/Programs/embassy/embassy-stm32/src/fmt.rs:130
0.001342 TRACE USART: presc=1, div=0x000022b9 (mantissa = 555, fraction = 9)
└─ embassy_stm32::usart::configure @ /home/eburgun/Programs/embassy/embassy-stm32/src/fmt.rs:117
0.007446 TRACE Using 16 bit oversampling, desired baudrate: 115200, actual baudrate: 114944
└─ embassy_stm32::usart::configure @ /home/eburgun/Programs/embassy/embassy-stm32/src/fmt.rs:117
0.016418 INFO  wrote Hello, starting echo
└─ usart::__cortex_m_rt_main @ src/bin/usart.rs:29
0.018524 ERROR panicked at 'unwrap failed: usart.blocking_read(& mut buf)'
error: `Framing`
└─ usart::__cortex_m_rt_main @ src/bin/usart.rs:33
0.021026 ERROR panicked at /home/eburgun/.cargo/registry/src/index.crates.io-6f17d22bba15001f/defmt-0.3.6/src/lib.rs:368:5:
explicit panic
└─ panic_probe::print_defmt::print @ /home/eburgun/.cargo/registry/src/index.crates.io-6f17d22bba15001f/panic-probe-0.3.1/src/lib.rs:104

What the issue (and fix) is

Looking at my C code, the following seems to be the strict minimum to get UART working with STMicro's HAL library:

UART_HandleTypeDef uart_handle = {0};

void setup_uart(uint32_t baud_rate) {
    __HAL_RCC_PWR_CLK_ENABLE();
    __HAL_RCC_LPUART1_CLK_ENABLE();
    __HAL_RCC_GPIOG_CLK_ENABLE();

    // Notice this odd line:
    HAL_PWREx_EnableVddIO2();

    GPIO_InitTypeDef GPIO_InitStruct = {0};
    GPIO_InitStruct.Pin = GPIO_PIN_7 | GPIO_PIN_8;
    GPIO_InitStruct.Alternate = GPIO_AF8_LPUART1;
    GPIO_InitStruct.Mode = GPIO_MODE_AF_PP;
    GPIO_InitStruct.Pull = GPIO_NOPULL;
    GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_VERY_HIGH;
    HAL_GPIO_Init(GPIOG, &GPIO_InitStruct);

    uart_handle.Init.BaudRate = baud_rate;
    uart_handle.Init.StopBits = UART_STOPBITS_1;
    uart_handle.Init.Parity = UART_PARITY_NONE;
    uart_handle.Init.Mode = UART_MODE_TX_RX;
    uart_handle.Init.HwFlowCtl = UART_HWCONTROL_NONE;
    uart_handle.Init.OverSampling = UART_OVERSAMPLING_16;
    uart_handle.Init.WordLength = UART_WORDLENGTH_8B;
    uart_handle.Init.OneBitSampling = UART_ONE_BIT_SAMPLE_DISABLE;
    uart_handle.Instance = LPUART1;

    if (HAL_UART_Init(&uart_handle) != HAL_OK) {
        Error_Handler();
    }

    // Data can then be transmitted using HAL_Transmit()
}

The HAL_PWREx_EnabledVddIO2() line stands out: looking into its source code, it sets the IOSV bit of the PWR->CR2 register to 1, and according to the MCU datasheet, this register toggles the separate power supply for the GPIOG pins, which is by default turned off. Normally, you would use the PVM API to gracefully turn on the separate power supply first (see page 164), but skipping that step seems to not cause any issues on my end.

Once I manually set this bit using stm32-metapac, the code then works:

embassy_stm32::pac::PWR.cr2().modify(|reg| reg.set_iosv(true));

I would happily contribute a fix to this, although I would need guidance, namely:

should there be a PVM API in embassy-stm32? It'd mostly let the user toggle the different power supplies, and the different power options.
should the IOSV register be automatically set by the GPIO API? The tricky bit there is that most of the GPIOG pins are powered by it, but it would be nice to give the user the option to turn it off.
alternatively, we could add a flag in the config for embassy_stm32::init, since the GPIO API currently doesn't have any configuration, that would turn on this power supply.
it looks like the USB peripheral also runs on a different power supply, so I wonder if it isn't also suffering from the same issue.

Dirbaio commented 5 months ago

alternatively, we could add a flag in the config for embassy_stm32::init, since the GPIO API currently doesn't have any configuration, that would turn on this power supply.

yes, this is what I'd do (and turn it on by default).

it looks like the USB peripheral also runs on a different power supply, so I wonder if it isn't also suffering from the same issue.

yes, this is handled automatically when you turn on USB: https://github.com/embassy-rs/embassy/blob/ea25112f7d1ce27a7216ce6c3aab983dc3a0d7c0/embassy-stm32/src/usb_otg/usb.rs#L565

I don't think we can do that for IOSV though. It'll be annoying to implement in the HAL since many peripherals can use the affected pins, and could be bad for perf/codesize too. a setting in init seems beter to me.

adri326 commented 5 months ago

And the Peripherals struct should then keep track of it to unset it on Drop?

Dirbaio commented 5 months ago

no, just enable it on init if requested, and leave it enabled forever.

embassy-rs / embassy