linux-kernel - New default binding for PWM devices? [Was: Re: [PATCH] dt-bindings: timer: xlnx,xps-timer: Make PWM in example usable]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <crk42dsypmbyqk7avldghjq32vslmalfmmouwxzgtdci4agfhz@rkbmxj5z22fx>
Date: Tue, 3 Jun 2025 19:41:28 +0200
From: Uwe Kleine-König <u.kleine-koenig@...libre.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, Rob Herring <robh@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>
Cc: Daniel Lezcano <daniel.lezcano@...aro.org>, 
	Thomas Gleixner <tglx@...utronix.de>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Sean Anderson <sean.anderson@...o.com>, linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org, 
	devicetree@...r.kernel.org, Chris Packham <Chris.Packham@...iedtelesis.co.nz>
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@...libre.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@...f0000 {
-        #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

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)