[PATCH 0/6] Fix polarity and bindings for GPIO-based NAND drivers

[PATCH 0/6] Fix polarity and bindings for GPIO-based NAND drivers

[Linus Walleij]

The AMD Delta and generic GPIO-based NAND drivers are using GPIO lines
extensively to communicate with a raw NAND flash.

Some confusion has crept into the naming leading to the two drivers using
inversed semantics differently for pins with the same name.

Fix the situation by naming the pins consistently without any inversion
names (such as nce for a negative active chip enable).

Fix up all in-tree users.

Next rewrite the device tree bindings in YAML schema, and fix up the
single in-tree DTS file (MIPS) to use the new bindings where each signal
is specified explicitly instead of an array with some "blanks" for unused
lines.

Last clean up the GPIO NAND driver to drop use of board file provided
data as no boards using this remain, and use device properties removing
the explicit reliance on device tree.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Linus Walleij (6):
      mtd: rawnand: ams-delta/gpio: Unify polarity
      dt-bindings: mtd: Rewrite gpio-control-nand in schema
      MIPS: NI 169445: Fix NAND GPIOs
      mtd: rawnand: gpio: Use device properties
      mtd: rawnand: gpio: Support standard nand width
      mtd: rawnand: gpio: Rename file

 .../devicetree/bindings/mtd/gpio-control-nand.txt  |  47 ------
 .../devicetree/bindings/mtd/gpio-control-nand.yaml | 168 +++++++++++++++++++++
 Documentation/devicetree/bindings/mtd/mtd.yaml     |   2 +-
 arch/arm/mach-omap1/board-ams-delta.c              |   8 +-
 arch/mips/boot/dts/ni/169445.dts                   |  13 +-
 drivers/mtd/nand/raw/Makefile                      |   2 +-
 drivers/mtd/nand/raw/ams-delta.c                   |  60 ++++----
 drivers/mtd/nand/raw/{gpio.c => nand-gpio.c}       | 120 +++++----------
 8 files changed, 251 insertions(+), 169 deletions(-)
---
base-commit: be3ca57cfb777ad820c6659d52e60bbdd36bf5ff
change-id: 20231105-fix-mips-nand-c91ebd80fa4f

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>

[PATCH 1/6] mtd: rawnand: ams-delta/gpio: Unify polarity

[Linus Walleij]

The AMD Delta and GPIO RAW NAND drivers share the same platform
data file and pass GPIO descriptors for the same type of signals
from a board file.

Fix the following problems:

- NCE (negative active chip enable) should be just CE (chip enable)
  and flagged as active low. Rename it in both drivers to just "CE".

- NWP (negative active write protect) should be just WP (write protect)
  and flagged as active low. Rename it in both drivers to just "WP".

- NRE (negative active read enable) should be just RE (read enable)
  and flagged as active low. Rename it in the AMD Delta driver to "RE".
  The GPIO driver does not have this.

- NWE (negative active write enable) should be just WE (write enable)
  and flagged as active low. Rename it in the AMD Delta driver to "WE".
  The GPIO driver does not have this.

- The generic GPIO NAND driver is not expecting the GPIO polarity on
  CE and WP to be correct and will instead invert the polarity in
  the usage of the lines (such as setting the CE GPIO descriptor to
  0 to activate the chip enable). Fix this by altering the semantics
  in the generic GPIO driver to assume it is flagged active low
  properly where the GPIO line is defined.

- Fix up the arch/arm/mach-omap1/board-ams-delta.c to use the
  non-prefixed line names. (The polarity is right in this board.)

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-omap1/board-ams-delta.c |  8 ++---
 drivers/mtd/nand/raw/ams-delta.c      | 60 +++++++++++++++++------------------
 drivers/mtd/nand/raw/gpio.c           | 40 +++++++++++------------
 3 files changed, 54 insertions(+), 54 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 0daf6c5b5c1c..3a6ab4e27e3e 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -336,13 +336,13 @@ static struct gpiod_lookup_table ams_delta_nand_gpio_table = {
 	.table = {
 		GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_NAND_RB, "rdy",
 			    0),
-		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NCE, "nce",
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NCE, "ce",
 			    GPIO_ACTIVE_LOW),
-		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NRE, "nre",
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NRE, "re",
 			    GPIO_ACTIVE_LOW),
-		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NWP, "nwp",
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NWP, "wp",
 			    GPIO_ACTIVE_LOW),
