[PATCH v3 0/8] mtd: spi-nor: fsl-quadspi fix and added i.mx7d and i.mxul support

[PATCH v3 0/8] mtd: spi-nor: fsl-quadspi fix and added i.mx7d and i.mxul support

[Frank.Li]

From: Frank Li <Frank.Li@freescale.com>

rebase to l2-mtd/next

Resend whole serial patch

Allen Xu (2):
  mtd: spi-nor: fsl-qspi: dynamically map memory space for AHB read
  mtd: spi-nor: fsl-quadspi: i.MX6SX: fixed the random QSPI access
    failed issue

Frank Li (5):
  mtd: spi-nor: fsl-quadspi: add imx7d support
  mtd: spi-nor: fsl-quadspi: add i.mx6ul support
  mtd: spi-nor: fsl-quadspi: workaround qspi can't wakeup from wait mode
  mtd: spi-nor: fsl-quadspi: reset the module in the probe
  mtd: spi-nor: fsl-quadspi: fix unsupported cmd when run flash_erase

Han Xu (1):
  mtd: spi-nor: fsl-quadspi: use quirk to distinguish different qspi
    version

 drivers/mtd/spi-nor/fsl-quadspi.c | 230 ++++++++++++++++++++++++++++++--------
 1 file changed, 183 insertions(+), 47 deletions(-)

-- 
1.9.1

[PATCH v3 1/8] mtd: spi-nor: fsl-qspi: dynamically map memory space for AHB read

[Frank.Li]

From: Allen Xu <b45815@freescale.com>

QSPI may failed to map enough memory (256MB) for AHB read in
previous implementation, especially in 3G/1G memory layout kernel.
Dynamically map memory to avoid such issue.

This implementation generally map 4MB memory for AHB read, it should
be enough for common scenarios, and the side effect (0.6% performance
drop) is minor.

Previous implementation

root@imx6qdlsolo:~# dd if=/dev/mtd0 of=/dev/null bs=1K count=32K
32768+0 records in
32768+0 records out
33554432 bytes (34 MB) copied, 2.16006 s, 15.5 MB/s

root@imx6qdlsolo:~# dd if=/dev/mtd0 of=/dev/null bs=32M count=1
1+0 records in
1+0 records out
33554432 bytes (34 MB) copied, 1.43149 s, 23.4 MB/s

After applied the patch

root@imx6qdlsolo:~# dd if=/dev/mtd0 of=/dev/null bs=1K count=32K
32768+0 records in
32768+0 records out
33554432 bytes (34 MB) copied, 2.1743 s, 15.4 MB/s

root@imx6qdlsolo:~# dd if=/dev/mtd0 of=/dev/null bs=32M count=1
1+0 records in
1+0 records out
33554432 bytes (34 MB) copied, 1.43158 s, 23.4 MB/s

Signed-off-by: Allen Xu <b45815@freescale.com>
Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 48 ++++++++++++++++++++++++++++++++-------
 1 file changed, 40 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 4fe13dd..ac9d633 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -223,8 +223,10 @@ struct fsl_qspi {
 	struct mtd_info mtd[FSL_QSPI_MAX_CHIP];
 	struct spi_nor nor[FSL_QSPI_MAX_CHIP];
 	void __iomem *iobase;
-	void __iomem *ahb_base; /* Used when read from AHB bus */
+	void __iomem *ahb_addr;
 	u32 memmap_phy;
+	u32 memmap_offs;
+	u32 memmap_len;
 	struct clk *clk, *clk_en;
 	struct device *dev;
 	struct completion c;
@@ -732,11 +734,41 @@ static int fsl_qspi_read(struct spi_nor *nor, loff_t from,
 	struct fsl_qspi *q = nor->priv;
 	u8 cmd = nor->read_opcode;
 
-	dev_dbg(q->dev, "cmd [%x],read from (0x%p, 0x%.8x, 0x%.8x),len:%d\n",
-		cmd, q->ahb_base, q->chip_base_addr, (unsigned int)from, len);
+	/* if necessary,ioremap buffer before AHB read, */
+	/* generally 4MB should be large enough */
+	if (!q->ahb_addr) {
+		q->ahb_addr = ioremap_nocache(
+				q->memmap_phy + q->chip_base_addr + from,
+				len > SZ_4M ? len : SZ_4M);
+		if (!q->ahb_addr) {
+			dev_err(q->dev, "ioremap failed\n");
+			return -ENOMEM;
+		}
+		q->memmap_offs = q->chip_base_addr + from;
+		q->memmap_len = len > SZ_4M ? len : SZ_4M;
+	/* ioremap if the data requested is out of range */
+	} else if (q->chip_base_addr + from < q->memmap_offs
+			|| q->chip_base_addr + from + len >
+			q->memmap_offs + q->memmap_len) {
+		iounmap(q->ahb_addr);
+		q->ahb_addr = ioremap_nocache(
+				q->memmap_phy + q->chip_base_addr + from,
+				len > SZ_4M ? len : SZ_4M);
+		if (!q->ahb_addr) {
+			dev_err(q->dev, "ioremap failed\n");
+			return -ENOMEM;
+		}
+		q->memmap_offs = q->chip_base_addr + from;
+		q->memmap_len = len > SZ_4M ? len : SZ_4M;
+	}
+
+	dev_dbg(q->dev, "cmd [%x],read from 0x%p, len:%d\n",
+		cmd, q->ahb_addr + q->chip_base_addr + from - q->memmap_offs,
+		len);
 
 	/* Read out the data directly from the AHB buffer.*/
-	memcpy(buf, q->ahb_base + q->chip_base_addr + from, len);
+	memcpy(buf, q->ahb_addr + q->chip_base_addr + from - q->memmap_offs,
+		len);
 
 	*retlen += len;
 	return 0;
@@ -821,10 +853,6 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
 					"QuadSPI-memory");
-	q->ahb_base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(q->ahb_base))
-		return PTR_ERR(q->ahb_base);
-
 	q->memmap_phy = res->start;
 
 	/* find the clocks */
@@ -989,6 +1017,10 @@ static int fsl_qspi_remove(struct platform_device *pdev)
 	mutex_destroy(&q->lock);
 	clk_unprepare(q->clk);
 	clk_unprepare(q->clk_en);
+
+	if (q->ahb_addr)
+		iounmap(q->ahb_addr);
+
 	return 0;
 }
 
-- 
1.9.1

[PATCH v3 2/8] mtd: spi-nor: fsl-quadspi: use quirk to distinguish different qspi version

[Frank.Li]

From: Han Xu <b45815@freescale.com>

add several quirk to distinguish different version of qspi module.

Signed-off-by: Han Xu <b45815@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index ac9d633..5f31bc7 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -28,6 +28,11 @@
 #include <linux/mtd/spi-nor.h>
 #include <linux/mutex.h>
 
+/* Controller needs driver to swap endian */
+#define QUADSPI_QUIRK_SWAP_ENDIAN	(1 << 0)
+/* Controller needs 4x internal clock */
+#define QUADSPI_QUIRK_4X_INT_CLK	(1 << 1)
+
 /* The registers */
 #define QUADSPI_MCR			0x00
 #define QUADSPI_MCR_RESERVED_SHIFT	16
@@ -202,20 +207,23 @@ struct fsl_qspi_devtype_data {
 	int rxfifo;
 	int txfifo;
 	int ahb_buf_size;
+	int driver_data;
 };
 
 static struct fsl_qspi_devtype_data vybrid_data = {
 	.devtype = FSL_QUADSPI_VYBRID,
 	.rxfifo = 128,
 	.txfifo = 64,
-	.ahb_buf_size = 1024
+	.ahb_buf_size = 1024,
+	.driver_data = QUADSPI_QUIRK_SWAP_ENDIAN
 };
 
 static struct fsl_qspi_devtype_data imx6sx_data = {
 	.devtype = FSL_QUADSPI_IMX6SX,
 	.rxfifo = 128,
 	.txfifo = 512,
-	.ahb_buf_size = 1024
+	.ahb_buf_size = 1024,
+	.driver_data = QUADSPI_QUIRK_4X_INT_CLK
 };
 
 #define FSL_QSPI_MAX_CHIP	4
@@ -239,14 +247,14 @@ struct fsl_qspi {
 	struct mutex lock;
 };
 
-static inline int is_vybrid_qspi(struct fsl_qspi *q)
+static inline int needs_swap_endian(struct fsl_qspi *q)
 {
-	return q->devtype_data->devtype == FSL_QUADSPI_VYBRID;
+	return q->devtype_data->driver_data & QUADSPI_QUIRK_SWAP_ENDIAN;
 }
 
-static inline int is_imx6sx_qspi(struct fsl_qspi *q)
+static inline int needs_4x_clock(struct fsl_qspi *q)
 {
-	return q->devtype_data->devtype == FSL_QUADSPI_IMX6SX;
+	return q->devtype_data->driver_data & QUADSPI_QUIRK_4X_INT_CLK;
 }
 
 /*
@@ -255,7 +263,7 @@ static inline int is_imx6sx_qspi(struct fsl_qspi *q)
  */
 static inline u32 fsl_qspi_endian_xchg(struct fsl_qspi *q, u32 a)
 {
-	return is_vybrid_qspi(q) ? __swab32(a) : a;
+	return needs_swap_endian(q) ? __swab32(a) : a;
 }
 
 static inline void fsl_qspi_unlock_lut(struct fsl_qspi *q)
@@ -650,7 +658,7 @@ static int fsl_qspi_nor_setup_last(struct fsl_qspi *q)
 	unsigned long rate = q->clk_rate;
 	int ret;
 
-	if (is_imx6sx_qspi(q))
+	if (needs_4x_clock(q))
 		rate *= 4;
 
 	ret = clk_set_rate(q->clk, rate);
-- 
1.9.1

[PATCH v3 3/8] mtd: spi-nor: fsl-quadspi: add imx7d support

[Frank.Li]

From: Frank Li <Frank.Li@freescale.com>

Support i.mx7d.
quadspi in i.mx7d increase rxfifo.
require fill at least 16byte to trigger data transfer.

Signed-off-by: Frank Li <Frank.Li@freescale.com>
Signed-off-by: Han Xu <b45815@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 5f31bc7..3fc94ad 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -32,6 +32,11 @@
 #define QUADSPI_QUIRK_SWAP_ENDIAN	(1 << 0)
 /* Controller needs 4x internal clock */
 #define QUADSPI_QUIRK_4X_INT_CLK	(1 << 1)
+/*
+ * TKT253890, Controller needs driver to fill txfifo till 16 byte to
+ * trigger data transfer even though extern data will not transferred.
+ */
+#define QUADSPI_QUIRK_TKT253890		(1 << 2)
 
 /* The registers */
 #define QUADSPI_MCR			0x00
