Hi Krzysztof, On 15/06/23 19:45, Krzysztof Kozlowski wrote: > On 15/06/2023 06:04, Chris Packham wrote: >> From: Vadym Kochan <vadym.kochan@xxxxxxxxxxx> >> >> 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@xxxxxxxxxxx> >> Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx> >> --- >> >> Notes: >> Changes in v9: >> - depend on series from Miquel https://lore.kernel.org/linux-mtd/20230606175246.190465-1-miquel.raynal@xxxxxxxxxxx/ >> - enforce minimum/maximum for nand-rb >> - move required: block for controller >> - move unevaluatedProperties: for nand chip >> - remove label, partitions and nand-on-flash-bbt which are covered by >> generic schema >> >> Changes in v8: >> - Mark deprecated compatible values as such >> - Allow "marvell,armada-8k-nand-controller" without >> "marvell,armada370-nand-controller" > ??? > > >> + >> +properties: >> + compatible: >> + oneOf: >> + - items: >> + - const: marvell,armada-8k-nand-controller >> + - const: marvell,armada370-nand-controller >> + - enum: >> + - marvell,armada-8k-nand-controller > This is wrong. 8k cannot be both: compatible and not compatible with > 370. It's not someone's cat to be in both states at the same time... The correct state (IMHO, Miquel correct me if I'm wrong) is `compatible = "marvell,armada-8k-nand-controller";` as there are some 8K specific requirements that aren't present on the 370 (specifically the system-controller and the 2nd clock). The only reason `compatible = "marvell,armada-8k-nand-controller", "marvell,armada370-nand-controller";` is supported is because that is what is in use in the existing dtses. >> + - marvell,armada370-nand-controller >> + - marvell,pxa3xx-nand-controller >> + - description: legacy bindings >> + deprecated: true >> + enum: >> + - marvell,armada-8k-nand >> + - marvell,armada370-nand >> + - marvell,pxa3xx-nand >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + clocks: >> + description: >> + Shall reference the NAND controller clocks, the second one is >> + is only needed for the Armada 7K/8K SoCs >> + minItems: 1 >> + maxItems: 2 >> + >> + clock-names: >> + minItems: 1 >> + items: >> + - const: core >> + - const: reg >> + >> + dmas: >> + maxItems: 1 >> + >> + dma-names: >> + items: >> + - const: data >> + >> + marvell,system-controller: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: Syscon node that handles NAND controller related registers >> + >> +patternProperties: >> + "^nand@[a-f0-9]$": >> + type: object >> + $ref: raw-nand-chip.yaml >> + >> + properties: >> + reg: >> + minimum: 0 >> + maximum: 3 >> + >> + nand-rb: >> + items: >> + - minimum: 0 >> + maximum: 1 >> + >> + nand-ecc-step-size: >> + const: 512 >> + >> + nand-ecc-strength: >> + enum: [1, 4, 8, 12, 16] >> + >> + nand-ecc-mode: >> + const: hw >> + >> + marvell,nand-keep-config: >> + $ref: /schemas/types.yaml#/definitions/flag >> + description: | > Do not need '|' unless you need to preserve formatting. Ack >> + Orders the driver not to take the timings from the core and >> + leaving them completely untouched. Bootloader timings will then >> + be used. >> + >> + marvell,nand-enable-arbiter: >> + $ref: /schemas/types.yaml#/definitions/flag >> + description: | > Do not need '|' unless you need to preserve formatting. > >> + To enable the arbiter, all boards blindly used it, >> + this bit was set by the bootloader for many boards and even if >> + it is marked reserved in several datasheets, it might be needed to set >> + it (otherwise it is harmless). >> + deprecated: true >> + >> + required: >> + - reg >> + - nand-rb >> + >> + unevaluatedProperties: false >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - clocks >> + >> +allOf: >> + - $ref: nand-controller.yaml# >> + >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: marvell,pxa3xx-nand-controller >> + then: >> + required: >> + - dmas >> + - dma-names >> + >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: marvell,armada-8k-nand-controller >> + then: >> + properties: >> + clocks: >> + minItems: 2 >> + >> + clock-names: >> + minItems: 2 >> + >> + required: >> + - marvell,system-controller > else: > narrow clocks to 1? Or is the second clod also valid for other variants? > Does not look like from your example. Correct. Will update. > > > Best regards, > Krzysztof >
