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@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 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@xxxxxxxxxxx> >>>> + >>>> +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
