2020-04-05 19:24:02

by Oleksandr Suvorov

[permalink] [raw]
Subject: [RFC PATCH v2 2/6] dt-bindings: pwm: document the PWM no-flag

Add the description of PWM_NOFLAGS flag property.

Signed-off-by: Oleksandr Suvorov <[email protected]>
---

Changes in v2:
- add the example of PWM_NOFLAGS usage;
- Use PWM_NOFLAGS instead of PWM_POLARITY_NORMAL to avoid possible
conflict with enum pwm_polarity in <linux/pwm.h>. Also, this name
reflects the sense of the original value more precisely.

Documentation/devicetree/bindings/pwm/pwm.txt | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt b/Documentation/devicetree/bindings/pwm/pwm.txt
index 084886bd721e..7f0bbda127c4 100644
--- a/Documentation/devicetree/bindings/pwm/pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm.txt
@@ -45,7 +45,8 @@ period in nanoseconds.

Optionally, the pwm-specifier can encode a number of flags (defined in
<dt-bindings/pwm/pwm.h>) in a third cell:
-- PWM_POLARITY_INVERTED: invert the PWM signal polarity
+- PWM_NOFLAGS: no modification flags.
+- PWM_POLARITY_INVERTED: invert the PWM signal polarity.

Example with optional PWM specifier for inverse polarity

@@ -54,6 +55,14 @@ Example with optional PWM specifier for inverse polarity
pwm-names = "backlight";
};

+ vibrator {
+ [...]
+ pwms = <&pwm1 0 10000000 PWM_POLARITY_INVERTED>,
+ <&pwm2 0 10000000 PWM_NOFLAGS>;
+ pwm-names = "enable", "direction";
+ [...]
+ };
+
2) PWM controller nodes
-----------------------

--
2.24.1


2020-04-07 06:18:47

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [RFC PATCH v2 2/6] dt-bindings: pwm: document the PWM no-flag

On Sun, Apr 05, 2020 at 10:22:42PM +0300, Oleksandr Suvorov wrote:
> Add the description of PWM_NOFLAGS flag property.
>
> Signed-off-by: Oleksandr Suvorov <[email protected]>

As I already wrote in reply to the v1 series I'd prefer a name for 0
that explicitly handles normal polarity.

Best regards
Uwe

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

2020-04-07 10:52:55

by Oleksandr Suvorov

[permalink] [raw]
Subject: Re: [RFC PATCH v2 2/6] dt-bindings: pwm: document the PWM no-flag

On Tue, Apr 7, 2020 at 9:17 AM Uwe Kleine-König
<[email protected]> wrote:
>
> On Sun, Apr 05, 2020 at 10:22:42PM +0300, Oleksandr Suvorov wrote:
> > Add the description of PWM_NOFLAGS flag property.
> >
> > Signed-off-by: Oleksandr Suvorov <[email protected]>
>
> As I already wrote in reply to the v1 series I'd prefer a name for 0
> that explicitly handles normal polarity.

Uwe, AFAIU, there is no flag that forces normal polarity, the normal polarity
is the default state if there is no flag to invert the polarity is set.
'0' value in the bit flags cell really means there are no flags set
for the PWM instance.

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



--
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00

2020-04-07 11:20:36

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [RFC PATCH v2 2/6] dt-bindings: pwm: document the PWM no-flag

On Tue, Apr 07, 2020 at 01:51:42PM +0300, Oleksandr Suvorov wrote:
> On Tue, Apr 7, 2020 at 9:17 AM Uwe Kleine-K?nig
> <[email protected]> wrote:
> >
> > On Sun, Apr 05, 2020 at 10:22:42PM +0300, Oleksandr Suvorov wrote:
> > > Add the description of PWM_NOFLAGS flag property.
> > >
> > > Signed-off-by: Oleksandr Suvorov <[email protected]>
> >
> > As I already wrote in reply to the v1 series I'd prefer a name for 0
> > that explicitly handles normal polarity.
>
> Uwe, AFAIU, there is no flag that forces normal polarity, the normal polarity
> is the default state if there is no flag to invert the polarity is set.

