Add YAML scheme for the Marvell's NAND controller
to validate it's DT bindings. Old txt file is deleted,
not included the compatibles and properties which were marked as
deprecated.
Also fix node name in cp11x DTSI acording to nand-controller.yaml
Vadym Kochan (2):
dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
arm64: dts: marvell: cp11x: Fix nand_controller node name according to
YAML
.../bindings/mtd/marvell,nand-controller.yaml | 199 ++++++++++++++++++
.../devicetree/bindings/mtd/marvell-nand.txt | 126 -----------
arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 2 +-
3 files changed, 200 insertions(+), 127 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml
delete mode 100644 Documentation/devicetree/bindings/mtd/marvell-nand.txt
--
2.17.1
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 <[email protected]>
---
.../bindings/mtd/marvell,nand-controller.yaml | 199 ++++++++++++++++++
.../devicetree/bindings/mtd/marvell-nand.txt | 126 -----------
2 files changed, 199 insertions(+), 126 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..535b7f8903c8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml
@@ -0,0 +1,199 @@
+# 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 <[email protected]>
+
+properties:
+
+ compatible:
+ oneOf:
+ - items:
+ - const: marvell,armada-8k-nand-controller
+ - const: marvell,armada370-nand-controller
+ - const: marvell,armada370-nand-controller
+ - const: marvell,pxa3xx-nand-controller
+
+ reg:
+ maxItems: 1
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ 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:
+ items:
+ - const: core
+ - const: reg
+ description: |
+ Mandatory if there is a second clock, in this case there
+ should be one clock named "core" and another one named "reg"
+
+ dmas:
+ maxItems: 1
+ description: rxtx DMA channel
+
+ 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:
+ const: 512
+ 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
+
+ additionalProperties: false
+
+ required:
+ - reg
+ - nand-rb
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+ - 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
+ 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) */
- };
--
2.17.1
Marvell NAND controller has now YAML to validate it's DT bindings, so
change the node name of cp11x DTSI as it is required by nand-controller.yaml
Signed-off-by: Vadym Kochan <[email protected]>
---
arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
index 7d0043824f2a..982b180b33e6 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
@@ -468,7 +468,7 @@
status = "disabled";
};
- CP11X_LABEL(nand_controller): nand@720000 {
+ CP11X_LABEL(nand_controller): nand-controller@720000 {
/*
* Due to the limitation of the pins available
* this controller is only usable on the CPM
--
2.17.1
On Fri, 21 Oct 2022 22:45:49 +0300, 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 <[email protected]>
> ---
> .../bindings/mtd/marvell,nand-controller.yaml | 199 ++++++++++++++++++
> .../devicetree/bindings/mtd/marvell-nand.txt | 126 -----------
> 2 files changed, 199 insertions(+), 126 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml
> delete mode 100644 Documentation/devicetree/bindings/mtd/marvell-nand.txt
>
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:
./Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml:17:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
MAINTAINERS: Documentation/devicetree/bindings/mtd/marvell-nand.txt
See https://patchwork.ozlabs.org/patch/
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 21/10/2022 15:45, 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 <[email protected]>
> ---
> .../bindings/mtd/marvell,nand-controller.yaml | 199 ++++++++++++++++++
> .../devicetree/bindings/mtd/marvell-nand.txt | 126 -----------
> 2 files changed, 199 insertions(+), 126 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..535b7f8903c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml
> @@ -0,0 +1,199 @@
> +# 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 <[email protected]>
This should be someone responsible for hardware, not subsystem
maintainer. Unless by coincidence Miquel matches both. :)
> +
> +properties:
> +
> + compatible:
> + oneOf:
> + - items:
> + - const: marvell,armada-8k-nand-controller
> + - const: marvell,armada370-nand-controller
Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
> + - const: marvell,armada370-nand-controller
> + - const: marvell,pxa3xx-nand-controller
These two are just enum.
> +
> + reg:
> + maxItems: 1
> +
> + "#address-cells":
> + const: 1
Drop, comes with nand-controller.yaml
> +
> + "#size-cells":
> + const: 0
Ditto
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + minItems: 1
> + maxItems: 2
> + description: |
No need for |
> + Shall reference the NAND controller clocks, the second one is
> + is only needed for the Armada 7K/8K SoCs
You need allOf:if:then restricting it further per variant.
> +
> + clock-names:
> + items:
> + - const: core
> + - const: reg
> + description: |
> + Mandatory if there is a second clock, in this case there
> + should be one clock named "core" and another one named "reg"
The message is confusing. What is mandatory if there is a second clock?
Plus, the binding requires two clocks.
Drop entire description.
minItems: 1
> +
> + dmas:
> + maxItems: 1
> + description: rxtx DMA channel
Drop description.
> +
> + 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:
> +
Drop blank line.
> + 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:
> + const: 512
Why this is const?
> + 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"
Drop quotes
unevalautedProperties: false
and then you will see errors, because you referenced schema for one
partition.
> +
> + 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
> +
> + additionalProperties: false
> +
> + required:
> + - reg
> + - nand-rb
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - "#address-cells"
> + - "#size-cells"
Drop these two - required by nand-controller.
> + - interrupts
> + - clocks
> +
> +allOf:
> + - $ref: "nand-controller.yaml#"
Drop quotes.
> +
> + - 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
> + 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>;
Use 4 spaces for example indentation.
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&coredivclk 0>;
> +
Best regards,
Krzysztof
Hello,
[email protected] wrote on Sat, 22 Oct 2022 12:18:49 -0400:
> On 21/10/2022 15:45, 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 <[email protected]>
> > ---
> > .../bindings/mtd/marvell,nand-controller.yaml | 199 ++++++++++++++++++
> > .../devicetree/bindings/mtd/marvell-nand.txt | 126 -----------
> > 2 files changed, 199 insertions(+), 126 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..535b7f8903c8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml
> > @@ -0,0 +1,199 @@
> > +# 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 <[email protected]>
>
> This should be someone responsible for hardware, not subsystem
> maintainer. Unless by coincidence Miquel matches both. :)
Haha, actually I do because I rewrote this driver entirely few years
ago. I don't actively maintain these platforms anymore but I don't mind
being assigned here if nobody else cares. Otherwise indeed, Vadym
or someone else from Marvell can take over, I don't mind.
Thanks,
Miquèl
Hi Krzysztof,
On Sat, 22 Oct 2022 12:18:49 -0400, Krzysztof Kozlowski <[email protected]> wrote:
> On 21/10/2022 15:45, 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 <[email protected]>
> > ---
> > .../bindings/mtd/marvell,nand-controller.yaml | 199 ++++++++++++++++++
> > .../devicetree/bindings/mtd/marvell-nand.txt | 126 -----------
> > 2 files changed, 199 insertions(+), 126 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..535b7f8903c8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml
> > @@ -0,0 +1,199 @@
> > +# 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 <[email protected]>
>
> This should be someone responsible for hardware, not subsystem
> maintainer. Unless by coincidence Miquel matches both. :)
>
> > +
> > +properties:
> > +
> > + compatible:
> > + oneOf:
> > + - items:
> > + - const: marvell,armada-8k-nand-controller
> > + - const: marvell,armada370-nand-controller
>
> Does not look like you tested the bindings. Please run `make
> dt_binding_check` (see
> Documentation/devicetree/bindings/writing-schema.rst for instructions).
Yes, on v1 I did not use yamllint, but installed after Rob pointed
on some lint warnings.
>
> > + - const: marvell,armada370-nand-controller
> > + - const: marvell,pxa3xx-nand-controller
>
> These two are just enum.
>
OK.
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + "#address-cells":
> > + const: 1
>
> Drop, comes with nand-controller.yaml
>
OK.
> > +
> > + "#size-cells":
> > + const: 0
>
> Ditto
>
OK.
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + minItems: 1
> > + maxItems: 2
> > + description: |
>
> No need for |
>
> > + Shall reference the NAND controller clocks, the second one is
> > + is only needed for the Armada 7K/8K SoCs
>
> You need allOf:if:then restricting it further per variant.
>
OK, added.
> > +
> > + clock-names:
> > + items:
> > + - const: core
> > + - const: reg
> > + description: |
> > + Mandatory if there is a second clock, in this case there
> > + should be one clock named "core" and another one named "reg"
>
> The message is confusing. What is mandatory if there is a second clock?
> Plus, the binding requires two clocks.
>
> Drop entire description.
>
> minItems: 1
>
OK, droped (I used from the txt version).
Added minItems.
>
> > +
> > + dmas:
> > + maxItems: 1
> > + description: rxtx DMA channel
>
> Drop description.
>
OK.
> > +
> > + 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:
> > +
>
> Drop blank line.
>
OK.
> > + 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:
> > + const: 512
>
> Why this is const?
>
Removed const.
> > + 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"
>
> Drop quotes
>
OK.
> unevalautedProperties: false
>
> and then you will see errors, because you referenced schema for one
> partition.
>
Hm, I did not see errors with partitions with- or without "unevaluatedProperties".
>
> > +
> > + 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
> > +
> > + additionalProperties: false
> > +
> > + required:
> > + - reg
> > + - nand-rb
> > +
> > +additionalProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - "#address-cells"
> > + - "#size-cells"
>
> Drop these two - required by nand-controller.
OK.
>
> > + - interrupts
> > + - clocks
> > +
> > +allOf:
> > + - $ref: "nand-controller.yaml#"
>
> Drop quotes.
>
OK.
> > +
> > + - 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
> > + 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>;
>
> Use 4 spaces for example indentation.
>
OK.
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&coredivclk 0>;
> > +
>
> Best regards,
> Krzysztof
>
Thanks for the comments.
On 24/10/2022 15:48, Vadym Kochan wrote:
> Hi Krzysztof,
>
> On Sat, 22 Oct 2022 12:18:49 -0400, Krzysztof Kozlowski <[email protected]> wrote:
>> On 21/10/2022 15:45, 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 <[email protected]>
>>> ---
>>> .../bindings/mtd/marvell,nand-controller.yaml | 199 ++++++++++++++++++
>>> .../devicetree/bindings/mtd/marvell-nand.txt | 126 -----------
>>> 2 files changed, 199 insertions(+), 126 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..535b7f8903c8
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml
>>> @@ -0,0 +1,199 @@
>>> +# 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 <[email protected]>
>>
>> This should be someone responsible for hardware, not subsystem
>> maintainer. Unless by coincidence Miquel matches both. :)
>>
>>> +
>>> +properties:
>>> +
>>> + compatible:
>>> + oneOf:
>>> + - items:
>>> + - const: marvell,armada-8k-nand-controller
>>> + - const: marvell,armada370-nand-controller
>>
>> Does not look like you tested the bindings. Please run `make
>> dt_binding_check` (see
>> Documentation/devicetree/bindings/writing-schema.rst for instructions).
>
> Yes, on v1 I did not use yamllint, but installed after Rob pointed
> on some lint warnings.
I did not say about yamllint.
>
>>
>>> + - const: marvell,armada370-nand-controller
>>> + - const: marvell,pxa3xx-nand-controller
>>
>> These two are just enum.
>>
>
> OK.
>
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + "#address-cells":
>>> + const: 1
>>
>> Drop, comes with nand-controller.yaml
>>
>
> OK.
>
>>> +
>>> + "#size-cells":
>>> + const: 0
>>
>> Ditto
>>
>
> OK.
>
>>> +
>>> + interrupts:
>>> + maxItems: 1
>>> +
>>> + clocks:
>>> + minItems: 1
>>> + maxItems: 2
>>> + description: |
>>
>> No need for |
>>
>>> + Shall reference the NAND controller clocks, the second one is
>>> + is only needed for the Armada 7K/8K SoCs
>>
>> You need allOf:if:then restricting it further per variant.
>>
>
> OK, added.
>
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: core
>>> + - const: reg
>>> + description: |
>>> + Mandatory if there is a second clock, in this case there
>>> + should be one clock named "core" and another one named "reg"
>>
>> The message is confusing. What is mandatory if there is a second clock?
>> Plus, the binding requires two clocks.
>>
>> Drop entire description.
>>
>> minItems: 1
>>
>
> OK, droped (I used from the txt version).
> Added minItems.
>
>>
>>> +
>>> + dmas:
>>> + maxItems: 1
>>> + description: rxtx DMA channel
>>
>> Drop description.
>>
>
> OK.
>
>>> +
>>> + 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:
>>> +
>>
>> Drop blank line.
>>
>
> OK.
>
>>> + 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:
>>> + const: 512
>>
>> Why this is const?
>>
>
> Removed const.
>
>>> + 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"
>>
>> Drop quotes
>>
>
> OK.
>
>> unevalautedProperties: false
>>
>> and then you will see errors, because you referenced schema for one
>> partition.
>>
>
> Hm, I did not see errors with partitions with- or without "unevaluatedProperties".
As pointed before and here - I am not sure if you tested the bindings,
so of course then will be no warnings...
Best regards,
Krzysztof