| Message ID | 1630499829-20059-1-git-send-email-pthombar@cadence.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | add support for Cadence's XSPI controller | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
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.
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
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
>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
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".
>> 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 --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>; + }; + };