Hello Andy,

On Mon, Jun 07, 2021 at 02:49:01PM +0300, Andy Shevchenko wrote:

On Mon, Jun 07, 2021 at 01:21:01PM +0300, Andy Shevchenko wrote:

quoted

On Mon, Jun 07, 2021 at 01:15:20PM +0300, Andy Shevchenko wrote:

quoted

On Mon, Jun 07, 2021 at 11:53:24AM +0200, Uwe Kleine-König wrote:

quoted

On Mon, Jun 07, 2021 at 12:02:37PM +0300, Andy Shevchenko wrote:

quoted

On Sun, Jun 06, 2021 at 11:30:54PM +0200, Uwe Kleine-König wrote:

quoted

On Mon, May 31, 2021 at 10:49:42PM +0300, Andy Shevchenko wrote:

quoted

It makes little sense to make PWM flags optional since in case
of multi-channel consumer the flags can be optional only for
the last listed channel.

I think the same holds true for dt references.

Can you elaborate this? I haven't got what you are talking about, not a DT
expert here.

Ah no, I mixed that up. While the function that parses the phandle is
flexible, for each pwm controller the number of arguments is fixed, so

	pwms = <&pwm1 100000 &pwm2 100000 &pwm3 1000000>;

cannot be interpreted as 3-argument references to two PWMs. This is
different to ACPI (I guess, not an ACPI expert here :-) because &pwm1
"knows" if it needs 1 or 2 additional parameters (#pwm-cells).

It's not about ACPI, it's about "the ACPI glue layer in Linux kernel".
Used API is a part of it and it does allow only two cases, either NULL entry
(by having 0 as an argument) or full-length supplied tuple (in case of PWM it's
3, so, means 4 parameters.

Let's consider examples:

(0, 0, x3, y3, z3, t3) // NULL, NULL, PWM3
(x1, y1, z1, t1, 0, x3, y3, z3, t3) // PWM1, NULL, PWM3

So, making last parameter "flexible" will work only for the last tuple in the
array.

Read this [1] for further information.

[1]: https://elixir.bootlin.com/linux/latest/source/drivers/acpi/property.c#L629

Hmm... I have read the actual implementation and it seems it's possible to have
flexible array, so this patch needs to be reconsidered.

I was thinking more about it and what we have here is positional-dependent
arguments. Either way we might end up in the same situation (when we need to
parse arguments based on their positions, rather than always have them being
present). So, while I won't change documentation example (to be more stricter
there), I will drop this change.

Also, the PWM initial state doesn't include duty cycle. Any explanations why is
that?

This isn't technically the initial state. It's a hint to the consumer
which period to pick. The duty-cycle is usually variable, but if I
designed the binding today I would not include the period in the pwm
handle. But to discuss this is moot---the binding is as it is.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |