[RFC/RFT PATCH v1 0/9] mtd: fsl: quadspi: Fixes for fsl-quadspi.c driver (vybrid HW)

[RFC/RFT PATCH v1 0/9] mtd: fsl: quadspi: Fixes for fsl-quadspi.c driver (vybrid HW)

[Lukasz Majewski]

Please find following set of patches, which provide improved behaviour
of the fsl-quadspi.c driver on Vybrid vf610 HW.

Below code is based on previous work done by Albert ARIBAUD:
https://patchwork.ozlabs.org/patch/675401/

I've cleaned up the code a bit, make separate patches, exclude not
needed parts and port it to v4.19-rc3.

After applying those patches, the quadspi controller on vf610 works
with UBI/UBIFS.
The problem is with some corner case writes to raw MTD device
(like writing 1023B with single byte writes). Those fail sometimes.

Regression tests can be found in the following repository:
https://github.com/lmajewski/tests-spi/tree/master/tests

(Please read README.txt).

The NXP's community thread regarding HW issues in this block on
the vf610:
https://community.nxp.com/thread/485139

Maybe those patches will be helpful with the new, work in progress
driver for spi-fsl-qspi.c

Lukasz Majewski (9):
  Revert "mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ"
  mtd: qspi: Provide quirk to read only half of RX buffer (NXP's vybrid)
  mtd: spi: Do not setup the default seqid as we got it set for DUAL and
    QUAD
  mtd: spi: Modify the HW capability mask according to supported RX
    lanes
  mtd: spi: Provide LUT entry to perform DUAL read
  mtd: spi: Enhance the fsl_qspi_read() method to support DUAL and QUAD
  mtd: spi: Add SPI_NOR_DUAL_READ property for the 'n25q128a13' Micron
    memory
  mtd: spi: Allocate memory corresponding to maximal fsl-quadspi.c
    controller area
  mtd: spi: Skip reading the Serial Flash Discoverable Parameters

 drivers/mtd/spi-nor/fsl-quadspi.c | 103 +++++++++++++++++++++++++++++---------
 drivers/mtd/spi-nor/spi-nor.c     |   2 +-
 2 files changed, 79 insertions(+), 26 deletions(-)

-- 
2.11.0

[RFC/RFT PATCH v1 1/9] Revert "mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ"

[Lukasz Majewski]

This reverts commit 9b2a34906c917e6dacfa08f2eafa5beb8baff5e1, as
it is misleading to have READ_QSPI only when we do plan to use
READ_DUAL_QSPI.

Lets bring back the old naming (explicitly showing the QUAD operation).

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 7d9620c7ff6c..f67f3fa5b9c9 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -193,7 +193,7 @@
 #define QUADSPI_LUT_NUM		64
 
 /* SEQID -- we can have 16 seqids at most. */
-#define SEQID_READ		0
+#define SEQID_QUAD_READ		0
 #define SEQID_WREN		1
 #define SEQID_WRDI		2
 #define SEQID_RDSR		3
@@ -396,8 +396,8 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	for (i = 0; i < QUADSPI_LUT_NUM; i++)
 		qspi_writel(q, 0, base + QUADSPI_LUT_BASE + i * 4);
 
-	/* Read */
-	lut_base = SEQID_READ * 4;
+	/* Quad Read */
+	lut_base = SEQID_QUAD_READ * 4;
 
 	qspi_writel(q, LUT0(CMD, PAD1, read_op) | LUT1(ADDR, PAD1, addrlen),
 			base + QUADSPI_LUT(lut_base));
@@ -478,7 +478,7 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
 {
 	switch (cmd) {
 	case SPINOR_OP_READ_1_1_4:
-		return SEQID_READ;
+		return SEQID_QUAD_READ;
 	case SPINOR_OP_WREN:
 		return SEQID_WREN;
 	case SPINOR_OP_WRDI:
-- 
2.11.0

[RFC/RFT PATCH v1 2/9] mtd: qspi: Provide quirk to read only half of RX buffer (NXP's vybrid)

[Lukasz Majewski]

This commit introduces new quirk for the NXP's quadspi driver for vybrid
SoC. It turns out that when reading for example 256B as single bytes:
dd if=/dev/mtd7 of=aaa.img bs=1 count=256

root@nix:~# hexdump aaa.img
0000000 464c eec0 baa5 c5ff 7b99 4dfb e0b6 8a2e
0000010 e98e 5265 683c a635 c069 e402 303f d936
0000020 c243 01a7 7064 fce8 e3a9 200a 7e85 28bc
0000030 4296 a30e 1bb4 88d4 b456 b4a6 f3aa 8cff
0000040 01c9 462d 0a43 f893 0e42 67f1 57f0 787c
0000050 49c0 fb2a e514 e954 1d21 affa bac4 38f1
0000060 1ca5 ec46 77eb a854 285b 8e21 12d7 f377
0000070 ffff ffff ffff ffff ffff ffff ffff ffff
*
0000100


Data got corrupted.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index f67f3fa5b9c9..2ef5bfc41d32 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -41,6 +41,8 @@
 #define QUADSPI_QUIRK_TKT253890		(1 << 2)
 /* Controller cannot wake up from wait mode, TKT245618 */
 #define QUADSPI_QUIRK_TKT245618         (1 << 3)
+/* Controller can only read half a rx fifo through AHB */
+#define QUADSPI_QUIRK_AHB_READ_HALF_FIFO  (1 << 4)
 
 /* The registers */
 #define QUADSPI_MCR			0x00
@@ -230,7 +232,8 @@ static const struct fsl_qspi_devtype_data vybrid_data = {
 	.rxfifo = 128,
 	.txfifo = 64,
 	.ahb_buf_size = 1024,
-	.driver_data = QUADSPI_QUIRK_SWAP_ENDIAN,
+	.driver_data = QUADSPI_QUIRK_SWAP_ENDIAN
+		       | QUADSPI_QUIRK_AHB_READ_HALF_FIFO,
 };
 
 static const struct fsl_qspi_devtype_data imx6sx_data = {
@@ -341,6 +344,11 @@ static u32 qspi_readl(struct fsl_qspi *q, void __iomem *addr)
 		return ioread32(addr);
 }
 
+static inline int needs_half_fifo(struct fsl_qspi *q)
+{
+	return q->devtype_data->driver_data & QUADSPI_QUIRK_AHB_READ_HALF_FIFO;
+}
+
 /*
  * An IC bug makes us to re-arrange the 32-bit data.
  * The following chips, such as IMX6SLX, have fixed this bug.
@@ -390,6 +398,10 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	u8 read_op = nor->read_opcode;
 	u8 read_dm = nor->read_dummy;
 
+	/* use only half fifo if controller needs that */
+	if (needs_half_fifo(q))
+		rxfifo /= 2;
+
 	fsl_qspi_unlock_lut(q);
 
 	/* Clear all the LUT table */
@@ -675,6 +687,7 @@ static void fsl_qspi_init_ahb_read(struct fsl_qspi *q)
 {
 	void __iomem *base = q->iobase;
 	int seqid;
+	u32 buf3cr;
 
 	/* AHB configuration for access buffer 0/1/2 .*/
 	qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + QUADSPI_BUF0CR);
@@ -682,12 +695,14 @@ static void fsl_qspi_init_ahb_read(struct fsl_qspi *q)
 	qspi_writel(q, QUADSPI_BUFXCR_INVALID_MSTRID, base + QUADSPI_BUF2CR);
 	/*
 	 * Set ADATSZ with the maximum AHB buffer size to improve the
-	 * read performance.
+	 * read performance, except when the controller should not use
+	 * more than half its RX fifo in AHB reads, in which case read
+	 * size is given in the LUT FSL_READ instructions.
 	 */
-	qspi_writel(q, QUADSPI_BUF3CR_ALLMST_MASK |
-			((q->devtype_data->ahb_buf_size / 8)
-			<< QUADSPI_BUF3CR_ADATSZ_SHIFT),
-			base + QUADSPI_BUF3CR);
+	buf3cr = QUADSPI_BUF3CR_ALLMST_MASK
+		| ( (needs_half_fifo(q)? 0 : q->devtype_data->ahb_buf_size / 8)
+		    << QUADSPI_BUF3CR_ADATSZ_SHIFT);
+	writel(buf3cr, base + QUADSPI_BUF3CR);
 
 	/* We only use the buffer3 */
 	qspi_writel(q, 0, base + QUADSPI_BUF0IND);
-- 
2.11.0

[RFC/RFT PATCH v1 3/9] mtd: spi: Do not setup the default seqid as we got it set for DUAL and QUAD

[Lukasz Majewski]

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 2ef5bfc41d32..ad951a46a628 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -686,7 +686,6 @@ static void fsl_qspi_set_map_addr(struct fsl_qspi *q)
 static void fsl_qspi_init_ahb_read(struct fsl_qspi *q)
 {
 	void __iomem *base = q->iobase;
-	int seqid;
 	u32 buf3cr;
 
 	/* AHB configuration for access buffer 0/1/2 .*/
@@ -708,11 +707,6 @@ static void fsl_qspi_init_ahb_read(struct fsl_qspi *q)
 	qspi_writel(q, 0, base + QUADSPI_BUF0IND);
 	qspi_writel(q, 0, base + QUADSPI_BUF1IND);
 	qspi_writel(q, 0, base + QUADSPI_BUF2IND);
-
-	/* Set the default lut sequence for AHB Read. */
-	seqid = fsl_qspi_get_seqid(q, q->nor[0].read_opcode);
-	qspi_writel(q, seqid << QUADSPI_BFGENCR_SEQID_SHIFT,
-		q->iobase + QUADSPI_BFGENCR);
 }
 
 /* This function was used to prepare and enable QSPI clock */
-- 
2.11.0

[RFC/RFT PATCH v1 4/9] mtd: spi: Modify the HW capability mask according to supported RX lanes

[Lukasz Majewski]

It may happen that we got two identical SPI devices connected to the QSPI
controller with asymmetrical number of RX lanes. Due to PCB constraints,
one can work as DUAL and second as QUAD.

For such scenario we do need support for setting different read commands.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index ad951a46a628..4f0c78ba6fcb 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -977,7 +977,7 @@ static void fsl_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
 
 static int fsl_qspi_probe(struct platform_device *pdev)
 {
-	const struct spi_nor_hwcaps hwcaps = {
+	struct spi_nor_hwcaps hwcaps = {
 		.mask = SNOR_HWCAPS_READ_1_1_4 |
 			SNOR_HWCAPS_PP,
 	};
@@ -987,7 +987,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct spi_nor *nor;
 	struct mtd_info *mtd;
-	int ret, i = 0;
+	int ret, i = 0, width;
 
 	q = devm_kzalloc(dev, sizeof(*q), GFP_KERNEL);
 	if (!q)
@@ -1104,6 +1104,14 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 		if (ret < 0)
 			goto mutex_failed;
 
+		if (!of_property_read_u32(np, "spi-rx-bus-width", &width)) {
+			if (width == 2) {
+				hwcaps.mask &= ~SNOR_HWCAPS_READ_QUAD;
+				hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
+			} else if (width != 4)
+				return -EINVAL;
+		}
+
 		/* set the chip address for READID */
 		fsl_qspi_set_base_addr(q, nor);
 
-- 
2.11.0

[RFC/RFT PATCH v1 5/9] mtd: spi: Provide LUT entry to perform DUAL read

[Lukasz Majewski]

This commit adds LUT sequence to perform DUAL read (as opposite to the
default QUAD for this controller).

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 4f0c78ba6fcb..0c381cdfb39f 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -207,6 +207,7 @@
 #define SEQID_RDCR		9
 #define SEQID_EN4B		10
 #define SEQID_BRWR		11
+#define SEQID_DUAL_READ	12
 
 #define QUADSPI_MIN_IOMAP SZ_4M
 
@@ -482,6 +483,15 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
 	qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_BRWR),
 			base + QUADSPI_LUT(lut_base));
 
+	/* Dual Read */
+	lut_base = SEQID_DUAL_READ * 4;
+
+	qspi_writel(q, LUT0(CMD, PAD1, SPINOR_OP_READ_1_1_2) |
+		    LUT1(ADDR, PAD1, addrlen), base + QUADSPI_LUT(lut_base));
+	qspi_writel(q, LUT0(DUMMY, PAD1, read_dm) |
+		    LUT1(FSL_READ, PAD2, rxfifo),
+			base + QUADSPI_LUT(lut_base + 1));
+
 	fsl_qspi_lock_lut(q);
 }
 
@@ -513,6 +523,8 @@ static int fsl_qspi_get_seqid(struct fsl_qspi *q, u8 cmd)
 		return SEQID_EN4B;
 	case SPINOR_OP_BRWR:
 		return SEQID_BRWR;
+	case SPINOR_OP_READ_1_1_2:
+		return SEQID_DUAL_READ;
 	default:
 		if (cmd == q->nor[0].erase_opcode)
 			return SEQID_SE;
-- 
2.11.0

[RFC/RFT PATCH v1 6/9] mtd: spi: Enhance the fsl_qspi_read() method to support DUAL and QUAD

[Lukasz Majewski]

This commit not only provides the DUAL and QUAD read capability for QSPI
controller, but also resets the AHB pointer sequence (as recommended) and
makes sure that AHB - not IP - mode is used for reading SPI-NOR data to
internal buffer.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 0c381cdfb39f..97546fa70b79 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -119,6 +119,10 @@
 #define QUADSPI_FR			0x160
 #define QUADSPI_FR_TFF_MASK		0x1
 
+#define QUADSPI_SPTRCLR                 0x16c
+#define QUADSPI_SPTRCLR_BFPTRC_SHIFT    0
+#define QUADSPI_SPTRCLR_BFPTRC_MASK     (0x1 << QUADSPI_SPTRCLR_BFPTRC_SHIFT)
+
 #define QUADSPI_SFA1AD			0x180
 #define QUADSPI_SFA2AD			0x184
 #define QUADSPI_SFB1AD			0x188
@@ -903,6 +907,22 @@ static ssize_t fsl_qspi_read(struct spi_nor *nor, loff_t from,
 {
 	struct fsl_qspi *q = nor->priv;
 	u8 cmd = nor->read_opcode;
+	int seqid;
+
+	/* Set the actual lut sequence for AHB Read from the considered nor. */
+	seqid = fsl_qspi_get_seqid(q, nor->read_opcode);
+	if (seqid < 0)
+		return seqid;
+
+	qspi_writel(q, seqid << QUADSPI_BFGENCR_SEQID_SHIFT,
+		    q->iobase + QUADSPI_BFGENCR);
+
+	/* Reset the AHB sequence pointer */
+	qspi_writel(q, QUADSPI_SPTRCLR_BFPTRC_MASK,
+		    q->iobase + QUADSPI_SPTRCLR);
+
+	/* make sure the Rx buffer is read through AHB, not IP */
+	qspi_writel(q, QUADSPI_RBCT_WMRK_MASK, q->iobase + QUADSPI_RBCT);
 
 	/* if necessary,ioremap buffer before AHB read, */
 	if (!q->ahb_addr) {
-- 
2.11.0

[RFC/RFT PATCH v1 7/9] mtd: spi: Add SPI_NOR_DUAL_READ property for the 'n25q128a13' Micron memory

[Lukasz Majewski]

This memory supports not only QUAD reads, but DUAL as well.

Those are needed when two identical memories are used, but one is not
using four lines for read I/O.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/mtd/spi-nor/spi-nor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index f028277fb1ce..442102be174e 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1103,7 +1103,7 @@ static const struct flash_info spi_nor_ids[] = {
 	{ "n25q064",     INFO(0x20ba17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
-	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
+	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ | SPI_NOR_DUAL_READ) },
 	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "n25q256ax1",  INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
-- 
2.11.0

[RFC/RFT PATCH v1 8/9] mtd: spi: Allocate memory corresponding to maximal fsl-quadspi.c controller area

[Lukasz Majewski]

This commit changes the way how QUADSPI controller allocates memory for
reading data via AHB.

After this change it is a fixed buffer with maximal size (for up to four
SPI-NOR memories connected).

In my case:

----------------- 0x0
SPI0 (QSPI0, A1 -> QUAD)
----------------- 0x1000000
HOLE
----------------- 0x2000000
SPI1 (QSPI0, B1 -> DUAL)
----------------- 0x3000000
HOLE
----------------- 0x4000000

Without this change, some single bytes are missing when reading single
bytes from 256B buffer.

dd if=/dev/mtd7 of=aaa.img bs=1 count=256

root@nix:~# hexdump aaa.img
0000000 464c eec0 baa5 c5ff 7b99 4dfb e0b6 8a2e
0000010 e98e 5265 683c a635 c069 e402 303f d936
0000020 c243 01a7 7064 fce8 e3a9 200a 7e85 28bc
0000030 4296 a30e 1bb4 88d4 b456 b4a6 f3aa 8cff
0000040 01c9 462d 0a43 f893 0e42 67f1 57f0 787c
0000050 49c0 fb2a e514 e954 1d21 affa bac4 38f1
0000060 1ca5 ec46 77eb a854 98b6 e71a c1cb 876c
0000070 b441 2baf ee33 596c 98b6 e71a c1cb 876c
0000080 7d8b 5739 5cca 873f c3df 8aca 2d5b 2bbb
0000090 5afe 6ad9 6072 b092 5ace 905b e217 faee
00000a0 58c6 6851 a1b9 c756 5ace 905b e217 faee
00000b0 b3dd 98be 0412 73d2 cbca c47c 6ab0 7c6d
00000c0 65cf b1e2 1457 a3cf 502b 6449 9a84 d83f
00000d0 a6e7 df30 b0e6 ea23 502b 6449 9a84 d83f
00000e0 5638 26bf d680 c47a 0225 6762 cf65 75fc
00000f0 2faa edf1 f8b6 dc13 ffff ffff ffff ffff
0000100


Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 97546fa70b79..915e5a203895 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -908,6 +908,9 @@ static ssize_t fsl_qspi_read(struct spi_nor *nor, loff_t from,
 	struct fsl_qspi *q = nor->priv;
 	u8 cmd = nor->read_opcode;
 	int seqid;
+	size_t qlen = q->nor_size * 4;
+	int nor_idx = nor - q->nor;
+	size_t nor_ofs = q->nor_size * nor_idx;
 
 	/* Set the actual lut sequence for AHB Read from the considered nor. */
 	seqid = fsl_qspi_get_seqid(q, nor->read_opcode);
@@ -926,8 +929,9 @@ static ssize_t fsl_qspi_read(struct spi_nor *nor, loff_t from,
 
 	/* if necessary,ioremap buffer before AHB read, */
 	if (!q->ahb_addr) {
-		q->memmap_offs = q->chip_base_addr + from;
-		q->memmap_len = len > QUADSPI_MIN_IOMAP ? len : QUADSPI_MIN_IOMAP;
+		q->memmap_offs = q->chip_base_addr;
+		q->memmap_len = qlen > QUADSPI_MIN_IOMAP ?
+			qlen : QUADSPI_MIN_IOMAP;
 
 		q->ahb_addr = ioremap_nocache(
 				q->memmap_phy + q->memmap_offs,
@@ -942,8 +946,9 @@ static ssize_t fsl_qspi_read(struct spi_nor *nor, loff_t from,
 			q->memmap_offs + q->memmap_len) {
 		iounmap(q->ahb_addr);
 
-		q->memmap_offs = q->chip_base_addr + from;
-		q->memmap_len = len > QUADSPI_MIN_IOMAP ? len : QUADSPI_MIN_IOMAP;
+		q->memmap_offs = q->chip_base_addr;
+		q->memmap_len = qlen > QUADSPI_MIN_IOMAP ?
+			qlen : QUADSPI_MIN_IOMAP;
 		q->ahb_addr = ioremap_nocache(
 				q->memmap_phy + q->memmap_offs,
 				q->memmap_len);
@@ -954,12 +959,11 @@ static ssize_t fsl_qspi_read(struct spi_nor *nor, loff_t from,
 	}
 
 	dev_dbg(q->dev, "cmd [%x],read from %p, len:%zd\n",
-		cmd, q->ahb_addr + q->chip_base_addr + from - q->memmap_offs,
+		cmd, q->ahb_addr + nor_ofs + from - q->memmap_offs,
 		len);
 
 	/* Read out the data directly from the AHB buffer.*/
-	memcpy(buf, q->ahb_addr + q->chip_base_addr + from - q->memmap_offs,
-		len);
+	memcpy(buf, q->ahb_addr + nor_ofs + from - q->memmap_offs, len);
 
 	return len;
 }
-- 
2.11.0

[RFC/RFT PATCH v1 9/9] mtd: spi: Skip reading the Serial Flash Discoverable Parameters

[Lukasz Majewski]

The fsl-quadspi.c driver is not supporting SPINOR_OP_RDSFDP (0x5a)
read opcode - in the legacy driver we do read some garbage
data from AHB mapped area and then return on the first check
(L2376 @ ./mtd/spi-nor/spi-nor.c)

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/mtd/spi-nor/spi-nor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 442102be174e..c79b8c33aeeb 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1103,7 +1103,7 @@ static const struct flash_info spi_nor_ids[] = {
 	{ "n25q064",     INFO(0x20ba17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
-	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ | SPI_NOR_DUAL_READ) },
+	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ | SPI_NOR_DUAL_READ | SPI_NOR_SKIP_SFDP) },
 	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "n25q256ax1",  INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
-- 
2.11.0

Re: [RFC/RFT PATCH v1 9/9] mtd: spi: Skip reading the Serial Flash Discoverable Parameters

[Cyrille Pitchen]

Hi Lukasz,

Le 27/09/2018 à 00:07, Lukasz Majewski a écrit :
> The fsl-quadspi.c driver is not supporting SPINOR_OP_RDSFDP (0x5a)
> read opcode - in the legacy driver we do read some garbage
> data from AHB mapped area and then return on the first check

If your controller reads garbage then spi_nor_parse_sfdp() fails and exists on the
very first check: signature of the SFDP header. Hence everything should already go on
as if the SPI_NOR_SKIP_SFDP flag is set.

So this patch doesn't change anything for your controller but prevents other controller
from using QSPI protocols like SPI 1-4-4.

This patch introduces a regression for other chips.

Best regards,

Cyrille

> (L2376 @ ./mtd/spi-nor/spi-nor.c)
> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 442102be174e..c79b8c33aeeb 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1103,7 +1103,7 @@ static const struct flash_info spi_nor_ids[] = {
>  	{ "n25q064",     INFO(0x20ba17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
> -	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ | SPI_NOR_DUAL_READ) },
> +	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ | SPI_NOR_DUAL_READ | SPI_NOR_SKIP_SFDP) },
>  	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>  	{ "n25q256ax1",  INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
> 

Re: [RFC/RFT PATCH v1 9/9] mtd: spi: Skip reading the Serial Flash Discoverable Parameters

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2517 bytes --]

Hi Cyrille,

> Hi Lukasz,
> 
> Le 27/09/2018 à 00:07, Lukasz Majewski a écrit :
> > The fsl-quadspi.c driver is not supporting SPINOR_OP_RDSFDP (0x5a)
> > read opcode - in the legacy driver we do read some garbage
> > data from AHB mapped area and then return on the first check  
> 
> If your controller reads garbage then spi_nor_parse_sfdp() fails and
> exists on the very first check: signature of the SFDP header. Hence
> everything should already go on as if the SPI_NOR_SKIP_SFDP flag is
> set.

Yes, this is the observed behaviour.

> 
> So this patch doesn't change anything for your controller but
> prevents other controller from using QSPI protocols like SPI 1-4-4.
> 
> This patch introduces a regression for other chips.

As stated in the patch series cover letter - this is to explicitly show
problems with qspi on vf610.

Thanks for your review.

> 
> Best regards,
> 
> Cyrille
> 
> > (L2376 @ ./mtd/spi-nor/spi-nor.c)
> > 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > b/drivers/mtd/spi-nor/spi-nor.c index 442102be174e..c79b8c33aeeb
> > 100644 --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -1103,7 +1103,7 @@ static const struct flash_info spi_nor_ids[]
> > = { { "n25q064",     INFO(0x20ba17, 0, 64 * 1024,  128, SECT_4K |
> > SPI_NOR_QUAD_READ) }, { "n25q064a",    INFO(0x20bb17, 0, 64 *
> > 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) }, { "n25q128a11",
> > INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
> > -	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256,
> > SECT_4K | SPI_NOR_QUAD_READ | SPI_NOR_DUAL_READ) },
> > +	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256,
> > SECT_4K | SPI_NOR_QUAD_READ | SPI_NOR_DUAL_READ |
> > SPI_NOR_SKIP_SFDP) }, { "n25q256a",    INFO(0x20ba19, 0, 64 *
> > 1024,  512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > { "n25q256ax1",  INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K |
> > SPI_NOR_QUAD_READ) }, { "n25q512a",    INFO(0x20bb20, 0, 64 * 1024,
> > 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) }, 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC/RFT PATCH v1 0/9] mtd: fsl: quadspi: Fixes for fsl-quadspi.c driver (vybrid HW)

[Boris Brezillon]

Hi Lukasz,

On Thu, 27 Sep 2018 00:07:30 +0200
Lukasz Majewski <lukma@denx.de> wrote:

> Please find following set of patches, which provide improved behaviour
> of the fsl-quadspi.c driver on Vybrid vf610 HW.
> 
> Below code is based on previous work done by Albert ARIBAUD:
> https://patchwork.ozlabs.org/patch/675401/
> 
> I've cleaned up the code a bit, make separate patches, exclude not
> needed parts and port it to v4.19-rc3.
> 
> After applying those patches, the quadspi controller on vf610 works
> with UBI/UBIFS.
> The problem is with some corner case writes to raw MTD device
> (like writing 1023B with single byte writes). Those fail sometimes.
> 
> Regression tests can be found in the following repository:
> https://github.com/lmajewski/tests-spi/tree/master/tests
> 
> (Please read README.txt).
> 
> The NXP's community thread regarding HW issues in this block on
> the vf610:
> https://community.nxp.com/thread/485139
> 
> Maybe those patches will be helpful with the new, work in progress
> driver for spi-fsl-qspi.c

Talking about that, can you try to port your fixes on top of Frieder's
patchset? I'm pretty sure some bug fixes are irrelevant after the
migration to spi-mem (patch 1, 3, 4, 5, 6, 7 and 9 should be dropped I
think).

> 
> Lukasz Majewski (9):
>   Revert "mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ"
>   mtd: qspi: Provide quirk to read only half of RX buffer (NXP's vybrid)
>   mtd: spi: Do not setup the default seqid as we got it set for DUAL and
>     QUAD
>   mtd: spi: Modify the HW capability mask according to supported RX
>     lanes
>   mtd: spi: Provide LUT entry to perform DUAL read
>   mtd: spi: Enhance the fsl_qspi_read() method to support DUAL and QUAD
>   mtd: spi: Add SPI_NOR_DUAL_READ property for the 'n25q128a13' Micron
>     memory
>   mtd: spi: Allocate memory corresponding to maximal fsl-quadspi.c
>     controller area
>   mtd: spi: Skip reading the Serial Flash Discoverable Parameters

Re: [RFC/RFT PATCH v1 0/9] mtd: fsl: quadspi: Fixes for fsl-quadspi.c driver (vybrid HW)

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2951 bytes --]

On Sat, 29 Sep 2018 00:03:59 +0200
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> Hi Lukasz,
> 
> On Thu, 27 Sep 2018 00:07:30 +0200
> Lukasz Majewski <lukma@denx.de> wrote:
> 
> > Please find following set of patches, which provide improved
> > behaviour of the fsl-quadspi.c driver on Vybrid vf610 HW.
> > 
> > Below code is based on previous work done by Albert ARIBAUD:
> > https://patchwork.ozlabs.org/patch/675401/
> > 
> > I've cleaned up the code a bit, make separate patches, exclude not
> > needed parts and port it to v4.19-rc3.
> > 
> > After applying those patches, the quadspi controller on vf610 works
> > with UBI/UBIFS.
> > The problem is with some corner case writes to raw MTD device
> > (like writing 1023B with single byte writes). Those fail sometimes.
> > 
> > Regression tests can be found in the following repository:
> > https://github.com/lmajewski/tests-spi/tree/master/tests
> > 
> > (Please read README.txt).
> > 
> > The NXP's community thread regarding HW issues in this block on
> > the vf610:
> > https://community.nxp.com/thread/485139
> > 
> > Maybe those patches will be helpful with the new, work in progress
> > driver for spi-fsl-qspi.c  
> 
> Talking about that, can you try to port your fixes on top of Frieder's
> patchset? I'm pretty sure some bug fixes are irrelevant after the
> migration to spi-mem (patch 1, 3, 4, 5, 6, 7 and 9 should be dropped I
> think).

The problem is that Frieder's patch is using IP command mode for
transfers smaller than AHB RX fifo size.
This according to the comment in the current driver is broken in HW for
Vybrid (so this is a regression).

I'm also wondering if other users of vf610 based boards experience
issues with QSPI?

In my case, after running the UBI/UBIFS tests (on the original and
new driver without those "fixes") I cannot mount the volume after
creation as the header is wrongly read.

> 
> > 
> > Lukasz Majewski (9):
> >   Revert "mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ"
> >   mtd: qspi: Provide quirk to read only half of RX buffer (NXP's
> > vybrid) mtd: spi: Do not setup the default seqid as we got it set
> > for DUAL and QUAD
> >   mtd: spi: Modify the HW capability mask according to supported RX
> >     lanes
> >   mtd: spi: Provide LUT entry to perform DUAL read
> >   mtd: spi: Enhance the fsl_qspi_read() method to support DUAL and
> > QUAD mtd: spi: Add SPI_NOR_DUAL_READ property for the 'n25q128a13'
> > Micron memory
> >   mtd: spi: Allocate memory corresponding to maximal fsl-quadspi.c
> >     controller area
> >   mtd: spi: Skip reading the Serial Flash Discoverable Parameters  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC/RFT PATCH v1 0/9] mtd: fsl: quadspi: Fixes for fsl-quadspi.c driver (vybrid HW)

[Boris Brezillon]

Hi Lukasz,

On Sat, 29 Sep 2018 23:02:40 +0200
Lukasz Majewski <lukma@denx.de> wrote:

> > Talking about that, can you try to port your fixes on top of Frieder's
> > patchset? I'm pretty sure some bug fixes are irrelevant after the
> > migration to spi-mem (patch 1, 3, 4, 5, 6, 7 and 9 should be dropped I
> > think).  
> 
> The problem is that Frieder's patch is using IP command mode for
> transfers smaller than AHB RX fifo size.
> This according to the comment in the current driver is broken in HW for
> Vybrid (so this is a regression).

Why not fixing that in the new driver?

> 
> I'm also wondering if other users of vf610 based boards experience
> issues with QSPI?

Are you sure the 4 I/O lines are wired on your design? Anyway, if it's
a bug that only hurts vf610, you should mask quad modes in ->hwcaps (or
patch fsl_qspi_supports_op() in the new driver), not change the SPI NOR
definition.

> 
> In my case, after running the UBI/UBIFS tests (on the original and
> new driver without those "fixes") I cannot mount the volume after
> creation as the header is wrongly read.

I'm not denying this fact, I'm just saying, now that you've found where
the issue comes from, you can also port the fix to the new driver.

Regards,

Boris

Re: [RFC/RFT PATCH v1 0/9] mtd: fsl: quadspi: Fixes for fsl-quadspi.c driver (vybrid HW)

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2387 bytes --]

Hi Boris,

> Hi Lukasz,
> 
> On Sat, 29 Sep 2018 23:02:40 +0200
> Lukasz Majewski <lukma@denx.de> wrote:
> 
> > > Talking about that, can you try to port your fixes on top of
> > > Frieder's patchset? I'm pretty sure some bug fixes are irrelevant
> > > after the migration to spi-mem (patch 1, 3, 4, 5, 6, 7 and 9
> > > should be dropped I think).    
> > 
> > The problem is that Frieder's patch is using IP command mode for
> > transfers smaller than AHB RX fifo size.
> > This according to the comment in the current driver is broken in HW
> > for Vybrid (so this is a regression).  
> 
> Why not fixing that in the new driver?

I did not say that I will not fix it in the new driver :-)

The problem is that I'm waiting for NXP's community reply - to get some
more info regarding the bug.

> 
> > 
> > I'm also wondering if other users of vf610 based boards experience
> > issues with QSPI?  
> 
> Are you sure the 4 I/O lines are wired on your design? 

This is a somewhat special case.

There are two identical SPI-NOR memories: one with QUAD lines connected
and second only for DUAL due to HW design decision.

> Anyway, if it's
> a bug that only hurts vf610, you should mask quad modes in ->hwcaps
> (or patch fsl_qspi_supports_op() in the new driver), not change the
> SPI NOR definition.

Yes, this was also pointed out by Cyrille - and yes, I do agree that I
shouldn't mask it.

> 
> > 
> > In my case, after running the UBI/UBIFS tests (on the original and
> > new driver without those "fixes") I cannot mount the volume after
> > creation as the header is wrongly read.  
> 
> I'm not denying this fact, I'm just saying, now that you've found
> where the issue comes from, you can also port the fix to the new
> driver.

Yes, as the old driver is now in a "good enough" shape (though I don't
know the exact HW bug reason) - the code can be ported to the new
driver.

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




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]