[PATCH v3 0/8] MTD: xway: fix driver

[PATCH v3 0/8] MTD: xway: fix driver

[Hauke Mehrtens]

Without these patches the driver does not work for me.
Some of these patches are in OpenWrt for years now and should go 
upstream. In addition this converts it from some hack with the 
plat_nand driver to a normal platform driver.

I will try to convert the spinlock to a mutex in a separate patch 
because this affects other parts of the kernel. 

changes since:
v2:
 - reorder patches
 - use of_property_read_u32()
 - rework error handling in xway_nand_probe()
 - simplyfy latched command handling
 - remove xway_reset_chip()
 - remove cast of nandaddr
 - add nandaddr to xway_nand_data
 - add csflags to xway_nand_data

v1:
 - convert to normal platform driver
 - do not use global variable xway_latchcmd
 - use mtd_to_nand()


Hauke Mehrtens (6):
  MTD: xway: add some more documentation
  MTD: xway: convert to normal platform driver
  MTD: xway: use generic reset function
  MTD: xway: extract read and write function
  MTD: xway: add nandaddr to own struct
  MTD: xway: add missing write_buf and read_buf to nand driver

John Crispin (2):
  MTD: xway: the latched command should be persistent
  MTD: xway: fix nand locking

 drivers/mtd/nand/Kconfig     |   1 -
 drivers/mtd/nand/xway_nand.c | 224 +++++++++++++++++++++++++++----------------
 2 files changed, 140 insertions(+), 85 deletions(-)

-- 
2.8.1

[PATCH v3 1/8] MTD: xway: add some more documentation

[Hauke Mehrtens]

This adds some register documentation which should make it easier to
understand how this controller works.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/xway_nand.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index 0cf0ac0..2d74637 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -16,20 +16,29 @@
 #define EBU_ADDSEL1		0x24
 #define EBU_NAND_CON		0xB0
 #define EBU_NAND_WAIT		0xB4
+#define  NAND_WAIT_RD		BIT(0) /* NAND flash status output */
+#define  NAND_WAIT_WR_C		BIT(3) /* NAND Write/Read complete */
 #define EBU_NAND_ECC0		0xB8
 #define EBU_NAND_ECC_AC		0xBC
 
-/* nand commands */
-#define NAND_CMD_ALE		(1 << 2)
-#define NAND_CMD_CLE		(1 << 3)
-#define NAND_CMD_CS		(1 << 4)
+/*
+ * nand commands
+ * The pins of the NAND chip are selected based on the address bits of the
+ * "register" read and write. There are no special registers, but an
+ * address range and the lower address bits are used to activate the
+ * correct line. For example when the bit (1 << 2) is set in the address
+ * the ALE pin will be activated.
+ */
+#define NAND_CMD_ALE		BIT(2) /* address latch enable */
+#define NAND_CMD_CLE		BIT(3) /* command latch enable */
+#define NAND_CMD_CS		BIT(4) /* chip select */
+#define NAND_CMD_SE		BIT(5) /* spare area access latch */
+#define NAND_CMD_WP		BIT(6) /* write protect */
 #define NAND_WRITE_CMD_RESET	0xff
 #define NAND_WRITE_CMD		(NAND_CMD_CS | NAND_CMD_CLE)
 #define NAND_WRITE_ADDR		(NAND_CMD_CS | NAND_CMD_ALE)
 #define NAND_WRITE_DATA		(NAND_CMD_CS)
 #define NAND_READ_DATA		(NAND_CMD_CS)
-#define NAND_WAIT_WR_C		(1 << 3)
-#define NAND_WAIT_RD		(0x1)
 
 /* we need to tel the ebu which addr we mapped the nand to */
 #define ADDSEL1_MASK(x)		(x << 4)
-- 
2.8.1

Re: [PATCH v3 1/8] MTD: xway: add some more documentation

[Boris Brezillon]

On Sun, 19 Jun 2016 18:08:10 +0200
Hauke Mehrtens <hauke@hauke-m.de> wrote:

> This adds some register documentation which should make it easier to
> understand how this controller works.

This commits also adds some new definitions and replace (1 << x) by
BIT(x).
Can you update the commit description accordingly?

> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/mtd/nand/xway_nand.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
> index 0cf0ac0..2d74637 100644
> --- a/drivers/mtd/nand/xway_nand.c
> +++ b/drivers/mtd/nand/xway_nand.c
> @@ -16,20 +16,29 @@
>  #define EBU_ADDSEL1		0x24
>  #define EBU_NAND_CON		0xB0
>  #define EBU_NAND_WAIT		0xB4
> +#define  NAND_WAIT_RD		BIT(0) /* NAND flash status output */
> +#define  NAND_WAIT_WR_C		BIT(3) /* NAND Write/Read complete */
>  #define EBU_NAND_ECC0		0xB8
>  #define EBU_NAND_ECC_AC		0xBC
>  
> -/* nand commands */
> -#define NAND_CMD_ALE		(1 << 2)
> -#define NAND_CMD_CLE		(1 << 3)
> -#define NAND_CMD_CS		(1 << 4)
> +/*
> + * nand commands
> + * The pins of the NAND chip are selected based on the address bits of the
> + * "register" read and write. There are no special registers, but an
> + * address range and the lower address bits are used to activate the
> + * correct line. For example when the bit (1 << 2) is set in the address
> + * the ALE pin will be activated.
> + */
> +#define NAND_CMD_ALE		BIT(2) /* address latch enable */
> +#define NAND_CMD_CLE		BIT(3) /* command latch enable */
> +#define NAND_CMD_CS		BIT(4) /* chip select */
> +#define NAND_CMD_SE		BIT(5) /* spare area access latch */
> +#define NAND_CMD_WP		BIT(6) /* write protect */
>  #define NAND_WRITE_CMD_RESET	0xff
>  #define NAND_WRITE_CMD		(NAND_CMD_CS | NAND_CMD_CLE)
>  #define NAND_WRITE_ADDR		(NAND_CMD_CS | NAND_CMD_ALE)
>  #define NAND_WRITE_DATA		(NAND_CMD_CS)
>  #define NAND_READ_DATA		(NAND_CMD_CS)
> -#define NAND_WAIT_WR_C		(1 << 3)
> -#define NAND_WAIT_RD		(0x1)
>  
>  /* we need to tel the ebu which addr we mapped the nand to */
>  #define ADDSEL1_MASK(x)		(x << 4)