@@ -200,6 +205,7 @@
 enum fsl_qspi_devtype {
 	FSL_QUADSPI_VYBRID,
 	FSL_QUADSPI_IMX6SX,
+	FSL_QUADSPI_IMX7D,
 };
 
 struct fsl_qspi_devtype_data {
@@ -226,6 +232,15 @@ static struct fsl_qspi_devtype_data imx6sx_data = {
 	.driver_data = QUADSPI_QUIRK_4X_INT_CLK
 };
 
+static struct fsl_qspi_devtype_data imx7d_data = {
+	.devtype = FSL_QUADSPI_IMX7D,
+	.rxfifo = 512,
+	.txfifo = 512,
+	.ahb_buf_size = 1024,
+	.driver_data = QUADSPI_QUIRK_TKT253890
+		       | QUADSPI_QUIRK_4X_INT_CLK
+};
+
 #define FSL_QSPI_MAX_CHIP	4
 struct fsl_qspi {
 	struct mtd_info mtd[FSL_QSPI_MAX_CHIP];
@@ -257,6 +272,11 @@ static inline int needs_4x_clock(struct fsl_qspi *q)
 	return q->devtype_data->driver_data & QUADSPI_QUIRK_4X_INT_CLK;
 }
 
+static inline int needs_fill_txfifo(struct fsl_qspi *q)
+{
+	return q->devtype_data->driver_data & QUADSPI_QUIRK_TKT253890;
+}
+
 /*
  * An IC bug makes us to re-arrange the 32-bit data.
  * The following chips, such as IMX6SLX, have fixed this bug.
@@ -558,6 +578,11 @@ static int fsl_qspi_nor_write(struct fsl_qspi *q, struct spi_nor *nor,
 		txbuf++;
 	}
 
+	/* fill the TXFIFO upto 16 bytes for i.MX7d */
+	if (needs_fill_txfifo(q))
+		for (; i < 4; i++)
+			writel(tmp, q->iobase + QUADSPI_TBDR);
+
 	/* Trigger it */
 	ret = fsl_qspi_runcmd(q, opcode, to, count);
 
@@ -677,6 +702,7 @@ static int fsl_qspi_nor_setup_last(struct fsl_qspi *q)
 static const struct of_device_id fsl_qspi_dt_ids[] = {
 	{ .compatible = "fsl,vf610-qspi", .data = (void *)&vybrid_data, },
 	{ .compatible = "fsl,imx6sx-qspi", .data = (void *)&imx6sx_data, },
+	{ .compatible = "fsl,imx7d-qspi", .data = (void *)&imx7d_data, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, fsl_qspi_dt_ids);
-- 
1.9.1

[PATCH v3 4/8] mtd: spi-nor: fsl-quadspi: add i.mx6ul support

[Frank.Li]

From: Frank Li <Frank.Li@freescale.com>

Add i.mx6ul chip support

Signed-off-by: Frank Li <Frank.Li@freescale.com>
Acked-by: Allen Xu <b45815@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 3fc94ad..3746542 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -206,6 +206,7 @@ enum fsl_qspi_devtype {
 	FSL_QUADSPI_VYBRID,
 	FSL_QUADSPI_IMX6SX,
 	FSL_QUADSPI_IMX7D,
+	FSL_QUADSPI_IMX6UL,
 };
 
 struct fsl_qspi_devtype_data {
@@ -241,6 +242,15 @@ static struct fsl_qspi_devtype_data imx7d_data = {
 		       | QUADSPI_QUIRK_4X_INT_CLK
 };
 
+static struct fsl_qspi_devtype_data imx6ul_data = {
+	.devtype = FSL_QUADSPI_IMX6UL,
+	.rxfifo = 128,
+	.txfifo = 512,
+	.ahb_buf_size = 1024,
+	.driver_data = QUADSPI_QUIRK_TKT253890
+		       | QUADSPI_QUIRK_4X_INT_CLK
+};
+
 #define FSL_QSPI_MAX_CHIP	4
 struct fsl_qspi {
 	struct mtd_info mtd[FSL_QSPI_MAX_CHIP];
@@ -703,6 +713,7 @@ static const struct of_device_id fsl_qspi_dt_ids[] = {
 	{ .compatible = "fsl,vf610-qspi", .data = (void *)&vybrid_data, },
 	{ .compatible = "fsl,imx6sx-qspi", .data = (void *)&imx6sx_data, },
 	{ .compatible = "fsl,imx7d-qspi", .data = (void *)&imx7d_data, },
+	{ .compatible = "fsl,imx6ul-qspi", .data = (void *)&imx6ul_data, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, fsl_qspi_dt_ids);
-- 
1.9.1

[PATCH v3 5/8] mtd: spi-nor: fsl-quadspi: i.MX6SX: fixed the random QSPI access failed issue

[Frank.Li]

From: Allen Xu <b45815@freescale.com>

We found there is a low probability(5%) QSPI access timeout issue,
usually it happened on kernel boot stage, the first time kernel tried to
access QSPI chip. The READ_ID command was sent but not executed,
consequently the probe function failed.

Finally we located the issue by these steps.

1. Since the issue happened randomly and usually it cost half day to
reproduce, we add more debug code in driver, to create a timeout file if
the issue occurred.

2. Prepared an autorun script to keep rebooting the system and check if
the timeout file existed, if the file existed, stop reboot.

3. The system will stop rebooting when timeout error occurred, set the
CCM_CCOSR register and related IOMUX to measure QPSI clock, found there
is no clock output, while clock output can be measured when QSPI driver
successfully probed.

4. Check the code and found QSPI clock rate was changed while not
disabled clock gate, most of the multiplexers on i.MX6 are glitch ones,
clock glitch may occurred and propagated into downstream clock dividers

Based on the new clock flag(CLK_SET_RATE_GATE) and new framework, we
need to change the approach of seting clock rate. In current
implementation, there are several places in which the clock was touched.

1. probe function. prepare and enable clock before setting the QSPI
register, disable and unprepare the clock before exit.

2. nor_setup & nor_setup_last, since we change clock rate in these two
functions.

3. fsl_qspi_prep and fsl_qspi_unprep, clock was enabled only when got
QSPI access request.

4. resume function. Clock was required to restroe the setting after
resume, disable the clock before exit.

Signed-off-by: Allen Xu <b45815@freescale.com>
Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 83 +++++++++++++++++++++++++++------------
 1 file changed, 58 insertions(+), 25 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 3746542..9ee6db9 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -653,6 +653,32 @@ static void fsl_qspi_init_abh_read(struct fsl_qspi *q)
 		q->iobase + QUADSPI_BFGENCR);
 }
 
+/* This function was used to prepare and enable QSPI clock */
+static int fsl_qspi_clk_prep_enable(struct fsl_qspi *q)
+{
+	int ret;
+
+	ret = clk_prepare_enable(q->clk_en);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(q->clk);
+	if (ret) {
+		clk_disable_unprepare(q->clk_en);
+		return ret;
+	}
+
+	return 0;
+}
+
+/* This function was used to disable and unprepare QSPI clock */
+static void fsl_qspi_clk_disable_unprep(struct fsl_qspi *q)
+{
+	clk_disable_unprepare(q->clk);
+	clk_disable_unprepare(q->clk_en);
+
+}
+
 /* We use this function to do some basic init for spi_nor_scan(). */
 static int fsl_qspi_nor_setup(struct fsl_qspi *q)
 {
@@ -660,11 +686,19 @@ static int fsl_qspi_nor_setup(struct fsl_qspi *q)
 	u32 reg;
 	int ret;
 
-	/* the default frequency, we will change it in the future.*/
+	/* disable and unprepare clock first */
+	fsl_qspi_clk_disable_unprep(q);
+
+	/* the default frequency, we will change it in the future. */
 	ret = clk_set_rate(q->clk, 66000000);
 	if (ret)
 		return ret;
 
+	/* prepare and enable the clock */
+	ret = fsl_qspi_clk_prep_enable(q);
+	if (ret)
+		return ret;
+
 	/* Init the LUT table. */
 	fsl_qspi_init_lut(q);
 
@@ -696,10 +730,18 @@ static int fsl_qspi_nor_setup_last(struct fsl_qspi *q)
 	if (needs_4x_clock(q))
 		rate *= 4;
 
+	/* disable and unprepare clock first */
+	fsl_qspi_clk_disable_unprep(q);
+
 	ret = clk_set_rate(q->clk, rate);
 	if (ret)
 		return ret;
 
+	/* prepare and enable the clock */
+	ret = fsl_qspi_clk_prep_enable(q);
+	if (ret)
+		return ret;
+
 	/* Init the LUT table again. */
 	fsl_qspi_init_lut(q);
 
@@ -841,22 +883,16 @@ static int fsl_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
 	int ret;
 
 	mutex_lock(&q->lock);
-	ret = clk_enable(q->clk_en);
-	if (ret)
-		goto err_mutex;
 
-	ret = clk_enable(q->clk);
+	ret = fsl_qspi_clk_prep_enable(q);
 	if (ret)
-		goto err_clk;
+		goto err_mutex;
 
 	fsl_qspi_set_base_addr(q, nor);
 	return 0;
 
-err_clk:
-	clk_disable(q->clk_en);
 err_mutex:
 	mutex_unlock(&q->lock);
-
 	return ret;
 }
 
@@ -864,8 +900,7 @@ static void fsl_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
 {
 	struct fsl_qspi *q = nor->priv;
 
-	clk_disable(q->clk);
-	clk_disable(q->clk_en);
+	fsl_qspi_clk_disable_unprep(q);
 	mutex_unlock(&q->lock);
 }
 
@@ -909,15 +944,9 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	if (IS_ERR(q->clk))
 		return PTR_ERR(q->clk);
 
-	ret = clk_prepare_enable(q->clk_en);
+	ret = fsl_qspi_clk_prep_enable(q);
 	if (ret) {
-		dev_err(dev, "cannot enable the qspi_en clock: %d\n", ret);
-		return ret;
-	}
-
-	ret = clk_prepare_enable(q->clk);
-	if (ret) {
-		dev_err(dev, "cannot enable the qspi clock: %d\n", ret);
+		dev_err(dev, "can not enable the clock\n");
 		goto clk_failed;
 	}
 
@@ -1023,8 +1052,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	if (ret)
 		goto last_init_failed;
 
-	clk_disable(q->clk);
-	clk_disable(q->clk_en);
+	fsl_qspi_clk_disable_unprep(q);
 	return 0;
 
 last_init_failed:
@@ -1037,9 +1065,9 @@ last_init_failed:
 mutex_failed:
 	mutex_destroy(&q->lock);
 irq_failed:
-	clk_disable_unprepare(q->clk);
+	fsl_qspi_clk_disable_unprep(q);
 clk_failed:
-	clk_disable_unprepare(q->clk_en);
+	dev_err(dev, "Freescale QuadSPI probe failed\n");
 	return ret;
 }
 
@@ -1060,8 +1088,6 @@ static int fsl_qspi_remove(struct platform_device *pdev)
 	writel(0x0, q->iobase + QUADSPI_RSER);
 
 	mutex_destroy(&q->lock);
-	clk_unprepare(q->clk);
-	clk_unprepare(q->clk_en);
 
 	if (q->ahb_addr)
 		iounmap(q->ahb_addr);
@@ -1076,12 +1102,19 @@ static int fsl_qspi_suspend(struct platform_device *pdev, pm_message_t state)
 
 static int fsl_qspi_resume(struct platform_device *pdev)
 {
+	int ret;
 	struct fsl_qspi *q = platform_get_drvdata(pdev);
 
+	ret = fsl_qspi_clk_prep_enable(q);
+	if (ret)
+		return ret;
+
 	fsl_qspi_nor_setup(q);
 	fsl_qspi_set_map_addr(q);
 	fsl_qspi_nor_setup_last(q);
 
+	fsl_qspi_clk_disable_unprep(q);
+
 	return 0;
 }
 
-- 
1.9.1

[PATCH v3 6/8] mtd: spi-nor: fsl-quadspi: workaround qspi can't wakeup from wait mode

[Frank.Li]

From: Frank Li <Frank.Li@freescale.com>

QSPI1 cannot wake up CCM from WAIT mode on SX ARD board, add pmqos to
let PM NOT enter WAIT mode when accessing QSPI1, refer to TKT245618.

Signed-off-by: Frank Li <Frank.Li@freescale.com>
Signed-off-by: Allen Xu <b45815@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 9ee6db9..0ddc2d1 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -27,6 +27,7 @@
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/spi-nor.h>
 #include <linux/mutex.h>
+#include <linux/pm_qos.h>
 
 /* Controller needs driver to swap endian */
 #define QUADSPI_QUIRK_SWAP_ENDIAN	(1 << 0)
@@ -37,6 +38,8 @@
  * trigger data transfer even though extern data will not transferred.
  */
 #define QUADSPI_QUIRK_TKT253890		(1 << 2)
+/* Controller cannot wake up from wait mode, TKT245618 */
+#define QUADSPI_QUIRK_TKT245618         (1 << 3)
 
 /* The registers */
 #define QUADSPI_MCR			0x00
@@ -231,6 +234,7 @@ static struct fsl_qspi_devtype_data imx6sx_data = {
 	.txfifo = 512,
 	.ahb_buf_size = 1024,
 	.driver_data = QUADSPI_QUIRK_4X_INT_CLK
+		       | QUADSPI_QUIRK_TKT245618
 };
 
 static struct fsl_qspi_devtype_data imx7d_data = {
@@ -270,6 +274,7 @@ struct fsl_qspi {
 	unsigned int chip_base_addr; /* We may support two chips. */
 	bool has_second_chip;
 	struct mutex lock;
+	struct pm_qos_request pm_qos_req;
 };
 
 static inline int needs_swap_endian(struct fsl_qspi *q)
@@ -287,6 +292,11 @@ static inline int needs_fill_txfifo(struct fsl_qspi *q)
 	return q->devtype_data->driver_data & QUADSPI_QUIRK_TKT253890;
 }
 
+static inline int needs_wakeup_wait_mode(struct fsl_qspi *q)
+{
+	return q->devtype_data->driver_data & QUADSPI_QUIRK_TKT245618;
+}
+
 /*
  * An IC bug makes us to re-arrange the 32-bit data.
  * The following chips, such as IMX6SLX, have fixed this bug.
@@ -668,12 +678,18 @@ static int fsl_qspi_clk_prep_enable(struct fsl_qspi *q)
 		return ret;
 	}
 
+	if (needs_wakeup_wait_mode(q))
+		pm_qos_add_request(&q->pm_qos_req, PM_QOS_CPU_DMA_LATENCY, 0);
+
 	return 0;
 }
 
 /* This function was used to disable and unprepare QSPI clock */
 static void fsl_qspi_clk_disable_unprep(struct fsl_qspi *q)
 {
+	if (needs_wakeup_wait_mode(q))
+		pm_qos_remove_request(&q->pm_qos_req);
+
 	clk_disable_unprepare(q->clk);
 	clk_disable_unprepare(q->clk_en);
 
@@ -925,6 +941,10 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	if (!q->nor_num || q->nor_num > FSL_QSPI_MAX_CHIP)
 		return -ENODEV;
 
+	q->dev = dev;
+	q->devtype_data = (struct fsl_qspi_devtype_data *)of_id->data;
+	platform_set_drvdata(pdev, q);
+
 	/* find the resources */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "QuadSPI");
 	q->iobase = devm_ioremap_resource(dev, res);
@@ -964,10 +984,6 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 		goto irq_failed;
 	}
 
-	q->dev = dev;
-	q->devtype_data = (struct fsl_qspi_devtype_data *)of_id->data;
-	platform_set_drvdata(pdev, q);
-
 	ret = fsl_qspi_nor_setup(q);
 	if (ret)
 		goto irq_failed;
-- 
1.9.1

[PATCH v3 7/8] mtd: spi-nor: fsl-quadspi: reset the module in the probe

[Frank.Li]

From: Frank Li <Frank.Li@freescale.com>

The uboot may run the QuadSpi controler with command:
    #sf probe

So we should reset the module in the probe.
This patch also clear the pending interrupts which arised by the uboot
code.

Signed-off-by: Huang Shijie <shijie8@gmail.com>
Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 0ddc2d1..0f3f22d 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -715,6 +715,11 @@ static int fsl_qspi_nor_setup(struct fsl_qspi *q)
 	if (ret)
 		return ret;
 
+	/* Reset the module */
+	writel(QUADSPI_MCR_SWRSTSD_MASK | QUADSPI_MCR_SWRSTHD_MASK,
+		base + QUADSPI_MCR);
+	udelay(1);
+
 	/* Init the LUT table. */
 	fsl_qspi_init_lut(q);
 
@@ -732,6 +737,9 @@ static int fsl_qspi_nor_setup(struct fsl_qspi *q)
 	writel(QUADSPI_MCR_RESERVED_MASK | QUADSPI_MCR_END_CFG_MASK,
 			base + QUADSPI_MCR);
 
+	/* clear all interrupt status */
+	writel(0xffffffff, q->iobase + QUADSPI_FR);
+
 	/* enable the interrupt */
 	writel(QUADSPI_RSER_TFIE, q->iobase + QUADSPI_RSER);
 
-- 
1.9.1

[PATCH v3 8/8] mtd: spi-nor: fsl-quadspi: fix unsupported cmd when run flash_erase

[Frank.Li]

From: Frank Li <Frank.Li@freescale.com>

fsl-quadspi 21e0000.qspi: Unsupported cmd 0x20

when config CONFIG_MTD_SPI_NOR_USE_4K_SECTORS enable,
erase will use SPINOR_OP_BE_4K, which was not supported by fsl-quadspi
driver

Signed-off-by: Frank Li <Frank.Li@freescale.com>
Acked-by: Allen Xu <b45815@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 0f3f22d..e50da5c 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -396,11 +396,11 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	lut_base = SEQID_SE * 4;
 
 	if (q->nor_size <= SZ_16M) {
-		cmd = SPINOR_OP_SE;
+		cmd = q->nor[0].erase_opcode;
 		addrlen = ADDR24BIT;
 	} else {
 		/* use the 4-byte address */
-		cmd = SPINOR_OP_SE;
+		cmd = q->nor[0].erase_opcode;
 		addrlen = ADDR32BIT;
 	}
 
@@ -471,6 +471,8 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
 	case SPINOR_OP_BRWR:
 		return SEQID_BRWR;
 	default:
+		if (cmd == q->nor[0].erase_opcode)
+			return SEQID_SE;
 		dev_err(q->dev, "Unsupported cmd 0x%.2x\n", cmd);
 		break;
 	}
