drivers: pwm: introduce ti_am3352_ecap #88860
Conversation
natto1784
force-pushed
the
ecap
branch
3 times, most recently
from
d7eee0f to
90ca724
Compare
github-actions
bot
added
area: PWM
github-actions
bot
requested review from
anangl,
henrikbrixandersen,
vaishnavachath and
vanti
This patch adds driver support for TI ECAP module which can capture an external input or work as a standalone APWM generator. Signed-off-by: Amneesh Singh <[email protected]>
|
CC: @gramsay0 @Ayush1325 @glneo - Please feel free to review if and when you get some time. |
| uint32_t pulse_cycles, pwm_flags_t flags) | ||
| { | ||
| ARG_UNUSED(channel); | ||
| struct ti_ecap_regs *regs = DEV_REGS(dev); |
There was a problem hiding this comment.
Maybe this should be volatile?
I know it is not used in many other drivers, but I feel like registers should use the volatile keyword.
There was a problem hiding this comment.
Individual registers all have volatile keyword in front of them in struct ti_ecap_regs
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a new PWM driver for TI ECAP modules to support both capture and APWM operations on TI SoCs.
- Added a DTS binding file for ti,am3352-ecap.
- Implemented the driver in drivers/pwm/pwm_ti_am3352_ecap.c with capture and PWM functionalities.
- Updated Kconfig and CMakeLists.txt to integrate the new driver.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| dts/bindings/pwm/ti,am3352-ecap.yaml | Added DTS binding with required properties for the new driver. |
| drivers/pwm/pwm_ti_am3352_ecap.c | New driver implementation handling both PWM and capture modes. |
| drivers/pwm/Kconfig.ti_am3352_ecap | Config file for enabling the TI ECAP based PWM controller. |
| drivers/pwm/Kconfig | Extended to include new Kconfig file for TI ECAP driver. |
| drivers/pwm/CMakeLists.txt | Updated build file to incorporate the new driver source file. |
Comments suppressed due to low confidence (2)
drivers/pwm/pwm_ti_am3352_ecap.c:119
- Casting the pointer 'cycles' from uint64_t* to uint32_t* can potentially truncate the clock rate if it exceeds 32 bits. Consider using a temporary uint32_t variable to retrieve the clock rate, then assign it to *cycles for improved clarity and type safety.
return clock_control_get_rate(cfg->clock_dev, cfg->clock_subsys, (uint32_t *)cycles);
drivers/pwm/pwm_ti_am3352_ecap.c:253
- [nitpick] The error message would be more helpful if it included the error code or additional details from the pinctrl configuration failure to assist in debugging.
LOG_ERR("Fail to configure pinctrl\n");
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
|
not stale, needs rebasing and DT nodes however, also depends on TISCI clock controller PR |
4 participants
This patch adds driver support for TI ECAP module which can capture an external input or work as a standalone APWM generator.
Tested on AM243x EVM using loopback from EPWM (#88757)