[PATCH 0/3] dt-bindings: mtd: atmel-nand: convert txt to yaml

[PATCH 0/3] dt-bindings: mtd: atmel-nand: convert txt to yaml

[Balamanikandan Gunasundar]

Convert the text bindings to yaml.

Note:

This patch is done on top of the commit I sent recently.
https://lore.kernel.org/all/20240320025258.6854-1-balamanikandan.gunasundar@microchip.com/
Please let me know if I should resend the above commit in this series.

Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
---
Balamanikandan Gunasundar (3):
      dt-bindings: mtd: atmel-nand: convert txt to yaml
      dt-bindings: mtd: atmel-nand: add atmel pmecc
      dt-bindings: mtd: atmel-nand: add deprecated bindings

 .../bindings/mtd/atmel-nand-deprecated.yaml        | 156 ++++++++++++++
 .../devicetree/bindings/mtd/atmel-nand.txt         | 236 ---------------------
 .../devicetree/bindings/mtd/atmel-nand.yaml        | 166 +++++++++++++++
 .../devicetree/bindings/mtd/atmel-pmecc.yaml       |  58 +++++
 MAINTAINERS                                        |   2 +-
 5 files changed, 381 insertions(+), 237 deletions(-)
---
base-commit: 66e1bf9c75595f96cefe0c32a7a719ba71c11bef
change-id: 20240320-linux-next-nand-yaml-b8674ce41e13

Best regards,
-- 
Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>

[PATCH 1/3] dt-bindings: mtd: atmel-nand: convert txt to yaml

[Balamanikandan Gunasundar]

Convert text to yaml for atmel nand controller

Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
---
 .../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.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
index 4598930851d9..e332515c499a 100644
--- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
@@ -1,53 +1,3 @@
-Atmel NAND flash controller bindings
-
-The NAND flash controller node should be defined under the EBI bus (see
-Documentation/devicetree/bindings/memory-controllers/atmel,ebi.txt).
-One or several NAND devices can be defined under this NAND controller.
-The NAND controller might be connected to an ECC engine.
-
-* NAND controller bindings:
-
-Required properties:
-- compatible: should be one of the following
-	"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: empty ranges property to forward EBI ranges definitions.
-- #address-cells: should be set to 2.
-- #size-cells: should be set to 1.
-- atmel,nfc-io: phandle to the NFC IO block. Only required for sama5d3
-		controllers.
-- atmel,nfc-sram: phandle to the NFC SRAM block. Only required for sama5d3
-		  controllers.
-
-Optional properties:
-- ecc-engine: phandle to the PMECC block. Only meaningful if the SoC embeds
-	      a PMECC engine.
-
-* NAND device/chip bindings:
-
-Required properties:
-- reg: 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.
-       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)
-
-Optional properties:
-- rb-gpios: the GPIO(s) used to check the Ready/Busy status of the NAND.
-- cs-gpios: the GPIO(s) used to control the CS line.
-- det-gpios: the GPIO used to detect if a Smartmedia Card is present.
-- atmel,rb: an integer identifying the native Ready/Busy pin. Only meaningful
-	    on sama5 SoCs.
-
-All generic properties are described in the generic yaml files under
-Documentation/devicetree/bindings/mtd/.
-
 * ECC engine (PMECC) bindings:
 
 Required properties:
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
@@ -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@microchip.com>
+
+description: |
+  The NAND flash controller node should be defined under the EBI bus (see
+  Documentation/devicetree/bindings/memory-controllers/atmel,ebi.txt|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:
+              - 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
+
+  - 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
+
+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.
+
+    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.
+         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.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    nfc_io: nfc-io@70000000 {
+            compatible = "atmel,sama5d3-nfc-io", "syscon";
+            reg = <0x70000000 0x8000000>;
+    };
+
+    pmecc: ecc-engine@ffffc070 {
+            compatible = "atmel,at91sam9g45-pmecc";
+            reg = <0xffffc070 0x490>,
+                  <0xffffc500 0x100>;
+    };
+
+    ebi: ebi@10000000 {
+            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@microchip.com>
 L:	linux-mtd@lists.infradead.org
 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

Re: [PATCH 1/3] dt-bindings: mtd: atmel-nand: convert txt to yaml

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 7656 bytes --]

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@microchip.com>
> ---
>  .../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@microchip.com>
> +
> +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?

> +
> +  - 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.

> +    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.

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@microchip.com>
>  L:	linux-mtd@lists.infradead.org
>  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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] dt-bindings: mtd: atmel-nand: convert txt to yaml

