[PATCH 0/5] mtd: rawnand: davinci: Convert to exec_op()

[PATCH 0/5] mtd: rawnand: davinci: 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 couple of 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 Davinci NAND controller driver to
exec_op(), plus a bunch of minor improvements done along the way.

Regards,

Boris

Boris Brezillon (5):
  mtd: rawnand: davinci: Inherit from nand_controller
  mtd: rawnand: davinci: Stop using nand_chip.legacy.IO_ADDR_{R,W}
  mtd: rawnand: davinci: Implement exec_op()
  mtd: rawnand: davinci: Get rid of the legacy interface implementation
  mtd: rawnand: davinci: Change the {read,write}_buf prototypes

 drivers/mtd/nand/raw/davinci_nand.c | 161 +++++++++++++++-------------
 1 file changed, 85 insertions(+), 76 deletions(-)

-- 
2.25.3


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

[PATCH 1/5] mtd: rawnand: davinci: Inherit from nand_controller

[Boris Brezillon]

Let's not rely on the dummy_controller embedded in nand_chip.legacy
and explicitly inherit from nand_controller instead.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.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 25c185bea50c..0312c632d86a 100644
--- a/drivers/mtd/nand/raw/davinci_nand.c
+++ b/drivers/mtd/nand/raw/davinci_nand.c
@@ -38,6 +38,7 @@
  * outputs in a "wire-AND" configuration, with no per-chip signals.
  */
 struct davinci_nand_info {
+	struct nand_controller	controller;
 	struct nand_chip	chip;
 
 	struct platform_device	*pdev;
@@ -788,7 +789,9 @@ static int nand_davinci_probe(struct platform_device *pdev)
 	spin_unlock_irq(&davinci_nand_lock);
 
 	/* Scan to find existence of the device(s) */
-	info->chip.legacy.dummy_controller.ops = &davinci_nand_controller_ops;
+	nand_controller_init(&info->controller);
+	info->controller.ops = &davinci_nand_controller_ops;
+	info->chip.controller = &info->controller;
 	ret = nand_scan(&info->chip, pdata->mask_chipsel ? 2 : 1);
 	if (ret < 0) {
 		dev_dbg(&pdev->dev, "no NAND chip(s) found\n");
-- 
2.25.3


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

Re: [PATCH 1/5] mtd: rawnand: davinci: Inherit from nand_controller

[Miquel Raynal]

Boris Brezillon <boris.brezillon@collabora.com> wrote on Fri,  1 May
2020 12:07:25 +0200:

> Let's not rely on the dummy_controller embedded in nand_chip.legacy
> and explicitly inherit from nand_controller instead.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

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

[PATCH 2/5] mtd: rawnand: davinci: Stop using nand_chip.legacy.IO_ADDR_{R, W}

[Boris Brezillon]

We can use info->current_cs directly instead of doing this weird
IO_ADDR_{R,W} re-assignment dance.

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

diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
index 0312c632d86a..779f708c791f 100644
--- a/drivers/mtd/nand/raw/davinci_nand.c
+++ b/drivers/mtd/nand/raw/davinci_nand.c
@@ -91,18 +91,13 @@ static void nand_davinci_hwcontrol(struct nand_chip *nand, int cmd,
 	struct davinci_nand_info *info = to_davinci_nand(nand_to_mtd(nand));
 	void __iomem			*addr = info->current_cs;
 
-	/* Did the control lines change? */
-	if (ctrl & NAND_CTRL_CHANGE) {
-		if ((ctrl & NAND_CTRL_CLE) == NAND_CTRL_CLE)
-			addr += info->mask_cle;
-		else if ((ctrl & NAND_CTRL_ALE) == NAND_CTRL_ALE)
-			addr += info->mask_ale;
-
-		nand->legacy.IO_ADDR_W = addr;
-	}
+	if (ctrl & NAND_CTRL_CLE)
+		addr += info->mask_cle;
+	else if (ctrl & NAND_CTRL_ALE)
+		addr += info->mask_ale;
 
 	if (cmd != NAND_CMD_NONE)
-		iowrite8(cmd, nand->legacy.IO_ADDR_W);
+		iowrite8(cmd, addr);
 }
 
 static void nand_davinci_select_chip(struct nand_chip *nand, int chip)
@@ -114,9 +109,6 @@ static void nand_davinci_select_chip(struct nand_chip *nand, int chip)
 	/* maybe kick in a second chipselect */
 	if (chip > 0)
 		info->current_cs += info->mask_chipsel;
-
-	info->chip.legacy.IO_ADDR_W = info->current_cs;
-	info->chip.legacy.IO_ADDR_R = info->chip.legacy.IO_ADDR_W;
 }
 
 /*----------------------------------------------------------------------*/
@@ -425,23 +417,27 @@ static int nand_davinci_correct_4bit(struct nand_chip *chip, u_char *data,
 static void nand_davinci_read_buf(struct nand_chip *chip, uint8_t *buf,
 				  int len)
 {
+	struct davinci_nand_info *info = to_davinci_nand(nand_to_mtd(chip));
+
 	if ((0x03 & ((uintptr_t)buf)) == 0 && (0x03 & len) == 0)
-		ioread32_rep(chip->legacy.IO_ADDR_R, buf, len >> 2);
+		ioread32_rep(info->current_cs, buf, len >> 2);
 	else if ((0x01 & ((uintptr_t)buf)) == 0 && (0x01 & len) == 0)
-		ioread16_rep(chip->legacy.IO_ADDR_R, buf, len >> 1);
+		ioread16_rep(info->current_cs, buf, len >> 1);
 	else
-		ioread8_rep(chip->legacy.IO_ADDR_R, buf, len);
+		ioread8_rep(info->current_cs, buf, len);
 }
 
 static void nand_davinci_write_buf(struct nand_chip *chip, const uint8_t *buf,
 				   int len)
 {
+	struct davinci_nand_info *info = to_davinci_nand(nand_to_mtd(chip));
+
 	if ((0x03 & ((uintptr_t)buf)) == 0 && (0x03 & len) == 0)
-		iowrite32_rep(chip->legacy.IO_ADDR_R, buf, len >> 2);
+		iowrite32_rep(info->current_cs, buf, len >> 2);
 	else if ((0x01 & ((uintptr_t)buf)) == 0 && (0x01 & len) == 0)
-		iowrite16_rep(chip->legacy.IO_ADDR_R, buf, len >> 1);
+		iowrite16_rep(info->current_cs, buf, len >> 1);
 	else
-		iowrite8_rep(chip->legacy.IO_ADDR_R, buf, len);
+		iowrite8_rep(info->current_cs, buf, len);
 }
 
 /*
@@ -747,8 +743,6 @@ static int nand_davinci_probe(struct platform_device *pdev)
 	mtd->dev.parent		= &pdev->dev;
 	nand_set_flash_node(&info->chip, pdev->dev.of_node);
 
-	info->chip.legacy.IO_ADDR_R	= vaddr;
-	info->chip.legacy.IO_ADDR_W	= vaddr;
 	info->chip.legacy.chip_delay	= 0;
 	info->chip.legacy.select_chip	= nand_davinci_select_chip;
 
-- 
2.25.3


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

Re: [PATCH 2/5] mtd: rawnand: davinci: Stop using nand_chip.legacy.IO_ADDR_{R,W}

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Fri,  1 May
2020 12:07:26 +0200:

> We can use info->current_cs directly instead of doing this weird
> IO_ADDR_{R,W} re-assignment dance.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

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

[PATCH 3/5] mtd: rawnand: davinci: Implement exec_op()

[Boris Brezillon]

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

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

diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
index 779f708c791f..296047884c6a 100644
--- a/drivers/mtd/nand/raw/davinci_nand.c
+++ b/drivers/mtd/nand/raw/davinci_nand.c
@@ -14,7 +14,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/err.h>
-#include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/mtd/rawnand.h>
 #include <linux/mtd/partitions.h>
 #include <linux/slab.h>
@@ -678,8 +678,78 @@ static int davinci_nand_attach_chip(struct nand_chip *chip)
 	return ret;
 }
 
+static int davinci_nand_exec_instr(struct davinci_nand_info *info,
+				   const struct nand_op_instr *instr)
+{
+	unsigned int i, timeout_us;
+	u32 status;
+	int ret;
+
+	switch (instr->type) {
+	case NAND_OP_CMD_INSTR:
+		iowrite8(instr->ctx.cmd.opcode,
+			 info->current_cs + info->mask_cle);
+		break;
+
+	case NAND_OP_ADDR_INSTR:
+		for (i = 0; i < instr->ctx.addr.naddrs; i++) {
+			iowrite8(instr->ctx.addr.addrs[i],
+				 info->current_cs + info->mask_ale);
+		}
+		break;
+
+	case NAND_OP_DATA_IN_INSTR:
+		nand_davinci_read_buf(&info->chip, instr->ctx.data.buf.in,
+				      instr->ctx.data.len);
+		break;
+
+	case NAND_OP_DATA_OUT_INSTR:
+		nand_davinci_write_buf(&info->chip, instr->ctx.data.buf.out,
+				       instr->ctx.data.len);
+		break;
+
+	case NAND_OP_WAITRDY_INSTR:
+		timeout_us = instr->ctx.waitrdy.timeout_ms * 1000;
+		ret = readl_relaxed_poll_timeout(info->base + NANDFSR_OFFSET,
+						 status, status & BIT(0), 100,
+						 timeout_us);
+		if (ret)
+			return ret;
+
+		break;
+	}
+
+	if (instr->delay_ns)
+		ndelay(instr->delay_ns);
+
+	return 0;
+}
+
+static int davinci_nand_exec_op(struct nand_chip *chip,
+				const struct nand_operation *op,
+				bool check_only)
+{
+	struct davinci_nand_info *info = to_davinci_nand(nand_to_mtd(chip));
+	unsigned int i;
+	int ret;
+
+	if (check_only)
+		return true;
+
+	info->current_cs = info->vaddr + (op->cs * info->mask_chipsel);
+
+	for (i = 0; i < op->ninstrs; i++) {
+		ret = davinci_nand_exec_instr(info, &op->instrs[i]);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
 static const struct nand_controller_ops davinci_nand_controller_ops = {
 	.attach_chip = davinci_nand_attach_chip,
+	.exec_op = davinci_nand_exec_op,
 };
 
 static int nand_davinci_probe(struct platform_device *pdev)
-- 
2.25.3


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

[PATCH 4/5] mtd: rawnand: davinci: 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/davinci_nand.c | 54 -----------------------------
 1 file changed, 54 deletions(-)

diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
index 296047884c6a..c629e1a6ed71 100644
--- a/drivers/mtd/nand/raw/davinci_nand.c
+++ b/drivers/mtd/nand/raw/davinci_nand.c
@@ -81,38 +81,6 @@ static inline void davinci_nand_writel(struct davinci_nand_info *info,
 
 /*----------------------------------------------------------------------*/
 
-/*
- * Access to hardware control lines:  ALE, CLE, secondary chipselect.
- */
-
-static void nand_davinci_hwcontrol(struct nand_chip *nand, int cmd,
-				   unsigned int ctrl)
-{
-	struct davinci_nand_info *info = to_davinci_nand(nand_to_mtd(nand));
-	void __iomem			*addr = info->current_cs;
-
-	if (ctrl & NAND_CTRL_CLE)
-		addr += info->mask_cle;
-	else if (ctrl & NAND_CTRL_ALE)
-		addr += info->mask_ale;
-
-	if (cmd != NAND_CMD_NONE)
-		iowrite8(cmd, addr);
-}
-
-static void nand_davinci_select_chip(struct nand_chip *nand, int chip)
-{
-	struct davinci_nand_info *info = to_davinci_nand(nand_to_mtd(nand));
-
-	info->current_cs = info->vaddr;
-
-	/* maybe kick in a second chipselect */
-	if (chip > 0)
-		info->current_cs += info->mask_chipsel;
-}
-
-/*----------------------------------------------------------------------*/
-
 /*
  * 1-bit hardware ECC ... context maintained for each core chipselect
  */
@@ -440,17 +408,6 @@ static void nand_davinci_write_buf(struct nand_chip *chip, const uint8_t *buf,
 		iowrite8_rep(info->current_cs, buf, len);
 }
 
-/*
- * Check hardware register for wait status. Returns 1 if device is ready,
- * 0 if it is still busy.
- */
-static int nand_davinci_dev_ready(struct nand_chip *chip)
-{
-	struct davinci_nand_info *info = to_davinci_nand(nand_to_mtd(chip));
-
-	return davinci_nand_readl(info, NANDFSR_OFFSET) & BIT(0);
-}
-
 /*----------------------------------------------------------------------*/
 
 /* An ECC layout for using 4-bit ECC with small-page flash, storing
@@ -813,9 +770,6 @@ static int nand_davinci_probe(struct platform_device *pdev)
 	mtd->dev.parent		= &pdev->dev;
 	nand_set_flash_node(&info->chip, pdev->dev.of_node);
 
-	info->chip.legacy.chip_delay	= 0;
-	info->chip.legacy.select_chip	= nand_davinci_select_chip;
-
 	/* options such as NAND_BBT_USE_FLASH */
 	info->chip.bbt_options	= pdata->bbt_options;
 	/* options such as 16-bit widths */
@@ -832,14 +786,6 @@ static int nand_davinci_probe(struct platform_device *pdev)
 	info->mask_ale		= pdata->mask_ale ? : MASK_ALE;
 	info->mask_cle		= pdata->mask_cle ? : MASK_CLE;
 
-	/* Set address of hardware control function */
-	info->chip.legacy.cmd_ctrl	= nand_davinci_hwcontrol;
-	info->chip.legacy.dev_ready	= nand_davinci_dev_ready;
-
-	/* Speed up buffer I/O */
-	info->chip.legacy.read_buf     = nand_davinci_read_buf;
-	info->chip.legacy.write_buf    = nand_davinci_write_buf;
-
 	/* Use board-specific ECC config */
 	info->chip.ecc.mode	= pdata->ecc_mode;
 
-- 
2.25.3


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

Re: [PATCH 4/5] mtd: rawnand: davinci: Get rid of the legacy interface implementation

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Fri,  1 May
2020 12:07:28 +0200:

> Now that exec_op() is implemented we can get rid of the legacy interface
> implementation.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

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

[PATCH 5/5] mtd: rawnand: davinci: Change the {read, write}_buf prototypes

[Boris Brezillon]

Change the {read,write}_buf() helper prototypes to pass a
davinci_nand_info object and match the types stored in nand_op_instr.

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

diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
index c629e1a6ed71..29a785d426d3 100644
--- a/drivers/mtd/nand/raw/davinci_nand.c
+++ b/drivers/mtd/nand/raw/davinci_nand.c
@@ -382,11 +382,9 @@ static int nand_davinci_correct_4bit(struct nand_chip *chip, u_char *data,
  * the two LSBs for NAND access ... so we can issue 32-bit reads/writes
  * and have that transparently morphed into multiple NAND operations.
  */
-static void nand_davinci_read_buf(struct nand_chip *chip, uint8_t *buf,
-				  int len)
+static void nand_davinci_read_buf(struct davinci_nand_info *info, void *buf,
+				  unsigned int len)
 {
-	struct davinci_nand_info *info = to_davinci_nand(nand_to_mtd(chip));
-
 	if ((0x03 & ((uintptr_t)buf)) == 0 && (0x03 & len) == 0)
 		ioread32_rep(info->current_cs, buf, len >> 2);
 	else if ((0x01 & ((uintptr_t)buf)) == 0 && (0x01 & len) == 0)
@@ -395,11 +393,9 @@ static void nand_davinci_read_buf(struct nand_chip *chip, uint8_t *buf,
 		ioread8_rep(info->current_cs, buf, len);
 }
 
-static void nand_davinci_write_buf(struct nand_chip *chip, const uint8_t *buf,
-				   int len)
+static void nand_davinci_write_buf(struct davinci_nand_info *info,
+				   const void *buf, unsigned int len)
 {
-	struct davinci_nand_info *info = to_davinci_nand(nand_to_mtd(chip));
-
 	if ((0x03 & ((uintptr_t)buf)) == 0 && (0x03 & len) == 0)
 		iowrite32_rep(info->current_cs, buf, len >> 2);
 	else if ((0x01 & ((uintptr_t)buf)) == 0 && (0x01 & len) == 0)
@@ -656,12 +652,12 @@ static int davinci_nand_exec_instr(struct davinci_nand_info *info,
 		break;
 
 	case NAND_OP_DATA_IN_INSTR:
-		nand_davinci_read_buf(&info->chip, instr->ctx.data.buf.in,
+		nand_davinci_read_buf(info, instr->ctx.data.buf.in,
 				      instr->ctx.data.len);
 		break;
 
 	case NAND_OP_DATA_OUT_INSTR:
-		nand_davinci_write_buf(&info->chip, instr->ctx.data.buf.out,
+		nand_davinci_write_buf(info, instr->ctx.data.buf.out,
 				       instr->ctx.data.len);
 		break;
 
-- 
2.25.3


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

Re: [PATCH 5/5] mtd: rawnand: davinci: Change the {read,write}_buf prototypes

[Miquel Raynal]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Fri,  1 May
2020 12:07:29 +0200:

> Change the {read,write}_buf() helper prototypes to pass a
> davinci_nand_info object and match the types stored in nand_op_instr.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

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

Re: [PATCH 0/5] mtd: rawnand: davinci: Convert to exec_op()

[Bartosz Golaszewski]

pt., 1 maj 2020 o 12:07 Boris Brezillon
<boris.brezillon@collabora.com> napisał(a):
>
> Hello,
>
> A bit of context to explain the motivation behind those conversions
> I've been sending for the last couple of 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 Davinci NAND controller driver to
> exec_op(), plus a bunch of minor improvements done along the way.
>
> Regards,
>
> Boris
>
> Boris Brezillon (5):
>   mtd: rawnand: davinci: Inherit from nand_controller
>   mtd: rawnand: davinci: Stop using nand_chip.legacy.IO_ADDR_{R,W}
>   mtd: rawnand: davinci: Implement exec_op()
>   mtd: rawnand: davinci: Get rid of the legacy interface implementation
>   mtd: rawnand: davinci: Change the {read,write}_buf prototypes
>
>  drivers/mtd/nand/raw/davinci_nand.c | 161 +++++++++++++++-------------
>  1 file changed, 85 insertions(+), 76 deletions(-)
>
> --
> 2.25.3
>

Hi Boris,

Thanks for doing this. Unfortunately this breaks NAND on da850-lcdk
with the following error message:

    nand: No NAND device found

I'm super busy this week and so I don't have the time to investigate
further, I can get back to it next week hopefully.

Bart

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

Re: [PATCH 0/5] mtd: rawnand: davinci: Convert to exec_op()

[Boris Brezillon]

On Mon, 4 May 2020 18:34:55 +0200
Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote:

> pt., 1 maj 2020 o 12:07 Boris Brezillon
> <boris.brezillon@collabora.com> napisał(a):
> >
> > Hello,
> >
> > A bit of context to explain the motivation behind those conversions
> > I've been sending for the last couple of 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 Davinci NAND controller driver to
> > exec_op(), plus a bunch of minor improvements done along the way.
> >
> > Regards,
> >
> > Boris
> >
> > Boris Brezillon (5):
> >   mtd: rawnand: davinci: Inherit from nand_controller
> >   mtd: rawnand: davinci: Stop using nand_chip.legacy.IO_ADDR_{R,W}
> >   mtd: rawnand: davinci: Implement exec_op()
> >   mtd: rawnand: davinci: Get rid of the legacy interface implementation
> >   mtd: rawnand: davinci: Change the {read,write}_buf prototypes
> >
> >  drivers/mtd/nand/raw/davinci_nand.c | 161 +++++++++++++++-------------
> >  1 file changed, 85 insertions(+), 76 deletions(-)
> >
> > --
> > 2.25.3
> >  
> 
> Hi Boris,
> 
> Thanks for doing this. Unfortunately this breaks NAND on da850-lcdk
> with the following error message:
> 
>     nand: No NAND device found
> 
> I'm super busy this week and so I don't have the time to investigate
> further, I can get back to it next week hopefully.

No worries. And let me know if you need any help to debug that.

Thanks!

Boris

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

Re: [PATCH 0/5] mtd: rawnand: davinci: Convert to exec_op()

[Boris Brezillon]

On Mon, 4 May 2020 18:34:55 +0200
Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote:

> pt., 1 maj 2020 o 12:07 Boris Brezillon
> <boris.brezillon@collabora.com> napisał(a):
> >
> > Hello,
> >
> > A bit of context to explain the motivation behind those conversions
> > I've been sending for the last couple of 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 Davinci NAND controller driver to
> > exec_op(), plus a bunch of minor improvements done along the way.
> >
> > Regards,
> >
> > Boris
> >
> > Boris Brezillon (5):
> >   mtd: rawnand: davinci: Inherit from nand_controller
> >   mtd: rawnand: davinci: Stop using nand_chip.legacy.IO_ADDR_{R,W}
> >   mtd: rawnand: davinci: Implement exec_op()
> >   mtd: rawnand: davinci: Get rid of the legacy interface implementation
> >   mtd: rawnand: davinci: Change the {read,write}_buf prototypes
> >
> >  drivers/mtd/nand/raw/davinci_nand.c | 161 +++++++++++++++-------------
> >  1 file changed, 85 insertions(+), 76 deletions(-)
> >
> > --
> > 2.25.3
> >  
> 
> Hi Boris,
> 
> Thanks for doing this. Unfortunately this breaks NAND on da850-lcdk
> with the following error message:
> 
>     nand: No NAND device found

I had a second look and the below diff should fix the detection (you
can also find those changes in my exec-op-conversion branch [1]).

[1]https://github.com/bbrezillon/linux/tree/nand/exec-op-conversion

--->8---
diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
index 296047884c6a..0eeb30c7fc4e 100644
--- a/drivers/mtd/nand/raw/davinci_nand.c
+++ b/drivers/mtd/nand/raw/davinci_nand.c
@@ -678,6 +678,33 @@ static int davinci_nand_attach_chip(struct nand_chip *chip)
        return ret;
 }
 
+static void nand_davinci_data_in(struct davinci_nand_info *info, void *buf,
+                                unsigned int len, bool force_8bit)
+{
+       u32 alignment = ((uintptr_t)buf | len) & 3;
+
+       if (force_8bit || (alignment & 1))
+               ioread8_rep(info->current_cs, buf, len);
+       else if (alignment & 3)
+               ioread16_rep(info->current_cs, buf, len >> 1);
+       else
+               ioread32_rep(info->current_cs, buf, len >> 2);
+}
+
+static void nand_davinci_data_out(struct davinci_nand_info *info,
+                                 const void *buf, unsigned int len,
+                                 bool force_8bit)
+{
+       u32 alignment = ((uintptr_t)buf | len) & 3;
+
+       if (force_8bit || (alignment & 1))
+               iowrite8_rep(info->current_cs, buf, len);
+       else if (alignment & 3)
+               iowrite16_rep(info->current_cs, buf, len >> 1);
+       else
+               iowrite32_rep(info->current_cs, buf, len >> 2);
+}
+
 static int davinci_nand_exec_instr(struct davinci_nand_info *info,
                                   const struct nand_op_instr *instr)
 {
@@ -699,13 +726,15 @@ static int davinci_nand_exec_instr(struct davinci_nand_info *info,
                break;
 
        case NAND_OP_DATA_IN_INSTR:
-               nand_davinci_read_buf(&info->chip, instr->ctx.data.buf.in,
-                                     instr->ctx.data.len);
+               nand_davinci_data_in(info, instr->ctx.data.buf.in,
+                                    instr->ctx.data.len,
+                                    instr->ctx.data.force_8bit);
                break;
 
        case NAND_OP_DATA_OUT_INSTR:
-               nand_davinci_write_buf(&info->chip, instr->ctx.data.buf.out,
-                                      instr->ctx.data.len);
+               nand_davinci_data_out(info, instr->ctx.data.buf.out,
+                                     instr->ctx.data.len,
+                                     instr->ctx.data.force_8bit);
                break;
 
        case NAND_OP_WAITRDY_INSTR:
@@ -731,20 +760,21 @@ static int davinci_nand_exec_op(struct nand_chip *chip,
 {
        struct davinci_nand_info *info = to_davinci_nand(nand_to_mtd(chip));
        unsigned int i;
-       int ret;
 
        if (check_only)
-               return true;
+               return 0;
 
        info->current_cs = info->vaddr + (op->cs * info->mask_chipsel);
 
        for (i = 0; i < op->ninstrs; i++) {
+               int ret;
+
                ret = davinci_nand_exec_instr(info, &op->instrs[i]);
                if (ret)
-                       break;
+                       return ret;
        }
 
-       return ret;
+       return 0;
 }
 
 static const struct nand_controller_ops davinci_nand_controller_ops = {


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

Re: [PATCH 0/5] mtd: rawnand: davinci: Convert to exec_op()

[Bartosz Golaszewski]

sob., 9 maj 2020 o 16:13 Boris Brezillon
<boris.brezillon@collabora.com> napisał(a):
>
> [snip]
>
> >
> > Hi Boris,
> >
> > Thanks for doing this. Unfortunately this breaks NAND on da850-lcdk
> > with the following error message:
> >
> >     nand: No NAND device found
>
> I had a second look and the below diff should fix the detection (you
> can also find those changes in my exec-op-conversion branch [1]).
>
> [1]https://github.com/bbrezillon/linux/tree/nand/exec-op-conversion
>

Hi Boris,

Yes, this branch works now. Do you want to send out a v2? I'm happy to
add a Tested-by to it.

Bart

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