[PATCH 0/6] MTD: lantiq: xway: various nand fixes

[PATCH 0/6] MTD: lantiq: xway: various nand fixes

[John Crispin]

This series adds various fixes for the nand driver used on Lantiq DSL SoC.
These patches have accumulated in openwrt over the last 2 years and the
names of the original contributors got lost over the various rebases. If it
is required to find out all the names and emails, then I can dig into the
mailing list archive and try to track them down.

John Crispin (6):
  MTD: lantiq: xway: fix invalid operator
  MTD: lantiq: xway: the latched command should be persistent
  MTD: lantiq: xway: remove endless loop
  MTD: lantiq: xway: add missing write_buf and read_buf to nand driver
  MTD: lantiq: xway: fix nand locking
  MTD: lantiq: xway: allow request line masking

 drivers/mtd/nand/xway_nand.c |  118 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 103 insertions(+), 15 deletions(-)

-- 
1.7.10.4

[PATCH 1/6] MTD: lantiq: xway: fix invalid operator

[John Crispin]

xway_read_byte should use a logic or and not an add operator when working
out the nand address.

Signed-off-by: John Crispin <blogic@openwrt.org>
---
 drivers/mtd/nand/xway_nand.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index 3b28db4..81ec685 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -124,7 +124,7 @@ static unsigned char xway_read_byte(struct mtd_info *mtd)
 	int ret;
 
 	spin_lock_irqsave(&ebu_lock, flags);
-	ret = ltq_r8((void __iomem *)(nandaddr + NAND_READ_DATA));
+	ret = ltq_r8((void __iomem *)(nandaddr | NAND_READ_DATA));
 	spin_unlock_irqrestore(&ebu_lock, flags);
 
 	return ret;
-- 
1.7.10.4

Re: [PATCH 1/6] MTD: lantiq: xway: fix invalid operator

[Brian Norris]

On Mon, Jan 04, 2016 at 10:04:56PM +0100, John Crispin wrote:
> xway_read_byte should use a logic or and not an add operator when working
> out the nand address.

Why? It looks like a typical base address + offset use case. Or am I
missing something?

It would help if there was some kind of documentation, like the missing
DT doc that I mentioned on the cover letter, so I can know what the IO
mem range is supposed to be.

> Signed-off-by: John Crispin <blogic@openwrt.org>
> ---
>  drivers/mtd/nand/xway_nand.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
> index 3b28db4..81ec685 100644
> --- a/drivers/mtd/nand/xway_nand.c
> +++ b/drivers/mtd/nand/xway_nand.c
> @@ -124,7 +124,7 @@ static unsigned char xway_read_byte(struct mtd_info *mtd)
>  	int ret;
>  
>  	spin_lock_irqsave(&ebu_lock, flags);
> -	ret = ltq_r8((void __iomem *)(nandaddr + NAND_READ_DATA));
> +	ret = ltq_r8((void __iomem *)(nandaddr | NAND_READ_DATA));

This looks like odd code anyway; why all the casting? We have:

  void __iomem * --> unsigned long --> void __iomem *

Brian

>  	spin_unlock_irqrestore(&ebu_lock, flags);
>  
>  	return ret;
> -- 
> 1.7.10.4
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/6] MTD: lantiq: xway: the latched command should be persistent

[John Crispin]

Without this the addresses used will be bad.

Signed-off-by: John Crispin <blogic@openwrt.org>
---
 drivers/mtd/nand/xway_nand.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index 81ec685..548a2b8 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -54,6 +54,8 @@
 #define NAND_CON_CSMUX		(1 << 1)
 #define NAND_CON_NANDM		1
 