[Balamanikandan.Gunasundar]

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@kernel.org>
> Date:
> 20/03/24, 10:05 pm
> 
> To:
> Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
> CC:
> Miquel Raynal <miquel.raynal@bootlin.com>, Richard Weinberger 
> <richard@nod.at>, Vignesh Raghavendra <vigneshr@ti.com>, Rob Herring 
> <robh@kernel.org>, Krzysztof Kozlowski 
> <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, 
> Nicolas Ferre <nicolas.ferre@microchip.com>, Alexandre Belloni 
> <alexandre.belloni@bootlin.com>, Claudiu Beznea 
> <claudiu.beznea@tuxon.dev>, linux-mtd@lists.infradead.org, 
> devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, 
> linux-kernel@vger.kernel.org
> 
> 
> 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@microchip.com>
>> ---
>>   .../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@microchip.com>
>> +
>> +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@microchip.com>
>>   L:	linux-mtd@lists.infradead.org
>>   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
>>

Re: [PATCH 1/3] dt-bindings: mtd: atmel-nand: convert txt to yaml

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2313 bytes --]

On Fri, Mar 22, 2024 at 04:27:29AM +0000, Balamanikandan.Gunasundar@microchip.com wrote:
> On 20/03/24 10:05 pm, Conor Dooley wrote:
> > On Wed, Mar 20, 2024 at 11:22:07AM +0530, Balamanikandan Gunasundar wrote:

> >> +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.

You're not overriding anything as you don't have a ref to
nand-controller.yaml in this file, AFAICT. Why don't you?

> May be I should think about better approach ?

You should be able to apply this unconditionally for this file. I don't
see why the if would be needed?


> >> +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.

You don't need these comments at all I think. You have the ref to
nand-chip.yaml, so at least the first text file reference can be
removed.

> Yes. I should fix the alignment. I will send a v2 shortly

I did make other comments, so I assume you agree with everything else I
mentioned and will implement them in v2.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] dt-bindings: mtd: atmel-nand: convert txt to yaml

[Balamanikandan.Gunasundar]

On 22/03/24 1:00 pm, Conor Dooley wrote:
> On Fri, Mar 22, 2024 at 04:27:29AM +0000, Balamanikandan.Gunasundar@microchip.com wrote:
>> On 20/03/24 10:05 pm, Conor Dooley wrote:
>>> On Wed, Mar 20, 2024 at 11:22:07AM +0530, Balamanikandan Gunasundar wrote:
> 
>>>> +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.
> 
> You're not overriding anything as you don't have a ref to
> nand-controller.yaml in this file, AFAICT. Why don't you?
> 
>> May be I should think about better approach ?
> 
> You should be able to apply this unconditionally for this file. I don't
> see why the if would be needed?
> 
> 
>>>> +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.
> 
> You don't need these comments at all I think. You have the ref to
> nand-chip.yaml, so at least the first text file reference can be
> removed.
> 

Agree with this. I will remove it.

>> Yes. I should fix the alignment. I will send a v2 shortly
> 
> I did make other comments, so I assume you agree with everything else I
> mentioned and will implement them in v2.

Yes. I agree with other comments as well. I will also address all the 
comments with the other 2 patches and send a v2. Thanks for reviewing.

Regards,
Bala.

> 
> Thanks,
> Conor.

[PATCH 2/3] dt-bindings: mtd: atmel-nand: add atmel pmecc

