diff mbox series

[v3,1/2] spi: cadence: add dt-bindings documentation for Cadence XSPI controller

Message ID 1630499829-20059-1-git-send-email-pthombar@cadence.com (mailing list archive)
State Superseded
Headers show
Series add support for Cadence's XSPI controller | expand

Commit Message

Parshuram Raju Thombare Sept. 1, 2021, 12:37 p.m. UTC
Add DT binding for Cadence's XSPI controller driver.

Signed-off-by: Konrad Kociolek <konrad@cadence.com>
Signed-off-by: Jayshri Pawar <jpawar@cadence.com>
Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
---
 .../devicetree/bindings/spi/cdns,xspi.yaml         | 66 ++++++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/cdns,xspi.yaml

Comments

Rob Herring Sept. 2, 2021, 12:03 p.m. UTC | #1
On Wed, 01 Sep 2021 14:37:09 +0200, Parshuram Thombare wrote:
> Add DT binding for Cadence's XSPI controller driver.
> 
> Signed-off-by: Konrad Kociolek <konrad@cadence.com>
> Signed-off-by: Jayshri Pawar <jpawar@cadence.com>
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
>  .../devicetree/bindings/spi/cdns,xspi.yaml         | 66 ++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/cdns,xspi.yaml: 'additionalProperties' is a required property
	hint: A schema without a "$ref" to another schema must define all properties and use "additionalProperties"
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/cdns,xspi.yaml: ignoring, error in schema: 
warning: no schema found in file: ./Documentation/devicetree/bindings/spi/cdns,xspi.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml: example-0: spi@a0010000:reg:0: [0, 2684420096, 0, 65536] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml: example-0: spi@a0010000:reg:1: [0, 2952790016, 0, 65536] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml: example-0: spi@a0010000:reg:2: [0, 2684485632, 0, 65536] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml:0:0: /example-0/spi@a0010000: failed to match any schema with compatible: ['cdns,xspi-nor']
Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml:0:0: /example-0/spi@a0010000/mt35xu512@0: failed to match any schema with compatible: ['spi-nor', 'micron,mt35xu512']
Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml:0:0: /example-0/spi@a0010000/mt35xu512@0: failed to match any schema with compatible: ['spi-nor', 'micron,mt35xu512']
Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml:0:0: /example-0/spi@a0010000/mt35xu512@1: failed to match any schema with compatible: ['spi-nor', 'micron,mt35xu512']
Documentation/devicetree/bindings/spi/cdns,xspi.example.dt.yaml:0:0: /example-0/spi@a0010000/mt35xu512@1: failed to match any schema with compatible: ['spi-nor', 'micron,mt35xu512']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1523137

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Parshuram Raju Thombare Sept. 3, 2021, 8:03 a.m. UTC | #2
Hi Rob,

>See
>https://urldefense.com/v3/__https://patchwork.ozlabs.org/patch/1523137__;!!E
>HscmS1ygiU1lA!V71KMbkd1YPERLm96tbPDX_W05cj0TCcttcwVXU9H3lDADvSBnc
>5GbQunqxwGIA$
>
>This check can fail if there are any dependencies. The base for a patch
>series is generally the most recent rc1.
>
>If you already ran 'make dt_binding_check' and didn't see the above
>error(s), then make sure 'yamllint' is installed and dt-schema is up to
>date:
>
>pip3 install dtschema --upgrade
>
>Please check and re-submit.

Thanks for your reply. I will fix this issue in next version of patchset.

Regards,
Parshuram Thombare
Pratyush Yadav Sept. 3, 2021, 6:17 p.m. UTC | #3
On 01/09/21 02:37PM, Parshuram Thombare wrote:
> Add DT binding for Cadence's XSPI controller driver.
> 
> Signed-off-by: Konrad Kociolek <konrad@cadence.com>
> Signed-off-by: Jayshri Pawar <jpawar@cadence.com>
> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
> ---
>  .../devicetree/bindings/spi/cdns,xspi.yaml         | 66 ++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> new file mode 100644
> index 0000000..e52d6fa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2020-21 Cadence
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/spi/cdns,xspi.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Cadence XSPI Controller
> +
> +maintainers:
> +  - Parshuram Thombare <pthombar@cadence.com>
> +
> +description: |
> +  The XSPI controller allows SPI protocol communication in
> +  single, dual, quad or octal wire transmission modes for
> +  read/write access to slaves such as SPI-NOR flash.

This needs to be a "subclass" of the spi-controller.yaml binding.

allOf:
  - $ref: spi-controller.yaml#

> +
> +properties:
> +  compatible:
> +    const: cdns,xspi-nor
> +
> +  reg:
> +    items:
> +      - description: address and length of the controller register set
> +      - description: address and length of the Slave DMA data port
> +      - description: address and length of the auxiliary registers
> +
> +  reg-names:
> +    items:
> +      - const: xspi-iobase
> +      - const: xspi-sdmabase
> +      - const: xspi-auxbase
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    xspi: spi@a0010000 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        compatible = "cdns,xspi-nor";
> +        reg = <0x0 0xa0010000 0x0 0x10000>,
> +              <0x0 0xb0000000 0x0 0x10000>,
> +              <0x0 0xa0020000 0x0 0x10000>;
> +        reg-names = "xspi-iobase", "xspi-sdmabase", "xspi-auxbase";
> +        interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-parent = <&gic>;
> +        mt35xu512@0 {

Node name should be flash@0.

> +            compatible = "spi-nor", "micron,mt35xu512";

These compatibles are arbitrary and undocumented. You probably just need 
"jedec,spi-nor". If you need anything else, you need to justify why.

Please run dt_binding_check, it should point this out. See [0].

> +            spi-max-frequency = <75000000>;
> +            reg = <0>;
> +        };
> +        mt35xu512@1 {
> +            compatible = "spi-nor", "micron,mt35xu512";

Same as above.

> +            spi-max-frequency = <75000000>;
> +            reg = <1>;
> +        };
> +    };
> -- 
> 2.7.4
> 

[0] https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-schema.html#testing
Parshuram Raju Thombare Sept. 8, 2021, 6:52 a.m. UTC | #4
>This needs to be a "subclass" of the spi-controller.yaml binding.
>
>allOf:
>  - $ref: spi-controller.yaml#

Isn't stating that validation need against spi-controller.yaml as well as
this schema sufficient ? Can you please point an example how to make
controller binding a "subclass" of spi-controller.yaml binding ?

>Node name should be flash@0.

I think spi-controller.yaml uses wildcard for the name of a device node,
so anything in string@hexvalue: should work.

>> +            compatible = "spi-nor", "micron,mt35xu512";
>
>These compatibles are arbitrary and undocumented. You probably just need
>"jedec,spi-nor". If you need anything else, you need to justify why.

Although just "spi-nor" also works, I agree to use "jedec, spi-nor" and drop
device name.

Regards,
Parshuram Thombare
Pratyush Yadav Sept. 8, 2021, 11:32 a.m. UTC | #5
On 08/09/21 06:52AM, Parshuram Raju Thombare wrote:
> >This needs to be a "subclass" of the spi-controller.yaml binding.
> >
> >allOf:
> >  - $ref: spi-controller.yaml#
> 
> Isn't stating that validation need against spi-controller.yaml as well as
> this schema sufficient ? Can you please point an example how to make
> controller binding a "subclass" of spi-controller.yaml binding ?

I just showed you. You need to add the below lines:

allOf:
  - $ref: spi-controller.yaml#

See cdns,qspi-nor.yaml or nvidia,tegra210-quad.yaml or any of the 
multiple controller bindings we already have.

By "subclass" I did not mean a programming construct, I just meant it 
should logically be a subclass of the spi-controller.yaml binding, which 
can be done by the allOf.

> 
> >Node name should be flash@0.
> 
> I think spi-controller.yaml uses wildcard for the name of a device node,
> so anything in string@hexvalue: should work.

Sure, but mtd.yaml (which the SPI NOR binding depends on) requires it.

> 
> >> +            compatible = "spi-nor", "micron,mt35xu512";
> >
> >These compatibles are arbitrary and undocumented. You probably just need
> >"jedec,spi-nor". If you need anything else, you need to justify why.
> 
> Although just "spi-nor" also works, I agree to use "jedec, spi-nor" and drop
> device name.

Does it? I see "jedec,spi-nor" compatible documented, but not just 
"spi-nor". And I don't see "micron,mt35xu512" compatible documented 
anywhere either.

Anyway, please just use "jedec,spi-nor".
Parshuram Raju Thombare Sept. 8, 2021, 11:58 a.m. UTC | #6
>> Isn't stating that validation need against spi-controller.yaml as well as
>> this schema sufficient ? Can you please point an example how to make
>> controller binding a "subclass" of spi-controller.yaml binding ?
>
>I just showed you. You need to add the below lines:

Oh, my bad. I already added that dependency in my local patches for next version,
I thought you are suggesting something else. 

>> I think spi-controller.yaml uses wildcard for the name of a device node,
>> so anything in string@hexvalue: should work.
>
>Sure, but mtd.yaml (which the SPI NOR binding depends on) requires it.

Ok, using "jedec,spi-nor" creates dependency on spi-nor schema, otherwise
I was not seeing any issue in " make dt_binding_check" with just "spi-nor".

This will be taken care in next version of patch set.

Regards,
Parshuram Thombare
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
new file mode 100644
index 0000000..e52d6fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
@@ -0,0 +1,66 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2020-21 Cadence
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/spi/cdns,xspi.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Cadence XSPI Controller
+
+maintainers:
+  - Parshuram Thombare <pthombar@cadence.com>
+
+description: |
+  The XSPI controller allows SPI protocol communication in
+  single, dual, quad or octal wire transmission modes for
+  read/write access to slaves such as SPI-NOR flash.
+
+properties:
+  compatible:
+    const: cdns,xspi-nor
+
+  reg:
+    items:
+      - description: address and length of the controller register set
+      - description: address and length of the Slave DMA data port
+      - description: address and length of the auxiliary registers
+
+  reg-names:
+    items:
+      - const: xspi-iobase
+      - const: xspi-sdmabase
+      - const: xspi-auxbase
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    xspi: spi@a0010000 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        compatible = "cdns,xspi-nor";
+        reg = <0x0 0xa0010000 0x0 0x10000>,
+              <0x0 0xb0000000 0x0 0x10000>,
+              <0x0 0xa0020000 0x0 0x10000>;
+        reg-names = "xspi-iobase", "xspi-sdmabase", "xspi-auxbase";
+        interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-parent = <&gic>;
+        mt35xu512@0 {
+            compatible = "spi-nor", "micron,mt35xu512";
+            spi-max-frequency = <75000000>;
+            reg = <0>;
+        };
+        mt35xu512@1 {
+            compatible = "spi-nor", "micron,mt35xu512";
+            spi-max-frequency = <75000000>;
+            reg = <1>;
+        };
+    };