Yes, that's the status quo.

> '0' value in the bit flags cell really means there are no flags set
> for the PWM instance.

For me the relevance of giving 0 a name is mostly for human consumption.
Currently there is only a single flag encoded in the number in question.
But as soon as we add another, say PWM_AUTOSTART we have the following
possible settings:

PWM_NOFLAGS
PWM_POLARITY_INVERTED
PWM_AUTOSTART
PWM_POLARITY_INVERTED | PWM_AUTOSTART

Then for the first two a reader doesn't see if autostart is not in use
because the dt author doesn't know this feature (e.g. because autostart
is too new) or if they don't want autostart at all.

If however we had PWM_POLARITY_NORMAL and PWM_NO_AUTOSTART to complement
PWM_POLARITY_INVERTED and PWM_AUTOSTART every flag's setting could be
explicit and if there is a device tree that only has

PWM_POLARITY_NORMAL

it would be obvious that nobody thought enough about autostarting to
explicitly mention it.

Best regards
Uwe

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

2020-04-07 15:05:53

by Oleksandr Suvorov

[permalink] [raw]
Subject: Re: [RFC PATCH v2 2/6] dt-bindings: pwm: document the PWM no-flag

On Tue, Apr 7, 2020 at 2:19 PM Uwe Kleine-König
<[email protected]> wrote:
>
> On Tue, Apr 07, 2020 at 01:51:42PM +0300, Oleksandr Suvorov wrote:
> > On Tue, Apr 7, 2020 at 9:17 AM Uwe Kleine-König
> > <[email protected]> wrote:
> > >
> > > On Sun, Apr 05, 2020 at 10:22:42PM +0300, Oleksandr Suvorov wrote:
> > > > Add the description of PWM_NOFLAGS flag property.
> > > >
> > > > Signed-off-by: Oleksandr Suvorov <[email protected]>
> > >
> > > As I already wrote in reply to the v1 series I'd prefer a name for 0
> > > that explicitly handles normal polarity.
> >
> > Uwe, AFAIU, there is no flag that forces normal polarity, the normal polarity
> > is the default state if there is no flag to invert the polarity is set.
>
> Yes, that's the status quo.
>
> > '0' value in the bit flags cell really means there are no flags set
> > for the PWM instance.
>
> For me the relevance of giving 0 a name is mostly for human consumption.
> Currently there is only a single flag encoded in the number in question.
> But as soon as we add another, say PWM_AUTOSTART we have the following
> possible settings:
>
> PWM_NOFLAGS
> PWM_POLARITY_INVERTED
> PWM_AUTOSTART
> PWM_POLARITY_INVERTED | PWM_AUTOSTART
>
> Then for the first two a reader doesn't see if autostart is not in use
> because the dt author doesn't know this feature (e.g. because autostart
> is too new) or if they don't want autostart at all.
>
> If however we had PWM_POLARITY_NORMAL and PWM_NO_AUTOSTART to complement
> PWM_POLARITY_INVERTED and PWM_AUTOSTART

So using this approach, in theory, we'll have several flags that all
just equals to 0 (0 << 0, 0 << 1, 0 << 2 ...).
What if just describe default states for each flag in the DT documentation?

> every flag's setting could be explicit and if there is a device tree that only has
>
> PWM_POLARITY_NORMAL
>
> it would be obvious that nobody thought enough about autostarting to
> explicitly mention it.

If you insist on the flag complement model, I have another suggestion.
As the normal polarity is the default state, can we use PWM_NO_POLARITY_INVERTED
instead of PWM_POLARITY_NORMAL?
It gives us 2 benefits:
1. The name will not interfere with enum PWM_POLARITY_NORMAL in <linux/pmw.h>
2. Each flag complement will be made with the same scheme:
PWM_flagA -> PWM_NO_flagA...

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

--
Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00