[PATCH 0/3] mtd: rawnand: gpio: Convert to exec_op()

[PATCH 0/3] mtd: rawnand: gpio: Convert to exec_op()

[Boris Brezillon]

Hello,

A bit of context to explain the motivation behind those conversions
I've been sending for the last few weeks. The raw NAND subsystem
carries a lot of history which makes any rework not only painful, but
also subject to regressions which we only detect when someone dares to
update its kernel on one of those ancient HW. While carrying drivers
for old HW is not a problem per se, carrying ancient and unmaintained
drivers that are not converted to new APIs is a maintenance burden,
hence this massive conversion attempt I'm conducting here.

So here is a series converting the GPIO NAND controller driver to
exec_op(). I hope I'll find someone to test those changes, but if
there's no one still having access to this HW or no interest in keeping
it supported in recent kernel versions, we should definitely consider
removing the driver instead.

Regards,

Boris

Boris Brezillon (3):
  mtd: rawnand: gpio: Inherit from nand_controller
  mtd: rawnand: gpio: Implement exec_op()
  mtd: rawnand: gpio: Get rid of the legacy interface implementation

 drivers/mtd/nand/raw/gpio.c | 116 ++++++++++++++++++++++++++++--------
 1 file changed, 91 insertions(+), 25 deletions(-)

-- 
2.25.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 1/3] mtd: rawnand: gpio: Inherit from nand_controller

[Boris Brezillon]

Inherit from nand_controller so we don't rely on the
nand_chip.legacy.dummy_controller field.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/nand/raw/gpio.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mtd/nand/raw/gpio.c b/drivers/mtd/nand/raw/gpio.c
index 938077e5c6a9..33828fb20a13 100644
--- a/drivers/mtd/nand/raw/gpio.c
+++ b/drivers/mtd/nand/raw/gpio.c
@@ -27,6 +27,7 @@
 #include <linux/of_address.h>
 
 struct gpiomtd {
+	struct nand_controller	base;
 	void __iomem		*io_sync;
 	struct nand_chip	nand_chip;
 	struct gpio_nand_platdata plat;
@@ -273,6 +274,7 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	if (gpiomtd->rdy)
 		chip->legacy.dev_ready = gpio_nand_devready;
 
+	nand_controller_init(&gpiomtd->base);
 	nand_set_flash_node(chip, pdev->dev.of_node);
 	chip->legacy.IO_ADDR_W	= chip->legacy.IO_ADDR_R;
 	chip->ecc.mode		= NAND_ECC_SOFT;
@@ -280,6 +282,7 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	chip->options		= gpiomtd->plat.options;
 	chip->legacy.chip_delay	= gpiomtd->plat.chip_delay;
 	chip->legacy.cmd_ctrl	= gpio_nand_cmd_ctrl;
+	chip->controller	= &gpiomtd->base;
 
 	mtd			= nand_to_mtd(chip);
 	mtd->dev.parent		= dev;
-- 
2.25.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/3] mtd: rawnand: gpio: Inherit from nand_controller

[Miquel Raynal]

On Wed, 2020-06-03 at 15:07:44 UTC, Boris Brezillon wrote:
> Inherit from nand_controller so we don't rely on the
> nand_chip.legacy.dummy_controller field.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/3] mtd: rawnand: gpio: Implement exec_op()

[Boris Brezillon]

Implement exec_op() so we can get rid of the legacy interface
implementation.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/nand/raw/gpio.c | 104 ++++++++++++++++++++++++++++++++++--
 1 file changed, 101 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/gpio.c b/drivers/mtd/nand/raw/gpio.c
index 33828fb20a13..115998011192 100644
--- a/drivers/mtd/nand/raw/gpio.c
+++ b/drivers/mtd/nand/raw/gpio.c
@@ -25,9 +25,11 @@
 #include <linux/mtd/nand-gpio.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/delay.h>
 
 struct gpiomtd {
 	struct nand_controller	base;
+	void __iomem		*io;
 	void __iomem		*io_sync;
 	struct nand_chip	nand_chip;
 	struct gpio_nand_platdata plat;
@@ -98,6 +100,99 @@ static int gpio_nand_devready(struct nand_chip *chip)
 	return gpiod_get_value(gpiomtd->rdy);
 }
 