-		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NWE, "nwe",
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NWE, "we",
 			    GPIO_ACTIVE_LOW),
 		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_ALE, "ale", 0),
 		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_CLE, "cle", 0),
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 919816a7aca7..ab3c8d3da41d 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -33,10 +33,10 @@ struct gpio_nand {
 	struct nand_controller	base;
 	struct nand_chip	nand_chip;
 	struct gpio_desc	*gpiod_rdy;
-	struct gpio_desc	*gpiod_nce;
-	struct gpio_desc	*gpiod_nre;
-	struct gpio_desc	*gpiod_nwp;
-	struct gpio_desc	*gpiod_nwe;
+	struct gpio_desc	*gpiod_ce;
+	struct gpio_desc	*gpiod_re;
+	struct gpio_desc	*gpiod_wp;
+	struct gpio_desc	*gpiod_we;
 	struct gpio_desc	*gpiod_ale;
 	struct gpio_desc	*gpiod_cle;
 	struct gpio_descs	*data_gpiods;
@@ -49,9 +49,9 @@ struct gpio_nand {
 
 static void gpio_nand_write_commit(struct gpio_nand *priv)
 {
-	gpiod_set_value(priv->gpiod_nwe, 1);
+	gpiod_set_value(priv->gpiod_we, 1);
 	ndelay(priv->tWP);
-	gpiod_set_value(priv->gpiod_nwe, 0);
+	gpiod_set_value(priv->gpiod_we, 0);
 }
 
 static void gpio_nand_io_write(struct gpio_nand *priv, u8 byte)
@@ -86,13 +86,13 @@ static u8 gpio_nand_io_read(struct gpio_nand *priv)
 	struct gpio_descs *data_gpiods = priv->data_gpiods;
 	DECLARE_BITMAP(values, BITS_PER_TYPE(res)) = { 0, };
 
-	gpiod_set_value(priv->gpiod_nre, 1);
+	gpiod_set_value(priv->gpiod_re, 1);
 	ndelay(priv->tRP);
 
 	gpiod_get_raw_array_value(data_gpiods->ndescs, data_gpiods->desc,
 				  data_gpiods->info, values);
 
-	gpiod_set_value(priv->gpiod_nre, 0);
+	gpiod_set_value(priv->gpiod_re, 0);
 
 	res = values[0];
 	return res;
@@ -133,7 +133,7 @@ static void gpio_nand_read_buf(struct gpio_nand *priv, u8 *buf, int len)
 
 static void gpio_nand_ctrl_cs(struct gpio_nand *priv, bool assert)
 {
-	gpiod_set_value(priv->gpiod_nce, assert);
+	gpiod_set_value(priv->gpiod_ce, assert);
 }
 
 static int gpio_nand_exec_op(struct nand_chip *this,
@@ -204,7 +204,7 @@ static int gpio_nand_setup_interface(struct nand_chip *this, int csline,
 	if (csline == NAND_DATA_IFACE_CHECK_ONLY)
 		return 0;
 
-	if (priv->gpiod_nre) {
+	if (priv->gpiod_re) {
 		priv->tRP = DIV_ROUND_UP(sdr->tRP_min, 1000);
 		dev_dbg(dev, "using %u ns read pulse width\n", priv->tRP);
 	}
@@ -273,35 +273,35 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, priv);
 
 	/* Set chip enabled but write protected */
-	priv->gpiod_nwp = devm_gpiod_get_optional(&pdev->dev, "nwp",
+	priv->gpiod_wp = devm_gpiod_get_optional(&pdev->dev, "wp",
 						  GPIOD_OUT_HIGH);
-	if (IS_ERR(priv->gpiod_nwp)) {
-		err = PTR_ERR(priv->gpiod_nwp);
-		dev_err(&pdev->dev, "NWP GPIO request failed (%d)\n", err);
+	if (IS_ERR(priv->gpiod_wp)) {
+		err = PTR_ERR(priv->gpiod_wp);
+		dev_err(&pdev->dev, "WP GPIO request failed (%d)\n", err);
 		return err;
 	}
 
-	priv->gpiod_nce = devm_gpiod_get_optional(&pdev->dev, "nce",
+	priv->gpiod_ce = devm_gpiod_get_optional(&pdev->dev, "ce",
 						  GPIOD_OUT_LOW);
-	if (IS_ERR(priv->gpiod_nce)) {
-		err = PTR_ERR(priv->gpiod_nce);
-		dev_err(&pdev->dev, "NCE GPIO request failed (%d)\n", err);
+	if (IS_ERR(priv->gpiod_ce)) {
+		err = PTR_ERR(priv->gpiod_ce);
+		dev_err(&pdev->dev, "CE GPIO request failed (%d)\n", err);
 		return err;
 	}
 
-	priv->gpiod_nre = devm_gpiod_get_optional(&pdev->dev, "nre",
+	priv->gpiod_re = devm_gpiod_get_optional(&pdev->dev, "re",
 						  GPIOD_OUT_LOW);
-	if (IS_ERR(priv->gpiod_nre)) {
-		err = PTR_ERR(priv->gpiod_nre);
-		dev_err(&pdev->dev, "NRE GPIO request failed (%d)\n", err);
+	if (IS_ERR(priv->gpiod_re)) {
+		err = PTR_ERR(priv->gpiod_re);
+		dev_err(&pdev->dev, "RE GPIO request failed (%d)\n", err);
 		return err;
 	}
 
-	priv->gpiod_nwe = devm_gpiod_get_optional(&pdev->dev, "nwe",
+	priv->gpiod_we = devm_gpiod_get_optional(&pdev->dev, "we",
 						  GPIOD_OUT_LOW);
-	if (IS_ERR(priv->gpiod_nwe)) {
-		err = PTR_ERR(priv->gpiod_nwe);
-		dev_err(&pdev->dev, "NWE GPIO request failed (%d)\n", err);
+	if (IS_ERR(priv->gpiod_we)) {
+		err = PTR_ERR(priv->gpiod_we);
+		dev_err(&pdev->dev, "WE GPIO request failed (%d)\n", err);
 		return err;
 	}
 
@@ -328,9 +328,9 @@ static int gpio_nand_probe(struct platform_device *pdev)
 		return err;
 	}
 	if (priv->data_gpiods) {
-		if (!priv->gpiod_nwe) {
+		if (!priv->gpiod_we) {
 			dev_err(&pdev->dev,
-				"mandatory NWE pin not provided by platform\n");
+				"mandatory WE pin not provided by platform\n");
 			return -ENODEV;
 		}
 
@@ -367,7 +367,7 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	 * chip detection/initialization.
 	 */
 	/* Release write protection */
-	gpiod_set_value(priv->gpiod_nwp, 0);
+	gpiod_set_value(priv->gpiod_wp, 0);
 
 	/*
 	 * This driver assumes that the default ECC engine should be TYPE_SOFT.
@@ -404,7 +404,7 @@ static void gpio_nand_remove(struct platform_device *pdev)
 	int ret;
 
 	/* Apply write protection */
-	gpiod_set_value(priv->gpiod_nwp, 1);
+	gpiod_set_value(priv->gpiod_wp, 1);
 
 	/* Unregister device */
 	ret = mtd_device_unregister(mtd);
diff --git a/drivers/mtd/nand/raw/gpio.c b/drivers/mtd/nand/raw/gpio.c
index d6cc2cb65214..df6facf0ec9a 100644
--- a/drivers/mtd/nand/raw/gpio.c
+++ b/drivers/mtd/nand/raw/gpio.c
@@ -33,11 +33,11 @@ struct gpiomtd {
 	void __iomem		*io_sync;
 	struct nand_chip	nand_chip;
 	struct gpio_nand_platdata plat;
-	struct gpio_desc *nce; /* Optional chip enable */
+	struct gpio_desc *ce; /* Optional chip enable */
 	struct gpio_desc *cle;
 	struct gpio_desc *ale;
 	struct gpio_desc *rdy;
-	struct gpio_desc *nwp; /* Optional write protection */
+	struct gpio_desc *wp; /* Optional write protection */
 };
 
 static inline struct gpiomtd *gpio_nand_getpriv(struct mtd_info *mtd)
@@ -146,7 +146,7 @@ static int gpio_nand_exec_op(struct nand_chip *chip,
 		return 0;
 
 	gpio_nand_dosync(gpiomtd);
-	gpiod_set_value(gpiomtd->nce, 0);
+	gpiod_set_value(gpiomtd->ce, 1);
 	for (i = 0; i < op->ninstrs; i++) {
 		ret = gpio_nand_exec_instr(chip, &op->instrs[i]);
 		if (ret)
@@ -156,7 +156,7 @@ static int gpio_nand_exec_op(struct nand_chip *chip,
 			ndelay(op->instrs[i].delay_ns);
 	}
 	gpio_nand_dosync(gpiomtd);
-	gpiod_set_value(gpiomtd->nce, 1);
+	gpiod_set_value(gpiomtd->ce, 0);
 
 	return ret;
 }
@@ -276,10 +276,10 @@ static void gpio_nand_remove(struct platform_device *pdev)
 	nand_cleanup(chip);
 
 	/* Enable write protection and disable the chip */
-	if (gpiomtd->nwp && !IS_ERR(gpiomtd->nwp))
-		gpiod_set_value(gpiomtd->nwp, 0);
-	if (gpiomtd->nce && !IS_ERR(gpiomtd->nce))
-		gpiod_set_value(gpiomtd->nce, 0);
+	if (gpiomtd->wp && !IS_ERR(gpiomtd->wp))
+		gpiod_set_value(gpiomtd->wp, 1);
+	if (gpiomtd->ce && !IS_ERR(gpiomtd->ce))
+		gpiod_set_value(gpiomtd->ce, 0);
 }
 
 static int gpio_nand_probe(struct platform_device *pdev)
@@ -316,14 +316,14 @@ static int gpio_nand_probe(struct platform_device *pdev)
 		return ret;
 
 	/* Just enable the chip */
-	gpiomtd->nce = devm_gpiod_get_optional(dev, "nce", GPIOD_OUT_HIGH);
-	if (IS_ERR(gpiomtd->nce))
-		return PTR_ERR(gpiomtd->nce);
+	gpiomtd->ce = devm_gpiod_get_optional(dev, "ce", GPIOD_OUT_HIGH);
+	if (IS_ERR(gpiomtd->ce))
+		return PTR_ERR(gpiomtd->ce);
 
 	/* We disable write protection once we know probe() will succeed */
-	gpiomtd->nwp = devm_gpiod_get_optional(dev, "nwp", GPIOD_OUT_LOW);
-	if (IS_ERR(gpiomtd->nwp)) {
-		ret = PTR_ERR(gpiomtd->nwp);
+	gpiomtd->wp = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_HIGH);
+	if (IS_ERR(gpiomtd->wp)) {
+		ret = PTR_ERR(gpiomtd->wp);
 		goto out_ce;
 	}
 
@@ -358,8 +358,8 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, gpiomtd);
 
 	/* Disable write protection, if wired up */
-	if (gpiomtd->nwp && !IS_ERR(gpiomtd->nwp))
-		gpiod_direction_output(gpiomtd->nwp, 1);
+	if (gpiomtd->wp && !IS_ERR(gpiomtd->wp))
+		gpiod_direction_output(gpiomtd->wp, 0);
 
 	/*
 	 * This driver assumes that the default ECC engine should be TYPE_SOFT.
@@ -381,11 +381,11 @@ static int gpio_nand_probe(struct platform_device *pdev)
 		return 0;
 
 err_wp:
-	if (gpiomtd->nwp && !IS_ERR(gpiomtd->nwp))
-		gpiod_set_value(gpiomtd->nwp, 0);
+	if (gpiomtd->wp && !IS_ERR(gpiomtd->wp))
+		gpiod_set_value(gpiomtd->wp, 1);
 out_ce:
-	if (gpiomtd->nce && !IS_ERR(gpiomtd->nce))
-		gpiod_set_value(gpiomtd->nce, 0);
+	if (gpiomtd->ce && !IS_ERR(gpiomtd->ce))
+		gpiod_set_value(gpiomtd->ce, 0);
 
 	return ret;
 }

-- 
2.34.1

[PATCH 2/6] dt-bindings: mtd: Rewrite gpio-control-nand in schema

[Linus Walleij]

This creates a schema for GPIO controlled NAND. The txt
schema was old and wrong.

Mark the old way of passing GPIOs in a long array as
deprecated and encourage per-pin GPIO assignments with
the named nnn-gpios phandles.

I was unable to re-use raw-nand-chip.yaml or even
nand-chip.yaml: the reason is that they both assume
that we have potentially several NAND chips with chip
selects and thus enforce a node name "nand@0" etc,
which doesn't quite work for this device.

Since the GPIO controlled NAND is both a NAND controller
and a NAND chip jitted together, I have to modify the
mtd.yaml nodename requirement to include nand-controller@
as this is the nodename that this device should use.

Deprecate the custom "band-width" property in favor of
"nand-bus-width".

Reported-by: Howard Harte <hharte@magicandroidapps.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Check the required section especially. Since there is no
hardware default for anything when using GPIOs for this,
I think all these parameters are compulsory.
---
 .../devicetree/bindings/mtd/gpio-control-nand.txt  |  47 ------
 .../devicetree/bindings/mtd/gpio-control-nand.yaml | 168 +++++++++++++++++++++
 Documentation/devicetree/bindings/mtd/mtd.yaml     |   2 +-
 3 files changed, 169 insertions(+), 48 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
deleted file mode 100644
index 486a17d533d7..000000000000
--- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
+++ /dev/null
@@ -1,47 +0,0 @@
-GPIO assisted NAND flash
-
-The GPIO assisted NAND flash uses a memory mapped interface to
-read/write the NAND commands and data and GPIO pins for the control
-signals.
-
-Required properties:
-- compatible : "gpio-control-nand"
-- reg : should specify localbus chip select and size used for the chip.  The
-  resource describes the data bus connected to the NAND flash and all accesses
-  are made in native endianness.
-- #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.  The order of
-  GPIO references is:  RDY, nCE, ALE, CLE, and nWP. nCE and nWP are optional.
-
-Optional properties:
-- bank-width : Width (in bytes) of the device.  If not present, the width
-  defaults to 1 byte.
-- chip-delay : chip dependent delay for transferring data from array to
-  read registers (tR).  If not present then a default of 20us is used.
-- gpio-control-nand,io-sync-reg : A 64-bit physical address for a read
-  location used to guard against bus reordering with regards to accesses to
-  the GPIO's and the NAND flash data bus.  If present, then after changing
-  GPIO state and before and after command byte writes, this register will be
-  read to ensure that the GPIO accesses have completed.
-
-The device tree may optionally contain sub-nodes describing partitions of the
-address space. See partition.txt for more detail.
-
-Examples:
-
-gpio-nand@1,0 {
-	compatible = "gpio-control-nand";
-	reg = <1 0x0000 0x2>;
-	#address-cells = <1>;
-	#size-cells = <1>;
-	gpios = <&banka 1 0>,	/* RDY */
-		<0>, 		/* nCE */
-		<&banka 3 0>, 	/* ALE */
-		<&banka 4 0>, 	/* CLE */
-		<0>;		/* nWP */
-
-	partition@0 {
-	...
-	};
-};
diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.yaml b/Documentation/devicetree/bindings/mtd/gpio-control-nand.yaml
new file mode 100644
index 000000000000..5b30ee7ad4a5
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.yaml
@@ -0,0 +1,168 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/gpio-control-nand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NAND memory exclusively connected to GPIO lines
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+description: |
+  It is possible to connect a NAND flash memory without any
+  dedicated NAND controller hardware, using just general purpose
+  I/O (GPIO) pins. This will not be fast, but it will work.
+  The address and data lines of the chip will still need to be
+  connected so that the contents of a NAND page can be
+  memory-mapped and accessed after the special lines are toggled
+  by GPIO.
+
+# The GPIO controlled NAND has wires going directly to one single
+# NAND chip, so it is both a nand controller and a nand chip at
+# the same time, but it does not have things such as chip select
+# since the use is hammered down to one single chip only.
+# There is no point for the chip itself to be a subnode of the
+# controller so the raw NAND chip properties are added right into
+# the controller node like this.
+
+allOf:
+  - $ref: mtd.yaml#
+
+properties:
+  $nodename:
+    pattern: "^(nand|nand-controller)@[a-f0-9]+$"
+
+  compatible:
+    const: gpio-control-nand
+
+  reg:
+    description: |
+      This should specify the address where the NAND page currently
+      accessed gets memory-mapped, and the size of the page. Usually
+      this will be the same as the page size of the NAND.
+
+  label: true
+
+  partitions: true
+
+  nand-ecc-algo: true
+
+  nand-ecc-step-size: true
+
+  nand-ecc-strength: true
+
+  nand-use-soft-ecc-engine: true
+
+  gpio-control-nand,io-sync-reg:
+    description: |
+      A 64-bit physical address for a read location used to guard
+      against bus reordering with regards to accesses to the GPIOs and
+      the NAND flash data bus. If present, then after changing GPIO state
+      and before and after command byte writes, this register will be
+      read to ensure that the GPIO accesses have completed.
+    $ref: /schemas/types.yaml#/definitions/uint64
+
+  gpios:
+    description:
+      Legacy GPIO array for the NAND chip lines, order RDY,
+      NCE, ALE, CLE, NWP.
+    deprecated: true
+    maxItems: 5
+
+  rdy-gpios:
+    description:
+      GPIO for the NAND chip RDY line
+    maxItems: 1
+
+  ce-gpios:
+    description:
+      GPIO for the NAND chip CE chip enable line, usually
+      this is active low, so it should be tagged with the GPIO
+      flag GPIO_ACTIVE_LOW.
+    maxItems: 1
+
+  ale-gpios:
+    description:
+      GPIO for the NAND chip ALE line
+    maxItems: 1
+
+  cle-gpios:
+    description:
+      GPIO for the NAND chip CLE line
+    maxItems: 1
+
+  wp-gpios:
+    description:
+      GPIO for the NAND chip WP line, usually this is
+      active low, so it should be tagged with the GPIO
+      flag GPIO_ACTIVE_LOW.
+    maxItems: 1
+
+  bank-width:
+    description:
+      Width (in bytes) of the device.  If not present, the
+      width defaults to 1 byte. This is deprecated, use
+      nand-bus-width instead.
+    deprecated: true
+    enum: [ 1, 2 ]
+    default: 1
+
+  nand-bus-width:
+    description:
+      Bus width to the NAND chip
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [8, 16]
+    default: 8
+
+  chip-delay:
+    description:
+      chip dependent delay for transferring data from array to
+      read registers (tR). If not present then a default of 20us
+      is used.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - reg
+  - ale-gpios
+  - cle-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    nand@20200000 {
+      compatible = "gpio-control-nand";
+      /* 512 bytes memory window at 0x20200000 */
+      reg = <0x20200000 0x200>;
+      rdy-gpios = <&gpio0 7 GPIO_ACTIVE_HIGH>;
+      ce-gpios = <&gpio0 12 GPIO_ACTIVE_LOW>;
+      ale-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
+      cle-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
+      wp-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
+
+      label = "ixp400 NAND";
+
+      nand-use-soft-ecc-engine;
+      nand-ecc-algo = "bch";
+      nand-ecc-step-size = <512>;
+      nand-ecc-strength = <4>;
+
+      partitions {
+        compatible = "fixed-partitions";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        fs@0 {
+            label = "SysA Kernel";
+            reg = <0x0 0x400000>;
+        };
+
+        fs@400000 {
+            label = "SysA Code";
+            reg = <0x400000 0x7C00000>;
+        };
+      };
+    };
diff --git a/Documentation/devicetree/bindings/mtd/mtd.yaml b/Documentation/devicetree/bindings/mtd/mtd.yaml
index f322290ee516..e6fd82cbc35d 100644
--- a/Documentation/devicetree/bindings/mtd/mtd.yaml
+++ b/Documentation/devicetree/bindings/mtd/mtd.yaml
@@ -12,7 +12,7 @@ maintainers:
 
 properties:
   $nodename:
-    pattern: "^(flash|.*sram|nand)(@.*)?$"
+    pattern: "^(flash|.*sram|nand|nand-controller)(@.*)?$"
 
   label:
     description:

-- 
2.34.1

Re: [PATCH 2/6] dt-bindings: mtd: Rewrite gpio-control-nand in schema

[Miquel Raynal]

Hello Linus,

linus.walleij@linaro.org wrote on Wed, 08 Nov 2023 15:33:50 +0100:

> This creates a schema for GPIO controlled NAND. The txt
> schema was old and wrong.
> 
> Mark the old way of passing GPIOs in a long array as
> deprecated and encourage per-pin GPIO assignments with
> the named nnn-gpios phandles.
> 
> I was unable to re-use raw-nand-chip.yaml or even
> nand-chip.yaml: the reason is that they both assume
> that we have potentially several NAND chips with chip
> selects and thus enforce a node name "nand@0" etc,
> which doesn't quite work for this device.

But what about nand-controller.yaml? This driver is just about
emulating what a NAND controller would do with GPIOs, any NAND chip can
be wired, no?

> Since the GPIO controlled NAND is both a NAND controller
> and a NAND chip jitted together, 

Not really, it's just the controller part? I know for years
NAND controllers, ECC engines and NAND chips have been considered a
single hardware entity, but I believe this one is just about emulating
the host controller part.

> I have to modify the
> mtd.yaml nodename requirement to include nand-controller@
> as this is the nodename that this device should use.
> 
> Deprecate the custom "band-width" property in favor of
> "nand-bus-width".
> 
> Reported-by: Howard Harte <hharte@magicandroidapps.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Check the required section especially. Since there is no
> hardware default for anything when using GPIOs for this,
> I think all these parameters are compulsory.
> ---
>  .../devicetree/bindings/mtd/gpio-control-nand.txt  |  47 ------
>  .../devicetree/bindings/mtd/gpio-control-nand.yaml | 168 +++++++++++++++++++++
>  Documentation/devicetree/bindings/mtd/mtd.yaml     |   2 +-
>  3 files changed, 169 insertions(+), 48 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> deleted file mode 100644
> index 486a17d533d7..000000000000
> --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> +++ /dev/null
> @@ -1,47 +0,0 @@
> -GPIO assisted NAND flash
> -
> -The GPIO assisted NAND flash uses a memory mapped interface to
> -read/write the NAND commands and data and GPIO pins for the control
> -signals.
> -
> -Required properties:
> -- compatible : "gpio-control-nand"
> -- reg : should specify localbus chip select and size used for the chip.  The
> -  resource describes the data bus connected to the NAND flash and all accesses
> -  are made in native endianness.
> -- #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.  The order of
> -  GPIO references is:  RDY, nCE, ALE, CLE, and nWP. nCE and nWP are optional.
> -
> -Optional properties:
> -- bank-width : Width (in bytes) of the device.  If not present, the width
> -  defaults to 1 byte.
> -- chip-delay : chip dependent delay for transferring data from array to
> -  read registers (tR).  If not present then a default of 20us is used.
> -- gpio-control-nand,io-sync-reg : A 64-bit physical address for a read
> -  location used to guard against bus reordering with regards to accesses to
> -  the GPIO's and the NAND flash data bus.  If present, then after changing
> -  GPIO state and before and after command byte writes, this register will be
> -  read to ensure that the GPIO accesses have completed.
> -
> -The device tree may optionally contain sub-nodes describing partitions of the
> -address space. See partition.txt for more detail.
> -
> -Examples:
> -
> -gpio-nand@1,0 {
> -	compatible = "gpio-control-nand";
> -	reg = <1 0x0000 0x2>;
> -	#address-cells = <1>;
> -	#size-cells = <1>;
> -	gpios = <&banka 1 0>,	/* RDY */
> -		<0>, 		/* nCE */
> -		<&banka 3 0>, 	/* ALE */
> -		<&banka 4 0>, 	/* CLE */
> -		<0>;		/* nWP */
> -
> -	partition@0 {
> -	...
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.yaml b/Documentation/devicetree/bindings/mtd/gpio-control-nand.yaml
> new file mode 100644
> index 000000000000..5b30ee7ad4a5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.yaml
> @@ -0,0 +1,168 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/gpio-control-nand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NAND memory exclusively connected to GPIO lines
> +
> +maintainers:
> +  - Linus Walleij <linus.walleij@linaro.org>
> +
> +description: |
> +  It is possible to connect a NAND flash memory without any
> +  dedicated NAND controller hardware, using just general purpose
> +  I/O (GPIO) pins. This will not be fast, but it will work.
> +  The address and data lines of the chip will still need to be
> +  connected so that the contents of a NAND page can be
> +  memory-mapped and accessed after the special lines are toggled
> +  by GPIO.
> +
> +# The GPIO controlled NAND has wires going directly to one single
> +# NAND chip, so it is both a nand controller and a nand chip at
> +# the same time, but it does not have things such as chip select
> +# since the use is hammered down to one single chip only.
> +# There is no point for the chip itself to be a subnode of the
> +# controller so the raw NAND chip properties are added right into
> +# the controller node like this.

I kind of disagree here, this "piece of software" only replaces a NAND
controller. You always need a NAND chip in front of it, and that's a
specific piece of hardware anyway. Or maybe I don't understand the
hardware behind? (truly not impossible)

> +
> +allOf:
> +  - $ref: mtd.yaml#
> +
> +properties:
> +  $nodename:
> +    pattern: "^(nand|nand-controller)@[a-f0-9]+$"
> +
> +  compatible:
> +    const: gpio-control-nand
> +
> +  reg:
> +    description: |
> +      This should specify the address where the NAND page currently
> +      accessed gets memory-mapped, and the size of the page. Usually
> +      this will be the same as the page size of the NAND.

This is definitely a host controller parameter. Even if the hardware
only supports a single NAND chip, I believe it should be described as a
subnode with a dummy "reg = <0>;" property.

Thanks,
Miquèl

Re: [PATCH 2/6] dt-bindings: mtd: Rewrite gpio-control-nand in schema

[Rob Herring]

On Wed, Nov 08, 2023 at 03:33:50PM +0100, Linus Walleij wrote:
> This creates a schema for GPIO controlled NAND. The txt
> schema was old and wrong.
> 
> Mark the old way of passing GPIOs in a long array as
> deprecated and encourage per-pin GPIO assignments with
> the named nnn-gpios phandles.

We have 1 user upstream with only a single commit adding it in 2017. 
This doesn't seem like something that's going to gain new users either. 
Is it really worth modernizing this binding? 

Rob

Re: [PATCH 2/6] dt-bindings: mtd: Rewrite gpio-control-nand in schema

[Linus Walleij]

On Wed, Nov 8, 2023 at 8:11 PM Rob Herring <robh@kernel.org> wrote:
> On Wed, Nov 08, 2023 at 03:33:50PM +0100, Linus Walleij wrote:

> > This creates a schema for GPIO controlled NAND. The txt
> > schema was old and wrong.
> >
> > Mark the old way of passing GPIOs in a long array as
> > deprecated and encourage per-pin GPIO assignments with
> > the named nnn-gpios phandles.
>
> We have 1 user upstream with only a single commit adding it in 2017.
> This doesn't seem like something that's going to gain new users either.
> Is it really worth modernizing this binding?

The whole ordeal was actually prompted by the emergence of a
new user who wants to add a device tree for a new device.
So I don't want to add new users for the old bindings.

Yours,
Linus Walleij

[PATCH 3/6] MIPS: NI 169445: Fix NAND GPIOs

[Linus Walleij]

This changes the GPIOs defined in the device tree to recommended
practice, which is also what the Linux NAND GPIO driver is actually
using.

In the process, fix up the CE and WP lines to be active low, as is
required for proper hardware description.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/mips/boot/dts/ni/169445.dts | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/mips/boot/dts/ni/169445.dts b/arch/mips/boot/dts/ni/169445.dts
index 5389ef46c480..3e7b46d5072c 100644
--- a/arch/mips/boot/dts/ni/169445.dts
+++ b/arch/mips/boot/dts/ni/169445.dts
@@ -1,4 +1,5 @@
 /dts-v1/;
+#include <dt-bindings/gpio/gpio.h>
 
 / {
 	#address-cells = <1>;
@@ -57,18 +58,18 @@ gpio2: gpio@14 {
 			no-output;
 		};
 
-		nand@0 {
+		nand-controller@0 {
 			compatible = "gpio-control-nand";
 			nand-on-flash-bbt;
 			nand-ecc-mode = "soft_bch";
 			nand-ecc-step-size = <512>;
 			nand-ecc-strength = <4>;
 			reg = <0x0 4>;
-			gpios = <&gpio2 0 0>, /* rdy */
-				<&gpio1 1 0>, /* nce */
-				<&gpio1 2 0>, /* ale */
-				<&gpio1 3 0>, /* cle */
-				<&gpio1 4 0>; /* nwp */
+			rdy-gpios = <&gpio2 0 GPIO_ACTIVE_HIGH>;
+			ce-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+			ale-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
+			cle-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>;
+			wp-gpios = <&gpio1 4 GPIO_ACTIVE_LOW>;
 		};
 
 		serial@80000 {

-- 
2.34.1

[PATCH 4/6] mtd: rawnand: gpio: Use device properties

[Linus Walleij]

The platform data (struct gpio_nand_platdata) isn't really used
in any boardfile in the kernel: the only probe path is from
device tree.

Convert the driver to not use the platform data header at all
and read out the device tree properties using device
properties so we don't need to have the driver be exclusively
device tree either: ACPI or software nodes work fine if
need be. Drop the ifdeffery around CONFIG_OF as a consequence.

The code reads "bank-width" to plat->options flags and passes
it directly to the NAND chip struct, so just assign this
directly to the chip instead.

The code reads one property "chip-delay" that it stores
in pdata->delay and never use, so drop this altogether.
If timings should be supported this can probably be done in
a more detailed way using the new elaborate timings structs
that exist for NAND.

The platform data contains a callback to augment partitions,
but since there are no board files using this platform
data to define a gpio NAND device, this is never used so
the code handling it can be deleted.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mtd/nand/raw/gpio.c | 72 ++++++++-------------------------------------
 1 file changed, 12 insertions(+), 60 deletions(-)

diff --git a/drivers/mtd/nand/raw/gpio.c b/drivers/mtd/nand/raw/gpio.c
index df6facf0ec9a..5553101c709c 100644
--- a/drivers/mtd/nand/raw/gpio.c
+++ b/drivers/mtd/nand/raw/gpio.c
@@ -22,9 +22,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/rawnand.h>
 #include <linux/mtd/partitions.h>
-#include <linux/mtd/nand-gpio.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
+#include <linux/property.h>
 #include <linux/delay.h>
 
 struct gpiomtd {
@@ -32,7 +30,6 @@ struct gpiomtd {
 	void __iomem		*io;
 	void __iomem		*io_sync;
 	struct nand_chip	nand_chip;
-	struct gpio_nand_platdata plat;
 	struct gpio_desc *ce; /* Optional chip enable */
 	struct gpio_desc *cle;
 	struct gpio_desc *ale;
@@ -175,46 +172,38 @@ static const struct nand_controller_ops gpio_nand_ops = {
 	.attach_chip = gpio_nand_attach_chip,
 };
 
-#ifdef CONFIG_OF
 static const struct of_device_id gpio_nand_id_table[] = {
 	{ .compatible = "gpio-control-nand" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, gpio_nand_id_table);
 
-static int gpio_nand_get_config_of(const struct device *dev,
-				   struct gpio_nand_platdata *plat)
+static int gpio_nand_get_config(struct device *dev,
+				struct nand_chip *chip)
 {
 	u32 val;
 
-	if (!dev->of_node)
-		return -ENODEV;
-
-	if (!of_property_read_u32(dev->of_node, "bank-width", &val)) {
+	if (!device_property_read_u32(dev, "bank-width", &val)) {
 		if (val == 2) {
-			plat->options |= NAND_BUSWIDTH_16;
+			chip->options |= NAND_BUSWIDTH_16;
 		} else if (val != 1) {
 			dev_err(dev, "invalid bank-width %u\n", val);
 			return -EINVAL;
 		}
 	}
 
-	if (!of_property_read_u32(dev->of_node, "chip-delay", &val))
-		plat->chip_delay = val;
-
 	return 0;
 }
 
-static struct resource *gpio_nand_get_io_sync_of(struct platform_device *pdev)
+static struct resource *gpio_nand_get_io_sync_prop(struct device *dev)
 {
 	struct resource *r;
 	u64 addr;
 
-	if (of_property_read_u64(pdev->dev.of_node,
-				       "gpio-control-nand,io-sync-reg", &addr))
+	if (device_property_read_u64(dev, "gpio-control-nand,io-sync-reg", &addr))
 		return NULL;
 
-	r = devm_kzalloc(&pdev->dev, sizeof(*r), GFP_KERNEL);
+	r = devm_kzalloc(dev, sizeof(*r), GFP_KERNEL);
 	if (!r)
 		return NULL;
 
@@ -224,40 +213,11 @@ static struct resource *gpio_nand_get_io_sync_of(struct platform_device *pdev)
 
 	return r;
 }
-#else /* CONFIG_OF */
-static inline int gpio_nand_get_config_of(const struct device *dev,
-					  struct gpio_nand_platdata *plat)
-{
-	return -ENOSYS;
-}
-
-static inline struct resource *
-gpio_nand_get_io_sync_of(struct platform_device *pdev)
-{
-	return NULL;
-}
-#endif /* CONFIG_OF */
-
-static inline int gpio_nand_get_config(const struct device *dev,
-				       struct gpio_nand_platdata *plat)
-{
-	int ret = gpio_nand_get_config_of(dev, plat);
-
-	if (!ret)
-		return ret;
-
-	if (dev_get_platdata(dev)) {
-		memcpy(plat, dev_get_platdata(dev), sizeof(*plat));
-		return 0;
-	}
-
-	return -EINVAL;
-}
 
 static inline struct resource *
 gpio_nand_get_io_sync(struct platform_device *pdev)
 {
-	struct resource *r = gpio_nand_get_io_sync_of(pdev);
+	struct resource *r = gpio_nand_get_io_sync_prop(&pdev->dev);
 
 	if (r)
 		return r;
@@ -291,9 +251,6 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	int ret = 0;
 
-	if (!dev->of_node && !dev_get_platdata(dev))
-		return -EINVAL;
-
 	gpiomtd = devm_kzalloc(dev, sizeof(*gpiomtd), GFP_KERNEL);
 	if (!gpiomtd)
 		return -ENOMEM;
@@ -311,7 +268,7 @@ static int gpio_nand_probe(struct platform_device *pdev)
 			return PTR_ERR(gpiomtd->io_sync);
 	}
 
-	ret = gpio_nand_get_config(dev, &gpiomtd->plat);
+	ret = gpio_nand_get_config(dev, chip);
 	if (ret)
 		return ret;
 
@@ -349,7 +306,6 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	gpiomtd->base.ops = &gpio_nand_ops;
 
 	nand_set_flash_node(chip, pdev->dev.of_node);
-	chip->options		= gpiomtd->plat.options;
 	chip->controller	= &gpiomtd->base;
 
 	mtd			= nand_to_mtd(chip);
@@ -372,11 +328,7 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_wp;
 
-	if (gpiomtd->plat.adjust_parts)
-		gpiomtd->plat.adjust_parts(&gpiomtd->plat, mtd->size);
-
-	ret = mtd_device_register(mtd, gpiomtd->plat.parts,
-				  gpiomtd->plat.num_parts);
+	ret = mtd_device_register(mtd, NULL, 0);
 	if (!ret)
 		return 0;
 
@@ -395,7 +347,7 @@ static struct platform_driver gpio_nand_driver = {
 	.remove_new	= gpio_nand_remove,
 	.driver		= {
 		.name	= "gpio-nand",
-		.of_match_table = of_match_ptr(gpio_nand_id_table),
+		.of_match_table = gpio_nand_id_table,
 	},
 };
 

-- 
2.34.1

Re: [PATCH 4/6] mtd: rawnand: gpio: Use device properties

[Miquel Raynal]

Hi Linus,

linus.walleij@linaro.org wrote on Wed, 08 Nov 2023 15:33:52 +0100:

> The platform data (struct gpio_nand_platdata) isn't really used
> in any boardfile in the kernel: the only probe path is from
> device tree.
> 
> Convert the driver to not use the platform data header at all
> and read out the device tree properties using device
> properties so we don't need to have the driver be exclusively
> device tree either: ACPI or software nodes work fine if
> need be. Drop the ifdeffery around CONFIG_OF as a consequence.
> 
> The code reads "bank-width" to plat->options flags and passes
> it directly to the NAND chip struct, so just assign this
> directly to the chip instead.
> 
> The code reads one property "chip-delay" that it stores
> in pdata->delay and never use, so drop this altogether.
> If timings should be supported this can probably be done in
> a more detailed way using the new elaborate timings structs
> that exist for NAND.
> 
> The platform data contains a callback to augment partitions,
> but since there are no board files using this platform
> data to define a gpio NAND device, this is never used so
> the code handling it can be deleted.

Nice cleanup. I'm a bit more reluctant on the bindings side, I'll come
back to it later, but the driver side looks neat.

Thanks,
Miquèl

[PATCH 5/6] mtd: rawnand: gpio: Support standard nand width

[Linus Walleij]

The standard property for describing the band width of a NAND
memory is "nand-bus-width" not "bank-width". The new bindings
support both so make Linux check both in priority order.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mtd/nand/raw/gpio.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/gpio.c b/drivers/mtd/nand/raw/gpio.c
index 5553101c709c..d5bd245b0c0d 100644
--- a/drivers/mtd/nand/raw/gpio.c
+++ b/drivers/mtd/nand/raw/gpio.c
@@ -183,7 +183,15 @@ static int gpio_nand_get_config(struct device *dev,
 {
 	u32 val;
 
-	if (!device_property_read_u32(dev, "bank-width", &val)) {
+	/* The preferred binding takes precedence */
+	if (!device_property_read_u32(dev, "nand-bus-width", &val)) {
+		if (val == 16) {
+			chip->options |= NAND_BUSWIDTH_16;
+		} else if (val != 8) {
+			dev_err(dev, "invalid nand-bus-width %u\n", val);
+			return -EINVAL;
+		}
+	} else if (!device_property_read_u32(dev, "bank-width", &val)) {
 		if (val == 2) {
 			chip->options |= NAND_BUSWIDTH_16;
 		} else if (val != 1) {

-- 
2.34.1

Re: [PATCH 5/6] mtd: rawnand: gpio: Support standard nand width

[Miquel Raynal]

Hi Linus,

linus.walleij@linaro.org wrote on Wed, 08 Nov 2023 15:33:53 +0100:

> The standard property for describing the band width of a NAND
> memory is "nand-bus-width" not "bank-width". The new bindings
> support both so make Linux check both in priority order.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/mtd/nand/raw/gpio.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/gpio.c b/drivers/mtd/nand/raw/gpio.c
> index 5553101c709c..d5bd245b0c0d 100644
> --- a/drivers/mtd/nand/raw/gpio.c
> +++ b/drivers/mtd/nand/raw/gpio.c
> @@ -183,7 +183,15 @@ static int gpio_nand_get_config(struct device *dev,
>  {
>  	u32 val;
>  
> -	if (!device_property_read_u32(dev, "bank-width", &val)) {
> +	/* The preferred binding takes precedence */
> +	if (!device_property_read_u32(dev, "nand-bus-width", &val)) {
> +		if (val == 16) {
> +			chip->options |= NAND_BUSWIDTH_16;
> +		} else if (val != 8) {
> +			dev_err(dev, "invalid nand-bus-width %u\n", val);
> +			return -EINVAL;
> +		}
> +	} else if (!device_property_read_u32(dev, "bank-width", &val)) {
>  		if (val == 2) {
>  			chip->options |= NAND_BUSWIDTH_16;
>  		} else if (val != 1) {
> 

I'm not sure this is actually needed. I believe of_get_nand_bus_width
is already called (nand_scan_ident -> rawnand_dt_init) and will set the
NAND_BUSWIDTH_16 flag automatically. So the above 'if' is already a
fallback. Maybe you can add a comment if you want to make this more
explicit that the real property is nand-bus-width and the property
parsed in the driver is for backward compatibility only?

Thanks,
Miquèl

[PATCH 6/6] mtd: rawnand: gpio: Rename file

[Linus Walleij]

The implementation of the GPIO NAND controller is just "gpio"
with the usecase for NAND implied from the folder nand/raw.

This is not so great when the module gets the name "gpio.ko".
Rename the implementation to nand-gpio.c so the module is
named nand-gpio.ko which is more reasonable.

We put "nand" first instead of "gpio" because the order is
usually <subsystem>-<driver>.c, cf ls drivers/gpio/

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mtd/nand/raw/Makefile                | 2 +-
 drivers/mtd/nand/raw/{gpio.c => nand-gpio.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index 25120a4afada..f0e377332812 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -18,7 +18,7 @@ obj-$(CONFIG_MTD_NAND_NANDSIM)		+= nandsim.o
 obj-$(CONFIG_MTD_NAND_CS553X)		+= cs553x_nand.o
 obj-$(CONFIG_MTD_NAND_NDFC)		+= ndfc.o
 obj-$(CONFIG_MTD_NAND_ATMEL)		+= atmel/
-obj-$(CONFIG_MTD_NAND_GPIO)		+= gpio.o
+obj-$(CONFIG_MTD_NAND_GPIO)		+= nand-gpio.o
 omap2_nand-objs := omap2.o
 obj-$(CONFIG_MTD_NAND_OMAP2) 		+= omap2_nand.o
 obj-$(CONFIG_MTD_NAND_OMAP_BCH_BUILD)	+= omap_elm.o
diff --git a/drivers/mtd/nand/raw/gpio.c b/drivers/mtd/nand/raw/nand-gpio.c
similarity index 100%
rename from drivers/mtd/nand/raw/gpio.c
rename to drivers/mtd/nand/raw/nand-gpio.c

-- 
2.34.1

Re: [PATCH 6/6] mtd: rawnand: gpio: Rename file

[Miquel Raynal]

Hi Linus,

linus.walleij@linaro.org wrote on Wed, 08 Nov 2023 15:33:54 +0100:

> The implementation of the GPIO NAND controller is just "gpio"
> with the usecase for NAND implied from the folder nand/raw.
> 
> This is not so great when the module gets the name "gpio.ko".
> Rename the implementation to nand-gpio.c so the module is
> named nand-gpio.ko which is more reasonable.
> 
> We put "nand" first instead of "gpio" because the order is
> usually <subsystem>-<driver>.c, cf ls drivers/gpio/

Do you mind if we take the "english" version which would rather be
"gpio-nand-controller.c/o"? I _really_ want people to understand we
don't emulate the storage but the host part here.

Thanks,
Miquèl