From: Vadym Kochan <[email protected]>
Switch the DT binding to a YAML schema to enable the DT validation.
The text binding didn't mention it as a requirement but existing usage
has
compatible = "marvell,armada-8k-nand-controller",
"marvell,armada370-nand-controller";
so the YAML allows this in addition to the individual compatible values.
There was also an incorrect reference to dma-names being "rxtx" where
the driver and existing device trees actually use dma-names = "data" so
this is corrected in the conversion.
Signed-off-by: Vadym Kochan <[email protected]>
Signed-off-by: Chris Packham <[email protected]>
---
Notes:
Changes in v9:
- depend on series from Miquel
https://lore.kernel.org/linux-mtd/[email protected]/
- enforce minimum/maximum for nand-rb
- move required: block for controller
- move unevaluatedProperties: for nand chip
- remove label, partitions and nand-on-flash-bbt which are covered by
generic schema
Changes in v8:
- Mark deprecated compatible values as such
- Allow "marvell,armada-8k-nand-controller" without
"marvell,armada370-nand-controller"
- Make dma-names usage reflect reality
- Update commit message
Changes in v7:
- Restore "label" and "partitions" properties (should be picked up via
nand-controller.yaml but aren't)
- Add/restore nand-on-flash-bbt and nand-ecc-mode which aren't covered
by nand-controller.yaml.
- Use "unevalautedProperties: false"
- Corrections for clock-names, dma-names, nand-rb and nand-ecc-strength
- Add pxa3xx-nand-controller example
Changes in v6:
- remove properties covered by nand-controller.yaml
- add example using armada-8k compatible
earlier changes:
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 | 218 ++++++++++++++++++
.../devicetree/bindings/mtd/marvell-nand.txt | 126 ----------
MAINTAINERS | 1 -
3 files changed, 218 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..252bd0f3e222
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml
@@ -0,0 +1,218 @@
+# 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
+ - enum:
+ - marvell,armada-8k-nand-controller
+ - marvell,armada370-nand-controller
+ - marvell,pxa3xx-nand-controller
+ - description: legacy bindings
+ deprecated: true
+ enum:
+ - marvell,armada-8k-nand
+ - marvell,armada370-nand
+ - marvell,pxa3xx-nand
+
+ 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
+ minItems: 1
+ maxItems: 2
+
+ clock-names:
+ minItems: 1
+ items:
+ - const: core
+ - const: reg
+
+ dmas:
+ maxItems: 1
+
+ dma-names:
+ items:
+ - const: data
+
+ marvell,system-controller:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: Syscon node that handles NAND controller related registers
+
+patternProperties:
+ "^nand@[a-f0-9]$":
+ type: object
+ $ref: raw-nand-chip.yaml
+
+ properties:
+ reg:
+ minimum: 0
+ maximum: 3
+
+ nand-rb:
+ items:
+ - minimum: 0
+ maximum: 1
+
+ nand-ecc-step-size:
+ const: 512
+
+ nand-ecc-strength:
+ enum: [1, 4, 8, 12, 16]
+
+ nand-ecc-mode:
+ const: hw
+
+ marvell,nand-keep-config:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ Orders the driver not to take the timings from the core and
+ leaving them completely untouched. Bootloader timings will then
+ be used.
+
+ marvell,nand-enable-arbiter:
+ $ref: /schemas/types.yaml#/definitions/flag
+ 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).
+ deprecated: true
+
+ 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
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: marvell,armada-8k-nand-controller
+ then:
+ properties:
+ clocks:
+ minItems: 2
+
+ clock-names:
+ minItems: 2
+
+ required:
+ - marvell,system-controller
+
+unevaluatedProperties: 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>;
+ };
+ };
+ };
+ };
+
+ - |
+ cp0_nand_controller: nand-controller@720000 {
+ compatible = "marvell,armada-8k-nand-controller",
+ "marvell,armada370-nand-controller";
+ reg = <0x720000 0x54>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <115 IRQ_TYPE_LEVEL_HIGH>;
+ clock-names = "core", "reg";
+ clocks = <&cp0_clk 1 2>,
+ <&cp0_clk 1 17>;
+ marvell,system-controller = <&cp0_syscon0>;
+
+ nand@0 {
+ reg = <0>;
+ label = "main-storage";
+ nand-rb = <0>;
+ nand-ecc-mode = "hw";
+ nand-ecc-strength = <8>;
+ nand-ecc-step-size = <512>;
+ };
+ };
+
+ - |
+ nand-controller@43100000 {
+ compatible = "marvell,pxa3xx-nand-controller";
+ reg = <0x43100000 90>;
+ interrupts = <45>;
+ clocks = <&clks 1>;
+ clock-names = "core";
+ dmas = <&pdma 97 3>;
+ dma-names = "data";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ nand@0 {
+ reg = <0>;
+ nand-rb = <0>;
+ nand-ecc-mode = "hw";
+ marvell,nand-keep-config;
+ };
+ };
+
+...
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 5d463f171c90..5342798d4e4f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12448,7 +12448,6 @@ MARVELL NAND CONTROLLER DRIVER
M: Miquel Raynal <[email protected]>
L: [email protected]
S: Maintained
-F: Documentation/devicetree/bindings/mtd/marvell-nand.txt
F: drivers/mtd/nand/raw/marvell_nand.c
MARVELL OCTEONTX2 PHYSICAL FUNCTION DRIVER
--
2.40.1
On Thu, 15 Jun 2023 16:04:47 +1200, Chris Packham wrote:
> From: Vadym Kochan <[email protected]>
>
> Switch the DT binding to a YAML schema to enable the DT validation.
>
> The text binding didn't mention it as a requirement but existing usage
> has
>
> compatible = "marvell,armada-8k-nand-controller",
> "marvell,armada370-nand-controller";
>
> so the YAML allows this in addition to the individual compatible values.
>
> There was also an incorrect reference to dma-names being "rxtx" where
> the driver and existing device trees actually use dma-names = "data" so
> this is corrected in the conversion.
>
> Signed-off-by: Vadym Kochan <[email protected]>
> Signed-off-by: Chris Packham <[email protected]>
> ---
>
> Notes:
> Changes in v9:
> - depend on series from Miquel
> https://lore.kernel.org/linux-mtd/[email protected]/
> - enforce minimum/maximum for nand-rb
> - move required: block for controller
> - move unevaluatedProperties: for nand chip
> - remove label, partitions and nand-on-flash-bbt which are covered by
> generic schema
>
> Changes in v8:
> - Mark deprecated compatible values as such
> - Allow "marvell,armada-8k-nand-controller" without
> "marvell,armada370-nand-controller"
> - Make dma-names usage reflect reality
> - Update commit message
>
> Changes in v7:
> - Restore "label" and "partitions" properties (should be picked up via
> nand-controller.yaml but aren't)
> - Add/restore nand-on-flash-bbt and nand-ecc-mode which aren't covered
> by nand-controller.yaml.
> - Use "unevalautedProperties: false"
> - Corrections for clock-names, dma-names, nand-rb and nand-ecc-strength
> - Add pxa3xx-nand-controller example
>
> Changes in v6:
> - remove properties covered by nand-controller.yaml
> - add example using armada-8k compatible
>
> earlier changes:
>
> 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 | 218 ++++++++++++++++++
> .../devicetree/bindings/mtd/marvell-nand.txt | 126 ----------
> MAINTAINERS | 1 -
> 3 files changed, 218 insertions(+), 127 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:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/mtd/raw-nand-chip.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/marvell,nand-controller.example.dtb: nand-controller@d0000: nand@0: False schema does not allow {'reg': [[0]], 'label': ['main-storage'], 'nand-rb': [[0]], 'nand-ecc-mode': ['hw'], 'marvell,nand-keep-config': True, 'nand-on-flash-bbt': True, 'nand-ecc-strength': [[4]], 'nand-ecc-step-size': [[512]], 'partitions': {'compatible': ['fixed-partitions'], '#address-cells': [[1]], '#size-cells': [[1]], 'partition@0': {'label': ['Rootfs'], 'reg': [[0, 1073741824]]}}}
from schema $id: http://devicetree.org/schemas/mtd/marvell,nand-controller.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/marvell,nand-controller.example.dtb: nand-controller@d0000: nand@0: Unevaluated properties are not allowed ('label', 'nand-on-flash-bbt', 'partitions' were unexpected)
from schema $id: http://devicetree.org/schemas/mtd/marvell,nand-controller.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/marvell,nand-controller.example.dtb: nand-controller@720000: nand@0: False schema does not allow {'reg': [[0]], 'label': ['main-storage'], 'nand-rb': [[0]], 'nand-ecc-mode': ['hw'], 'nand-ecc-strength': [[8]], 'nand-ecc-step-size': [[512]]}
from schema $id: http://devicetree.org/schemas/mtd/marvell,nand-controller.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/marvell,nand-controller.example.dtb: nand-controller@720000: nand@0: Unevaluated properties are not allowed ('label' was unexpected)
from schema $id: http://devicetree.org/schemas/mtd/marvell,nand-controller.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/marvell,nand-controller.example.dtb: nand-controller@43100000: nand@0: False schema does not allow {'reg': [[0]], 'nand-rb': [[0]], 'nand-ecc-mode': ['hw'], 'marvell,nand-keep-config': True}
from schema $id: http://devicetree.org/schemas/mtd/marvell,nand-controller.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On 15/06/2023 06:04, Chris Packham wrote:
> From: Vadym Kochan <[email protected]>
>
> Switch the DT binding to a YAML schema to enable the DT validation.
>
> The text binding didn't mention it as a requirement but existing usage
> has
>
> compatible = "marvell,armada-8k-nand-controller",
> "marvell,armada370-nand-controller";
>
> so the YAML allows this in addition to the individual compatible values.
>
> There was also an incorrect reference to dma-names being "rxtx" where
> the driver and existing device trees actually use dma-names = "data" so
> this is corrected in the conversion.
>
> Signed-off-by: Vadym Kochan <[email protected]>
> Signed-off-by: Chris Packham <[email protected]>
> ---
>
> Notes:
> Changes in v9:
> - depend on series from Miquel
> https://lore.kernel.org/linux-mtd/[email protected]/
> - enforce minimum/maximum for nand-rb
> - move required: block for controller
> - move unevaluatedProperties: for nand chip
> - remove label, partitions and nand-on-flash-bbt which are covered by
> generic schema
>
> Changes in v8:
> - Mark deprecated compatible values as such
> - Allow "marvell,armada-8k-nand-controller" without
> "marvell,armada370-nand-controller"
???
> +
> +properties:
> + compatible:
> + oneOf:
> + - items:
> + - const: marvell,armada-8k-nand-controller
> + - const: marvell,armada370-nand-controller
> + - enum:
> + - marvell,armada-8k-nand-controller
This is wrong. 8k cannot be both: compatible and not compatible with
370. It's not someone's cat to be in both states at the same time...
> + - marvell,armada370-nand-controller
> + - marvell,pxa3xx-nand-controller
> + - description: legacy bindings
> + deprecated: true
> + enum:
> + - marvell,armada-8k-nand
> + - marvell,armada370-nand
> + - marvell,pxa3xx-nand
> +
> + 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
> + minItems: 1
> + maxItems: 2
> +
> + clock-names:
> + minItems: 1
> + items:
> + - const: core
> + - const: reg
> +
> + dmas:
> + maxItems: 1
> +
> + dma-names:
> + items:
> + - const: data
> +
> + marvell,system-controller:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: Syscon node that handles NAND controller related registers
> +
> +patternProperties:
> + "^nand@[a-f0-9]$":
> + type: object
> + $ref: raw-nand-chip.yaml
> +
> + properties:
> + reg:
> + minimum: 0
> + maximum: 3
> +
> + nand-rb:
> + items:
> + - minimum: 0
> + maximum: 1
> +
> + nand-ecc-step-size:
> + const: 512
> +
> + nand-ecc-strength:
> + enum: [1, 4, 8, 12, 16]
> +
> + nand-ecc-mode:
> + const: hw
> +
> + marvell,nand-keep-config:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description: |
Do not need '|' unless you need to preserve formatting.
> + Orders the driver not to take the timings from the core and
> + leaving them completely untouched. Bootloader timings will then
> + be used.
> +
> + marvell,nand-enable-arbiter:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description: |
Do not need '|' unless you need to preserve formatting.
> + 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).
> + deprecated: true
> +
> + 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
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: marvell,armada-8k-nand-controller
> + then:
> + properties:
> + clocks:
> + minItems: 2
> +
> + clock-names:
> + minItems: 2
> +
> + required:
> + - marvell,system-controller
else:
narrow clocks to 1? Or is the second clod also valid for other variants?
Does not look like from your example.
Best regards,
Krzysztof
Hi Krzysztof,
On 15/06/23 19:45, Krzysztof Kozlowski wrote:
> On 15/06/2023 06:04, Chris Packham wrote:
>> From: Vadym Kochan <[email protected]>
>>
>> Switch the DT binding to a YAML schema to enable the DT validation.
>>
>> The text binding didn't mention it as a requirement but existing usage
>> has
>>
>> compatible = "marvell,armada-8k-nand-controller",
>> "marvell,armada370-nand-controller";
>>
>> so the YAML allows this in addition to the individual compatible values.
>>
>> There was also an incorrect reference to dma-names being "rxtx" where
>> the driver and existing device trees actually use dma-names = "data" so
>> this is corrected in the conversion.
>>
>> Signed-off-by: Vadym Kochan <[email protected]>
>> Signed-off-by: Chris Packham <[email protected]>
>> ---
>>
>> Notes:
>> Changes in v9:
>> - depend on series from Miquel https://lore.kernel.org/linux-mtd/[email protected]/
>> - enforce minimum/maximum for nand-rb
>> - move required: block for controller
>> - move unevaluatedProperties: for nand chip
>> - remove label, partitions and nand-on-flash-bbt which are covered by
>> generic schema
>>
>> Changes in v8:
>> - Mark deprecated compatible values as such
>> - Allow "marvell,armada-8k-nand-controller" without
>> "marvell,armada370-nand-controller"
> ???
>
>
>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + - items:
>> + - const: marvell,armada-8k-nand-controller
>> + - const: marvell,armada370-nand-controller
>> + - enum:
>> + - marvell,armada-8k-nand-controller
> This is wrong. 8k cannot be both: compatible and not compatible with
> 370. It's not someone's cat to be in both states at the same time...
The correct state (IMHO, Miquel correct me if I'm wrong) is `compatible
= "marvell,armada-8k-nand-controller";` as there are some 8K specific
requirements that aren't present on the 370 (specifically the
system-controller and the 2nd clock). The only reason `compatible =
"marvell,armada-8k-nand-controller",
"marvell,armada370-nand-controller";` is supported is because that is
what is in use in the existing dtses.
>> + - marvell,armada370-nand-controller
>> + - marvell,pxa3xx-nand-controller
>> + - description: legacy bindings
>> + deprecated: true
>> + enum:
>> + - marvell,armada-8k-nand
>> + - marvell,armada370-nand
>> + - marvell,pxa3xx-nand
>> +
>> + 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
>> + minItems: 1
>> + maxItems: 2
>> +
>> + clock-names:
>> + minItems: 1
>> + items:
>> + - const: core
>> + - const: reg
>> +
>> + dmas:
>> + maxItems: 1
>> +
>> + dma-names:
>> + items:
>> + - const: data
>> +
>> + marvell,system-controller:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description: Syscon node that handles NAND controller related registers
>> +
>> +patternProperties:
>> + "^nand@[a-f0-9]$":
>> + type: object
>> + $ref: raw-nand-chip.yaml
>> +
>> + properties:
>> + reg:
>> + minimum: 0
>> + maximum: 3
>> +
>> + nand-rb:
>> + items:
>> + - minimum: 0
>> + maximum: 1
>> +
>> + nand-ecc-step-size:
>> + const: 512
>> +
>> + nand-ecc-strength:
>> + enum: [1, 4, 8, 12, 16]
>> +
>> + nand-ecc-mode:
>> + const: hw
>> +
>> + marvell,nand-keep-config:
>> + $ref: /schemas/types.yaml#/definitions/flag
>> + description: |
> Do not need '|' unless you need to preserve formatting.
Ack
>> + Orders the driver not to take the timings from the core and
>> + leaving them completely untouched. Bootloader timings will then
>> + be used.
>> +
>> + marvell,nand-enable-arbiter:
>> + $ref: /schemas/types.yaml#/definitions/flag
>> + description: |
> Do not need '|' unless you need to preserve formatting.
>
>> + 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).
>> + deprecated: true
>> +
>> + 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
>> +
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: marvell,armada-8k-nand-controller
>> + then:
>> + properties:
>> + clocks:
>> + minItems: 2
>> +
>> + clock-names:
>> + minItems: 2
>> +
>> + required:
>> + - marvell,system-controller
> else:
> narrow clocks to 1? Or is the second clod also valid for other variants?
> Does not look like from your example.
Correct. Will update.
>
>
> Best regards,
> Krzysztof
>
On 16/06/2023 10:24, Miquel Raynal wrote:
> Hi Krzysztof,
>
> [email protected] wrote on Fri, 16 Jun 2023 10:15:31 +0200:
>
>> On 15/06/2023 23:06, Chris Packham wrote:
>>>>
>>>>> +
>>>>> +properties:
>>>>> + compatible:
>>>>> + oneOf:
>>>>> + - items:
>>>>> + - const: marvell,armada-8k-nand-controller
>>>>> + - const: marvell,armada370-nand-controller
>>>>> + - enum:
>>>>> + - marvell,armada-8k-nand-controller
>>>> This is wrong. 8k cannot be both: compatible and not compatible with
>>>> 370. It's not someone's cat to be in both states at the same time...
>>>
>>> The correct state (IMHO, Miquel correct me if I'm wrong) is `compatible
>>> = "marvell,armada-8k-nand-controller";` as there are some 8K specific
>>> requirements that aren't present on the 370 (specifically the
>>> system-controller and the 2nd clock).
>>
>> Presence of specific requirements does not invalidate compatibility. Two
>> devices are compatible if the 8k can bind and work with 370 compatible
>> string, even if this means some lower performance or less features (e.g.
>> subset of features).
>
> Quoting myself from 2019 (comment in the driver):
> "Some SoCs like A7k/A8k need to enable manually the NAND controller,
> gated clocks and reset bits to avoid being bootloader dependent."
>
> So can the 8k controller work using a 370 compatible : yes, if the
> booloader enabled the NAND controller already, no otherwise. But in
> practice it is the same controller.
>
> Given this information I don't know whether it makes sense to qualify
> the 8k controller compatible with the 370 compatible or not.
Indeed, a bit tricky decision. For me they could stay compatible,
because apparently programming model is the same or similar.
Best regards,
Krzysztof
Hi Krzysztof,
[email protected] wrote on Fri, 16 Jun 2023 10:15:31 +0200:
> On 15/06/2023 23:06, Chris Packham wrote:
> >>
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + oneOf:
> >>> + - items:
> >>> + - const: marvell,armada-8k-nand-controller
> >>> + - const: marvell,armada370-nand-controller
> >>> + - enum:
> >>> + - marvell,armada-8k-nand-controller
> >> This is wrong. 8k cannot be both: compatible and not compatible with
> >> 370. It's not someone's cat to be in both states at the same time...
> >
> > The correct state (IMHO, Miquel correct me if I'm wrong) is `compatible
> > = "marvell,armada-8k-nand-controller";` as there are some 8K specific
> > requirements that aren't present on the 370 (specifically the
> > system-controller and the 2nd clock).
>
> Presence of specific requirements does not invalidate compatibility. Two
> devices are compatible if the 8k can bind and work with 370 compatible
> string, even if this means some lower performance or less features (e.g.
> subset of features).
Quoting myself from 2019 (comment in the driver):
"Some SoCs like A7k/A8k need to enable manually the NAND controller,
gated clocks and reset bits to avoid being bootloader dependent."
So can the 8k controller work using a 370 compatible : yes, if the
booloader enabled the NAND controller already, no otherwise. But in
practice it is the same controller.
Given this information I don't know whether it makes sense to qualify
the 8k controller compatible with the 370 compatible or not.
> Now whether they are really compatible or not - I don't know. I expect
> the answer from you (as plural you).
:-)
> > The only reason `compatible =
> > "marvell,armada-8k-nand-controller",
> > "marvell,armada370-nand-controller";` is supported is because that is
> > what is in use in the existing dtses.
>
> Fix the DTS, use here the correct combination and, if it is a deviation
> from old binding, mention this in commit msg.
Fine by me.
> Best regards,
> Krzysztof
>
Thanks,
Miquèl
On 15/06/2023 23:06, Chris Packham wrote:
>>
>>> +
>>> +properties:
>>> + compatible:
>>> + oneOf:
>>> + - items:
>>> + - const: marvell,armada-8k-nand-controller
>>> + - const: marvell,armada370-nand-controller
>>> + - enum:
>>> + - marvell,armada-8k-nand-controller
>> This is wrong. 8k cannot be both: compatible and not compatible with
>> 370. It's not someone's cat to be in both states at the same time...
>
> The correct state (IMHO, Miquel correct me if I'm wrong) is `compatible
> = "marvell,armada-8k-nand-controller";` as there are some 8K specific
> requirements that aren't present on the 370 (specifically the
> system-controller and the 2nd clock).
Presence of specific requirements does not invalidate compatibility. Two
devices are compatible if the 8k can bind and work with 370 compatible
string, even if this means some lower performance or less features (e.g.
subset of features).
Now whether they are really compatible or not - I don't know. I expect
the answer from you (as plural you).
> The only reason `compatible =
> "marvell,armada-8k-nand-controller",
> "marvell,armada370-nand-controller";` is supported is because that is
> what is in use in the existing dtses.
Fix the DTS, use here the correct combination and, if it is a deviation
from old binding, mention this in commit msg.
Best regards,
Krzysztof
Hi Krzysztof,
[email protected] wrote on Fri, 16 Jun 2023 10:38:38 +0200:
> On 16/06/2023 10:24, Miquel Raynal wrote:
> > Hi Krzysztof,
> >
> > [email protected] wrote on Fri, 16 Jun 2023 10:15:31 +0200:
> >
> >> On 15/06/2023 23:06, Chris Packham wrote:
> >>>>
> >>>>> +
> >>>>> +properties:
> >>>>> + compatible:
> >>>>> + oneOf:
> >>>>> + - items:
> >>>>> + - const: marvell,armada-8k-nand-controller
> >>>>> + - const: marvell,armada370-nand-controller
> >>>>> + - enum:
> >>>>> + - marvell,armada-8k-nand-controller
> >>>> This is wrong. 8k cannot be both: compatible and not compatible with
> >>>> 370. It's not someone's cat to be in both states at the same time...
> >>>
> >>> The correct state (IMHO, Miquel correct me if I'm wrong) is `compatible
> >>> = "marvell,armada-8k-nand-controller";` as there are some 8K specific
> >>> requirements that aren't present on the 370 (specifically the
> >>> system-controller and the 2nd clock).
> >>
> >> Presence of specific requirements does not invalidate compatibility. Two
> >> devices are compatible if the 8k can bind and work with 370 compatible
> >> string, even if this means some lower performance or less features (e.g.
> >> subset of features).
> >
> > Quoting myself from 2019 (comment in the driver):
> > "Some SoCs like A7k/A8k need to enable manually the NAND controller,
> > gated clocks and reset bits to avoid being bootloader dependent."
> >
> > So can the 8k controller work using a 370 compatible : yes, if the
> > booloader enabled the NAND controller already, no otherwise. But in
> > practice it is the same controller.
> >
> > Given this information I don't know whether it makes sense to qualify
> > the 8k controller compatible with the 370 compatible or not.
>
> Indeed, a bit tricky decision. For me they could stay compatible,
> because apparently programming model is the same or similar.
Agreed, the programming model wrt the controller itself is the same. So
we will expect 8k controllers to be described with both 8k and 370
compatible strings then.
Thanks for your feedback.
Cheers,
Miquèl