Re: [PATCH 1/3] dt-bindings: pwm: imx-pwm: Add constraints for #pwm-cells

From: Alexander Stein <alexander.stein@ew.tq-group.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Thierry Reding <thierry.reding@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	devicetree@vger.kernel.org, linux-pwm@vger.kernel.org,
	NXP Linux Team <linux-imx@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/3] dt-bindings: pwm: imx-pwm: Add constraints for #pwm-cells
Date: Mon, 06 Nov 2023 11:49:19 +0100	[thread overview]
Message-ID: <8300676.T7Z3S40VBb@steina-w> (raw)
In-Reply-To: <20231106103621.2fh6fkpdafpz4toa@pengutronix.de>

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.

Best regards,
Alexander

[1] https://www.nxp.com/docs/en/reference-manual/MC9328MX1RM.pdf

> 
> Best regards
> Uwe

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/