+static u32 xway_latchcmd;
+
 static void xway_reset_chip(struct nand_chip *chip)
 {
 	unsigned long nandaddr = (unsigned long) chip->IO_ADDR_W;
@@ -94,17 +96,15 @@ static void xway_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 	unsigned long flags;
 
 	if (ctrl & NAND_CTRL_CHANGE) {
-		nandaddr &= ~(NAND_WRITE_CMD | NAND_WRITE_ADDR);
 		if (ctrl & NAND_CLE)
-			nandaddr |= NAND_WRITE_CMD;
-		else
-			nandaddr |= NAND_WRITE_ADDR;
-		this->IO_ADDR_W = (void __iomem *) nandaddr;
+			xway_latchcmd = NAND_WRITE_CMD;
+		else if (ctrl & NAND_ALE)
+			xway_latchcmd = NAND_WRITE_ADDR;
 	}
 
 	if (cmd != NAND_CMD_NONE) {
 		spin_lock_irqsave(&ebu_lock, flags);
-		writeb(cmd, this->IO_ADDR_W);
+		writeb(cmd, (void __iomem *) (nandaddr | xway_latchcmd));
 		while ((ltq_ebu_r32(EBU_NAND_WAIT) & NAND_WAIT_WR_C) == 0)
 			;
 		spin_unlock_irqrestore(&ebu_lock, flags);
-- 
1.7.10.4

[PATCH 3/6] MTD: lantiq: xway: remove endless loop

[John Crispin]

The reset loop logic could run into a endless loop. Lets fix it as
requested.

http://lists.infradead.org/pipermail/linux-mtd/2012-September/044240.html

Signed-off-by: John Crispin <blogic@openwrt.org>
---
 drivers/mtd/nand/xway_nand.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index 548a2b8..c75a2bc 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -59,16 +59,22 @@ static u32 xway_latchcmd;
 static void xway_reset_chip(struct nand_chip *chip)
 {
 	unsigned long nandaddr = (unsigned long) chip->IO_ADDR_W;
+	unsigned long timeout;
 	unsigned long flags;
 
 	nandaddr &= ~NAND_WRITE_ADDR;
 	nandaddr |= NAND_WRITE_CMD;
 
 	/* finish with a reset */
+	timeout = jiffies + msecs_to_jiffies(20);
+
 	spin_lock_irqsave(&ebu_lock, flags);
 	writeb(NAND_WRITE_CMD_RESET, (void __iomem *) nandaddr);
-	while ((ltq_ebu_r32(EBU_NAND_WAIT) & NAND_WAIT_WR_C) == 0)
-		;
+	do {
+		if ((ltq_ebu_r32(EBU_NAND_WAIT) & NAND_WAIT_WR_C) == 0)
+			break;
+		cond_resched();
+	} while (!time_after_eq(jiffies, timeout));
 	spin_unlock_irqrestore(&ebu_lock, flags);
 }
 
-- 
1.7.10.4

Re: [PATCH 3/6] MTD: lantiq: xway: remove endless loop

[Brian Norris]

On Mon, Jan 04, 2016 at 10:04:58PM +0100, John Crispin wrote:
> The reset loop logic could run into a endless loop. Lets fix it as
> requested.
> 
> http://lists.infradead.org/pipermail/linux-mtd/2012-September/044240.html

Ah, 3 to 4 years late on the follow up; nice :) Better late than never,
I guess...

[PATCH 4/6] MTD: lantiq: xway: add missing write_buf and read_buf to nand driver

[John Crispin]

Signed-off-by: John Crispin <blogic@openwrt.org>
---
 drivers/mtd/nand/xway_nand.c |   28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index c75a2bc..ef7c7ce 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -136,6 +136,32 @@ static unsigned char xway_read_byte(struct mtd_info *mtd)
 	return ret;
 }
 
+static void xway_read_buf(struct mtd_info *mtd, u_char *buf, int len)
+{
+	struct nand_chip *this = mtd->priv;
+	unsigned long nandaddr = (unsigned long) this->IO_ADDR_R;
+	unsigned long flags;
+	int i;
+
+	spin_lock_irqsave(&ebu_lock, flags);
+	for (i = 0; i < len; i++)
+		buf[i] = ltq_r8((void __iomem *)(nandaddr | NAND_READ_DATA));
+	spin_unlock_irqrestore(&ebu_lock, flags);
+}
+
+static void xway_write_buf(struct mtd_info *mtd, const u_char *buf, int len)
+{
+	struct nand_chip *this = mtd->priv;
+	unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
+	unsigned long flags;
+	int i;
+
+	spin_lock_irqsave(&ebu_lock, flags);
+	for (i = 0; i < len; i++)
+		ltq_w8(buf[i], (void __iomem *)(nandaddr | NAND_WRITE_DATA));
+	spin_unlock_irqrestore(&ebu_lock, flags);
+}
+
 static int xway_nand_probe(struct platform_device *pdev)
 {
 	struct nand_chip *this = platform_get_drvdata(pdev);
@@ -177,6 +203,8 @@ static struct platform_nand_data xway_nand_data = {
 		.dev_ready	= xway_dev_ready,
 		.select_chip	= xway_select_chip,
 		.read_byte	= xway_read_byte,
+		.read_buf	= xway_read_buf,
+		.write_buf	= xway_write_buf,
 	}
 };
 
