[PATCH 0/3] spi: dt-bindings: cdns,qspi-nor: Improve description

[PATCH 0/3] spi: dt-bindings: cdns,qspi-nor: Improve description

[Miquel Raynal]

While working with this controller I figured out a couple of small
issues which I propose to fix. They are not super impacting, but I
believe this goes into the right direction.

Miquel Raynal (3):
  spi: dt-bindings: cdns,qspi-nor: Be more descriptive regarding what
    this controller is
  spi: dt-bindings: cdns,qspi-nor: Deprecate the Cadence compatible
    alone
  spi: dt-bindings: cdns,qspi-nor: Make the example real

 .../devicetree/bindings/spi/cdns,qspi-nor.yaml        | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

-- 
2.48.1

[PATCH 1/3] spi: dt-bindings: cdns,qspi-nor: Be more descriptive regarding what this controller is

[Miquel Raynal]

Despite being very common in commit logs, SPI NOR controllers simply do
not exist. At least, they are not as specific as the name implies. There
are SPI memory controllers which are indeed "specialized" and optimized
for handling "memories", but most of them are just generic and accept
almost any kind of opcode, address, dummy and data cycles, making them
as suitable for NANDs than NORs.

Furthermore, this controller supports any kind of bus, from single to
octal NAND, so make it clear.

Also add a comment to mention that the initial compatible naming is too
specific (but obviously kept for backward compatibility reasons).

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
index b6bc71d19286..c4315b2e04f2 100644
--- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/spi/cdns,qspi-nor.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Cadence Quad SPI controller
+title: Cadence Quad/Octal SPI controller
 
 maintainers:
   - Vaishnav Achath <vaishnav.a@ti.com>
@@ -76,6 +76,9 @@ properties:
               - ti,am654-ospi
               - ti,k2g-qspi
               - xlnx,versal-ospi-1.0
+          # The compatible is qspi-nor for historical reasons but such
+          # controllers are meant to be used with flashes of all kinds,
+          # ie. also NAND flashes, not only NOR flashes.
           - const: cdns,qspi-nor
       - const: cdns,qspi-nor
 
-- 
2.48.1

Re: [PATCH 1/3] spi: dt-bindings: cdns,qspi-nor: Be more descriptive regarding what this controller is

[Krzysztof Kozlowski]

On Wed, Mar 05, 2025 at 09:01:31PM +0100, Miquel Raynal wrote:
> Despite being very common in commit logs, SPI NOR controllers simply do
> not exist. At least, they are not as specific as the name implies. There
> are SPI memory controllers which are indeed "specialized" and optimized
> for handling "memories", but most of them are just generic and accept
> almost any kind of opcode, address, dummy and data cycles, making them
> as suitable for NANDs than NORs.
> 
> Furthermore, this controller supports any kind of bus, from single to
> octal NAND, so make it clear.
> 
> Also add a comment to mention that the initial compatible naming is too
> specific (but obviously kept for backward compatibility reasons).
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

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

Best regards,
Krzysztof

[PATCH 2/3] spi: dt-bindings: cdns,qspi-nor: Deprecate the Cadence compatible alone

[Miquel Raynal]

The initial SPI controller IP from Cadence has always been implemented
into controllers from various hardware manufacturers and because of
that, it has always been (rightfully) doubled with a more specific
compatible. There are likely no reasons to keep this compatible
legitimate, alone. Make sure people do not get mislead by officially
deprecating this compatible.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
index c4315b2e04f2..0104cc5283db 100644
--- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
@@ -81,6 +81,7 @@ properties:
           # ie. also NAND flashes, not only NOR flashes.
           - const: cdns,qspi-nor
       - const: cdns,qspi-nor
+        deprecated: true
 
   reg:
     items:
-- 
2.48.1

Re: [PATCH 2/3] spi: dt-bindings: cdns,qspi-nor: Deprecate the Cadence compatible alone

[Krzysztof Kozlowski]

On Wed, Mar 05, 2025 at 09:01:32PM +0100, Miquel Raynal wrote:
> The initial SPI controller IP from Cadence has always been implemented
> into controllers from various hardware manufacturers and because of
> that, it has always been (rightfully) doubled with a more specific
> compatible. There are likely no reasons to keep this compatible
> legitimate, alone. Make sure people do not get mislead by officially
> deprecating this compatible.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> index c4315b2e04f2..0104cc5283db 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> @@ -81,6 +81,7 @@ properties:
>            # ie. also NAND flashes, not only NOR flashes.
>            - const: cdns,qspi-nor
>        - const: cdns,qspi-nor
> +        deprecated: true
>  

Please update the example, so non-deprecated device will be documented.

Best regards,
Krzysztof

Re: [PATCH 2/3] spi: dt-bindings: cdns,qspi-nor: Deprecate the Cadence compatible alone

[Miquel Raynal]

Hello,

>> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
>> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
>> @@ -81,6 +81,7 @@ properties:
>>            # ie. also NAND flashes, not only NOR flashes.
>>            - const: cdns,qspi-nor
>>        - const: cdns,qspi-nor
>> +        deprecated: true
>>  
>
> Please update the example, so non-deprecated device will be
> documented.

Good point.

Thanks,
Miquèl

[PATCH 3/3] spi: dt-bindings: cdns,qspi-nor: Make the example real

[Miquel Raynal]

Add several properties to the example. They are actually mandatory from
a description point of view. Unfortunately, because of some YAML
limitations, they are just listed in a file showing the peripheral
properties that "can" be used for this specific controller without
marking them "required" explicitly.

At least mention these properties in the example to hint people they
exist.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
index 0104cc5283db..a22752806ae1 100644
--- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
@@ -177,5 +177,10 @@ examples:
         flash@0 {
             compatible = "jedec,spi-nor";
             reg = <0x0>;
+            cdns,read-delay = <4>;
+            cdns,tshsl-ns = <60>;
+            cdns,tsd2d-ns = <60>;
+            cdns,tchsh-ns = <60>;
+            cdns,tslch-ns = <60>;
         };
     };
-- 
2.48.1

Re: [PATCH 3/3] spi: dt-bindings: cdns,qspi-nor: Make the example real

[Krzysztof Kozlowski]

On Wed, Mar 05, 2025 at 09:01:33PM +0100, Miquel Raynal wrote:
> Add several properties to the example. They are actually mandatory from
> a description point of view. Unfortunately, because of some YAML
> limitations, they are just listed in a file showing the peripheral
> properties that "can" be used for this specific controller without
> marking them "required" explicitly.

You can mark them as required - in this binding. See exynos-srom.yaml
(I fixed missing ref to mc-peripheral-props in separate patch).

> 
> At least mention these properties in the example to hint people they
> exist.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Best regards,
Krzysztof