[Balamanikandan Gunasundar]

Add bindings for programmable multibit error correction code controller
(PMECC).

Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
---
 .../devicetree/bindings/mtd/atmel-nand.txt         | 70 ----------------------
 .../devicetree/bindings/mtd/atmel-pmecc.yaml       | 58 ++++++++++++++++++
 2 files changed, 58 insertions(+), 70 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
index e332515c499a..1934614a9298 100644
--- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
@@ -1,73 +1,3 @@
-* ECC engine (PMECC) bindings:
-
-Required properties:
-- compatible: should be one of the following
-	"atmel,at91sam9g45-pmecc"
-	"atmel,sama5d4-pmecc"
-	"atmel,sama5d2-pmecc"
-	"microchip,sam9x60-pmecc"
-	"microchip,sam9x7-pmecc", "atmel,at91sam9g45-pmecc"
-- reg: should contain 2 register ranges. The first one is pointing to the PMECC
-       block, and the second one to the PMECC_ERRLOC block.
-
-* SAMA5 NFC I/O bindings:
-
-SAMA5 SoCs embed an advanced NAND controller logic to automate READ/WRITE page
-operations. This interface to this logic is placed in a separate I/O range and
-should thus have its own DT node.
-
-- compatible: should be "atmel,sama5d3-nfc-io", "syscon".
-- reg: should contain the I/O range used to interact with the NFC logic.
-
-Example:
-
-	nfc_io: nfc-io@70000000 {
-		compatible = "atmel,sama5d3-nfc-io", "syscon";
-		reg = <0x70000000 0x8000000>;
-	};
-
-	pmecc: ecc-engine@ffffc070 {
-		compatible = "atmel,at91sam9g45-pmecc";
-                reg = <0xffffc070 0x490>,
-                      <0xffffc500 0x100>;
-	};
-
-	ebi: ebi@10000000 {
-		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>;
-
-                nand_controller: nand-controller {
-			compatible = "atmel,sama5d3-nand-controller";
-			atmel,nfc-sram = <&nfc_sram>;
-			atmel,nfc-io = <&nfc_io>;
-			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.
-				 */
-			};
-		};
-	};
-
------------------------------------------------------------------------
-
 Deprecated bindings (should not be used in new device trees):
 
 Required properties:
diff --git a/Documentation/devicetree/bindings/mtd/atmel-pmecc.yaml b/Documentation/devicetree/bindings/mtd/atmel-pmecc.yaml
new file mode 100644
index 000000000000..872401e9dda3
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/atmel-pmecc.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/atmel-pmecc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip pmecc controller
+
+maintainers:
+  - Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
+
+description: |
+  Bindings for microchip Programmable Multibit Error Correction Code
+  Controller (PMECC). pmecc is a programmable BCH encoder/decoder. This
+  block is passed as the value to the "ecc-engine" property of microchip
+  nand flash controller node.
+
+properties:
+  compatible:
+    oneOf:
+      - enum:
+          - atmel,at91sam9g45-pmecc
+          - atmel,sama5d2-pmecc
+          - atmel,sama5d4-pmecc
+          - microchip,sam9x60-pmecc
+          - microchip,sam9x7-pmecc
+      - items:
+          - const: microchip,sam9x60-pmecc
+          - const: atmel,at91sam9g45-pmecc
+
+  reg:
+    description:
+      The first should point to the PMECC block. The second should point to the
+      PMECC_ERRLOC block.
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: microchip,sam9x7-pmecc
+    then:
+      properties:
+        clocks:
+          description:
+            The clock source for pmecc controller
+          maxItems: 1
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    pmecc: ecc-engine@ffffc070 {
+            compatible = "microchip,sam9x7-pmecc";
+            reg = <0xffffe000 0x300>,
+                  <0xffffe600 0x100>;
+            clocks = <&pmc 2 48>;
+    };

-- 
2.25.1

Re: [PATCH 2/3] dt-bindings: mtd: atmel-nand: add atmel pmecc

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2663 bytes --]

