| Message ID | 20210609080112.1753221-18-miquel.raynal@bootlin.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | ARM Primecell PL35x support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | fail | build log |
On 09/06/2021 10:01, Miquel Raynal wrote: > Add a yaml description of this NAND controller which is described as a > subnode of the SMC bus. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > .../bindings/mtd/arm,pl353-nand-r2p1.yaml | 57 +++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml > > diff --git a/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml b/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml > new file mode 100644 > index 000000000000..e72fa14b4385 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml > @@ -0,0 +1,57 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mtd/arm,pl353-nand-r2p1.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: PL353 NAND Controller device tree bindings > + > +allOf: > + - $ref: "nand-controller.yaml" > + > +maintainers: > + - Miquel Raynal <miquel.raynal@bootlin.com> > + - Naga Sureshkumar Relli <naga.sureshkumar.relli@xilinx.com> > + > +properties: > + compatible: > + items: > + - enum: That's not an enum, but simple const without items. > + - arm,pl353-nand-r2p1 > + > + reg: > + items: > + - items: > + - description: CS with regard to the parent ranges property > + - description: Offset of the memory region requested by the device > + - description: Length of the memory region requested by the device Doesn't it depend on parent's address/size cells? > + > + "#address-cells": true > + "#size-cells": true These should come from nand-controller.yaml Best regards, Krzysztof
Hi Krzysztof, Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> wrote on Wed, 9 Jun 2021 14:01:10 +0200: > On 09/06/2021 10:01, Miquel Raynal wrote: > > Add a yaml description of this NAND controller which is described as a > > subnode of the SMC bus. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > .../bindings/mtd/arm,pl353-nand-r2p1.yaml | 57 +++++++++++++++++++ > > 1 file changed, 57 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml > > > > diff --git a/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml b/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml > > new file mode 100644 > > index 000000000000..e72fa14b4385 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml > > @@ -0,0 +1,57 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mtd/arm,pl353-nand-r2p1.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: PL353 NAND Controller device tree bindings > > + > > +allOf: > > + - $ref: "nand-controller.yaml" > > + > > +maintainers: > > + - Miquel Raynal <miquel.raynal@bootlin.com> > > + - Naga Sureshkumar Relli <naga.sureshkumar.relli@xilinx.com> > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > That's not an enum, but simple const without items. Ok. > > > + - arm,pl353-nand-r2p1 > > + > > + reg: > > + items: > > + - items: > > + - description: CS with regard to the parent ranges property > > + - description: Offset of the memory region requested by the device > > + - description: Length of the memory region requested by the device > > Doesn't it depend on parent's address/size cells? Yes, but as the child nodes are not defined in the parent's binding (ie. the SMC) I think it's interesting to have them defined here, no? > > + > > + "#address-cells": true > > + "#size-cells": true > > These should come from nand-controller.yaml Right, I'll drop these, they are probably redundant. Thanks, Miquèl
On 09/06/2021 15:36, Miquel Raynal wrote: > Hi Krzysztof, > > Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> wrote on Wed, 9 > Jun 2021 14:01:10 +0200: > >> On 09/06/2021 10:01, Miquel Raynal wrote: >>> Add a yaml description of this NAND controller which is described as a >>> subnode of the SMC bus. >>> >>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> >>> --- >>> .../bindings/mtd/arm,pl353-nand-r2p1.yaml | 57 +++++++++++++++++++ >>> 1 file changed, 57 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml b/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml >>> new file mode 100644 >>> index 000000000000..e72fa14b4385 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml >>> @@ -0,0 +1,57 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/mtd/arm,pl353-nand-r2p1.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: PL353 NAND Controller device tree bindings >>> + >>> +allOf: >>> + - $ref: "nand-controller.yaml" >>> + >>> +maintainers: >>> + - Miquel Raynal <miquel.raynal@bootlin.com> >>> + - Naga Sureshkumar Relli <naga.sureshkumar.relli@xilinx.com> >>> + >>> +properties: >>> + compatible: >>> + items: >>> + - enum: >> >> That's not an enum, but simple const without items. > > Ok. > >> >>> + - arm,pl353-nand-r2p1 >>> + >>> + reg: >>> + items: >>> + - items: >>> + - description: CS with regard to the parent ranges property >>> + - description: Offset of the memory region requested by the device >>> + - description: Length of the memory region requested by the device >> >> Doesn't it depend on parent's address/size cells? > > Yes, but as the child nodes are not defined in the parent's binding > (ie. the SMC) I think it's interesting to have them defined here, no? The trouble is if parent decides to have different address/size cells. The schema will stop matching. I am actually not that sure if such case is real since the pl353 NAND part will usually be connected to pl353 SMC. However the schema now hard-codes specific dependency against parent schema/node. Rob, Maybe you have here some thoughts? Best regards, Krzysztof
On Wed, 09 Jun 2021 10:01:11 +0200, Miquel Raynal wrote: > Add a yaml description of this NAND controller which is described as a > subnode of the SMC bus. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > .../bindings/mtd/arm,pl353-nand-r2p1.yaml | 57 +++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.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: Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.example.dt.yaml:0:0: /example-0/memory-controller@e000e000: failed to match any schema with compatible: ['arm,pl353-smc-r2p1', 'arm,primecell'] \ndoc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1489731 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.
On Wed, Jun 9, 2021 at 11:16 AM Rob Herring <robh@kernel.org> wrote: > > On Wed, 09 Jun 2021 10:01:11 +0200, Miquel Raynal wrote: > > Add a yaml description of this NAND controller which is described as a > > subnode of the SMC bus. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > .../bindings/mtd/arm,pl353-nand-r2p1.yaml | 57 +++++++++++++++++++ > > 1 file changed, 57 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.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: > Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.example.dt.yaml:0:0: /example-0/memory-controller@e000e000: failed to match any schema with compatible: ['arm,pl353-smc-r2p1', 'arm,primecell'] Ignore these errors. Rob
On Wed, Jun 09, 2021 at 03:57:05PM +0200, Krzysztof Kozlowski wrote: > On 09/06/2021 15:36, Miquel Raynal wrote: > > Hi Krzysztof, > > > > Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> wrote on Wed, 9 > > Jun 2021 14:01:10 +0200: > > > >> On 09/06/2021 10:01, Miquel Raynal wrote: > >>> Add a yaml description of this NAND controller which is described as a > >>> subnode of the SMC bus. > >>> > >>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > >>> --- > >>> .../bindings/mtd/arm,pl353-nand-r2p1.yaml | 57 +++++++++++++++++++ > >>> 1 file changed, 57 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml > >>> > >>> diff --git a/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml b/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml > >>> new file mode 100644 > >>> index 000000000000..e72fa14b4385 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml > >>> @@ -0,0 +1,57 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>> +%YAML 1.2 > >>> +--- > >>> +$id: http://devicetree.org/schemas/mtd/arm,pl353-nand-r2p1.yaml# > >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>> + > >>> +title: PL353 NAND Controller device tree bindings > >>> + > >>> +allOf: > >>> + - $ref: "nand-controller.yaml" > >>> + > >>> +maintainers: > >>> + - Miquel Raynal <miquel.raynal@bootlin.com> > >>> + - Naga Sureshkumar Relli <naga.sureshkumar.relli@xilinx.com> > >>> + > >>> +properties: > >>> + compatible: > >>> + items: > >>> + - enum: > >> > >> That's not an enum, but simple const without items. > > > > Ok. > > > >> > >>> + - arm,pl353-nand-r2p1 > >>> + > >>> + reg: > >>> + items: > >>> + - items: > >>> + - description: CS with regard to the parent ranges property > >>> + - description: Offset of the memory region requested by the device > >>> + - description: Length of the memory region requested by the device > >> > >> Doesn't it depend on parent's address/size cells? > > > > Yes, but as the child nodes are not defined in the parent's binding > > (ie. the SMC) I think it's interesting to have them defined here, no? > > The trouble is if parent decides to have different address/size cells. > The schema will stop matching. I am actually not that sure if such case > is real since the pl353 NAND part will usually be connected to pl353 > SMC. However the schema now hard-codes specific dependency against > parent schema/node. > > Rob, > Maybe you have here some thoughts? I think it is fine given the parent child coupling. Rob
diff --git a/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml b/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml new file mode 100644 index 000000000000..e72fa14b4385 --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/arm,pl353-nand-r2p1.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: PL353 NAND Controller device tree bindings + +allOf: + - $ref: "nand-controller.yaml" + +maintainers: + - Miquel Raynal <miquel.raynal@bootlin.com> + - Naga Sureshkumar Relli <naga.sureshkumar.relli@xilinx.com> + +properties: + compatible: + items: + - enum: + - arm,pl353-nand-r2p1 + + reg: + items: + - items: + - description: CS with regard to the parent ranges property + - description: Offset of the memory region requested by the device + - description: Length of the memory region requested by the device + + "#address-cells": true + "#size-cells": true + +required: + - compatible + - reg + +unevaluatedProperties: false + +examples: + - | + smcc: memory-controller@e000e000 { + compatible = "arm,pl353-smc-r2p1", "arm,primecell"; + reg = <0xe000e000 0x0001000>; + clock-names = "memclk", "apb_pclk"; + clocks = <&clkc 11>, <&clkc 44>; + ranges = <0x0 0x0 0xe1000000 0x1000000 /* Nand CS region */ + 0x1 0x0 0xe2000000 0x2000000 /* SRAM/NOR CS0 region */ + 0x2 0x0 0xe4000000 0x2000000>; /* SRAM/NOR CS1 region */ + #address-cells = <2>; + #size-cells = <1>; + + nfc0: nand-controller@0,0 { + compatible = "arm,pl353-nand-r2p1"; + reg = <0 0 0x1000000>; + #address-cells = <1>; + #size-cells = <0>; + }; + };
Add a yaml description of this NAND controller which is described as a subnode of the SMC bus. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- .../bindings/mtd/arm,pl353-nand-r2p1.yaml | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/arm,pl353-nand-r2p1.yaml