* [PATCH v5 0/2] dt-bindings: mtd: marvell-nand: Add YAML scheme
@ 2022-10-26 13:45 Vadym Kochan
2022-10-26 13:45 ` [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme Vadym Kochan
2022-10-26 13:45 ` [PATCH v5 2/2] arm64: dts: marvell: cp11x: Fix nand_controller node name according to YAML Vadym Kochan
0 siblings, 2 replies; 15+ messages in thread
From: Vadym Kochan @ 2022-10-26 13:45 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-mtd, devicetree, linux-kernel,
linux-arm-kernel
Cc: Elad Nachman, Vadym Kochan, Chris Packham
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
v5:
1) Get back "label" and "partitions" properties but without
ref to the "partition.yaml" which was wrongly used.
2) Add "additionalProperties: false" for nand@ because all possible
properties are described.
v4:
1) Remove "label" and "partitions" properties
2) Use 2 clocks for A7K/8K platform which is a requirement
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
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 | 195 ++++++++++++++++++
.../devicetree/bindings/mtd/marvell-nand.txt | 126 -----------
MAINTAINERS | 1 -
arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 2 +-
4 files changed, 196 insertions(+), 128 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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-26 13:45 [PATCH v5 0/2] dt-bindings: mtd: marvell-nand: Add YAML scheme Vadym Kochan
@ 2022-10-26 13:45 ` Vadym Kochan
2022-10-26 14:03 ` Krzysztof Kozlowski
2022-10-26 13:45 ` [PATCH v5 2/2] arm64: dts: marvell: cp11x: Fix nand_controller node name according to YAML Vadym Kochan
1 sibling, 1 reply; 15+ messages in thread
From: Vadym Kochan @ 2022-10-26 13:45 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-mtd, devicetree, linux-kernel,
linux-arm-kernel
Cc: Elad Nachman, Vadym Kochan, Chris Packham
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>
---
v5:
1) Get back "label" and "partitions" properties but without
ref to the "partition.yaml" which was wrongly used.
2) Add "additionalProperties: false" for nand@ because all possible
properties are described.
v4:
1) Remove "label" and "partitions" properties
2) Use 2 clocks for A7K/8K platform which is a requirement
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 | 195 ++++++++++++++++++
.../devicetree/bindings/mtd/marvell-nand.txt | 126 -----------
MAINTAINERS | 1 -
3 files changed, 195 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..544e98ed12bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml
@@ -0,0 +1,195 @@
+# 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:
+ 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
+
+ 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
+
+ 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
+
+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
+ maxItems: 2
+
+ clock-names:
+ minItems: 2
+ else:
+ properties:
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ maxItems: 1
+
+ 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
--
2.17.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v5 2/2] arm64: dts: marvell: cp11x: Fix nand_controller node name according to YAML
2022-10-26 13:45 [PATCH v5 0/2] dt-bindings: mtd: marvell-nand: Add YAML scheme Vadym Kochan
2022-10-26 13:45 ` [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme Vadym Kochan
@ 2022-10-26 13:45 ` Vadym Kochan
1 sibling, 0 replies; 15+ messages in thread
From: Vadym Kochan @ 2022-10-26 13:45 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-mtd, devicetree, linux-kernel,
linux-arm-kernel
Cc: Elad Nachman, Vadym Kochan, Chris Packham
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 <vadym.kochan@plvision.eu>
---
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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-26 13:45 ` [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme Vadym Kochan
@ 2022-10-26 14:03 ` Krzysztof Kozlowski
2022-10-26 21:57 ` Vadym Kochan
0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-26 14:03 UTC (permalink / raw)
To: Vadym Kochan, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski,
Andrew Lunn, Gregory Clement, Sebastian Hesselbarth, linux-mtd,
devicetree, linux-kernel, linux-arm-kernel
Cc: Elad Nachman, Chris Packham
On 26/10/2022 09: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 <vadym.kochan@plvision.eu>
> ---
>
> v5:
> 1) Get back "label" and "partitions" properties but without
> ref to the "partition.yaml" which was wrongly used.
>
> 2) Add "additionalProperties: false" for nand@ because all possible
> properties are described.
>
> v4:
> 1) Remove "label" and "partitions" properties
>
> 2) Use 2 clocks for A7K/8K platform which is a requirement
>
> 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 | 195 ++++++++++++++++++
> .../devicetree/bindings/mtd/marvell-nand.txt | 126 -----------
> MAINTAINERS | 1 -
> 3 files changed, 195 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..544e98ed12bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml
> @@ -0,0 +1,195 @@
> +# 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:
What happened to maxItems here? This is wrong. You keep changing random
things, again. V3 was correct.
> + 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
> +
> + 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
That's not what I asked for. Like four times I asked you to add here
unevaluatedProperties: false and I never said that ref to partition.yaml
should be removed and you... instead remove that ref.
You need to define here children and specify their ref.
You must use unevaluatedProperties: false here. So this is fifth time I
am writing this feedback.
> +
> + 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
> +
> +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
> + maxItems: 2
This does not make sense now...
Best regards,
Krzysztof
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-26 14:03 ` Krzysztof Kozlowski
@ 2022-10-26 21:57 ` Vadym Kochan
2022-10-27 13:02 ` Krzysztof Kozlowski
0 siblings, 1 reply; 15+ messages in thread
From: Vadym Kochan @ 2022-10-26 21:57 UTC (permalink / raw)
To: Krzysztof Kozlowski, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski,
Andrew Lunn, Gregory Clement, Sebastian Hesselbarth, linux-mtd,
devicetree, linux-kernel, linux-arm-kernel
Cc: Elad Nachman, Chris Packham
Hi Krzysztof,
On Wed, 26 Oct 2022 10:03:51 -0400, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> On 26/10/2022 09: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 <vadym.kochan@plvision.eu>
> > ---
> >
> > v5:
> > 1) Get back "label" and "partitions" properties but without
> > ref to the "partition.yaml" which was wrongly used.
> >
> > 2) Add "additionalProperties: false" for nand@ because all possible
> > properties are described.
> >
> > v4:
> > 1) Remove "label" and "partitions" properties
> >
> > 2) Use 2 clocks for A7K/8K platform which is a requirement
> >
> > 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 | 195 ++++++++++++++++++
> > .../devicetree/bindings/mtd/marvell-nand.txt | 126 -----------
> > MAINTAINERS | 1 -
> > 3 files changed, 195 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..544e98ed12bb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml
> > @@ -0,0 +1,195 @@
> > +# 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:
>
> What happened to maxItems here? This is wrong. You keep changing random
> things, again. V3 was correct.
>
It is not random, I tried to enforce to use 2 clocks for A7k/8K case.
>
> > + 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
> > +
> > + 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
>
> That's not what I asked for. Like four times I asked you to add here
> unevaluatedProperties: false and I never said that ref to partition.yaml
> should be removed and you... instead remove that ref.
>
> You need to define here children and specify their ref.
>
> You must use unevaluatedProperties: false here. So this is fifth time I
> am writing this feedback.
>
>
It is a bit confusing that it is needed to define "partitions" and "label" rules particulary
in this nand controller instead of some common place like nand-chip.yaml, these properties
are common also for the other nand controllers.
>
> > +
> > + 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
> > +
> > +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
> > + maxItems: 2
>
> This does not make sense now...
>
> Best regards,
> Krzysztof
>
Thank you,
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-26 21:57 ` Vadym Kochan
@ 2022-10-27 13:02 ` Krzysztof Kozlowski
2022-10-27 13:18 ` Miquel Raynal
0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-27 13:02 UTC (permalink / raw)
To: Vadym Kochan, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Rob Herring, Krzysztof Kozlowski,
Andrew Lunn, Gregory Clement, Sebastian Hesselbarth, linux-mtd,
devicetree, linux-kernel, linux-arm-kernel
Cc: Elad Nachman, Chris Packham
On 26/10/2022 17:57, Vadym Kochan wrote:
>>> + interrupts:
>>> + maxItems: 1
>>> +
>>> + clocks:
>>
>> What happened to maxItems here? This is wrong. You keep changing random
>> things, again. V3 was correct.
>>
>
> It is not random, I tried to enforce to use 2 clocks for A7k/8K case.
I think I gave you example how these clocks are being enforced, but
let's paste it here one more time:
https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L38
If you have always two clocks, you just put here maxItems and skip the
allOf:if:then.
If you have here for some variants one clock, for some two, then define
wide constraints here and narrow them for each variant in allOf:if:then
("else:")
>
>>
>>> + 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
>>> +
>>> + 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
>>
>> That's not what I asked for. Like four times I asked you to add here
>> unevaluatedProperties: false and I never said that ref to partition.yaml
>> should be removed and you... instead remove that ref.
>>
>> You need to define here children and specify their ref.
>>
>> You must use unevaluatedProperties: false here. So this is fifth time I
>> am writing this feedback.
>>
>>
>
> It is a bit confusing that it is needed to define "partitions" and "label" rules particulary
> in this nand controller instead of some common place like nand-chip.yaml, these properties
> are common also for the other nand controllers.
No one speaks about label, I never commented about label, I think...
If you think the property is really generic and every NAND controller
bindings implement it, then feel free to include them there, in a
separate patch. It sounds sensible, but I did not check other bindings.
Best regards,
Krzysztof
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-27 13:02 ` Krzysztof Kozlowski
@ 2022-10-27 13:18 ` Miquel Raynal
2022-10-27 13:24 ` Krzysztof Kozlowski
0 siblings, 1 reply; 15+ messages in thread
From: Miquel Raynal @ 2022-10-27 13:18 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vadym Kochan, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-mtd, devicetree, linux-kernel,
linux-arm-kernel, Elad Nachman, Chris Packham
Hi Vadym,
> >>> +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
> >>
> >> That's not what I asked for. Like four times I asked you to add here
> >> unevaluatedProperties: false and I never said that ref to partition.yaml
> >> should be removed and you... instead remove that ref.
> >>
> >> You need to define here children and specify their ref.
> >>
> >> You must use unevaluatedProperties: false here. So this is fifth time I
> >> am writing this feedback.
> >>
> >>
> >
> > It is a bit confusing that it is needed to define "partitions" and "label" rules particulary
> > in this nand controller instead of some common place like nand-chip.yaml, these properties
> > are common also for the other nand controllers.
>
> No one speaks about label, I never commented about label, I think...
>
> If you think the property is really generic and every NAND controller
> bindings implement it, then feel free to include them there, in a
> separate patch. It sounds sensible, but I did not check other bindings.
FYI, label is already defined in mtd/mtd.yaml.
Partitions do not need to be defined in your binding, just don't put
any in your example and you'll be fine. These partitions are either
static and may be described in the DT (see
mtd/partition/partition.yaml) or there is some dynamic discovery
involved and a proper parser shall be referenced (parsers have their
own binding).
Cheers,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-27 13:18 ` Miquel Raynal
@ 2022-10-27 13:24 ` Krzysztof Kozlowski
2022-10-27 13:50 ` Miquel Raynal
0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-27 13:24 UTC (permalink / raw)
To: Miquel Raynal
Cc: Vadym Kochan, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-mtd, devicetree, linux-kernel,
linux-arm-kernel, Elad Nachman, Chris Packham
On 27/10/2022 09:18, Miquel Raynal wrote:
> Hi Vadym,
>
>>>>> +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
>>>>
>>>> That's not what I asked for. Like four times I asked you to add here
>>>> unevaluatedProperties: false and I never said that ref to partition.yaml
>>>> should be removed and you... instead remove that ref.
>>>>
>>>> You need to define here children and specify their ref.
>>>>
>>>> You must use unevaluatedProperties: false here. So this is fifth time I
>>>> am writing this feedback.
>>>>
>>>>
>>>
>>> It is a bit confusing that it is needed to define "partitions" and "label" rules particulary
>>> in this nand controller instead of some common place like nand-chip.yaml, these properties
>>> are common also for the other nand controllers.
>>
>> No one speaks about label, I never commented about label, I think...
>>
>> If you think the property is really generic and every NAND controller
>> bindings implement it, then feel free to include them there, in a
>> separate patch. It sounds sensible, but I did not check other bindings.
>
> FYI, label is already defined in mtd/mtd.yaml.
Which is not included here and in nand-controller.yaml
> Partitions do not need to be defined in your binding, just don't put
> any in your example and you'll be fine. These partitions are either
> static and may be described in the DT (see
> mtd/partition/partition.yaml) or there is some dynamic discovery
> involved and a proper parser shall be referenced (parsers have their
> own binding).
I don't think this is correct. Basically you allow any node to be under
partitions as there is no schema validating them (without compatibles).
Best regards,
Krzysztof
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-27 13:24 ` Krzysztof Kozlowski
@ 2022-10-27 13:50 ` Miquel Raynal
2022-10-27 14:51 ` Krzysztof Kozlowski
0 siblings, 1 reply; 15+ messages in thread
From: Miquel Raynal @ 2022-10-27 13:50 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vadym Kochan, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-mtd, devicetree, linux-kernel,
linux-arm-kernel, Elad Nachman, Chris Packham
Hi Krzysztof,
krzysztof.kozlowski@linaro.org wrote on Thu, 27 Oct 2022 09:24:24 -0400:
> On 27/10/2022 09:18, Miquel Raynal wrote:
> > Hi Vadym,
> >
> >>>>> +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
> >>>>
> >>>> That's not what I asked for. Like four times I asked you to add here
> >>>> unevaluatedProperties: false and I never said that ref to partition.yaml
> >>>> should be removed and you... instead remove that ref.
> >>>>
> >>>> You need to define here children and specify their ref.
> >>>>
> >>>> You must use unevaluatedProperties: false here. So this is fifth time I
> >>>> am writing this feedback.
> >>>>
> >>>>
> >>>
> >>> It is a bit confusing that it is needed to define "partitions" and "label" rules particulary
> >>> in this nand controller instead of some common place like nand-chip.yaml, these properties
> >>> are common also for the other nand controllers.
> >>
> >> No one speaks about label, I never commented about label, I think...
> >>
> >> If you think the property is really generic and every NAND controller
> >> bindings implement it, then feel free to include them there, in a
> >> separate patch. It sounds sensible, but I did not check other bindings.
> >
> > FYI, label is already defined in mtd/mtd.yaml.
>
> Which is not included here and in nand-controller.yaml
Maybe nand-chip.yaml should?
> > Partitions do not need to be defined in your binding, just don't put
> > any in your example and you'll be fine. These partitions are either
> > static and may be described in the DT (see
> > mtd/partition/partition.yaml) or there is some dynamic discovery
> > involved and a proper parser shall be referenced (parsers have their
> > own binding).
>
> I don't think this is correct. Basically you allow any node to be under
> partitions as there is no schema validating them (without compatibles).
Sorry if that was unclear, what I meant is: partitions should not be
defined in the bindings for Marvell NAND controller because they should
be defined somewhere else already.
NAND controller subnodes should define the storage devices (the
flashes themselves) connected to the controller. "nand-chip.yaml"
describes generic properties for these. Additional subnodes are allowed
and expected to be partitions (this is not enforced anywhere I think),
they should use one of the existing compatibles to define the parser.
The most common parser is named fixed-partitions and has its own
compatible. Every parser references partitions.yaml.
There are a few controller bindings however which reference
partition.yaml anyway, probably to make the examples validation work,
I'm not sure it should be done like that though:
https://elixir.bootlin.com/linux/v6.0/source/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
https://elixir.bootlin.com/linux/v6.0/source/Documentation/devicetree/bindings/mtd/ti,gpmc-onenand.yaml
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-27 13:50 ` Miquel Raynal
@ 2022-10-27 14:51 ` Krzysztof Kozlowski
2022-10-28 7:47 ` Miquel Raynal
0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-27 14:51 UTC (permalink / raw)
To: Miquel Raynal
Cc: Vadym Kochan, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-mtd, devicetree, linux-kernel,
linux-arm-kernel, Elad Nachman, Chris Packham
On 27/10/2022 09:50, Miquel Raynal wrote:
> Hi Krzysztof,
>
> krzysztof.kozlowski@linaro.org wrote on Thu, 27 Oct 2022 09:24:24 -0400:
>
>> On 27/10/2022 09:18, Miquel Raynal wrote:
>>> Hi Vadym,
>>>
>>>>>>> +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
>>>>>>
>>>>>> That's not what I asked for. Like four times I asked you to add here
>>>>>> unevaluatedProperties: false and I never said that ref to partition.yaml
>>>>>> should be removed and you... instead remove that ref.
>>>>>>
>>>>>> You need to define here children and specify their ref.
>>>>>>
>>>>>> You must use unevaluatedProperties: false here. So this is fifth time I
>>>>>> am writing this feedback.
>>>>>>
>>>>>>
>>>>>
>>>>> It is a bit confusing that it is needed to define "partitions" and "label" rules particulary
>>>>> in this nand controller instead of some common place like nand-chip.yaml, these properties
>>>>> are common also for the other nand controllers.
>>>>
>>>> No one speaks about label, I never commented about label, I think...
>>>>
>>>> If you think the property is really generic and every NAND controller
>>>> bindings implement it, then feel free to include them there, in a
>>>> separate patch. It sounds sensible, but I did not check other bindings.
>>>
>>> FYI, label is already defined in mtd/mtd.yaml.
>>
>> Which is not included here and in nand-controller.yaml
>
> Maybe nand-chip.yaml should?
mtd.yaml looks a bit more than that - also allows nvmem nodes. Maybe
let's just add label to nand-chip?
>
>>> Partitions do not need to be defined in your binding, just don't put
>>> any in your example and you'll be fine. These partitions are either
>>> static and may be described in the DT (see
>>> mtd/partition/partition.yaml) or there is some dynamic discovery
>>> involved and a proper parser shall be referenced (parsers have their
>>> own binding).
>>
>> I don't think this is correct. Basically you allow any node to be under
>> partitions as there is no schema validating them (without compatibles).
>
> Sorry if that was unclear, what I meant is: partitions should not be
> defined in the bindings for Marvell NAND controller because they should
> be defined somewhere else already.
Ah, right. Then it seems reasonable.
>
> NAND controller subnodes should define the storage devices (the
> flashes themselves) connected to the controller. "nand-chip.yaml"
> describes generic properties for these. Additional subnodes are allowed
> and expected to be partitions (this is not enforced anywhere I think),
> they should use one of the existing compatibles to define the parser.
> The most common parser is named fixed-partitions and has its own
> compatible. Every parser references partitions.yaml.
>
> There are a few controller bindings however which reference
> partition.yaml anyway, probably to make the examples validation work,
> I'm not sure it should be done like that though:
> https://elixir.bootlin.com/linux/v6.0/source/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> https://elixir.bootlin.com/linux/v6.0/source/Documentation/devicetree/bindings/mtd/ti,gpmc-onenand.yaml
Yes, so the nand-chip implementation (like Marvell NAND) could reference
the parser and we would be done. If it doesn't, then we must have
generic partitions in the nand-chip.
Best regards,
Krzysztof
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-27 14:51 ` Krzysztof Kozlowski
@ 2022-10-28 7:47 ` Miquel Raynal
2022-10-28 11:31 ` Krzysztof Kozlowski
0 siblings, 1 reply; 15+ messages in thread
From: Miquel Raynal @ 2022-10-28 7:47 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vadym Kochan, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-mtd, devicetree, linux-kernel,
linux-arm-kernel, Elad Nachman, Chris Packham
Hi Krzysztof,
krzysztof.kozlowski@linaro.org wrote on Thu, 27 Oct 2022 10:51:29 -0400:
> On 27/10/2022 09:50, Miquel Raynal wrote:
> > Hi Krzysztof,
> >
> > krzysztof.kozlowski@linaro.org wrote on Thu, 27 Oct 2022 09:24:24 -0400:
> >
> >> On 27/10/2022 09:18, Miquel Raynal wrote:
> >>> Hi Vadym,
> >>>
> >>>>>>> +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
> >>>>>>
> >>>>>> That's not what I asked for. Like four times I asked you to add here
> >>>>>> unevaluatedProperties: false and I never said that ref to partition.yaml
> >>>>>> should be removed and you... instead remove that ref.
> >>>>>>
> >>>>>> You need to define here children and specify their ref.
> >>>>>>
> >>>>>> You must use unevaluatedProperties: false here. So this is fifth time I
> >>>>>> am writing this feedback.
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> It is a bit confusing that it is needed to define "partitions" and "label" rules particulary
> >>>>> in this nand controller instead of some common place like nand-chip.yaml, these properties
> >>>>> are common also for the other nand controllers.
> >>>>
> >>>> No one speaks about label, I never commented about label, I think...
> >>>>
> >>>> If you think the property is really generic and every NAND controller
> >>>> bindings implement it, then feel free to include them there, in a
> >>>> separate patch. It sounds sensible, but I did not check other bindings.
> >>>
> >>> FYI, label is already defined in mtd/mtd.yaml.
> >>
> >> Which is not included here and in nand-controller.yaml
> >
> > Maybe nand-chip.yaml should?
>
> mtd.yaml looks a bit more than that - also allows nvmem nodes. Maybe
> let's just add label to nand-chip?
I don't get the reason behind this proposal, mtd.yaml really is
kind of a definition of generic properties any mtd device might
have, so duplicating label (or whatever else inside) does not seem
legitimate to me. The jedec,spi-nor.yaml file already references it for
instance.
> >>> Partitions do not need to be defined in your binding, just don't put
> >>> any in your example and you'll be fine. These partitions are either
> >>> static and may be described in the DT (see
> >>> mtd/partition/partition.yaml) or there is some dynamic discovery
> >>> involved and a proper parser shall be referenced (parsers have their
> >>> own binding).
> >>
> >> I don't think this is correct. Basically you allow any node to be under
> >> partitions as there is no schema validating them (without compatibles).
> >
> > Sorry if that was unclear, what I meant is: partitions should not be
> > defined in the bindings for Marvell NAND controller because they should
> > be defined somewhere else already.
>
> Ah, right. Then it seems reasonable.
>
> >
> > NAND controller subnodes should define the storage devices (the
> > flashes themselves) connected to the controller. "nand-chip.yaml"
> > describes generic properties for these. Additional subnodes are allowed
> > and expected to be partitions (this is not enforced anywhere I think),
> > they should use one of the existing compatibles to define the parser.
> > The most common parser is named fixed-partitions and has its own
> > compatible. Every parser references partitions.yaml.
> >
> > There are a few controller bindings however which reference
> > partition.yaml anyway, probably to make the examples validation work,
> > I'm not sure it should be done like that though:
> > https://elixir.bootlin.com/linux/v6.0/source/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> > https://elixir.bootlin.com/linux/v6.0/source/Documentation/devicetree/bindings/mtd/ti,gpmc-onenand.yaml
>
>
> Yes, so the nand-chip implementation (like Marvell NAND) could reference
> the parser and we would be done. If it doesn't, then we must have
> generic partitions in the nand-chip.
In this case, I am not aware of any parser that would be relevant.
In the generic case, should we really reference a parser in particular?
If yes then maybe we should make a yaml file that just gathers all the
parsers and include it within mtd.yaml (and have it referenced in
nand-chip.yaml). What do you think?
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-28 7:47 ` Miquel Raynal
@ 2022-10-28 11:31 ` Krzysztof Kozlowski
2022-10-28 12:50 ` Miquel Raynal
0 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-28 11:31 UTC (permalink / raw)
To: Miquel Raynal
Cc: Vadym Kochan, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-mtd, devicetree, linux-kernel,
linux-arm-kernel, Elad Nachman, Chris Packham
On 28/10/2022 03:47, Miquel Raynal wrote:
> Hi Krzysztof,
>
> krzysztof.kozlowski@linaro.org wrote on Thu, 27 Oct 2022 10:51:29 -0400:
>
>> On 27/10/2022 09:50, Miquel Raynal wrote:
>>> Hi Krzysztof,
>>>
>>> krzysztof.kozlowski@linaro.org wrote on Thu, 27 Oct 2022 09:24:24 -0400:
>>>
>>>> On 27/10/2022 09:18, Miquel Raynal wrote:
>>>>> Hi Vadym,
>>>>>
>>>>>>>>> +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
>>>>>>>>
>>>>>>>> That's not what I asked for. Like four times I asked you to add here
>>>>>>>> unevaluatedProperties: false and I never said that ref to partition.yaml
>>>>>>>> should be removed and you... instead remove that ref.
>>>>>>>>
>>>>>>>> You need to define here children and specify their ref.
>>>>>>>>
>>>>>>>> You must use unevaluatedProperties: false here. So this is fifth time I
>>>>>>>> am writing this feedback.
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> It is a bit confusing that it is needed to define "partitions" and "label" rules particulary
>>>>>>> in this nand controller instead of some common place like nand-chip.yaml, these properties
>>>>>>> are common also for the other nand controllers.
>>>>>>
>>>>>> No one speaks about label, I never commented about label, I think...
>>>>>>
>>>>>> If you think the property is really generic and every NAND controller
>>>>>> bindings implement it, then feel free to include them there, in a
>>>>>> separate patch. It sounds sensible, but I did not check other bindings.
>>>>>
>>>>> FYI, label is already defined in mtd/mtd.yaml.
>>>>
>>>> Which is not included here and in nand-controller.yaml
>>>
>>> Maybe nand-chip.yaml should?
>>
>> mtd.yaml looks a bit more than that - also allows nvmem nodes. Maybe
>> let's just add label to nand-chip?
>
> I don't get the reason behind this proposal, mtd.yaml really is
> kind of a definition of generic properties any mtd device might
> have, so duplicating label (or whatever else inside) does not seem
> legitimate to me. The jedec,spi-nor.yaml file already references it for
> instance.
spi-nor is not a NAND chip... By including mtd.yaml in nand-chip you
also allow the NVMEM properties which are not applicable.
>
>>>>> Partitions do not need to be defined in your binding, just don't put
>>>>> any in your example and you'll be fine. These partitions are either
>>>>> static and may be described in the DT (see
>>>>> mtd/partition/partition.yaml) or there is some dynamic discovery
>>>>> involved and a proper parser shall be referenced (parsers have their
>>>>> own binding).
>>>>
>>>> I don't think this is correct. Basically you allow any node to be under
>>>> partitions as there is no schema validating them (without compatibles).
>>>
>>> Sorry if that was unclear, what I meant is: partitions should not be
>>> defined in the bindings for Marvell NAND controller because they should
>>> be defined somewhere else already.
>>
>> Ah, right. Then it seems reasonable.
>>
>>>
>>> NAND controller subnodes should define the storage devices (the
>>> flashes themselves) connected to the controller. "nand-chip.yaml"
>>> describes generic properties for these. Additional subnodes are allowed
>>> and expected to be partitions (this is not enforced anywhere I think),
>>> they should use one of the existing compatibles to define the parser.
>>> The most common parser is named fixed-partitions and has its own
>>> compatible. Every parser references partitions.yaml.
>>>
>>> There are a few controller bindings however which reference
>>> partition.yaml anyway, probably to make the examples validation work,
>>> I'm not sure it should be done like that though:
>>> https://elixir.bootlin.com/linux/v6.0/source/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
>>> https://elixir.bootlin.com/linux/v6.0/source/Documentation/devicetree/bindings/mtd/ti,gpmc-onenand.yaml
>>
>>
>> Yes, so the nand-chip implementation (like Marvell NAND) could reference
>> the parser and we would be done. If it doesn't, then we must have
>> generic partitions in the nand-chip.
>
> In this case, I am not aware of any parser that would be relevant.
>
> In the generic case, should we really reference a parser in particular?
> If yes then maybe we should make a yaml file that just gathers all the
> parsers and include it within mtd.yaml (and have it referenced in
> nand-chip.yaml). What do you think?
>
Not all MTD devices have partitions so putting this into mtd.yaml does
not look correct. Adding it into nand-chip seems fine.
Best regards,
Krzysztof
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-28 11:31 ` Krzysztof Kozlowski
@ 2022-10-28 12:50 ` Miquel Raynal
2022-10-31 12:29 ` Vadym Kochan
0 siblings, 1 reply; 15+ messages in thread
From: Miquel Raynal @ 2022-10-28 12:50 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vadym Kochan, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-mtd, devicetree, linux-kernel,
linux-arm-kernel, Elad Nachman, Chris Packham
Hi Krzysztof,
krzysztof.kozlowski@linaro.org wrote on Fri, 28 Oct 2022 07:31:39 -0400:
> On 28/10/2022 03:47, Miquel Raynal wrote:
> > Hi Krzysztof,
> >
> > krzysztof.kozlowski@linaro.org wrote on Thu, 27 Oct 2022 10:51:29 -0400:
> >
> >> On 27/10/2022 09:50, Miquel Raynal wrote:
> >>> Hi Krzysztof,
> >>>
> >>> krzysztof.kozlowski@linaro.org wrote on Thu, 27 Oct 2022 09:24:24 -0400:
> >>>
> >>>> On 27/10/2022 09:18, Miquel Raynal wrote:
> >>>>> Hi Vadym,
> >>>>>
> >>>>>>>>> +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
> >>>>>>>>
> >>>>>>>> That's not what I asked for. Like four times I asked you to add here
> >>>>>>>> unevaluatedProperties: false and I never said that ref to partition.yaml
> >>>>>>>> should be removed and you... instead remove that ref.
> >>>>>>>>
> >>>>>>>> You need to define here children and specify their ref.
> >>>>>>>>
> >>>>>>>> You must use unevaluatedProperties: false here. So this is fifth time I
> >>>>>>>> am writing this feedback.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>> It is a bit confusing that it is needed to define "partitions" and "label" rules particulary
> >>>>>>> in this nand controller instead of some common place like nand-chip.yaml, these properties
> >>>>>>> are common also for the other nand controllers.
> >>>>>>
> >>>>>> No one speaks about label, I never commented about label, I think...
> >>>>>>
> >>>>>> If you think the property is really generic and every NAND controller
> >>>>>> bindings implement it, then feel free to include them there, in a
> >>>>>> separate patch. It sounds sensible, but I did not check other bindings.
> >>>>>
> >>>>> FYI, label is already defined in mtd/mtd.yaml.
> >>>>
> >>>> Which is not included here and in nand-controller.yaml
> >>>
> >>> Maybe nand-chip.yaml should?
> >>
> >> mtd.yaml looks a bit more than that - also allows nvmem nodes. Maybe
> >> let's just add label to nand-chip?
> >
> > I don't get the reason behind this proposal, mtd.yaml really is
> > kind of a definition of generic properties any mtd device might
> > have, so duplicating label (or whatever else inside) does not seem
> > legitimate to me. The jedec,spi-nor.yaml file already references it for
> > instance.
>
> spi-nor is not a NAND chip... By including mtd.yaml in nand-chip you
> also allow the NVMEM properties which are not applicable.
MTD is an NVMEM provider, any MTD device (including NANDs) can use
these properties IMHO. It's not reserved to spi-nor (even though it is
more common, I conceed).
> >>>>> Partitions do not need to be defined in your binding, just don't put
> >>>>> any in your example and you'll be fine. These partitions are either
> >>>>> static and may be described in the DT (see
> >>>>> mtd/partition/partition.yaml) or there is some dynamic discovery
> >>>>> involved and a proper parser shall be referenced (parsers have their
> >>>>> own binding).
> >>>>
> >>>> I don't think this is correct. Basically you allow any node to be under
> >>>> partitions as there is no schema validating them (without compatibles).
> >>>
> >>> Sorry if that was unclear, what I meant is: partitions should not be
> >>> defined in the bindings for Marvell NAND controller because they should
> >>> be defined somewhere else already.
> >>
> >> Ah, right. Then it seems reasonable.
> >>
> >>>
> >>> NAND controller subnodes should define the storage devices (the
> >>> flashes themselves) connected to the controller. "nand-chip.yaml"
> >>> describes generic properties for these. Additional subnodes are allowed
> >>> and expected to be partitions (this is not enforced anywhere I think),
> >>> they should use one of the existing compatibles to define the parser.
> >>> The most common parser is named fixed-partitions and has its own
> >>> compatible. Every parser references partitions.yaml.
> >>>
> >>> There are a few controller bindings however which reference
> >>> partition.yaml anyway, probably to make the examples validation work,
> >>> I'm not sure it should be done like that though:
> >>> https://elixir.bootlin.com/linux/v6.0/source/Documentation/devicetree/bindings/mtd/ti,gpmc-nand.yaml
> >>> https://elixir.bootlin.com/linux/v6.0/source/Documentation/devicetree/bindings/mtd/ti,gpmc-onenand.yaml
> >>
> >>
> >> Yes, so the nand-chip implementation (like Marvell NAND) could reference
> >> the parser and we would be done. If it doesn't, then we must have
> >> generic partitions in the nand-chip.
> >
> > In this case, I am not aware of any parser that would be relevant.
> >
> > In the generic case, should we really reference a parser in particular?
> > If yes then maybe we should make a yaml file that just gathers all the
> > parsers and include it within mtd.yaml (and have it referenced in
> > nand-chip.yaml). What do you think?
> >
>
> Not all MTD devices have partitions so putting this into mtd.yaml does
> not look correct. Adding it into nand-chip seems fine.
Not all MTD devices have partitions but all of them can have
partitions. It's not a required subnode, but it is definitely common to
all mtd devices. I would then consider mtd.yaml a _very_ generic place
where we put anything that is not specific to the underlying storage
technology.
Anything that is specific to NAND goes into nand-chip.yaml or
nand-controller.yaml, anything specific to SPI-NOR goes into
jedec,spi-nor.yaml.
nand-chip.yaml and jedec,spi-nor.yaml should reference mtd.yaml.
mtd.yaml could probably reference some kind of "partition.yaml" to
define subnodes with partition parsers. I don't yet know exactly how to
make everything coherent but I liked the idea of having a file
referencing all the documented parsers, so that they could all apply if
necessary. I did not understand if you were in favor or opposed to this
idea? If opposed, how could we make all the partition parsers
schemas (and only them) to be validated as MTD devices subnodes?
Thanks a lot for all your feedback,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-28 12:50 ` Miquel Raynal
@ 2022-10-31 12:29 ` Vadym Kochan
2022-11-04 16:51 ` Miquel Raynal
0 siblings, 1 reply; 15+ messages in thread
From: Vadym Kochan @ 2022-10-31 12:29 UTC (permalink / raw)
To: Miquel Raynal, Krzysztof Kozlowski
Cc: Richard Weinberger, Vignesh Raghavendra, Rob Herring,
Krzysztof Kozlowski, Andrew Lunn,
Gregory
Clement, Sebastian Hesselbarth, linux-mtd, devicetree,
linux-kernel, linux-arm-kernel, Elad Nachman, Chris Packham
Hi Krzysztof, Miquel,
On Fri, 28 Oct 2022 14:50:49 +0200, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Hi Krzysztof,
>
[snip]
For me it looks like the "label" and "partitions" can be dropped from this series,
so in the future they can be handled by the common mtd-related yaml ?
Regards,
Vadym Kochan
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
2022-10-31 12:29 ` Vadym Kochan
@ 2022-11-04 16:51 ` Miquel Raynal
0 siblings, 0 replies; 15+ messages in thread
From: Miquel Raynal @ 2022-11-04 16:51 UTC (permalink / raw)
To: Vadym Kochan
Cc: Krzysztof Kozlowski, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Krzysztof Kozlowski, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, linux-mtd, devicetree, linux-kernel,
linux-arm-kernel, Elad Nachman, Chris Packham
Hi Vadym,
vadym.kochan@plvision.eu wrote on Mon, 31 Oct 2022 14:29:22 +0200:
> Hi Krzysztof, Miquel,
>
> On Fri, 28 Oct 2022 14:50:49 +0200, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > Hi Krzysztof,
> >
>
> [snip]
>
> For me it looks like the "label" and "partitions" can be dropped from this series,
> so in the future they can be handled by the common mtd-related yaml ?
I am currently working on this, see below. This series has not yet been
accepted but I believe you can rebase your work on top of it. Please be
sure your reference the right yaml files and do not redefine generic
properties (from mtd.yaml, nand-controller.yaml or nand-chip.yaml).
Link: https://lore.kernel.org/linux-mtd/20221104164718.1290859-1-miquel.raynal@bootlin.com/T/#mf2a94fa36b4760b29269c5b94a7dfcb533c8cafe
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-11-04 16:52 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-26 13:45 [PATCH v5 0/2] dt-bindings: mtd: marvell-nand: Add YAML scheme Vadym Kochan
2022-10-26 13:45 ` [PATCH v5 1/2] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme Vadym Kochan
2022-10-26 14:03 ` Krzysztof Kozlowski
2022-10-26 21:57 ` Vadym Kochan
2022-10-27 13:02 ` Krzysztof Kozlowski
2022-10-27 13:18 ` Miquel Raynal
2022-10-27 13:24 ` Krzysztof Kozlowski
2022-10-27 13:50 ` Miquel Raynal
2022-10-27 14:51 ` Krzysztof Kozlowski
2022-10-28 7:47 ` Miquel Raynal
2022-10-28 11:31 ` Krzysztof Kozlowski
2022-10-28 12:50 ` Miquel Raynal
2022-10-31 12:29 ` Vadym Kochan
2022-11-04 16:51 ` Miquel Raynal
2022-10-26 13:45 ` [PATCH v5 2/2] arm64: dts: marvell: cp11x: Fix nand_controller node name according to YAML Vadym Kochan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).