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

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Geert,

geert@linux-m68k.org wrote on Tue, 14 Feb 2023 16:22:15 +0100:

> 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.

Oops, I overlooked that line.

I actually see no point in constraining device trees on that regard,
but, well, Krzysztof is the author, I believe he knows his stuff, so
let's go for it.

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl