drivers: pwm: add STM32 LPTIM PWM driver #101205
Conversation
zephyrbot
added
area: LED
|
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
github-actions
bot
added
the
DNM (manifest)
zephyrbot
requested review from
FRASTM,
GeorgeCGV,
JarmouniA,
anangl,
djiatsaf-st,
erwango,
etienne-lms,
gautierg-st,
henrikbrixandersen,
jeppenodgaard,
juickar,
kartben,
mathieuchopstm,
nashif and
simonguinot
tpambor
force-pushed
the
lptim-pwm-stm32
branch
3 times, most recently
from
85c2d2a to
382711e
Compare
gautierg-st
left a comment
gautierg-st
left a comment
There was a problem hiding this comment.
Thanks for this. Driver side looks ok to me.
As I said in one comment, a pwm child node should be added in the lptim nodes of the dtsi, like it is done for the timers nodes. This will require some minor changes to the driver and the overlay.
And make that 3 commits:
- Driver
- Dtsi update
- Tests/samples
| return 0; | ||
| } | ||
|
|
||
| #define PWM(index) DT_DRV_INST(index) |
There was a problem hiding this comment.
Ideally, LPTIM PWM driver should be as close as possible to TIM PWM driver. So there should be a pwm child node to the lptim instances in the dtsi files.
And this line, like it's done in the TIM PWM driver, should be
| #define PWM(index) DT_DRV_INST(index) | |
| #define PWM(index) DT_INST_PARENT(index) |
There was a problem hiding this comment.
I generally agree, but this would require major refactoring and introduce breaking changes. The current top-level st,stm32-lptim binding is tailored specifically for using LPTIM as a system tick source. Therefore, I see the following steps:
- Introduce a new top-level binding:
st,stm32-lptimers, essentially based onst,stm32-lptimbut without the tick-source-specificst,timeoutproperty. - Add a new binding for tick source functionality and include it as a subnode within LPTIM instances.
- Update the current LPTIM tick source driver to align with the new structure.
- Update samples and tests to reflect these changes.
- Update the binding of this PWM driver so that it can be added as a subnode to LPTIM instances that support PWM.
- Update this PWM driver
All users using LPTIM as systick source would then need to update their device tree to adapt to these changes.
There was a problem hiding this comment.
You're right, I didn't have the full picture in mind.
Even though it's a lot more work, I still think it's a better way to go. Changing the compatible value of a node doesn't seem like a good idea for readability and maintenance.
Besides, it's better if the dtsi can describe all available features.
And this could also be used to add support for other potential features of the LPTIM in the future.
But before diving into it, let's have other opinions: @erwango, @etienne-lms ?
There was a problem hiding this comment.
Introduce a new top-level binding: st,stm32-lptimers, essentially based on st,stm32-lptim but without the tick-source-specific st,timeout property.
Why is that? I must be missing something, the property is not required, so just don't use it in the PWM driver.
Add a PWM driver for the STM32 low-power timer (LPTIM) peripheral on STM32 MCUs. Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
Add overlay and update sample.yaml to include stm32h573i_dk board. This board can utilize LPTIM5 for PWM generation to drive the red (LD3) LED. Signed-off-by: Tim Pambor <tim.pambor@codewrights.de>
tpambor
force-pushed
the
lptim-pwm-stm32
branch
from
382711e to
39f0c3e
Compare
github-actions
bot
removed
manifest
manifest-hal_stm32
DNM (manifest)
|
|
|
||
| &lptim5 { | ||
| compatible = "st,stm32-lptim-pwm"; | ||
| clocks = <&rcc STM32_CLOCK(APB3, 14)>, |
There was a problem hiding this comment.
I see no point in inventing another compatible for the same hw device (
zephyr/dts/arm/st/h5/stm32h562.dtsi
Line 106 in 72360f8
Add a child PWM node and use it with the PWM driver.
6 participants
This PR introduces a PWM driver for the STM32 low-power timer (LPTIM) peripheral on STM32 MCUs.
This driver is loosely based on the PWM driver for standard timers on STM32. For maintainability and readability, I split this into a new driver instead of integrating with the existing STM PWM driver, as the supported features differ and LPTIMs use a different low-level API
LL_LPTIM_*.Fixes #57316.