-- 
1.7.10.4

Re: [PATCH 4/6] MTD: lantiq: xway: add missing write_buf and read_buf to nand driver

[Brian Norris]

On Mon, Jan 04, 2016 at 10:04:59PM +0100, John Crispin wrote:
> Signed-off-by: John Crispin <blogic@openwrt.org>

Patch description? Why do we need this?

Re: [PATCH 4/6] MTD: lantiq: xway: add missing write_buf and read_buf to nand driver

[Brian Norris]

On Mon, Jan 04, 2016 at 10:04:59PM +0100, John Crispin wrote:
> Signed-off-by: John Crispin <blogic@openwrt.org>
> ---
>  drivers/mtd/nand/xway_nand.c |   28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
> index c75a2bc..ef7c7ce 100644
> --- a/drivers/mtd/nand/xway_nand.c
> +++ b/drivers/mtd/nand/xway_nand.c
> @@ -136,6 +136,32 @@ static unsigned char xway_read_byte(struct mtd_info *mtd)
>  	return ret;
>  }
>  
> +static void xway_read_buf(struct mtd_info *mtd, u_char *buf, int len)
> +{
> +	struct nand_chip *this = mtd->priv;

This is no longer legal. Use mtd_to_nand().

And please rebase (and test) your entire series on l2-mtd.git. It
doesn't apply:

http://linux-mtd.infradead.org/source.html

Thanks,
Brian

> +	unsigned long nandaddr = (unsigned long) this->IO_ADDR_R;
> +	unsigned long flags;
> +	int i;
> +
> +	spin_lock_irqsave(&ebu_lock, flags);
> +	for (i = 0; i < len; i++)
> +		buf[i] = ltq_r8((void __iomem *)(nandaddr | NAND_READ_DATA));
> +	spin_unlock_irqrestore(&ebu_lock, flags);
> +}
> +
> +static void xway_write_buf(struct mtd_info *mtd, const u_char *buf, int len)
> +{
> +	struct nand_chip *this = mtd->priv;
> +	unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
> +	unsigned long flags;
> +	int i;
> +
> +	spin_lock_irqsave(&ebu_lock, flags);
> +	for (i = 0; i < len; i++)
> +		ltq_w8(buf[i], (void __iomem *)(nandaddr | NAND_WRITE_DATA));
> +	spin_unlock_irqrestore(&ebu_lock, flags);
> +}
> +
>  static int xway_nand_probe(struct platform_device *pdev)
>  {
>  	struct nand_chip *this = platform_get_drvdata(pdev);
> @@ -177,6 +203,8 @@ static struct platform_nand_data xway_nand_data = {
>  		.dev_ready	= xway_dev_ready,
>  		.select_chip	= xway_select_chip,
>  		.read_byte	= xway_read_byte,
> +		.read_buf	= xway_read_buf,
> +		.write_buf	= xway_write_buf,
>  	}
>  };
>  
> -- 
> 1.7.10.4
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 5/6] MTD: lantiq: xway: fix nand locking

[John Crispin]

use a global lock in the cs function.