-- 
1.9.1

Re: [PATCH v3 0/8] mtd: spi-nor: fsl-quadspi fix and added i.mx7d and i.mxul support

[Brian Norris]

On Sat, Jul 25, 2015 at 02:06:20AM +0800, Frank.Li@freescale.com wrote:
> From: Frank Li <Frank.Li@freescale.com>
> 
> rebase to l2-mtd/next
> 
> Resend whole serial patch

Han Xu (listed in MAINTAINERS) has graciously ack'd all of your patches.
In the future, please include those next to the Signed-off-by's when you
resend, unless you've had to make major changes.

I also notice that you failed to CC him. scripts/get_maintainer.pl
should have suggested you include him.

Brian

Re: [PATCH v3 0/8] mtd: spi-nor: fsl-quadspi fix and added i.mx7d and i.mxul support

[Zhi Li]

On Fri, Jul 24, 2015 at 2:42 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Sat, Jul 25, 2015 at 02:06:20AM +0800, Frank.Li@freescale.com wrote:
>> From: Frank Li <Frank.Li@freescale.com>
>>
>> rebase to l2-mtd/next
>>
>> Resend whole serial patch
>
> Han Xu (listed in MAINTAINERS) has graciously ack'd all of your patches.
> In the future, please include those next to the Signed-off-by's when you
> resend, unless you've had to make major changes.

