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

[Rob Herring <robh@kernel.org>]

On Tue, Feb 14, 2023 at 04:56:37PM +0100, Miquel Raynal wrote:
> 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.

It's a feature of the specific device as to what modes it supports. For 
most cases where there is only one choice it should be implied from the 
compatible, but there's some exceptions like this one. 

Rob