On 24/10/2022 15:48, Vadym Kochan wrote: > Hi Krzysztof, > > On Sat, 22 Oct 2022 12:18:49 -0400, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> On 21/10/2022 15:45, Vadym Kochan wrote: >>> Switch the DT binding to a YAML schema to enable the DT validation. >>> >>> Dropped deprecated compatibles and properties described in txt file. >>> >>> Signed-off-by: Vadym Kochan <vadym.kochan@xxxxxxxxxxx> >>> --- >>> .../bindings/mtd/marvell,nand-controller.yaml | 199 ++++++++++++++++++ >>> .../devicetree/bindings/mtd/marvell-nand.txt | 126 ----------- >>> 2 files changed, 199 insertions(+), 126 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml >>> delete mode 100644 Documentation/devicetree/bindings/mtd/marvell-nand.txt >>> >>> diff --git a/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml >>> new file mode 100644 >>> index 000000000000..535b7f8903c8 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/mtd/marvell,nand-controller.yaml >>> @@ -0,0 +1,199 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/mtd/marvell,nand-controller.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Marvell NAND Flash Controller (NFC) >>> + >>> +maintainers: >>> + - Miquel Raynal <miquel.raynal@xxxxxxxxxxx> >> >> This should be someone responsible for hardware, not subsystem >> maintainer. Unless by coincidence Miquel matches both. :) >> >>> + >>> +properties: >>> + >>> + compatible: >>> + oneOf: >>> + - items: >>> + - const: marvell,armada-8k-nand-controller >>> + - const: marvell,armada370-nand-controller >> >> Does not look like you tested the bindings. Please run `make >> dt_binding_check` (see >> Documentation/devicetree/bindings/writing-schema.rst for instructions). > > Yes, on v1 I did not use yamllint, but installed after Rob pointed > on some lint warnings. I did not say about yamllint. > >> >>> + - const: marvell,armada370-nand-controller >>> + - const: marvell,pxa3xx-nand-controller >> >> These two are just enum. >> > > OK. > >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + "#address-cells": >>> + const: 1 >> >> Drop, comes with nand-controller.yaml >> > > OK. > >>> + >>> + "#size-cells": >>> + const: 0 >> >> Ditto >> > > OK. > >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> + clocks: >>> + minItems: 1 >>> + maxItems: 2 >>> + description: | >> >> No need for | >> >>> + Shall reference the NAND controller clocks, the second one is >>> + is only needed for the Armada 7K/8K SoCs >> >> You need allOf:if:then restricting it further per variant. >> > > OK, added. > >>> + >>> + clock-names: >>> + items: >>> + - const: core >>> + - const: reg >>> + description: | >>> + Mandatory if there is a second clock, in this case there >>> + should be one clock named "core" and another one named "reg" >> >> The message is confusing. What is mandatory if there is a second clock? >> Plus, the binding requires two clocks. >> >> Drop entire description. >> >> minItems: 1 >> > > OK, droped (I used from the txt version). > Added minItems. > >> >>> + >>> + dmas: >>> + maxItems: 1 >>> + description: rxtx DMA channel >> >> Drop description. >> > > OK. > >>> + >>> + dma-names: >>> + items: >>> + - const: rxtx >>> + >>> + marvell,system-controller: >>> + $ref: /schemas/types.yaml#/definitions/phandle >>> + description: Syscon node that handles NAND controller related registers >>> + >>> +patternProperties: >>> + "^nand@[0-3]$": >>> + type: object >>> + properties: >>> + >> >> Drop blank line. >> > > OK. > >>> + reg: >>> + minimum: 0 >>> + maximum: 3 >>> + >>> + nand-rb: >>> + minimum: 0 >>> + maximum: 1 >>> + >>> + nand-ecc-strength: >>> + enum: [1, 4, 8] >>> + >>> + nand-on-flash-bbt: true >>> + >>> + nand-ecc-mode: true >>> + >>> + nand-ecc-algo: >>> + description: | >>> + This property is essentially useful when not using hardware ECC. >>> + Howerver, it may be added when using hardware ECC for clarification >>> + but will be ignored by the driver because ECC mode is chosen depending >>> + on the page size and the strength required by the NAND chip. >>> + This value may be overwritten with nand-ecc-strength property. >>> + >>> + nand-ecc-step-size: >>> + const: 512 >> >> Why this is const? >> > > Removed const. > >>> + description: | >>> + Marvell's NAND flash controller does use fixed strength >>> + (1-bit for Hamming, 16-bit for BCH), so the actual step size >>> + will shrink or grow in order to fit the required strength. >>> + Step sizes are not completely random for all and follow certain >>> + patterns described in AN-379, "Marvell SoC NFC ECC". >>> + >>> + label: >>> + $ref: /schemas/types.yaml#/definitions/string >>> + >>> + partitions: >>> + type: object >>> + $ref: "/schemas/mtd/partitions/partition.yaml" >> >> Drop quotes >> > > OK. > >> unevalautedProperties: false >> >> and then you will see errors, because you referenced schema for one >> partition. >> > > Hm, I did not see errors with partitions with- or without "unevaluatedProperties". As pointed before and here - I am not sure if you tested the bindings, so of course then will be no warnings... Best regards, Krzysztof