On Wed, Mar 20, 2024 at 11:22:08AM +0530, Balamanikandan Gunasundar wrote:
> Add bindings for programmable multibit error correction code controller
> (PMECC).
> 
> Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-pmecc.yaml b/Documentation/devicetree/bindings/mtd/atmel-pmecc.yaml

Filename matching a compatible please.

> new file mode 100644
> index 000000000000..872401e9dda3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/atmel-pmecc.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/atmel-pmecc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip pmecc controller
> +
> +maintainers:
> +  - Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
> +
> +description: |
> +  Bindings for microchip Programmable Multibit Error Correction Code
> +  Controller (PMECC). pmecc is a programmable BCH encoder/decoder. This
> +  block is passed as the value to the "ecc-engine" property of microchip
> +  nand flash controller node.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - enum:
> +          - atmel,at91sam9g45-pmecc
> +          - atmel,sama5d2-pmecc
> +          - atmel,sama5d4-pmecc
> +          - microchip,sam9x60-pmecc
> +          - microchip,sam9x7-pmecc
> +      - items:
> +          - const: microchip,sam9x60-pmecc
> +          - const: atmel,at91sam9g45-pmecc
> +
> +  reg:
> +    description:
> +      The first should point to the PMECC block. The second should point to the
> +      PMECC_ERRLOC block.

Constraints please. In fact, describe it as an items list and then you
don't need constraints or a free-form text explanation of what each
entry is :)

> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: microchip,sam9x7-pmecc
> +    then:
> +      properties:
> +        clocks:
> +          description:
> +            The clock source for pmecc controller
> +          maxItems: 1

Please define the property at the top level and constrain it on a per
device basis.

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    pmecc: ecc-engine@ffffc070 {

Drop the unused label please.

Thanks,
Conor.

> +            compatible = "microchip,sam9x7-pmecc";
> +            reg = <0xffffe000 0x300>,
> +                  <0xffffe600 0x100>;
> +            clocks = <&pmc 2 48>;
> +    };
> 
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 3/3] dt-bindings: mtd: atmel-nand: add deprecated bindings

[Balamanikandan Gunasundar]

Add nand bindings for legacy nand controllers. These bindings are not used
with the new device trees. This is still maintained to support legacy dt
bindings.

Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
---
 .../bindings/mtd/atmel-nand-deprecated.yaml        | 156 +++++++++++++++++++++
 .../devicetree/bindings/mtd/atmel-nand.txt         | 116 ---------------
 2 files changed, 156 insertions(+), 116 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand-deprecated.yaml b/Documentation/devicetree/bindings/mtd/atmel-nand-deprecated.yaml
