| Message ID | 20200904152404.20636-13-krzk@kernel.org |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | dt-bindings: Cleanup of i.MX 8 | expand |
On Fri, Sep 4, 2020 at 9:25 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > Driver requires different amount of clocks for different SoCs. Describe > these requirements properly to fix dtbs_check warnings like: > > arch/arm64/boot/dts/freescale/imx8mm-beacon-kit.dt.yaml: nand-controller@33002000: clock-names:1: 'gpmi_apb' was expected > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > --- > > Changes since v1: > 1. Do not require order of clocks (use pattern). To the extent that you can, you should fix the order in dts files first. If we just adjust the schemas to match the dts files, then what's the point? > --- > .../devicetree/bindings/mtd/gpmi-nand.yaml | 76 +++++++++++++++---- > 1 file changed, 61 insertions(+), 15 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml > index 28ff8c581837..e08e0a50929e 100644 > --- a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml > +++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml > @@ -9,9 +9,6 @@ title: Freescale General-Purpose Media Interface (GPMI) binding > maintainers: > - Han Xu <han.xu@nxp.com> > > -allOf: > - - $ref: "nand-controller.yaml" > - > description: | > The GPMI nand controller provides an interface to control the NAND > flash chips. The device tree may optionally contain sub-nodes > @@ -58,22 +55,10 @@ properties: > clocks: > minItems: 1 > maxItems: 5 > - items: > - - description: SoC gpmi io clock > - - description: SoC gpmi apb clock > - - description: SoC gpmi bch clock > - - description: SoC gpmi bch apb clock > - - description: SoC per1 bch clock > > clock-names: > minItems: 1 > maxItems: 5 > - items: > - - const: gpmi_io > - - const: gpmi_apb > - - const: gpmi_bch > - - const: gpmi_bch_apb > - - const: per1_bch > > fsl,use-minimum-ecc: > type: boolean > @@ -107,6 +92,67 @@ required: > > unevaluatedProperties: false > > +allOf: > + - $ref: "nand-controller.yaml" > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - fsl,imx23-gpmi-nand > + - fsl,imx28-gpmi-nand > + then: > + properties: > + clocks: > + items: > + - description: SoC gpmi io clock > + clock-names: > + items: > + - const: gpmi_io > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - fsl,imx6q-gpmi-nand > + - fsl,imx6sx-gpmi-nand > + then: > + properties: > + clocks: > + items: > + - description: SoC gpmi io clock > + - description: SoC gpmi apb clock > + - description: SoC gpmi bch clock > + - description: SoC gpmi bch apb clock > + - description: SoC per1 bch clock > + clock-names: > + items: > + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" > + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" > + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" > + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" > + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" BTW, you can make 'items' a schema rather than a list to apply a constraint to all entries: maxItems: 5 items: pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" > + > + - if: > + properties: > + compatible: > + contains: > + const: fsl,imx7d-gpmi-nand > + then: > + properties: > + clocks: > + items: > + - description: SoC gpmi io clock > + - description: SoC gpmi bch apb clock > + clock-names: > + minItems: 2 > + maxItems: 2 You can drop these. It's the default based on the size of 'items'. > + items: > + - pattern: "^gpmi_(io|bch_apb)$" > + - pattern: "^gpmi_(io|bch_apb)$" Surely here we can define the order. > + > examples: > - | > nand-controller@8000c000 { > -- > 2.17.1 >
On Fri, Sep 04, 2020 at 04:36:39PM -0600, Rob Herring wrote: > On Fri, Sep 4, 2020 at 9:25 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > Driver requires different amount of clocks for different SoCs. Describe > > these requirements properly to fix dtbs_check warnings like: > > > > arch/arm64/boot/dts/freescale/imx8mm-beacon-kit.dt.yaml: nand-controller@33002000: clock-names:1: 'gpmi_apb' was expected > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > > > --- > > > > Changes since v1: > > 1. Do not require order of clocks (use pattern). > > To the extent that you can, you should fix the order in dts files > first. If we just adjust the schemas to match the dts files, then > what's the point? The DTSes do not have mixed order of clocks between each other, as fair as I remember. It was fix after Sasha Hauer comment that order is not necessarily good. We have the clock-names property, why enforcing the order? > > > --- > > .../devicetree/bindings/mtd/gpmi-nand.yaml | 76 +++++++++++++++---- > > 1 file changed, 61 insertions(+), 15 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml > > index 28ff8c581837..e08e0a50929e 100644 > > --- a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml > > +++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml > > @@ -9,9 +9,6 @@ title: Freescale General-Purpose Media Interface (GPMI) binding > > maintainers: > > - Han Xu <han.xu@nxp.com> > > > > -allOf: > > - - $ref: "nand-controller.yaml" > > - > > description: | > > The GPMI nand controller provides an interface to control the NAND > > flash chips. The device tree may optionally contain sub-nodes > > @@ -58,22 +55,10 @@ properties: > > clocks: > > minItems: 1 > > maxItems: 5 > > - items: > > - - description: SoC gpmi io clock > > - - description: SoC gpmi apb clock > > - - description: SoC gpmi bch clock > > - - description: SoC gpmi bch apb clock > > - - description: SoC per1 bch clock > > > > clock-names: > > minItems: 1 > > maxItems: 5 > > - items: > > - - const: gpmi_io > > - - const: gpmi_apb > > - - const: gpmi_bch > > - - const: gpmi_bch_apb > > - - const: per1_bch > > > > fsl,use-minimum-ecc: > > type: boolean > > @@ -107,6 +92,67 @@ required: > > > > unevaluatedProperties: false > > > > +allOf: > > + - $ref: "nand-controller.yaml" > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - fsl,imx23-gpmi-nand > > + - fsl,imx28-gpmi-nand > > + then: > > + properties: > > + clocks: > > + items: > > + - description: SoC gpmi io clock > > + clock-names: > > + items: > > + - const: gpmi_io > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - fsl,imx6q-gpmi-nand > > + - fsl,imx6sx-gpmi-nand > > + then: > > + properties: > > + clocks: > > + items: > > + - description: SoC gpmi io clock > > + - description: SoC gpmi apb clock > > + - description: SoC gpmi bch clock > > + - description: SoC gpmi bch apb clock > > + - description: SoC per1 bch clock > > + clock-names: > > + items: > > + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" > > + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" > > + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" > > + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" > > + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" > > BTW, you can make 'items' a schema rather than a list to apply a > constraint to all entries: > > maxItems: 5 > items: > pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" Right, I forgot about such syntax. > > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: fsl,imx7d-gpmi-nand > > + then: > > + properties: > > + clocks: > > + items: > > + - description: SoC gpmi io clock > > + - description: SoC gpmi bch apb clock > > + clock-names: > > + minItems: 2 > > + maxItems: 2 > > You can drop these. It's the default based on the size of 'items'. Sure. > > > + items: > > + - pattern: "^gpmi_(io|bch_apb)$" > > + - pattern: "^gpmi_(io|bch_apb)$" > > Surely here we can define the order. Yes, but still the same question as before - do we want the order? Why enforcing it? Best regards, Krzysztof
On Mon, Sep 7, 2020 at 12:10 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Fri, Sep 04, 2020 at 04:36:39PM -0600, Rob Herring wrote: > > On Fri, Sep 4, 2020 at 9:25 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > Driver requires different amount of clocks for different SoCs. Describe > > > these requirements properly to fix dtbs_check warnings like: > > > > > > arch/arm64/boot/dts/freescale/imx8mm-beacon-kit.dt.yaml: nand-controller@33002000: clock-names:1: 'gpmi_apb' was expected > > > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > > > > > --- > > > > > > Changes since v1: > > > 1. Do not require order of clocks (use pattern). > > > > To the extent that you can, you should fix the order in dts files > > first. If we just adjust the schemas to match the dts files, then > > what's the point? > > The DTSes do not have mixed order of clocks between each other, as fair > as I remember. It was fix after Sasha Hauer comment that order is not > necessarily good. > > We have the clock-names property, why enforcing the order? Because DT/OpenFirmware has always had a defined order for property values. '*-names' is just extra information. Rob
On Tue, 8 Sep 2020 at 18:51, Rob Herring <robh+dt@kernel.org> wrote: > > On Mon, Sep 7, 2020 at 12:10 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > On Fri, Sep 04, 2020 at 04:36:39PM -0600, Rob Herring wrote: > > > On Fri, Sep 4, 2020 at 9:25 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > > > > Driver requires different amount of clocks for different SoCs. Describe > > > > these requirements properly to fix dtbs_check warnings like: > > > > > > > > arch/arm64/boot/dts/freescale/imx8mm-beacon-kit.dt.yaml: nand-controller@33002000: clock-names:1: 'gpmi_apb' was expected > > > > > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > > > > > > > --- > > > > > > > > Changes since v1: > > > > 1. Do not require order of clocks (use pattern). > > > > > > To the extent that you can, you should fix the order in dts files > > > first. If we just adjust the schemas to match the dts files, then > > > what's the point? > > > > The DTSes do not have mixed order of clocks between each other, as fair > > as I remember. It was fix after Sasha Hauer comment that order is not > > necessarily good. > > > > We have the clock-names property, why enforcing the order? > > Because DT/OpenFirmware has always had a defined order for property > values. '*-names' is just extra information. Thanks for the explanation. There are few nonobvious requirements about writing schema which seems many (including me) miss. It might be a good topic for some conference. Too bad ELCE CFP ended some time ago. :) Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml index 28ff8c581837..e08e0a50929e 100644 --- a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml +++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml @@ -9,9 +9,6 @@ title: Freescale General-Purpose Media Interface (GPMI) binding maintainers: - Han Xu <han.xu@nxp.com> -allOf: - - $ref: "nand-controller.yaml" - description: | The GPMI nand controller provides an interface to control the NAND flash chips. The device tree may optionally contain sub-nodes @@ -58,22 +55,10 @@ properties: clocks: minItems: 1 maxItems: 5 - items: - - description: SoC gpmi io clock - - description: SoC gpmi apb clock - - description: SoC gpmi bch clock - - description: SoC gpmi bch apb clock - - description: SoC per1 bch clock clock-names: minItems: 1 maxItems: 5 - items: - - const: gpmi_io - - const: gpmi_apb - - const: gpmi_bch - - const: gpmi_bch_apb - - const: per1_bch fsl,use-minimum-ecc: type: boolean @@ -107,6 +92,67 @@ required: unevaluatedProperties: false +allOf: + - $ref: "nand-controller.yaml" + + - if: + properties: + compatible: + contains: + enum: + - fsl,imx23-gpmi-nand + - fsl,imx28-gpmi-nand + then: + properties: + clocks: + items: + - description: SoC gpmi io clock + clock-names: + items: + - const: gpmi_io + + - if: + properties: + compatible: + contains: + enum: + - fsl,imx6q-gpmi-nand + - fsl,imx6sx-gpmi-nand + then: + properties: + clocks: + items: + - description: SoC gpmi io clock + - description: SoC gpmi apb clock + - description: SoC gpmi bch clock + - description: SoC gpmi bch apb clock + - description: SoC per1 bch clock + clock-names: + items: + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" + - pattern: "^(gpmi_(io|apb|bch|bch_apb)|per1_bch)$" + + - if: + properties: + compatible: + contains: + const: fsl,imx7d-gpmi-nand + then: + properties: + clocks: + items: + - description: SoC gpmi io clock + - description: SoC gpmi bch apb clock + clock-names: + minItems: 2 + maxItems: 2 + items: + - pattern: "^gpmi_(io|bch_apb)$" + - pattern: "^gpmi_(io|bch_apb)$" + examples: - | nand-controller@8000c000 {
Driver requires different amount of clocks for different SoCs. Describe these requirements properly to fix dtbs_check warnings like: arch/arm64/boot/dts/freescale/imx8mm-beacon-kit.dt.yaml: nand-controller@33002000: clock-names:1: 'gpmi_apb' was expected Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- Changes since v1: 1. Do not require order of clocks (use pattern). --- .../devicetree/bindings/mtd/gpmi-nand.yaml | 76 +++++++++++++++---- 1 file changed, 61 insertions(+), 15 deletions(-)