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

[Herve Codina]

Hi Uwe,

On Wed, 5 Feb 2025 17:29:30 +0100
Uwe Kleine-König <ukleinek@xxxxxxxxxx> wrote:

> Hello Hervé,
> 
> On Wed, Feb 05, 2025 at 02:37:37PM +0100, Herve Codina wrote:
> > On Wed, 5 Feb 2025 12:38:32 +0100
> > Uwe Kleine-König <ukleinek@xxxxxxxxxx> wrote:  
> > > 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.  
> 
> Makes sense.
> 
> > > 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 :)  
> 
> Some pwm chip devices with only a single output line use this. The first
> paramter is the default period (which is passed in the 2nd parameter
> normally) and the 2nd paramter are flags (normally the 3rd parameter).
> Back then the rationale was that for such hardware, the line index is
> zero always anyhow, and so could better be skipped.
> 
> Compare of_pwm_xlate_with_flags() to of_pwm_single_xlate(). pwm-pxa is
> the single offender using the latter. Thinking about that, it's easy
> enough to fix without breaking compatibility. I'll tackle that.
> 
> So for a PWM on pxa `<&soc_pwm2 57000 0>` works fine.

I see. In this case, a parameter shift during translation would be needed to
skip the PWM line index in the translated arguments. This is not currently
neither described in device-tree specicication [0] nor handled in the common
code of_parse_phandle_with_args_map() in the kernel.

This use case can appear for resources other than PWMs and IMHO it should be
nice to have it supported.

I think this support should proposed out of this series adding support for
PWM nexus nodes.

Is it blocking for this current series ?

[0] https://github.com/devicetree-org/devicetree-specification/releases/download/v0.4/devicetree-specification-v0.4.pdf

Best regards,
Hervé