* [PATCH v8 0/3] dt-bindings: mtd: marvell-nand: Add YAML scheme
@ 2023-05-31 23:49 Chris Packham
2023-05-31 23:49 ` [PATCH v8 1/3] arm64: dts: marvell: cp11x: Fix nand_controller node name according to YAML Chris Packham
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Chris Packham @ 2023-05-31 23:49 UTC (permalink / raw)
To: miquel.raynal, richard, vigneshr, robh+dt, krzysztof.kozlowski+dt,
conor+dt, andrew, gregory.clement, sebastian.hesselbarth, conor
Cc: linux-mtd, devicetree, linux-kernel, linux-arm-kernel, enachman,
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
I've picked up this series to hopefully get it over the line. I think I've
addressed all the feedback from the last round of review.
As of v7 I've re-ordered the patches so the fixups for the device trees come
before the schema so if the series is applied in order the various checkers
should pass cleanly.
Chris Packham (1):
ARM: dts: mvebu: align MTD partition nodes to dtschema
Vadym Kochan (2):
arm64: dts: marvell: cp11x: Fix nand_controller node name according to
YAML
dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
.../bindings/mtd/marvell,nand-controller.yaml | 223 ++++++++++++++++++
.../devicetree/bindings/mtd/marvell-nand.txt | 126 ----------
MAINTAINERS | 1 -
arch/arm/boot/dts/armada-385-atl-x530.dts | 14 +-
arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 2 +-
5 files changed, 231 insertions(+), 135 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml
delete mode 100644 Documentation/devicetree/bindings/mtd/marvell-nand.txt
--
2.40.1
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH v8 1/3] arm64: dts: marvell: cp11x: Fix nand_controller node name according to YAML 2023-05-31 23:49 [PATCH v8 0/3] dt-bindings: mtd: marvell-nand: Add YAML scheme Chris Packham @ 2023-05-31 23:49 ` Chris Packham 2023-05-31 23:49 ` [PATCH v8 2/3] ARM: dts: mvebu: align MTD partition nodes to dtschema Chris Packham 2023-05-31 23:49 ` [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme Chris Packham 2 siblings, 0 replies; 21+ messages in thread From: Chris Packham @ 2023-05-31 23:49 UTC (permalink / raw) To: miquel.raynal, richard, vigneshr, robh+dt, krzysztof.kozlowski+dt, conor+dt, andrew, gregory.clement, sebastian.hesselbarth, conor Cc: linux-mtd, devicetree, linux-kernel, linux-arm-kernel, enachman, Vadym Kochan, Chris Packham From: Vadym Kochan <vadym.kochan@plvision.eu> 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> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> --- Notes: Changes in v8: - Add r-by from Miquel 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 0cc9ee9871e7..4ec1aae0a3a9 100644 --- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi +++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi @@ -468,7 +468,7 @@ CP11X_LABEL(uart3): serial@702300 { 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.40.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v8 2/3] ARM: dts: mvebu: align MTD partition nodes to dtschema 2023-05-31 23:49 [PATCH v8 0/3] dt-bindings: mtd: marvell-nand: Add YAML scheme Chris Packham 2023-05-31 23:49 ` [PATCH v8 1/3] arm64: dts: marvell: cp11x: Fix nand_controller node name according to YAML Chris Packham @ 2023-05-31 23:49 ` Chris Packham 2023-05-31 23:49 ` [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme Chris Packham 2 siblings, 0 replies; 21+ messages in thread From: Chris Packham @ 2023-05-31 23:49 UTC (permalink / raw) To: miquel.raynal, richard, vigneshr, robh+dt, krzysztof.kozlowski+dt, conor+dt, andrew, gregory.clement, sebastian.hesselbarth, conor Cc: linux-mtd, devicetree, linux-kernel, linux-arm-kernel, enachman, Chris Packham Update the node names for the SPI NOR and NAND partitions to conform to the partition properties in the relevant dtschema. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> --- Notes: Changes in v8: - Add r-by from Miquel arch/arm/boot/dts/armada-385-atl-x530.dts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/arch/arm/boot/dts/armada-385-atl-x530.dts b/arch/arm/boot/dts/armada-385-atl-x530.dts index 241f5d7c80e9..5a9ab8410b7b 100644 --- a/arch/arm/boot/dts/armada-385-atl-x530.dts +++ b/arch/arm/boot/dts/armada-385-atl-x530.dts @@ -179,19 +179,19 @@ partitions { compatible = "fixed-partitions"; #address-cells = <1>; #size-cells = <1>; - partition@u-boot { + partition@0 { reg = <0x00000000 0x00100000>; label = "u-boot"; }; - partition@u-boot-env { + partition@100000 { reg = <0x00100000 0x00040000>; label = "u-boot-env"; }; - partition@unused { + partition@140000 { reg = <0x00140000 0x00e80000>; label = "unused"; }; - partition@idprom { + partition@fc0000 { reg = <0x00fc0000 0x00040000>; label = "idprom"; }; @@ -216,16 +216,16 @@ partitions { compatible = "fixed-partitions"; #address-cells = <1>; #size-cells = <1>; - partition@user { + partition@0 { reg = <0x00000000 0x0f000000>; label = "user"; }; - partition@errlog { + partition@f000000 { /* Maximum mtdoops size is 8MB, so set to that. */ reg = <0x0f000000 0x00800000>; label = "errlog"; }; - partition@nand-bbt { + partition@f800000 { reg = <0x0f800000 0x00800000>; label = "nand-bbt"; }; -- 2.40.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-05-31 23:49 [PATCH v8 0/3] dt-bindings: mtd: marvell-nand: Add YAML scheme Chris Packham 2023-05-31 23:49 ` [PATCH v8 1/3] arm64: dts: marvell: cp11x: Fix nand_controller node name according to YAML Chris Packham 2023-05-31 23:49 ` [PATCH v8 2/3] ARM: dts: mvebu: align MTD partition nodes to dtschema Chris Packham @ 2023-05-31 23:49 ` Chris Packham 2023-06-01 7:05 ` Krzysztof Kozlowski 2 siblings, 1 reply; 21+ messages in thread From: Chris Packham @ 2023-05-31 23:49 UTC (permalink / raw) To: miquel.raynal, richard, vigneshr, robh+dt, krzysztof.kozlowski+dt, conor+dt, andrew, gregory.clement, sebastian.hesselbarth, conor Cc: linux-mtd, devicetree, linux-kernel, linux-arm-kernel, enachman, Vadym Kochan, Chris Packham From: Vadym Kochan <vadym.kochan@plvision.eu> 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 <vadym.kochan@plvision.eu> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- Notes: 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 | 223 ++++++++++++++++++ .../devicetree/bindings/mtd/marvell-nand.txt | 126 ---------- MAINTAINERS | 1 - 3 files changed, 223 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..433feb430555 --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml @@ -0,0 +1,223 @@ +# 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,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@[0-3]$": + type: object + unevaluatedProperties: false + properties: + reg: + minimum: 0 + maximum: 3 + + nand-rb: + minItems: 1 + maxItems: 1 + + nand-ecc-step-size: + const: 512 + + nand-ecc-strength: + enum: [1, 4, 8, 12, 16] + + nand-on-flash-bbt: + $ref: /schemas/types.yaml#/definitions/flag + + nand-ecc-mode: + const: hw + + 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). + $ref: /schemas/types.yaml#/definitions/flag + deprecated: true + + additionalProperties: false + + required: + - reg + - nand-rb + +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 + +required: + - compatible + - reg + - interrupts + - clocks + +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 2a42a75c304c..fdd2027529ea 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12535,7 +12535,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 OCTEON ENDPOINT DRIVER -- 2.40.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-05-31 23:49 ` [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme Chris Packham @ 2023-06-01 7:05 ` Krzysztof Kozlowski 2023-06-01 21:07 ` Chris Packham 2023-06-01 23:06 ` Chris Packham 0 siblings, 2 replies; 21+ messages in thread From: Krzysztof Kozlowski @ 2023-06-01 7:05 UTC (permalink / raw) To: Chris Packham, miquel.raynal, richard, vigneshr, robh+dt, krzysztof.kozlowski+dt, conor+dt, andrew, gregory.clement, sebastian.hesselbarth, conor Cc: linux-mtd, devicetree, linux-kernel, linux-arm-kernel, enachman, Vadym Kochan On 01/06/2023 01:49, Chris Packham wrote: > From: Vadym Kochan <vadym.kochan@plvision.eu> > > 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 <vadym.kochan@plvision.eu> > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- > > Notes: > 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) What do you mean by "aren't"? They are not needed. > - 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 | 223 ++++++++++++++++++ > .../devicetree/bindings/mtd/marvell-nand.txt | 126 ---------- > MAINTAINERS | 1 - > 3 files changed, 223 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..433feb430555 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml > @@ -0,0 +1,223 @@ > +# 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,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@[0-3]$": > + type: object > + unevaluatedProperties: false > + properties: > + reg: > + minimum: 0 > + maximum: 3 > + > + nand-rb: > + minItems: 1 Drop minItems. > + maxItems: 1 Didn't you have here minimum and maximum? I think I did not ask to remove them. > + > + nand-ecc-step-size: > + const: 512 > + > + nand-ecc-strength: > + enum: [1, 4, 8, 12, 16] > + > + nand-on-flash-bbt: > + $ref: /schemas/types.yaml#/definitions/flag > + > + nand-ecc-mode: > + const: hw > + > + label: > + $ref: /schemas/types.yaml#/definitions/string Drop label > + > + partitions: > + type: object Drop partitions. > + > + 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). > + $ref: /schemas/types.yaml#/definitions/flag > + deprecated: true > + > + additionalProperties: false unevaluatedProperties: false > + > + required: > + - reg > + - nand-rb > + required: block goes here > +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 > + Best regards, Krzysztof ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-01 7:05 ` Krzysztof Kozlowski @ 2023-06-01 21:07 ` Chris Packham 2023-06-01 23:06 ` Chris Packham 1 sibling, 0 replies; 21+ messages in thread From: Chris Packham @ 2023-06-01 21:07 UTC (permalink / raw) To: Krzysztof Kozlowski, miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org Cc: linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan On 1/06/23 19:05, Krzysztof Kozlowski wrote: > On 01/06/2023 01:49, Chris Packham wrote: >> From: Vadym Kochan <vadym.kochan@plvision.eu> >> >> 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 <vadym.kochan@plvision.eu> >> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >> --- >> >> Notes: >> 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) > What do you mean by "aren't"? They are not needed. I mean I simply cannot make it work and I'm out of ideas (I'm also in an awkward timezone so it takes 24hrs for me to ask a question and get a response which leads to me making guesses instead of waiting). nand-controller.yaml references nand-chip.yaml which references mtd.yaml which defines the "label" and "partitions" property. I thought marvell,nand-controller.yaml could just say `$ref: nand-controller.yaml` and it would mean I'd get all the definitions down the chain but this doesn't seem to work the way I expect (or more likely I'm not doing it right). I thought it might have something to do with the different patternProperties pattern but even when I make that match what is used in nand-controller.yaml it doesn't seem to pick up those properties. >> - 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 | 223 ++++++++++++++++++ >> .../devicetree/bindings/mtd/marvell-nand.txt | 126 ---------- >> MAINTAINERS | 1 - >> 3 files changed, 223 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..433feb430555 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml >> @@ -0,0 +1,223 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://scanmail.trustwave.com/?c=20988&d=18P45NWaR4V6pXt_kuivNCiVAXCm3C7MEF-_8xrP2A&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fmtd%2fmarvell%2cnand-controller%2eyaml%23 >> +$schema: http://scanmail.trustwave.com/?c=20988&d=18P45NWaR4V6pXt_kuivNCiVAXCm3C7MEFvq8B-ZjQ&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23 >> + >> +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,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@[0-3]$": >> + type: object >> + unevaluatedProperties: false >> + properties: >> + reg: >> + minimum: 0 >> + maximum: 3 >> + >> + nand-rb: >> + minItems: 1 > Drop minItems. > >> + maxItems: 1 > Didn't you have here minimum and maximum? I think I did not ask to > remove them. > >> + >> + nand-ecc-step-size: >> + const: 512 >> + >> + nand-ecc-strength: >> + enum: [1, 4, 8, 12, 16] >> + >> + nand-on-flash-bbt: >> + $ref: /schemas/types.yaml#/definitions/flag >> + >> + nand-ecc-mode: >> + const: hw >> + >> + label: >> + $ref: /schemas/types.yaml#/definitions/string > Drop label > >> + >> + partitions: >> + type: object > Drop partitions. > >> + >> + 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). >> + $ref: /schemas/types.yaml#/definitions/flag >> + deprecated: true >> + >> + additionalProperties: false > unevaluatedProperties: false > >> + >> + required: >> + - reg >> + - nand-rb >> + > required: block goes here > >> +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 >> + > > Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-01 7:05 ` Krzysztof Kozlowski 2023-06-01 21:07 ` Chris Packham @ 2023-06-01 23:06 ` Chris Packham 2023-06-04 9:26 ` Krzysztof Kozlowski 1 sibling, 1 reply; 21+ messages in thread From: Chris Packham @ 2023-06-01 23:06 UTC (permalink / raw) To: Krzysztof Kozlowski, miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org Cc: linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan Hi Krzystof, On 1/06/23 19:05, Krzysztof Kozlowski wrote: > On 01/06/2023 01:49, Chris Packham wrote: >> From: Vadym Kochan <vadym.kochan@plvision.eu> >> >> 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 <vadym.kochan@plvision.eu> >> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >> --- >> >> Notes: >> 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) > What do you mean by "aren't"? They are not needed. (sorry I keep responding to snippets rather than putting all the replies in one place. For posterity here's the same response I provided in a separate message). I mean I simply cannot make it work and I'm out of ideas (I'm also in an awkward timezone so it takes 24hrs for me to ask a question and get a response which leads to me making guesses instead of waiting). nand-controller.yaml references nand-chip.yaml which references mtd.yaml which defines the "label" and "partitions" property. I thought marvell,nand-controller.yaml could just say `$ref: nand-controller.yaml` and it would mean I'd get all the definitions down the chain but this doesn't seem to work the way I expect (or more likely I'm not doing it right). I thought it might have something to do with the different patternProperties pattern but even when I make that match what is used in nand-controller.yaml it doesn't seem to pick up those properties. >> - 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 | 223 ++++++++++++++++++ >> .../devicetree/bindings/mtd/marvell-nand.txt | 126 ---------- >> MAINTAINERS | 1 - >> 3 files changed, 223 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..433feb430555 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml >> @@ -0,0 +1,223 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://scanmail.trustwave.com/?c=20988&d=18P45NWaR4V6pXt_kuivNCiVAXCm3C7MEF-_8xrP2A&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fmtd%2fmarvell%2cnand-controller%2eyaml%23 >> +$schema: http://scanmail.trustwave.com/?c=20988&d=18P45NWaR4V6pXt_kuivNCiVAXCm3C7MEFvq8B-ZjQ&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23 >> + >> +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,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@[0-3]$": >> + type: object >> + unevaluatedProperties: false >> + properties: >> + reg: >> + minimum: 0 >> + maximum: 3 >> + >> + nand-rb: >> + minItems: 1 > Drop minItems. > >> + maxItems: 1 > Didn't you have here minimum and maximum? I think I did not ask to > remove them. I did but I couldn't figure out how to do minimum and maximum with an array would the following be correct (note removing both minItems and maxItems as dtb_check complains if I have maxItems and items). nand-rb: items: - minimum: 0 maximum: 1 > >> + >> + nand-ecc-step-size: >> + const: 512 >> + >> + nand-ecc-strength: >> + enum: [1, 4, 8, 12, 16] >> + >> + nand-on-flash-bbt: >> + $ref: /schemas/types.yaml#/definitions/flag >> + >> + nand-ecc-mode: >> + const: hw >> + >> + label: >> + $ref: /schemas/types.yaml#/definitions/string > Drop label > >> + >> + partitions: >> + type: object > Drop partitions. This is the part I can't get to work. It should pick it up via nand-controller.yaml but nothing I do seems to work. >> + >> + 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). >> + $ref: /schemas/types.yaml#/definitions/flag >> + deprecated: true >> + >> + additionalProperties: false > unevaluatedProperties: false It was hiding by '"^nand@[0-3]$":'. Should I move it here? > >> + >> + required: >> + - reg >> + - nand-rb >> + > required: block goes here Will move > >> +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 >> + > > Best regards, > Krzysztof > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-01 23:06 ` Chris Packham @ 2023-06-04 9:26 ` Krzysztof Kozlowski 2023-06-05 20:44 ` Chris Packham 0 siblings, 1 reply; 21+ messages in thread From: Krzysztof Kozlowski @ 2023-06-04 9:26 UTC (permalink / raw) To: Chris Packham, miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org Cc: linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan On 02/06/2023 01:06, Chris Packham wrote: > Hi Krzystof, > > On 1/06/23 19:05, Krzysztof Kozlowski wrote: >> On 01/06/2023 01:49, Chris Packham wrote: >>> From: Vadym Kochan <vadym.kochan@plvision.eu> >>> >>> 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 <vadym.kochan@plvision.eu> >>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >>> --- >>> >>> Notes: >>> 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) >> What do you mean by "aren't"? They are not needed. > > (sorry I keep responding to snippets rather than putting all the replies > in one place. For posterity here's the same response I provided in a > separate message). > > I mean I simply cannot make it work and I'm out of ideas (I'm also in an > awkward timezone so it takes 24hrs for me to ask a question and get a > response which leads to me making guesses instead of waiting). > > nand-controller.yaml references nand-chip.yaml which references mtd.yaml > which defines the "label" and "partitions" property. > > I thought marvell,nand-controller.yaml could just say `$ref: > nand-controller.yaml` and it would mean I'd get all the definitions down > the chain but this doesn't seem to work the way I expect (or more likely > I'm not doing it right). I thought it might have something to do with > the different patternProperties pattern but even when I make that match > what is used in nand-controller.yaml it doesn't seem to pick up those > properties. Then you are doing something different than all other bindings. >>> - 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 | 223 ++++++++++++++++++ >>> .../devicetree/bindings/mtd/marvell-nand.txt | 126 ---------- >>> MAINTAINERS | 1 - >>> 3 files changed, 223 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..433feb430555 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml >>> @@ -0,0 +1,223 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://scanmail.trustwave.com/?c=20988&d=18P45NWaR4V6pXt_kuivNCiVAXCm3C7MEF-_8xrP2A&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fmtd%2fmarvell%2cnand-controller%2eyaml%23 >>> +$schema: http://scanmail.trustwave.com/?c=20988&d=18P45NWaR4V6pXt_kuivNCiVAXCm3C7MEFvq8B-ZjQ&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23 >>> + >>> +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,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@[0-3]$": >>> + type: object >>> + unevaluatedProperties: false >>> + properties: >>> + reg: >>> + minimum: 0 >>> + maximum: 3 >>> + >>> + nand-rb: >>> + minItems: 1 >> Drop minItems. >> >>> + maxItems: 1 >> Didn't you have here minimum and maximum? I think I did not ask to >> remove them. > > I did but I couldn't figure out how to do minimum and maximum with an > array would the following be correct (note removing both minItems and > maxItems as dtb_check complains if I have maxItems and items). items: minimum: n maximum: n maxItems: n or items: - minimum: n maximum: n See for example Documentation/devicetree/bindings/arm/l2c2x0.yaml > > nand-rb: > items: > - minimum: 0 > maximum: 1 > >> >>> + >>> + nand-ecc-step-size: >>> + const: 512 >>> + >>> + nand-ecc-strength: >>> + enum: [1, 4, 8, 12, 16] >>> + >>> + nand-on-flash-bbt: >>> + $ref: /schemas/types.yaml#/definitions/flag >>> + >>> + nand-ecc-mode: >>> + const: hw >>> + >>> + label: >>> + $ref: /schemas/types.yaml#/definitions/string >> Drop label >> >>> + >>> + partitions: >>> + type: object >> Drop partitions. > > This is the part I can't get to work. It should pick it up via > nand-controller.yaml but nothing I do seems to work. > >>> + >>> + 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). >>> + $ref: /schemas/types.yaml#/definitions/flag >>> + deprecated: true >>> + >>> + additionalProperties: false >> unevaluatedProperties: false > It was hiding by '"^nand@[0-3]$":'. Should I move it here? You cannot have both additionalProps and unevaluatedProps at the same time, so we do not talk about same thing or this was never working? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-04 9:26 ` Krzysztof Kozlowski @ 2023-06-05 20:44 ` Chris Packham 2023-06-05 21:26 ` Andrew Lunn 2023-06-06 4:38 ` Chris Packham 0 siblings, 2 replies; 21+ messages in thread From: Chris Packham @ 2023-06-05 20:44 UTC (permalink / raw) To: Krzysztof Kozlowski, miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org Cc: linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan On 4/06/23 21:26, Krzysztof Kozlowski wrote: > On 02/06/2023 01:06, Chris Packham wrote: >> Hi Krzystof, >> >> On 1/06/23 19:05, Krzysztof Kozlowski wrote: >>> On 01/06/2023 01:49, Chris Packham wrote: >>>> From: Vadym Kochan <vadym.kochan@plvision.eu> >>>> >>>> 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 <vadym.kochan@plvision.eu> >>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >>>> --- >>>> >>>> Notes: >>>> 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) >>> What do you mean by "aren't"? They are not needed. >> (sorry I keep responding to snippets rather than putting all the replies >> in one place. For posterity here's the same response I provided in a >> separate message). >> >> I mean I simply cannot make it work and I'm out of ideas (I'm also in an >> awkward timezone so it takes 24hrs for me to ask a question and get a >> response which leads to me making guesses instead of waiting). >> >> nand-controller.yaml references nand-chip.yaml which references mtd.yaml >> which defines the "label" and "partitions" property. >> >> I thought marvell,nand-controller.yaml could just say `$ref: >> nand-controller.yaml` and it would mean I'd get all the definitions down >> the chain but this doesn't seem to work the way I expect (or more likely >> I'm not doing it right). I thought it might have something to do with >> the different patternProperties pattern but even when I make that match >> what is used in nand-controller.yaml it doesn't seem to pick up those >> properties. > Then you are doing something different than all other bindings. Not intentionally. I should probably check that the existing bindings actually work as expected. One thing that this has that the others don't is including "label" and "partitions" in the examples. >>>> - 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 | 223 ++++++++++++++++++ >>>> .../devicetree/bindings/mtd/marvell-nand.txt | 126 ---------- >>>> MAINTAINERS | 1 - >>>> 3 files changed, 223 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..433feb430555 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml >>>> @@ -0,0 +1,223 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://scanmail.trustwave.com/?c=20988&d=yNj85IkMld8k0XBdA9CH4pQjE5peaXAdz-exk_Hdww&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fmtd%2fmarvell%2cnand-controller%2eyaml%23 >>>> +$schema: http://scanmail.trustwave.com/?c=20988&d=yNj85IkMld8k0XBdA9CH4pQjE5peaXAdz-PkkPSLlg&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23 >>>> + >>>> +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,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@[0-3]$": >>>> + type: object >>>> + unevaluatedProperties: false >>>> + properties: >>>> + reg: >>>> + minimum: 0 >>>> + maximum: 3 >>>> + >>>> + nand-rb: >>>> + minItems: 1 >>> Drop minItems. >>> >>>> + maxItems: 1 >>> Didn't you have here minimum and maximum? I think I did not ask to >>> remove them. >> I did but I couldn't figure out how to do minimum and maximum with an >> array would the following be correct (note removing both minItems and >> maxItems as dtb_check complains if I have maxItems and items). > items: > minimum: n > maximum: n > maxItems: n > > or > > items: > - minimum: n > maximum: n > > See for example Documentation/devicetree/bindings/arm/l2c2x0.yaml Thanks, so my suggestion below should be OK then. >> nand-rb: >> items: >> - minimum: 0 >> maximum: 1 >> >>>> + >>>> + nand-ecc-step-size: >>>> + const: 512 >>>> + >>>> + nand-ecc-strength: >>>> + enum: [1, 4, 8, 12, 16] >>>> + >>>> + nand-on-flash-bbt: >>>> + $ref: /schemas/types.yaml#/definitions/flag >>>> + >>>> + nand-ecc-mode: >>>> + const: hw >>>> + >>>> + label: >>>> + $ref: /schemas/types.yaml#/definitions/string >>> Drop label >>> >>>> + >>>> + partitions: >>>> + type: object >>> Drop partitions. >> This is the part I can't get to work. It should pick it up via >> nand-controller.yaml but nothing I do seems to work. >> >>>> + >>>> + 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). >>>> + $ref: /schemas/types.yaml#/definitions/flag >>>> + deprecated: true >>>> + >>>> + additionalProperties: false >>> unevaluatedProperties: false >> It was hiding by '"^nand@[0-3]$":'. Should I move it here? > You cannot have both additionalProps and unevaluatedProps at the same > time, so we do not talk about same thing or this was never working? Hmm, I'm a little confused then. At various times I've been told to put 'additionalProperties: false' or 'unevaluatedProperties: false' (although never at the same time). I'm not sure when to use one or the other. From what I've been able to glean 'additionalProperties: true' indicates that the node is expected to have child nodes defined in a different schema so I would have thought 'additionalProperties: false' would be appropriate for a schema covering a leaf node. 'unevaluatedProperties: false' seems to enable stricter checking which makes sense when all the properties are described in the schema. > > Best regards, > Krzysztof ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-05 20:44 ` Chris Packham @ 2023-06-05 21:26 ` Andrew Lunn 2023-06-06 4:38 ` Chris Packham 1 sibling, 0 replies; 21+ messages in thread From: Andrew Lunn @ 2023-06-05 21:26 UTC (permalink / raw) To: Chris Packham Cc: Krzysztof Kozlowski, miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan > > Then you are doing something different than all other bindings. > > Not intentionally. I should probably check that the existing bindings > actually work as expected. This binding supports marvell,pxa3xx-nand. That is really really old. So it could well be it works in the kernel, but the YAML does not fully implement what the kernel actually supports. Best practices could of pushed modern day DT to a subset, and YAML only supports that subset of reality. And once you get outside of this subset, you run into trouble. I don't actually know this is the case, but you should keep it in mind. Andrew ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-05 20:44 ` Chris Packham 2023-06-05 21:26 ` Andrew Lunn @ 2023-06-06 4:38 ` Chris Packham 2023-06-06 7:48 ` Miquel Raynal 1 sibling, 1 reply; 21+ messages in thread From: Chris Packham @ 2023-06-06 4:38 UTC (permalink / raw) To: Krzysztof Kozlowski, miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org Cc: linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan On 6/06/23 08:44, Chris Packham wrote: > > On 4/06/23 21:26, Krzysztof Kozlowski wrote: >> On 02/06/2023 01:06, Chris Packham wrote: >>> Hi Krzystof, >>> >>> On 1/06/23 19:05, Krzysztof Kozlowski wrote: >>>> On 01/06/2023 01:49, Chris Packham wrote: >>>>> From: Vadym Kochan <vadym.kochan@plvision.eu> >>>>> >>>>> 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 <vadym.kochan@plvision.eu> >>>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >>>>> --- >>>>> >>>>> Notes: >>>>> 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) >>>> What do you mean by "aren't"? They are not needed. >>> (sorry I keep responding to snippets rather than putting all the >>> replies >>> in one place. For posterity here's the same response I provided in a >>> separate message). >>> >>> I mean I simply cannot make it work and I'm out of ideas (I'm also >>> in an >>> awkward timezone so it takes 24hrs for me to ask a question and get a >>> response which leads to me making guesses instead of waiting). >>> >>> nand-controller.yaml references nand-chip.yaml which references >>> mtd.yaml >>> which defines the "label" and "partitions" property. >>> >>> I thought marvell,nand-controller.yaml could just say `$ref: >>> nand-controller.yaml` and it would mean I'd get all the definitions >>> down >>> the chain but this doesn't seem to work the way I expect (or more >>> likely >>> I'm not doing it right). I thought it might have something to do with >>> the different patternProperties pattern but even when I make that match >>> what is used in nand-controller.yaml it doesn't seem to pick up those >>> properties. >> Then you are doing something different than all other bindings. > > Not intentionally. I should probably check that the existing bindings > actually work as expected. > > One thing that this has that the others don't is including "label" and > "partitions" in the examples. > >>>>> - 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 | 223 >>>>> ++++++++++++++++++ >>>>> .../devicetree/bindings/mtd/marvell-nand.txt | 126 ---------- >>>>> MAINTAINERS | 1 - >>>>> 3 files changed, 223 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..433feb430555 >>>>> --- /dev/null >>>>> +++ >>>>> b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml >>>>> @@ -0,0 +1,223 @@ >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>> +%YAML 1.2 >>>>> +--- >>>>> +$id: >>>>> http://scanmail.trustwave.com/?c=20988&d=yNj85IkMld8k0XBdA9CH4pQjE5peaXAdz-exk_Hdww&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fmtd%2fmarvell%2cnand-controller%2eyaml%23 >>>>> +$schema: >>>>> http://scanmail.trustwave.com/?c=20988&d=yNj85IkMld8k0XBdA9CH4pQjE5peaXAdz-PkkPSLlg&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23 >>>>> + >>>>> +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,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@[0-3]$": >>>>> + type: object >>>>> + unevaluatedProperties: false >>>>> + properties: >>>>> + reg: >>>>> + minimum: 0 >>>>> + maximum: 3 >>>>> + >>>>> + nand-rb: >>>>> + minItems: 1 >>>> Drop minItems. >>>> >>>>> + maxItems: 1 >>>> Didn't you have here minimum and maximum? I think I did not ask to >>>> remove them. >>> I did but I couldn't figure out how to do minimum and maximum with an >>> array would the following be correct (note removing both minItems and >>> maxItems as dtb_check complains if I have maxItems and items). >> items: >> minimum: n >> maximum: n >> maxItems: n >> >> or >> >> items: >> - minimum: n >> maximum: n >> >> See for example Documentation/devicetree/bindings/arm/l2c2x0.yaml > Thanks, so my suggestion below should be OK then. >>> nand-rb: >>> items: >>> - minimum: 0 >>> maximum: 1 >>> >>>>> + >>>>> + nand-ecc-step-size: >>>>> + const: 512 >>>>> + >>>>> + nand-ecc-strength: >>>>> + enum: [1, 4, 8, 12, 16] >>>>> + >>>>> + nand-on-flash-bbt: >>>>> + $ref: /schemas/types.yaml#/definitions/flag >>>>> + >>>>> + nand-ecc-mode: >>>>> + const: hw >>>>> + >>>>> + label: >>>>> + $ref: /schemas/types.yaml#/definitions/string >>>> Drop label >>>> >>>>> + >>>>> + partitions: >>>>> + type: object >>>> Drop partitions. >>> This is the part I can't get to work. It should pick it up via >>> nand-controller.yaml but nothing I do seems to work. >>> >>>>> + >>>>> + 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). >>>>> + $ref: /schemas/types.yaml#/definitions/flag >>>>> + deprecated: true >>>>> + >>>>> + additionalProperties: false >>>> unevaluatedProperties: false >>> It was hiding by '"^nand@[0-3]$":'. Should I move it here? >> You cannot have both additionalProps and unevaluatedProps at the same >> time, so we do not talk about same thing or this was never working? > > Hmm, I'm a little confused then. At various times I've been told to > put 'additionalProperties: false' or 'unevaluatedProperties: false' > (although never at the same time). I'm not sure when to use one or the > other. > > From what I've been able to glean 'additionalProperties: true' > indicates that the node is expected to have child nodes defined in a > different schema so I would have thought 'additionalProperties: false' > would be appropriate for a schema covering a leaf node. > 'unevaluatedProperties: false' seems to enable stricter checking which > makes sense when all the properties are described in the schema. So I think this might be the problem. If I look at qcom,nandc.yaml or ingenic,nand.yaml which both have a partitions property in their example. Neither have 'unevaluatedProperties: false' on the nand@... subnode. If I add it sure enough I start getting complaints about the 'partitions' node being unexpected. > >> >> Best regards, >> Krzysztof ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-06 4:38 ` Chris Packham @ 2023-06-06 7:48 ` Miquel Raynal 2023-06-06 8:44 ` Krzysztof Kozlowski 0 siblings, 1 reply; 21+ messages in thread From: Miquel Raynal @ 2023-06-06 7:48 UTC (permalink / raw) To: Chris Packham Cc: Krzysztof Kozlowski, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan Hi Chris, Chris.Packham@alliedtelesis.co.nz wrote on Tue, 6 Jun 2023 04:38:01 +0000: > On 6/06/23 08:44, Chris Packham wrote: > > > > On 4/06/23 21:26, Krzysztof Kozlowski wrote: > >> On 02/06/2023 01:06, Chris Packham wrote: > >>> Hi Krzystof, > >>> > >>> On 1/06/23 19:05, Krzysztof Kozlowski wrote: > >>>> On 01/06/2023 01:49, Chris Packham wrote: > >>>>> From: Vadym Kochan <vadym.kochan@plvision.eu> > >>>>> > >>>>> 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 <vadym.kochan@plvision.eu> > >>>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > >>>>> --- > >>>>> > >>>>> Notes: > >>>>> 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) > >>>> What do you mean by "aren't"? They are not needed. > >>> (sorry I keep responding to snippets rather than putting all the > >>> replies > >>> in one place. For posterity here's the same response I provided in a > >>> separate message). > >>> > >>> I mean I simply cannot make it work and I'm out of ideas (I'm also > >>> in an > >>> awkward timezone so it takes 24hrs for me to ask a question and get a > >>> response which leads to me making guesses instead of waiting). > >>> > >>> nand-controller.yaml references nand-chip.yaml which references > >>> mtd.yaml > >>> which defines the "label" and "partitions" property. > >>> > >>> I thought marvell,nand-controller.yaml could just say `$ref: > >>> nand-controller.yaml` and it would mean I'd get all the definitions > >>> down > >>> the chain but this doesn't seem to work the way I expect (or more > >>> likely > >>> I'm not doing it right). I thought it might have something to do with > >>> the different patternProperties pattern but even when I make that match > >>> what is used in nand-controller.yaml it doesn't seem to pick up those > >>> properties. > >> Then you are doing something different than all other bindings. > > > > Not intentionally. I should probably check that the existing bindings > > actually work as expected. > > > > One thing that this has that the others don't is including "label" and > > "partitions" in the examples. > > > >>>>> - 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 | 223 > >>>>> ++++++++++++++++++ > >>>>> .../devicetree/bindings/mtd/marvell-nand.txt | 126 ---------- > >>>>> MAINTAINERS | 1 - > >>>>> 3 files changed, 223 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..433feb430555 > >>>>> --- /dev/null > >>>>> +++ > >>>>> b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml > >>>>> @@ -0,0 +1,223 @@ > >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>>>> +%YAML 1.2 > >>>>> +--- > >>>>> +$id: > >>>>> http://scanmail.trustwave.com/?c=20988&d=yNj85IkMld8k0XBdA9CH4pQjE5peaXAdz-exk_Hdww&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fmtd%2fmarvell%2cnand-controller%2eyaml%23 > >>>>> +$schema: > >>>>> http://scanmail.trustwave.com/?c=20988&d=yNj85IkMld8k0XBdA9CH4pQjE5peaXAdz-PkkPSLlg&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23 > >>>>> + > >>>>> +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,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@[0-3]$": > >>>>> + type: object > >>>>> + unevaluatedProperties: false > >>>>> + properties: > >>>>> + reg: > >>>>> + minimum: 0 > >>>>> + maximum: 3 > >>>>> + > >>>>> + nand-rb: > >>>>> + minItems: 1 > >>>> Drop minItems. > >>>> > >>>>> + maxItems: 1 > >>>> Didn't you have here minimum and maximum? I think I did not ask to > >>>> remove them. > >>> I did but I couldn't figure out how to do minimum and maximum with an > >>> array would the following be correct (note removing both minItems and > >>> maxItems as dtb_check complains if I have maxItems and items). > >> items: > >> minimum: n > >> maximum: n > >> maxItems: n > >> > >> or > >> > >> items: > >> - minimum: n > >> maximum: n > >> > >> See for example Documentation/devicetree/bindings/arm/l2c2x0.yaml > > Thanks, so my suggestion below should be OK then. > >>> nand-rb: > >>> items: > >>> - minimum: 0 > >>> maximum: 1 > >>> > >>>>> + > >>>>> + nand-ecc-step-size: > >>>>> + const: 512 > >>>>> + > >>>>> + nand-ecc-strength: > >>>>> + enum: [1, 4, 8, 12, 16] > >>>>> + > >>>>> + nand-on-flash-bbt: > >>>>> + $ref: /schemas/types.yaml#/definitions/flag > >>>>> + > >>>>> + nand-ecc-mode: > >>>>> + const: hw > >>>>> + > >>>>> + label: > >>>>> + $ref: /schemas/types.yaml#/definitions/string > >>>> Drop label > >>>> > >>>>> + > >>>>> + partitions: > >>>>> + type: object > >>>> Drop partitions. > >>> This is the part I can't get to work. It should pick it up via > >>> nand-controller.yaml but nothing I do seems to work. > >>> > >>>>> + > >>>>> + 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). > >>>>> + $ref: /schemas/types.yaml#/definitions/flag > >>>>> + deprecated: true > >>>>> + > >>>>> + additionalProperties: false > >>>> unevaluatedProperties: false > >>> It was hiding by '"^nand@[0-3]$":'. Should I move it here? > >> You cannot have both additionalProps and unevaluatedProps at the same > >> time, so we do not talk about same thing or this was never working? > > > > Hmm, I'm a little confused then. At various times I've been told to > > put 'additionalProperties: false' or 'unevaluatedProperties: false' > > (although never at the same time). I'm not sure when to use one or the > > other. > > > > From what I've been able to glean 'additionalProperties: true' > > indicates that the node is expected to have child nodes defined in a > > different schema so I would have thought 'additionalProperties: false' > > would be appropriate for a schema covering a leaf node. > > 'unevaluatedProperties: false' seems to enable stricter checking which > > makes sense when all the properties are described in the schema. > > So I think this might be the problem. If I look at qcom,nandc.yaml or > ingenic,nand.yaml which both have a partitions property in their > example. Neither have 'unevaluatedProperties: false' on the nand@... > subnode. If I add it sure enough I start getting complaints about the > 'partitions' node being unexpected. Sorry if that was unclear, I think the whole logic around the yaml files is to progressively constrain the descriptions, schema after schema. IOW, in the marvell binding you should set unevaluatedProperties: false for the NAND controller. What is inside (NAND chips, partition container, partition parsers, "mtd" properties, etc) will be handled by other files. Of course you can constrain a bit what can/cannot be used inside these subnodes, but I think you don't need to set unevaluatedProperties in these subnodes (the NAND chip in this case, or even the partitions) because you already reference nand-controller.yaml which references nand-chip.yaml, mtd.yaml, partitions.yaml, etc. *they* will make the generic checks and hopefully apply stricter checks, when deemed relevant. Thanks, Miquèl ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-06 7:48 ` Miquel Raynal @ 2023-06-06 8:44 ` Krzysztof Kozlowski 2023-06-06 10:28 ` Miquel Raynal 0 siblings, 1 reply; 21+ messages in thread From: Krzysztof Kozlowski @ 2023-06-06 8:44 UTC (permalink / raw) To: Miquel Raynal, Chris Packham Cc: richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan On 06/06/2023 09:48, Miquel Raynal wrote: >>>>>>> + it (otherwise it is harmless). >>>>>>> + $ref: /schemas/types.yaml#/definitions/flag >>>>>>> + deprecated: true >>>>>>> + >>>>>>> + additionalProperties: false >>>>>> unevaluatedProperties: false >>>>> It was hiding by '"^nand@[0-3]$":'. Should I move it here? >>>> You cannot have both additionalProps and unevaluatedProps at the same >>>> time, so we do not talk about same thing or this was never working? >>> >>> Hmm, I'm a little confused then. At various times I've been told to >>> put 'additionalProperties: false' or 'unevaluatedProperties: false' >>> (although never at the same time). I'm not sure when to use one or the >>> other. >>> >>> From what I've been able to glean 'additionalProperties: true' >>> indicates that the node is expected to have child nodes defined in a >>> different schema so I would have thought 'additionalProperties: false' >>> would be appropriate for a schema covering a leaf node. >>> 'unevaluatedProperties: false' seems to enable stricter checking which >>> makes sense when all the properties are described in the schema. >> >> So I think this might be the problem. If I look at qcom,nandc.yaml or >> ingenic,nand.yaml which both have a partitions property in their >> example. Neither have 'unevaluatedProperties: false' on the nand@... >> subnode. If I add it sure enough I start getting complaints about the >> 'partitions' node being unexpected. > > Sorry if that was unclear, I think the whole logic around the yaml > files is to progressively constrain the descriptions, schema after > schema. IOW, in the marvell binding you should set > unevaluatedProperties: false for the NAND controller. What is inside > (NAND chips, partition container, partition parsers, "mtd" properties, > etc) will be handled by other files. Of course you can constrain a bit > what can/cannot be used inside these subnodes, but I think you don't > need to set unevaluatedProperties in these subnodes (the NAND chip in > this case, or even the partitions) because you already reference > nand-controller.yaml which references nand-chip.yaml, mtd.yaml, > partitions.yaml, etc. *they* will make the generic checks and hopefully > apply stricter checks, when deemed relevant. No, neither nand-controller.yaml nor nand-chip.yaml limit the properties in this context, so each device schema must have unevaluatedProperties: false, for which I asked few emails ago. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-06 8:44 ` Krzysztof Kozlowski @ 2023-06-06 10:28 ` Miquel Raynal 2023-06-06 10:37 ` Krzysztof Kozlowski 0 siblings, 1 reply; 21+ messages in thread From: Miquel Raynal @ 2023-06-06 10:28 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Chris Packham, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan Hi Krzysztof, krzysztof.kozlowski@linaro.org wrote on Tue, 6 Jun 2023 10:44:34 +0200: > On 06/06/2023 09:48, Miquel Raynal wrote: > >>>>>>> + it (otherwise it is harmless). > >>>>>>> + $ref: /schemas/types.yaml#/definitions/flag > >>>>>>> + deprecated: true > >>>>>>> + > >>>>>>> + additionalProperties: false > >>>>>> unevaluatedProperties: false > >>>>> It was hiding by '"^nand@[0-3]$":'. Should I move it here? > >>>> You cannot have both additionalProps and unevaluatedProps at the same > >>>> time, so we do not talk about same thing or this was never working? > >>> > >>> Hmm, I'm a little confused then. At various times I've been told to > >>> put 'additionalProperties: false' or 'unevaluatedProperties: false' > >>> (although never at the same time). I'm not sure when to use one or the > >>> other. > >>> > >>> From what I've been able to glean 'additionalProperties: true' > >>> indicates that the node is expected to have child nodes defined in a > >>> different schema so I would have thought 'additionalProperties: false' > >>> would be appropriate for a schema covering a leaf node. > >>> 'unevaluatedProperties: false' seems to enable stricter checking which > >>> makes sense when all the properties are described in the schema. > >> > >> So I think this might be the problem. If I look at qcom,nandc.yaml or > >> ingenic,nand.yaml which both have a partitions property in their > >> example. Neither have 'unevaluatedProperties: false' on the nand@... > >> subnode. If I add it sure enough I start getting complaints about the > >> 'partitions' node being unexpected. > > > > Sorry if that was unclear, I think the whole logic around the yaml > > files is to progressively constrain the descriptions, schema after > > schema. IOW, in the marvell binding you should set > > unevaluatedProperties: false for the NAND controller. What is inside > > (NAND chips, partition container, partition parsers, "mtd" properties, > > etc) will be handled by other files. Of course you can constrain a bit > > what can/cannot be used inside these subnodes, but I think you don't > > need to set unevaluatedProperties in these subnodes (the NAND chip in > > this case, or even the partitions) because you already reference > > nand-controller.yaml which references nand-chip.yaml, mtd.yaml, > > partitions.yaml, etc. *they* will make the generic checks and hopefully > > apply stricter checks, when deemed relevant. > > No, neither nand-controller.yaml nor nand-chip.yaml limit the properties > in this context, so each device schema must have unevaluatedProperties: > false, for which I asked few emails ago. The controller description shall be guarded by unevaluatedProperties: false, we agree. Do you mean the nand chip description in each nand controller binding should also include it at its own level? Because that is not what we enforced so far IIRC. I am totally fine doing so starting from now on if this is a new requirement (which makes sense). If yes, then it means we would need to list *all* the nand chip properties in each schema, which clearly involves a lot of duplication as you would need to define all types of partitions, partition parsers, generic properties, etc in order for the examples to pass all the checks. Only the properties like pinctrl-* would not need to be listed I guess. As Chris was having issues comparing his work with the ingenic and qcom yaml files, I gave your input a try and hopefully "fixed" these bindings. I'll Cc Chris on the submission so that he has an example of working -but maybe not fully valid, let's see- binding to take as example. Thanks, Miquèl ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-06 10:28 ` Miquel Raynal @ 2023-06-06 10:37 ` Krzysztof Kozlowski 2023-06-06 10:40 ` Krzysztof Kozlowski 0 siblings, 1 reply; 21+ messages in thread From: Krzysztof Kozlowski @ 2023-06-06 10:37 UTC (permalink / raw) To: Miquel Raynal Cc: Chris Packham, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan On 06/06/2023 12:28, Miquel Raynal wrote: > Hi Krzysztof, > > krzysztof.kozlowski@linaro.org wrote on Tue, 6 Jun 2023 10:44:34 +0200: > >> On 06/06/2023 09:48, Miquel Raynal wrote: >>>>>>>>> + it (otherwise it is harmless). >>>>>>>>> + $ref: /schemas/types.yaml#/definitions/flag >>>>>>>>> + deprecated: true >>>>>>>>> + >>>>>>>>> + additionalProperties: false >>>>>>>> unevaluatedProperties: false >>>>>>> It was hiding by '"^nand@[0-3]$":'. Should I move it here? >>>>>> You cannot have both additionalProps and unevaluatedProps at the same >>>>>> time, so we do not talk about same thing or this was never working? >>>>> >>>>> Hmm, I'm a little confused then. At various times I've been told to >>>>> put 'additionalProperties: false' or 'unevaluatedProperties: false' >>>>> (although never at the same time). I'm not sure when to use one or the >>>>> other. >>>>> >>>>> From what I've been able to glean 'additionalProperties: true' >>>>> indicates that the node is expected to have child nodes defined in a >>>>> different schema so I would have thought 'additionalProperties: false' >>>>> would be appropriate for a schema covering a leaf node. >>>>> 'unevaluatedProperties: false' seems to enable stricter checking which >>>>> makes sense when all the properties are described in the schema. >>>> >>>> So I think this might be the problem. If I look at qcom,nandc.yaml or >>>> ingenic,nand.yaml which both have a partitions property in their >>>> example. Neither have 'unevaluatedProperties: false' on the nand@... >>>> subnode. If I add it sure enough I start getting complaints about the >>>> 'partitions' node being unexpected. >>> >>> Sorry if that was unclear, I think the whole logic around the yaml >>> files is to progressively constrain the descriptions, schema after >>> schema. IOW, in the marvell binding you should set >>> unevaluatedProperties: false for the NAND controller. What is inside >>> (NAND chips, partition container, partition parsers, "mtd" properties, >>> etc) will be handled by other files. Of course you can constrain a bit >>> what can/cannot be used inside these subnodes, but I think you don't >>> need to set unevaluatedProperties in these subnodes (the NAND chip in >>> this case, or even the partitions) because you already reference >>> nand-controller.yaml which references nand-chip.yaml, mtd.yaml, >>> partitions.yaml, etc. *they* will make the generic checks and hopefully >>> apply stricter checks, when deemed relevant. >> >> No, neither nand-controller.yaml nor nand-chip.yaml limit the properties >> in this context, so each device schema must have unevaluatedProperties: >> false, for which I asked few emails ago. > > The controller description shall be guarded by unevaluatedProperties: > false, we agree. Do you mean the nand chip description in each nand > controller binding should also include it at its own level? Because > that is not what we enforced so far IIRC. I am totally fine doing so > starting from now on if this is a new requirement (which makes sense). > > If yes, then it means we would need to list *all* the nand > chip properties in each schema, which clearly involves a lot of > duplication as you would need to define all types of partitions, > partition parsers, generic properties, etc in order for the examples to > pass all the checks. Only the properties like pinctrl-* would not need > to be listed I guess. Yes, this is what should be done. Each node should have either additional or unevaluatedProperties: false. It might come from referenced schema or from final (device) schema), but it looks here it is missing. It's exactly the same in all bindings. You will find plenty of examples, e.g. individual leds children and common.yaml. > > As Chris was having issues comparing his work with the ingenic and qcom > yaml files, I gave your input a try and hopefully "fixed" these > bindings. Both are wrong in this matter. You can easily test it. Patch like: diff --git a/Documentation/devicetree/bindings/mtd/ingenic,nand.yaml b/Documentation/devicetree/bindings/mtd/ingenic,nand.yaml index a7bdb5d3675c..968c42f6b5a2 100644 --- a/Documentation/devicetree/bindings/mtd/ingenic,nand.yaml +++ b/Documentation/devicetree/bindings/mtd/ingenic,nand.yaml @@ -97,6 +97,8 @@ examples: nand-ecc-mode = "hw"; nand-on-flash-bbt; + fake-stuff; + pinctrl-names = "default"; pinctrl-0 = <&pins_nemc_cs1>; should trigger warning as this is clearly fake-stuff. No warning though... Best regards, Krzysztof ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-06 10:37 ` Krzysztof Kozlowski @ 2023-06-06 10:40 ` Krzysztof Kozlowski 2023-06-06 10:57 ` Miquel Raynal 0 siblings, 1 reply; 21+ messages in thread From: Krzysztof Kozlowski @ 2023-06-06 10:40 UTC (permalink / raw) To: Miquel Raynal Cc: Chris Packham, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan On 06/06/2023 12:37, Krzysztof Kozlowski wrote: > On 06/06/2023 12:28, Miquel Raynal wrote: >> Hi Krzysztof, >> >> krzysztof.kozlowski@linaro.org wrote on Tue, 6 Jun 2023 10:44:34 +0200: >> >>> On 06/06/2023 09:48, Miquel Raynal wrote: >>>>>>>>>> + it (otherwise it is harmless). >>>>>>>>>> + $ref: /schemas/types.yaml#/definitions/flag >>>>>>>>>> + deprecated: true >>>>>>>>>> + >>>>>>>>>> + additionalProperties: false >>>>>>>>> unevaluatedProperties: false >>>>>>>> It was hiding by '"^nand@[0-3]$":'. Should I move it here? >>>>>>> You cannot have both additionalProps and unevaluatedProps at the same >>>>>>> time, so we do not talk about same thing or this was never working? >>>>>> >>>>>> Hmm, I'm a little confused then. At various times I've been told to >>>>>> put 'additionalProperties: false' or 'unevaluatedProperties: false' >>>>>> (although never at the same time). I'm not sure when to use one or the >>>>>> other. >>>>>> >>>>>> From what I've been able to glean 'additionalProperties: true' >>>>>> indicates that the node is expected to have child nodes defined in a >>>>>> different schema so I would have thought 'additionalProperties: false' >>>>>> would be appropriate for a schema covering a leaf node. >>>>>> 'unevaluatedProperties: false' seems to enable stricter checking which >>>>>> makes sense when all the properties are described in the schema. >>>>> >>>>> So I think this might be the problem. If I look at qcom,nandc.yaml or >>>>> ingenic,nand.yaml which both have a partitions property in their >>>>> example. Neither have 'unevaluatedProperties: false' on the nand@... >>>>> subnode. If I add it sure enough I start getting complaints about the >>>>> 'partitions' node being unexpected. >>>> >>>> Sorry if that was unclear, I think the whole logic around the yaml >>>> files is to progressively constrain the descriptions, schema after >>>> schema. IOW, in the marvell binding you should set >>>> unevaluatedProperties: false for the NAND controller. What is inside >>>> (NAND chips, partition container, partition parsers, "mtd" properties, >>>> etc) will be handled by other files. Of course you can constrain a bit >>>> what can/cannot be used inside these subnodes, but I think you don't >>>> need to set unevaluatedProperties in these subnodes (the NAND chip in >>>> this case, or even the partitions) because you already reference >>>> nand-controller.yaml which references nand-chip.yaml, mtd.yaml, >>>> partitions.yaml, etc. *they* will make the generic checks and hopefully >>>> apply stricter checks, when deemed relevant. >>> >>> No, neither nand-controller.yaml nor nand-chip.yaml limit the properties >>> in this context, so each device schema must have unevaluatedProperties: >>> false, for which I asked few emails ago. >> >> The controller description shall be guarded by unevaluatedProperties: >> false, we agree. Do you mean the nand chip description in each nand >> controller binding should also include it at its own level? Because >> that is not what we enforced so far IIRC. I am totally fine doing so >> starting from now on if this is a new requirement (which makes sense). >> >> If yes, then it means we would need to list *all* the nand >> chip properties in each schema, which clearly involves a lot of >> duplication as you would need to define all types of partitions, >> partition parsers, generic properties, etc in order for the examples to >> pass all the checks. Only the properties like pinctrl-* would not need >> to be listed I guess. > > Yes, this is what should be done. Each node should have either Eh, no, I responded in wrong part of message. My yes was for: " Do you mean the nand chip description in each nand controller binding should also include it at its own level?" Now for actual paragraph: "If yes, then it means we would need to list *all* the nand chip properties in each schema," No, why? I don't understand. Use the same pattern as all other bindings, this is not special. Absolutely all have the same behavior, e.g. mentioned leds. You finish with unevaluatedProps and you're done, which is what I wrote here long, long time ago. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-06 10:40 ` Krzysztof Kozlowski @ 2023-06-06 10:57 ` Miquel Raynal 2023-06-06 11:07 ` Miquel Raynal 2023-06-06 11:09 ` Krzysztof Kozlowski 0 siblings, 2 replies; 21+ messages in thread From: Miquel Raynal @ 2023-06-06 10:57 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Chris Packham, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan Hi Krzysztof, krzysztof.kozlowski@linaro.org wrote on Tue, 6 Jun 2023 12:40:45 +0200: > On 06/06/2023 12:37, Krzysztof Kozlowski wrote: > > On 06/06/2023 12:28, Miquel Raynal wrote: > >> Hi Krzysztof, > >> > >> krzysztof.kozlowski@linaro.org wrote on Tue, 6 Jun 2023 10:44:34 +0200: > >> > >>> On 06/06/2023 09:48, Miquel Raynal wrote: > >>>>>>>>>> + it (otherwise it is harmless). > >>>>>>>>>> + $ref: /schemas/types.yaml#/definitions/flag > >>>>>>>>>> + deprecated: true > >>>>>>>>>> + > >>>>>>>>>> + additionalProperties: false > >>>>>>>>> unevaluatedProperties: false > >>>>>>>> It was hiding by '"^nand@[0-3]$":'. Should I move it here? > >>>>>>> You cannot have both additionalProps and unevaluatedProps at the same > >>>>>>> time, so we do not talk about same thing or this was never working? > >>>>>> > >>>>>> Hmm, I'm a little confused then. At various times I've been told to > >>>>>> put 'additionalProperties: false' or 'unevaluatedProperties: false' > >>>>>> (although never at the same time). I'm not sure when to use one or the > >>>>>> other. > >>>>>> > >>>>>> From what I've been able to glean 'additionalProperties: true' > >>>>>> indicates that the node is expected to have child nodes defined in a > >>>>>> different schema so I would have thought 'additionalProperties: false' > >>>>>> would be appropriate for a schema covering a leaf node. > >>>>>> 'unevaluatedProperties: false' seems to enable stricter checking which > >>>>>> makes sense when all the properties are described in the schema. > >>>>> > >>>>> So I think this might be the problem. If I look at qcom,nandc.yaml or > >>>>> ingenic,nand.yaml which both have a partitions property in their > >>>>> example. Neither have 'unevaluatedProperties: false' on the nand@... > >>>>> subnode. If I add it sure enough I start getting complaints about the > >>>>> 'partitions' node being unexpected. > >>>> > >>>> Sorry if that was unclear, I think the whole logic around the yaml > >>>> files is to progressively constrain the descriptions, schema after > >>>> schema. IOW, in the marvell binding you should set > >>>> unevaluatedProperties: false for the NAND controller. What is inside > >>>> (NAND chips, partition container, partition parsers, "mtd" properties, > >>>> etc) will be handled by other files. Of course you can constrain a bit > >>>> what can/cannot be used inside these subnodes, but I think you don't > >>>> need to set unevaluatedProperties in these subnodes (the NAND chip in > >>>> this case, or even the partitions) because you already reference > >>>> nand-controller.yaml which references nand-chip.yaml, mtd.yaml, > >>>> partitions.yaml, etc. *they* will make the generic checks and hopefully > >>>> apply stricter checks, when deemed relevant. > >>> > >>> No, neither nand-controller.yaml nor nand-chip.yaml limit the properties > >>> in this context, so each device schema must have unevaluatedProperties: > >>> false, for which I asked few emails ago. > >> > >> The controller description shall be guarded by unevaluatedProperties: > >> false, we agree. Do you mean the nand chip description in each nand > >> controller binding should also include it at its own level? Because > >> that is not what we enforced so far IIRC. I am totally fine doing so > >> starting from now on if this is a new requirement (which makes sense). > >> > >> If yes, then it means we would need to list *all* the nand > >> chip properties in each schema, which clearly involves a lot of > >> duplication as you would need to define all types of partitions, > >> partition parsers, generic properties, etc in order for the examples to > >> pass all the checks. Only the properties like pinctrl-* would not need > >> to be listed I guess. > > > > Yes, this is what should be done. Each node should have either > > Eh, no, I responded in wrong part of message. My yes was for: > > " Do you mean the nand chip description in each nand > controller binding should also include it at its own level?" Clear. > > Now for actual paragraph: > > "If yes, then it means we would need to list *all* the nand chip > properties in each schema," > > No, why? I don't understand. Use the same pattern as all other bindings, > this is not special. Absolutely all have the same behavior, e.g. > mentioned leds. You finish with unevaluatedProps and you're done, which > is what I wrote here long, long time ago. Maybe because so far we did not bother referencing another schema in the NAND chip nodes? For your hint to work I guess we should have, in each controller binding, something along: patternProperties: "^nand@[a-f0-9]$": type: object + $ref: nand-chip.yaml# properties: If yes, please ignore the series sent aside, I will work on it again and send a v2. Thanks, Miquèl ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-06 10:57 ` Miquel Raynal @ 2023-06-06 11:07 ` Miquel Raynal 2023-06-06 11:11 ` Krzysztof Kozlowski 2023-06-06 11:09 ` Krzysztof Kozlowski 1 sibling, 1 reply; 21+ messages in thread From: Miquel Raynal @ 2023-06-06 11:07 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Chris Packham, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan Hi Krzysztof, miquel.raynal@bootlin.com wrote on Tue, 6 Jun 2023 12:57:24 +0200: > Hi Krzysztof, > > krzysztof.kozlowski@linaro.org wrote on Tue, 6 Jun 2023 12:40:45 +0200: > > > On 06/06/2023 12:37, Krzysztof Kozlowski wrote: > > > On 06/06/2023 12:28, Miquel Raynal wrote: > > >> Hi Krzysztof, > > >> > > >> krzysztof.kozlowski@linaro.org wrote on Tue, 6 Jun 2023 10:44:34 +0200: > > >> > > >>> On 06/06/2023 09:48, Miquel Raynal wrote: > > >>>>>>>>>> + it (otherwise it is harmless). > > >>>>>>>>>> + $ref: /schemas/types.yaml#/definitions/flag > > >>>>>>>>>> + deprecated: true > > >>>>>>>>>> + > > >>>>>>>>>> + additionalProperties: false > > >>>>>>>>> unevaluatedProperties: false > > >>>>>>>> It was hiding by '"^nand@[0-3]$":'. Should I move it here? > > >>>>>>> You cannot have both additionalProps and unevaluatedProps at the same > > >>>>>>> time, so we do not talk about same thing or this was never working? > > >>>>>> > > >>>>>> Hmm, I'm a little confused then. At various times I've been told to > > >>>>>> put 'additionalProperties: false' or 'unevaluatedProperties: false' > > >>>>>> (although never at the same time). I'm not sure when to use one or the > > >>>>>> other. > > >>>>>> > > >>>>>> From what I've been able to glean 'additionalProperties: true' > > >>>>>> indicates that the node is expected to have child nodes defined in a > > >>>>>> different schema so I would have thought 'additionalProperties: false' > > >>>>>> would be appropriate for a schema covering a leaf node. > > >>>>>> 'unevaluatedProperties: false' seems to enable stricter checking which > > >>>>>> makes sense when all the properties are described in the schema. > > >>>>> > > >>>>> So I think this might be the problem. If I look at qcom,nandc.yaml or > > >>>>> ingenic,nand.yaml which both have a partitions property in their > > >>>>> example. Neither have 'unevaluatedProperties: false' on the nand@... > > >>>>> subnode. If I add it sure enough I start getting complaints about the > > >>>>> 'partitions' node being unexpected. > > >>>> > > >>>> Sorry if that was unclear, I think the whole logic around the yaml > > >>>> files is to progressively constrain the descriptions, schema after > > >>>> schema. IOW, in the marvell binding you should set > > >>>> unevaluatedProperties: false for the NAND controller. What is inside > > >>>> (NAND chips, partition container, partition parsers, "mtd" properties, > > >>>> etc) will be handled by other files. Of course you can constrain a bit > > >>>> what can/cannot be used inside these subnodes, but I think you don't > > >>>> need to set unevaluatedProperties in these subnodes (the NAND chip in > > >>>> this case, or even the partitions) because you already reference > > >>>> nand-controller.yaml which references nand-chip.yaml, mtd.yaml, > > >>>> partitions.yaml, etc. *they* will make the generic checks and hopefully > > >>>> apply stricter checks, when deemed relevant. > > >>> > > >>> No, neither nand-controller.yaml nor nand-chip.yaml limit the properties > > >>> in this context, so each device schema must have unevaluatedProperties: > > >>> false, for which I asked few emails ago. > > >> > > >> The controller description shall be guarded by unevaluatedProperties: > > >> false, we agree. Do you mean the nand chip description in each nand > > >> controller binding should also include it at its own level? Because > > >> that is not what we enforced so far IIRC. I am totally fine doing so > > >> starting from now on if this is a new requirement (which makes sense). > > >> > > >> If yes, then it means we would need to list *all* the nand > > >> chip properties in each schema, which clearly involves a lot of > > >> duplication as you would need to define all types of partitions, > > >> partition parsers, generic properties, etc in order for the examples to > > >> pass all the checks. Only the properties like pinctrl-* would not need > > >> to be listed I guess. > > > > > > Yes, this is what should be done. Each node should have either > > > > Eh, no, I responded in wrong part of message. My yes was for: > > > > " Do you mean the nand chip description in each nand > > controller binding should also include it at its own level?" > > Clear. > > > > > Now for actual paragraph: > > > > "If yes, then it means we would need to list *all* the nand chip > > properties in each schema," > > > > No, why? I don't understand. Use the same pattern as all other bindings, > > this is not special. Absolutely all have the same behavior, e.g. > > mentioned leds. You finish with unevaluatedProps and you're done, which > > is what I wrote here long, long time ago. > > Maybe because so far we did not bother referencing another schema in > the NAND chip nodes? For your hint to work I guess we should have, in > each controller binding, something along: > > patternProperties: > "^nand@[a-f0-9]$": > type: object > + $ref: nand-chip.yaml# > properties: > > If yes, please ignore the series sent aside, I will work on it again > and send a v2. Actually I already see a problem, let's the ingenic,nand.yaml example. The goal, IIUC, is to do: patternProperties: "^nand@[a-f0-9]$": type: object + $ref: nand-chip.yaml properties: ... + unevaluatedProperties: false The example in this file uses a property, nand-on-flash-bbt, which is described inside nand-controller.yaml instead of nand-chip.yaml. Indeed, the former actually describes many properties which are a bit more controller related than chip related. With the above description, the example fails because nand-on-flash-bbt is not allowed (it is not listed in nand-chip.yaml). How would you proceed in this case? Maybe I could move all the NAND chip properties which are somehow related to NAND controllers (and defined in nand-controller.yaml) in a dedicated file and reference it from nand-chip.yaml? Any other idea is welcome. Thanks, Miquèl ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-06 11:07 ` Miquel Raynal @ 2023-06-06 11:11 ` Krzysztof Kozlowski 2023-06-06 11:14 ` Krzysztof Kozlowski 0 siblings, 1 reply; 21+ messages in thread From: Krzysztof Kozlowski @ 2023-06-06 11:11 UTC (permalink / raw) To: Miquel Raynal Cc: Chris Packham, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan On 06/06/2023 13:07, Miquel Raynal wrote: > Hi Krzysztof, > > miquel.raynal@bootlin.com wrote on Tue, 6 Jun 2023 12:57:24 +0200: > >> Hi Krzysztof, >> >> krzysztof.kozlowski@linaro.org wrote on Tue, 6 Jun 2023 12:40:45 +0200: >> >>> On 06/06/2023 12:37, Krzysztof Kozlowski wrote: >>>> On 06/06/2023 12:28, Miquel Raynal wrote: >>>>> Hi Krzysztof, >>>>> >>>>> krzysztof.kozlowski@linaro.org wrote on Tue, 6 Jun 2023 10:44:34 +0200: >>>>> >>>>>> On 06/06/2023 09:48, Miquel Raynal wrote: >>>>>>>>>>>>> + it (otherwise it is harmless). >>>>>>>>>>>>> + $ref: /schemas/types.yaml#/definitions/flag >>>>>>>>>>>>> + deprecated: true >>>>>>>>>>>>> + >>>>>>>>>>>>> + additionalProperties: false >>>>>>>>>>>> unevaluatedProperties: false >>>>>>>>>>> It was hiding by '"^nand@[0-3]$":'. Should I move it here? >>>>>>>>>> You cannot have both additionalProps and unevaluatedProps at the same >>>>>>>>>> time, so we do not talk about same thing or this was never working? >>>>>>>>> >>>>>>>>> Hmm, I'm a little confused then. At various times I've been told to >>>>>>>>> put 'additionalProperties: false' or 'unevaluatedProperties: false' >>>>>>>>> (although never at the same time). I'm not sure when to use one or the >>>>>>>>> other. >>>>>>>>> >>>>>>>>> From what I've been able to glean 'additionalProperties: true' >>>>>>>>> indicates that the node is expected to have child nodes defined in a >>>>>>>>> different schema so I would have thought 'additionalProperties: false' >>>>>>>>> would be appropriate for a schema covering a leaf node. >>>>>>>>> 'unevaluatedProperties: false' seems to enable stricter checking which >>>>>>>>> makes sense when all the properties are described in the schema. >>>>>>>> >>>>>>>> So I think this might be the problem. If I look at qcom,nandc.yaml or >>>>>>>> ingenic,nand.yaml which both have a partitions property in their >>>>>>>> example. Neither have 'unevaluatedProperties: false' on the nand@... >>>>>>>> subnode. If I add it sure enough I start getting complaints about the >>>>>>>> 'partitions' node being unexpected. >>>>>>> >>>>>>> Sorry if that was unclear, I think the whole logic around the yaml >>>>>>> files is to progressively constrain the descriptions, schema after >>>>>>> schema. IOW, in the marvell binding you should set >>>>>>> unevaluatedProperties: false for the NAND controller. What is inside >>>>>>> (NAND chips, partition container, partition parsers, "mtd" properties, >>>>>>> etc) will be handled by other files. Of course you can constrain a bit >>>>>>> what can/cannot be used inside these subnodes, but I think you don't >>>>>>> need to set unevaluatedProperties in these subnodes (the NAND chip in >>>>>>> this case, or even the partitions) because you already reference >>>>>>> nand-controller.yaml which references nand-chip.yaml, mtd.yaml, >>>>>>> partitions.yaml, etc. *they* will make the generic checks and hopefully >>>>>>> apply stricter checks, when deemed relevant. >>>>>> >>>>>> No, neither nand-controller.yaml nor nand-chip.yaml limit the properties >>>>>> in this context, so each device schema must have unevaluatedProperties: >>>>>> false, for which I asked few emails ago. >>>>> >>>>> The controller description shall be guarded by unevaluatedProperties: >>>>> false, we agree. Do you mean the nand chip description in each nand >>>>> controller binding should also include it at its own level? Because >>>>> that is not what we enforced so far IIRC. I am totally fine doing so >>>>> starting from now on if this is a new requirement (which makes sense). >>>>> >>>>> If yes, then it means we would need to list *all* the nand >>>>> chip properties in each schema, which clearly involves a lot of >>>>> duplication as you would need to define all types of partitions, >>>>> partition parsers, generic properties, etc in order for the examples to >>>>> pass all the checks. Only the properties like pinctrl-* would not need >>>>> to be listed I guess. >>>> >>>> Yes, this is what should be done. Each node should have either >>> >>> Eh, no, I responded in wrong part of message. My yes was for: >>> >>> " Do you mean the nand chip description in each nand >>> controller binding should also include it at its own level?" >> >> Clear. >> >>> >>> Now for actual paragraph: >>> >>> "If yes, then it means we would need to list *all* the nand chip >>> properties in each schema," >>> >>> No, why? I don't understand. Use the same pattern as all other bindings, >>> this is not special. Absolutely all have the same behavior, e.g. >>> mentioned leds. You finish with unevaluatedProps and you're done, which >>> is what I wrote here long, long time ago. >> >> Maybe because so far we did not bother referencing another schema in >> the NAND chip nodes? For your hint to work I guess we should have, in >> each controller binding, something along: >> >> patternProperties: >> "^nand@[a-f0-9]$": >> type: object >> + $ref: nand-chip.yaml# >> properties: >> >> If yes, please ignore the series sent aside, I will work on it again >> and send a v2. > > Actually I already see a problem, let's the ingenic,nand.yaml example. > The goal, IIUC, is to do: > > patternProperties: > "^nand@[a-f0-9]$": > type: object > + $ref: nand-chip.yaml > properties: > > ... > > + unevaluatedProperties: false > > The example in this file uses a property, nand-on-flash-bbt, which is > described inside nand-controller.yaml instead of nand-chip.yaml. > Indeed, the former actually describes many properties which are a bit > more controller related than chip related. With the above description, > the example fails because nand-on-flash-bbt is not allowed (it is not > listed in nand-chip.yaml). > > How would you proceed in this case? > > Maybe I could move all the NAND chip properties which are somehow > related to NAND controllers (and defined in nand-controller.yaml) in a > dedicated file and reference it from nand-chip.yaml? Any other idea is > welcome. Yes, this would work and seems reasonable. Other way could be to add unevaluatedProperties:false on this level (so after ref:nand-chip.yaml) in nand-controller.yaml. This however would not allow any new properties to be defined in device bindings. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-06 11:11 ` Krzysztof Kozlowski @ 2023-06-06 11:14 ` Krzysztof Kozlowski 0 siblings, 0 replies; 21+ messages in thread From: Krzysztof Kozlowski @ 2023-06-06 11:14 UTC (permalink / raw) To: Miquel Raynal Cc: Chris Packham, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan On 06/06/2023 13:11, Krzysztof Kozlowski wrote: >>> If yes, please ignore the series sent aside, I will work on it again >>> and send a v2. >> >> Actually I already see a problem, let's the ingenic,nand.yaml example. >> The goal, IIUC, is to do: >> >> patternProperties: >> "^nand@[a-f0-9]$": >> type: object >> + $ref: nand-chip.yaml >> properties: >> >> ... >> >> + unevaluatedProperties: false >> >> The example in this file uses a property, nand-on-flash-bbt, which is >> described inside nand-controller.yaml instead of nand-chip.yaml. >> Indeed, the former actually describes many properties which are a bit >> more controller related than chip related. With the above description, >> the example fails because nand-on-flash-bbt is not allowed (it is not >> listed in nand-chip.yaml). >> >> How would you proceed in this case? >> >> Maybe I could move all the NAND chip properties which are somehow >> related to NAND controllers (and defined in nand-controller.yaml) in a >> dedicated file and reference it from nand-chip.yaml? Any other idea is >> welcome. > > Yes, this would work and seems reasonable. Actually, since nand-chip is used by both SPI and NAND, then I think better would be to create separate file - nand-only-chip.yaml (name to be discussed): nand-controller.yaml: "^nand@[a-f0-9]$": $ref: nand-only-chip.yaml nand-only-chip.yaml: $ref: nand-chip.yaml all nand-controller-chip properties follow > Other way could be to add > unevaluatedProperties:false on this level (so after ref:nand-chip.yaml) > in nand-controller.yaml. This however would not allow any new properties > to be defined in device bindings. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme 2023-06-06 10:57 ` Miquel Raynal 2023-06-06 11:07 ` Miquel Raynal @ 2023-06-06 11:09 ` Krzysztof Kozlowski 1 sibling, 0 replies; 21+ messages in thread From: Krzysztof Kozlowski @ 2023-06-06 11:09 UTC (permalink / raw) To: Miquel Raynal Cc: Chris Packham, richard@nod.at, vigneshr@ti.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, andrew@lunn.ch, gregory.clement@bootlin.com, sebastian.hesselbarth@gmail.com, conor@kernel.org, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, enachman@marvell.com, Vadym Kochan On 06/06/2023 12:57, Miquel Raynal wrote: > >> >> Now for actual paragraph: >> >> "If yes, then it means we would need to list *all* the nand chip >> properties in each schema," >> >> No, why? I don't understand. Use the same pattern as all other bindings, >> this is not special. Absolutely all have the same behavior, e.g. >> mentioned leds. You finish with unevaluatedProps and you're done, which >> is what I wrote here long, long time ago. > > Maybe because so far we did not bother referencing another schema in > the NAND chip nodes? For your hint to work I guess we should have, in > each controller binding, something along: > > patternProperties: > "^nand@[a-f0-9]$": > type: object > + $ref: nand-chip.yaml# > properties: > > If yes, please ignore the series sent aside, I will work on it again > and send a v2. nand-controller.yaml has it, so ideally each device binding should not need it, because it already references nand-controller.yaml. However if it doesn't work, then you need nand-chip in each device binding. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-06-06 11:14 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-05-31 23:49 [PATCH v8 0/3] dt-bindings: mtd: marvell-nand: Add YAML scheme Chris Packham 2023-05-31 23:49 ` [PATCH v8 1/3] arm64: dts: marvell: cp11x: Fix nand_controller node name according to YAML Chris Packham 2023-05-31 23:49 ` [PATCH v8 2/3] ARM: dts: mvebu: align MTD partition nodes to dtschema Chris Packham 2023-05-31 23:49 ` [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme Chris Packham 2023-06-01 7:05 ` Krzysztof Kozlowski 2023-06-01 21:07 ` Chris Packham 2023-06-01 23:06 ` Chris Packham 2023-06-04 9:26 ` Krzysztof Kozlowski 2023-06-05 20:44 ` Chris Packham 2023-06-05 21:26 ` Andrew Lunn 2023-06-06 4:38 ` Chris Packham 2023-06-06 7:48 ` Miquel Raynal 2023-06-06 8:44 ` Krzysztof Kozlowski 2023-06-06 10:28 ` Miquel Raynal 2023-06-06 10:37 ` Krzysztof Kozlowski 2023-06-06 10:40 ` Krzysztof Kozlowski 2023-06-06 10:57 ` Miquel Raynal 2023-06-06 11:07 ` Miquel Raynal 2023-06-06 11:11 ` Krzysztof Kozlowski 2023-06-06 11:14 ` Krzysztof Kozlowski 2023-06-06 11:09 ` Krzysztof Kozlowski
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).