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

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

[Geert Uytterhoeven]

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
+
+dependencies:
+  spi-cpol: [ spi-cpha ]
+  spi-cpha: [ spi-cpol ]
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1

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

[Miquel Raynal]

Hi Geert, Krzysztof, Rob,

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?

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


Thanks,
Miquèl

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

[Geert Uytterhoeven]

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

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

[Miquel Raynal]

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

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

[Rob Herring]

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

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

[Krzysztof Kozlowski]

On 14/02/2023 15:26, Geert Uytterhoeven wrote:
> 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.:
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

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

[Tudor Ambarus]

Miquel,

Would you please take this through mtd/fixes?

On 14.02.2023 16:26, Geert Uytterhoeven wrote:
> 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.
>

Would be good if you can amend the commit message to Cc stable:
Cc: <stable@vger.kernel.org>

> Fixes: 233363aba72ac638 ("spi/panel: dt-bindings: drop CPHA and CPOL from common properties")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>

Cheers,
ta

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

[Miquel Raynal]

On Tue, 2023-02-14 at 14:26:43 UTC, Geert Uytterhoeven wrote:
> 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>
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks.

Miquel