[PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT scheme
Chris Packham
Chris.Packham at alliedtelesis.co.nz
Mon Jun 5 21:38:01 PDT 2023
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 at 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 at plvision.eu>
>>>>> Signed-off-by: Chris Packham <chris.packham at 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 at 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 at ...
subnode. If I add it sure enough I start getting complaints about the
'partitions' node being unexpected.
>
>>
>> Best regards,
>> Krzysztof
More information about the linux-arm-kernel
mailing list