| Message ID | 20231106095205.231210-2-alexander.stein@ew.tq-group.com |
|---|---|
| State | New |
| Headers | show |
| Series | imx8qm/imx8qxp: Support for inverted PWM | expand |
On Mon, Nov 06, 2023 at 11:49:19AM +0100, Alexander Stein wrote: > Hello Uwe, > > Am Montag, 6. November 2023, 11:36:21 CET schrieb Uwe Kleine-König: > > Hello, > > > > [dropped Philippe Schenker from Cc as his email bounced in the past] > > > > On Mon, Nov 06, 2023 at 10:52:03AM +0100, Alexander Stein wrote: > > > Only fsl,imx1-pwm comptabile devices use #pwm-cells = <2>. Newer SoCs > > > supportinverted PWM output, thus #pwm-cells needs to be set to 3. > > > > > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > > > --- > > > > > > .../devicetree/bindings/pwm/imx-pwm.yaml | 20 ++++++++++++++++--- > > > 1 file changed, 17 insertions(+), 3 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.yaml > > > b/Documentation/devicetree/bindings/pwm/imx-pwm.yaml index > > > c01dff3b7f843..59a981c0f39ab 100644 > > > --- a/Documentation/devicetree/bindings/pwm/imx-pwm.yaml > > > +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.yaml > > > @@ -9,9 +9,6 @@ title: Freescale i.MX PWM controller > > > > > > maintainers: > > > - Philipp Zabel <p.zabel@pengutronix.de> > > > > > > -allOf: > > > - - $ref: pwm.yaml# > > > - > > > > > > properties: > > > "#pwm-cells": > > > description: | > > > > > > @@ -74,6 +71,23 @@ required: > > > additionalProperties: false > > > > > > +allOf: > > > + - $ref: pwm.yaml# > > > + > > > + - if: > > > + properties: > > > + compatible: > > > + contains: > > > + const: fsl,imx1-pwm > > > + then: > > > + properties: > > > + "#pwm-cells": > > > + enum: [2] > > > > Given that arch/arm/boot/dts/nxp/imx/imx1.dtsi has: > > > > pwm: pwm@208000 { > > #pwm-cells = <3>; > > compatible = "fsl,imx1-pwm"; > > ... > > > > this looks wrong. > > Indeed, something i doesn't match. Checking with [1] section 22.4.1 there are > no bits regarding output inversion. Also pwm_imx1_apply returns -EINVAL if > state->polarity != PWM_POLARITY_NORMAL. > So IMO "#pwm-cells = <3>" is wrong for imx1. If fixed to 2, this also matches > the description for the value of #pwm-cells in imx-pwm.yaml. The reasoning only works in one direction, i.e. if your PWM supports inversed polarity you need #pwm-cells = <3>. But there no problem in using 3 cells for a PWM that doesn't support inversed output. Some might call it unnecessary verboseness, other might call it consistency. I'd say the right thing to do here is to stick to = 3 and adapt this patch and the already existing comment in imx-pwm.yaml. Best regards Uwe
On Mon, Nov 06, 2023 at 10:52:03AM +0100, Alexander Stein wrote: > Only fsl,imx1-pwm comptabile devices use #pwm-cells = <2>. Newer SoCs > supportinverted PWM output, thus #pwm-cells needs to be set to 3. > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> As suggested by Uwe, I picked this up too. Shawn
diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.yaml b/Documentation/devicetree/bindings/pwm/imx-pwm.yaml index c01dff3b7f843..59a981c0f39ab 100644 --- a/Documentation/devicetree/bindings/pwm/imx-pwm.yaml +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.yaml @@ -9,9 +9,6 @@ title: Freescale i.MX PWM controller maintainers: - Philipp Zabel <p.zabel@pengutronix.de> -allOf: - - $ref: pwm.yaml# - properties: "#pwm-cells": description: | @@ -74,6 +71,23 @@ required: additionalProperties: false +allOf: + - $ref: pwm.yaml# + + - if: + properties: + compatible: + contains: + const: fsl,imx1-pwm + then: + properties: + "#pwm-cells": + enum: [2] + else: + properties: + "#pwm-cells": + enum: [3] + examples: - | #include <dt-bindings/clock/imx5-clock.h>
Only fsl,imx1-pwm comptabile devices use #pwm-cells = <2>. Newer SoCs supportinverted PWM output, thus #pwm-cells needs to be set to 3. Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> --- .../devicetree/bindings/pwm/imx-pwm.yaml | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)