+static int gpio_nand_exec_instr(struct nand_chip *chip,
+				const struct nand_op_instr *instr)
+{
+	struct gpiomtd *gpiomtd = gpio_nand_getpriv(nand_to_mtd(chip));
+	unsigned int i;
+
+	switch (instr->type) {
+	case NAND_OP_CMD_INSTR:
+		gpio_nand_dosync(gpiomtd);
+		gpiod_set_value(gpiomtd->cle, 1);
+		gpio_nand_dosync(gpiomtd);
+		writeb(instr->ctx.cmd.opcode, gpiomtd->io);
+		gpio_nand_dosync(gpiomtd);
+		gpiod_set_value(gpiomtd->cle, 0);
+		return 0;
+
+	case NAND_OP_ADDR_INSTR:
+		gpio_nand_dosync(gpiomtd);
+		gpiod_set_value(gpiomtd->ale, 1);
+		gpio_nand_dosync(gpiomtd);
+		for (i = 0; i < instr->ctx.addr.naddrs; i++)
+			writeb(instr->ctx.addr.addrs[i], gpiomtd->io);
+		gpio_nand_dosync(gpiomtd);
+		gpiod_set_value(gpiomtd->ale, 0);
+		return 0;
+
+	case NAND_OP_DATA_IN_INSTR:
+		gpio_nand_dosync(gpiomtd);
+		if ((chip->options & NAND_BUSWIDTH_16) &&
+		    !instr->ctx.data.force_8bit)
+			ioread16_rep(gpiomtd->io, instr->ctx.data.buf.in,
+				     instr->ctx.data.len / 2);
+		else
+			ioread8_rep(gpiomtd->io, instr->ctx.data.buf.in,
+				    instr->ctx.data.len);
+		return 0;
+
+	case NAND_OP_DATA_OUT_INSTR:
+		gpio_nand_dosync(gpiomtd);
+		if ((chip->options & NAND_BUSWIDTH_16) &&
+		    !instr->ctx.data.force_8bit)
+			iowrite16_rep(gpiomtd->io, instr->ctx.data.buf.out,
+				      instr->ctx.data.len / 2);
+		else
+			iowrite8_rep(gpiomtd->io, instr->ctx.data.buf.out,
+				     instr->ctx.data.len);
+		return 0;
+
+	case NAND_OP_WAITRDY_INSTR:
+		if (!gpiomtd->rdy)
+			return nand_soft_waitrdy(chip, instr->ctx.waitrdy.timeout_ms);
+
+		return nand_gpio_waitrdy(chip, gpiomtd->rdy,
+					 instr->ctx.waitrdy.timeout_ms);
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int gpio_nand_exec_op(struct nand_chip *chip,
+			     const struct nand_operation *op,
+			     bool check_only)
+{
+	struct gpiomtd *gpiomtd = gpio_nand_getpriv(nand_to_mtd(chip));
+	unsigned int i;
+	int ret = 0;
+
+	if (check_only)
+		return 0;
+
+	gpio_nand_dosync(gpiomtd);
+	gpiod_set_value(gpiomtd->nce, 0);
+	for (i = 0; i < op->ninstrs; i++) {
+		ret = gpio_nand_exec_instr(chip, &op->instrs[i]);
+		if (ret)
+			break;
+
+		if (op->instrs[i].delay_ns)
+			ndelay(op->instrs[i].delay_ns);
+	}
+	gpio_nand_dosync(gpiomtd);
+	gpiod_set_value(gpiomtd->nce, 1);
+
+	return ret;
+}
+
+static const struct nand_controller_ops gpio_nand_ops = {
+	.exec_op = gpio_nand_exec_op,
+};
+
 #ifdef CONFIG_OF
 static const struct of_device_id gpio_nand_id_table[] = {
 	{ .compatible = "gpio-control-nand" },
@@ -226,9 +321,9 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	chip = &gpiomtd->nand_chip;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	chip->legacy.IO_ADDR_R = devm_ioremap_resource(dev, res);
-	if (IS_ERR(chip->legacy.IO_ADDR_R))
-		return PTR_ERR(chip->legacy.IO_ADDR_R);
+	gpiomtd->io = devm_ioremap_resource(dev, res);
+	if (IS_ERR(gpiomtd->io))
+		return PTR_ERR(gpiomtd->io);
 
 	res = gpio_nand_get_io_sync(pdev);
 	if (res) {
@@ -275,7 +370,10 @@ static int gpio_nand_probe(struct platform_device *pdev)
 		chip->legacy.dev_ready = gpio_nand_devready;
 
 	nand_controller_init(&gpiomtd->base);
+	gpiomtd->base.ops = &gpio_nand_ops;
+
 	nand_set_flash_node(chip, pdev->dev.of_node);
+	chip->legacy.IO_ADDR_R	= gpiomtd->io;
 	chip->legacy.IO_ADDR_W	= chip->legacy.IO_ADDR_R;
 	chip->ecc.mode		= NAND_ECC_SOFT;
 	chip->ecc.algo		= NAND_ECC_HAMMING;
-- 
2.25.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/3] mtd: rawnand: gpio: Implement exec_op()

[Miquel Raynal]

On Wed, 2020-06-03 at 15:07:45 UTC, Boris Brezillon wrote:
> Implement exec_op() so we can get rid of the legacy interface
> implementation.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 3/3] mtd: rawnand: gpio: Get rid of the legacy interface implementation

[Boris Brezillon]

Now that exec_op() is implemented, we can get rid of the legacy interface
implementation.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/nand/raw/gpio.c | 35 -----------------------------------
 1 file changed, 35 deletions(-)

diff --git a/drivers/mtd/nand/raw/gpio.c b/drivers/mtd/nand/raw/gpio.c
index 115998011192..3bd847ccc3f3 100644
--- a/drivers/mtd/nand/raw/gpio.c
+++ b/drivers/mtd/nand/raw/gpio.c
@@ -72,34 +72,6 @@ static void gpio_nand_dosync(struct gpiomtd *gpiomtd)
 static inline void gpio_nand_dosync(struct gpiomtd *gpiomtd) {}
 #endif
 
-static void gpio_nand_cmd_ctrl(struct nand_chip *chip, int cmd,
-			       unsigned int ctrl)
-{
-	struct gpiomtd *gpiomtd = gpio_nand_getpriv(nand_to_mtd(chip));
-
-	gpio_nand_dosync(gpiomtd);
-
-	if (ctrl & NAND_CTRL_CHANGE) {
-		if (gpiomtd->nce)
-			gpiod_set_value(gpiomtd->nce, !(ctrl & NAND_NCE));
-		gpiod_set_value(gpiomtd->cle, !!(ctrl & NAND_CLE));
-		gpiod_set_value(gpiomtd->ale, !!(ctrl & NAND_ALE));
-		gpio_nand_dosync(gpiomtd);
-	}
-	if (cmd == NAND_CMD_NONE)
-		return;
-
-	writeb(cmd, gpiomtd->nand_chip.legacy.IO_ADDR_W);
-	gpio_nand_dosync(gpiomtd);
-}
-
-static int gpio_nand_devready(struct nand_chip *chip)
-{
-	struct gpiomtd *gpiomtd = gpio_nand_getpriv(nand_to_mtd(chip));
-
-	return gpiod_get_value(gpiomtd->rdy);
-}
-
 static int gpio_nand_exec_instr(struct nand_chip *chip,
 				const struct nand_op_instr *instr)
 {
@@ -365,21 +337,14 @@ static int gpio_nand_probe(struct platform_device *pdev)
 		ret = PTR_ERR(gpiomtd->rdy);
 		goto out_ce;
 	}
-	/* Using RDY pin */
-	if (gpiomtd->rdy)
-		chip->legacy.dev_ready = gpio_nand_devready;
 
 	nand_controller_init(&gpiomtd->base);
 	gpiomtd->base.ops = &gpio_nand_ops;
 
 	nand_set_flash_node(chip, pdev->dev.of_node);
-	chip->legacy.IO_ADDR_R	= gpiomtd->io;
-	chip->legacy.IO_ADDR_W	= chip->legacy.IO_ADDR_R;
 	chip->ecc.mode		= NAND_ECC_SOFT;
 	chip->ecc.algo		= NAND_ECC_HAMMING;
 	chip->options		= gpiomtd->plat.options;
-	chip->legacy.chip_delay	= gpiomtd->plat.chip_delay;
-	chip->legacy.cmd_ctrl	= gpio_nand_cmd_ctrl;
 	chip->controller	= &gpiomtd->base;
 
 	mtd			= nand_to_mtd(chip);
-- 
2.25.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 3/3] mtd: rawnand: gpio: Get rid of the legacy interface implementation

[Miquel Raynal]

On Wed, 2020-06-03 at 15:07:46 UTC, Boris Brezillon wrote:
> Now that exec_op() is implemented, we can get rid of the legacy interface
> implementation.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 0/3] mtd: rawnand: gpio: Convert to exec_op()