Okay, b45815@freescale.com is Han xu's email address.

best regards
Frank Li

>
> I also notice that you failed to CC him. scripts/get_maintainer.pl
> should have suggested you include him.
>
> Brian

Re: [PATCH v3 0/8] mtd: spi-nor: fsl-quadspi fix and added i.mx7d and i.mxul support

[Zhi Li]

On Fri, Jul 24, 2015 at 2:46 PM, Zhi Li <lznuaa@gmail.com> wrote:
> On Fri, Jul 24, 2015 at 2:42 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
>> On Sat, Jul 25, 2015 at 02:06:20AM +0800, Frank.Li@freescale.com wrote:
>>> From: Frank Li <Frank.Li@freescale.com>
>>>
>>> rebase to l2-mtd/next
>>>
>>> Resend whole serial patch
>>
>> Han Xu (listed in MAINTAINERS) has graciously ack'd all of your patches.
>> In the future, please include those next to the Signed-off-by's when you
>> resend, unless you've had to make major changes.
>
> Okay, b45815@freescale.com is Han xu's email address.

I know why you confused.

Allen Xu <b45815@freescale.com>, Han Xu <b45815@freescale.com> and
han.xu@freescale.com
is the same person.

best regards
Frank Li

>
> best regards
> Frank Li
>
>>
>> I also notice that you failed to CC him. scripts/get_maintainer.pl
>> should have suggested you include him.
>>
>> Brian

Re: [PATCH v3 0/8] mtd: spi-nor: fsl-quadspi fix and added i.mx7d and i.mxul support

[Brian Norris]

On Fri, Jul 24, 2015 at 02:51:57PM -0500, Zhi Li wrote:
> On Fri, Jul 24, 2015 at 2:46 PM, Zhi Li <lznuaa@gmail.com> wrote:
> > On Fri, Jul 24, 2015 at 2:42 PM, Brian Norris
> > <computersforpeace@gmail.com> wrote:
> >> Han Xu (listed in MAINTAINERS) has graciously ack'd all of your patches.
> >> In the future, please include those next to the Signed-off-by's when you
> >> resend, unless you've had to make major changes.
> >
> > Okay, b45815@freescale.com is Han xu's email address.
> 
> I know why you confused.
> 
> Allen Xu <b45815@freescale.com>, Han Xu <b45815@freescale.com> and
> han.xu@freescale.com
> is the same person.

I see. Yes, that clears up my current confusion, though I can't
guarantee no one else will be confused in the future.

Regards,
Brian

Re: [PATCH v3 0/8] mtd: spi-nor: fsl-quadspi fix and added i.mx7d and i.mxul support

[Zhi Li]

On Fri, Jul 24, 2015 at 2:54 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> Yes, that clears up my current confusion, though I can't
> guarantee no one else will be confused in the future.

Okay, I will tell han xu.
Just use Han Xu <han.xu@freescale.com> in future, which in MAINTAINERS.
Sorry for that.

best regards
Frank Li

Re: [PATCH v3 1/8] mtd: spi-nor: fsl-qspi: dynamically map memory space for AHB read

[Brian Norris]

Hi Frank,

Thanks for getting all the dependencies in one place, so I can actually
review them. A few comments:

On Sat, Jul 25, 2015 at 02:06:21AM +0800, Frank.Li@freescale.com wrote:
> From: Allen Xu <b45815@freescale.com>
> 
> QSPI may failed to map enough memory (256MB) for AHB read in
> previous implementation, especially in 3G/1G memory layout kernel.
> Dynamically map memory to avoid such issue.
> 
> This implementation generally map 4MB memory for AHB read, it should
> be enough for common scenarios, and the side effect (0.6% performance
> drop) is minor.

So, you're running out of virtual address space? What says 4MB will be
small enough? At a minimum, we should factor out the magic constant
(SZ_4M) to a macro, so that if someone needs to change it, it's
relatively easy.