Re: [PATCH v3 1/8] MTD: xway: add some more documentation

[Hauke Mehrtens]

On 06/19/2016 06:14 PM, Boris Brezillon wrote:
> On Sun, 19 Jun 2016 18:08:10 +0200
> Hauke Mehrtens <hauke@hauke-m.de> wrote:
> 
>> This adds some register documentation which should make it easier to
>> understand how this controller works.
> 
> This commits also adds some new definitions and replace (1 << x) by
> BIT(x).
> Can you update the commit description accordingly?
> 

Ok, I will add it.

[PATCH v3 2/8] MTD: xway: convert to normal platform driver

[Hauke Mehrtens]

Instead of hacking this into the plat_nand driver just make this a
normal nand driver.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/Kconfig     |   1 -
 drivers/mtd/nand/xway_nand.c | 116 +++++++++++++++++++++++++++++--------------
 2 files changed, 80 insertions(+), 37 deletions(-)

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index f05e0e9..d7de2c97 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -539,7 +539,6 @@ config MTD_NAND_FSMC
 config MTD_NAND_XWAY
 	tristate "Support for NAND on Lantiq XWAY SoC"
 	depends on LANTIQ && SOC_TYPE_XWAY
-	select MTD_NAND_PLATFORM
 	help
 	  Enables support for NAND Flash chips on Lantiq XWAY SoCs. NAND is attached
 	  to the External Bus Unit (EBU).
diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index 2d74637..ec189e5 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -4,6 +4,7 @@
  *  by the Free Software Foundation.
  *
  *  Copyright © 2012 John Crispin <blogic@openwrt.org>
+ *  Copyright © 2016 Hauke Mehrtens <hauke@hauke-m.de>
  */
 
 #include <linux/mtd/nand.h>
@@ -63,6 +64,10 @@
 #define NAND_CON_CSMUX		(1 << 1)
 #define NAND_CON_NANDM		1
 
+struct xway_nand_data {
+	struct nand_chip	chip;
+};
+
 static void xway_reset_chip(struct nand_chip *chip)
 {
 	unsigned long nandaddr = (unsigned long) chip->IO_ADDR_W;
@@ -139,16 +144,51 @@ static unsigned char xway_read_byte(struct mtd_info *mtd)
 	return ret;
 }
 
+/*
+ * Probe for the NAND device.
+ */
 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 *cs = of_get_property(pdev->dev.of_node,
-					"lantiq,cs", NULL);
+	struct xway_nand_data *data;
+	struct mtd_info *mtd;
+	struct resource *res;
+	int err;
+	void __iomem *nandaddr;
+	u32 cs;
 	u32 cs_flag = 0;
 
+	/* Allocate memory for the device structure (and zero it) */
+	data = devm_kzalloc(&pdev->dev, sizeof(struct xway_nand_data),
+			    GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	nandaddr = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(nandaddr))
+		return PTR_ERR(nandaddr);
+
+	nand_set_flash_node(&data->chip, pdev->dev.of_node);
+	mtd = nand_to_mtd(&data->chip);
+	mtd->dev.parent = &pdev->dev;
+
+	data->chip.IO_ADDR_R = nandaddr;
+	data->chip.IO_ADDR_W = nandaddr;
+	data->chip.cmd_ctrl = xway_cmd_ctrl;
+	data->chip.dev_ready = xway_dev_ready;
+	data->chip.select_chip = xway_select_chip;
+	data->chip.read_byte = xway_read_byte;
+	data->chip.chip_delay = 30;
+
+	data->chip.ecc.mode = NAND_ECC_SOFT;
+	data->chip.ecc.algo = NAND_ECC_HAMMING;
+
+	platform_set_drvdata(pdev, data);
+	nand_set_controller_data(&data->chip, data);
+
 	/* load our CS from the DT. Either we find a valid 1 or default to 0 */
-	if (cs && (*cs == 1))
+	err = of_property_read_u32(pdev->dev.of_node, "lantiq,cs", &cs);
+	if (!err && cs == 1)
 		cs_flag = NAND_CON_IN_CS1 | NAND_CON_OUT_CS1;
 
 	/* setup the EBU to run in NAND mode on our base addr */
@@ -164,43 +204,47 @@ static int xway_nand_probe(struct platform_device *pdev)
 		| cs_flag, EBU_NAND_CON);
 
 	/* finish with a reset */
-	xway_reset_chip(this);
+	xway_reset_chip(&data->chip);
 
-	return 0;
-}
+	/* Scan to find existence of the device */
+	err = nand_scan(mtd, 1);
+	if (err)
+		return err;
 
-static struct platform_nand_data xway_nand_data = {
-	.chip = {
-		.nr_chips		= 1,
-		.chip_delay		= 30,
-	},
-	.ctrl = {
-		.probe		= xway_nand_probe,
-		.cmd_ctrl	= xway_cmd_ctrl,
-		.dev_ready	= xway_dev_ready,
-		.select_chip	= xway_select_chip,
-		.read_byte	= xway_read_byte,
-	}
-};
+	err = mtd_device_register(mtd, NULL, 0);
+	if (err)
+		nand_release(mtd);
+
+	return err;
+}
 
 /*
- * Try to find the node inside the DT. If it is available attach out
- * platform_nand_data
+ * Remove a NAND device.
  */
-static int __init xway_register_nand(void)
+static int xway_nand_remove(struct platform_device *pdev)
 {
-	struct device_node *node;
-	struct platform_device *pdev;
-
-	node = of_find_compatible_node(NULL, NULL, "lantiq,nand-xway");
-	if (!node)
-		return -ENOENT;
-	pdev = of_find_device_by_node(node);
-	if (!pdev)
-		return -EINVAL;
-	pdev->dev.platform_data = &xway_nand_data;
-	of_node_put(node);
+	struct xway_nand_data *data = platform_get_drvdata(pdev);
+
+	nand_release(nand_to_mtd(&data->chip));
+
 	return 0;
 }
 