new file mode 100644
index 000000000000..c8922ab0f1d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/atmel-nand-deprecated.yaml
@@ -0,0 +1,156 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/atmel-nand-deprecated.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel NAND flash controller deprecated bindings
+
+maintainers:
+  - Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
+
+description: |
+  This should not be used in the new device trees.
+
+properties:
+  compatible:
+    enum:
+      - atmel,at91rm9200-nand
+      - atmel,sama5d2-nand
+      - atmel,sama5d4-nand
+
+  reg:
+    description:
+      should specify localbus address and size used for the chip, and
+      hardware ECC controller if available. If the hardware ECC is PMECC,
+      it should contain address and size for PMECC and PMECC Error Location
+      controller.  The PMECC lookup table address and size in ROM is
+      optional. If not specified, driver will build it in runtime.
+
+  atmel,nand-addr-offset:
+    description:
+      offset for the address latch.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  atmel,nand-cmd-offset:
+    description:
+      offset for the command latch.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  "#address-cells":
+    description:
+      Must be present if the device has sub-nodes representing partitions
+
+  "#size-cells":
+    description:
+      Must be present if the device has sub-nodes representing partitions.
+
+  gpios:
+    description:
+      specifies the gpio pins to control the NAND device. detect is an
+      optional gpio and may be set to 0 if not present.
+
+  atmel,nand-has-dma:
+    description:
+      support dma transfer for nand read/write.
+    $ref: /schemas/types.yaml#/definitions/flag
+
+  atmel,has-pmecc:
+    description:
+      enable Programmable Multibit ECC hardware, capable of BCH encoding
+      and decoding, on devices where it is present.
+    $ref: /schemas/types.yaml#/definitions/flag
+
+  nand-on-flash-bbt:
+    description:
+      enable on flash bbt option if not present false
+    $ref: /schemas/types.yaml#/definitions/flag
+
+  nand-ecc-mode:
+    description:
+      operation mode of the NAND ecc mode, soft by default.  Supported
+    enum:
+      [none, soft, hw, hw_syndrome, hw_oob_first, soft_bch]
+    $ref: /schemas/types.yaml#/definitions/string
+
+  atmel,pmecc-cap:
+    description:
+      error correct capability for Programmable Multibit ECC Controller. If
+      the compatible string is "atmel,sama5d2-nand", 32 is also valid.
+    enum:
+      [2, 4, 8, 12, 24]
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  atmel,pmecc-sector-size:
+    description:
+      sector size for ECC computation.
+    enum:
+      [512, 1024]
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  atmel,pmecc-lookup-table-offset:
+    description:
+      includes two offsets of lookup table in ROM for different sector
+      size. First one is for sector size 512, the next is for sector size
+      1024. If not specified, driver will build the table in runtime.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  nand-bus-width:
+    description:
+      nand bus width
+    enum:
+      [8, 16]
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - reg
+  - atmel,nand-addr-offset
+  - atmel,nand-cmd-offset
+  - "#address-cells"
+  - "#size-cells"
+  - gpios
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    nand0: nand@40000000,0 {
+            compatible = "atmel,at91rm9200-nand";
+            #address-cells = <1>;
+            #size-cells = <1>;
+            reg = <0x40000000 0x10000000
+                   0xffffe800 0x200>;
+            atmel,nand-addr-offset = <21>;	/* ale */
+            atmel,nand-cmd-offset = <22>;	/* cle */
+            nand-on-flash-bbt;
+            nand-ecc-mode = "soft";
+            gpios = <&pioC 13 0	/* rdy */
+                     &pioC 14 0 /* nce */
+                     0		/* cd */
+                    >;
+    };
+  - |
+    /* for PMECC supported chips */
+    nand1@40000000 {
+            compatible = "atmel,at91rm9200-nand";
+            #address-cells = <1>;
+            #size-cells = <1>;
+            reg = <0x40000000 0x10000000	/* bus addr & size */
+                   0xffffe000 0x00000600	/* PMECC addr & size */
+                   0xffffe600 0x00000200	/* PMECC ERRLOC addr & size */
+                   0x00100000 0x00100000>;	/* ROM addr & size */
+
+            atmel,nand-addr-offset = <21>;	/* ale */
+            atmel,nand-cmd-offset = <22>;	/* cle */
+            nand-on-flash-bbt;
+            nand-ecc-mode = "hw";
+            atmel,has-pmecc;	/* enable PMECC */
+            atmel,pmecc-cap = <2>;
+            atmel,pmecc-sector-size = <512>;
+            atmel,pmecc-lookup-table-offset = <0x8000 0x10000>;
+            gpios = <&pioD 5 0	/* rdy */
+                     &pioD 4 0	/* nce */
+                     0		/* cd */
+                    >;
+    };
diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
deleted file mode 100644
index 1934614a9298..000000000000
--- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
+++ /dev/null
@@ -1,116 +0,0 @@
-Deprecated bindings (should not be used in new device trees):
-
-Required properties:
-- compatible: The possible values are:
-	"atmel,at91rm9200-nand"
-	"atmel,sama5d2-nand"
-	"atmel,sama5d4-nand"
-- reg : should specify localbus address and size used for the chip,
-	and hardware ECC controller if available.
-	If the hardware ECC is PMECC, it should contain address and size for
-	PMECC and PMECC Error Location controller.
-	The PMECC lookup table address and size in ROM is optional. If not
-	specified, driver will build it in runtime.
-- atmel,nand-addr-offset : offset for the address latch.
-- atmel,nand-cmd-offset : offset for the command latch.
-- #address-cells, #size-cells : Must be present if the device has sub-nodes
-  representing partitions.
-
-- gpios : specifies the gpio pins to control the NAND device. detect is an
-  optional gpio and may be set to 0 if not present.
-
-Optional properties:
-- atmel,nand-has-dma : boolean to support dma transfer for nand read/write.
-- nand-ecc-mode : String, operation mode of the NAND ecc mode, soft by default.
-  Supported values are: "none", "soft", "hw", "hw_syndrome", "hw_oob_first",
-  "soft_bch".
-- atmel,has-pmecc : boolean to enable Programmable Multibit ECC hardware,
-  capable of BCH encoding and decoding, on devices where it is present.
-- atmel,pmecc-cap : error correct capability for Programmable Multibit ECC
-  Controller. Supported values are: 2, 4, 8, 12, 24. If the compatible string
-  is "atmel,sama5d2-nand", 32 is also valid.
-- atmel,pmecc-sector-size : sector size for ECC computation. Supported values
-  are: 512, 1024.
-- atmel,pmecc-lookup-table-offset : includes two offsets of lookup table in ROM
-  for different sector size. First one is for sector size 512, the next is for
-  sector size 1024. If not specified, driver will build the table in runtime.
-- nand-bus-width : 8 or 16 bus width if not present 8
-- nand-on-flash-bbt: boolean to enable on flash bbt option if not present false
-
-Nand Flash Controller(NFC) is an optional sub-node
-Required properties:
-- compatible : "atmel,sama5d3-nfc".
-- reg : should specify the address and size used for NFC command registers,
-        NFC registers and NFC SRAM. NFC SRAM address and size can be absent
-        if don't want to use it.
-- clocks: phandle to the peripheral clock
-Optional properties:
-- atmel,write-by-sram: boolean to enable NFC write by SRAM.
-
-Examples:
-nand0: nand@40000000,0 {
-	compatible = "atmel,at91rm9200-nand";
-	#address-cells = <1>;
-	#size-cells = <1>;
-	reg = <0x40000000 0x10000000
-	       0xffffe800 0x200
-	      >;
-	atmel,nand-addr-offset = <21>;	/* ale */
-	atmel,nand-cmd-offset = <22>;	/* cle */
-	nand-on-flash-bbt;
-	nand-ecc-mode = "soft";
-	gpios = <&pioC 13 0	/* rdy */
-		 &pioC 14 0 	/* nce */
-		 0		/* cd */
-		>;
-	partition@0 {
-		...
-	};
-};
-
-/* for PMECC supported chips */
-nand0: nand@40000000 {
-	compatible = "atmel,at91rm9200-nand";
-	#address-cells = <1>;
-	#size-cells = <1>;
-	reg = < 0x40000000 0x10000000	/* bus addr & size */
-		0xffffe000 0x00000600	/* PMECC addr & size */
-		0xffffe600 0x00000200	/* PMECC ERRLOC addr & size */
-		0x00100000 0x00100000	/* ROM addr & size */
-		>;
-	atmel,nand-addr-offset = <21>;	/* ale */
-	atmel,nand-cmd-offset = <22>;	/* cle */
-	nand-on-flash-bbt;
-	nand-ecc-mode = "hw";
-	atmel,has-pmecc;	/* enable PMECC */
-	atmel,pmecc-cap = <2>;
-	atmel,pmecc-sector-size = <512>;
-	atmel,pmecc-lookup-table-offset = <0x8000 0x10000>;
-	gpios = <&pioD 5 0	/* rdy */
-		 &pioD 4 0	/* nce */
-		 0		/* cd */
-		>;
-	partition@0 {
-		...
-	};
-};
-
-/* for NFC supported chips */
-nand0: nand@40000000 {
-	compatible = "atmel,at91rm9200-nand";
-	#address-cells = <1>;
-	#size-cells = <1>;
-	ranges;
-        ...
-        nfc@70000000 {
-		compatible = "atmel,sama5d3-nfc";
-		#address-cells = <1>;
-		#size-cells = <1>;
-		clocks = <&hsmc_clk>
-		reg = <
-			0x70000000 0x10000000	/* NFC Command Registers */
-			0xffffc000 0x00000070	/* NFC HSMC regs */
-			0x00200000 0x00100000	/* NFC SRAM banks */
-		>;
-	};
-};