> Previous implementation
> 
> root@imx6qdlsolo:~# dd if=/dev/mtd0 of=/dev/null bs=1K count=32K
> 32768+0 records in
> 32768+0 records out
> 33554432 bytes (34 MB) copied, 2.16006 s, 15.5 MB/s
> 
> root@imx6qdlsolo:~# dd if=/dev/mtd0 of=/dev/null bs=32M count=1
> 1+0 records in
> 1+0 records out
> 33554432 bytes (34 MB) copied, 1.43149 s, 23.4 MB/s
> 
> After applied the patch
> 
> root@imx6qdlsolo:~# dd if=/dev/mtd0 of=/dev/null bs=1K count=32K
> 32768+0 records in
> 32768+0 records out
> 33554432 bytes (34 MB) copied, 2.1743 s, 15.4 MB/s
> 
> root@imx6qdlsolo:~# dd if=/dev/mtd0 of=/dev/null bs=32M count=1
> 1+0 records in
> 1+0 records out
> 33554432 bytes (34 MB) copied, 1.43158 s, 23.4 MB/s
> 
> Signed-off-by: Allen Xu <b45815@freescale.com>
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 48 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 4fe13dd..ac9d633 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -223,8 +223,10 @@ struct fsl_qspi {
>  	struct mtd_info mtd[FSL_QSPI_MAX_CHIP];
>  	struct spi_nor nor[FSL_QSPI_MAX_CHIP];
>  	void __iomem *iobase;
> -	void __iomem *ahb_base; /* Used when read from AHB bus */
> +	void __iomem *ahb_addr;
>  	u32 memmap_phy;
> +	u32 memmap_offs;
> +	u32 memmap_len;
>  	struct clk *clk, *clk_en;
>  	struct device *dev;
>  	struct completion c;
> @@ -732,11 +734,41 @@ static int fsl_qspi_read(struct spi_nor *nor, loff_t from,
>  	struct fsl_qspi *q = nor->priv;
>  	u8 cmd = nor->read_opcode;
>  
> -	dev_dbg(q->dev, "cmd [%x],read from (0x%p, 0x%.8x, 0x%.8x),len:%d\n",
> -		cmd, q->ahb_base, q->chip_base_addr, (unsigned int)from, len);
> +	/* if necessary,ioremap buffer before AHB read, */
> +	/* generally 4MB should be large enough */

Can you use the proper multiline comment style? It would look something
like this:

	/*
	 * If necessary, ioremap buffer before AHB read. Generally, 4MB
	 * should be large enough
	 */

> +	if (!q->ahb_addr) {
> +		q->ahb_addr = ioremap_nocache(
> +				q->memmap_phy + q->chip_base_addr + from,
> +				len > SZ_4M ? len : SZ_4M);

Here, we should factor out the magic constant as its own macro
(QUADSPI_MAX_IOMAP?). You can also reduce the duplication of these
offset and length computations by rearranging this block to look more
like:

		q->memmap_offs = q->chip_base_addr + from;
		q->memmap_len = len > SZ_4M ? len : SZ_4M;
		q->ahb_addr = ioremap_nocache(q->memmap_phy + q->memmap_offs,
					      q->memmap_len);
		if (!q->ahb_addr) {
			...

You can do similarly for the 'else if' clause.

> +		if (!q->ahb_addr) {
> +			dev_err(q->dev, "ioremap failed\n");
> +			return -ENOMEM;
> +		}
> +		q->memmap_offs = q->chip_base_addr + from;
> +		q->memmap_len = len > SZ_4M ? len : SZ_4M;
> +	/* ioremap if the data requested is out of range */
> +	} else if (q->chip_base_addr + from < q->memmap_offs
> +			|| q->chip_base_addr + from + len >
> +			q->memmap_offs + q->memmap_len) {
> +		iounmap(q->ahb_addr);
> +		q->ahb_addr = ioremap_nocache(
> +				q->memmap_phy + q->chip_base_addr + from,
> +				len > SZ_4M ? len : SZ_4M);
> +		if (!q->ahb_addr) {
> +			dev_err(q->dev, "ioremap failed\n");
> +			return -ENOMEM;
> +		}
> +		q->memmap_offs = q->chip_base_addr + from;
> +		q->memmap_len = len > SZ_4M ? len : SZ_4M;
> +	}
> +
> +	dev_dbg(q->dev, "cmd [%x],read from 0x%p, len:%d\n",
> +		cmd, q->ahb_addr + q->chip_base_addr + from - q->memmap_offs,
> +		len);
>  
>  	/* Read out the data directly from the AHB buffer.*/
> -	memcpy(buf, q->ahb_base + q->chip_base_addr + from, len);
> +	memcpy(buf, q->ahb_addr + q->chip_base_addr + from - q->memmap_offs,
> +		len);
>  
>  	*retlen += len;
>  	return 0;
> @@ -821,10 +853,6 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>  
>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>  					"QuadSPI-memory");
> -	q->ahb_base = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(q->ahb_base))
> -		return PTR_ERR(q->ahb_base);
> -

Please reintroduce the call to devm_request_mem_region(). Something
like:

	if (!devm_request_mem_region(dev, res->start, resource_size(res),
				     res->name)) {
		dev_err(dev, "can't request region for resource %pR\n", res);
		return -EBUSY;
	}

>  	q->memmap_phy = res->start;
>  
>  	/* find the clocks */
> @@ -989,6 +1017,10 @@ static int fsl_qspi_remove(struct platform_device *pdev)
>  	mutex_destroy(&q->lock);
>  	clk_unprepare(q->clk);
>  	clk_unprepare(q->clk_en);
> +
> +	if (q->ahb_addr)
> +		iounmap(q->ahb_addr);
> +
>  	return 0;
>  }
>  

Regards,
Brian

Re: [PATCH v3 2/8] mtd: spi-nor: fsl-quadspi: use quirk to distinguish different qspi version

[Brian Norris]

On Sat, Jul 25, 2015 at 02:06:22AM +0800, Frank.Li@freescale.com wrote:
> From: Han Xu <b45815@freescale.com>
> 
> add several quirk to distinguish different version of qspi module.
> 
> Signed-off-by: Han Xu <b45815@freescale.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index ac9d633..5f31bc7 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -28,6 +28,11 @@
>  #include <linux/mtd/spi-nor.h>
>  #include <linux/mutex.h>
>  
> +/* Controller needs driver to swap endian */
> +#define QUADSPI_QUIRK_SWAP_ENDIAN	(1 << 0)
> +/* Controller needs 4x internal clock */
> +#define QUADSPI_QUIRK_4X_INT_CLK	(1 << 1)
> +
>  /* The registers */
>  #define QUADSPI_MCR			0x00
>  #define QUADSPI_MCR_RESERVED_SHIFT	16
> @@ -202,20 +207,23 @@ struct fsl_qspi_devtype_data {
>  	int rxfifo;
>  	int txfifo;
>  	int ahb_buf_size;
> +	int driver_data;
>  };
>  
>  static struct fsl_qspi_devtype_data vybrid_data = {
>  	.devtype = FSL_QUADSPI_VYBRID,
>  	.rxfifo = 128,
>  	.txfifo = 64,
> -	.ahb_buf_size = 1024
> +	.ahb_buf_size = 1024,
> +	.driver_data = QUADSPI_QUIRK_SWAP_ENDIAN

All field entries (including the last) should end the line with a comma,
so if you add fields in the future, you don't need to mess with the
previous line. This helps to keep the line-diff history a little more
clear.

>  };
>  
>  static struct fsl_qspi_devtype_data imx6sx_data = {
>  	.devtype = FSL_QUADSPI_IMX6SX,
>  	.rxfifo = 128,
>  	.txfifo = 512,
> -	.ahb_buf_size = 1024
> +	.ahb_buf_size = 1024,
> +	.driver_data = QUADSPI_QUIRK_4X_INT_CLK

Ditto.

Otherwise, looks good:

Reviewed-by: Brian Norris <computersforpeace@gmail.com>

>  };
>  
>  #define FSL_QSPI_MAX_CHIP	4
> @@ -239,14 +247,14 @@ struct fsl_qspi {
>  	struct mutex lock;
>  };
>  
> -static inline int is_vybrid_qspi(struct fsl_qspi *q)
> +static inline int needs_swap_endian(struct fsl_qspi *q)
>  {
> -	return q->devtype_data->devtype == FSL_QUADSPI_VYBRID;
> +	return q->devtype_data->driver_data & QUADSPI_QUIRK_SWAP_ENDIAN;
>  }
>  
> -static inline int is_imx6sx_qspi(struct fsl_qspi *q)
> +static inline int needs_4x_clock(struct fsl_qspi *q)
>  {
> -	return q->devtype_data->devtype == FSL_QUADSPI_IMX6SX;
> +	return q->devtype_data->driver_data & QUADSPI_QUIRK_4X_INT_CLK;
>  }
>  
>  /*
> @@ -255,7 +263,7 @@ static inline int is_imx6sx_qspi(struct fsl_qspi *q)
>   */
>  static inline u32 fsl_qspi_endian_xchg(struct fsl_qspi *q, u32 a)
>  {
> -	return is_vybrid_qspi(q) ? __swab32(a) : a;
> +	return needs_swap_endian(q) ? __swab32(a) : a;
>  }
>  
>  static inline void fsl_qspi_unlock_lut(struct fsl_qspi *q)
> @@ -650,7 +658,7 @@ static int fsl_qspi_nor_setup_last(struct fsl_qspi *q)
>  	unsigned long rate = q->clk_rate;
>  	int ret;
>  
> -	if (is_imx6sx_qspi(q))
> +	if (needs_4x_clock(q))
>  		rate *= 4;
>  
>  	ret = clk_set_rate(q->clk, rate);
> -- 
> 1.9.1
> 