[Miquel Raynal]

Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed,  3 Jun
2020 17:07:43 +0200:

> Hello,
> 
> A bit of context to explain the motivation behind those conversions
> I've been sending for the last few weeks. The raw NAND subsystem
> carries a lot of history which makes any rework not only painful, but
> also subject to regressions which we only detect when someone dares to
> update its kernel on one of those ancient HW. While carrying drivers
> for old HW is not a problem per se, carrying ancient and unmaintained
> drivers that are not converted to new APIs is a maintenance burden,
> hence this massive conversion attempt I'm conducting here.
> 
> So here is a series converting the GPIO NAND controller driver to
> exec_op(). I hope I'll find someone to test those changes, but if
> there's no one still having access to this HW or no interest in keeping
> it supported in recent kernel versions, we should definitely consider
> removing the driver instead.
> 
> Regards,
> 
> Boris
> 
> Boris Brezillon (3):
>   mtd: rawnand: gpio: Inherit from nand_controller
>   mtd: rawnand: gpio: Implement exec_op()
>   mtd: rawnand: gpio: Get rid of the legacy interface implementation
> 
>  drivers/mtd/nand/raw/gpio.c | 116 ++++++++++++++++++++++++++++--------
>  1 file changed, 91 insertions(+), 25 deletions(-)
> 

Looks fine too!

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/