pinctrl: npcx: config pwm open-drain without enabling STORE_REG #45712
Conversation
github-actions
bot
added
area: Devicetree
area: Devicetree Binding
keith-zephyr
left a comment
keith-zephyr
left a comment
There was a problem hiding this comment.
Because the open-drain support is limited to the PWM channels, I'm good with this approach and I like the saving from disabling CONFIG_PINCTRL_STORE_REG
There was a problem hiding this comment.
The PWM nodes should set the channel property, correct?
There was a problem hiding this comment.
Yes, drive-supported is useless now. Helper macro checks whether channel property exists. If so, the driver will create an item for pwm drive mode configuration automatically.
dts/arm/nuvoton/npcx.dtsi
Outdated
There was a problem hiding this comment.
note: channel seems to be computable like this:
channel = (my_pwm_addr - DT_REG_ADDR(DT_NODELABEL(pwm0))) >> 13U;
(should potentially allow you to save some ROM)
There was a problem hiding this comment.
From a saving ROM usage point of view, adding a new property in DT node has no harm. There are two benefits to add a channel property.
- Helper macro will check the peripheral node is pwm device node or not. It prevents the unexpected behavior if users enable
open-drainproperty in the wrong pinctrl node. - Currently, the offset for each pwm module is 0x2000. But I cannot make sure it won't be broken if designers run out of memory space. (For example, the new pwm register address is overlapped within the other existed module.) It happened before.
|
Hi @keith-zephyr and @gmarull, this is the other attempt to config pwm open-drain mode in npcx pinctrl driver. This approach saves more ROM usage. Any comment is appreciated. |
Config pwm open-drain mode without enabling STORE_REG. This CL collects all active PWM's base address and related index in an array. Then, pinctrl driver configs its open-drain mode by finding the corresponding 'channel' index. Signed-off-by: Mulin Chao <[email protected]>
|
@MulinChao since we're in stabilization period for 3.1 and this is resolving an issue, please file a bug report so that it can be merged without the need for an exception. |
Hi @gmarull, I have created an issue 45795 for merging this PR during the stabilization period. Thanks for sharing this information. |
MaureenHelm
left a comment
MaureenHelm
left a comment
There was a problem hiding this comment.
The twister failure looks unrelated; please try rebasing to resolve it.
Please also add "Fixes #45795" to the PR summary to automatically close the bug when the PR gets merged.
|
Hey I've rerun that step manually and somehow it passed now. |
Done. Thanks for reminding. |
6 participants
Config pwm open-drain mode without enabling STORE_REG. This CL
collects all active PWM's base address and related index in an
array. Then, pinctrl driver configs its open-drain mode by
finding the corresponding 'channel' index.
Signed-off-by: Mulin Chao [email protected]
Fixes #45795