[PATCH v4 0/2] Add support for "on-die" ECC on Davinci.

[PATCH v4 0/2] Add support for "on-die" ECC on Davinci.

[Marcus Folkesson]

Some chips, e.g. Micron MT29F1G08ABBFAH4, has a mandatory on-die ECC.
Add "on-die" as ECC engine type in order to be compatible with those.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
Changes in v4:
- Silent errors in `make  dt_binding_check  DT_SCHEMA_FILES=ti,davinci-nand.yaml`
- Link to v3: https://lore.kernel.org/r/20241005-ondie-v3-0-459d9c32225c@gmail.com

Changes in v3:
- Fix formatting issues in yaml file
- Link to v2: https://lore.kernel.org/r/20241002-ondie-v2-0-318156d8c7b4@gmail.com

Changes in v2:
- Convert dt-bindings file to yaml
- Link to v1: https://lore.kernel.org/r/20241001-ondie-v1-0-a3daae15c89d@gmail.com

---
Marcus Folkesson (2):
      mtd: nand: davinci: add support for on-die ECC engine type
      dt-bindings: mtd: davinci: convert to yaml

 .../devicetree/bindings/mtd/davinci-nand.txt       |  94 -----------------
 .../devicetree/bindings/mtd/ti,davinci-nand.yaml   | 115 +++++++++++++++++++++
 drivers/mtd/nand/raw/davinci_nand.c                |   5 +-
 3 files changed, 119 insertions(+), 95 deletions(-)
---
base-commit: 200289db261f0c8131a5756133e9d30966289c3b
change-id: 20241001-ondie-a0edcf28f846

Best regards,
-- 
Marcus Folkesson <marcus.folkesson@gmail.com>

[PATCH v4 1/2] mtd: nand: davinci: add support for on-die ECC engine type

[Marcus Folkesson]

