Add few devices to the STM32WL family #34751

fabiobaltieri · 2021-05-01T14:21:27Z

Hi, this is adding various devices (most of the ones that have a driver anyway) to the STM32WL series. I tested most of these with the various examples on a STM32WE5 based Seed Lora-E5 module, so they should work fine on the WL55 as well, they only needed few extra defines for the RTC and ADC drivers.

str4t0m

One nit. Register definitions are consistent with the reference manual.

dts/arm/st/wl/stm32wl.dtsi

fabiobaltieri · 2021-05-03T15:45:37Z

Rebased, see if buildkite is happy again.

FRASTM · 2021-05-05T14:15:12Z

dts/arm/st/wl/stm32wl.dtsi

+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x58010000 0x400>;
+			interrupts = <44 5>;


Can you check the position of the interrupt as the entry 44 is mentioned as "Reserved" by the RefMan
subghzspi is present at position 30 of the CPU2 for the stm32wl5x.

Hey you are right! I was cross referencing stm32wle5xx.h from the cube package when I did this, the header file defines:

#if defined(CORE_CM0PLUS) ... SUBGHZSPI_IRQn = 30, ... #else ... SUBGHZSPI_IRQn = 44, ...

and stm32wle5xx.h only defines it on 44. Curiously enough there is a Radio IRQ on channel 44 of the EXTI, but it's not the SPI Interrupt. And the SPI driver in the cube package uses that SPI in polling mode anyway.

But setting this to 30 would not be correct either since it would try to resolve to the wrong nvic controller, and we don't have anything for the second cpu defined in the dts yet. Plus it would not help me because I'm working on a STM32WLE5, the single core version. :-)

Do you have a contact to check if maybe the definition in the cube file is correct and that reserved line is actually usable and the reference manual has to be rectified? Otherwise it means that this SPI cannot be used in interrupt mode in a single core setup.

For what is worth, I hacked together a sample to probe something on that SPI and the interrupt does fire (E5 single core interrupt position 44):

Breakpoint 1, spi_stm32_isr (dev=0x20000160 <__device_dts_ord_165>) at zephyrproject/zephyr/drivers/spi/spi_ll_stm32.c:409 409 struct spi_stm32_data *data = dev->data; (gdb) p *dev $1 = {name = 0x8006649 "SUBGHZSPI", config = 0x8005d74 <spi_stm32_cfg_0>, api = 0x8005d4c <api_funcs>, state = 0x200004cc <__devstate_dts_ord_165>, data = 0x20000020 <spi_stm32_dev_data_0>, handles = 0x8005b88 <__devicehdl_DT_N_S_soc_S_spi_58010000>}

Found few other missing bits on the clock setup to get to that point though so I'm adding a patch for those as well. :-)

This is probably a missing line in the ref manual (of the stm32wleX - RM0461): The stm32wle5xx.h from the cube is setting the SUBGHZSPI_IRQn at position = 44, for the M4. Let me check internally.

Right, sorry I pasted the wrong file name on my first reply, stm32wl55xx.h defines that on both 30 and 44 depending on whether it's building for the M0 or M4, and stm32wle5xx.h only has it on 44, and only has the M4. Thanks for checking it!

After checking, the entry 44 of the NVIC is the SUBGHZ SPI instance.

str4t0m · 2021-05-06T08:05:02Z

drivers/clock_control/clock_stm32_ll_common.c

