| Message ID | 20221024210153.28852-2-vadym.kochan@plvision.eu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: mtd: marvell-nand: Add YAML scheme | expand |
On 24/10/2022 17:01, Vadym Kochan wrote: > Switch the DT binding to a YAML schema to enable the DT validation. > > Dropped deprecated compatibles and properties described in txt file. > > Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu> > --- > > v3: > 1) Remove txt version from the MAINTAINERS list > > 2) Use enum for some of compatible strings > > 3) Drop: > #address-cells > #size-cells: > > as they are inherited from the nand-controller.yaml > > 4) Add restriction to use 2 clocks for A8K SoC > > 5) Dropped description for clock-names and extend it with > minItems: 1 > > 6) Drop description for "dmas" > > 7) Use "unevalautedProperties: false" Hmm... In one place it makes sense. In "nand@" not really. > > 8) Drop quites from yaml refs. > > 9) Use 4-space indentation for the example section > > v2: > 1) Fixed warning by yamllint with incorrect indentation for compatible list > > .../bindings/mtd/marvell,nand-controller.yaml | 189 ++++++++++++++++++ > .../devicetree/bindings/mtd/marvell-nand.txt | 126 ------------ > MAINTAINERS | 1 - > 3 files changed, 189 insertions(+), 127 deletions(-) > create mode 100644 Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml > delete mode 100644 Documentation/devicetree/bindings/mtd/marvell-nand.txt > > diff --git a/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml > new file mode 100644 > index 000000000000..cb20f14d4aea > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml > @@ -0,0 +1,189 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mtd/marvell,nand-controller.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Marvell NAND Flash Controller (NFC) > + > +maintainers: > + - Miquel Raynal <miquel.raynal@bootlin.com> > + > +properties: > + compatible: > + oneOf: > + - items: > + - const: marvell,armada-8k-nand-controller > + - const: marvell,armada370-nand-controller > + - enum: > + - marvell,armada370-nand-controller > + - marvell,pxa3xx-nand-controller > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + minItems: 1 > + maxItems: 2 > + description: > + Shall reference the NAND controller clocks, the second one is > + is only needed for the Armada 7K/8K SoCs > + > + clock-names: > + minItems: 1 > + items: > + - const: core > + - const: reg > + > + dmas: > + maxItems: 1 > + > + dma-names: > + items: > + - const: rxtx > + > + marvell,system-controller: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: Syscon node that handles NAND controller related registers > + > +patternProperties: > + "^nand@[0-3]$": > + type: object > + properties: > + reg: > + minimum: 0 > + maximum: 3 > + > + nand-rb: > + minimum: 0 > + maximum: 1 > + > + nand-ecc-strength: > + enum: [1, 4, 8] > + > + nand-on-flash-bbt: true > + > + nand-ecc-mode: true > + > + nand-ecc-algo: > + description: | > + This property is essentially useful when not using hardware ECC. > + Howerver, it may be added when using hardware ECC for clarification > + but will be ignored by the driver because ECC mode is chosen depending > + on the page size and the strength required by the NAND chip. > + This value may be overwritten with nand-ecc-strength property. > + > + nand-ecc-step-size: > + description: | > + Marvell's NAND flash controller does use fixed strength > + (1-bit for Hamming, 16-bit for BCH), so the actual step size > + will shrink or grow in order to fit the required strength. > + Step sizes are not completely random for all and follow certain > + patterns described in AN-379, "Marvell SoC NFC ECC". > + > + label: > + $ref: /schemas/types.yaml#/definitions/string > + > + partitions: > + type: object > + $ref: /schemas/mtd/partitions/partition.yaml This is a friendly reminder during the review process. It seems my previous comments were not fully addressed. Maybe my feedback got lost between the quotes, maybe you just forgot to apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them. Thank you. > + > + marvell,nand-keep-config: > + description: | > + Orders the driver not to take the timings from the core and > + leaving them completely untouched. Bootloader timings will then > + be used. > + $ref: /schemas/types.yaml#/definitions/flag > + > + marvell,nand-enable-arbiter: > + description: | > + To enable the arbiter, all boards blindly used it, > + this bit was set by the bootloader for many boards and even if > + it is marked reserved in several datasheets, it might be needed to set > + it (otherwise it is harmless) so whether or not this property is set, > + the bit is selected by the driver. > + $ref: /schemas/types.yaml#/definitions/flag > + deprecated: true > + > + unevaluatedProperties: false This looks wrong. I do not see referencing schema here and I did not ask to change it. > + > + required: > + - reg > + - nand-rb > + > +unevaluatedProperties: false This one is ok. > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + > +allOf: > + - $ref: nand-controller.yaml# > + > + - if: > + properties: > + compatible: > + contains: > + const: marvell,pxa3xx-nand-controller > + then: > + required: > + - dmas > + - dma-names > + else: > + properties: > + dmas: false > + dma-names: false > + > + - if: > + properties: > + compatible: > + contains: > + const: marvell,armada-8k-nand-controller > + then: > + required: > + - marvell,system-controller > + properties: > + clocks: > + minItems: 2 Also for clock-names "minItems: 2" > + else: > + properties: > + marvell,system-controller: false clocks: maxItems: 1 and the same for clock-names... unless these variants also have two clocks? Best regards, Krzysztof
Hi Krzysztof, On Mon, 24 Oct 2022 17:31:48 -0400, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 24/10/2022 17:01, Vadym Kochan wrote: > > Switch the DT binding to a YAML schema to enable the DT validation. > > > > Dropped deprecated compatibles and properties described in txt file. > > > > Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu> > > --- > > > > v3: > > 1) Remove txt version from the MAINTAINERS list > > > > 2) Use enum for some of compatible strings > > > > 3) Drop: > > #address-cells > > #size-cells: > > > > as they are inherited from the nand-controller.yaml > > > > 4) Add restriction to use 2 clocks for A8K SoC > > > > 5) Dropped description for clock-names and extend it with > > minItems: 1 > > > > 6) Drop description for "dmas" > > > > 7) Use "unevalautedProperties: false" > > Hmm... In one place it makes sense. In "nand@" not really. > > > > > 8) Drop quites from yaml refs. > > > > 9) Use 4-space indentation for the example section > > > > v2: > > 1) Fixed warning by yamllint with incorrect indentation for compatible list > > > > .../bindings/mtd/marvell,nand-controller.yaml | 189 ++++++++++++++++++ > > .../devicetree/bindings/mtd/marvell-nand.txt | 126 ------------ > > MAINTAINERS | 1 - > > 3 files changed, 189 insertions(+), 127 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml > > delete mode 100644 Documentation/devicetree/bindings/mtd/marvell-nand.txt > > > > diff --git a/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml > > new file mode 100644 > > index 000000000000..cb20f14d4aea > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml > > @@ -0,0 +1,189 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mtd/marvell,nand-controller.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Marvell NAND Flash Controller (NFC) > > + > > +maintainers: > > + - Miquel Raynal <miquel.raynal@bootlin.com> > > + > > + [...] > > + nand-ecc-step-size: > > + description: | > > + Marvell's NAND flash controller does use fixed strength > > + (1-bit for Hamming, 16-bit for BCH), so the actual step size > > + will shrink or grow in order to fit the required strength. > > + Step sizes are not completely random for all and follow certain > > + patterns described in AN-379, "Marvell SoC NFC ECC". > > + > > + label: > > + $ref: /schemas/types.yaml#/definitions/string > > + > > + partitions: > > + type: object > > + $ref: /schemas/mtd/partitions/partition.yaml > > This is a friendly reminder during the review process. > > It seems my previous comments were not fully addressed. Maybe my > feedback got lost between the quotes, maybe you just forgot to apply it. > Please go back to the previous discussion and either implement all > requested changes or keep discussing them. > > Thank you. > I see that I have to drop "label" and "partitions" properties. > > + > > + marvell,nand-keep-config: > > + description: | > > + Orders the driver not to take the timings from the core and > > + leaving them completely untouched. Bootloader timings will then > > + be used. > > + $ref: /schemas/types.yaml#/definitions/flag > > + > > + marvell,nand-enable-arbiter: > > + description: | > > + To enable the arbiter, all boards blindly used it, > > + this bit was set by the bootloader for many boards and even if > > + it is marked reserved in several datasheets, it might be needed to set > > + it (otherwise it is harmless) so whether or not this property is set, > > + the bit is selected by the driver. > > + $ref: /schemas/types.yaml#/definitions/flag > > + deprecated: true > > + > > + unevaluatedProperties: false > > This looks wrong. I do not see referencing schema here and I did not ask > to change it. Yes, I will drop it. > > > + > > + required: > > + - reg > > + - nand-rb > > + > > +unevaluatedProperties: false > > This one is ok. > > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - clocks > > + > > +allOf: > > + - $ref: nand-controller.yaml# > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: marvell,pxa3xx-nand-controller > > + then: > > + required: > > + - dmas > > + - dma-names > > + else: > > + properties: > > + dmas: false > > + dma-names: false > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: marvell,armada-8k-nand-controller > > + then: > > + required: > > + - marvell,system-controller > > + properties: > > + clocks: > > + minItems: 2 > > Also for clock-names "minItems: 2" > > > + else: > > + properties: > > + marvell,system-controller: false > > clocks: > maxItems: 1 > OK, so it goes under the "else". > and the same for clock-names... unless these variants also have two clocks? > > > Best regards, > Krzysztof > Thank you, Vadym
On 24/10/2022 20:29, Vadym Kochan wrote: > [...] > >>> + nand-ecc-step-size: >>> + description: | >>> + Marvell's NAND flash controller does use fixed strength >>> + (1-bit for Hamming, 16-bit for BCH), so the actual step size >>> + will shrink or grow in order to fit the required strength. >>> + Step sizes are not completely random for all and follow certain >>> + patterns described in AN-379, "Marvell SoC NFC ECC". >>> + >>> + label: >>> + $ref: /schemas/types.yaml#/definitions/string >>> + >>> + partitions: >>> + type: object >>> + $ref: /schemas/mtd/partitions/partition.yaml >> >> This is a friendly reminder during the review process. >> >> It seems my previous comments were not fully addressed. Maybe my >> feedback got lost between the quotes, maybe you just forgot to apply it. >> Please go back to the previous discussion and either implement all >> requested changes or keep discussing them. >> >> Thank you. >> > > I see that I have to drop "label" and "partitions" properties. Hm.... why? I commented about missing unevaluatedProperties. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml new file mode 100644 index 000000000000..cb20f14d4aea --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml @@ -0,0 +1,189 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/marvell,nand-controller.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Marvell NAND Flash Controller (NFC) + +maintainers: + - Miquel Raynal <miquel.raynal@bootlin.com> + +properties: + compatible: + oneOf: + - items: + - const: marvell,armada-8k-nand-controller + - const: marvell,armada370-nand-controller + - enum: + - marvell,armada370-nand-controller + - marvell,pxa3xx-nand-controller + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + minItems: 1 + maxItems: 2 + description: + Shall reference the NAND controller clocks, the second one is + is only needed for the Armada 7K/8K SoCs + + clock-names: + minItems: 1 + items: + - const: core + - const: reg + + dmas: + maxItems: 1 + + dma-names: + items: + - const: rxtx + + marvell,system-controller: + $ref: /schemas/types.yaml#/definitions/phandle + description: Syscon node that handles NAND controller related registers + +patternProperties: + "^nand@[0-3]$": + type: object + properties: + reg: + minimum: 0 + maximum: 3 + + nand-rb: + minimum: 0 + maximum: 1 + + nand-ecc-strength: + enum: [1, 4, 8] + + nand-on-flash-bbt: true + + nand-ecc-mode: true + + nand-ecc-algo: + description: | + This property is essentially useful when not using hardware ECC. + Howerver, it may be added when using hardware ECC for clarification + but will be ignored by the driver because ECC mode is chosen depending + on the page size and the strength required by the NAND chip. + This value may be overwritten with nand-ecc-strength property. + + nand-ecc-step-size: + description: | + Marvell's NAND flash controller does use fixed strength + (1-bit for Hamming, 16-bit for BCH), so the actual step size + will shrink or grow in order to fit the required strength. + Step sizes are not completely random for all and follow certain + patterns described in AN-379, "Marvell SoC NFC ECC". + + label: + $ref: /schemas/types.yaml#/definitions/string + + partitions: + type: object + $ref: /schemas/mtd/partitions/partition.yaml + + marvell,nand-keep-config: + description: | + Orders the driver not to take the timings from the core and + leaving them completely untouched. Bootloader timings will then + be used. + $ref: /schemas/types.yaml#/definitions/flag + + marvell,nand-enable-arbiter: + description: | + To enable the arbiter, all boards blindly used it, + this bit was set by the bootloader for many boards and even if + it is marked reserved in several datasheets, it might be needed to set + it (otherwise it is harmless) so whether or not this property is set, + the bit is selected by the driver. + $ref: /schemas/types.yaml#/definitions/flag + deprecated: true + + unevaluatedProperties: false + + required: + - reg + - nand-rb + +unevaluatedProperties: false + +required: + - compatible + - reg + - interrupts + - clocks + +allOf: + - $ref: nand-controller.yaml# + + - if: + properties: + compatible: + contains: + const: marvell,pxa3xx-nand-controller + then: + required: + - dmas + - dma-names + else: + properties: + dmas: false + dma-names: false + + - if: + properties: + compatible: + contains: + const: marvell,armada-8k-nand-controller + then: + required: + - marvell,system-controller + properties: + clocks: + minItems: 2 + else: + properties: + marvell,system-controller: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + nand_controller: nand-controller@d0000 { + compatible = "marvell,armada370-nand-controller"; + reg = <0xd0000 0x54>; + #address-cells = <1>; + #size-cells = <0>; + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&coredivclk 0>; + + nand@0 { + reg = <0>; + label = "main-storage"; + nand-rb = <0>; + nand-ecc-mode = "hw"; + marvell,nand-keep-config; + nand-on-flash-bbt; + nand-ecc-strength = <4>; + nand-ecc-step-size = <512>; + + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + partition@0 { + label = "Rootfs"; + reg = <0x00000000 0x40000000>; + }; + }; + }; + }; diff --git a/Documentation/devicetree/bindings/mtd/marvell-nand.txt b/Documentation/devicetree/bindings/mtd/marvell-nand.txt deleted file mode 100644 index a2d9a0f2b683..000000000000 --- a/Documentation/devicetree/bindings/mtd/marvell-nand.txt +++ /dev/null @@ -1,126 +0,0 @@ -Marvell NAND Flash Controller (NFC) - -Required properties: -- compatible: can be one of the following: - * "marvell,armada-8k-nand-controller" - * "marvell,armada370-nand-controller" - * "marvell,pxa3xx-nand-controller" - * "marvell,armada-8k-nand" (deprecated) - * "marvell,armada370-nand" (deprecated) - * "marvell,pxa3xx-nand" (deprecated) - Compatibles marked deprecated support only the old bindings described - at the bottom. -- reg: NAND flash controller memory area. -- #address-cells: shall be set to 1. Encode the NAND CS. -- #size-cells: shall be set to 0. -- interrupts: shall define the NAND controller interrupt. -- clocks: shall reference the NAND controller clocks, the second one is - is only needed for the Armada 7K/8K SoCs -- clock-names: mandatory if there is a second clock, in this case there - should be one clock named "core" and another one named "reg" -- marvell,system-controller: Set to retrieve the syscon node that handles - NAND controller related registers (only required with the - "marvell,armada-8k-nand[-controller]" compatibles). - -Optional properties: -- label: see partition.txt. New platforms shall omit this property. -- dmas: shall reference DMA channel associated to the NAND controller. - This property is only used with "marvell,pxa3xx-nand[-controller]" - compatible strings. -- dma-names: shall be "rxtx". - This property is only used with "marvell,pxa3xx-nand[-controller]" - compatible strings. - -Optional children nodes: -Children nodes represent the available NAND chips. - -Required properties: -- reg: shall contain the native Chip Select ids (0-3). -- nand-rb: see nand-controller.yaml (0-1). - -Optional properties: -- marvell,nand-keep-config: orders the driver not to take the timings - from the core and leaving them completely untouched. Bootloader - timings will then be used. -- label: MTD name. -- nand-on-flash-bbt: see nand-controller.yaml. -- nand-ecc-mode: see nand-controller.yaml. Will use hardware ECC if not specified. -- nand-ecc-algo: see nand-controller.yaml. This property is essentially useful when - not using hardware ECC. Howerver, it may be added when using hardware - ECC for clarification but will be ignored by the driver because ECC - mode is chosen depending on the page size and the strength required by - the NAND chip. This value may be overwritten with nand-ecc-strength - property. -- nand-ecc-strength: see nand-controller.yaml. -- nand-ecc-step-size: see nand-controller.yaml. Marvell's NAND flash controller does - use fixed strength (1-bit for Hamming, 16-bit for BCH), so the actual - step size will shrink or grow in order to fit the required strength. - Step sizes are not completely random for all and follow certain - patterns described in AN-379, "Marvell SoC NFC ECC". - -See Documentation/devicetree/bindings/mtd/nand-controller.yaml for more details on -generic bindings. - - -Example: -nand_controller: nand-controller@d0000 { - compatible = "marvell,armada370-nand-controller"; - reg = <0xd0000 0x54>; - #address-cells = <1>; - #size-cells = <0>; - interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&coredivclk 0>; - - nand@0 { - reg = <0>; - label = "main-storage"; - nand-rb = <0>; - nand-ecc-mode = "hw"; - marvell,nand-keep-config; - nand-on-flash-bbt; - nand-ecc-strength = <4>; - nand-ecc-step-size = <512>; - - partitions { - compatible = "fixed-partitions"; - #address-cells = <1>; - #size-cells = <1>; - - partition@0 { - label = "Rootfs"; - reg = <0x00000000 0x40000000>; - }; - }; - }; -}; - - -Note on legacy bindings: One can find, in not-updated device trees, -bindings slightly different than described above with other properties -described below as well as the partitions node at the root of a so -called "nand" node (without clear controller/chip separation). - -Legacy properties: -- marvell,nand-enable-arbiter: To enable the arbiter, all boards blindly - used it, this bit was set by the bootloader for many boards and even if - it is marked reserved in several datasheets, it might be needed to set - it (otherwise it is harmless) so whether or not this property is set, - the bit is selected by the driver. -- num-cs: Number of chip-select lines to use, all boards blindly set 1 - to this and for a reason, other values would have failed. The value of - this property is ignored. - -Example: - - nand0: nand@43100000 { - compatible = "marvell,pxa3xx-nand"; - reg = <0x43100000 90>; - interrupts = <45>; - dmas = <&pdma 97 0>; - dma-names = "rxtx"; - #address-cells = <1>; - marvell,nand-keep-config; - marvell,nand-enable-arbiter; - num-cs = <1>; - /* Partitions (optional) */ - }; diff --git a/MAINTAINERS b/MAINTAINERS index d7d76760ef93..9b165112be3e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12344,7 +12344,6 @@ MARVELL NAND CONTROLLER DRIVER M: Miquel Raynal <miquel.raynal@bootlin.com> L: linux-mtd@lists.infradead.org S: Maintained -F: Documentation/devicetree/bindings/mtd/marvell-nand.txt F: drivers/mtd/nand/raw/marvell_nand.c MARVELL OCTEONTX2 PHYSICAL FUNCTION DRIVER
Switch the DT binding to a YAML schema to enable the DT validation. Dropped deprecated compatibles and properties described in txt file. Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu> --- v3: 1) Remove txt version from the MAINTAINERS list 2) Use enum for some of compatible strings 3) Drop: #address-cells #size-cells: as they are inherited from the nand-controller.yaml 4) Add restriction to use 2 clocks for A8K SoC 5) Dropped description for clock-names and extend it with minItems: 1 6) Drop description for "dmas" 7) Use "unevalautedProperties: false" 8) Drop quites from yaml refs. 9) Use 4-space indentation for the example section v2: 1) Fixed warning by yamllint with incorrect indentation for compatible list .../bindings/mtd/marvell,nand-controller.yaml | 189 ++++++++++++++++++ .../devicetree/bindings/mtd/marvell-nand.txt | 126 ------------ MAINTAINERS | 1 - 3 files changed, 189 insertions(+), 127 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml delete mode 100644 Documentation/devicetree/bindings/mtd/marvell-nand.txt