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

Convert the text bindings to yaml.

Note:

This patch is done on top of the commit I sent recently.
https://lore.kernel.org/all/[email protected]/
Please let me know if I should resend the above commit in this series.

Signed-off-by: Balamanikandan Gunasundar <[email protected]>
---
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 <[email protected]>



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

Convert text to yaml for atmel nand controller

Signed-off-by: Balamanikandan Gunasundar <[email protected]>
---
.../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 <[email protected]>
+
+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 <[email protected]>
L: [email protected]
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


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

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

Signed-off-by: Balamanikandan Gunasundar <[email protected]>
---
.../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 <[email protected]>
+
+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


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

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 <[email protected]>
---
.../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 <[email protected]>
+
+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


2024-03-20 16:26:12

by Conor Dooley

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

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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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 */
> + >;
> + };


Attachments:
(No filename) (7.78 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-20 16:35:48

by Conor Dooley

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

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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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 <[email protected]>
> L: [email protected]
> 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
>


Attachments:
(No filename) (7.48 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-20 16:40:23

by Conor Dooley

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

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 <[email protected]>
> 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 <[email protected]>
> +
> +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
>


Attachments:
(No filename) (2.60 kB)
signature.asc (235.00 B)
Download all attachments
Subject: Re: [PATCH 1/3] dt-bindings: mtd: atmel-nand: convert txt to yaml

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 <[email protected]>
> Date:
> 20/03/24, 10:05 pm
>
> To:
> Balamanikandan Gunasundar <[email protected]>
> CC:
> Miquel Raynal <[email protected]>, Richard Weinberger
> <[email protected]>, Vignesh Raghavendra <[email protected]>, Rob Herring
> <[email protected]>, Krzysztof Kozlowski
> <[email protected]>, Conor Dooley <[email protected]>,
> Nicolas Ferre <[email protected]>, Alexandre Belloni
> <[email protected]>, Claudiu Beznea
> <[email protected]>, [email protected],
> [email protected], [email protected],
> [email protected]
>
>
> 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<[email protected]>
>> ---
>> .../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<[email protected]>
>> +
>> +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<[email protected]>
>> L: [email protected]
>> 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
>>

2024-03-22 07:31:48

by Conor Dooley

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

On Fri, Mar 22, 2024 at 04:27:29AM +0000, [email protected] 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.


Attachments:
(No filename) (2.26 kB)
signature.asc (235.00 B)
Download all attachments
Subject: Re: [PATCH 1/3] dt-bindings: mtd: atmel-nand: convert txt to yaml

On 22/03/24 1:00 pm, Conor Dooley wrote:
> On Fri, Mar 22, 2024 at 04:27:29AM +0000, [email protected] 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.