@@ -232,6 +242,9 @@ static int stm32_clock_control_get_subsys_rate(const struct device *clock,
 	!defined (CONFIG_SOC_SERIES_STM32G0X)
 	uint32_t apb2_clock = get_bus_clock(ahb_clock, STM32_APB2_PRESCALER);
 #endif
+#if defined(CONFIG_SOC_SERIES_STM32WLX)
+	uint32_t apb3_clock = get_bus_clock(ahb_clock, STM32_AHB3_PRESCALER);


Here you are assigning a different prescaler (STM32_AHB3_PRESCALER).

The apb3 prescaler is a good addition, but a few more steps are necessary to introduce it:
You need to introduce the apb3 flag in the device tree bindings before, then the symbol for STM32_APB3_PRESCALER, and additionally it would be fine if that prescaler is then set in the nucleo_board definitions.

I also had a short look on the adc part, will add a few comments after I had a more thorough look.

Hi! From what I understand of the WL55/WLE5 clock architecture, the AHB3 (shared) prescaler output goes straight to both HCLK3 and PCLK3, so there's no APB3 prescaler in the chip. Guess I could #define STM32_APB3_PRESCALER STM32_AHB3_PRESCALER? Though that would sound more confusing than anything else to me.

@ABOSTM Can you have a check ?

Sorry, please ignore the comment above, I didn't check against the datasheet, and assumed it would have it's own prescaler.
In that case it may be preferable to call the variable ahb3_clock and add a fix to return this rate for STM32_CLOCK_BUS_AHB3 as well as STM32_CLOCK_BUS_APB3 in the switch case below?

I confirm no APB3 dedicated prescaler,
variable apb3_clock, is good to me as this is the clock we want to compute.
Maybe add a comment to say HCLK3=APBCLK3 to avoid thinking there is a typo.

Yeah that would work, but looking more into it I found another inconsistency: the current logic uses SystemCoreClock for STM32_CLOCK_BUS_AHB3, but that's incorrect for WL series. So I did this:

renamed the variable to ahb3_clock (from AHB3 prescaler, so no confusion here)

defined out the default STM32_CLOCK_BUS_AHB3 for the WL

added STM32_CLOCK_BUS_AHB3 and STM32_CLOCK_BUS_APB3 to return ahb3_clock, with a comment

How does that look?

Thanks, yes that is how I'd prefer it. Now only the issue about the SystemCoreClock raised by @ABOSTM should be left.

str4t0m

A few suggested changes.
As I can comment only on changed lines, additional comments below:

init function:

add stmwl to wait LL_ADC_DELAY_CALIB_ENABLE_ADC_CYCLES (around lines 712)
need to wait for adrdy flag (around lines 733)

str4t0m · 2021-05-06T08:43:50Z

drivers/adc/adc_stm32.c

@@ -339,7 +342,8 @@ static int start_read(const struct device *dev,
 #endif

 #if defined(CONFIG_SOC_SERIES_STM32F0X) || \
-	defined(CONFIG_SOC_SERIES_STM32L0X)
+	defined(CONFIG_SOC_SERIES_STM32L0X) || \
+	defined(CONFIG_SOC_SERIES_STM32WLX)


Here you should wait until ccrdy flag is set like stm32g0

str4t0m · 2021-05-06T09:09:16Z

drivers/adc/adc_stm32.c

 	LL_ADC_EnableInternalRegulator(adc);
 	k_busy_wait(LL_ADC_DELAY_INTERNAL_REGUL_STAB_US);
 #endif

 #if defined(CONFIG_SOC_SERIES_STM32F0X) || \
-	defined(CONFIG_SOC_SERIES_STM32L0X)
+	defined(CONFIG_SOC_SERIES_STM32L0X) || \
+	defined(CONFIG_SOC_SERIES_STM32WLX)


Here you could consider doing it the same way wb, g4,.. instead.

LL_ADC_SetCommonClock with PCLK_DIV4? If I do that it does not initialize, stays spinning on while (LL_ADC_IsCalibrationOnGoing(adc)) {}

fabiobaltieri · 2021-05-06T23:22:36Z

A few suggested changes.
As I can comment only on changed lines, additional comments below:

init function:
* add stmwl to wait LL_ADC_DELAY_CALIB_ENABLE_ADC_CYCLES (around lines 712)

done

* need to wait for adrdy flag (around lines 733)

done

erwango

Thanks for this work.

Have you been able to validate all these additions (at least one instance per driver) ?

ABOSTM

LGTM

ABOSTM · 2021-05-07T09:22:37Z

drivers/clock_control/clock_stm32_ll_common.c

@@ -232,6 +242,9 @@ static int stm32_clock_control_get_subsys_rate(const struct device *clock,
 	!defined (CONFIG_SOC_SERIES_STM32G0X)
 	uint32_t apb2_clock = get_bus_clock(ahb_clock, STM32_APB2_PRESCALER);
 #endif
+#if defined(CONFIG_SOC_SERIES_STM32WLX)
+	uint32_t apb3_clock = get_bus_clock(ahb_clock, STM32_AHB3_PRESCALER);


I confirm no APB3 dedicated prescaler,
variable apb3_clock, is good to me as this is the clock we want to compute.
Maybe add a comment to say HCLK3=APBCLK3 to avoid thinking there is a typo.

fabiobaltieri · 2021-05-07T13:34:57Z

Thanks for this work.

Have you been able to validate all these additions (at least one instance per driver) ?

Yeah I managed to test most things with a bit of hacking of the nucleo WL55 dts (but running on the Seed LoRa-E5 module I have), this what I tested:

rtc with the alarm sample
iwdg and wwdg with the watchdog sample
usart1 & usart2 (the module I have actually uses usart1 on the connector, not the lpuart)
subghzpi (sort of, tested against the NOR example, haven't actually managed to talk to the radio yet, the CS is not on a GPIO and there's probably more to get sorted, but the interrupt fires and returns 0xff so I'd say it's working enough to validate the settings)
adc & dac (the driver samples, the dac works and adc output moves when I ground the pin)
timers1 (the fade_led PWM sample)

haven't checked the other timers, can try move the led pwm sample to those if I have the pin available (it's a small module) but they should be alright.

aurel32 · 2021-05-07T13:39:31Z

* subghzpi (sort of, tested against the NOR example, haven't actually managed to talk to the radio yet, the CS is not on a GPIO and there's probably more to get sorted, but the interrupt fires and returns 0xff so I'd say it's working enough to validate the settings)

Not necessarily relevant for this PR, but for getting the radio module working, you might want to check if CONFIG_SPI_STM32_USE_HW_SS helps here.

fabiobaltieri · 2021-05-07T13:59:13Z

Not necessarily relevant for this PR, but for getting the radio module working, you might want to check if CONFIG_SPI_STM32_USE_HW_SS helps here.

Hey thanks, I'll look into it. So far I've seen it driven from a special bit in the PWR module so that may need some special treatment, no mention about the the SPI HW SS but worth trying.

About the other devices, I've intentionally omitted the TRNG from this PR. Gave it a try but could not get it working, it gets stuck spinning in the interrupt handler with the "invalid clock" bit set. I'll look into troubleshoot it again at some stage and send a separate PR if I can get it to work.

drivers/clock_control/clock_stm32_ll_common.c

erwango · 2021-05-07T15:21:09Z

Yeah I managed to test most things with a bit of hacking of the nucleo WL55 dts (but running on the Seed LoRa-E5 module I have), this what I tested:

Great, what would be nice is to add support directly to in tree nucleo board so that drivers could be compiled/ validated in CI.
Maybe @ABOSTM could help on this task.

ABOSTM

LGTM

Add a few device nodes present in the stm32wl family. Signed-off-by: Fabio Baltieri <fabio.baltieri@gmail.com>

Add RTC counter support for the STM32WL family. Signed-off-by: Fabio Baltieri <fabio.baltieri@gmail.com>

Add ADC support for the STM32WL family, this seems to work following most of the L0X code path. Signed-off-by: Fabio Baltieri <fabio.baltieri@gmail.com>

STM32WL series have an extra APB3 bus with the SUBGHZSPI device on it. Add the relevant code to enable and disable that clock, and to obtain the actual clock rate. This is enough to run the STM32 SPI driver against it. Signed-off-by: Fabio Baltieri <fabio.baltieri@gmail.com>

fabiobaltieri requested review from cybertale, erwango and nordic-krch as code owners May 1, 2021 14:21

github-actions bot added area: ADC Analog-to-Digital Converter (ADC) area: Counter area: Devicetree platform: STM32 ST Micro STM32 labels May 1, 2021

str4t0m reviewed May 3, 2021

View reviewed changes

dts/arm/st/wl/stm32wl.dtsi Outdated Show resolved Hide resolved

erwango requested a review from ABOSTM May 3, 2021 08:07

fabiobaltieri force-pushed the wl-dts branch from 45feb1a to fa87658 Compare May 3, 2021 10:06

zephyrbot requested review from anangl and FRASTM May 3, 2021 15:05

zephyrbot assigned erwango May 3, 2021

fabiobaltieri force-pushed the wl-dts branch from fa87658 to 64893f3 Compare May 3, 2021 15:45

fabiobaltieri force-pushed the wl-dts branch 2 times, most recently from fe23c98 to c041959 Compare May 4, 2021 13:25

FRASTM reviewed May 5, 2021

View reviewed changes

fabiobaltieri force-pushed the wl-dts branch from c041959 to 06aa9c1 Compare May 5, 2021 22:28

str4t0m reviewed May 6, 2021

View reviewed changes

str4t0m requested changes May 6, 2021

View reviewed changes

fabiobaltieri force-pushed the wl-dts branch from 06aa9c1 to 7b4c20a Compare May 6, 2021 23:26

erwango reviewed May 7, 2021

View reviewed changes

ABOSTM approved these changes May 7, 2021

View reviewed changes

FRASTM approved these changes May 7, 2021

View reviewed changes

fabiobaltieri force-pushed the wl-dts branch from 7b4c20a to 33b49c8 Compare May 7, 2021 13:24

ABOSTM requested changes May 7, 2021

View reviewed changes

drivers/clock_control/clock_stm32_ll_common.c Outdated Show resolved Hide resolved

fabiobaltieri force-pushed the wl-dts branch from 33b49c8 to 1d43d11 Compare May 7, 2021 15:42

ABOSTM approved these changes May 7, 2021

View reviewed changes

erwango added this to the v2.7.0 milestone May 10, 2021

str4t0m mentioned this pull request May 10, 2021

Add additional STM32WL SOCS and Seeed Studio LoRa-E5 Dev board #35037

Merged

galak requested a review from erwango June 4, 2021 21:14

galak force-pushed the wl-dts branch from 1d43d11 to 8fab7f0 Compare June 4, 2021 21:32

fabiobaltieri added 4 commits June 6, 2021 22:20

dts: stm32wl: add various device nodes

cc753c9

Add a few device nodes present in the stm32wl family. Signed-off-by: Fabio Baltieri <fabio.baltieri@gmail.com>

drivers: stm32_rtc: add STM32WL family support

5868b31

Add RTC counter support for the STM32WL family. Signed-off-by: Fabio Baltieri <fabio.baltieri@gmail.com>

drivers: stm32_adc: add STM32WL family support

bf1cfb4

Add ADC support for the STM32WL family, this seems to work following most of the L0X code path. Signed-off-by: Fabio Baltieri <fabio.baltieri@gmail.com>

fabiobaltieri force-pushed the wl-dts branch from 8fab7f0 to c1950f3 Compare June 6, 2021 21:21

carlescufi merged commit 03adc1e into zephyrproject-rtos:main Jun 7, 2021

fabiobaltieri deleted the wl-dts branch June 7, 2021 12:18

erwango mentioned this pull request Feb 24, 2022

STM32WL ADC idles / doesn't work #43130

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add few devices to the STM32WL family #34751

Add few devices to the STM32WL family #34751

fabiobaltieri commented May 1, 2021

str4t0m left a comment

fabiobaltieri commented May 3, 2021

FRASTM May 5, 2021

fabiobaltieri May 5, 2021

fabiobaltieri May 5, 2021 •

edited

Loading

FRASTM May 6, 2021

fabiobaltieri May 6, 2021

FRASTM May 7, 2021

str4t0m May 6, 2021

fabiobaltieri May 6, 2021

erwango May 7, 2021

str4t0m May 7, 2021

ABOSTM May 7, 2021

fabiobaltieri May 7, 2021

str4t0m May 7, 2021

str4t0m left a comment

str4t0m May 6, 2021

fabiobaltieri May 6, 2021

str4t0m May 6, 2021

fabiobaltieri May 6, 2021

fabiobaltieri commented May 6, 2021

erwango left a comment

ABOSTM left a comment

ABOSTM May 7, 2021

fabiobaltieri commented May 7, 2021

aurel32 commented May 7, 2021

fabiobaltieri commented May 7, 2021

erwango commented May 7, 2021

ABOSTM left a comment

Add few devices to the STM32WL family #34751

Add few devices to the STM32WL family #34751

Conversation

fabiobaltieri commented May 1, 2021

str4t0m left a comment

Choose a reason for hiding this comment

fabiobaltieri commented May 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiobaltieri May 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

str4t0m left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiobaltieri commented May 6, 2021

erwango left a comment

Choose a reason for hiding this comment

ABOSTM left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiobaltieri commented May 7, 2021

aurel32 commented May 7, 2021

fabiobaltieri commented May 7, 2021

erwango commented May 7, 2021

ABOSTM left a comment

Choose a reason for hiding this comment

fabiobaltieri May 5, 2021 •

edited

Loading