Re: [PATCH] dt-bindings: mtd: jedec,spi-nor: Document CPOL/CPHA support

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Tudor Ambarus <tudor.ambarus@linaro.org>,
	Pratyush Yadav <pratyush@kernel.org>,
	Michael Walle <michael@walle.cc>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Mark Brown <broonie@kernel.org>,
	linux-mtd@lists.infradead.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH] dt-bindings: mtd: jedec,spi-nor: Document CPOL/CPHA support
Date: Tue, 14 Feb 2023 16:22:15 +0100	[thread overview]
Message-ID: <CAMuHMdVK7vOTNMF53SvHn8LtiHX7MSN-Fp80jngSsosYBhhrHQ@mail.gmail.com> (raw)
In-Reply-To: <20230214154433.6a421c51@xps-13>

Hi Miquel,

On Tue, Feb 14, 2023 at 3:44 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> geert+renesas@glider.be wrote on Tue, 14 Feb 2023 15:26:43 +0100:
> > SPI EEPROMs typically support both SPI Mode 0 (CPOL=CPHA=0) and Mode 3
> > (CPOL=CPHA=1).  However, using the latter is currently flagged as an
> > error by "make dtbs_check", e.g.:
> >
> >     arch/arm/boot/dts/r8a7791-koelsch.dtb: flash@0: Unevaluated properties are not allowed ('spi-cpha', 'spi-cpol' were unexpected)
> >           From schema: Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> >
> > Fix this by documenting support for CPOL=CPHA=1.
> >
> > Fixes: 233363aba72ac638 ("spi/panel: dt-bindings: drop CPHA and CPOL from common properties")
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > index f86255ce13af0871..bb62ac4585822982 100644
> > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> > @@ -76,6 +76,13 @@ properties:
> >        If "broken-flash-reset" is present then having this property does not
> >        make any difference.
> >
> > +  spi-cpol: true
> > +  spi-cpha: true
>
> I see that spi-cpol and spi-cpha are described in spi-controller.yaml
> which references spi-peripheral-props.yaml, but jedec,spi-nor.yaml
> only references spi-peripheral-props.yaml leading to spi-cpol and
> spi-cpha not being recognized as valid properties.
>
> Wouldn't it be cleaner to to have these two properties defined in
> spi-peripheral-props.yaml instead?

They were moved out of that file by the commit referenced in the
Fixes: tag above, because they are not supported by all SPI targets.
It's the responsibility of the SPI target bindings to list what is supported.

>
> > +
> > +dependencies:
> > +  spi-cpol: [ spi-cpha ]
> > +  spi-cpha: [ spi-cpol ]
> > +
> >  unevaluatedProperties: false
> >
> >  examples:

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds