Re: [PATCH v3 2/2] pwm: Add support for pwm nexus dt bindings

[Herve Codina]

Hi Uwe,

On Wed, 5 Feb 2025 12:38:32 +0100
Uwe Kleine-König <ukleinek@xxxxxxxxxx> wrote:

> Hello,
> 
> I really like this mechanism. Assuming the dt guys are happy yet, I
> intend to merge it. Just some detail question below.
> 
> On Wed, Feb 05, 2025 at 10:55:43AM +0100, Herve Codina wrote:
> > Nexus node support in PWM allows the following description:
> > 	soc {
> > 		soc_pwm1: pwm-controller1 {
> > 			#pwm-cells = <3>;
> > 		};
> > 
> > 		soc_pwm2: pwm-controller2 {
> > 			#pwm-cells = <3>;
> > 		};
> > 	};
> > 
> > 	connector: connector {
> > 		#pwm-cells = <3>;
> > 		pwm-map = <0 0 0 &soc_pwm1 1 0 0>,
> > 			  <1 0 0 &soc_pwm2 4 0 0>,
> > 			  <2 0 0 &soc_pwm1 3 0 0>;
> > 		pwm-map-mask = <0xffffffff 0x0 0x0>;
> > 		pwm-map-pass-thru = <0x0 0xffffffff 0xffffffff>;
> > 	};
> > 
> > 	expansion_device {
> > 		pwms = <&connector 1 57000 0>;
> > 	};  
> 
> Does this also work if &soc_pwm2 has #pwm-cells = <2>? Would I need just
> 
> 	pwm-map = <0 0 0 &soc_pwm1 1 0 0>,
> 		  <1 0 0 &soc_pwm2 4 0>,
> 		  <2 0 0 &soc_pwm1 3 0 0>;

Yes, exactly.

> 
> then and
> 
> 	pwms = <&connector 1 57000 0>;
> 
> would then have the same effect as
> 
> 	pwms = <&soc_pwm2 4 57000>

Yes, the last 0 (or any other values) in pwms = <&connector 1 57000 0> is
simply dropped in the translation (#pwm-cells = 3 in connector nexus to
#pwm-cells = 2 in soc_pwm1 node).

In more generic terms, it works in translation from #pwm-cells = N to
#pwm-cells = M by simply dropping the last N-M values.

Also note that even if values are dropped, you need to have them set when
you point the nexus node because #pwm-cells = 3 is set in the connector
node and need to be fixed and usable for all the entries in the
pwm-map table.

> 
> and the 0 is dropped then? Could I adapt the mapping that the effect is
> 
> 	pwms = <&soc_pwm2 57000 0>

In this one, I think you miss the PWM number
If I read correctly this line you ask for the PWM 57000 from the soc_pwm2
controller. This doesn't make sense :)

If I didn't answer already, can you clarify your point here ?
> 
> instead?


> 
> This smells a bit ugly and I wonder if this gives a motivation to extend
> the binding for PWMs that use #pwm-cells = <2> (or less) to also accept
> the default 3-cell binding.

With my understanding in the translation performed, this is not needed.

Also the following translation works:

	soc {
 		soc_pwm1: pwm-controller1 {
 			#pwm-cells = <2>;
		};

		soc_pwm2: pwm-controller2 {
			#pwm-cells = <3>;
		};
 	};

	connector: connector {
		#pwm-cells = <2>;   <--------- Note the 2 here
 		pwm-map = <0 0 &soc_pwm1 1 0>,
 			  <1 0 &soc_pwm2 4 0 123>,
 		pwm-map-mask = <0xffffffff 0x0>;   <---- #pwm-cells = <2> 
 		pwm-map-pass-thru = <0x0 0xffffffff>; <---- #pwm-cells = <2>
 	};

pwms = <&connector 1 57000> translates to pwms = <&soc_pwm2 1 57000 123>

The last value in the translation (i.e 123) was added during the translation
(#pwm-cells = <2> to #pwm-cells = <3>) from the value set in the pwm-map
table.

Hope I answered your questions.

Best regards,
Hervé