-- 
2.25.1

Re: [PATCH 3/3] dt-bindings: mtd: atmel-nand: add deprecated bindings

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 7969 bytes --]

On Wed, Mar 20, 2024 at 11:22:09AM +0530, Balamanikandan Gunasundar wrote:
> Add nand bindings for legacy nand controllers. These bindings are not used
> with the new device trees. This is still maintained to support legacy dt
> bindings.
> 
> Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
> ---
>  .../bindings/mtd/atmel-nand-deprecated.yaml        | 156 +++++++++++++++++++++
>  .../devicetree/bindings/mtd/atmel-nand.txt         | 116 ---------------
>  2 files changed, 156 insertions(+), 116 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand-deprecated.yaml b/Documentation/devicetree/bindings/mtd/atmel-nand-deprecated.yaml
> new file mode 100644
> index 000000000000..c8922ab0f1d5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand-deprecated.yaml

Node name matching the devices please.

> @@ -0,0 +1,156 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/atmel-nand-deprecated.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel NAND flash controller deprecated bindings

/stuff/linux-dt/Documentation/devicetree/bindings/mtd/atmel-nand-deprecated.yaml: title: 'Atmel NAND flash controller deprecated bindings' should not be valid under {'pattern': '([Bb]inding| [Ss]chema)'}
	hint: Everything is a binding/schema, no need to say it. Describe what hardware the binding is for.