Re: [PATCH v3 3/8] mtd: spi-nor: fsl-quadspi: add imx7d support

[Brian Norris]

On Sat, Jul 25, 2015 at 02:06:23AM +0800, Frank.Li@freescale.com wrote:
> From: Frank Li <Frank.Li@freescale.com>
> 
> Support i.mx7d.
> quadspi in i.mx7d increase rxfifo.
> require fill at least 16byte to trigger data transfer.
> 
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> Signed-off-by: Han Xu <b45815@freescale.com>

Looks good. I can take it when you resend the fixed-up series:

Reviewed-by: Brian Norris <computersforpeace@gmail.com>

Re: [PATCH v3 3/8] mtd: spi-nor: fsl-quadspi: add imx7d support

[Brian Norris]

Forgot one thing:

On Sat, Jul 25, 2015 at 02:06:23AM +0800, Frank.Li@freescale.com wrote:
> From: Frank Li <Frank.Li@freescale.com>
> 
> Support i.mx7d.
> quadspi in i.mx7d increase rxfifo.
> require fill at least 16byte to trigger data transfer.
> 
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> Signed-off-by: Han Xu <b45815@freescale.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 5f31bc7..3fc94ad 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
...
> @@ -677,6 +702,7 @@ static int fsl_qspi_nor_setup_last(struct fsl_qspi *q)
>  static const struct of_device_id fsl_qspi_dt_ids[] = {
>  	{ .compatible = "fsl,vf610-qspi", .data = (void *)&vybrid_data, },
>  	{ .compatible = "fsl,imx6sx-qspi", .data = (void *)&imx6sx_data, },
> +	{ .compatible = "fsl,imx7d-qspi", .data = (void *)&imx7d_data, },

^^ This compatible string needs to be documented in
Documentation/devicetree/bindings/...

Please add a patch for the new properties in your next revision.

Brian

>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, fsl_qspi_dt_ids);
> -- 
> 1.9.1
> 

Re: [PATCH v3 4/8] mtd: spi-nor: fsl-quadspi: add i.mx6ul support

[Brian Norris]

Same comments from the other patches:

On Sat, Jul 25, 2015 at 02:06:24AM +0800, Frank.Li@freescale.com wrote:
> From: Frank Li <Frank.Li@freescale.com>
> 
> Add i.mx6ul chip support
> 
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> Acked-by: Allen Xu <b45815@freescale.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 3fc94ad..3746542 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -206,6 +206,7 @@ enum fsl_qspi_devtype {
>  	FSL_QUADSPI_VYBRID,
>  	FSL_QUADSPI_IMX6SX,
>  	FSL_QUADSPI_IMX7D,
> +	FSL_QUADSPI_IMX6UL,
>  };
>  
>  struct fsl_qspi_devtype_data {
> @@ -241,6 +242,15 @@ static struct fsl_qspi_devtype_data imx7d_data = {
>  		       | QUADSPI_QUIRK_4X_INT_CLK
>  };
>  
> +static struct fsl_qspi_devtype_data imx6ul_data = {
> +	.devtype = FSL_QUADSPI_IMX6UL,
> +	.rxfifo = 128,
> +	.txfifo = 512,
> +	.ahb_buf_size = 1024,
> +	.driver_data = QUADSPI_QUIRK_TKT253890
> +		       | QUADSPI_QUIRK_4X_INT_CLK

End this line with a comma.

> +};
> +
>  #define FSL_QSPI_MAX_CHIP	4
>  struct fsl_qspi {
>  	struct mtd_info mtd[FSL_QSPI_MAX_CHIP];
> @@ -703,6 +713,7 @@ static const struct of_device_id fsl_qspi_dt_ids[] = {
>  	{ .compatible = "fsl,vf610-qspi", .data = (void *)&vybrid_data, },
>  	{ .compatible = "fsl,imx6sx-qspi", .data = (void *)&imx6sx_data, },
>  	{ .compatible = "fsl,imx7d-qspi", .data = (void *)&imx7d_data, },
> +	{ .compatible = "fsl,imx6ul-qspi", .data = (void *)&imx6ul_data, },

Document this compatible property.

>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, fsl_qspi_dt_ids);
> -- 
> 1.9.1
> 

Brian

Re: [PATCH v3 5/8] mtd: spi-nor: fsl-quadspi: i.MX6SX: fixed the random QSPI access failed issue

[Brian Norris]

On Sat, Jul 25, 2015 at 02:06:25AM +0800, Frank.Li@freescale.com wrote:
> From: Allen Xu <b45815@freescale.com>
> 
> We found there is a low probability(5%) QSPI access timeout issue,
> usually it happened on kernel boot stage, the first time kernel tried to
> access QSPI chip. The READ_ID command was sent but not executed,
> consequently the probe function failed.
> 
> Finally we located the issue by these steps.
> 
> 1. Since the issue happened randomly and usually it cost half day to
> reproduce, we add more debug code in driver, to create a timeout file if
> the issue occurred.
> 
> 2. Prepared an autorun script to keep rebooting the system and check if
> the timeout file existed, if the file existed, stop reboot.
> 
> 3. The system will stop rebooting when timeout error occurred, set the
> CCM_CCOSR register and related IOMUX to measure QPSI clock, found there
> is no clock output, while clock output can be measured when QSPI driver
> successfully probed.
> 
> 4. Check the code and found QSPI clock rate was changed while not
> disabled clock gate, most of the multiplexers on i.MX6 are glitch ones,
> clock glitch may occurred and propagated into downstream clock dividers
> 
> Based on the new clock flag(CLK_SET_RATE_GATE) and new framework, we
> need to change the approach of seting clock rate. In current
> implementation, there are several places in which the clock was touched.

I appreciate the long, descriptive commit message, but I feel like
there's still an important detail that's not very obvious -- what was
the actual *bug* you're solving? (You spend more time on how you found
the problem (which is nice, but readers probably don't really care), and
leave the actual problem to be inferred by reading up on
CLK_SET_RATE_GATE.)

AIUI, the problem is that your clocks need to be gated when their rate
is changed, correct? Please put a short note here that explains it.

> 1. probe function. prepare and enable clock before setting the QSPI
> register, disable and unprepare the clock before exit.
> 
> 2. nor_setup & nor_setup_last, since we change clock rate in these two
> functions.
> 
> 3. fsl_qspi_prep and fsl_qspi_unprep, clock was enabled only when got
> QSPI access request.
> 
> 4. resume function. Clock was required to restroe the setting after
> resume, disable the clock before exit.
> 
> Signed-off-by: Allen Xu <b45815@freescale.com>
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 83 +++++++++++++++++++++++++++------------
>  1 file changed, 58 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 3746542..9ee6db9 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -653,6 +653,32 @@ static void fsl_qspi_init_abh_read(struct fsl_qspi *q)
>  		q->iobase + QUADSPI_BFGENCR);
>  }
>  
> +/* This function was used to prepare and enable QSPI clock */
> +static int fsl_qspi_clk_prep_enable(struct fsl_qspi *q)
> +{
> +	int ret;
> +
> +	ret = clk_prepare_enable(q->clk_en);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(q->clk);
> +	if (ret) {
> +		clk_disable_unprepare(q->clk_en);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/* This function was used to disable and unprepare QSPI clock */
> +static void fsl_qspi_clk_disable_unprep(struct fsl_qspi *q)
> +{
> +	clk_disable_unprepare(q->clk);
> +	clk_disable_unprepare(q->clk_en);
> +
> +}
> +
>  /* We use this function to do some basic init for spi_nor_scan(). */
>  static int fsl_qspi_nor_setup(struct fsl_qspi *q)
>  {
> @@ -660,11 +686,19 @@ static int fsl_qspi_nor_setup(struct fsl_qspi *q)
>  	u32 reg;
>  	int ret;
>  
> -	/* the default frequency, we will change it in the future.*/
> +	/* disable and unprepare clock first */

The comment (above) doesn't add much that the function name (below)
doesn't say already (i.e., the function says "disable" and "unprep").
But perhaps you can explain the "why"? e.g.:

	/* Clocks must be gated when changing rates, to avoid glitching */

> +	fsl_qspi_clk_disable_unprep(q);
> +
> +	/* the default frequency, we will change it in the future. */
>  	ret = clk_set_rate(q->clk, 66000000);
>  	if (ret)
>  		return ret;
>  
> +	/* prepare and enable the clock */
> +	ret = fsl_qspi_clk_prep_enable(q);
> +	if (ret)
> +		return ret;
> +
>  	/* Init the LUT table. */
>  	fsl_qspi_init_lut(q);
>  
> @@ -696,10 +730,18 @@ static int fsl_qspi_nor_setup_last(struct fsl_qspi *q)
>  	if (needs_4x_clock(q))
>  		rate *= 4;
>  
> +	/* disable and unprepare clock first */

Same here.

> +	fsl_qspi_clk_disable_unprep(q);
> +
>  	ret = clk_set_rate(q->clk, rate);
>  	if (ret)
>  		return ret;
>  
> +	/* prepare and enable the clock */
> +	ret = fsl_qspi_clk_prep_enable(q);
> +	if (ret)
> +		return ret;
> +
>  	/* Init the LUT table again. */
>  	fsl_qspi_init_lut(q);
>  
...

Brian

Re: [PATCH v3 8/8] mtd: spi-nor: fsl-quadspi: fix unsupported cmd when run flash_erase

[Brian Norris]

On Sat, Jul 25, 2015 at 02:06:28AM +0800, Frank.Li@freescale.com wrote:
> From: Frank Li <Frank.Li@freescale.com>
> 
> fsl-quadspi 21e0000.qspi: Unsupported cmd 0x20
> 
> when config CONFIG_MTD_SPI_NOR_USE_4K_SECTORS enable,
> erase will use SPINOR_OP_BE_4K, which was not supported by fsl-quadspi
> driver

Slightly off topic: things looks pretty fragile here. /me thinks most of
this can be written more cleanly...

> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> Acked-by: Allen Xu <b45815@freescale.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 0f3f22d..e50da5c 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -396,11 +396,11 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  	lut_base = SEQID_SE * 4;
>  
>  	if (q->nor_size <= SZ_16M) {
> -		cmd = SPINOR_OP_SE;
> +		cmd = q->nor[0].erase_opcode;
>  		addrlen = ADDR24BIT;
>  	} else {
>  		/* use the 4-byte address */
> -		cmd = SPINOR_OP_SE;
> +		cmd = q->nor[0].erase_opcode;
>  		addrlen = ADDR32BIT;
>  	}

This whole block can be refactored to:

	cmd = q->nor[0].erase_opcode;
	addrlen = q->nor_size <= SZ_16M ? ADDR24BIT : ADDR32BIT;

But really, this should be based on nor->addr_width...

>  
> @@ -471,6 +471,8 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
>  	case SPINOR_OP_BRWR:
>  		return SEQID_BRWR;
>  	default:
> +		if (cmd == q->nor[0].erase_opcode)

Related question: what happens if you have multiple flash chips
connected, and they don't need the same opcodes? It looks like you
program the LUT only for the opcodes of the first flash, so the second
wouldn't work right.

> +			return SEQID_SE;
>  		dev_err(q->dev, "Unsupported cmd 0x%.2x\n", cmd);
>  		break;
>  	}

Brian

Re: [PATCH v3 8/8] mtd: spi-nor: fsl-quadspi: fix unsupported cmd when run flash_erase

[Zhi Li]

On Fri, Jul 31, 2015 at 4:20 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Sat, Jul 25, 2015 at 02:06:28AM +0800, Frank.Li@freescale.com wrote:
>> From: Frank Li <Frank.Li@freescale.com>
>>
>> fsl-quadspi 21e0000.qspi: Unsupported cmd 0x20
>>
>> when config CONFIG_MTD_SPI_NOR_USE_4K_SECTORS enable,
>> erase will use SPINOR_OP_BE_4K, which was not supported by fsl-quadspi
>> driver
>
> Slightly off topic: things looks pretty fragile here. /me thinks most of
> this can be written more cleanly...
>
>> Signed-off-by: Frank Li <Frank.Li@freescale.com>
>> Acked-by: Allen Xu <b45815@freescale.com>
>> ---
>>  drivers/mtd/spi-nor/fsl-quadspi.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
>> index 0f3f22d..e50da5c 100644
>> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
>> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
>> @@ -396,11 +396,11 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>>       lut_base = SEQID_SE * 4;
>>
>>       if (q->nor_size <= SZ_16M) {
>> -             cmd = SPINOR_OP_SE;
>> +             cmd = q->nor[0].erase_opcode;
>>               addrlen = ADDR24BIT;
>>       } else {
>>               /* use the 4-byte address */
>> -             cmd = SPINOR_OP_SE;
>> +             cmd = q->nor[0].erase_opcode;
>>               addrlen = ADDR32BIT;
>>       }
>
> This whole block can be refactored to:
>
>         cmd = q->nor[0].erase_opcode;
>         addrlen = q->nor_size <= SZ_16M ? ADDR24BIT : ADDR32BIT;
>
> But really, this should be based on nor->addr_width...
>
>>
>> @@ -471,6 +471,8 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
>>       case SPINOR_OP_BRWR:
>>               return SEQID_BRWR;
>>       default:
>> +             if (cmd == q->nor[0].erase_opcode)
>
> Related question: what happens if you have multiple flash chips
> connected, and they don't need the same opcodes? It looks like you
> program the LUT only for the opcodes of the first flash, so the second
> wouldn't work right.

Fsl-qspi controller assume you connect two same flash chips.
Two chip share the same opcodes.  all other command
like read,  both chip also must use the same opcode.

Frank Li

>
>> +                     return SEQID_SE;
>>               dev_err(q->dev, "Unsupported cmd 0x%.2x\n", cmd);
>>               break;
>>       }
>
> Brian