-subsys_initcall(xway_register_nand);
+static const struct of_device_id xway_nand_match[] = {
+	{ .compatible = "lantiq,nand-xway" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, xway_nand_match);
+
+static struct platform_driver xway_nand_driver = {
+	.probe	= xway_nand_probe,
+	.remove	= xway_nand_remove,
+	.driver	= {
+		.name		= "lantiq,nand-xway",
+		.of_match_table = xway_nand_match,
+	},
+};
+
+module_platform_driver(xway_nand_driver);
+
+MODULE_LICENSE("GPL");
-- 
2.8.1

[PATCH v3 3/8] MTD: xway: the latched command should be persistent

[Hauke Mehrtens]

From: John Crispin <john@phrozen.org>

With each write the pins on the NAND flash chip have to be activated.
Instead of setting them only when NAND_CTRL_CHANGE is given, always set
them.

Signed-off-by: John Crispin <john@phrozen.org>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/xway_nand.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index ec189e5..6028edb 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -107,22 +107,18 @@ static void xway_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 	unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
 	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;
-	}
+	if (cmd == NAND_CMD_NONE)
+		return;
 
-	if (cmd != NAND_CMD_NONE) {
-		spin_lock_irqsave(&ebu_lock, flags);
-		writeb(cmd, this->IO_ADDR_W);
-		while ((ltq_ebu_r32(EBU_NAND_WAIT) & NAND_WAIT_WR_C) == 0)
-			;
-		spin_unlock_irqrestore(&ebu_lock, flags);
-	}
+	spin_lock_irqsave(&ebu_lock, flags);
+	if (ctrl & NAND_CLE)
+		writeb(cmd, (void __iomem *) (nandaddr | NAND_WRITE_CMD));
+	else if (ctrl & NAND_ALE)
+		writeb(cmd, (void __iomem *) (nandaddr | NAND_WRITE_ADDR));
+
+	while ((ltq_ebu_r32(EBU_NAND_WAIT) & NAND_WAIT_WR_C) == 0)
+		;
+	spin_unlock_irqrestore(&ebu_lock, flags);
 }
 
 static int xway_dev_ready(struct mtd_info *mtd)
-- 
2.8.1

Re: [PATCH v3 3/8] MTD: xway: the latched command should be persistent

[Boris Brezillon]

On Sun, 19 Jun 2016 18:08:12 +0200
Hauke Mehrtens <hauke@hauke-m.de> wrote:

> From: John Crispin <john@phrozen.org>
> 
> With each write the pins on the NAND flash chip have to be activated.
> Instead of setting them only when NAND_CTRL_CHANGE is given, always set
> them.

This commit message is a bit fuzzy. How about:

"
mtd: nand: xway: Avoid messing up with IO_ADDR_W in ->cmd_ctrl()

The ->cmd_ctrl() function is adjusting the ->IO_ADDR_W value depending
on the command type each time NAND_CTRL_CHANGE is passed. This is not
only useless but can lead to an ->IO_ADDR_W corruption.

Get rid of this logic and rely on the NAND_CLE and NAND_ALE flags to
deduce the iomem address to write the cmd argument to.
"

BTW, please use the "mtd: nand: <nand-controller-driver>: " prefix in
your subject line (this applies to all the patches you sent so far).

> 
> Signed-off-by: John Crispin <john@phrozen.org>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/mtd/nand/xway_nand.c | 26 +++++++++++---------------
>  1 file changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
> index ec189e5..6028edb 100644
> --- a/drivers/mtd/nand/xway_nand.c
> +++ b/drivers/mtd/nand/xway_nand.c
> @@ -107,22 +107,18 @@ static void xway_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
>  	unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
>  	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;
> -	}
> +	if (cmd == NAND_CMD_NONE)
> +		return;
>  
> -	if (cmd != NAND_CMD_NONE) {
> -		spin_lock_irqsave(&ebu_lock, flags);
> -		writeb(cmd, this->IO_ADDR_W);
> -		while ((ltq_ebu_r32(EBU_NAND_WAIT) & NAND_WAIT_WR_C) == 0)
> -			;
> -		spin_unlock_irqrestore(&ebu_lock, flags);
> -	}
> +	spin_lock_irqsave(&ebu_lock, flags);
> +	if (ctrl & NAND_CLE)
> +		writeb(cmd, (void __iomem *) (nandaddr | NAND_WRITE_CMD));
> +	else if (ctrl & NAND_ALE)
> +		writeb(cmd, (void __iomem *) (nandaddr | NAND_WRITE_ADDR));
> +
> +	while ((ltq_ebu_r32(EBU_NAND_WAIT) & NAND_WAIT_WR_C) == 0)
> +		;
> +	spin_unlock_irqrestore(&ebu_lock, flags);
>  }
>  
>  static int xway_dev_ready(struct mtd_info *mtd)

[PATCH v3 4/8] MTD: xway: use generic reset function

[Hauke Mehrtens]

Instead of writing an own reset function use the generic NAND reset
function. This way we can remove our own reset function.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/xway_nand.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index 6028edb..8a0019a 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -35,7 +35,6 @@
 #define NAND_CMD_CS		BIT(4) /* chip select */
 #define NAND_CMD_SE		BIT(5) /* spare area access latch */
 #define NAND_CMD_WP		BIT(6) /* write protect */
-#define NAND_WRITE_CMD_RESET	0xff
 #define NAND_WRITE_CMD		(NAND_CMD_CS | NAND_CMD_CLE)
 #define NAND_WRITE_ADDR		(NAND_CMD_CS | NAND_CMD_ALE)
 #define NAND_WRITE_DATA		(NAND_CMD_CS)
@@ -68,22 +67,6 @@ struct xway_nand_data {
 	struct nand_chip	chip;
 };
 
