Messages in this thread Patch in this message |  | | Date | Tue, 3 Jun 2025 19:41:28 +0200 | | From | Uwe Kleine-König <> | | Subject | New default binding for PWM devices? [Was: Re: [PATCH] dt-bindings: timer: xlnx,xps-timer: Make PWM in example usable] |
| |
Hello,
On Wed, May 28, 2025 at 09:43:48AM +0200, Krzysztof Kozlowski wrote: > On 27/05/2025 19:15, Uwe Kleine-König wrote: > > With #pwm-cells = <0> no usable reference to that PWM can be created. > > Even though a xlnx,xps-timer device only provides a single PWM line, Linux > > would fail to determine the right (pwmchip, pwmnumber) combination. > > > > Fix the example to use the recommended value 3 for #pwm-cells. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> > > --- > > Documentation/devicetree/bindings/timer/xlnx,xps-timer.yaml | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > And what about the binding itself? It allows any arbitrary value. > Setting it to const=3 would not break the ABI, as long as driver does > not care.
Oh indeed. Now I wonder about myself that I didn't notice that without a hint.
So with the intention to move all drivers to #pwm-cells = <3>, the patch to create here is:
diff --git a/Documentation/devicetree/bindings/timer/xlnx,xps-timer.yaml b/Documentation/devicetree/bindings/timer/xlnx,xps-timer.yaml index b1597db04263..8d7a87fb2d35 100644 --- a/Documentation/devicetree/bindings/timer/xlnx,xps-timer.yaml +++ b/Documentation/devicetree/bindings/timer/xlnx,xps-timer.yaml @@ -26,7 +26,8 @@ properties: reg: maxItems: 1 - '#pwm-cells': true + '#pwm-cells': + const: 3 xlnx,count-width: $ref: /schemas/types.yaml#/definitions/uint32 @@ -82,7 +83,7 @@ examples: }; timer@800f0000 { - #pwm-cells = <0>; + #pwm-cells = <3>; clock-names = "s_axi_aclk"; clocks = <&zynqmp_clk 71>; compatible = "xlnx,xps-timer-1.00.a"; There is however one concern that I want to get resolved first to prevent churn:
In principle I think it's bad that a phandle to a PWM must contain a period and flags specifying the polarity. For some use cases the period might not matter or is implicitly given or more than one period length is relevant.
So I wonder if instead of unifying all PWM bindings to #pwm-cells = <3> I should instead go to something like
mypwm: pwm { compatible = "...." #pwm-cells = <1>; }; fan { compatible = "pwm-fan"; pwms = <&mypwm 1>; assigned-pwms = <&mypwm>; assigned-pwm-default-period-lengths-ns = <40000>; assigned-pwm-default-flags = <PWM_POLARITY_INVERTED>; }; (where the single cell specifies the index of the PWM's output).
I already suggested that in https://lore.kernel.org/linux-pwm/jmxmxzzfyobuheqe75lj7qcq5rlt625wddb3rlhiernunjdodu@tgxghvfef4tl/. When I asked about that in #armlinux Rob said "no. We don't need a 2nd way to set period and flags." Is this still a bad idea if the traditional binding with 3 cells will be deprecated for all PWM devices? If this would be ok then, I'm also open for improvements to the new concept. Maybe something like:
fan { compatible = "pwm-fan"; pwms = <&mypwm 1>; pwm-default-period-lengths-ns = <40000>; pwm-default-flags = <PWM_POLARITY_INVERTED>; }; ?
Looking forward to your feedback.
Best regards Uwe [unhandled content-type:application/pgp-signature] |  |