| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <eaf9d7cf-c9f5-a5d5-67af-c43761c3c6cf@linaro.org>
Date: Sun, 4 Jun 2023 11:26:20 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Chris Packham <Chris.Packham@...iedtelesis.co.nz>,
"miquel.raynal@...tlin.com" <miquel.raynal@...tlin.com>,
"richard@....at" <richard@....at>,
"vigneshr@...com" <vigneshr@...com>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org"
<krzysztof.kozlowski+dt@...aro.org>,
"conor+dt@...nel.org" <conor+dt@...nel.org>,
"andrew@...n.ch" <andrew@...n.ch>,
"gregory.clement@...tlin.com" <gregory.clement@...tlin.com>,
"sebastian.hesselbarth@...il.com" <sebastian.hesselbarth@...il.com>,
"conor@...nel.org" <conor@...nel.org>
Cc: "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"enachman@...vell.com" <enachman@...vell.com>,
Vadym Kochan <vadym.kochan@...ision.eu>
Subject: Re: [PATCH v8 3/3] dt-bindings: mtd: marvell-nand: Convert to YAML DT
scheme
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@...ision.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@...ision.eu>
>>> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.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@...tlin.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
Powered by blists - more mailing lists