-static void xway_reset_chip(struct nand_chip *chip)
-{
-	unsigned long nandaddr = (unsigned long) chip->IO_ADDR_W;
-	unsigned long flags;
-
-	nandaddr &= ~NAND_WRITE_ADDR;
-	nandaddr |= NAND_WRITE_CMD;
-
-	/* finish with a reset */
-	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)
-		;
-	spin_unlock_irqrestore(&ebu_lock, flags);
-}
-
 static void xway_select_chip(struct mtd_info *mtd, int chip)
 {
 
@@ -199,14 +182,14 @@ static int xway_nand_probe(struct platform_device *pdev)
 		| NAND_CON_SE_P | NAND_CON_WP_P | NAND_CON_PRE_P
 		| cs_flag, EBU_NAND_CON);
 
-	/* finish with a reset */
-	xway_reset_chip(&data->chip);
-
 	/* Scan to find existence of the device */
 	err = nand_scan(mtd, 1);
 	if (err)
 		return err;
 
+	/* finish with a reset */
+	data->chip.cmdfunc(mtd, NAND_CMD_RESET, -1, -1);
+
 	err = mtd_device_register(mtd, NULL, 0);
 	if (err)
 		nand_release(mtd);
-- 
2.8.1

[PATCH v3 5/8] MTD: xway: fix nand locking

[Hauke Mehrtens]

From: John Crispin <john@phrozen.org>

The external Bus Unit (EBU) can control different flash devices, but
these NAND flash commands have to be atomic and should not be
interrupted in between. Lock the EBU from the beginning of the command
till the end by moving the lock to the chip select.

Signed-off-by: John Crispin <john@phrozen.org>
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/xway_nand.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index 8a0019a..618cf34 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -65,17 +65,22 @@
 
 struct xway_nand_data {
 	struct nand_chip	chip;
+	unsigned long		csflags;
 };
 