Signed-off-by: John Crispin <blogic@openwrt.org>
---
 drivers/mtd/nand/xway_nand.c |   15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index ef7c7ce..4127049 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -80,13 +80,16 @@ static void xway_reset_chip(struct nand_chip *chip)
 
 static void xway_select_chip(struct mtd_info *mtd, int chip)
 {
+	static unsigned long csflags;
 
 	switch (chip) {
 	case -1:
 		ltq_ebu_w32_mask(NAND_CON_CE, 0, EBU_NAND_CON);
 		ltq_ebu_w32_mask(NAND_CON_NANDM, 0, EBU_NAND_CON);
+		spin_unlock_irqrestore(&ebu_lock, csflags);
 		break;
 	case 0:
+		spin_lock_irqsave(&ebu_lock, csflags);
 		ltq_ebu_w32_mask(0, NAND_CON_NANDM, EBU_NAND_CON);
 		ltq_ebu_w32_mask(0, NAND_CON_CE, EBU_NAND_CON);
 		break;
@@ -99,7 +102,6 @@ static void xway_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 {
 	struct nand_chip *this = mtd->priv;
 	unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
-	unsigned long flags;
 
 	if (ctrl & NAND_CTRL_CHANGE) {
 		if (ctrl & NAND_CLE)
@@ -109,11 +111,9 @@ static void xway_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 	}
 
 	if (cmd != NAND_CMD_NONE) {
-		spin_lock_irqsave(&ebu_lock, flags);
 		writeb(cmd, (void __iomem *) (nandaddr | xway_latchcmd));
 		while ((ltq_ebu_r32(EBU_NAND_WAIT) & NAND_WAIT_WR_C) == 0)
 			;
-		spin_unlock_irqrestore(&ebu_lock, flags);
 	}
 }
 
@@ -126,12 +126,9 @@ static unsigned char xway_read_byte(struct mtd_info *mtd)
 {
 	struct nand_chip *this = mtd->priv;
 	unsigned long nandaddr = (unsigned long) this->IO_ADDR_R;
-	unsigned long flags;
 	int ret;
 
-	spin_lock_irqsave(&ebu_lock, flags);
 	ret = ltq_r8((void __iomem *)(nandaddr | NAND_READ_DATA));
-	spin_unlock_irqrestore(&ebu_lock, flags);
 
 	return ret;
 }
@@ -140,26 +137,20 @@ static void xway_read_buf(struct mtd_info *mtd, u_char *buf, int len)
 {
 	struct nand_chip *this = mtd->priv;
 	unsigned long nandaddr = (unsigned long) this->IO_ADDR_R;
-	unsigned long flags;
 	int i;
 
-	spin_lock_irqsave(&ebu_lock, flags);
 	for (i = 0; i < len; i++)
 		buf[i] = ltq_r8((void __iomem *)(nandaddr | NAND_READ_DATA));
-	spin_unlock_irqrestore(&ebu_lock, flags);
 }
 
 static void xway_write_buf(struct mtd_info *mtd, const u_char *buf, int len)
 {
 	struct nand_chip *this = mtd->priv;
 	unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
-	unsigned long flags;
 	int i;
 
-	spin_lock_irqsave(&ebu_lock, flags);
 	for (i = 0; i < len; i++)
 		ltq_w8(buf[i], (void __iomem *)(nandaddr | NAND_WRITE_DATA));
-	spin_unlock_irqrestore(&ebu_lock, flags);
 }
 
 static int xway_nand_probe(struct platform_device *pdev)
-- 
1.7.10.4

Re: [PATCH 5/6] MTD: lantiq: xway: fix nand locking

[Brian Norris]

On Mon, Jan 04, 2016 at 10:05:00PM +0100, John Crispin wrote:
> use a global lock in the cs function.
> 
> Signed-off-by: John Crispin <blogic@openwrt.org>

Again, patch description? Why would you need this change?

[PATCH 6/6] MTD: lantiq: xway: allow request line masking

[John Crispin]

There are several request lines. Allow masking the unused ones via OF.

Signed-off-by: John Crispin <blogic@openwrt.org>
---
 drivers/mtd/nand/xway_nand.c |   63 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index 4127049..c253720 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -54,8 +54,27 @@
 #define NAND_CON_CSMUX		(1 << 1)
 #define NAND_CON_NANDM		1
 
+#define DANUBE_PCI_REG32(addr)	(*(volatile u32 *)(addr))
+#define PCI_CR_PR_OFFSET	(KSEG1+0x1E105400)
+#define PCI_CR_PC_ARB		(PCI_CR_PR_OFFSET + 0x0080)
+
 static u32 xway_latchcmd;
 
+/*
+ * req_mask provides a mechanism to prevent interference between
+ * nand and pci (probably only relevant for the BT Home Hub 2B).
+ * Setting it causes the corresponding pci req pins to be masked
+ * during nand access, and also moves ebu locking from the read/write
+ * functions to the chip select function to ensure that the whole
+ * operation runs with interrupts disabled.
+ * In addition it switches on some extra waiting in xway_cmd_ctrl().
+ * This seems to be necessary if the ebu_cs1 pin has open-drain disabled,
+ * which in turn seems to be necessary for the nor chip to be recognised
+ * reliably, on a board (Home Hub 2B again) which has both nor and nand.
+ */
+
+static __be32 req_mask;
+
 static void xway_reset_chip(struct nand_chip *chip)
 {
 	unsigned long nandaddr = (unsigned long) chip->IO_ADDR_W;
@@ -86,12 +105,24 @@ static void xway_select_chip(struct mtd_info *mtd, int chip)
 	case -1:
 		ltq_ebu_w32_mask(NAND_CON_CE, 0, EBU_NAND_CON);
 		ltq_ebu_w32_mask(NAND_CON_NANDM, 0, EBU_NAND_CON);
+
+		if (req_mask) {
+			/* Unmask all external PCI request */
+			DANUBE_PCI_REG32(PCI_CR_PC_ARB) &= ~(req_mask << 16);
+		}
 		spin_unlock_irqrestore(&ebu_lock, csflags);
+
 		break;
 	case 0:
 		spin_lock_irqsave(&ebu_lock, csflags);
+		if (req_mask) {
+			/* Mask all external PCI request */
+			DANUBE_PCI_REG32(PCI_CR_PC_ARB) |= (req_mask << 16);
+		}
+
 		ltq_ebu_w32_mask(0, NAND_CON_NANDM, EBU_NAND_CON);
 		ltq_ebu_w32_mask(0, NAND_CON_CE, EBU_NAND_CON);
+
 		break;
 	default:
 		BUG();
@@ -103,6 +134,12 @@ static void xway_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 	struct nand_chip *this = mtd->priv;
 	unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
 
+	if (req_mask) {
+		if (cmd != NAND_CMD_STATUS)
+			ltq_ebu_w32(EBU_NAND_WAIT, 0); /* Clear nand ready */
+	}
+
+
 	if (ctrl & NAND_CTRL_CHANGE) {
 		if (ctrl & NAND_CLE)
 			xway_latchcmd = NAND_WRITE_CMD;
@@ -115,6 +152,24 @@ static void xway_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 		while ((ltq_ebu_r32(EBU_NAND_WAIT) & NAND_WAIT_WR_C) == 0)
 			;
 	}
+
+	if (req_mask) {
+	       /*
+		* program and erase have their own busy handlers
+		* status and sequential in needs no delay
+		*/
+		switch (cmd) {
+		case NAND_CMD_ERASE1:
+		case NAND_CMD_SEQIN:
+		case NAND_CMD_STATUS:
+		case NAND_CMD_READID:
+		return;
+		}
+
+		/* wait until command is processed */
+		while ((ltq_ebu_r32(EBU_NAND_WAIT) & NAND_WAIT_RD) == 0)
+			;
+	}
 }
 
 static int xway_dev_ready(struct mtd_info *mtd)
@@ -157,6 +212,8 @@ static int xway_nand_probe(struct platform_device *pdev)
 {
 	struct nand_chip *this = platform_get_drvdata(pdev);
 	unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
+	const __be32 *req_mask_ptr = of_get_property(pdev->dev.of_node,
+					"req-mask", NULL);
 	const __be32 *cs = of_get_property(pdev->dev.of_node,
 					"lantiq,cs", NULL);
 	u32 cs_flag = 0;
@@ -165,6 +222,12 @@ static int xway_nand_probe(struct platform_device *pdev)
 	if (cs && (*cs == 1))
 		cs_flag = NAND_CON_IN_CS1 | NAND_CON_OUT_CS1;
 
+	/*
+	 * Load the PCI req lines to mask from the device tree. If the
+	 * property is not present, setting req_mask to 0 disables masking.
+	 */
+	req_mask = (req_mask_ptr ? *req_mask_ptr : 0);
+
 	/* setup the EBU to run in NAND mode on our base addr */
 	ltq_ebu_w32(CPHYSADDR(nandaddr)
 		| ADDSEL1_MASK(3) | ADDSEL1_REGEN, EBU_ADDSEL1);
-- 
1.7.10.4

Re: [PATCH 6/6] MTD: lantiq: xway: allow request line masking

[Brian Norris]

On Mon, Jan 04, 2016 at 10:05:01PM +0100, John Crispin wrote:
> There are several request lines. Allow masking the unused ones via OF.
> 
> Signed-off-by: John Crispin <blogic@openwrt.org>

There's no DT binding doc in the first place, so it's hard to add
properties. Please add a binding doc, then document your additions
properly.

Thanks,
Brian

Re: [PATCH 0/6] MTD: lantiq: xway: various nand fixes

[Brian Norris]

On Mon, Jan 04, 2016 at 10:04:55PM +0100, John Crispin wrote:
> This series adds various fixes for the nand driver used on Lantiq DSL SoC.

Is there a good reason this driver uses plat_nand? That seems like an
unnecessary abstraction layer. It'd be clearer to just refactor the
driver to be a proper platform driver...

Also, I see that there's no DT binding doc.

Can these things be cleaned up?

Brian

Re: [PATCH 0/6] MTD: lantiq: xway: various nand fixes

[John Crispin]

On 05/01/2016 00:30, Brian Norris wrote:
> On Mon, Jan 04, 2016 at 10:04:55PM +0100, John Crispin wrote:
>> This series adds various fixes for the nand driver used on Lantiq DSL SoC.
> 
> Is there a good reason this driver uses plat_nand? That seems like an
> unnecessary abstraction layer. It'd be clearer to just refactor the
> driver to be a proper platform driver...
> 
> Also, I see that there's no DT binding doc.
> 
> Can these things be cleaned up?
> 
> Brian
> 


grml, i was kinda hoping this would be a no brainer. the problem is that
i don't even have the HW so refactoring / big changes are a no go or
would require me to find users with the HW first.

i'll try to get this resolved for v4.6.

	John

Re: [PATCH 0/6] MTD: lantiq: xway: various nand fixes

[Brian Norris]

On Tue, Jan 05, 2016 at 08:34:04AM +0100, John Crispin wrote:
> On 05/01/2016 00:30, Brian Norris wrote:
> > Is there a good reason this driver uses plat_nand? That seems like an
> > unnecessary abstraction layer. It'd be clearer to just refactor the
> > driver to be a proper platform driver...
> > 
> > Also, I see that there's no DT binding doc.
> > 
> > Can these things be cleaned up?
> 
> grml, i was kinda hoping this would be a no brainer. the problem is that
> i don't even have the HW so refactoring / big changes are a no go or
> would require me to find users with the HW first.

Well, I could be convinced to take patches that are a little better
documented (i.e., have a little better commit descriptions), if they
fix real issues. If you're going to add device tree properties, though,
you need a DT binding doc. And the refactoring to a platform driver
should be pretty trivial, but it's not an absolute blocking requirement,
since the driver's already in mainline.

BTW, one issue with the current driver, if you're going to add a DT
binding doc: you currently require the "gen_nand" string, like this, in
your openwrt DTS(I) files:

	nand-parts@0 {
		compatible = "gen_nand", "lantiq,nand-xway";
		...
	};

That's not an acceptable binding, and that's where refactoring the
driver would help too.

> i'll try to get this resolved for v4.6.

Brian

Re: [PATCH 0/6] MTD: lantiq: xway: various nand fixes

[John Crispin]

On 05/01/2016 18:53, Brian Norris wrote:
> On Tue, Jan 05, 2016 at 08:34:04AM +0100, John Crispin wrote:
>> On 05/01/2016 00:30, Brian Norris wrote:
>>> Is there a good reason this driver uses plat_nand? That seems like an
>>> unnecessary abstraction layer. It'd be clearer to just refactor the
>>> driver to be a proper platform driver...
>>>
>>> Also, I see that there's no DT binding doc.
>>>
>>> Can these things be cleaned up?
>>
>> grml, i was kinda hoping this would be a no brainer. the problem is that
>> i don't even have the HW so refactoring / big changes are a no go or
>> would require me to find users with the HW first.
> 
> Well, I could be convinced to take patches that are a little better
> documented (i.e., have a little better commit descriptions), if they
> fix real issues. If you're going to add device tree properties, though,
> you need a DT binding doc. And the refactoring to a platform driver
> should be pretty trivial, but it's not an absolute blocking requirement,
> since the driver's already in mainline.
> 
> BTW, one issue with the current driver, if you're going to add a DT
> binding doc: you currently require the "gen_nand" string, like this, in
> your openwrt DTS(I) files:
> 
> 	nand-parts@0 {
> 		compatible = "gen_nand", "lantiq,nand-xway";
> 		...
> 	};
> 
> That's not an acceptable binding, and that's where refactoring the
> driver would help too.
> 
>> i'll try to get this resolved for v4.6.
> 
> Brian
> 
Hi Brian,

would you take this stuff around rc2-3 or would that be too late ? i
already manage to track down one of the original authors and he agreed
to help testing.

	John

Re: [PATCH 0/6] MTD: lantiq: xway: various nand fixes

[Brian Norris]

On Tue, Jan 05, 2016 at 07:44:27PM +0100, John Crispin wrote:
> >> i'll try to get this resolved for v4.6.
> 
> would you take this stuff around rc2-3 or would that be too late ? i
> already manage to track down one of the original authors and he agreed
> to help testing.

Fixes, probably. Refactoring and new features, probably not. If you
prioritize the fixes first in the series, then that leaves me the most
flexibility.

Regards,
Brian