> +
> +maintainers:
> +  - Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com>
> +
> +description: |
> +  This should not be used in the new device trees.

If these should not be used in new files, where are the replacement
bindings for the three devices listed here? I think, rather than being
deprecated, these are the only bindings for these devices and what you
actually want to say here is that these should not be used for /new
devices/. I'd drop mention of deprecation from the title as these
bindings are the only ones for this hardware AFAICT and just say that
new devices should be documented in $new_file.

> +properties:
> +  compatible:
> +    enum:
> +      - atmel,at91rm9200-nand
> +      - atmel,sama5d2-nand
> +      - atmel,sama5d4-nand
> +
> +  reg:

Missing constraints.

> +    description:
> +      should specify localbus address and size used for the chip, and
> +      hardware ECC controller if available. If the hardware ECC is PMECC,
> +      it should contain address and size for PMECC and PMECC Error Location
> +      controller.  The PMECC lookup table address and size in ROM is
> +      optional. If not specified, driver will build it in runtime.
> +
> +  atmel,nand-addr-offset:
> +    description:
> +      offset for the address latch.
> +    $ref: /schemas/types.yaml#/definitions/uint32

Missing constraints.

> +
> +  atmel,nand-cmd-offset:
> +    description:
> +      offset for the command latch.
> +    $ref: /schemas/types.yaml#/definitions/uint32

Missing contraints.

> +
> +  "#address-cells":
> +    description:
> +      Must be present if the device has sub-nodes representing partitions

Does this binding even allow partition child nodes? Hint: it doesn't.

> +  "#size-cells":
> +    description:
> +      Must be present if the device has sub-nodes representing partitions.
> +
> +  gpios:
> +    description:
> +      specifies the gpio pins to control the NAND device. detect is an
> +      optional gpio and may be set to 0 if not present.

Missing constraints (and maybe a type too? I dunno if "gpios" has a
special case in the -gpios regexes).

