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@xxxxxxxxxxxxxxx>
> > > ---
> > >
> > > .../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@xxxxxxxxxxxxxx>
> > >
> > > -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
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