-static void xway_select_chip(struct mtd_info *mtd, int chip)
+static void xway_select_chip(struct mtd_info *mtd, int select)
 {
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct xway_nand_data *data = nand_get_controller_data(chip);
 
-	switch (chip) {
+	switch (select) {
 	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, data->csflags);
 		break;
 	case 0:
+		spin_lock_irqsave(&ebu_lock, data->csflags);
 		ltq_ebu_w32_mask(0, NAND_CON_NANDM, EBU_NAND_CON);
 		ltq_ebu_w32_mask(0, NAND_CON_CE, EBU_NAND_CON);
 		break;
@@ -88,12 +93,10 @@ static void xway_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 {
 	struct nand_chip *this = mtd_to_nand(mtd);
 	unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
-	unsigned long flags;
 
 	if (cmd == NAND_CMD_NONE)
 		return;
 
-	spin_lock_irqsave(&ebu_lock, flags);
 	if (ctrl & NAND_CLE)
 		writeb(cmd, (void __iomem *) (nandaddr | NAND_WRITE_CMD));
 	else if (ctrl & NAND_ALE)
@@ -101,7 +104,6 @@ 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)
 		;
-	spin_unlock_irqrestore(&ebu_lock, flags);
 }
 
 static int xway_dev_ready(struct mtd_info *mtd)
@@ -113,14 +115,8 @@ static unsigned char xway_read_byte(struct mtd_info *mtd)
 {
 	struct nand_chip *this = mtd_to_nand(mtd);
 	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;
+	return ltq_r8((void __iomem *)(nandaddr + NAND_READ_DATA));
 }
 
 /*
-- 
2.8.1

Re: [PATCH v3 5/8] MTD: xway: fix nand locking

[Boris Brezillon]

On Sun, 19 Jun 2016 18:08:14 +0200
Hauke Mehrtens <hauke@hauke-m.de> wrote:

> From: John Crispin <john@phrozen.org>
> 
> The external Bus Unit (EBU) can control different flash devices, but
> these NAND flash commands have to be atomic and should not be
> interrupted in between. Lock the EBU from the beginning of the command
> till the end by moving the lock to the chip select.
> 
> Signed-off-by: John Crispin <john@phrozen.org>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/mtd/nand/xway_nand.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
> index 8a0019a..618cf34 100644
> --- a/drivers/mtd/nand/xway_nand.c
> +++ b/drivers/mtd/nand/xway_nand.c
> @@ -65,17 +65,22 @@
>  
>  struct xway_nand_data {
>  	struct nand_chip	chip;
> +	unsigned long		csflags;
>  };
>  
> -static void xway_select_chip(struct mtd_info *mtd, int chip)
> +static void xway_select_chip(struct mtd_info *mtd, int select)
>  {
> +	struct nand_chip *chip = mtd_to_nand(mtd);
> +	struct xway_nand_data *data = nand_get_controller_data(chip);
>  
> -	switch (chip) {
> +	switch (select) {
>  	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, data->csflags);
>  		break;
>  	case 0:
> +		spin_lock_irqsave(&ebu_lock, data->csflags);
>  		ltq_ebu_w32_mask(0, NAND_CON_NANDM, EBU_NAND_CON);
>  		ltq_ebu_w32_mask(0, NAND_CON_CE, EBU_NAND_CON);
>  		break;
> @@ -88,12 +93,10 @@ static void xway_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
>  {
>  	struct nand_chip *this = mtd_to_nand(mtd);
>  	unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
> -	unsigned long flags;
>  
>  	if (cmd == NAND_CMD_NONE)
>  		return;
>  
> -	spin_lock_irqsave(&ebu_lock, flags);
>  	if (ctrl & NAND_CLE)
>  		writeb(cmd, (void __iomem *) (nandaddr | NAND_WRITE_CMD));
>  	else if (ctrl & NAND_ALE)
> @@ -101,7 +104,6 @@ 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)
>  		;
> -	spin_unlock_irqrestore(&ebu_lock, flags);
>  }
>  
>  static int xway_dev_ready(struct mtd_info *mtd)
> @@ -113,14 +115,8 @@ static unsigned char xway_read_byte(struct mtd_info *mtd)
>  {
>  	struct nand_chip *this = mtd_to_nand(mtd);
>  	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;
> +	return ltq_r8((void __iomem *)(nandaddr + NAND_READ_DATA));

	Drop the cast and use this->IO_ADDR_R here.

>  }
>  
>  /*

Re: [PATCH v3 5/8] MTD: xway: fix nand locking

[Hauke Mehrtens]

On 06/19/2016 06:26 PM, Boris Brezillon wrote:
> On Sun, 19 Jun 2016 18:08:14 +0200
> Hauke Mehrtens <hauke@hauke-m.de> wrote:
> 
>> From: John Crispin <john@phrozen.org>
>>
>> The external Bus Unit (EBU) can control different flash devices, but
>> these NAND flash commands have to be atomic and should not be
>> interrupted in between. Lock the EBU from the beginning of the command
>> till the end by moving the lock to the chip select.
>>
>> Signed-off-by: John Crispin <john@phrozen.org>
>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
>> ---
>>  drivers/mtd/nand/xway_nand.c | 20 ++++++++------------
>>  1 file changed, 8 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
>> index 8a0019a..618cf34 100644
>> --- a/drivers/mtd/nand/xway_nand.c
>> +++ b/drivers/mtd/nand/xway_nand.c
>> @@ -65,17 +65,22 @@
>>  
>>  struct xway_nand_data {
>>  	struct nand_chip	chip;
>> +	unsigned long		csflags;
>>  };
>>  
>> -static void xway_select_chip(struct mtd_info *mtd, int chip)
>> +static void xway_select_chip(struct mtd_info *mtd, int select)
>>  {
>> +	struct nand_chip *chip = mtd_to_nand(mtd);
>> +	struct xway_nand_data *data = nand_get_controller_data(chip);
>>  
>> -	switch (chip) {
>> +	switch (select) {
>>  	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, data->csflags);
>>  		break;
>>  	case 0:
>> +		spin_lock_irqsave(&ebu_lock, data->csflags);
>>  		ltq_ebu_w32_mask(0, NAND_CON_NANDM, EBU_NAND_CON);
>>  		ltq_ebu_w32_mask(0, NAND_CON_CE, EBU_NAND_CON);
>>  		break;
>> @@ -88,12 +93,10 @@ static void xway_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
>>  {
>>  	struct nand_chip *this = mtd_to_nand(mtd);
>>  	unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
>> -	unsigned long flags;
>>  
>>  	if (cmd == NAND_CMD_NONE)
>>  		return;
>>  
>> -	spin_lock_irqsave(&ebu_lock, flags);
>>  	if (ctrl & NAND_CLE)
>>  		writeb(cmd, (void __iomem *) (nandaddr | NAND_WRITE_CMD));
>>  	else if (ctrl & NAND_ALE)
>> @@ -101,7 +104,6 @@ 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)
>>  		;
>> -	spin_unlock_irqrestore(&ebu_lock, flags);
>>  }
>>  
>>  static int xway_dev_ready(struct mtd_info *mtd)
>> @@ -113,14 +115,8 @@ static unsigned char xway_read_byte(struct mtd_info *mtd)
>>  {
>>  	struct nand_chip *this = mtd_to_nand(mtd);
>>  	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;
>> +	return ltq_r8((void __iomem *)(nandaddr + NAND_READ_DATA));
> 
> 	Drop the cast and use this->IO_ADDR_R here.

This will be done in the following patch, here I am only chaining the
locking.

> 
>>  }
>>  
>>  /*
> 

Re: [PATCH v3 5/8] MTD: xway: fix nand locking

[Boris Brezillon]

On Sun, 19 Jun 2016 18:34:30 +0200
Hauke Mehrtens <hauke@hauke-m.de> wrote:
> >> @@ -113,14 +115,8 @@ static unsigned char xway_read_byte(struct mtd_info *mtd)
> >>  {
> >>  	struct nand_chip *this = mtd_to_nand(mtd);
> >>  	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;
> >> +	return ltq_r8((void __iomem *)(nandaddr + NAND_READ_DATA));  
> > 
> > 	Drop the cast and use this->IO_ADDR_R here.  
> 
> This will be done in the following patch, here I am only chaining the
> locking.

My bad, you're right.

[PATCH v3 6/8] MTD: xway: extract read and write function

[Hauke Mehrtens]

Extract the functions to read and write to the register of the NAND
flash controller.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/xway_nand.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index 618cf34..6fac4c6 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -68,6 +68,22 @@ struct xway_nand_data {
 	unsigned long		csflags;
 };
 
+static u8 xway_readb(struct mtd_info *mtd, int op)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	void __iomem *nandaddr = chip->IO_ADDR_R;
+
+	return readb(nandaddr + op);
+}
+
+static void xway_writeb(struct mtd_info *mtd, int op, u8 value)
+{
+	struct nand_chip *chip = mtd_to_nand(mtd);
+	void __iomem *nandaddr = chip->IO_ADDR_W;
+
+	writeb(value, nandaddr + op);
+}
+
 static void xway_select_chip(struct mtd_info *mtd, int select)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
@@ -91,16 +107,13 @@ static void xway_select_chip(struct mtd_info *mtd, int select)
 
 static void xway_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 {
-	struct nand_chip *this = mtd_to_nand(mtd);
-	unsigned long nandaddr = (unsigned long) this->IO_ADDR_W;
-
 	if (cmd == NAND_CMD_NONE)
 		return;
 
 	if (ctrl & NAND_CLE)
-		writeb(cmd, (void __iomem *) (nandaddr | NAND_WRITE_CMD));
+		xway_writeb(mtd, NAND_WRITE_CMD, cmd);
 	else if (ctrl & NAND_ALE)
-		writeb(cmd, (void __iomem *) (nandaddr | NAND_WRITE_ADDR));
+		xway_writeb(mtd, NAND_WRITE_ADDR, cmd);
 
 	while ((ltq_ebu_r32(EBU_NAND_WAIT) & NAND_WAIT_WR_C) == 0)
 		;
@@ -113,10 +126,7 @@ static int xway_dev_ready(struct mtd_info *mtd)
 
 static unsigned char xway_read_byte(struct mtd_info *mtd)
 {
-	struct nand_chip *this = mtd_to_nand(mtd);
-	unsigned long nandaddr = (unsigned long) this->IO_ADDR_R;
-
-	return ltq_r8((void __iomem *)(nandaddr + NAND_READ_DATA));
+	return xway_readb(mtd, NAND_READ_DATA);
 }
 
 /*
-- 
2.8.1

[PATCH v3 7/8] MTD: xway: add nandaddr to own struct

[Hauke Mehrtens]

Instead of using IO_ADDR_W and IO_ADDR_R use an own pointer to the NAND
controller memory area.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/xway_nand.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index 6fac4c6..282258f 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -66,22 +66,23 @@
 struct xway_nand_data {
 	struct nand_chip	chip;
 	unsigned long		csflags;
+	void __iomem		*nandaddr;
 };
 
 static u8 xway_readb(struct mtd_info *mtd, int op)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
-	void __iomem *nandaddr = chip->IO_ADDR_R;
+	struct xway_nand_data *data = nand_get_controller_data(chip);
 
-	return readb(nandaddr + op);
+	return readb(data->nandaddr + op);
 }
 
 static void xway_writeb(struct mtd_info *mtd, int op, u8 value)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
-	void __iomem *nandaddr = chip->IO_ADDR_W;
+	struct xway_nand_data *data = nand_get_controller_data(chip);
 
-	writeb(value, nandaddr + op);
+	writeb(value, data->nandaddr + op);
 }
 
 static void xway_select_chip(struct mtd_info *mtd, int select)
@@ -138,7 +139,6 @@ static int xway_nand_probe(struct platform_device *pdev)
 	struct mtd_info *mtd;
 	struct resource *res;
 	int err;
-	void __iomem *nandaddr;
 	u32 cs;
 	u32 cs_flag = 0;
 
@@ -149,16 +149,16 @@ static int xway_nand_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	nandaddr = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(nandaddr))
-		return PTR_ERR(nandaddr);
+	data->nandaddr = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(data->nandaddr))
+		return PTR_ERR(data->nandaddr);
 
 	nand_set_flash_node(&data->chip, pdev->dev.of_node);
 	mtd = nand_to_mtd(&data->chip);
 	mtd->dev.parent = &pdev->dev;
 
-	data->chip.IO_ADDR_R = nandaddr;
-	data->chip.IO_ADDR_W = nandaddr;
+	data->chip.IO_ADDR_R = data->nandaddr;
+	data->chip.IO_ADDR_W = data->nandaddr;
 	data->chip.cmd_ctrl = xway_cmd_ctrl;
 	data->chip.dev_ready = xway_dev_ready;
 	data->chip.select_chip = xway_select_chip;
@@ -177,7 +177,7 @@ static int xway_nand_probe(struct platform_device *pdev)
 		cs_flag = NAND_CON_IN_CS1 | NAND_CON_OUT_CS1;
 
 	/* setup the EBU to run in NAND mode on our base addr */
-	ltq_ebu_w32(CPHYSADDR(nandaddr)
+	ltq_ebu_w32(CPHYSADDR(data->nandaddr)
 		| ADDSEL1_MASK(3) | ADDSEL1_REGEN, EBU_ADDSEL1);
 
 	ltq_ebu_w32(BUSCON1_SETUP | BUSCON1_BCGEN_RES | BUSCON1_WAITWRC2
-- 
2.8.1

Re: [PATCH v3 7/8] MTD: xway: add nandaddr to own struct

[Boris Brezillon]

On Sun, 19 Jun 2016 18:08:16 +0200
Hauke Mehrtens <hauke@hauke-m.de> wrote:

> Instead of using IO_ADDR_W and IO_ADDR_R use an own pointer to the NAND
> controller memory area.
> 
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/mtd/nand/xway_nand.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
> index 6fac4c6..282258f 100644
> --- a/drivers/mtd/nand/xway_nand.c
> +++ b/drivers/mtd/nand/xway_nand.c
> @@ -66,22 +66,23 @@
>  struct xway_nand_data {
>  	struct nand_chip	chip;
>  	unsigned long		csflags;
> +	void __iomem		*nandaddr;
>  };
>  
>  static u8 xway_readb(struct mtd_info *mtd, int op)
>  {
>  	struct nand_chip *chip = mtd_to_nand(mtd);
> -	void __iomem *nandaddr = chip->IO_ADDR_R;
> +	struct xway_nand_data *data = nand_get_controller_data(chip);
>  
> -	return readb(nandaddr + op);
> +	return readb(data->nandaddr + op);
>  }
>  
>  static void xway_writeb(struct mtd_info *mtd, int op, u8 value)
>  {
>  	struct nand_chip *chip = mtd_to_nand(mtd);
> -	void __iomem *nandaddr = chip->IO_ADDR_W;
> +	struct xway_nand_data *data = nand_get_controller_data(chip);
>  
> -	writeb(value, nandaddr + op);
> +	writeb(value, data->nandaddr + op);
>  }
>  
>  static void xway_select_chip(struct mtd_info *mtd, int select)
> @@ -138,7 +139,6 @@ static int xway_nand_probe(struct platform_device *pdev)
>  	struct mtd_info *mtd;
>  	struct resource *res;
>  	int err;
> -	void __iomem *nandaddr;
>  	u32 cs;
>  	u32 cs_flag = 0;
>  
> @@ -149,16 +149,16 @@ static int xway_nand_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	nandaddr = devm_ioremap_resource(&pdev->dev, res);
> -	if (IS_ERR(nandaddr))
> -		return PTR_ERR(nandaddr);
> +	data->nandaddr = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(data->nandaddr))
> +		return PTR_ERR(data->nandaddr);
>  
>  	nand_set_flash_node(&data->chip, pdev->dev.of_node);
>  	mtd = nand_to_mtd(&data->chip);
>  	mtd->dev.parent = &pdev->dev;
>  
> -	data->chip.IO_ADDR_R = nandaddr;
> -	data->chip.IO_ADDR_W = nandaddr;
> +	data->chip.IO_ADDR_R = data->nandaddr;
> +	data->chip.IO_ADDR_W = data->nandaddr;

If you patched all places using ->IO_ADDR_R/W you should be able to get
rid of these assignments. If you didn't, please do it in this patch.

>  	data->chip.cmd_ctrl = xway_cmd_ctrl;
>  	data->chip.dev_ready = xway_dev_ready;
>  	data->chip.select_chip = xway_select_chip;
> @@ -177,7 +177,7 @@ static int xway_nand_probe(struct platform_device *pdev)
>  		cs_flag = NAND_CON_IN_CS1 | NAND_CON_OUT_CS1;
>  
>  	/* setup the EBU to run in NAND mode on our base addr */
> -	ltq_ebu_w32(CPHYSADDR(nandaddr)
> +	ltq_ebu_w32(CPHYSADDR(data->nandaddr)
>  		| ADDSEL1_MASK(3) | ADDSEL1_REGEN, EBU_ADDSEL1);

Try to align parameters on the open parenthesis.

	ltq_ebu_w32(CPHYSADDR(data->nandaddr) | ADDSEL1_MASK(3) |
		    ADDSEL1_REGEN, EBU_ADDSEL1);

>  
>  	ltq_ebu_w32(BUSCON1_SETUP | BUSCON1_BCGEN_RES | BUSCON1_WAITWRC2

Re: [PATCH v3 7/8] MTD: xway: add nandaddr to own struct

[Boris Brezillon]

On Sun, 19 Jun 2016 18:29:46 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Sun, 19 Jun 2016 18:08:16 +0200
> Hauke Mehrtens <hauke@hauke-m.de> wrote:
> 
> > Instead of using IO_ADDR_W and IO_ADDR_R use an own pointer to the NAND
> > controller memory area.
> > 
> > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> > ---
> >  drivers/mtd/nand/xway_nand.c | 22 +++++++++++-----------
> >  1 file changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
> > index 6fac4c6..282258f 100644
> > --- a/drivers/mtd/nand/xway_nand.c
> > +++ b/drivers/mtd/nand/xway_nand.c
> > @@ -66,22 +66,23 @@
> >  struct xway_nand_data {
> >  	struct nand_chip	chip;
> >  	unsigned long		csflags;
> > +	void __iomem		*nandaddr;
> >  };
> >  
> >  static u8 xway_readb(struct mtd_info *mtd, int op)
> >  {
> >  	struct nand_chip *chip = mtd_to_nand(mtd);
> > -	void __iomem *nandaddr = chip->IO_ADDR_R;
> > +	struct xway_nand_data *data = nand_get_controller_data(chip);
> >  
> > -	return readb(nandaddr + op);
> > +	return readb(data->nandaddr + op);
> >  }
> >  
> >  static void xway_writeb(struct mtd_info *mtd, int op, u8 value)
> >  {
> >  	struct nand_chip *chip = mtd_to_nand(mtd);
> > -	void __iomem *nandaddr = chip->IO_ADDR_W;
> > +	struct xway_nand_data *data = nand_get_controller_data(chip);
> >  
> > -	writeb(value, nandaddr + op);
> > +	writeb(value, data->nandaddr + op);
> >  }
> >  
> >  static void xway_select_chip(struct mtd_info *mtd, int select)
> > @@ -138,7 +139,6 @@ static int xway_nand_probe(struct platform_device *pdev)
> >  	struct mtd_info *mtd;
> >  	struct resource *res;
> >  	int err;
> > -	void __iomem *nandaddr;
> >  	u32 cs;
> >  	u32 cs_flag = 0;
> >  
> > @@ -149,16 +149,16 @@ static int xway_nand_probe(struct platform_device *pdev)
> >  		return -ENOMEM;
> >  
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -	nandaddr = devm_ioremap_resource(&pdev->dev, res);
> > -	if (IS_ERR(nandaddr))
> > -		return PTR_ERR(nandaddr);
> > +	data->nandaddr = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(data->nandaddr))
> > +		return PTR_ERR(data->nandaddr);
> >  
> >  	nand_set_flash_node(&data->chip, pdev->dev.of_node);
> >  	mtd = nand_to_mtd(&data->chip);
> >  	mtd->dev.parent = &pdev->dev;
> >  
> > -	data->chip.IO_ADDR_R = nandaddr;
> > -	data->chip.IO_ADDR_W = nandaddr;
> > +	data->chip.IO_ADDR_R = data->nandaddr;
> > +	data->chip.IO_ADDR_W = data->nandaddr;  
> 
> If you patched all places using ->IO_ADDR_R/W you should be able to get
> rid of these assignments. If you didn't, please do it in this patch.

Just realized this implies moving patch 8 before this one, because the
default ->read_buf()/->write_buf() are making use of ->IO_ADDR_R/W.

Re: [PATCH v3 7/8] MTD: xway: add nandaddr to own struct

[Hauke Mehrtens]

On 06/19/2016 06:32 PM, Boris Brezillon wrote:
> On Sun, 19 Jun 2016 18:29:46 +0200
> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> 
>> On Sun, 19 Jun 2016 18:08:16 +0200
>> Hauke Mehrtens <hauke@hauke-m.de> wrote:
>>
>>> Instead of using IO_ADDR_W and IO_ADDR_R use an own pointer to the NAND
>>> controller memory area.
>>>
>>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
>>> ---
>>>  drivers/mtd/nand/xway_nand.c | 22 +++++++++++-----------
>>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
>>> index 6fac4c6..282258f 100644
>>> --- a/drivers/mtd/nand/xway_nand.c
>>> +++ b/drivers/mtd/nand/xway_nand.c
>>> @@ -66,22 +66,23 @@
>>>  struct xway_nand_data {
>>>  	struct nand_chip	chip;
>>>  	unsigned long		csflags;
>>> +	void __iomem		*nandaddr;
>>>  };
>>>  
>>>  static u8 xway_readb(struct mtd_info *mtd, int op)
>>>  {
>>>  	struct nand_chip *chip = mtd_to_nand(mtd);
>>> -	void __iomem *nandaddr = chip->IO_ADDR_R;
>>> +	struct xway_nand_data *data = nand_get_controller_data(chip);
>>>  
>>> -	return readb(nandaddr + op);
>>> +	return readb(data->nandaddr + op);
>>>  }
>>>  
>>>  static void xway_writeb(struct mtd_info *mtd, int op, u8 value)
>>>  {
>>>  	struct nand_chip *chip = mtd_to_nand(mtd);
>>> -	void __iomem *nandaddr = chip->IO_ADDR_W;
>>> +	struct xway_nand_data *data = nand_get_controller_data(chip);
>>>  
>>> -	writeb(value, nandaddr + op);
>>> +	writeb(value, data->nandaddr + op);
>>>  }
>>>  
>>>  static void xway_select_chip(struct mtd_info *mtd, int select)
>>> @@ -138,7 +139,6 @@ static int xway_nand_probe(struct platform_device *pdev)
>>>  	struct mtd_info *mtd;
>>>  	struct resource *res;
>>>  	int err;
>>> -	void __iomem *nandaddr;
>>>  	u32 cs;
>>>  	u32 cs_flag = 0;
>>>  
>>> @@ -149,16 +149,16 @@ static int xway_nand_probe(struct platform_device *pdev)
>>>  		return -ENOMEM;
>>>  
>>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> -	nandaddr = devm_ioremap_resource(&pdev->dev, res);
>>> -	if (IS_ERR(nandaddr))
>>> -		return PTR_ERR(nandaddr);
>>> +	data->nandaddr = devm_ioremap_resource(&pdev->dev, res);
>>> +	if (IS_ERR(data->nandaddr))
>>> +		return PTR_ERR(data->nandaddr);
>>>  
>>>  	nand_set_flash_node(&data->chip, pdev->dev.of_node);
>>>  	mtd = nand_to_mtd(&data->chip);
>>>  	mtd->dev.parent = &pdev->dev;
>>>  
>>> -	data->chip.IO_ADDR_R = nandaddr;
>>> -	data->chip.IO_ADDR_W = nandaddr;
>>> +	data->chip.IO_ADDR_R = data->nandaddr;
>>> +	data->chip.IO_ADDR_W = data->nandaddr;  
>>
>> If you patched all places using ->IO_ADDR_R/W you should be able to get
>> rid of these assignments. If you didn't, please do it in this patch.
> 
> Just realized this implies moving patch 8 before this one, because the
> default ->read_buf()/->write_buf() are making use of ->IO_ADDR_R/W.
> 

Ok, I will change the order and not set IO_ADDR_R and IO_ADDR_W.

Hauke

[PATCH v3 8/8] MTD: xway: add missing write_buf and read_buf to nand driver

[Hauke Mehrtens]

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 drivers/mtd/nand/xway_nand.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
index 282258f..8f208cb 100644
--- a/drivers/mtd/nand/xway_nand.c
+++ b/drivers/mtd/nand/xway_nand.c
@@ -130,6 +130,22 @@ static unsigned char xway_read_byte(struct mtd_info *mtd)
 	return xway_readb(mtd, NAND_READ_DATA);
 }
 
+static void xway_read_buf(struct mtd_info *mtd, u_char *buf, int len)
+{
+	int i;
+
+	for (i = 0; i < len; i++)
+		buf[i] = xway_readb(mtd, NAND_WRITE_DATA);
+}
+
+static void xway_write_buf(struct mtd_info *mtd, const u_char *buf, int len)
+{
+	int i;
+
+	for (i = 0; i < len; i++)
+		xway_writeb(mtd, NAND_WRITE_DATA, buf[i]);
+}
+
 /*
  * Probe for the NAND device.
  */
@@ -162,6 +178,8 @@ static int xway_nand_probe(struct platform_device *pdev)
 	data->chip.cmd_ctrl = xway_cmd_ctrl;
 	data->chip.dev_ready = xway_dev_ready;
 	data->chip.select_chip = xway_select_chip;
+	data->chip.write_buf = xway_write_buf;
+	data->chip.read_buf = xway_read_buf;
 	data->chip.read_byte = xway_read_byte;
 	data->chip.chip_delay = 30;
 
-- 
2.8.1

Re: [PATCH v3 8/8] MTD: xway: add missing write_buf and read_buf to nand driver

[Boris Brezillon]

On Sun, 19 Jun 2016 18:08:17 +0200
Hauke Mehrtens <hauke@hauke-m.de> wrote:

Still no descriptions...

> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  drivers/mtd/nand/xway_nand.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/mtd/nand/xway_nand.c b/drivers/mtd/nand/xway_nand.c
> index 282258f..8f208cb 100644
> --- a/drivers/mtd/nand/xway_nand.c
> +++ b/drivers/mtd/nand/xway_nand.c
> @@ -130,6 +130,22 @@ static unsigned char xway_read_byte(struct mtd_info *mtd)
>  	return xway_readb(mtd, NAND_READ_DATA);
>  }
>  
> +static void xway_read_buf(struct mtd_info *mtd, u_char *buf, int len)
> +{
> +	int i;
> +
> +	for (i = 0; i < len; i++)
> +		buf[i] = xway_readb(mtd, NAND_WRITE_DATA);
> +}
> +
> +static void xway_write_buf(struct mtd_info *mtd, const u_char *buf, int len)
> +{
> +	int i;
> +
> +	for (i = 0; i < len; i++)
> +		xway_writeb(mtd, NAND_WRITE_DATA, buf[i]);
> +}
> +
>  /*
>   * Probe for the NAND device.
>   */
> @@ -162,6 +178,8 @@ static int xway_nand_probe(struct platform_device *pdev)
>  	data->chip.cmd_ctrl = xway_cmd_ctrl;
>  	data->chip.dev_ready = xway_dev_ready;
>  	data->chip.select_chip = xway_select_chip;
> +	data->chip.write_buf = xway_write_buf;
> +	data->chip.read_buf = xway_read_buf;
>  	data->chip.read_byte = xway_read_byte;
>  	data->chip.chip_delay = 30;
>