Hi Conor, On 20/03/24 10:05 pm, Conor Dooley wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > ForwardedMessage.eml > > Subject: > Re: [PATCH 1/3] dt-bindings: mtd: atmel-nand: convert txt to yaml > From: > Conor Dooley <conor@xxxxxxxxxx> > Date: > 20/03/24, 10:05 pm > > To: > Balamanikandan Gunasundar <balamanikandan.gunasundar@xxxxxxxxxxxxx> > CC: > Miquel Raynal <miquel.raynal@xxxxxxxxxxx>, Richard Weinberger > <richard@xxxxxx>, Vignesh Raghavendra <vigneshr@xxxxxx>, Rob Herring > <robh@xxxxxxxxxx>, Krzysztof Kozlowski > <krzysztof.kozlowski+dt@xxxxxxxxxx>, Conor Dooley <conor+dt@xxxxxxxxxx>, > Nicolas Ferre <nicolas.ferre@xxxxxxxxxxxxx>, Alexandre Belloni > <alexandre.belloni@xxxxxxxxxxx>, Claudiu Beznea > <claudiu.beznea@xxxxxxxxx>, linux-mtd@xxxxxxxxxxxxxxxxxxx, > devicetree@xxxxxxxxxxxxxxx, linux-arm-kernel@xxxxxxxxxxxxxxxxxxx, > linux-kernel@xxxxxxxxxxxxxxx > > > On Wed, Mar 20, 2024 at 11:22:07AM +0530, Balamanikandan Gunasundar wrote: >> Convert text to yaml for atmel nand controller >> >> Signed-off-by: Balamanikandan Gunasundar<balamanikandan.gunasundar@xxxxxxxxxxxxx> >> --- >> .../devicetree/bindings/mtd/atmel-nand.txt | 50 ------- >> .../devicetree/bindings/mtd/atmel-nand.yaml | 166 +++++++++++++++++++++ >> MAINTAINERS | 2 +- >> 3 files changed, 167 insertions(+), 51 deletions(-) >> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.yaml b/Documentation/devicetree/bindings/mtd/atmel-nand.yaml >> new file mode 100644 >> index 000000000000..a5482d292293 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.yaml > Filename matching a compatible please. > >> @@ -0,0 +1,166 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id:http://devicetree.org/schemas/mtd/atmel-nand.yaml# >> +$schema:http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Atmel NAND flash controller >> + >> +maintainers: >> + - Balamanikandan Gunasundar<balamanikandan.gunasundar@xxxxxxxxxxxxx> >> + >> +description: | >> + The NAND flash controller node should be defined under the EBI bus (see >> + Documentation/devicetree/bindings/memory-controllers/atmel,ebi.txt|yaml). > text|yaml? > >> + One or several NAND devices can be defined under this NAND controller. >> + The NAND controller might be connected to an ECC engine. >> + >> +properties: >> + compatible: >> + oneOf: >> + - items: >> + - enum: > This is just an enum, drop the items and oneof. > >> + - atmel,at91rm9200-nand-controller >> + - atmel,at91sam9260-nand-controller >> + - atmel,at91sam9261-nand-controller >> + - atmel,at91sam9g45-nand-controller >> + - atmel,sama5d3-nand-controller >> + - microchip,sam9x60-nand-controller >> + >> + ranges: >> + description: empty ranges property to forward EBI ranges definitions. >> + >> + ecc-engine: >> + description: >> + phandle to the PMECC block. Only meaningful if the SoC embeds a PMECC >> + engine. >> + >> +allOf: >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - atmel,at91rm9200-nand-controller >> + - atmel,at91sam9260-nand-controller >> + - atmel,at91sam9261-nand-controller >> + - atmel,at91sam9g45-nand-controller >> + - atmel,sama5d3-nand-controller >> + - microchip,sam9x60-nand-controller >> + then: >> + properties: >> + "#address-cells": >> + const: 2 >> + >> + "#size-cells": >> + const: 1 > Why is this in an if? Isn't this all of the devices in the binding? > The default nand-controller.yaml defines this as const values. (#address-cell : 1 and #size-cells : 1). I am trying to override this const value. May be I should think about better approach ? >> + >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: atmel,sama5d3-nand-controller >> + then: >> + properties: >> + atmel,nfc-io: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: phandle to the NFC IO block. >> + >> + atmel,nfc-sram: >> + $ref: /schemas/types.yaml#/definitions/phandle >> + description: phandle to the NFC SRAM block > Please define the properties at the top level and use if statements to > constrain them. > >> + >> +required: >> + - compatible >> + - ranges >> + - "#address-cells" >> + - "#size-cells" >> + >> +patternProperties: >> + "^nand@[a-f0-9]$": >> + type: object >> + $ref: nand-chip.yaml# >> + description: >> + NAND chip bindings. All generic properties described in >> + Documentation/devicetree/bindings/mtd/{common,nand}.txt also apply to >> + the NAND device node, and NAND partitions should be defined under the >> + NAND node as described in >> + Documentation/devicetree/bindings/mtd/partition.txt. > These files do not exist. > Apologies for copying the content from the text file. I will correct this. >> + properties: >> + reg: >> + minItems: 1 >> + description: >> + describes the CS lines assigned to the NAND device. If the NAND device >> + exposes multiple CS lines (multi-dies chips), your reg property will >> + contain X tuples of 3 entries. > The "if" here is not accurate. Your binding mandates there being 3 > entries. > >> + reg = <0x3 0x0 0x800000>; >> + 1st entry - the CS line this NAND chip is connected to >> + 2nd entry - the base offset of the memory region assigned to this >> + device (always 0) >> + 3rd entry - the memory region size (always 0x800000) >> + >> + rb-gpios: >> + description: >> + the GPIO(s) used to check the Ready/Busy status of the NAND. >> + >> + cs-gpios: >> + description: >> + the GPIO(s) used to control the CS line. >> + >> + det-gpios: >> + description: >> + the GPIO used to detect if a Smartmedia Card is present. >> + >> + "atmel,rb": >> + description: >> + an integer identifying the native Ready/Busy pin. Only meaningful >> + on sama5 SoCs. > Then please constrain it to sama5 SoCs only 🙂 > >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + >> +unevaluatedProperties: false >> + >> +examples: >> + - | >> + nfc_io: nfc-io@70000000 { >> + compatible = "atmel,sama5d3-nfc-io", "syscon"; >> + reg = <0x70000000 0x8000000>; >> + }; > What's this got to do with the binding? > >> + pmecc: ecc-engine@ffffc070 { >> + compatible = "atmel,at91sam9g45-pmecc"; >> + reg = <0xffffc070 0x490>, >> + <0xffffc500 0x100>; >> + }; >> + >> + ebi: ebi@10000000 { > Drop the unused label. > > Same applies here, read the coding style about how to write dts nodes > please. > Yes. I should fix the alignment. I will send a v2 shortly Thanks, Bala > Thanks, > Conor. > >> + compatible = "atmel,sama5d3-ebi"; >> + #address-cells = <2>; >> + #size-cells = <1>; >> + atmel,smc = <&hsmc>; >> + reg = <0x10000000 0x10000000 >> + 0x40000000 0x30000000>; >> + ranges = <0x0 0x0 0x10000000 0x10000000 >> + 0x1 0x0 0x40000000 0x10000000 >> + 0x2 0x0 0x50000000 0x10000000 >> + 0x3 0x0 0x60000000 0x10000000>; >> + clocks = <&mck>; >> + >> + nandflash_controller: nandflash-controller { >> + compatible = "atmel,sama5d3-nand-controller"; >> + ecc-engine = <&pmecc>; >> + #address-cells = <2>; >> + #size-cells = <1>; >> + ranges; >> + >> + nand@3 { >> + reg = <0x3 0x0 0x800000>; >> + atmel,rb = <0>; >> + >> + /* >> + * Put generic NAND/MTD properties and >> + * subnodes here. >> + */ >> + }; >> + }; >> + }; >> diff --git a/MAINTAINERS b/MAINTAINERS >> index b6582bd3eb2c..3f2a6756223f 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -14503,7 +14503,7 @@ MICROCHIP NAND DRIVER >> M: Balamanikandan Gunasundar<balamanikandan.gunasundar@xxxxxxxxxxxxx> >> L: linux-mtd@xxxxxxxxxxxxxxxxxxx >> S: Supported >> -F: Documentation/devicetree/bindings/mtd/atmel-nand.txt >> +F: Documentation/devicetree/bindings/mtd/atmel-*.yaml >> F: drivers/mtd/nand/raw/atmel/* >> >> MICROCHIP OTPC DRIVER >> >> -- >> 2.25.1 >>