Some chips, e.g. Micron MT29F1G08ABBFAH4, has a mandatory on-die ECC.
Add "on-die" as ECC engine type in order to be compatible with those.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/mtd/nand/raw/davinci_nand.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
index 392678143a36b20b42c1827eee8203dc2e41889a..79e768d337ae12f6e8d7f21f1acd4e259f4f3020 100644
--- a/drivers/mtd/nand/raw/davinci_nand.c
+++ b/drivers/mtd/nand/raw/davinci_nand.c
@@ -66,6 +66,7 @@ struct davinci_nand_pdata {
 
 	/* none  == NAND_ECC_ENGINE_TYPE_NONE (strongly *not* advised!!)
 	 * soft  == NAND_ECC_ENGINE_TYPE_SOFT
+	 * on-die == NAND_ECC_ENGINE_TYPE_ON_DIE
 	 * else  == NAND_ECC_ENGINE_TYPE_ON_HOST, according to ecc_bits
 	 *
 	 * All DaVinci-family chips support 1-bit hardware ECC.
@@ -524,6 +525,8 @@ static struct davinci_nand_pdata
 				pdata->engine_type = NAND_ECC_ENGINE_TYPE_SOFT;
 			if (!strncmp("hw", mode, 2))
 				pdata->engine_type = NAND_ECC_ENGINE_TYPE_ON_HOST;
+			if (!strncmp("on-die", mode, 6))
+				pdata->engine_type = NAND_ECC_ENGINE_TYPE_ON_DIE;
 		}
 		if (!of_property_read_u32(pdev->dev.of_node,
 			"ti,davinci-ecc-bits", &prop))
@@ -580,6 +583,7 @@ static int davinci_nand_attach_chip(struct nand_chip *chip)
 
 	switch (chip->ecc.engine_type) {
 	case NAND_ECC_ENGINE_TYPE_NONE:
+	case NAND_ECC_ENGINE_TYPE_ON_DIE:
 		pdata->ecc_bits = 0;
 		break;
 	case NAND_ECC_ENGINE_TYPE_SOFT:
@@ -914,4 +918,3 @@ module_platform_driver(nand_davinci_driver);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Texas Instruments");
 MODULE_DESCRIPTION("Davinci NAND flash driver");
-

-- 
2.46.0

[PATCH v4 2/2] dt-bindings: mtd: davinci: convert to yaml

[Marcus Folkesson]

Convert the bindings to yaml format.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 .../devicetree/bindings/mtd/davinci-nand.txt       |  94 -----------------
 .../devicetree/bindings/mtd/ti,davinci-nand.yaml   | 115 +++++++++++++++++++++
 2 files changed, 115 insertions(+), 94 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
deleted file mode 100644
index eb8e2ff4dbd2901b3c396f2e66c1f590a32dcf67..0000000000000000000000000000000000000000
--- a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
+++ /dev/null
@@ -1,94 +0,0 @@
-Device tree bindings for Texas instruments Davinci/Keystone NAND controller
-
-This file provides information, what the device node for the davinci/keystone
-NAND interface contains.
-
-Documentation:
-Davinci DM646x - https://www.ti.com/lit/ug/sprueq7c/sprueq7c.pdf
-Kestone - https://www.ti.com/lit/ug/sprugz3a/sprugz3a.pdf
-
-Required properties:
-
-- compatible:			"ti,davinci-nand"
-				"ti,keystone-nand"
-
-- reg:				Contains 2 offset/length values:
-				- offset and length for the access window.
-				- offset and length for accessing the AEMIF
-				control registers.
-
-- ti,davinci-chipselect:	number of chipselect. Indicates on the
-				davinci_nand driver which chipselect is used
-				for accessing the nand.
-				Can be in the range [0-3].
-
-Recommended properties :
-
-- ti,davinci-mask-ale:		mask for ALE. Needed for executing address
-				phase. These offset will be added to the base
-				address for the chip select space the NAND Flash
-				device is connected to.
-				If not set equal to 0x08.
-
-- ti,davinci-mask-cle:		mask for CLE. Needed for executing command
-				phase. These offset will be added to the base
-				address for the chip select space the NAND Flash
-				device is connected to.
-				If not set equal to 0x10.
-
-- ti,davinci-mask-chipsel:	mask for chipselect address. Needed to mask
-				addresses for given chipselect.
-
-- nand-ecc-mode:		operation mode of the NAND ecc mode. ECC mode
-				valid values for davinci driver:
-				- "none"
-				- "soft"
-				- "hw"
-
-- ti,davinci-ecc-bits:		used ECC bits, currently supported 1 or 4.
-
-- nand-bus-width:		buswidth 8 or 16. If not present 8.
-
-- nand-on-flash-bbt:		use flash based bad block table support. OOB
-				identifier is saved in OOB area. If not present
-				false.
-
-Deprecated properties:
-
-- ti,davinci-ecc-mode:		operation mode of the NAND ecc mode. ECC mode
-				valid values for davinci driver:
-				- "none"
-				- "soft"
-				- "hw"
-
-- ti,davinci-nand-buswidth:	buswidth 8 or 16. If not present 8.
-
-- ti,davinci-nand-use-bbt:	use flash based bad block table support. OOB
-				identifier is saved in OOB area. If not present
-				false.
-
-Nand device bindings may contain additional sub-nodes describing partitions of
-the address space. See mtd.yaml for more detail. The NAND Flash timing
-values must be programmed in the chip select’s node of AEMIF
-memory-controller (see Documentation/devicetree/bindings/memory-controllers/
-davinci-aemif.txt).
-
-Example(da850 EVM ):
-
-nand_cs3@62000000 {
-	compatible = "ti,davinci-nand";
-	reg = <0x62000000 0x807ff
-	       0x68000000 0x8000>;
-	ti,davinci-chipselect = <1>;
-	ti,davinci-mask-ale = <0>;
-	ti,davinci-mask-cle = <0>;
-	ti,davinci-mask-chipsel = <0>;
-	nand-ecc-mode = "hw";
-	ti,davinci-ecc-bits = <4>;
-	nand-on-flash-bbt;
-
-	partition@180000 {
-		label = "ubifs";
-		reg = <0x180000 0x7e80000>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/mtd/ti,davinci-nand.yaml b/Documentation/devicetree/bindings/mtd/ti,davinci-nand.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..d87fe7136d64ab15fcd7fc824f7888d6643f1b91
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/ti,davinci-nand.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/ti,davinci-nand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI DaVinci NAND controller
+
+maintainers:
+  - Marcus Folkesson <marcus.folkesson@gmail.com>
+
+allOf:
+  - $ref: nand-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - ti,davinci-nand
+      - ti,keystone-nand
+
+  reg:
+    maxItems: 1
+
+  partitions:
+    $ref: /schemas/mtd/partitions/partitions.yaml
+
+  ti,davinci-chipselect:
+    description:
+      Number of chipselect. Indicate on the davinci_nand driver which
+      chipselect is used for accessing the nand.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2, 3]
+
+  ti,davinci-mask-ale:
+    description:
+      Mask for ALE. Needed for executing address phase. These offset will be
+      added to the base address for the chip select space the NAND Flash
+      device is connected to.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 0x08
+
+  ti,davinci-mask-cle:
+    description:
+      Mask for CLE. Needed for executing command phase. These offset will be
+      added to the base address for the chip select space the NAND Flash device
+      is connected to.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 0x10
+
+  ti,davinci-mask-chipsel:
+    description:
+      Mask for chipselect address. Needed to mask addresses for given
+      chipselect.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 0
+
+  ti,davinci-ecc-bits:
+    description: Used ECC bits.
+    enum: [1, 4]
+
+  ti,davinci-ecc-mode:
+    description: Operation mode of the NAND ECC mode.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [none, soft, hw, on-die]
+    deprecated: true
+
+  ti,davinci-nand-buswidth:
+    description: Bus width to the NAND chip
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [8, 16]
+    default: 8
+    deprecated: true
+
+  ti,davinci-nand-use-bbt:
+    type: boolean
+    description:
+      Use flash based bad block table support. OOB identifier is saved in OOB
+      area.
+    deprecated: true
+
+required:
+  - compatible
+  - reg
+  - ti,davinci-chipselect
+
+additionalProperties: true
+
+examples:
+  - |
+    nand-controller@2000000 {
+      compatible = "ti,davinci-nand";
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      reg = <0 0x02000000>;
+      ti,davinci-chipselect = <1>;
+      ti,davinci-mask-ale = <0>;
+      ti,davinci-mask-cle = <0>;
+      ti,davinci-mask-chipsel = <0>;
+      ti,davinci-nand-buswidth = <16>;
+      ti,davinci-ecc-mode = "hw";
+      ti,davinci-ecc-bits = <4>;
+      ti,davinci-nand-use-bbt;
+
+      partitions {
+        compatible = "fixed-partitions";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        partition@0 {
+          label = "u-boot env";
+          reg = <0 0x020000>;
+        };
+      };
+    };

-- 
2.46.0

Re: [PATCH v4 2/2] dt-bindings: mtd: davinci: convert to yaml

[Krzysztof Kozlowski]

On Sun, Oct 06, 2024 at 03:05:47PM +0200, Marcus Folkesson wrote:
> +  ti,davinci-nand-use-bbt:
> +    type: boolean
> +    description:
> +      Use flash based bad block table support. OOB identifier is saved in OOB
> +      area.
> +    deprecated: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - ti,davinci-chipselect
> +
> +additionalProperties: true

That's a final device schema, this cannot be true. Why you are doing
this entirely different than all other bindings?

You wrote in changelog, that you "silent the errors". Sorry, but that's
a no go. You are not suppose to silent them, but FIX THEM.


> +
> +examples:
> +  - |
> +    nand-controller@2000000 {
> +      compatible = "ti,davinci-nand";
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      reg = <0 0x02000000>;

Keep order like in DTS coding style.

Best regards,
Krzysztof

Re: [PATCH v4 2/2] dt-bindings: mtd: davinci: convert to yaml

[Marcus Folkesson]

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

On Mon, Oct 07, 2024 at 08:07:27AM +0200, Krzysztof Kozlowski wrote:
> On Sun, Oct 06, 2024 at 03:05:47PM +0200, Marcus Folkesson wrote:
> > +  ti,davinci-nand-use-bbt:
> > +    type: boolean
> > +    description:
> > +      Use flash based bad block table support. OOB identifier is saved in OOB
> > +      area.
> > +    deprecated: true
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - ti,davinci-chipselect
> > +
> > +additionalProperties: true
> 
> That's a final device schema, this cannot be true. Why you are doing
> this entirely different than all other bindings?

From my understanding, additionalProperties is to indicate that the
schema could contain properties that were not explicit listed here but
inherited from e.g. nand-controller.yaml.

Obviously that is not the case.
Could you please give me some guidance?

> 
> You wrote in changelog, that you "silent the errors". Sorry, but that's
> a no go. You are not suppose to silent them, but FIX THEM.
> 
> 
> > +
> > +examples:
> > +  - |
> > +    nand-controller@2000000 {
> > +      compatible = "ti,davinci-nand";
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +
> > +      reg = <0 0x02000000>;
> 
> Keep order like in DTS coding style.
> 
> Best regards,
> Krzysztof
> 

Best regards,
Marcus Folkesson

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

Re: [PATCH v4 2/2] dt-bindings: mtd: davinci: convert to yaml

[Krzysztof Kozlowski]

On 07/10/2024 08:29, Marcus Folkesson wrote:
> On Mon, Oct 07, 2024 at 08:07:27AM +0200, Krzysztof Kozlowski wrote:
>> On Sun, Oct 06, 2024 at 03:05:47PM +0200, Marcus Folkesson wrote:
>>> +  ti,davinci-nand-use-bbt:
>>> +    type: boolean
>>> +    description:
>>> +      Use flash based bad block table support. OOB identifier is saved in OOB
>>> +      area.
>>> +    deprecated: true
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - ti,davinci-chipselect
>>> +
>>> +additionalProperties: true
>>
>> That's a final device schema, this cannot be true. Why you are doing
>> this entirely different than all other bindings?
> 
> From my understanding, additionalProperties is to indicate that the
> schema could contain properties that were not explicit listed here but
> inherited from e.g. nand-controller.yaml.
> 
> Obviously that is not the case.
> Could you please give me some guidance?

I suggested - open any other binding. What is there? What was the exact
error automation reported to you *twice*?



Best regards,
Krzysztof

Re: [PATCH v4 2/2] dt-bindings: mtd: davinci: convert to yaml

[Miquel Raynal]

Hi Marcus,

marcus.folkesson@gmail.com wrote on Mon, 7 Oct 2024 08:29:44 +0200:

> On Mon, Oct 07, 2024 at 08:07:27AM +0200, Krzysztof Kozlowski wrote:
> > On Sun, Oct 06, 2024 at 03:05:47PM +0200, Marcus Folkesson wrote:  
> > > +  ti,davinci-nand-use-bbt:
> > > +    type: boolean
> > > +    description:
> > > +      Use flash based bad block table support. OOB identifier is saved in OOB
> > > +      area.
> > > +    deprecated: true
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - ti,davinci-chipselect
> > > +
> > > +additionalProperties: true  
> > 
> > That's a final device schema, this cannot be true. Why you are doing
> > this entirely different than all other bindings?  
> 
> From my understanding, additionalProperties is to indicate that the
> schema could contain properties that were not explicit listed here but
> inherited from e.g. nand-controller.yaml.

additionalProperties: true means there is no control over the
properties allowed, so we generally don't want that (unless you're
writing specific "generic" bindings, which is not the case here).

You are describing a controller so here you should reference to
nand-controller.yaml, but then prevent any property not defined in
nand-controller.yaml from being used. In this case you probably want to
try unevaluatedProperties: false, which means that any property not
listed here or in the referenced schemas will be prohibited.

Thanks,
Miquèl