> +  atmel,nand-has-dma:
> +    description:
> +      support dma transfer for nand read/write.
> +    $ref: /schemas/types.yaml#/definitions/flag
> +
> +  atmel,has-pmecc:
> +    description:
> +      enable Programmable Multibit ECC hardware, capable of BCH encoding
> +      and decoding, on devices where it is present.
> +    $ref: /schemas/types.yaml#/definitions/flag
> +
> +  nand-on-flash-bbt:
> +    description:
> +      enable on flash bbt option if not present false
> +    $ref: /schemas/types.yaml#/definitions/flag
> +
> +  nand-ecc-mode:

Missing a default since this is optional.

> +    description:
> +      operation mode of the NAND ecc mode, soft by default.  Supported
> +    enum:
> +      [none, soft, hw, hw_syndrome, hw_oob_first, soft_bch]
> +    $ref: /schemas/types.yaml#/definitions/string
> +
> +  atmel,pmecc-cap:
> +    description:
> +      error correct capability for Programmable Multibit ECC Controller. If
> +      the compatible string is "atmel,sama5d2-nand", 32 is also valid.
> +    enum:
> +      [2, 4, 8, 12, 24]

You're missing an extra permitted value for the sama5d2.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  atmel,pmecc-sector-size:

Missing a default since this is optional.

> +    description:
> +      sector size for ECC computation.
> +    enum:
> +      [512, 1024]
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  atmel,pmecc-lookup-table-offset:

Missing a default since this is optional.

> +    description:
> +      includes two offsets of lookup table in ROM for different sector
> +      size. First one is for sector size 512, the next is for sector size
> +      1024. If not specified, driver will build the table in runtime.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> +  nand-bus-width:
> +    description:
> +      nand bus width
> +    enum:
> +      [8, 16]

Missing a default of 8 here.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +

You're missing the optional child node for the "nand flash controller" on
sama5d2.

> +required:
> +  - compatible
> +  - reg
> +  - atmel,nand-addr-offset
> +  - atmel,nand-cmd-offset
> +  - "#address-cells"
> +  - "#size-cells"
> +  - gpios
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    nand0: nand@40000000,0 {

Drop the unused node name.

> +            compatible = "atmel,at91rm9200-nand";
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            reg = <0x40000000 0x10000000
> +                   0xffffe800 0x200>;
> +            atmel,nand-addr-offset = <21>;	/* ale */
> +            atmel,nand-cmd-offset = <22>;	/* cle */
> +            nand-on-flash-bbt;
> +            nand-ecc-mode = "soft";
> +            gpios = <&pioC 13 0	/* rdy */
> +                     &pioC 14 0 /* nce */
> +                     0		/* cd */
> +                    >;

Please follow the coding style rather than copy-paste from the text
based binding. Applies to both examples. An example with the nfc would
be more helpful than two bindings for the same device.


Thanks,
Conor.

> +    };
> +  - |
> +    /* for PMECC supported chips */
> +    nand1@40000000 {
> +            compatible = "atmel,at91rm9200-nand";
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            reg = <0x40000000 0x10000000	/* bus addr & size */
> +                   0xffffe000 0x00000600	/* PMECC addr & size */
> +                   0xffffe600 0x00000200	/* PMECC ERRLOC addr & size */
> +                   0x00100000 0x00100000>;	/* ROM addr & size */
> +
> +            atmel,nand-addr-offset = <21>;	/* ale */
> +            atmel,nand-cmd-offset = <22>;	/* cle */
> +            nand-on-flash-bbt;
> +            nand-ecc-mode = "hw";
> +            atmel,has-pmecc;	/* enable PMECC */
> +            atmel,pmecc-cap = <2>;
> +            atmel,pmecc-sector-size = <512>;
> +            atmel,pmecc-lookup-table-offset = <0x8000 0x10000>;
> +            gpios = <&pioD 5 0	/* rdy */
> +                     &pioD 4 0	/* nce */
> +                     0		/* cd */
> +                    >;
> +    };

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]