linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework
@ 2020-06-01  7:04 Vignesh Raghavendra
  2020-06-01  7:04 ` [RESEND PATCH v3 1/8] mtd: spi-nor: cadence-quadspi: Make driver independent of flash geometry Vignesh Raghavendra
                   ` (9 more replies)
  0 siblings, 10 replies; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-06-01  7:04 UTC (permalink / raw)
  To: Tudor Ambarus, Mark Brown
  Cc: Vignesh Raghavendra, Boris Brezillon, Ramuthevar Vadivel Murugan,
	linux-mtd, linux-kernel, linux-spi, simon.k.r.goldschmidt,
	dinguyen, marex

This series is a subset of "[PATCH v12 0/4] spi: cadence-quadspi: Add
support for the Cadence QSPI controller" by Ramuthevar,Vadivel MuruganX
<vadivel.muruganx.ramuthevar@linux.intel.com> that intended to move
cadence-quadspi driver to spi-mem framework

Those patches were trying to accomplish too many things in a single set
of patches and need to split into smaller patches. This is reduced
version of above series.

Changes that are intended to make migration easy are split into separate
patches. Patches 1 to 3 drop features that cannot be supported under
spi-mem at the moment (backward compatibility is maintained).
Patch 4-5 are trivial cleanups. Patch 6 does the actual conversion to
spi-mem and patch 7 moves the driver to drivers/spi folder.

I have tested both INDAC mode (used by non TI platforms like Altera
SoCFPGA) and DAC mode (used by TI platforms) on TI EVMs.

Patches to move move bindings over to
"Documentation/devicetree/bindings/spi/" directory and also conversion
of bindig doc to YAML will be posted separately.  Support for Intel
platform would follow that.

Resend v3:
Rebased onto v5.7-c1

v3:
Split handling of probe deferral into separate patch (out of 5/6)
Split dropping of redundant WREN to separate patch (out of 5/6)
Fix a possible memleak due to lack of spi_master_put()
Parse all SPI slave nodes in cqspi_setup_flash()
Address misc comments from Tudor on v2
Rebase onto latest spi-nor/next

v2:
Rework patch 1/6 to keep "cdns,is-decoded-cs" property supported.


Ramuthevar Vadivel Murugan (2):
  mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework
  spi: Move cadence-quadspi driver to drivers/spi/

Vignesh Raghavendra (6):
  mtd: spi-nor: cadence-quadspi: Make driver independent of flash
    geometry
  mtd: spi-nor: cadence-quadspi: Provide a way to disable DAC mode
  mtd: spi-nor: cadence-quadspi: Don't initialize rx_dma_complete on
    failure
  mtd: spi-nor: cadence-quadspi: Fix error path on failure to acquire
    reset lines
  mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting
    DMA channel
  mtd: spi-nor: cadence-quadspi: Drop redundant WREN in erase path

 drivers/mtd/spi-nor/controllers/Kconfig       |  11 -
 drivers/mtd/spi-nor/controllers/Makefile      |   1 -
 drivers/spi/Kconfig                           |  11 +
 drivers/spi/Makefile                          |   1 +
 .../spi-cadence-quadspi.c}                    | 541 +++++++-----------
 5 files changed, 222 insertions(+), 343 deletions(-)
 rename drivers/{mtd/spi-nor/controllers/cadence-quadspi.c => spi/spi-cadence-quadspi.c} (74%)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [RESEND PATCH v3 1/8] mtd: spi-nor: cadence-quadspi: Make driver independent of flash geometry
  2020-06-01  7:04 [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Vignesh Raghavendra
@ 2020-06-01  7:04 ` Vignesh Raghavendra
  2020-06-01  7:04 ` [RESEND PATCH v3 2/8] mtd: spi-nor: cadence-quadspi: Provide a way to disable DAC mode Vignesh Raghavendra
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-06-01  7:04 UTC (permalink / raw)
  To: Tudor Ambarus, Mark Brown
  Cc: Vignesh Raghavendra, Boris Brezillon, Ramuthevar Vadivel Murugan,
	linux-mtd, linux-kernel, linux-spi, simon.k.r.goldschmidt,
	dinguyen, marex

Drop configuration of Flash size, erase size and page size
configuration. Flash size is needed only if using AHB decoder (BIT 23 of
CONFIG_REG) which is not used by the driver.
Erase size and page size are needed if IP is configured to send WREN
automatically. But since SPI NOR layer takes care of sending WREN, there
is no need to configure these fields either.

Therefore drop these in preparation to move the driver to spi-mem
framework where flash geometry is not visible to controller driver.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 .../mtd/spi-nor/controllers/cadence-quadspi.c | 36 +------------------
 1 file changed, 1 insertion(+), 35 deletions(-)

diff --git a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
index 494dcab4aaaab..9b8554d44fac8 100644
--- a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
@@ -77,9 +77,6 @@ struct cqspi_st {
 	dma_addr_t		mmap_phys_base;
 
 	int			current_cs;
-	int			current_page_size;
-	int			current_erase_size;
-	int			current_addr_width;
 	unsigned long		master_ref_clk_hz;
 	bool			is_decoded_cs;
 	u32			fifo_depth;
@@ -736,32 +733,6 @@ static void cqspi_chipselect(struct spi_nor *nor)
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
 }
 
-static void cqspi_configure_cs_and_sizes(struct spi_nor *nor)
-{
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
-	struct cqspi_st *cqspi = f_pdata->cqspi;
-	void __iomem *iobase = cqspi->iobase;
-	unsigned int reg;
-
-	/* configure page size and block size. */
-	reg = readl(iobase + CQSPI_REG_SIZE);
-	reg &= ~(CQSPI_REG_SIZE_PAGE_MASK << CQSPI_REG_SIZE_PAGE_LSB);
-	reg &= ~(CQSPI_REG_SIZE_BLOCK_MASK << CQSPI_REG_SIZE_BLOCK_LSB);
-	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
-	reg |= (nor->page_size << CQSPI_REG_SIZE_PAGE_LSB);
-	reg |= (ilog2(nor->mtd.erasesize) << CQSPI_REG_SIZE_BLOCK_LSB);
-	reg |= (nor->addr_width - 1);
-	writel(reg, iobase + CQSPI_REG_SIZE);
-
-	/* configure the chip select */
-	cqspi_chipselect(nor);
-
-	/* Store the new configuration of the controller */
-	cqspi->current_page_size = nor->page_size;
-	cqspi->current_erase_size = nor->mtd.erasesize;
-	cqspi->current_addr_width = nor->addr_width;
-}
-
 static unsigned int calculate_ticks_for_ns(const unsigned int ref_clk_hz,
 					   const unsigned int ns_val)
 {
@@ -867,18 +838,13 @@ static void cqspi_configure(struct spi_nor *nor)
 	int switch_cs = (cqspi->current_cs != f_pdata->cs);
 	int switch_ck = (cqspi->sclk != sclk);
 
-	if ((cqspi->current_page_size != nor->page_size) ||
-	    (cqspi->current_erase_size != nor->mtd.erasesize) ||
-	    (cqspi->current_addr_width != nor->addr_width))
-		switch_cs = 1;
-
 	if (switch_cs || switch_ck)
 		cqspi_controller_enable(cqspi, 0);
 
 	/* Switch chip select. */
 	if (switch_cs) {
 		cqspi->current_cs = f_pdata->cs;
-		cqspi_configure_cs_and_sizes(nor);
+		cqspi_chipselect(nor);
 	}
 
 	/* Setup baudrate divisor and delays */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND PATCH v3 2/8] mtd: spi-nor: cadence-quadspi: Provide a way to disable DAC mode
  2020-06-01  7:04 [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Vignesh Raghavendra
  2020-06-01  7:04 ` [RESEND PATCH v3 1/8] mtd: spi-nor: cadence-quadspi: Make driver independent of flash geometry Vignesh Raghavendra
@ 2020-06-01  7:04 ` Vignesh Raghavendra
  2020-06-01  7:04 ` [RESEND PATCH v3 3/8] mtd: spi-nor: cadence-quadspi: Don't initialize rx_dma_complete on failure Vignesh Raghavendra
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-06-01  7:04 UTC (permalink / raw)
  To: Tudor Ambarus, Mark Brown
  Cc: Vignesh Raghavendra, Boris Brezillon, Ramuthevar Vadivel Murugan,
	linux-mtd, linux-kernel, linux-spi, simon.k.r.goldschmidt,
	dinguyen, marex

Currently direct access mode is used on platforms that have AHB window
(memory mapped window) larger than flash size. This feature is limited
to TI platforms as non TI platforms have < 1MB of AHB window.
Therefore introduce a driver quirk to disable DAC mode and set it for
non TI compatibles. This is in preparation to move to spi-mem framework
where flash geometry cannot be known.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/controllers/cadence-quadspi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
index 9b8554d44fac8..8a9e17f79d8d9 100644
--- a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
@@ -34,6 +34,7 @@
 
 /* Quirks */
 #define CQSPI_NEEDS_WR_DELAY		BIT(0)
+#define CQSPI_DISABLE_DAC_MODE		BIT(1)
 
 /* Capabilities mask */
 #define CQSPI_BASE_HWCAPS_MASK					\
@@ -1261,7 +1262,8 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
 
 		f_pdata->registered = true;
 
-		if (mtd->size <= cqspi->ahb_size) {
+		if (mtd->size <= cqspi->ahb_size &&
+		    !(ddata->quirks & CQSPI_DISABLE_DAC_MODE)) {
 			f_pdata->use_direct_mode = true;
 			dev_dbg(nor->dev, "using direct mode for %s\n",
 				mtd->name);
@@ -1457,6 +1459,7 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
 
 static const struct cqspi_driver_platdata cdns_qspi = {
 	.hwcaps_mask = CQSPI_BASE_HWCAPS_MASK,
+	.quirks = CQSPI_DISABLE_DAC_MODE,
 };
 
 static const struct cqspi_driver_platdata k2g_qspi = {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND PATCH v3 3/8] mtd: spi-nor: cadence-quadspi: Don't initialize rx_dma_complete on failure
  2020-06-01  7:04 [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Vignesh Raghavendra
  2020-06-01  7:04 ` [RESEND PATCH v3 1/8] mtd: spi-nor: cadence-quadspi: Make driver independent of flash geometry Vignesh Raghavendra
  2020-06-01  7:04 ` [RESEND PATCH v3 2/8] mtd: spi-nor: cadence-quadspi: Provide a way to disable DAC mode Vignesh Raghavendra
@ 2020-06-01  7:04 ` Vignesh Raghavendra
  2020-06-01  7:04 ` [RESEND PATCH v3 4/8] mtd: spi-nor: cadence-quadspi: Fix error path on failure to acquire reset lines Vignesh Raghavendra
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-06-01  7:04 UTC (permalink / raw)
  To: Tudor Ambarus, Mark Brown
  Cc: Vignesh Raghavendra, Boris Brezillon, Ramuthevar Vadivel Murugan,
	linux-mtd, linux-kernel, linux-spi, simon.k.r.goldschmidt,
	dinguyen, marex

If driver fails to acquire DMA channel then don't initialize
rx_dma_complete struct as it won't be used.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/controllers/cadence-quadspi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
index 8a9e17f79d8d9..379e22c11c87f 100644
--- a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
@@ -1180,6 +1180,7 @@ static void cqspi_request_mmap_dma(struct cqspi_st *cqspi)
 	if (IS_ERR(cqspi->rx_chan)) {
 		dev_err(&cqspi->pdev->dev, "No Rx DMA available\n");
 		cqspi->rx_chan = NULL;
+		return;
 	}
 	init_completion(&cqspi->rx_dma_complete);
 }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND PATCH v3 4/8] mtd: spi-nor: cadence-quadspi: Fix error path on failure to acquire reset lines
  2020-06-01  7:04 [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Vignesh Raghavendra
                   ` (2 preceding siblings ...)
  2020-06-01  7:04 ` [RESEND PATCH v3 3/8] mtd: spi-nor: cadence-quadspi: Don't initialize rx_dma_complete on failure Vignesh Raghavendra
@ 2020-06-01  7:04 ` Vignesh Raghavendra
  2020-06-01  7:04 ` [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel Vignesh Raghavendra
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-06-01  7:04 UTC (permalink / raw)
  To: Tudor Ambarus, Mark Brown
  Cc: Vignesh Raghavendra, Boris Brezillon, Ramuthevar Vadivel Murugan,
	linux-mtd, linux-kernel, linux-spi, simon.k.r.goldschmidt,
	dinguyen, marex

Make sure to undo the prior changes done by the driver when exiting due
to failure to acquire reset lines.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/controllers/cadence-quadspi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
index 379e22c11c87f..608ca657ff7f5 100644
--- a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
@@ -1359,13 +1359,13 @@ static int cqspi_probe(struct platform_device *pdev)
 	rstc = devm_reset_control_get_optional_exclusive(dev, "qspi");
 	if (IS_ERR(rstc)) {
 		dev_err(dev, "Cannot get QSPI reset.\n");
-		return PTR_ERR(rstc);
+		goto probe_reset_failed;
 	}
 
 	rstc_ocp = devm_reset_control_get_optional_exclusive(dev, "qspi-ocp");
 	if (IS_ERR(rstc_ocp)) {
 		dev_err(dev, "Cannot get QSPI OCP reset.\n");
-		return PTR_ERR(rstc_ocp);
+		goto probe_reset_failed;
 	}
 
 	reset_control_assert(rstc);
@@ -1384,7 +1384,7 @@ static int cqspi_probe(struct platform_device *pdev)
 			       pdev->name, cqspi);
 	if (ret) {
 		dev_err(dev, "Cannot request IRQ.\n");
-		goto probe_irq_failed;
+		goto probe_reset_failed;
 	}
 
 	cqspi_wait_idle(cqspi);
@@ -1401,7 +1401,7 @@ static int cqspi_probe(struct platform_device *pdev)
 	return ret;
 probe_setup_failed:
 	cqspi_controller_enable(cqspi, 0);
-probe_irq_failed:
+probe_reset_failed:
 	clk_disable_unprepare(cqspi->clk);
 probe_clk_failed:
 	pm_runtime_put_sync(dev);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
  2020-06-01  7:04 [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Vignesh Raghavendra
                   ` (3 preceding siblings ...)
  2020-06-01  7:04 ` [RESEND PATCH v3 4/8] mtd: spi-nor: cadence-quadspi: Fix error path on failure to acquire reset lines Vignesh Raghavendra
@ 2020-06-01  7:04 ` Vignesh Raghavendra
  2020-08-22 18:05   ` Jan Kiszka
  2020-06-01  7:04 ` [RESEND PATCH v3 6/8] mtd: spi-nor: cadence-quadspi: Drop redundant WREN in erase path Vignesh Raghavendra
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-06-01  7:04 UTC (permalink / raw)
  To: Tudor Ambarus, Mark Brown
  Cc: Vignesh Raghavendra, Boris Brezillon, Ramuthevar Vadivel Murugan,
	linux-mtd, linux-kernel, linux-spi, simon.k.r.goldschmidt,
	dinguyen, marex

dma_request_chan_by_mask() can throw EPROBE_DEFER if DMA provider
is not yet probed. Currently driver just falls back to using PIO mode
(which is less efficient) in this case. Instead return probe deferral
error as is so that driver will be re probed once DMA provider is
available.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 .../mtd/spi-nor/controllers/cadence-quadspi.c  | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
index 608ca657ff7f5..0570ebca135a9 100644
--- a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
@@ -1169,7 +1169,7 @@ static void cqspi_controller_init(struct cqspi_st *cqspi)
 	cqspi_controller_enable(cqspi, 1);
 }
 
-static void cqspi_request_mmap_dma(struct cqspi_st *cqspi)
+static int cqspi_request_mmap_dma(struct cqspi_st *cqspi)
 {
 	dma_cap_mask_t mask;
 
@@ -1178,11 +1178,16 @@ static void cqspi_request_mmap_dma(struct cqspi_st *cqspi)
 
 	cqspi->rx_chan = dma_request_chan_by_mask(&mask);
 	if (IS_ERR(cqspi->rx_chan)) {
-		dev_err(&cqspi->pdev->dev, "No Rx DMA available\n");
+		int ret = PTR_ERR(cqspi->rx_chan);
+
+		if (ret != -EPROBE_DEFER)
+			dev_err(&cqspi->pdev->dev, "No Rx DMA available\n");
 		cqspi->rx_chan = NULL;
-		return;
+		return ret;
 	}
 	init_completion(&cqspi->rx_dma_complete);
+
+	return 0;
 }
 
 static const struct spi_nor_controller_ops cqspi_controller_ops = {
@@ -1269,8 +1274,11 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
 			dev_dbg(nor->dev, "using direct mode for %s\n",
 				mtd->name);
 
-			if (!cqspi->rx_chan)
-				cqspi_request_mmap_dma(cqspi);
+			if (!cqspi->rx_chan) {
+				ret = cqspi_request_mmap_dma(cqspi);
+				if (ret == -EPROBE_DEFER)
+					goto err;
+			}
 		}
 	}
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND PATCH v3 6/8] mtd: spi-nor: cadence-quadspi: Drop redundant WREN in erase path
  2020-06-01  7:04 [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Vignesh Raghavendra
                   ` (4 preceding siblings ...)
  2020-06-01  7:04 ` [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel Vignesh Raghavendra
@ 2020-06-01  7:04 ` Vignesh Raghavendra
  2020-06-01  7:04 ` [RESEND PATCH v3 7/8] mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework Vignesh Raghavendra
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-06-01  7:04 UTC (permalink / raw)
  To: Tudor Ambarus, Mark Brown
  Cc: Vignesh Raghavendra, Boris Brezillon, Ramuthevar Vadivel Murugan,
	linux-mtd, linux-kernel, linux-spi, simon.k.r.goldschmidt,
	dinguyen, marex

Drop redundant WREN command in cqspi_erase() as SPI NOR core takes care
of sending WREN command before sending erase command.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/controllers/cadence-quadspi.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
index 0570ebca135a9..6b1cbad25e3f6 100644
--- a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
@@ -1016,11 +1016,6 @@ static int cqspi_erase(struct spi_nor *nor, loff_t offs)
 	if (ret)
 		return ret;
 
-	/* Send write enable, then erase commands. */
-	ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
-	if (ret)
-		return ret;
-
 	/* Set up command buffer. */
 	ret = cqspi_command_write_addr(nor, nor->erase_opcode, offs);
 	if (ret)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND PATCH v3 7/8] mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework
  2020-06-01  7:04 [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Vignesh Raghavendra
                   ` (5 preceding siblings ...)
  2020-06-01  7:04 ` [RESEND PATCH v3 6/8] mtd: spi-nor: cadence-quadspi: Drop redundant WREN in erase path Vignesh Raghavendra
@ 2020-06-01  7:04 ` Vignesh Raghavendra
  2020-08-24  5:55   ` Jan Kiszka
  2020-06-01  7:04 ` [RESEND PATCH v3 8/8] spi: Move cadence-quadspi driver to drivers/spi/ Vignesh Raghavendra
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-06-01  7:04 UTC (permalink / raw)
  To: Tudor Ambarus, Mark Brown
  Cc: Vignesh Raghavendra, Boris Brezillon, Ramuthevar Vadivel Murugan,
	linux-mtd, linux-kernel, linux-spi, simon.k.r.goldschmidt,
	dinguyen, marex

From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>

Move cadence-quadspi driver to use spi-mem framework. This is required
to make the driver support for SPI NAND flashes in future.

Driver is feature compliant with existing SPI NOR version.

Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 .../mtd/spi-nor/controllers/cadence-quadspi.c | 476 +++++++-----------
 1 file changed, 191 insertions(+), 285 deletions(-)

diff --git a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
index 6b1cbad25e3f6..c12a1c0191b92 100644
--- a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
@@ -3,6 +3,8 @@
  * Driver for Cadence QSPI Controller
  *
  * Copyright Altera Corporation (C) 2012-2014. All rights reserved.
+ * Copyright Intel Corporation (C) 2019-2020. All rights reserved.
+ * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com
  */
 #include <linux/clk.h>
 #include <linux/completion.h>
@@ -17,9 +19,6 @@
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/mtd/mtd.h>
-#include <linux/mtd/partitions.h>
-#include <linux/mtd/spi-nor.h>
 #include <linux/of_device.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -27,6 +26,7 @@
 #include <linux/reset.h>
 #include <linux/sched.h>
 #include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
 #include <linux/timer.h>
 
 #define CQSPI_NAME			"cadence-qspi"
@@ -36,16 +36,12 @@
 #define CQSPI_NEEDS_WR_DELAY		BIT(0)
 #define CQSPI_DISABLE_DAC_MODE		BIT(1)
 
-/* Capabilities mask */
-#define CQSPI_BASE_HWCAPS_MASK					\
-	(SNOR_HWCAPS_READ | SNOR_HWCAPS_READ_FAST |		\
-	SNOR_HWCAPS_READ_1_1_2 | SNOR_HWCAPS_READ_1_1_4 |	\
-	SNOR_HWCAPS_PP)
+/* Capabilities */
+#define CQSPI_SUPPORTS_OCTAL		BIT(0)
 
 struct cqspi_st;
 
 struct cqspi_flash_pdata {
-	struct spi_nor	nor;
 	struct cqspi_st	*cqspi;
 	u32		clk_rate;
 	u32		read_delay;
@@ -57,8 +53,6 @@ struct cqspi_flash_pdata {
 	u8		addr_width;
 	u8		data_width;
 	u8		cs;
-	bool		registered;
-	bool		use_direct_mode;
 };
 
 struct cqspi_st {
@@ -71,7 +65,6 @@ struct cqspi_st {
 	void __iomem		*ahb_base;
 	resource_size_t		ahb_size;
 	struct completion	transfer_complete;
-	struct mutex		bus_mutex;
 
 	struct dma_chan		*rx_chan;
 	struct completion	rx_dma_complete;
@@ -85,6 +78,7 @@ struct cqspi_st {
 	bool			rclk_en;
 	u32			trigger_address;
 	u32			wr_delay;
+	bool			use_direct_mode;
 	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
 };
 
@@ -283,9 +277,8 @@ static irqreturn_t cqspi_irq_handler(int this_irq, void *dev)
 	return IRQ_HANDLED;
 }
 
-static unsigned int cqspi_calc_rdreg(struct spi_nor *nor)
+static unsigned int cqspi_calc_rdreg(struct cqspi_flash_pdata *f_pdata)
 {
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
 	u32 rdreg = 0;
 
 	rdreg |= f_pdata->inst_width << CQSPI_REG_RD_INSTR_TYPE_INSTR_LSB;
@@ -352,19 +345,21 @@ static int cqspi_exec_flash_cmd(struct cqspi_st *cqspi, unsigned int reg)
 	return cqspi_wait_idle(cqspi);
 }
 
-static int cqspi_command_read(struct spi_nor *nor, u8 opcode,
-			      u8 *rxbuf, size_t n_rx)
+static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
+			      const struct spi_mem_op *op)
 {
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
 	struct cqspi_st *cqspi = f_pdata->cqspi;
 	void __iomem *reg_base = cqspi->iobase;
+	u8 *rxbuf = op->data.buf.in;
+	u8 opcode = op->cmd.opcode;
+	size_t n_rx = op->data.nbytes;
 	unsigned int rdreg;
 	unsigned int reg;
 	size_t read_len;
 	int status;
 
 	if (!n_rx || n_rx > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) {
-		dev_err(nor->dev,
+		dev_err(&cqspi->pdev->dev,
 			"Invalid input argument, len %zu rxbuf 0x%p\n",
 			n_rx, rxbuf);
 		return -EINVAL;
@@ -372,7 +367,7 @@ static int cqspi_command_read(struct spi_nor *nor, u8 opcode,
 
 	reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;
 
-	rdreg = cqspi_calc_rdreg(nor);
+	rdreg = cqspi_calc_rdreg(f_pdata);
 	writel(rdreg, reg_base + CQSPI_REG_RD_INSTR);
 
 	reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB);
@@ -401,25 +396,36 @@ static int cqspi_command_read(struct spi_nor *nor, u8 opcode,
 	return 0;
 }
 
-static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
-			       const u8 *txbuf, size_t n_tx)
+static int cqspi_command_write(struct cqspi_flash_pdata *f_pdata,
+			       const struct spi_mem_op *op)
 {
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
 	struct cqspi_st *cqspi = f_pdata->cqspi;
 	void __iomem *reg_base = cqspi->iobase;
+	const u8 opcode = op->cmd.opcode;
+	const u8 *txbuf = op->data.buf.out;
+	size_t n_tx = op->data.nbytes;
 	unsigned int reg;
 	unsigned int data;
 	size_t write_len;
-	int ret;
 
 	if (n_tx > CQSPI_STIG_DATA_LEN_MAX || (n_tx && !txbuf)) {
-		dev_err(nor->dev,
+		dev_err(&cqspi->pdev->dev,
 			"Invalid input argument, cmdlen %zu txbuf 0x%p\n",
 			n_tx, txbuf);
 		return -EINVAL;
 	}
 
 	reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;
+
+	if (op->addr.nbytes) {
+		reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB);
+		reg |= ((op->addr.nbytes - 1) &
+			CQSPI_REG_CMDCTRL_ADD_BYTES_MASK)
+			<< CQSPI_REG_CMDCTRL_ADD_BYTES_LSB;
+
+		writel(op->addr.val, reg_base + CQSPI_REG_CMDADDRESS);
+	}
+
 	if (n_tx) {
 		reg |= (0x1 << CQSPI_REG_CMDCTRL_WR_EN_LSB);
 		reg |= ((n_tx - 1) & CQSPI_REG_CMDCTRL_WR_BYTES_MASK)
@@ -437,73 +443,46 @@ static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
 			writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
 		}
 	}
-	ret = cqspi_exec_flash_cmd(cqspi, reg);
-	return ret;
-}
-
-static int cqspi_command_write_addr(struct spi_nor *nor,
-				    const u8 opcode, const unsigned int addr)
-{
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
-	struct cqspi_st *cqspi = f_pdata->cqspi;
-	void __iomem *reg_base = cqspi->iobase;
-	unsigned int reg;
-
-	reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;
-	reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB);
-	reg |= ((nor->addr_width - 1) & CQSPI_REG_CMDCTRL_ADD_BYTES_MASK)
-		<< CQSPI_REG_CMDCTRL_ADD_BYTES_LSB;
-
-	writel(addr, reg_base + CQSPI_REG_CMDADDRESS);
 
 	return cqspi_exec_flash_cmd(cqspi, reg);
 }
 
-static int cqspi_read_setup(struct spi_nor *nor)
+static int cqspi_read_setup(struct cqspi_flash_pdata *f_pdata,
+			    const struct spi_mem_op *op)
 {
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
 	struct cqspi_st *cqspi = f_pdata->cqspi;
 	void __iomem *reg_base = cqspi->iobase;
 	unsigned int dummy_clk = 0;
 	unsigned int reg;
 
-	reg = nor->read_opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB;
-	reg |= cqspi_calc_rdreg(nor);
+	reg = op->cmd.opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB;
+	reg |= cqspi_calc_rdreg(f_pdata);
 
 	/* Setup dummy clock cycles */
-	dummy_clk = nor->read_dummy;
+	dummy_clk = op->dummy.nbytes * 8;
 	if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
 		dummy_clk = CQSPI_DUMMY_CLKS_MAX;
 
-	if (dummy_clk / 8) {
-		reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB);
-		/* Set mode bits high to ensure chip doesn't enter XIP */
-		writel(0xFF, reg_base + CQSPI_REG_MODE_BIT);
-
-		/* Need to subtract the mode byte (8 clocks). */
-		if (f_pdata->inst_width != CQSPI_INST_TYPE_QUAD)
-			dummy_clk -= 8;
-
-		if (dummy_clk)
-			reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
-			       << CQSPI_REG_RD_INSTR_DUMMY_LSB;
-	}
+	if (dummy_clk)
+		reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
+		       << CQSPI_REG_RD_INSTR_DUMMY_LSB;
 
 	writel(reg, reg_base + CQSPI_REG_RD_INSTR);
 
 	/* Set address width */
 	reg = readl(reg_base + CQSPI_REG_SIZE);
 	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
-	reg |= (nor->addr_width - 1);
+	reg |= (op->addr.nbytes - 1);
 	writel(reg, reg_base + CQSPI_REG_SIZE);
 	return 0;
 }
 
-static int cqspi_indirect_read_execute(struct spi_nor *nor, u8 *rxbuf,
-				       loff_t from_addr, const size_t n_rx)
+static int cqspi_indirect_read_execute(struct cqspi_flash_pdata *f_pdata,
+				       u8 *rxbuf, loff_t from_addr,
+				       const size_t n_rx)
 {
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
 	struct cqspi_st *cqspi = f_pdata->cqspi;
+	struct device *dev = &cqspi->pdev->dev;
 	void __iomem *reg_base = cqspi->iobase;
 	void __iomem *ahb_base = cqspi->ahb_base;
 	unsigned int remaining = n_rx;
@@ -526,13 +505,13 @@ static int cqspi_indirect_read_execute(struct spi_nor *nor, u8 *rxbuf,
 
 	while (remaining > 0) {
 		if (!wait_for_completion_timeout(&cqspi->transfer_complete,
-				msecs_to_jiffies(CQSPI_READ_TIMEOUT_MS)))
+						 msecs_to_jiffies(CQSPI_READ_TIMEOUT_MS)))
 			ret = -ETIMEDOUT;
 
 		bytes_to_read = cqspi_get_rd_sram_level(cqspi);
 
 		if (ret && bytes_to_read == 0) {
-			dev_err(nor->dev, "Indirect read timeout, no bytes\n");
+			dev_err(dev, "Indirect read timeout, no bytes\n");
 			goto failrd;
 		}
 
@@ -568,8 +547,7 @@ static int cqspi_indirect_read_execute(struct spi_nor *nor, u8 *rxbuf,
 	ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_INDIRECTRD,
 				 CQSPI_REG_INDIRECTRD_DONE_MASK, 0);
 	if (ret) {
-		dev_err(nor->dev,
-			"Indirect read completion error (%i)\n", ret);
+		dev_err(dev, "Indirect read completion error (%i)\n", ret);
 		goto failrd;
 	}
 
@@ -591,32 +569,32 @@ static int cqspi_indirect_read_execute(struct spi_nor *nor, u8 *rxbuf,
 	return ret;
 }
 
-static int cqspi_write_setup(struct spi_nor *nor)
+static int cqspi_write_setup(struct cqspi_flash_pdata *f_pdata,
+			     const struct spi_mem_op *op)
 {
 	unsigned int reg;
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
 	struct cqspi_st *cqspi = f_pdata->cqspi;
 	void __iomem *reg_base = cqspi->iobase;
 
 	/* Set opcode. */
-	reg = nor->program_opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
+	reg = op->cmd.opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
 	writel(reg, reg_base + CQSPI_REG_WR_INSTR);
-	reg = cqspi_calc_rdreg(nor);
+	reg = cqspi_calc_rdreg(f_pdata);
 	writel(reg, reg_base + CQSPI_REG_RD_INSTR);
 
 	reg = readl(reg_base + CQSPI_REG_SIZE);
 	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
-	reg |= (nor->addr_width - 1);
+	reg |= (op->addr.nbytes - 1);
 	writel(reg, reg_base + CQSPI_REG_SIZE);
 	return 0;
 }
 
-static int cqspi_indirect_write_execute(struct spi_nor *nor, loff_t to_addr,
-					const u8 *txbuf, const size_t n_tx)
+static int cqspi_indirect_write_execute(struct cqspi_flash_pdata *f_pdata,
+					loff_t to_addr, const u8 *txbuf,
+					const size_t n_tx)
 {
-	const unsigned int page_size = nor->page_size;
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
 	struct cqspi_st *cqspi = f_pdata->cqspi;
+	struct device *dev = &cqspi->pdev->dev;
 	void __iomem *reg_base = cqspi->iobase;
 	unsigned int remaining = n_tx;
 	unsigned int write_bytes;
@@ -646,7 +624,7 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor, loff_t to_addr,
 	while (remaining > 0) {
 		size_t write_words, mod_bytes;
 
-		write_bytes = remaining > page_size ? page_size : remaining;
+		write_bytes = remaining;
 		write_words = write_bytes / 4;
 		mod_bytes = write_bytes % 4;
 		/* Write 4 bytes at a time then single bytes. */
@@ -663,8 +641,8 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor, loff_t to_addr,
 		}
 
 		if (!wait_for_completion_timeout(&cqspi->transfer_complete,
-					msecs_to_jiffies(CQSPI_TIMEOUT_MS))) {
-			dev_err(nor->dev, "Indirect write timeout\n");
+						 msecs_to_jiffies(CQSPI_TIMEOUT_MS))) {
+			dev_err(dev, "Indirect write timeout\n");
 			ret = -ETIMEDOUT;
 			goto failwr;
 		}
@@ -679,8 +657,7 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor, loff_t to_addr,
 	ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_INDIRECTWR,
 				 CQSPI_REG_INDIRECTWR_DONE_MASK, 0);
 	if (ret) {
-		dev_err(nor->dev,
-			"Indirect write completion error (%i)\n", ret);
+		dev_err(dev, "Indirect write completion error (%i)\n", ret);
 		goto failwr;
 	}
 
@@ -704,9 +681,8 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor, loff_t to_addr,
 	return ret;
 }
 
-static void cqspi_chipselect(struct spi_nor *nor)
+static void cqspi_chipselect(struct cqspi_flash_pdata *f_pdata)
 {
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
 	struct cqspi_st *cqspi = f_pdata->cqspi;
 	void __iomem *reg_base = cqspi->iobase;
 	unsigned int chip_select = f_pdata->cs;
@@ -745,9 +721,8 @@ static unsigned int calculate_ticks_for_ns(const unsigned int ref_clk_hz,
 	return ticks;
 }
 
-static void cqspi_delay(struct spi_nor *nor)
+static void cqspi_delay(struct cqspi_flash_pdata *f_pdata)
 {
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
 	struct cqspi_st *cqspi = f_pdata->cqspi;
 	void __iomem *iobase = cqspi->iobase;
 	const unsigned int ref_clk_hz = cqspi->master_ref_clk_hz;
@@ -831,11 +806,10 @@ static void cqspi_controller_enable(struct cqspi_st *cqspi, bool enable)
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
 }
 
-static void cqspi_configure(struct spi_nor *nor)
+static void cqspi_configure(struct cqspi_flash_pdata *f_pdata,
+			    unsigned long sclk)
 {
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
 	struct cqspi_st *cqspi = f_pdata->cqspi;
-	const unsigned int sclk = f_pdata->clk_rate;
 	int switch_cs = (cqspi->current_cs != f_pdata->cs);
 	int switch_ck = (cqspi->sclk != sclk);
 
@@ -845,14 +819,14 @@ static void cqspi_configure(struct spi_nor *nor)
 	/* Switch chip select. */
 	if (switch_cs) {
 		cqspi->current_cs = f_pdata->cs;
-		cqspi_chipselect(nor);
+		cqspi_chipselect(f_pdata);
 	}
 
 	/* Setup baudrate divisor and delays */
 	if (switch_ck) {
 		cqspi->sclk = sclk;
 		cqspi_config_baudrate_div(cqspi);
-		cqspi_delay(nor);
+		cqspi_delay(f_pdata);
 		cqspi_readdata_capture(cqspi, !cqspi->rclk_en,
 				       f_pdata->read_delay);
 	}
@@ -861,26 +835,25 @@ static void cqspi_configure(struct spi_nor *nor)
 		cqspi_controller_enable(cqspi, 1);
 }
 
-static int cqspi_set_protocol(struct spi_nor *nor, const int read)
+static int cqspi_set_protocol(struct cqspi_flash_pdata *f_pdata,
+			      const struct spi_mem_op *op)
 {
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
-
 	f_pdata->inst_width = CQSPI_INST_TYPE_SINGLE;
 	f_pdata->addr_width = CQSPI_INST_TYPE_SINGLE;
 	f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
 
-	if (read) {
-		switch (nor->read_proto) {
-		case SNOR_PROTO_1_1_1:
+	if (op->data.dir == SPI_MEM_DATA_IN) {
+		switch (op->data.buswidth) {
+		case 1:
 			f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
 			break;
-		case SNOR_PROTO_1_1_2:
+		case 2:
 			f_pdata->data_width = CQSPI_INST_TYPE_DUAL;
 			break;
-		case SNOR_PROTO_1_1_4:
+		case 4:
 			f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
 			break;
-		case SNOR_PROTO_1_1_8:
+		case 8:
 			f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
 			break;
 		default:
@@ -888,36 +861,32 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
 		}
 	}
 
-	cqspi_configure(nor);
-
 	return 0;
 }
 
-static ssize_t cqspi_write(struct spi_nor *nor, loff_t to,
-			   size_t len, const u_char *buf)
+static ssize_t cqspi_write(struct cqspi_flash_pdata *f_pdata,
+			   const struct spi_mem_op *op)
 {
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
 	struct cqspi_st *cqspi = f_pdata->cqspi;
+	loff_t to = op->addr.val;
+	size_t len = op->data.nbytes;
+	const u_char *buf = op->data.buf.out;
 	int ret;
 
-	ret = cqspi_set_protocol(nor, 0);
+	ret = cqspi_set_protocol(f_pdata, op);
 	if (ret)
 		return ret;
 
-	ret = cqspi_write_setup(nor);
+	ret = cqspi_write_setup(f_pdata, op);
 	if (ret)
 		return ret;
 
-	if (f_pdata->use_direct_mode) {
+	if (cqspi->use_direct_mode && ((to + len) <= cqspi->ahb_size)) {
 		memcpy_toio(cqspi->ahb_base + to, buf, len);
-		ret = cqspi_wait_idle(cqspi);
-	} else {
-		ret = cqspi_indirect_write_execute(nor, to, buf, len);
+		return cqspi_wait_idle(cqspi);
 	}
-	if (ret)
-		return ret;
 
-	return len;
+	return cqspi_indirect_write_execute(f_pdata, to, buf, len);
 }
 
 static void cqspi_rx_dma_callback(void *param)
@@ -927,11 +896,11 @@ static void cqspi_rx_dma_callback(void *param)
 	complete(&cqspi->rx_dma_complete);
 }
 
-static int cqspi_direct_read_execute(struct spi_nor *nor, u_char *buf,
-				     loff_t from, size_t len)
+static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
+				     u_char *buf, loff_t from, size_t len)
 {
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
 	struct cqspi_st *cqspi = f_pdata->cqspi;
+	struct device *dev = &cqspi->pdev->dev;
 	enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
 	dma_addr_t dma_src = (dma_addr_t)cqspi->mmap_phys_base + from;
 	int ret = 0;
@@ -944,15 +913,15 @@ static int cqspi_direct_read_execute(struct spi_nor *nor, u_char *buf,
 		return 0;
 	}
 
-	dma_dst = dma_map_single(nor->dev, buf, len, DMA_FROM_DEVICE);
-	if (dma_mapping_error(nor->dev, dma_dst)) {
-		dev_err(nor->dev, "dma mapping failed\n");
+	dma_dst = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
+	if (dma_mapping_error(dev, dma_dst)) {
+		dev_err(dev, "dma mapping failed\n");
 		return -ENOMEM;
 	}
 	tx = dmaengine_prep_dma_memcpy(cqspi->rx_chan, dma_dst, dma_src,
 				       len, flags);
 	if (!tx) {
-		dev_err(nor->dev, "device_prep_dma_memcpy error\n");
+		dev_err(dev, "device_prep_dma_memcpy error\n");
 		ret = -EIO;
 		goto err_unmap;
 	}
@@ -964,7 +933,7 @@ static int cqspi_direct_read_execute(struct spi_nor *nor, u_char *buf,
 
 	ret = dma_submit_error(cookie);
 	if (ret) {
-		dev_err(nor->dev, "dma_submit_error %d\n", cookie);
+		dev_err(dev, "dma_submit_error %d\n", cookie);
 		ret = -EIO;
 		goto err_unmap;
 	}
@@ -973,94 +942,68 @@ static int cqspi_direct_read_execute(struct spi_nor *nor, u_char *buf,
 	if (!wait_for_completion_timeout(&cqspi->rx_dma_complete,
 					 msecs_to_jiffies(len))) {
 		dmaengine_terminate_sync(cqspi->rx_chan);
-		dev_err(nor->dev, "DMA wait_for_completion_timeout\n");
+		dev_err(dev, "DMA wait_for_completion_timeout\n");
 		ret = -ETIMEDOUT;
 		goto err_unmap;
 	}
 
 err_unmap:
-	dma_unmap_single(nor->dev, dma_dst, len, DMA_FROM_DEVICE);
+	dma_unmap_single(dev, dma_dst, len, DMA_FROM_DEVICE);
 
 	return ret;
 }
 
-static ssize_t cqspi_read(struct spi_nor *nor, loff_t from,
-			  size_t len, u_char *buf)
+static ssize_t cqspi_read(struct cqspi_flash_pdata *f_pdata,
+			  const struct spi_mem_op *op)
 {
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
+	struct cqspi_st *cqspi = f_pdata->cqspi;
+	loff_t from = op->addr.val;
+	size_t len = op->data.nbytes;
+	u_char *buf = op->data.buf.in;
 	int ret;
 
-	ret = cqspi_set_protocol(nor, 1);
+	ret = cqspi_set_protocol(f_pdata, op);
 	if (ret)
 		return ret;
 
-	ret = cqspi_read_setup(nor);
+	ret = cqspi_read_setup(f_pdata, op);
 	if (ret)
 		return ret;
 
-	if (f_pdata->use_direct_mode)
-		ret = cqspi_direct_read_execute(nor, buf, from, len);
-	else
-		ret = cqspi_indirect_read_execute(nor, buf, from, len);
-	if (ret)
-		return ret;
+	if (cqspi->use_direct_mode && ((from + len) <= cqspi->ahb_size))
+		return cqspi_direct_read_execute(f_pdata, buf, from, len);
 
-	return len;
+	return cqspi_indirect_read_execute(f_pdata, buf, from, len);
 }
 
-static int cqspi_erase(struct spi_nor *nor, loff_t offs)
+static int cqspi_mem_process(struct spi_mem *mem, const struct spi_mem_op *op)
 {
-	int ret;
-
-	ret = cqspi_set_protocol(nor, 0);
-	if (ret)
-		return ret;
-
-	/* Set up command buffer. */
-	ret = cqspi_command_write_addr(nor, nor->erase_opcode, offs);
-	if (ret)
-		return ret;
-
-	return 0;
-}
-
-static int cqspi_prep(struct spi_nor *nor)
-{
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
-	struct cqspi_st *cqspi = f_pdata->cqspi;
-
-	mutex_lock(&cqspi->bus_mutex);
-
-	return 0;
-}
+	struct cqspi_st *cqspi = spi_master_get_devdata(mem->spi->master);
+	struct cqspi_flash_pdata *f_pdata;
 
-static void cqspi_unprep(struct spi_nor *nor)
-{
-	struct cqspi_flash_pdata *f_pdata = nor->priv;
-	struct cqspi_st *cqspi = f_pdata->cqspi;
+	f_pdata = &cqspi->f_pdata[mem->spi->chip_select];
+	cqspi_configure(f_pdata, mem->spi->max_speed_hz);
 
-	mutex_unlock(&cqspi->bus_mutex);
-}
+	if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) {
+		if (!op->addr.nbytes)
+			return cqspi_command_read(f_pdata, op);
 
-static int cqspi_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, size_t len)
-{
-	int ret;
+		return cqspi_read(f_pdata, op);
+	}
 
-	ret = cqspi_set_protocol(nor, 0);
-	if (!ret)
-		ret = cqspi_command_read(nor, opcode, buf, len);
+	if (!op->addr.nbytes || !op->data.buf.out)
+		return cqspi_command_write(f_pdata, op);
 
-	return ret;
+	return cqspi_write(f_pdata, op);
 }
 
-static int cqspi_write_reg(struct spi_nor *nor, u8 opcode, const u8 *buf,
-			   size_t len)
+static int cqspi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 {
 	int ret;
 
-	ret = cqspi_set_protocol(nor, 0);
-	if (!ret)
-		ret = cqspi_command_write(nor, opcode, buf, len);
+	ret = cqspi_mem_process(mem, op);
+	if (ret)
+		dev_err(&mem->spi->dev, "operation failed with %d\n", ret);
 
 	return ret;
 }
@@ -1102,26 +1045,26 @@ static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
 	return 0;
 }
 
-static int cqspi_of_get_pdata(struct platform_device *pdev)
+static int cqspi_of_get_pdata(struct cqspi_st *cqspi)
 {
-	struct device_node *np = pdev->dev.of_node;
-	struct cqspi_st *cqspi = platform_get_drvdata(pdev);
+	struct device *dev = &cqspi->pdev->dev;
+	struct device_node *np = dev->of_node;
 
 	cqspi->is_decoded_cs = of_property_read_bool(np, "cdns,is-decoded-cs");
 
 	if (of_property_read_u32(np, "cdns,fifo-depth", &cqspi->fifo_depth)) {
-		dev_err(&pdev->dev, "couldn't determine fifo-depth\n");
+		dev_err(dev, "couldn't determine fifo-depth\n");
 		return -ENXIO;
 	}
 
 	if (of_property_read_u32(np, "cdns,fifo-width", &cqspi->fifo_width)) {
-		dev_err(&pdev->dev, "couldn't determine fifo-width\n");
+		dev_err(dev, "couldn't determine fifo-width\n");
 		return -ENXIO;
 	}
 
 	if (of_property_read_u32(np, "cdns,trigger-address",
 				 &cqspi->trigger_address)) {
-		dev_err(&pdev->dev, "couldn't determine trigger-address\n");
+		dev_err(dev, "couldn't determine trigger-address\n");
 		return -ENXIO;
 	}
 
@@ -1185,47 +1128,30 @@ static int cqspi_request_mmap_dma(struct cqspi_st *cqspi)
 	return 0;
 }
 
-static const struct spi_nor_controller_ops cqspi_controller_ops = {
-	.prepare = cqspi_prep,
-	.unprepare = cqspi_unprep,
-	.read_reg = cqspi_read_reg,
-	.write_reg = cqspi_write_reg,
-	.read = cqspi_read,
-	.write = cqspi_write,
-	.erase = cqspi_erase,
+static const struct spi_controller_mem_ops cqspi_mem_ops = {
+	.exec_op = cqspi_exec_mem_op,
 };
 
-static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
+static int cqspi_setup_flash(struct cqspi_st *cqspi)
 {
 	struct platform_device *pdev = cqspi->pdev;
 	struct device *dev = &pdev->dev;
-	const struct cqspi_driver_platdata *ddata;
-	struct spi_nor_hwcaps hwcaps;
+	struct device_node *np = dev->of_node;
 	struct cqspi_flash_pdata *f_pdata;
-	struct spi_nor *nor;
-	struct mtd_info *mtd;
 	unsigned int cs;
-	int i, ret;
-
-	ddata = of_device_get_match_data(dev);
-	if (!ddata) {
-		dev_err(dev, "Couldn't find driver data\n");
-		return -EINVAL;
-	}
-	hwcaps.mask = ddata->hwcaps_mask;
+	int ret;
 
 	/* Get flash device data */
 	for_each_available_child_of_node(dev->of_node, np) {
 		ret = of_property_read_u32(np, "reg", &cs);
 		if (ret) {
 			dev_err(dev, "Couldn't determine chip select.\n");
-			goto err;
+			return ret;
 		}
 
 		if (cs >= CQSPI_MAX_CHIPSELECT) {
-			ret = -EINVAL;
 			dev_err(dev, "Chip select %d out of range.\n", cs);
-			goto err;
+			return -EINVAL;
 		}
 
 		f_pdata = &cqspi->f_pdata[cs];
@@ -1234,90 +1160,51 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
 
 		ret = cqspi_of_get_flash_pdata(pdev, f_pdata, np);
 		if (ret)
-			goto err;
-
-		nor = &f_pdata->nor;
-		mtd = &nor->mtd;
-
-		mtd->priv = nor;
-
-		nor->dev = dev;
-		spi_nor_set_flash_node(nor, np);
-		nor->priv = f_pdata;
-		nor->controller_ops = &cqspi_controller_ops;
-
-		mtd->name = devm_kasprintf(dev, GFP_KERNEL, "%s.%d",
-					   dev_name(dev), cs);
-		if (!mtd->name) {
-			ret = -ENOMEM;
-			goto err;
-		}
-
-		ret = spi_nor_scan(nor, NULL, &hwcaps);
-		if (ret)
-			goto err;
-
-		ret = mtd_device_register(mtd, NULL, 0);
-		if (ret)
-			goto err;
-
-		f_pdata->registered = true;
-
-		if (mtd->size <= cqspi->ahb_size &&
-		    !(ddata->quirks & CQSPI_DISABLE_DAC_MODE)) {
-			f_pdata->use_direct_mode = true;
-			dev_dbg(nor->dev, "using direct mode for %s\n",
-				mtd->name);
-
-			if (!cqspi->rx_chan) {
-				ret = cqspi_request_mmap_dma(cqspi);
-				if (ret == -EPROBE_DEFER)
-					goto err;
-			}
-		}
+			return ret;
 	}
 
 	return 0;
-
-err:
-	for (i = 0; i < CQSPI_MAX_CHIPSELECT; i++)
-		if (cqspi->f_pdata[i].registered)
-			mtd_device_unregister(&cqspi->f_pdata[i].nor.mtd);
-	return ret;
 }
 
 static int cqspi_probe(struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node;
+	const struct cqspi_driver_platdata *ddata;
+	struct reset_control *rstc, *rstc_ocp;
 	struct device *dev = &pdev->dev;
+	struct spi_master *master;
+	struct resource *res_ahb;
 	struct cqspi_st *cqspi;
 	struct resource *res;
-	struct resource *res_ahb;
-	struct reset_control *rstc, *rstc_ocp;
-	const struct cqspi_driver_platdata *ddata;
 	int ret;
 	int irq;
 
-	cqspi = devm_kzalloc(dev, sizeof(*cqspi), GFP_KERNEL);
-	if (!cqspi)
+	master = spi_alloc_master(&pdev->dev, sizeof(*cqspi));
+	if (!master) {
+		dev_err(&pdev->dev, "spi_alloc_master failed\n");
 		return -ENOMEM;
+	}
+	master->mode_bits = SPI_RX_QUAD | SPI_RX_DUAL;
+	master->mem_ops = &cqspi_mem_ops;
+	master->dev.of_node = pdev->dev.of_node;
+
+	cqspi = spi_master_get_devdata(master);
 
-	mutex_init(&cqspi->bus_mutex);
 	cqspi->pdev = pdev;
-	platform_set_drvdata(pdev, cqspi);
 
 	/* Obtain configuration from OF. */
-	ret = cqspi_of_get_pdata(pdev);
+	ret = cqspi_of_get_pdata(cqspi);
 	if (ret) {
 		dev_err(dev, "Cannot get mandatory OF data.\n");
-		return -ENODEV;
+		ret = -ENODEV;
+		goto probe_master_put;
 	}
 
 	/* Obtain QSPI clock. */
 	cqspi->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(cqspi->clk)) {
 		dev_err(dev, "Cannot claim QSPI clock.\n");
-		return PTR_ERR(cqspi->clk);
+		ret = PTR_ERR(cqspi->clk);
+		goto probe_master_put;
 	}
 
 	/* Obtain and remap controller address. */
@@ -1325,7 +1212,8 @@ static int cqspi_probe(struct platform_device *pdev)
 	cqspi->iobase = devm_ioremap_resource(dev, res);
 	if (IS_ERR(cqspi->iobase)) {
 		dev_err(dev, "Cannot remap controller address.\n");
-		return PTR_ERR(cqspi->iobase);
+		ret = PTR_ERR(cqspi->iobase);
+		goto probe_master_put;
 	}
 
 	/* Obtain and remap AHB address. */
@@ -1333,7 +1221,8 @@ static int cqspi_probe(struct platform_device *pdev)
 	cqspi->ahb_base = devm_ioremap_resource(dev, res_ahb);
 	if (IS_ERR(cqspi->ahb_base)) {
 		dev_err(dev, "Cannot remap AHB address.\n");
-		return PTR_ERR(cqspi->ahb_base);
+		ret = PTR_ERR(cqspi->ahb_base);
+		goto probe_master_put;
 	}
 	cqspi->mmap_phys_base = (dma_addr_t)res_ahb->start;
 	cqspi->ahb_size = resource_size(res_ahb);
@@ -1342,14 +1231,16 @@ static int cqspi_probe(struct platform_device *pdev)
 
 	/* Obtain IRQ line. */
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return -ENXIO;
+	if (irq < 0) {
+		ret = -ENXIO;
+		goto probe_master_put;
+	}
 
 	pm_runtime_enable(dev);
 	ret = pm_runtime_get_sync(dev);
 	if (ret < 0) {
 		pm_runtime_put_noidle(dev);
-		return ret;
+		goto probe_master_put;
 	}
 
 	ret = clk_prepare_enable(cqspi->clk);
@@ -1379,9 +1270,15 @@ static int cqspi_probe(struct platform_device *pdev)
 
 	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
 	ddata  = of_device_get_match_data(dev);
-	if (ddata && (ddata->quirks & CQSPI_NEEDS_WR_DELAY))
-		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
-						   cqspi->master_ref_clk_hz);
+	if (ddata) {
+		if (ddata->quirks & CQSPI_NEEDS_WR_DELAY)
+			cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
+						cqspi->master_ref_clk_hz);
+		if (ddata->hwcaps_mask & CQSPI_SUPPORTS_OCTAL)
+			master->mode_bits |= SPI_RX_OCTAL;
+		if (!(ddata->quirks & CQSPI_DISABLE_DAC_MODE))
+			cqspi->use_direct_mode = true;
+	}
 
 	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
 			       pdev->name, cqspi);
@@ -1395,13 +1292,25 @@ static int cqspi_probe(struct platform_device *pdev)
 	cqspi->current_cs = -1;
 	cqspi->sclk = 0;
 
-	ret = cqspi_setup_flash(cqspi, np);
+	ret = cqspi_setup_flash(cqspi);
 	if (ret) {
-		dev_err(dev, "Cadence QSPI NOR probe failed %d\n", ret);
+		dev_err(dev, "failed to setup flash parameters %d\n", ret);
 		goto probe_setup_failed;
 	}
 
-	return ret;
+	if (cqspi->use_direct_mode) {
+		ret = cqspi_request_mmap_dma(cqspi);
+		if (ret == -EPROBE_DEFER)
+			goto probe_setup_failed;
+	}
+
+	ret = devm_spi_register_master(dev, master);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register SPI ctlr %d\n", ret);
+		goto probe_setup_failed;
+	}
+
+	return 0;
 probe_setup_failed:
 	cqspi_controller_enable(cqspi, 0);
 probe_reset_failed:
@@ -1409,17 +1318,14 @@ static int cqspi_probe(struct platform_device *pdev)
 probe_clk_failed:
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
+probe_master_put:
+	spi_master_put(master);
 	return ret;
 }
 
 static int cqspi_remove(struct platform_device *pdev)
 {
 	struct cqspi_st *cqspi = platform_get_drvdata(pdev);
-	int i;
-
-	for (i = 0; i < CQSPI_MAX_CHIPSELECT; i++)
-		if (cqspi->f_pdata[i].registered)
-			mtd_device_unregister(&cqspi->f_pdata[i].nor.mtd);
 
 	cqspi_controller_enable(cqspi, 0);
 
@@ -1462,17 +1368,15 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
 #endif
 
 static const struct cqspi_driver_platdata cdns_qspi = {
-	.hwcaps_mask = CQSPI_BASE_HWCAPS_MASK,
 	.quirks = CQSPI_DISABLE_DAC_MODE,
 };
 
 static const struct cqspi_driver_platdata k2g_qspi = {
-	.hwcaps_mask = CQSPI_BASE_HWCAPS_MASK,
 	.quirks = CQSPI_NEEDS_WR_DELAY,
 };
 
 static const struct cqspi_driver_platdata am654_ospi = {
-	.hwcaps_mask = CQSPI_BASE_HWCAPS_MASK | SNOR_HWCAPS_READ_1_1_8,
+	.hwcaps_mask = CQSPI_SUPPORTS_OCTAL,
 	.quirks = CQSPI_NEEDS_WR_DELAY,
 };
 
@@ -1511,3 +1415,5 @@ MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("platform:" CQSPI_NAME);
 MODULE_AUTHOR("Ley Foon Tan <lftan@altera.com>");
 MODULE_AUTHOR("Graham Moore <grmoore@opensource.altera.com>");
+MODULE_AUTHOR("Vadivel Murugan R <vadivel.muruganx.ramuthevar@intel.com>");
+MODULE_AUTHOR("Vignesh Raghavendra <vigneshr@ti.com>");
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [RESEND PATCH v3 8/8] spi: Move cadence-quadspi driver to drivers/spi/
  2020-06-01  7:04 [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Vignesh Raghavendra
                   ` (6 preceding siblings ...)
  2020-06-01  7:04 ` [RESEND PATCH v3 7/8] mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework Vignesh Raghavendra
@ 2020-06-01  7:04 ` Vignesh Raghavendra
  2020-06-19 10:57 ` [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Mark Brown
  2020-06-19 13:28 ` Mark Brown
  9 siblings, 0 replies; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-06-01  7:04 UTC (permalink / raw)
  To: Tudor Ambarus, Mark Brown
  Cc: Vignesh Raghavendra, Boris Brezillon, Ramuthevar Vadivel Murugan,
	linux-mtd, linux-kernel, linux-spi, simon.k.r.goldschmidt,
	dinguyen, marex

From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>

Now that cadence-quadspi has been converted to use spi-mem framework,
move it under drivers/spi/

Update license header to match SPI subsystem style

Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/controllers/Kconfig            | 11 -----------
 drivers/mtd/spi-nor/controllers/Makefile           |  1 -
 drivers/spi/Kconfig                                | 11 +++++++++++
 drivers/spi/Makefile                               |  1 +
 .../spi-cadence-quadspi.c}                         | 14 +++++++-------
 5 files changed, 19 insertions(+), 19 deletions(-)
 rename drivers/{mtd/spi-nor/controllers/cadence-quadspi.c => spi/spi-cadence-quadspi.c} (99%)

diff --git a/drivers/mtd/spi-nor/controllers/Kconfig b/drivers/mtd/spi-nor/controllers/Kconfig
index 10b86660b8214..95fcd4b435be5 100644
--- a/drivers/mtd/spi-nor/controllers/Kconfig
+++ b/drivers/mtd/spi-nor/controllers/Kconfig
@@ -9,17 +9,6 @@ config SPI_ASPEED_SMC
 	  and support for the SPI flash memory controller (SPI) for
 	  the host firmware. The implementation only supports SPI NOR.
 
-config SPI_CADENCE_QUADSPI
-	tristate "Cadence Quad SPI controller"
-	depends on OF && (ARM || ARM64 || COMPILE_TEST)
-	help
-	  Enable support for the Cadence Quad SPI Flash controller.
-
-	  Cadence QSPI is a specialized controller for connecting an SPI
-	  Flash over 1/2/4-bit wide bus. Enable this option if you have a
-	  device with a Cadence QSPI controller and want to access the
-	  Flash as an MTD device.
-
 config SPI_HISI_SFC
 	tristate "Hisilicon FMC SPI-NOR Flash Controller(SFC)"
 	depends on ARCH_HISI || COMPILE_TEST
diff --git a/drivers/mtd/spi-nor/controllers/Makefile b/drivers/mtd/spi-nor/controllers/Makefile
index 46e6fbe586e34..e7abba491d983 100644
--- a/drivers/mtd/spi-nor/controllers/Makefile
+++ b/drivers/mtd/spi-nor/controllers/Makefile
@@ -1,6 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_SPI_ASPEED_SMC)	+= aspeed-smc.o
-obj-$(CONFIG_SPI_CADENCE_QUADSPI)	+= cadence-quadspi.o
 obj-$(CONFIG_SPI_HISI_SFC)	+= hisi-sfc.o
 obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
 obj-$(CONFIG_SPI_INTEL_SPI)	+= intel-spi.o
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 741b9140992a8..a4f65d956fa0a 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -200,6 +200,17 @@ config SPI_CADENCE
 	  This selects the Cadence SPI controller master driver
 	  used by Xilinx Zynq and ZynqMP.
 
+config SPI_CADENCE_QUADSPI
+	tristate "Cadence Quad SPI controller"
+	depends on OF && (ARM || ARM64 || COMPILE_TEST)
+	help
+	  Enable support for the Cadence Quad SPI Flash controller.
+
+	  Cadence QSPI is a specialized controller for connecting an SPI
+	  Flash over 1/2/4-bit wide bus. Enable this option if you have a
+	  device with a Cadence QSPI controller and want to access the
+	  Flash as an MTD device.
+
 config SPI_CLPS711X
 	tristate "CLPS711X host SPI controller"
 	depends on ARCH_CLPS711X || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 28f601327f8c7..c02caeeb99b85 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_SPI_BCM_QSPI)		+= spi-iproc-qspi.o spi-brcmstb-qspi.o spi-bcm-qspi.
 obj-$(CONFIG_SPI_BITBANG)		+= spi-bitbang.o
 obj-$(CONFIG_SPI_BUTTERFLY)		+= spi-butterfly.o
 obj-$(CONFIG_SPI_CADENCE)		+= spi-cadence.o
+obj-$(CONFIG_SPI_CADENCE_QUADSPI)	+= spi-cadence-quadspi.o
 obj-$(CONFIG_SPI_CLPS711X)		+= spi-clps711x.o
 obj-$(CONFIG_SPI_COLDFIRE_QSPI)		+= spi-coldfire-qspi.o
 obj-$(CONFIG_SPI_DAVINCI)		+= spi-davinci.o
diff --git a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
similarity index 99%
rename from drivers/mtd/spi-nor/controllers/cadence-quadspi.c
rename to drivers/spi/spi-cadence-quadspi.c
index c12a1c0191b92..1c1a9d17eec0d 100644
--- a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1,11 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0-only
-/*
- * Driver for Cadence QSPI Controller
- *
- * Copyright Altera Corporation (C) 2012-2014. All rights reserved.
- * Copyright Intel Corporation (C) 2019-2020. All rights reserved.
- * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com
- */
+//
+// Driver for Cadence QSPI Controller
+//
+// Copyright Altera Corporation (C) 2012-2014. All rights reserved.
+// Copyright Intel Corporation (C) 2019-2020. All rights reserved.
+// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com
+
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework
  2020-06-01  7:04 [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Vignesh Raghavendra
                   ` (7 preceding siblings ...)
  2020-06-01  7:04 ` [RESEND PATCH v3 8/8] spi: Move cadence-quadspi driver to drivers/spi/ Vignesh Raghavendra
@ 2020-06-19 10:57 ` Mark Brown
  2020-06-19 11:47   ` Tudor.Ambarus
  2020-06-19 13:28 ` Mark Brown
  9 siblings, 1 reply; 28+ messages in thread
From: Mark Brown @ 2020-06-19 10:57 UTC (permalink / raw)
  To: Vignesh Raghavendra
  Cc: Tudor Ambarus, Boris Brezillon, Ramuthevar Vadivel Murugan,
	linux-mtd, linux-kernel, linux-spi, simon.k.r.goldschmidt,
	dinguyen, marex

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

On Mon, Jun 01, 2020 at 12:34:36PM +0530, Vignesh Raghavendra wrote:
> This series is a subset of "[PATCH v12 0/4] spi: cadence-quadspi: Add
> support for the Cadence QSPI controller" by Ramuthevar,Vadivel MuruganX
> <vadivel.muruganx.ramuthevar@linux.intel.com> that intended to move
> cadence-quadspi driver to spi-mem framework

Are people OK with me applying this to the SPI tree?

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework
  2020-06-19 10:57 ` [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Mark Brown
@ 2020-06-19 11:47   ` Tudor.Ambarus
  2020-06-19 15:17     ` Mark Brown
  0 siblings, 1 reply; 28+ messages in thread
From: Tudor.Ambarus @ 2020-06-19 11:47 UTC (permalink / raw)
  To: broonie, vigneshr
  Cc: bbrezillon, vadivel.muruganx.ramuthevar, linux-mtd, linux-kernel,
	linux-spi, simon.k.r.goldschmidt, dinguyen, marex

On 6/19/20 1:57 PM, Mark Brown wrote:
>> This series is a subset of "[PATCH v12 0/4] spi: cadence-quadspi: Add
>> support for the Cadence QSPI controller" by Ramuthevar,Vadivel MuruganX
>> <vadivel.muruganx.ramuthevar@linux.intel.com> that intended to move
>> cadence-quadspi driver to spi-mem framework
> Are people OK with me applying this to the SPI tree?

There's a small conflict on 8/8 when applying on top of v5.8-rc1. With
that addressed:

Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Would you please provide an immutable tag on top of v5.8-rc1 so that I
can merge back in spi-nor/next?

Cheers,
ta

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework
  2020-06-01  7:04 [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Vignesh Raghavendra
                   ` (8 preceding siblings ...)
  2020-06-19 10:57 ` [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Mark Brown
@ 2020-06-19 13:28 ` Mark Brown
  9 siblings, 0 replies; 28+ messages in thread
From: Mark Brown @ 2020-06-19 13:28 UTC (permalink / raw)
  To: Vignesh Raghavendra, Tudor Ambarus
  Cc: dinguyen, Boris Brezillon, Ramuthevar Vadivel Murugan, linux-spi,
	simon.k.r.goldschmidt, linux-kernel, marex, linux-mtd

On Mon, 1 Jun 2020 12:34:36 +0530, Vignesh Raghavendra wrote:
> This series is a subset of "[PATCH v12 0/4] spi: cadence-quadspi: Add
> support for the Cadence QSPI controller" by Ramuthevar,Vadivel MuruganX
> <vadivel.muruganx.ramuthevar@linux.intel.com> that intended to move
> cadence-quadspi driver to spi-mem framework
> 
> Those patches were trying to accomplish too many things in a single set
> of patches and need to split into smaller patches. This is reduced
> version of above series.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/8] mtd: spi-nor: cadence-quadspi: Make driver independent of flash geometry
      commit: 834b4e8d344139ba64cda22099b2b2ef6c9a542d
[2/8] mtd: spi-nor: cadence-quadspi: Provide a way to disable DAC mode
      commit: a99705079a91e6373122bd0ca2fc129b688fc5b3
[3/8] mtd: spi-nor: cadence-quadspi: Don't initialize rx_dma_complete on failure
      commit: 48aae57f0f9f57797772bd462b4d64902b1b4ae1
[4/8] mtd: spi-nor: cadence-quadspi: Fix error path on failure to acquire reset lines
      commit: c61088d1f9932940af780b674f028140eda09a94
[5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
      commit: 935da5e5100f57d843cac4781b21f1c235059aa0
[6/8] mtd: spi-nor: cadence-quadspi: Drop redundant WREN in erase path
      commit: 41b5ed6e677ca73cb031b7657eefb5cf27071be3
[7/8] mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework
      commit: a314f6367787ee1d767df9a2120f17e4511144d0
[8/8] spi: Move cadence-quadspi driver to drivers/spi/
      commit: 31fb632b5d43ca16713095b3a4fe17e3d7331e28

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework
  2020-06-19 11:47   ` Tudor.Ambarus
@ 2020-06-19 15:17     ` Mark Brown
  2020-06-26  9:25       ` Tudor.Ambarus
  0 siblings, 1 reply; 28+ messages in thread
From: Mark Brown @ 2020-06-19 15:17 UTC (permalink / raw)
  To: Tudor.Ambarus
  Cc: vigneshr, bbrezillon, vadivel.muruganx.ramuthevar, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex

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

On Fri, Jun 19, 2020 at 11:47:08AM +0000, Tudor.Ambarus@microchip.com wrote:

> Would you please provide an immutable tag on top of v5.8-rc1 so that I
> can merge back in spi-nor/next?

Here you go:

The following changes since commit b3a9e3b9622ae10064826dccb4f7a52bd88c7407:

  Linux 5.8-rc1 (2020-06-14 12:45:04 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git tags/cadence-mtd-spi-move

for you to fetch changes up to 31fb632b5d43ca16713095b3a4fe17e3d7331e28:

  spi: Move cadence-quadspi driver to drivers/spi/ (2020-06-19 14:26:54 +0100)

----------------------------------------------------------------
mtd/spi: Move the cadence-quadspi driver to spi-mem

----------------------------------------------------------------
Ramuthevar Vadivel Murugan (2):
      mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework
      spi: Move cadence-quadspi driver to drivers/spi/

Vignesh Raghavendra (6):
      mtd: spi-nor: cadence-quadspi: Make driver independent of flash geometry
      mtd: spi-nor: cadence-quadspi: Provide a way to disable DAC mode
      mtd: spi-nor: cadence-quadspi: Don't initialize rx_dma_complete on failure
      mtd: spi-nor: cadence-quadspi: Fix error path on failure to acquire reset lines
      mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
      mtd: spi-nor: cadence-quadspi: Drop redundant WREN in erase path

 drivers/mtd/spi-nor/controllers/Kconfig            |  11 -
 drivers/mtd/spi-nor/controllers/Makefile           |   1 -
 drivers/spi/Kconfig                                |  11 +
 drivers/spi/Makefile                               |   1 +
 .../spi-cadence-quadspi.c}                         | 541 ++++++++-------------
 5 files changed, 222 insertions(+), 343 deletions(-)
 rename drivers/{mtd/spi-nor/controllers/cadence-quadspi.c => spi/spi-cadence-quadspi.c} (74%)

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework
  2020-06-19 15:17     ` Mark Brown
@ 2020-06-26  9:25       ` Tudor.Ambarus
  0 siblings, 0 replies; 28+ messages in thread
From: Tudor.Ambarus @ 2020-06-26  9:25 UTC (permalink / raw)
  To: broonie
  Cc: vigneshr, bbrezillon, vadivel.muruganx.ramuthevar, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex

On 6/19/20 6:17 PM, Mark Brown wrote:
>> Would you please provide an immutable tag on top of v5.8-rc1 so that I
>> can merge back in spi-nor/next?
> Here you go:
> 
> The following changes since commit b3a9e3b9622ae10064826dccb4f7a52bd88c7407:
> 
>   Linux 5.8-rc1 (2020-06-14 12:45:04 -0700)
> 
> are available in the Git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git tags/cadence-mtd-spi-move
> 
> for you to fetch changes up to 31fb632b5d43ca16713095b3a4fe17e3d7331e28:
> 
>   spi: Move cadence-quadspi driver to drivers/spi/ (2020-06-19 14:26:54 +0100)
> 
> ----------------------------------------------------------------
> mtd/spi: Move the cadence-quadspi driver to spi-mem
> 
> ----------------------------------------------------------------
> Ramuthevar Vadivel Murugan (2):
>       mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework
>       spi: Move cadence-quadspi driver to drivers/spi/
> 
> Vignesh Raghavendra (6):
>       mtd: spi-nor: cadence-quadspi: Make driver independent of flash geometry
>       mtd: spi-nor: cadence-quadspi: Provide a way to disable DAC mode
>       mtd: spi-nor: cadence-quadspi: Don't initialize rx_dma_complete on failure
>       mtd: spi-nor: cadence-quadspi: Fix error path on failure to acquire reset lines
>       mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
>       mtd: spi-nor: cadence-quadspi: Drop redundant WREN in erase path
> 
>  drivers/mtd/spi-nor/controllers/Kconfig            |  11 -
>  drivers/mtd/spi-nor/controllers/Makefile           |   1 -
>  drivers/spi/Kconfig                                |  11 +
>  drivers/spi/Makefile                               |   1 +
>  .../spi-cadence-quadspi.c}                         | 541 ++++++++-------------
>  5 files changed, 222 insertions(+), 343 deletions(-)
>  rename drivers/{mtd/spi-nor/controllers/cadence-quadspi.c => spi/spi-cadence-quadspi.c} (74%)

Merged also in spi-nor/next in order to avoid conflicts during the release cycle.
Thanks,
ta

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
  2020-06-01  7:04 ` [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel Vignesh Raghavendra
@ 2020-08-22 18:05   ` Jan Kiszka
  2020-08-24 11:45     ` Vignesh Raghavendra
  0 siblings, 1 reply; 28+ messages in thread
From: Jan Kiszka @ 2020-08-22 18:05 UTC (permalink / raw)
  To: Vignesh Raghavendra, Tudor Ambarus, Mark Brown
  Cc: Boris Brezillon, Ramuthevar Vadivel Murugan, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex

On 01.06.20 09:04, Vignesh Raghavendra wrote:
> dma_request_chan_by_mask() can throw EPROBE_DEFER if DMA provider
> is not yet probed. Currently driver just falls back to using PIO mode
> (which is less efficient) in this case. Instead return probe deferral
> error as is so that driver will be re probed once DMA provider is
> available.
>
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  .../mtd/spi-nor/controllers/cadence-quadspi.c  | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
> index 608ca657ff7f5..0570ebca135a9 100644
> --- a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
> @@ -1169,7 +1169,7 @@ static void cqspi_controller_init(struct cqspi_st *cqspi)
>  	cqspi_controller_enable(cqspi, 1);
>  }
>
> -static void cqspi_request_mmap_dma(struct cqspi_st *cqspi)
> +static int cqspi_request_mmap_dma(struct cqspi_st *cqspi)
>  {
>  	dma_cap_mask_t mask;
>
> @@ -1178,11 +1178,16 @@ static void cqspi_request_mmap_dma(struct cqspi_st *cqspi)
>
>  	cqspi->rx_chan = dma_request_chan_by_mask(&mask);
>  	if (IS_ERR(cqspi->rx_chan)) {
> -		dev_err(&cqspi->pdev->dev, "No Rx DMA available\n");
> +		int ret = PTR_ERR(cqspi->rx_chan);
> +
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(&cqspi->pdev->dev, "No Rx DMA available\n");
>  		cqspi->rx_chan = NULL;
> -		return;
> +		return ret;
>  	}
>  	init_completion(&cqspi->rx_dma_complete);
> +
> +	return 0;
>  }
>
>  static const struct spi_nor_controller_ops cqspi_controller_ops = {
> @@ -1269,8 +1274,11 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>  			dev_dbg(nor->dev, "using direct mode for %s\n",
>  				mtd->name);
>
> -			if (!cqspi->rx_chan)
> -				cqspi_request_mmap_dma(cqspi);
> +			if (!cqspi->rx_chan) {
> +				ret = cqspi_request_mmap_dma(cqspi);
> +				if (ret == -EPROBE_DEFER)
> +					goto err;
> +			}
>  		}
>  	}
>
>

This seem to break reading the SPI flash on our IOT2050 [1] (didn't test
the eval board yet).

Without that commit, read happens via PIO, and that works. With the
commit, the pattern

with open("out.bin", "wb") as out:
    pos = 0
    while pos < 2:
        with open("/dev/mtd0", "rb") as mtd:
           mtd.seek(pos * 0x10000)
           out.write(mtd.read(0x10000))
        pos += 1

gives the wrong result for the second block while

with open("out2.bin", "wb") as out:
    with open("/dev/mtd0", "rb") as mtd:
        out.write(mtd.read(0x20000))

(or "mtd_debug read") is fine.

What could be the reason? Our DTBs and k3-am654-base-board.dtb had some
deviations /wrt the ospi node, but aligning ours to the base board made
no difference.

Jan

[1] https://github.com/siemens/linux/commits/jan/iot2050

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 7/8] mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework
  2020-06-01  7:04 ` [RESEND PATCH v3 7/8] mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework Vignesh Raghavendra
@ 2020-08-24  5:55   ` Jan Kiszka
  2020-08-24 11:44     ` Vignesh Raghavendra
  0 siblings, 1 reply; 28+ messages in thread
From: Jan Kiszka @ 2020-08-24  5:55 UTC (permalink / raw)
  To: Vignesh Raghavendra, Tudor Ambarus, Mark Brown
  Cc: Boris Brezillon, Ramuthevar Vadivel Murugan, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex

On 01.06.20 09:04, Vignesh Raghavendra wrote:
> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> 
> Move cadence-quadspi driver to use spi-mem framework. This is required
> to make the driver support for SPI NAND flashes in future.
> 
> Driver is feature compliant with existing SPI NOR version.
> 
> Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  .../mtd/spi-nor/controllers/cadence-quadspi.c | 476 +++++++-----------
>  1 file changed, 191 insertions(+), 285 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
> index 6b1cbad25e3f6..c12a1c0191b92 100644
> --- a/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/controllers/cadence-quadspi.c
> @@ -3,6 +3,8 @@
>   * Driver for Cadence QSPI Controller
>   *
>   * Copyright Altera Corporation (C) 2012-2014. All rights reserved.
> + * Copyright Intel Corporation (C) 2019-2020. All rights reserved.
> + * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com
>   */
>  #include <linux/clk.h>
>  #include <linux/completion.h>
> @@ -17,9 +19,6 @@
>  #include <linux/jiffies.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/mtd/mtd.h>
> -#include <linux/mtd/partitions.h>
> -#include <linux/mtd/spi-nor.h>
>  #include <linux/of_device.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
> @@ -27,6 +26,7 @@
>  #include <linux/reset.h>
>  #include <linux/sched.h>
>  #include <linux/spi/spi.h>
> +#include <linux/spi/spi-mem.h>
>  #include <linux/timer.h>
>  
>  #define CQSPI_NAME			"cadence-qspi"
> @@ -36,16 +36,12 @@
>  #define CQSPI_NEEDS_WR_DELAY		BIT(0)
>  #define CQSPI_DISABLE_DAC_MODE		BIT(1)
>  
> -/* Capabilities mask */
> -#define CQSPI_BASE_HWCAPS_MASK					\
> -	(SNOR_HWCAPS_READ | SNOR_HWCAPS_READ_FAST |		\
> -	SNOR_HWCAPS_READ_1_1_2 | SNOR_HWCAPS_READ_1_1_4 |	\
> -	SNOR_HWCAPS_PP)
> +/* Capabilities */
> +#define CQSPI_SUPPORTS_OCTAL		BIT(0)
>  
>  struct cqspi_st;
>  
>  struct cqspi_flash_pdata {
> -	struct spi_nor	nor;
>  	struct cqspi_st	*cqspi;
>  	u32		clk_rate;
>  	u32		read_delay;
> @@ -57,8 +53,6 @@ struct cqspi_flash_pdata {
>  	u8		addr_width;
>  	u8		data_width;
>  	u8		cs;
> -	bool		registered;
> -	bool		use_direct_mode;
>  };
>  
>  struct cqspi_st {
> @@ -71,7 +65,6 @@ struct cqspi_st {
>  	void __iomem		*ahb_base;
>  	resource_size_t		ahb_size;
>  	struct completion	transfer_complete;
> -	struct mutex		bus_mutex;
>  
>  	struct dma_chan		*rx_chan;
>  	struct completion	rx_dma_complete;
> @@ -85,6 +78,7 @@ struct cqspi_st {
>  	bool			rclk_en;
>  	u32			trigger_address;
>  	u32			wr_delay;
> +	bool			use_direct_mode;
>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>  };
>  
> @@ -283,9 +277,8 @@ static irqreturn_t cqspi_irq_handler(int this_irq, void *dev)
>  	return IRQ_HANDLED;
>  }
>  
> -static unsigned int cqspi_calc_rdreg(struct spi_nor *nor)
> +static unsigned int cqspi_calc_rdreg(struct cqspi_flash_pdata *f_pdata)
>  {
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
>  	u32 rdreg = 0;
>  
>  	rdreg |= f_pdata->inst_width << CQSPI_REG_RD_INSTR_TYPE_INSTR_LSB;
> @@ -352,19 +345,21 @@ static int cqspi_exec_flash_cmd(struct cqspi_st *cqspi, unsigned int reg)
>  	return cqspi_wait_idle(cqspi);
>  }
>  
> -static int cqspi_command_read(struct spi_nor *nor, u8 opcode,
> -			      u8 *rxbuf, size_t n_rx)
> +static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
> +			      const struct spi_mem_op *op)
>  {
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
>  	void __iomem *reg_base = cqspi->iobase;
> +	u8 *rxbuf = op->data.buf.in;
> +	u8 opcode = op->cmd.opcode;
> +	size_t n_rx = op->data.nbytes;
>  	unsigned int rdreg;
>  	unsigned int reg;
>  	size_t read_len;
>  	int status;
>  
>  	if (!n_rx || n_rx > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) {
> -		dev_err(nor->dev,
> +		dev_err(&cqspi->pdev->dev,
>  			"Invalid input argument, len %zu rxbuf 0x%p\n",
>  			n_rx, rxbuf);
>  		return -EINVAL;
> @@ -372,7 +367,7 @@ static int cqspi_command_read(struct spi_nor *nor, u8 opcode,
>  
>  	reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;
>  
> -	rdreg = cqspi_calc_rdreg(nor);
> +	rdreg = cqspi_calc_rdreg(f_pdata);
>  	writel(rdreg, reg_base + CQSPI_REG_RD_INSTR);
>  
>  	reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB);
> @@ -401,25 +396,36 @@ static int cqspi_command_read(struct spi_nor *nor, u8 opcode,
>  	return 0;
>  }
>  
> -static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
> -			       const u8 *txbuf, size_t n_tx)
> +static int cqspi_command_write(struct cqspi_flash_pdata *f_pdata,
> +			       const struct spi_mem_op *op)
>  {
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
>  	void __iomem *reg_base = cqspi->iobase;
> +	const u8 opcode = op->cmd.opcode;
> +	const u8 *txbuf = op->data.buf.out;
> +	size_t n_tx = op->data.nbytes;
>  	unsigned int reg;
>  	unsigned int data;
>  	size_t write_len;
> -	int ret;
>  
>  	if (n_tx > CQSPI_STIG_DATA_LEN_MAX || (n_tx && !txbuf)) {
> -		dev_err(nor->dev,
> +		dev_err(&cqspi->pdev->dev,
>  			"Invalid input argument, cmdlen %zu txbuf 0x%p\n",
>  			n_tx, txbuf);
>  		return -EINVAL;
>  	}
>  
>  	reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;
> +
> +	if (op->addr.nbytes) {
> +		reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB);
> +		reg |= ((op->addr.nbytes - 1) &
> +			CQSPI_REG_CMDCTRL_ADD_BYTES_MASK)
> +			<< CQSPI_REG_CMDCTRL_ADD_BYTES_LSB;
> +
> +		writel(op->addr.val, reg_base + CQSPI_REG_CMDADDRESS);
> +	}
> +
>  	if (n_tx) {
>  		reg |= (0x1 << CQSPI_REG_CMDCTRL_WR_EN_LSB);
>  		reg |= ((n_tx - 1) & CQSPI_REG_CMDCTRL_WR_BYTES_MASK)
> @@ -437,73 +443,46 @@ static int cqspi_command_write(struct spi_nor *nor, const u8 opcode,
>  			writel(data, reg_base + CQSPI_REG_CMDWRITEDATAUPPER);
>  		}
>  	}
> -	ret = cqspi_exec_flash_cmd(cqspi, reg);
> -	return ret;
> -}
> -
> -static int cqspi_command_write_addr(struct spi_nor *nor,
> -				    const u8 opcode, const unsigned int addr)
> -{
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
> -	struct cqspi_st *cqspi = f_pdata->cqspi;
> -	void __iomem *reg_base = cqspi->iobase;
> -	unsigned int reg;
> -
> -	reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;
> -	reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB);
> -	reg |= ((nor->addr_width - 1) & CQSPI_REG_CMDCTRL_ADD_BYTES_MASK)
> -		<< CQSPI_REG_CMDCTRL_ADD_BYTES_LSB;
> -
> -	writel(addr, reg_base + CQSPI_REG_CMDADDRESS);
>  
>  	return cqspi_exec_flash_cmd(cqspi, reg);
>  }
>  
> -static int cqspi_read_setup(struct spi_nor *nor)
> +static int cqspi_read_setup(struct cqspi_flash_pdata *f_pdata,
> +			    const struct spi_mem_op *op)
>  {
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
>  	void __iomem *reg_base = cqspi->iobase;
>  	unsigned int dummy_clk = 0;
>  	unsigned int reg;
>  
> -	reg = nor->read_opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB;
> -	reg |= cqspi_calc_rdreg(nor);
> +	reg = op->cmd.opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB;
> +	reg |= cqspi_calc_rdreg(f_pdata);
>  
>  	/* Setup dummy clock cycles */
> -	dummy_clk = nor->read_dummy;
> +	dummy_clk = op->dummy.nbytes * 8;
>  	if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
>  		dummy_clk = CQSPI_DUMMY_CLKS_MAX;
>  
> -	if (dummy_clk / 8) {
> -		reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB);
> -		/* Set mode bits high to ensure chip doesn't enter XIP */
> -		writel(0xFF, reg_base + CQSPI_REG_MODE_BIT);
> -
> -		/* Need to subtract the mode byte (8 clocks). */
> -		if (f_pdata->inst_width != CQSPI_INST_TYPE_QUAD)
> -			dummy_clk -= 8;
> -
> -		if (dummy_clk)
> -			reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
> -			       << CQSPI_REG_RD_INSTR_DUMMY_LSB;
> -	}
> +	if (dummy_clk)
> +		reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
> +		       << CQSPI_REG_RD_INSTR_DUMMY_LSB;
>  
>  	writel(reg, reg_base + CQSPI_REG_RD_INSTR);
>  
>  	/* Set address width */
>  	reg = readl(reg_base + CQSPI_REG_SIZE);
>  	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
> -	reg |= (nor->addr_width - 1);
> +	reg |= (op->addr.nbytes - 1);
>  	writel(reg, reg_base + CQSPI_REG_SIZE);
>  	return 0;
>  }
>  
> -static int cqspi_indirect_read_execute(struct spi_nor *nor, u8 *rxbuf,
> -				       loff_t from_addr, const size_t n_rx)
> +static int cqspi_indirect_read_execute(struct cqspi_flash_pdata *f_pdata,
> +				       u8 *rxbuf, loff_t from_addr,
> +				       const size_t n_rx)
>  {
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
> +	struct device *dev = &cqspi->pdev->dev;
>  	void __iomem *reg_base = cqspi->iobase;
>  	void __iomem *ahb_base = cqspi->ahb_base;
>  	unsigned int remaining = n_rx;
> @@ -526,13 +505,13 @@ static int cqspi_indirect_read_execute(struct spi_nor *nor, u8 *rxbuf,
>  
>  	while (remaining > 0) {
>  		if (!wait_for_completion_timeout(&cqspi->transfer_complete,
> -				msecs_to_jiffies(CQSPI_READ_TIMEOUT_MS)))
> +						 msecs_to_jiffies(CQSPI_READ_TIMEOUT_MS)))
>  			ret = -ETIMEDOUT;
>  
>  		bytes_to_read = cqspi_get_rd_sram_level(cqspi);
>  
>  		if (ret && bytes_to_read == 0) {
> -			dev_err(nor->dev, "Indirect read timeout, no bytes\n");
> +			dev_err(dev, "Indirect read timeout, no bytes\n");
>  			goto failrd;
>  		}
>  
> @@ -568,8 +547,7 @@ static int cqspi_indirect_read_execute(struct spi_nor *nor, u8 *rxbuf,
>  	ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_INDIRECTRD,
>  				 CQSPI_REG_INDIRECTRD_DONE_MASK, 0);
>  	if (ret) {
> -		dev_err(nor->dev,
> -			"Indirect read completion error (%i)\n", ret);
> +		dev_err(dev, "Indirect read completion error (%i)\n", ret);
>  		goto failrd;
>  	}
>  
> @@ -591,32 +569,32 @@ static int cqspi_indirect_read_execute(struct spi_nor *nor, u8 *rxbuf,
>  	return ret;
>  }
>  
> -static int cqspi_write_setup(struct spi_nor *nor)
> +static int cqspi_write_setup(struct cqspi_flash_pdata *f_pdata,
> +			     const struct spi_mem_op *op)
>  {
>  	unsigned int reg;
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
>  	void __iomem *reg_base = cqspi->iobase;
>  
>  	/* Set opcode. */
> -	reg = nor->program_opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
> +	reg = op->cmd.opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
>  	writel(reg, reg_base + CQSPI_REG_WR_INSTR);
> -	reg = cqspi_calc_rdreg(nor);
> +	reg = cqspi_calc_rdreg(f_pdata);
>  	writel(reg, reg_base + CQSPI_REG_RD_INSTR);
>  
>  	reg = readl(reg_base + CQSPI_REG_SIZE);
>  	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
> -	reg |= (nor->addr_width - 1);
> +	reg |= (op->addr.nbytes - 1);
>  	writel(reg, reg_base + CQSPI_REG_SIZE);
>  	return 0;
>  }
>  
> -static int cqspi_indirect_write_execute(struct spi_nor *nor, loff_t to_addr,
> -					const u8 *txbuf, const size_t n_tx)
> +static int cqspi_indirect_write_execute(struct cqspi_flash_pdata *f_pdata,
> +					loff_t to_addr, const u8 *txbuf,
> +					const size_t n_tx)
>  {
> -	const unsigned int page_size = nor->page_size;
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
> +	struct device *dev = &cqspi->pdev->dev;
>  	void __iomem *reg_base = cqspi->iobase;
>  	unsigned int remaining = n_tx;
>  	unsigned int write_bytes;
> @@ -646,7 +624,7 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor, loff_t to_addr,
>  	while (remaining > 0) {
>  		size_t write_words, mod_bytes;
>  
> -		write_bytes = remaining > page_size ? page_size : remaining;
> +		write_bytes = remaining;
>  		write_words = write_bytes / 4;
>  		mod_bytes = write_bytes % 4;
>  		/* Write 4 bytes at a time then single bytes. */
> @@ -663,8 +641,8 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor, loff_t to_addr,
>  		}
>  
>  		if (!wait_for_completion_timeout(&cqspi->transfer_complete,
> -					msecs_to_jiffies(CQSPI_TIMEOUT_MS))) {
> -			dev_err(nor->dev, "Indirect write timeout\n");
> +						 msecs_to_jiffies(CQSPI_TIMEOUT_MS))) {
> +			dev_err(dev, "Indirect write timeout\n");
>  			ret = -ETIMEDOUT;
>  			goto failwr;
>  		}
> @@ -679,8 +657,7 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor, loff_t to_addr,
>  	ret = cqspi_wait_for_bit(reg_base + CQSPI_REG_INDIRECTWR,
>  				 CQSPI_REG_INDIRECTWR_DONE_MASK, 0);
>  	if (ret) {
> -		dev_err(nor->dev,
> -			"Indirect write completion error (%i)\n", ret);
> +		dev_err(dev, "Indirect write completion error (%i)\n", ret);
>  		goto failwr;
>  	}
>  
> @@ -704,9 +681,8 @@ static int cqspi_indirect_write_execute(struct spi_nor *nor, loff_t to_addr,
>  	return ret;
>  }
>  
> -static void cqspi_chipselect(struct spi_nor *nor)
> +static void cqspi_chipselect(struct cqspi_flash_pdata *f_pdata)
>  {
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
>  	void __iomem *reg_base = cqspi->iobase;
>  	unsigned int chip_select = f_pdata->cs;
> @@ -745,9 +721,8 @@ static unsigned int calculate_ticks_for_ns(const unsigned int ref_clk_hz,
>  	return ticks;
>  }
>  
> -static void cqspi_delay(struct spi_nor *nor)
> +static void cqspi_delay(struct cqspi_flash_pdata *f_pdata)
>  {
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
>  	void __iomem *iobase = cqspi->iobase;
>  	const unsigned int ref_clk_hz = cqspi->master_ref_clk_hz;
> @@ -831,11 +806,10 @@ static void cqspi_controller_enable(struct cqspi_st *cqspi, bool enable)
>  	writel(reg, reg_base + CQSPI_REG_CONFIG);
>  }
>  
> -static void cqspi_configure(struct spi_nor *nor)
> +static void cqspi_configure(struct cqspi_flash_pdata *f_pdata,
> +			    unsigned long sclk)
>  {
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
> -	const unsigned int sclk = f_pdata->clk_rate;
>  	int switch_cs = (cqspi->current_cs != f_pdata->cs);
>  	int switch_ck = (cqspi->sclk != sclk);
>  
> @@ -845,14 +819,14 @@ static void cqspi_configure(struct spi_nor *nor)
>  	/* Switch chip select. */
>  	if (switch_cs) {
>  		cqspi->current_cs = f_pdata->cs;
> -		cqspi_chipselect(nor);
> +		cqspi_chipselect(f_pdata);
>  	}
>  
>  	/* Setup baudrate divisor and delays */
>  	if (switch_ck) {
>  		cqspi->sclk = sclk;
>  		cqspi_config_baudrate_div(cqspi);
> -		cqspi_delay(nor);
> +		cqspi_delay(f_pdata);
>  		cqspi_readdata_capture(cqspi, !cqspi->rclk_en,
>  				       f_pdata->read_delay);
>  	}
> @@ -861,26 +835,25 @@ static void cqspi_configure(struct spi_nor *nor)
>  		cqspi_controller_enable(cqspi, 1);
>  }
>  
> -static int cqspi_set_protocol(struct spi_nor *nor, const int read)
> +static int cqspi_set_protocol(struct cqspi_flash_pdata *f_pdata,
> +			      const struct spi_mem_op *op)
>  {
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
> -
>  	f_pdata->inst_width = CQSPI_INST_TYPE_SINGLE;
>  	f_pdata->addr_width = CQSPI_INST_TYPE_SINGLE;
>  	f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
>  
> -	if (read) {
> -		switch (nor->read_proto) {
> -		case SNOR_PROTO_1_1_1:
> +	if (op->data.dir == SPI_MEM_DATA_IN) {
> +		switch (op->data.buswidth) {
> +		case 1:
>  			f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
>  			break;
> -		case SNOR_PROTO_1_1_2:
> +		case 2:
>  			f_pdata->data_width = CQSPI_INST_TYPE_DUAL;
>  			break;
> -		case SNOR_PROTO_1_1_4:
> +		case 4:
>  			f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
>  			break;
> -		case SNOR_PROTO_1_1_8:
> +		case 8:
>  			f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
>  			break;
>  		default:
> @@ -888,36 +861,32 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
>  		}
>  	}
>  
> -	cqspi_configure(nor);
> -
>  	return 0;
>  }
>  
> -static ssize_t cqspi_write(struct spi_nor *nor, loff_t to,
> -			   size_t len, const u_char *buf)
> +static ssize_t cqspi_write(struct cqspi_flash_pdata *f_pdata,
> +			   const struct spi_mem_op *op)
>  {
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
> +	loff_t to = op->addr.val;
> +	size_t len = op->data.nbytes;
> +	const u_char *buf = op->data.buf.out;
>  	int ret;
>  
> -	ret = cqspi_set_protocol(nor, 0);
> +	ret = cqspi_set_protocol(f_pdata, op);
>  	if (ret)
>  		return ret;
>  
> -	ret = cqspi_write_setup(nor);
> +	ret = cqspi_write_setup(f_pdata, op);
>  	if (ret)
>  		return ret;
>  
> -	if (f_pdata->use_direct_mode) {
> +	if (cqspi->use_direct_mode && ((to + len) <= cqspi->ahb_size)) {
>  		memcpy_toio(cqspi->ahb_base + to, buf, len);
> -		ret = cqspi_wait_idle(cqspi);
> -	} else {
> -		ret = cqspi_indirect_write_execute(nor, to, buf, len);
> +		return cqspi_wait_idle(cqspi);
>  	}
> -	if (ret)
> -		return ret;
>  
> -	return len;
> +	return cqspi_indirect_write_execute(f_pdata, to, buf, len);
>  }
>  
>  static void cqspi_rx_dma_callback(void *param)
> @@ -927,11 +896,11 @@ static void cqspi_rx_dma_callback(void *param)
>  	complete(&cqspi->rx_dma_complete);
>  }
>  
> -static int cqspi_direct_read_execute(struct spi_nor *nor, u_char *buf,
> -				     loff_t from, size_t len)
> +static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
> +				     u_char *buf, loff_t from, size_t len)
>  {
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
>  	struct cqspi_st *cqspi = f_pdata->cqspi;
> +	struct device *dev = &cqspi->pdev->dev;
>  	enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
>  	dma_addr_t dma_src = (dma_addr_t)cqspi->mmap_phys_base + from;
>  	int ret = 0;
> @@ -944,15 +913,15 @@ static int cqspi_direct_read_execute(struct spi_nor *nor, u_char *buf,
>  		return 0;
>  	}
>  
> -	dma_dst = dma_map_single(nor->dev, buf, len, DMA_FROM_DEVICE);
> -	if (dma_mapping_error(nor->dev, dma_dst)) {
> -		dev_err(nor->dev, "dma mapping failed\n");
> +	dma_dst = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
> +	if (dma_mapping_error(dev, dma_dst)) {
> +		dev_err(dev, "dma mapping failed\n");
>  		return -ENOMEM;
>  	}
>  	tx = dmaengine_prep_dma_memcpy(cqspi->rx_chan, dma_dst, dma_src,
>  				       len, flags);
>  	if (!tx) {
> -		dev_err(nor->dev, "device_prep_dma_memcpy error\n");
> +		dev_err(dev, "device_prep_dma_memcpy error\n");
>  		ret = -EIO;
>  		goto err_unmap;
>  	}
> @@ -964,7 +933,7 @@ static int cqspi_direct_read_execute(struct spi_nor *nor, u_char *buf,
>  
>  	ret = dma_submit_error(cookie);
>  	if (ret) {
> -		dev_err(nor->dev, "dma_submit_error %d\n", cookie);
> +		dev_err(dev, "dma_submit_error %d\n", cookie);
>  		ret = -EIO;
>  		goto err_unmap;
>  	}
> @@ -973,94 +942,68 @@ static int cqspi_direct_read_execute(struct spi_nor *nor, u_char *buf,
>  	if (!wait_for_completion_timeout(&cqspi->rx_dma_complete,
>  					 msecs_to_jiffies(len))) {
>  		dmaengine_terminate_sync(cqspi->rx_chan);
> -		dev_err(nor->dev, "DMA wait_for_completion_timeout\n");
> +		dev_err(dev, "DMA wait_for_completion_timeout\n");
>  		ret = -ETIMEDOUT;
>  		goto err_unmap;
>  	}
>  
>  err_unmap:
> -	dma_unmap_single(nor->dev, dma_dst, len, DMA_FROM_DEVICE);
> +	dma_unmap_single(dev, dma_dst, len, DMA_FROM_DEVICE);
>  
>  	return ret;
>  }
>  
> -static ssize_t cqspi_read(struct spi_nor *nor, loff_t from,
> -			  size_t len, u_char *buf)
> +static ssize_t cqspi_read(struct cqspi_flash_pdata *f_pdata,
> +			  const struct spi_mem_op *op)
>  {
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
> +	struct cqspi_st *cqspi = f_pdata->cqspi;
> +	loff_t from = op->addr.val;
> +	size_t len = op->data.nbytes;
> +	u_char *buf = op->data.buf.in;
>  	int ret;
>  
> -	ret = cqspi_set_protocol(nor, 1);
> +	ret = cqspi_set_protocol(f_pdata, op);
>  	if (ret)
>  		return ret;
>  
> -	ret = cqspi_read_setup(nor);
> +	ret = cqspi_read_setup(f_pdata, op);
>  	if (ret)
>  		return ret;
>  
> -	if (f_pdata->use_direct_mode)
> -		ret = cqspi_direct_read_execute(nor, buf, from, len);
> -	else
> -		ret = cqspi_indirect_read_execute(nor, buf, from, len);
> -	if (ret)
> -		return ret;
> +	if (cqspi->use_direct_mode && ((from + len) <= cqspi->ahb_size))
> +		return cqspi_direct_read_execute(f_pdata, buf, from, len);
>  
> -	return len;
> +	return cqspi_indirect_read_execute(f_pdata, buf, from, len);
>  }
>  
> -static int cqspi_erase(struct spi_nor *nor, loff_t offs)
> +static int cqspi_mem_process(struct spi_mem *mem, const struct spi_mem_op *op)
>  {
> -	int ret;
> -
> -	ret = cqspi_set_protocol(nor, 0);
> -	if (ret)
> -		return ret;
> -
> -	/* Set up command buffer. */
> -	ret = cqspi_command_write_addr(nor, nor->erase_opcode, offs);
> -	if (ret)
> -		return ret;
> -
> -	return 0;
> -}
> -
> -static int cqspi_prep(struct spi_nor *nor)
> -{
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
> -	struct cqspi_st *cqspi = f_pdata->cqspi;
> -
> -	mutex_lock(&cqspi->bus_mutex);
> -
> -	return 0;
> -}
> +	struct cqspi_st *cqspi = spi_master_get_devdata(mem->spi->master);
> +	struct cqspi_flash_pdata *f_pdata;
>  
> -static void cqspi_unprep(struct spi_nor *nor)
> -{
> -	struct cqspi_flash_pdata *f_pdata = nor->priv;
> -	struct cqspi_st *cqspi = f_pdata->cqspi;
> +	f_pdata = &cqspi->f_pdata[mem->spi->chip_select];
> +	cqspi_configure(f_pdata, mem->spi->max_speed_hz);
>  
> -	mutex_unlock(&cqspi->bus_mutex);
> -}
> +	if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) {
> +		if (!op->addr.nbytes)
> +			return cqspi_command_read(f_pdata, op);
>  
> -static int cqspi_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, size_t len)
> -{
> -	int ret;
> +		return cqspi_read(f_pdata, op);
> +	}
>  
> -	ret = cqspi_set_protocol(nor, 0);
> -	if (!ret)
> -		ret = cqspi_command_read(nor, opcode, buf, len);
> +	if (!op->addr.nbytes || !op->data.buf.out)
> +		return cqspi_command_write(f_pdata, op);
>  
> -	return ret;
> +	return cqspi_write(f_pdata, op);
>  }
>  
> -static int cqspi_write_reg(struct spi_nor *nor, u8 opcode, const u8 *buf,
> -			   size_t len)
> +static int cqspi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  {
>  	int ret;
>  
> -	ret = cqspi_set_protocol(nor, 0);
> -	if (!ret)
> -		ret = cqspi_command_write(nor, opcode, buf, len);
> +	ret = cqspi_mem_process(mem, op);
> +	if (ret)
> +		dev_err(&mem->spi->dev, "operation failed with %d\n", ret);
>  
>  	return ret;
>  }
> @@ -1102,26 +1045,26 @@ static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
>  	return 0;
>  }
>  
> -static int cqspi_of_get_pdata(struct platform_device *pdev)
> +static int cqspi_of_get_pdata(struct cqspi_st *cqspi)
>  {
> -	struct device_node *np = pdev->dev.of_node;
> -	struct cqspi_st *cqspi = platform_get_drvdata(pdev);
> +	struct device *dev = &cqspi->pdev->dev;
> +	struct device_node *np = dev->of_node;
>  
>  	cqspi->is_decoded_cs = of_property_read_bool(np, "cdns,is-decoded-cs");
>  
>  	if (of_property_read_u32(np, "cdns,fifo-depth", &cqspi->fifo_depth)) {
> -		dev_err(&pdev->dev, "couldn't determine fifo-depth\n");
> +		dev_err(dev, "couldn't determine fifo-depth\n");
>  		return -ENXIO;
>  	}
>  
>  	if (of_property_read_u32(np, "cdns,fifo-width", &cqspi->fifo_width)) {
> -		dev_err(&pdev->dev, "couldn't determine fifo-width\n");
> +		dev_err(dev, "couldn't determine fifo-width\n");
>  		return -ENXIO;
>  	}
>  
>  	if (of_property_read_u32(np, "cdns,trigger-address",
>  				 &cqspi->trigger_address)) {
> -		dev_err(&pdev->dev, "couldn't determine trigger-address\n");
> +		dev_err(dev, "couldn't determine trigger-address\n");
>  		return -ENXIO;
>  	}
>  
> @@ -1185,47 +1128,30 @@ static int cqspi_request_mmap_dma(struct cqspi_st *cqspi)
>  	return 0;
>  }
>  
> -static const struct spi_nor_controller_ops cqspi_controller_ops = {
> -	.prepare = cqspi_prep,
> -	.unprepare = cqspi_unprep,
> -	.read_reg = cqspi_read_reg,
> -	.write_reg = cqspi_write_reg,
> -	.read = cqspi_read,
> -	.write = cqspi_write,
> -	.erase = cqspi_erase,
> +static const struct spi_controller_mem_ops cqspi_mem_ops = {
> +	.exec_op = cqspi_exec_mem_op,
>  };
>  
> -static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
> +static int cqspi_setup_flash(struct cqspi_st *cqspi)
>  {
>  	struct platform_device *pdev = cqspi->pdev;
>  	struct device *dev = &pdev->dev;
> -	const struct cqspi_driver_platdata *ddata;
> -	struct spi_nor_hwcaps hwcaps;
> +	struct device_node *np = dev->of_node;
>  	struct cqspi_flash_pdata *f_pdata;
> -	struct spi_nor *nor;
> -	struct mtd_info *mtd;
>  	unsigned int cs;
> -	int i, ret;
> -
> -	ddata = of_device_get_match_data(dev);
> -	if (!ddata) {
> -		dev_err(dev, "Couldn't find driver data\n");
> -		return -EINVAL;
> -	}
> -	hwcaps.mask = ddata->hwcaps_mask;
> +	int ret;
>  
>  	/* Get flash device data */
>  	for_each_available_child_of_node(dev->of_node, np) {
>  		ret = of_property_read_u32(np, "reg", &cs);
>  		if (ret) {
>  			dev_err(dev, "Couldn't determine chip select.\n");
> -			goto err;
> +			return ret;
>  		}
>  
>  		if (cs >= CQSPI_MAX_CHIPSELECT) {
> -			ret = -EINVAL;
>  			dev_err(dev, "Chip select %d out of range.\n", cs);
> -			goto err;
> +			return -EINVAL;
>  		}
>  
>  		f_pdata = &cqspi->f_pdata[cs];
> @@ -1234,90 +1160,51 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>  
>  		ret = cqspi_of_get_flash_pdata(pdev, f_pdata, np);
>  		if (ret)
> -			goto err;
> -
> -		nor = &f_pdata->nor;
> -		mtd = &nor->mtd;
> -
> -		mtd->priv = nor;
> -
> -		nor->dev = dev;
> -		spi_nor_set_flash_node(nor, np);
> -		nor->priv = f_pdata;
> -		nor->controller_ops = &cqspi_controller_ops;
> -
> -		mtd->name = devm_kasprintf(dev, GFP_KERNEL, "%s.%d",
> -					   dev_name(dev), cs);
> -		if (!mtd->name) {
> -			ret = -ENOMEM;
> -			goto err;
> -		}
> -
> -		ret = spi_nor_scan(nor, NULL, &hwcaps);
> -		if (ret)
> -			goto err;
> -
> -		ret = mtd_device_register(mtd, NULL, 0);
> -		if (ret)
> -			goto err;
> -
> -		f_pdata->registered = true;
> -
> -		if (mtd->size <= cqspi->ahb_size &&
> -		    !(ddata->quirks & CQSPI_DISABLE_DAC_MODE)) {
> -			f_pdata->use_direct_mode = true;
> -			dev_dbg(nor->dev, "using direct mode for %s\n",
> -				mtd->name);
> -
> -			if (!cqspi->rx_chan) {
> -				ret = cqspi_request_mmap_dma(cqspi);
> -				if (ret == -EPROBE_DEFER)
> -					goto err;
> -			}
> -		}
> +			return ret;
>  	}
>  
>  	return 0;
> -
> -err:
> -	for (i = 0; i < CQSPI_MAX_CHIPSELECT; i++)
> -		if (cqspi->f_pdata[i].registered)
> -			mtd_device_unregister(&cqspi->f_pdata[i].nor.mtd);
> -	return ret;
>  }
>  
>  static int cqspi_probe(struct platform_device *pdev)
>  {
> -	struct device_node *np = pdev->dev.of_node;
> +	const struct cqspi_driver_platdata *ddata;
> +	struct reset_control *rstc, *rstc_ocp;
>  	struct device *dev = &pdev->dev;
> +	struct spi_master *master;
> +	struct resource *res_ahb;
>  	struct cqspi_st *cqspi;
>  	struct resource *res;
> -	struct resource *res_ahb;
> -	struct reset_control *rstc, *rstc_ocp;
> -	const struct cqspi_driver_platdata *ddata;
>  	int ret;
>  	int irq;
>  
> -	cqspi = devm_kzalloc(dev, sizeof(*cqspi), GFP_KERNEL);
> -	if (!cqspi)
> +	master = spi_alloc_master(&pdev->dev, sizeof(*cqspi));
> +	if (!master) {
> +		dev_err(&pdev->dev, "spi_alloc_master failed\n");
>  		return -ENOMEM;
> +	}
> +	master->mode_bits = SPI_RX_QUAD | SPI_RX_DUAL;
> +	master->mem_ops = &cqspi_mem_ops;
> +	master->dev.of_node = pdev->dev.of_node;
> +
> +	cqspi = spi_master_get_devdata(master);
>  
> -	mutex_init(&cqspi->bus_mutex);
>  	cqspi->pdev = pdev;
> -	platform_set_drvdata(pdev, cqspi);
>  
>  	/* Obtain configuration from OF. */
> -	ret = cqspi_of_get_pdata(pdev);
> +	ret = cqspi_of_get_pdata(cqspi);
>  	if (ret) {
>  		dev_err(dev, "Cannot get mandatory OF data.\n");
> -		return -ENODEV;
> +		ret = -ENODEV;
> +		goto probe_master_put;
>  	}
>  
>  	/* Obtain QSPI clock. */
>  	cqspi->clk = devm_clk_get(dev, NULL);
>  	if (IS_ERR(cqspi->clk)) {
>  		dev_err(dev, "Cannot claim QSPI clock.\n");
> -		return PTR_ERR(cqspi->clk);
> +		ret = PTR_ERR(cqspi->clk);
> +		goto probe_master_put;
>  	}
>  
>  	/* Obtain and remap controller address. */
> @@ -1325,7 +1212,8 @@ static int cqspi_probe(struct platform_device *pdev)
>  	cqspi->iobase = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(cqspi->iobase)) {
>  		dev_err(dev, "Cannot remap controller address.\n");
> -		return PTR_ERR(cqspi->iobase);
> +		ret = PTR_ERR(cqspi->iobase);
> +		goto probe_master_put;
>  	}
>  
>  	/* Obtain and remap AHB address. */
> @@ -1333,7 +1221,8 @@ static int cqspi_probe(struct platform_device *pdev)
>  	cqspi->ahb_base = devm_ioremap_resource(dev, res_ahb);
>  	if (IS_ERR(cqspi->ahb_base)) {
>  		dev_err(dev, "Cannot remap AHB address.\n");
> -		return PTR_ERR(cqspi->ahb_base);
> +		ret = PTR_ERR(cqspi->ahb_base);
> +		goto probe_master_put;
>  	}
>  	cqspi->mmap_phys_base = (dma_addr_t)res_ahb->start;
>  	cqspi->ahb_size = resource_size(res_ahb);
> @@ -1342,14 +1231,16 @@ static int cqspi_probe(struct platform_device *pdev)
>  
>  	/* Obtain IRQ line. */
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0)
> -		return -ENXIO;
> +	if (irq < 0) {
> +		ret = -ENXIO;
> +		goto probe_master_put;
> +	}
>  
>  	pm_runtime_enable(dev);
>  	ret = pm_runtime_get_sync(dev);
>  	if (ret < 0) {
>  		pm_runtime_put_noidle(dev);
> -		return ret;
> +		goto probe_master_put;
>  	}
>  
>  	ret = clk_prepare_enable(cqspi->clk);
> @@ -1379,9 +1270,15 @@ static int cqspi_probe(struct platform_device *pdev)
>  
>  	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>  	ddata  = of_device_get_match_data(dev);
> -	if (ddata && (ddata->quirks & CQSPI_NEEDS_WR_DELAY))
> -		cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
> -						   cqspi->master_ref_clk_hz);
> +	if (ddata) {
> +		if (ddata->quirks & CQSPI_NEEDS_WR_DELAY)
> +			cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
> +						cqspi->master_ref_clk_hz);
> +		if (ddata->hwcaps_mask & CQSPI_SUPPORTS_OCTAL)
> +			master->mode_bits |= SPI_RX_OCTAL;
> +		if (!(ddata->quirks & CQSPI_DISABLE_DAC_MODE))
> +			cqspi->use_direct_mode = true;
> +	}
>  
>  	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>  			       pdev->name, cqspi);
> @@ -1395,13 +1292,25 @@ static int cqspi_probe(struct platform_device *pdev)
>  	cqspi->current_cs = -1;
>  	cqspi->sclk = 0;
>  
> -	ret = cqspi_setup_flash(cqspi, np);
> +	ret = cqspi_setup_flash(cqspi);
>  	if (ret) {
> -		dev_err(dev, "Cadence QSPI NOR probe failed %d\n", ret);
> +		dev_err(dev, "failed to setup flash parameters %d\n", ret);
>  		goto probe_setup_failed;
>  	}
>  
> -	return ret;
> +	if (cqspi->use_direct_mode) {
> +		ret = cqspi_request_mmap_dma(cqspi);
> +		if (ret == -EPROBE_DEFER)
> +			goto probe_setup_failed;
> +	}
> +
> +	ret = devm_spi_register_master(dev, master);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to register SPI ctlr %d\n", ret);
> +		goto probe_setup_failed;
> +	}
> +
> +	return 0;
>  probe_setup_failed:
>  	cqspi_controller_enable(cqspi, 0);
>  probe_reset_failed:
> @@ -1409,17 +1318,14 @@ static int cqspi_probe(struct platform_device *pdev)
>  probe_clk_failed:
>  	pm_runtime_put_sync(dev);
>  	pm_runtime_disable(dev);
> +probe_master_put:
> +	spi_master_put(master);
>  	return ret;
>  }
>  
>  static int cqspi_remove(struct platform_device *pdev)
>  {
>  	struct cqspi_st *cqspi = platform_get_drvdata(pdev);
> -	int i;
> -
> -	for (i = 0; i < CQSPI_MAX_CHIPSELECT; i++)
> -		if (cqspi->f_pdata[i].registered)
> -			mtd_device_unregister(&cqspi->f_pdata[i].nor.mtd);
>  
>  	cqspi_controller_enable(cqspi, 0);
>  
> @@ -1462,17 +1368,15 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>  #endif
>  
>  static const struct cqspi_driver_platdata cdns_qspi = {
> -	.hwcaps_mask = CQSPI_BASE_HWCAPS_MASK,
>  	.quirks = CQSPI_DISABLE_DAC_MODE,
>  };
>  
>  static const struct cqspi_driver_platdata k2g_qspi = {
> -	.hwcaps_mask = CQSPI_BASE_HWCAPS_MASK,
>  	.quirks = CQSPI_NEEDS_WR_DELAY,
>  };
>  
>  static const struct cqspi_driver_platdata am654_ospi = {
> -	.hwcaps_mask = CQSPI_BASE_HWCAPS_MASK | SNOR_HWCAPS_READ_1_1_8,
> +	.hwcaps_mask = CQSPI_SUPPORTS_OCTAL,
>  	.quirks = CQSPI_NEEDS_WR_DELAY,
>  };
>  
> @@ -1511,3 +1415,5 @@ MODULE_LICENSE("GPL v2");
>  MODULE_ALIAS("platform:" CQSPI_NAME);
>  MODULE_AUTHOR("Ley Foon Tan <lftan@altera.com>");
>  MODULE_AUTHOR("Graham Moore <grmoore@opensource.altera.com>");
> +MODULE_AUTHOR("Vadivel Murugan R <vadivel.muruganx.ramuthevar@intel.com>");
> +MODULE_AUTHOR("Vignesh Raghavendra <vigneshr@ti.com>");
> 

On the AM65x, this changes mtd->name (thus mtd-id for
parser/cmdlinepart) from 47040000.spi.0 to spi7.0. The besides having to
deal with both names now, "spi7" sounds like it could easily change
again if someone plugs or unplugs some other SPI device. Is this intended?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 7/8] mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework
  2020-08-24  5:55   ` Jan Kiszka
@ 2020-08-24 11:44     ` Vignesh Raghavendra
  2020-08-24 12:04       ` Boris Brezillon
  2020-08-24 12:06       ` Jan Kiszka
  0 siblings, 2 replies; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-08-24 11:44 UTC (permalink / raw)
  To: Jan Kiszka, Tudor Ambarus, Mark Brown
  Cc: Boris Brezillon, Ramuthevar Vadivel Murugan, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex

Hi Jan,

On 8/24/20 11:25 AM, Jan Kiszka wrote:
[...]

>> +MODULE_AUTHOR("Vignesh Raghavendra <vigneshr@ti.com>");
>>
> On the AM65x, this changes mtd->name (thus mtd-id for
> parser/cmdlinepart) from 47040000.spi.0 to spi7.0. The besides having to
> deal with both names now, "spi7" sounds like it could easily change
> again if someone plugs or unplugs some other SPI device. Is this intended?
> 

You could use DT aliases to make sure OSPI0 is always at given bus num
(say spi7):

        aliases {
                spi7 = &ospi0;
        };

Regards
Vignesh

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
  2020-08-22 18:05   ` Jan Kiszka
@ 2020-08-24 11:45     ` Vignesh Raghavendra
  2020-08-24 12:49       ` Jan Kiszka
  0 siblings, 1 reply; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-08-24 11:45 UTC (permalink / raw)
  To: Jan Kiszka, Tudor Ambarus, Mark Brown
  Cc: Boris Brezillon, Ramuthevar Vadivel Murugan, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex



On 8/22/20 11:35 PM, Jan Kiszka wrote:
> On 01.06.20 09:04, Vignesh Raghavendra wrote:
>> dma_request_chan_by_mask() can throw EPROBE_DEFER if DMA provider
>> is not yet probed. Currently driver just falls back to using PIO mode
>> (which is less efficient) in this case. Instead return probe deferral
>> error as is so that driver will be re probed once DMA provider is
>> available.
>>
>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
[...]
>>
>>  static const struct spi_nor_controller_ops cqspi_controller_ops = {
>> @@ -1269,8 +1274,11 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>>  			dev_dbg(nor->dev, "using direct mode for %s\n",
>>  				mtd->name);
>>
>> -			if (!cqspi->rx_chan)
>> -				cqspi_request_mmap_dma(cqspi);
>> +			if (!cqspi->rx_chan) {
>> +				ret = cqspi_request_mmap_dma(cqspi);
>> +				if (ret == -EPROBE_DEFER)
>> +					goto err;
>> +			}
>>  		}
>>  	}
>>
>>
> 
> This seem to break reading the SPI flash on our IOT2050 [1] (didn't test
> the eval board yet).
> 
> Without that commit, read happens via PIO, and that works. With the
> commit, the pattern
> 
> with open("out.bin", "wb") as out:
>     pos = 0
>     while pos < 2:
>         with open("/dev/mtd0", "rb") as mtd:
>            mtd.seek(pos * 0x10000)
>            out.write(mtd.read(0x10000))
>         pos += 1
> 
> gives the wrong result for the second block while

Interesting... Could you please explain wrong result? Is the data move
around or completely garbage?

Does this fail even on AM654 EVM? Could you share full script for me to
test locally?

What is the flash on the board?

> 
> with open("out2.bin", "wb") as out:
>     with open("/dev/mtd0", "rb") as mtd:
>         out.write(mtd.read(0x20000))
> 
> (or "mtd_debug read") is fine.
> 
> What could be the reason? Our DTBs and k3-am654-base-board.dtb had some
> deviations /wrt the ospi node, but aligning ours to the base board made
> no difference.
> 
> Jan
> 
> [1] https://github.com/siemens/linux/commits/jan/iot2050
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 7/8] mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework
  2020-08-24 11:44     ` Vignesh Raghavendra
@ 2020-08-24 12:04       ` Boris Brezillon
  2020-08-24 13:52         ` Jan Kiszka
  2020-08-24 12:06       ` Jan Kiszka
  1 sibling, 1 reply; 28+ messages in thread
From: Boris Brezillon @ 2020-08-24 12:04 UTC (permalink / raw)
  To: Vignesh Raghavendra
  Cc: Jan Kiszka, Tudor Ambarus, Mark Brown, Boris Brezillon,
	Ramuthevar Vadivel Murugan, linux-mtd, linux-kernel, linux-spi,
	simon.k.r.goldschmidt, dinguyen, marex

On Mon, 24 Aug 2020 17:14:56 +0530
Vignesh Raghavendra <vigneshr@ti.com> wrote:

> Hi Jan,
> 
> On 8/24/20 11:25 AM, Jan Kiszka wrote:
> [...]
> 
> >> +MODULE_AUTHOR("Vignesh Raghavendra <vigneshr@ti.com>");
> >>  
> > On the AM65x, this changes mtd->name (thus mtd-id for
> > parser/cmdlinepart) from 47040000.spi.0 to spi7.0. The besides having to
> > deal with both names now, "spi7" sounds like it could easily change
> > again if someone plugs or unplugs some other SPI device. Is this intended?
> >   
> 
> You could use DT aliases to make sure OSPI0 is always at given bus num
> (say spi7):
> 
>         aliases {
>                 spi7 = &ospi0;
>         };

FWIW, we've added the ->get_name() method [1][2] to avoid such
regressions.

[1]https://elixir.bootlin.com/linux/latest/source/include/linux/spi/spi-mem.h#L218
[2]https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi-fsl-qspi.c#L810

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 7/8] mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework
  2020-08-24 11:44     ` Vignesh Raghavendra
  2020-08-24 12:04       ` Boris Brezillon
@ 2020-08-24 12:06       ` Jan Kiszka
  1 sibling, 0 replies; 28+ messages in thread
From: Jan Kiszka @ 2020-08-24 12:06 UTC (permalink / raw)
  To: Vignesh Raghavendra, Tudor Ambarus, Mark Brown
  Cc: Boris Brezillon, Ramuthevar Vadivel Murugan, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex

On 24.08.20 13:44, Vignesh Raghavendra wrote:
> Hi Jan,
> 
> On 8/24/20 11:25 AM, Jan Kiszka wrote:
> [...]
> 
>>> +MODULE_AUTHOR("Vignesh Raghavendra <vigneshr@ti.com>");
>>>
>> On the AM65x, this changes mtd->name (thus mtd-id for
>> parser/cmdlinepart) from 47040000.spi.0 to spi7.0. The besides having to
>> deal with both names now, "spi7" sounds like it could easily change
>> again if someone plugs or unplugs some other SPI device. Is this intended?
>>
> 
> You could use DT aliases to make sure OSPI0 is always at given bus num
> (say spi7):
> 
>         aliases {
>                 spi7 = &ospi0;
>         };

Ah, looks like this is a common pattern... Thanks, will try that.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
  2020-08-24 11:45     ` Vignesh Raghavendra
@ 2020-08-24 12:49       ` Jan Kiszka
  2020-08-24 17:20         ` Jan Kiszka
  2020-08-25  3:12         ` Jin, Le
  0 siblings, 2 replies; 28+ messages in thread
From: Jan Kiszka @ 2020-08-24 12:49 UTC (permalink / raw)
  To: Vignesh Raghavendra, Tudor Ambarus, Mark Brown, Jin,
	Le (RC-CN DF FA R&D)
  Cc: Boris Brezillon, Ramuthevar Vadivel Murugan, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex

On 24.08.20 13:45, Vignesh Raghavendra wrote:
> 
> 
> On 8/22/20 11:35 PM, Jan Kiszka wrote:
>> On 01.06.20 09:04, Vignesh Raghavendra wrote:
>>> dma_request_chan_by_mask() can throw EPROBE_DEFER if DMA provider
>>> is not yet probed. Currently driver just falls back to using PIO mode
>>> (which is less efficient) in this case. Instead return probe deferral
>>> error as is so that driver will be re probed once DMA provider is
>>> available.
>>>
>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>> ---
> [...]
>>>
>>>  static const struct spi_nor_controller_ops cqspi_controller_ops = {
>>> @@ -1269,8 +1274,11 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>>>  			dev_dbg(nor->dev, "using direct mode for %s\n",
>>>  				mtd->name);
>>>
>>> -			if (!cqspi->rx_chan)
>>> -				cqspi_request_mmap_dma(cqspi);
>>> +			if (!cqspi->rx_chan) {
>>> +				ret = cqspi_request_mmap_dma(cqspi);
>>> +				if (ret == -EPROBE_DEFER)
>>> +					goto err;
>>> +			}
>>>  		}
>>>  	}
>>>
>>>
>>
>> This seem to break reading the SPI flash on our IOT2050 [1] (didn't test
>> the eval board yet).
>>
>> Without that commit, read happens via PIO, and that works. With the
>> commit, the pattern
>>
>> with open("out.bin", "wb") as out:
>>     pos = 0
>>     while pos < 2:
>>         with open("/dev/mtd0", "rb") as mtd:
>>            mtd.seek(pos * 0x10000)
>>            out.write(mtd.read(0x10000))
>>         pos += 1
>>
>> gives the wrong result for the second block while
> 
> Interesting... Could you please explain wrong result? Is the data move
> around or completely garbage?

It looks like some stripes contain data from other parts of the flash or
kernel RAM. It's not just garbage, there are readable strings included.

> 
> Does this fail even on AM654 EVM? Could you share full script for me to
> test locally?

The scripts are complete (python). Just binary-diff the outputs.

I'll try on the EVM later.

> 
> What is the flash on the board?

Le, could you answer that more precisely than I could?

Thanks,
Jan

> 
>>
>> with open("out2.bin", "wb") as out:
>>     with open("/dev/mtd0", "rb") as mtd:
>>         out.write(mtd.read(0x20000))
>>
>> (or "mtd_debug read") is fine.
>>
>> What could be the reason? Our DTBs and k3-am654-base-board.dtb had some
>> deviations /wrt the ospi node, but aligning ours to the base board made
>> no difference.
>>
>> Jan
>>
>> [1] https://github.com/siemens/linux/commits/jan/iot2050
>>

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 7/8] mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework
  2020-08-24 12:04       ` Boris Brezillon
@ 2020-08-24 13:52         ` Jan Kiszka
  0 siblings, 0 replies; 28+ messages in thread
From: Jan Kiszka @ 2020-08-24 13:52 UTC (permalink / raw)
  To: Boris Brezillon, Vignesh Raghavendra
  Cc: Tudor Ambarus, Mark Brown, Boris Brezillon,
	Ramuthevar Vadivel Murugan, linux-mtd, linux-kernel, linux-spi,
	simon.k.r.goldschmidt, dinguyen, marex

On 24.08.20 14:04, Boris Brezillon wrote:
> On Mon, 24 Aug 2020 17:14:56 +0530
> Vignesh Raghavendra <vigneshr@ti.com> wrote:
> 
>> Hi Jan,
>>
>> On 8/24/20 11:25 AM, Jan Kiszka wrote:
>> [...]
>>
>>>> +MODULE_AUTHOR("Vignesh Raghavendra <vigneshr@ti.com>");
>>>>  
>>> On the AM65x, this changes mtd->name (thus mtd-id for
>>> parser/cmdlinepart) from 47040000.spi.0 to spi7.0. The besides having to
>>> deal with both names now, "spi7" sounds like it could easily change
>>> again if someone plugs or unplugs some other SPI device. Is this intended?
>>>   
>>
>> You could use DT aliases to make sure OSPI0 is always at given bus num
>> (say spi7):
>>
>>         aliases {
>>                 spi7 = &ospi0;
>>         };
> 
> FWIW, we've added the ->get_name() method [1][2] to avoid such
> regressions.
> 
> [1]https://elixir.bootlin.com/linux/latest/source/include/linux/spi/spi-mem.h#L218
> [2]https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi-fsl-qspi.c#L810
> 

OK, that reads like I was not the first to run into this.

Vignesh, please use it so that I can drop the local workaround.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
  2020-08-24 12:49       ` Jan Kiszka
@ 2020-08-24 17:20         ` Jan Kiszka
  2020-08-26 10:12           ` Jan Kiszka
  2020-08-25  3:12         ` Jin, Le
  1 sibling, 1 reply; 28+ messages in thread
From: Jan Kiszka @ 2020-08-24 17:20 UTC (permalink / raw)
  To: Vignesh Raghavendra, Tudor Ambarus, Mark Brown, Jin,
	Le (RC-CN DF FA R&D)
  Cc: Boris Brezillon, Ramuthevar Vadivel Murugan, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex

On 24.08.20 14:49, Jan Kiszka wrote:
> On 24.08.20 13:45, Vignesh Raghavendra wrote:
>>
>>
>> On 8/22/20 11:35 PM, Jan Kiszka wrote:
>>> On 01.06.20 09:04, Vignesh Raghavendra wrote:
>>>> dma_request_chan_by_mask() can throw EPROBE_DEFER if DMA provider
>>>> is not yet probed. Currently driver just falls back to using PIO mode
>>>> (which is less efficient) in this case. Instead return probe deferral
>>>> error as is so that driver will be re probed once DMA provider is
>>>> available.
>>>>
>>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>>>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>> ---
>> [...]
>>>>
>>>>  static const struct spi_nor_controller_ops cqspi_controller_ops = {
>>>> @@ -1269,8 +1274,11 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>>>>  			dev_dbg(nor->dev, "using direct mode for %s\n",
>>>>  				mtd->name);
>>>>
>>>> -			if (!cqspi->rx_chan)
>>>> -				cqspi_request_mmap_dma(cqspi);
>>>> +			if (!cqspi->rx_chan) {
>>>> +				ret = cqspi_request_mmap_dma(cqspi);
>>>> +				if (ret == -EPROBE_DEFER)
>>>> +					goto err;
>>>> +			}
>>>>  		}
>>>>  	}
>>>>
>>>>
>>>
>>> This seem to break reading the SPI flash on our IOT2050 [1] (didn't test
>>> the eval board yet).
>>>
>>> Without that commit, read happens via PIO, and that works. With the
>>> commit, the pattern
>>>
>>> with open("out.bin", "wb") as out:
>>>     pos = 0
>>>     while pos < 2:
>>>         with open("/dev/mtd0", "rb") as mtd:
>>>            mtd.seek(pos * 0x10000)
>>>            out.write(mtd.read(0x10000))
>>>         pos += 1
>>>
>>> gives the wrong result for the second block while
>>
>> Interesting... Could you please explain wrong result? Is the data move
>> around or completely garbage?
> 
> It looks like some stripes contain data from other parts of the flash or
> kernel RAM. It's not just garbage, there are readable strings included.
> 
>>
>> Does this fail even on AM654 EVM? Could you share full script for me to
>> test locally?
> 
> The scripts are complete (python). Just binary-diff the outputs.
> 
> I'll try on the EVM later.

Done so now, could reproduce.

But the OSPIs are definitely different. EVM reports

spi-nor spi0.0: mt35xu512aba (65536 Kbytes)

with 4K erase size. Our our board, we have

spi-nor spi7.0: w25q128 (16384 Kbytes)

with 64K erase size.

Here is some extract of the hex-diffs between out.bin and out2.bin (the 
latter being the good one):

--- /dev/fd/63  2020-08-24 17:16:58.776409282 +0000
+++ /dev/fd/62  2020-08-24 17:16:58.776409282 +0000
@@ -6,18 +6,18 @@
 00000050  0f 30 0d 06 03 55 04 07  0c 06 44 61 6c 6c 61 73  |.0...U....Dallas|
 00000060  31 27 30 25 06 03 55 04  0a 0c 1e 54 65 78 61 73  |1'0%..U....Texas|
 00000070  20 49 6e 73 74 72 75 6d  65 6e 74 73 20 49 6e 63  | Instruments Inc|
-00000080  84 8b 96 2c 0c 12 18 03  01 05 05 04 01 02 00 00  |...,............|
-00000090  07 06 44 45 20 01 0d 14  2a 01 00 32 05 24 30 48  |..DE ...*..2.$0H|
-000000a0  60 6c 30 14 01 00 00 0f  ac 04 01 00 00 0f ac 04  |`l0.............|
-000000b0  01 00 00 0f ac 02 0c 00  2d 1a 6f 18 17 ff ff ff  |........-.o.....|
+00000080  6f 72 70 6f 72 61 74 65  64 31 13 30 11 06 03 55  |orporated1.0...U|
+00000090  04 0b 0c 0a 50 72 6f 63  65 73 73 6f 72 73 31 13  |....Processors1.|
+000000a0  30 11 06 03 55 04 03 0c  0a 54 49 20 73 75 70 70  |0...U....TI supp|
+000000b0  6f 72 74 31 1d 30 1b 06  09 2a 86 48 86 f7 0d 01  |ort1.0...*.H....|
 000000c0  09 01 16 0e 73 75 70 70  6f 72 74 40 74 69 2e 63  |....support@ti.c|
 000000d0  6f 6d 30 1e 17 0d 32 30  30 37 32 32 31 31 30 30  |om0...2007221100|
 000000e0  30 30 5a 17 0d 32 30 30  38 32 31 31 31 30 30 30  |00Z..20082111000|
 000000f0  30 5a 30 81 9d 31 0b 30  09 06 03 55 04 06 13 02  |0Z0..1.0...U....|
-00000100  00 00 27 a4 00 00 42 43  5e 00 62 32 2f 00 b4 96  |..'...BC^.b2/...|
-00000110  24 44 0c 00 c6 00 43 0a  00 00 0b f0 43 a5 2a 01  |$D....C.....C.*.|
-00000120  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
-*
+00000100  55 53 31 0b 30 09 06 03  55 04 08 0c 02 54 58 31  |US1.0...U....TX1|
+00000110  0f 30 0d 06 03 55 04 07  0c 06 44 61 6c 6c 61 73  |.0...U....Dallas|
+00000120  31 27 30 25 06 03 55 04  0a 0c 1e 54 65 78 61 73  |1'0%..U....Texas|
+00000130  20 49 6e 73 74 72 75 6d  65 6e 74 73 20 49 6e 63  | Instruments Inc|
 00000140  6f 72 70 6f 72 61 74 65  64 31 13 30 11 06 03 55  |orporated1.0...U|
 00000150  04 0b 0c 0a 50 72 6f 63  65 73 73 6f 72 73 31 13  |....Processors1.|
 00000160  30 11 06 03 55 04 03 0c  0a 54 49 20 73 75 70 70  |0...U....TI supp|

[...]

 000017a0  02 8a e5 06 c8 8c e2 14  c2 8a e5 7c 01 00 ea ed  |...........|....|
 000017b0  1f 8f e2 66 02 00 ea 5b  45 72 72 6f 72 5d 20 52  |...f...[Error] R|
-000017c0  69 64 20 55 54 43 20 49  44 21 21 21 0a 00 00 5b  |id UTC ID!!!...[|
-000017d0  45 72 72 6f 72 5d 20 49  6e 76 61 6c 69 64 20 50  |Error] Invalid P|
-000017e0  65 65 72 20 43 68 61 6e  6e 65 6c 20 4e 75 6d 62  |eer Channel Numb|
-000017f0  65 72 21 21 21 0a 00 41  73 73 65 72 74 69 6f 6e  |er!!!..Assertion|
+000017c0  4d 20 41 6c 6c 6f 63 20  54 58 20 43 68 20 66 61  |M Alloc TX Ch fa|
+000017d0  69 6c 65 64 21 21 21 0a  00 00 00 73 72 63 2f 75  |iled!!!....src/u|
+000017e0  64 6d 61 5f 63 68 2e 63  00 00 00 75 74 63 49 6e  |dma_ch.c...utcIn|
+000017f0  66 6f 20 21 3d 20 4e 55  4c 4c 5f 50 54 52 00 75  |fo != NULL_PTR.u|
 00001800  74 63 49 64 20 3c 3d 20  55 44 4d 41 5f 4e 55 4d  |tcId <= UDMA_NUM|
 00001810  5f 55 54 43 5f 49 4e 53  54 41 4e 43 45 00 00 72  |_UTC_INSTANCE..r|

Jan

> 
>>
>> What is the flash on the board?
> 
> Le, could you answer that more precisely than I could?
> 
> Thanks,
> Jan
> 
>>
>>>
>>> with open("out2.bin", "wb") as out:
>>>     with open("/dev/mtd0", "rb") as mtd:
>>>         out.write(mtd.read(0x20000))
>>>
>>> (or "mtd_debug read") is fine.
>>>
>>> What could be the reason? Our DTBs and k3-am654-base-board.dtb had some
>>> deviations /wrt the ospi node, but aligning ours to the base board made
>>> no difference.
>>>
>>> Jan
>>>
>>> [1] https://github.com/siemens/linux/commits/jan/iot2050
>>>
> 

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 28+ messages in thread

* RE: [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
  2020-08-24 12:49       ` Jan Kiszka
  2020-08-24 17:20         ` Jan Kiszka
@ 2020-08-25  3:12         ` Jin, Le
  1 sibling, 0 replies; 28+ messages in thread
From: Jin, Le @ 2020-08-25  3:12 UTC (permalink / raw)
  To: Kiszka, Jan, Vignesh Raghavendra, Tudor Ambarus, Mark Brown
  Cc: Boris Brezillon, Ramuthevar Vadivel Murugan, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex

Hello Jan,

The flash on our board is Winbond W25Q128JV.

Best Regards,
Le Jin

-----Original Message-----
From: Kiszka, Jan (CT RDA IOT SES-DE) <jan.kiszka@siemens.com> 
Sent: Monday, August 24, 2020 8:49 PM
To: Vignesh Raghavendra <vigneshr@ti.com>; Tudor Ambarus <tudor.ambarus@microchip.com>; Mark Brown <broonie@kernel.org>; Jin, Le (RC-CN DI FA R&D SW) <le.jin@siemens.com>
Cc: Boris Brezillon <bbrezillon@kernel.org>; Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>; linux-mtd@lists.infradead.org; linux-kernel@vger.kernel.org; linux-spi@vger.kernel.org; simon.k.r.goldschmidt@gmail.com; dinguyen@kernel.org; marex@denx.de
Subject: Re: [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel

On 24.08.20 13:45, Vignesh Raghavendra wrote:
> 
> 
> On 8/22/20 11:35 PM, Jan Kiszka wrote:
>> On 01.06.20 09:04, Vignesh Raghavendra wrote:
>>> dma_request_chan_by_mask() can throw EPROBE_DEFER if DMA provider is 
>>> not yet probed. Currently driver just falls back to using PIO mode 
>>> (which is less efficient) in this case. Instead return probe 
>>> deferral error as is so that driver will be re probed once DMA 
>>> provider is available.
>>>
>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>> ---
> [...]
>>>
>>>  static const struct spi_nor_controller_ops cqspi_controller_ops = { 
>>> @@ -1269,8 +1274,11 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>>>  			dev_dbg(nor->dev, "using direct mode for %s\n",
>>>  				mtd->name);
>>>
>>> -			if (!cqspi->rx_chan)
>>> -				cqspi_request_mmap_dma(cqspi);
>>> +			if (!cqspi->rx_chan) {
>>> +				ret = cqspi_request_mmap_dma(cqspi);
>>> +				if (ret == -EPROBE_DEFER)
>>> +					goto err;
>>> +			}
>>>  		}
>>>  	}
>>>
>>>
>>
>> This seem to break reading the SPI flash on our IOT2050 [1] (didn't 
>> test the eval board yet).
>>
>> Without that commit, read happens via PIO, and that works. With the 
>> commit, the pattern
>>
>> with open("out.bin", "wb") as out:
>>     pos = 0
>>     while pos < 2:
>>         with open("/dev/mtd0", "rb") as mtd:
>>            mtd.seek(pos * 0x10000)
>>            out.write(mtd.read(0x10000))
>>         pos += 1
>>
>> gives the wrong result for the second block while
> 
> Interesting... Could you please explain wrong result? Is the data move 
> around or completely garbage?

It looks like some stripes contain data from other parts of the flash or kernel RAM. It's not just garbage, there are readable strings included.

> 
> Does this fail even on AM654 EVM? Could you share full script for me 
> to test locally?

The scripts are complete (python). Just binary-diff the outputs.

I'll try on the EVM later.

> 
> What is the flash on the board?

Le, could you answer that more precisely than I could?

Thanks,
Jan

> 
>>
>> with open("out2.bin", "wb") as out:
>>     with open("/dev/mtd0", "rb") as mtd:
>>         out.write(mtd.read(0x20000))
>>
>> (or "mtd_debug read") is fine.
>>
>> What could be the reason? Our DTBs and k3-am654-base-board.dtb had 
>> some deviations /wrt the ospi node, but aligning ours to the base 
>> board made no difference.
>>
>> Jan
>>
>> [1] https://github.com/siemens/linux/commits/jan/iot2050
>>

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
  2020-08-24 17:20         ` Jan Kiszka
@ 2020-08-26 10:12           ` Jan Kiszka
  2020-08-26 12:18             ` Vignesh Raghavendra
  0 siblings, 1 reply; 28+ messages in thread
From: Jan Kiszka @ 2020-08-26 10:12 UTC (permalink / raw)
  To: Vignesh Raghavendra, Tudor Ambarus, Mark Brown, Jin,
	Le (RC-CN DF FA R&D)
  Cc: Boris Brezillon, Ramuthevar Vadivel Murugan, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex

On 24.08.20 19:20, Jan Kiszka wrote:
> On 24.08.20 14:49, Jan Kiszka wrote:
>> On 24.08.20 13:45, Vignesh Raghavendra wrote:
>>>
>>>
>>> On 8/22/20 11:35 PM, Jan Kiszka wrote:
>>>> On 01.06.20 09:04, Vignesh Raghavendra wrote:
>>>>> dma_request_chan_by_mask() can throw EPROBE_DEFER if DMA provider
>>>>> is not yet probed. Currently driver just falls back to using PIO mode
>>>>> (which is less efficient) in this case. Instead return probe deferral
>>>>> error as is so that driver will be re probed once DMA provider is
>>>>> available.
>>>>>
>>>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>>>>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>>> ---
>>> [...]
>>>>>
>>>>>  static const struct spi_nor_controller_ops cqspi_controller_ops = {
>>>>> @@ -1269,8 +1274,11 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>>>>>  			dev_dbg(nor->dev, "using direct mode for %s\n",
>>>>>  				mtd->name);
>>>>>
>>>>> -			if (!cqspi->rx_chan)
>>>>> -				cqspi_request_mmap_dma(cqspi);
>>>>> +			if (!cqspi->rx_chan) {
>>>>> +				ret = cqspi_request_mmap_dma(cqspi);
>>>>> +				if (ret == -EPROBE_DEFER)
>>>>> +					goto err;
>>>>> +			}
>>>>>  		}
>>>>>  	}
>>>>>
>>>>>
>>>>
>>>> This seem to break reading the SPI flash on our IOT2050 [1] (didn't test
>>>> the eval board yet).
>>>>
>>>> Without that commit, read happens via PIO, and that works. With the
>>>> commit, the pattern
>>>>
>>>> with open("out.bin", "wb") as out:
>>>>     pos = 0
>>>>     while pos < 2:
>>>>         with open("/dev/mtd0", "rb") as mtd:
>>>>            mtd.seek(pos * 0x10000)
>>>>            out.write(mtd.read(0x10000))
>>>>         pos += 1
>>>>
>>>> gives the wrong result for the second block while
>>>
>>> Interesting... Could you please explain wrong result? Is the data move
>>> around or completely garbage?
>>
>> It looks like some stripes contain data from other parts of the flash or
>> kernel RAM. It's not just garbage, there are readable strings included.
>>
>>>
>>> Does this fail even on AM654 EVM? Could you share full script for me to
>>> test locally?
>>
>> The scripts are complete (python). Just binary-diff the outputs.
>>
>> I'll try on the EVM later.
> 
> Done so now, could reproduce.

..."could *not* reproduce" there. Sorry if that caused confusion.

> 
> But the OSPIs are definitely different. EVM reports
> 
> spi-nor spi0.0: mt35xu512aba (65536 Kbytes)
> 
> with 4K erase size. Our our board, we have
> 
> spi-nor spi7.0: w25q128 (16384 Kbytes)
> 
> with 64K erase size.
> 
> Here is some extract of the hex-diffs between out.bin and out2.bin (the 
> latter being the good one):
> 
> --- /dev/fd/63  2020-08-24 17:16:58.776409282 +0000
> +++ /dev/fd/62  2020-08-24 17:16:58.776409282 +0000
> @@ -6,18 +6,18 @@
>  00000050  0f 30 0d 06 03 55 04 07  0c 06 44 61 6c 6c 61 73  |.0...U....Dallas|
>  00000060  31 27 30 25 06 03 55 04  0a 0c 1e 54 65 78 61 73  |1'0%..U....Texas|
>  00000070  20 49 6e 73 74 72 75 6d  65 6e 74 73 20 49 6e 63  | Instruments Inc|
> -00000080  84 8b 96 2c 0c 12 18 03  01 05 05 04 01 02 00 00  |...,............|
> -00000090  07 06 44 45 20 01 0d 14  2a 01 00 32 05 24 30 48  |..DE ...*..2.$0H|
> -000000a0  60 6c 30 14 01 00 00 0f  ac 04 01 00 00 0f ac 04  |`l0.............|
> -000000b0  01 00 00 0f ac 02 0c 00  2d 1a 6f 18 17 ff ff ff  |........-.o.....|
> +00000080  6f 72 70 6f 72 61 74 65  64 31 13 30 11 06 03 55  |orporated1.0...U|
> +00000090  04 0b 0c 0a 50 72 6f 63  65 73 73 6f 72 73 31 13  |....Processors1.|
> +000000a0  30 11 06 03 55 04 03 0c  0a 54 49 20 73 75 70 70  |0...U....TI supp|
> +000000b0  6f 72 74 31 1d 30 1b 06  09 2a 86 48 86 f7 0d 01  |ort1.0...*.H....|
>  000000c0  09 01 16 0e 73 75 70 70  6f 72 74 40 74 69 2e 63  |....support@ti.c|
>  000000d0  6f 6d 30 1e 17 0d 32 30  30 37 32 32 31 31 30 30  |om0...2007221100|
>  000000e0  30 30 5a 17 0d 32 30 30  38 32 31 31 31 30 30 30  |00Z..20082111000|
>  000000f0  30 5a 30 81 9d 31 0b 30  09 06 03 55 04 06 13 02  |0Z0..1.0...U....|
> -00000100  00 00 27 a4 00 00 42 43  5e 00 62 32 2f 00 b4 96  |..'...BC^.b2/...|
> -00000110  24 44 0c 00 c6 00 43 0a  00 00 0b f0 43 a5 2a 01  |$D....C.....C.*.|
> -00000120  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> -*
> +00000100  55 53 31 0b 30 09 06 03  55 04 08 0c 02 54 58 31  |US1.0...U....TX1|
> +00000110  0f 30 0d 06 03 55 04 07  0c 06 44 61 6c 6c 61 73  |.0...U....Dallas|
> +00000120  31 27 30 25 06 03 55 04  0a 0c 1e 54 65 78 61 73  |1'0%..U....Texas|
> +00000130  20 49 6e 73 74 72 75 6d  65 6e 74 73 20 49 6e 63  | Instruments Inc|
>  00000140  6f 72 70 6f 72 61 74 65  64 31 13 30 11 06 03 55  |orporated1.0...U|
>  00000150  04 0b 0c 0a 50 72 6f 63  65 73 73 6f 72 73 31 13  |....Processors1.|
>  00000160  30 11 06 03 55 04 03 0c  0a 54 49 20 73 75 70 70  |0...U....TI supp|
> 
> [...]
> 
>  000017a0  02 8a e5 06 c8 8c e2 14  c2 8a e5 7c 01 00 ea ed  |...........|....|
>  000017b0  1f 8f e2 66 02 00 ea 5b  45 72 72 6f 72 5d 20 52  |...f...[Error] R|
> -000017c0  69 64 20 55 54 43 20 49  44 21 21 21 0a 00 00 5b  |id UTC ID!!!...[|
> -000017d0  45 72 72 6f 72 5d 20 49  6e 76 61 6c 69 64 20 50  |Error] Invalid P|
> -000017e0  65 65 72 20 43 68 61 6e  6e 65 6c 20 4e 75 6d 62  |eer Channel Numb|
> -000017f0  65 72 21 21 21 0a 00 41  73 73 65 72 74 69 6f 6e  |er!!!..Assertion|
> +000017c0  4d 20 41 6c 6c 6f 63 20  54 58 20 43 68 20 66 61  |M Alloc TX Ch fa|
> +000017d0  69 6c 65 64 21 21 21 0a  00 00 00 73 72 63 2f 75  |iled!!!....src/u|
> +000017e0  64 6d 61 5f 63 68 2e 63  00 00 00 75 74 63 49 6e  |dma_ch.c...utcIn|
> +000017f0  66 6f 20 21 3d 20 4e 55  4c 4c 5f 50 54 52 00 75  |fo != NULL_PTR.u|
>  00001800  74 63 49 64 20 3c 3d 20  55 44 4d 41 5f 4e 55 4d  |tcId <= UDMA_NUM|
>  00001810  5f 55 54 43 5f 49 4e 53  54 41 4e 43 45 00 00 72  |_UTC_INSTANCE..r|
> 

I've done [1] for now in order to make the OSPI usable again here. It
looks like reading an mtd device in one chunk (single read syscall) is
fine, ie. "dd if=/dev/mtd3 of=content2 bs=<sizeof-mtd3>", while reading
it in multiple chunks is problematic, e.g. "dd if=/dev/mtd3 of=content2
bs=4096". Interestingly, the deviation is already on the first block,
which may speak against a setup issue for a second transfer.

The content I've seen in the corrupted output may come from other parts
of the memory. I've found my WIFI SSID there, which is definitely not
part of our OSPI image.

Jan

[1]
https://github.com/siemens/linux/commit/0abc3696f89f3a89214e483f7216b54e1b2196cd

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
  2020-08-26 10:12           ` Jan Kiszka
@ 2020-08-26 12:18             ` Vignesh Raghavendra
  2020-08-26 13:31               ` Jan Kiszka
  0 siblings, 1 reply; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-08-26 12:18 UTC (permalink / raw)
  To: Jan Kiszka, Tudor Ambarus, Mark Brown, Jin, Le (RC-CN DF FA R&D)
  Cc: Boris Brezillon, Ramuthevar Vadivel Murugan, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex



On 8/26/20 3:42 PM, Jan Kiszka wrote:
> On 24.08.20 19:20, Jan Kiszka wrote:
>> On 24.08.20 14:49, Jan Kiszka wrote:
>>> On 24.08.20 13:45, Vignesh Raghavendra wrote:
>>>>
>>>>
>>>> On 8/22/20 11:35 PM, Jan Kiszka wrote:
>>>>> On 01.06.20 09:04, Vignesh Raghavendra wrote:
>>>>>> dma_request_chan_by_mask() can throw EPROBE_DEFER if DMA provider
>>>>>> is not yet probed. Currently driver just falls back to using PIO mode
>>>>>> (which is less efficient) in this case. Instead return probe deferral
>>>>>> error as is so that driver will be re probed once DMA provider is
>>>>>> available.
>>>>>>
>>>>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>>>>>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>>>> ---
>>>> [...]
>>>>>>
>>>>>>  static const struct spi_nor_controller_ops cqspi_controller_ops = {
>>>>>> @@ -1269,8 +1274,11 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>>>>>>  			dev_dbg(nor->dev, "using direct mode for %s\n",
>>>>>>  				mtd->name);
>>>>>>
>>>>>> -			if (!cqspi->rx_chan)
>>>>>> -				cqspi_request_mmap_dma(cqspi);
>>>>>> +			if (!cqspi->rx_chan) {
>>>>>> +				ret = cqspi_request_mmap_dma(cqspi);
>>>>>> +				if (ret == -EPROBE_DEFER)
>>>>>> +					goto err;
>>>>>> +			}
>>>>>>  		}
>>>>>>  	}
>>>>>>
>>>>>>
>>>>>
>>>>> This seem to break reading the SPI flash on our IOT2050 [1] (didn't test
>>>>> the eval board yet).
>>>>>
>>>>> Without that commit, read happens via PIO, and that works. With the
>>>>> commit, the pattern
>>>>>
>>>>> with open("out.bin", "wb") as out:
>>>>>     pos = 0
>>>>>     while pos < 2:
>>>>>         with open("/dev/mtd0", "rb") as mtd:
>>>>>            mtd.seek(pos * 0x10000)
>>>>>            out.write(mtd.read(0x10000))
>>>>>         pos += 1
>>>>>
>>>>> gives the wrong result for the second block while
>>>>
>>>> Interesting... Could you please explain wrong result? Is the data move
>>>> around or completely garbage?
>>>
>>> It looks like some stripes contain data from other parts of the flash or
>>> kernel RAM. It's not just garbage, there are readable strings included.
>>>
>>>>
>>>> Does this fail even on AM654 EVM? Could you share full script for me to
>>>> test locally?
>>>
>>> The scripts are complete (python). Just binary-diff the outputs.
>>>
>>> I'll try on the EVM later.
>>
>> Done so now, could reproduce.
> 
> ..."could *not* reproduce" there. Sorry if that caused confusion.
> 

Oh, thanks! I was wondering why I cannot see this issue on AM654 EVM at my end...

>>
>> But the OSPIs are definitely different. EVM reports
>>
>> spi-nor spi0.0: mt35xu512aba (65536 Kbytes)
>>
>> with 4K erase size. Our our board, we have
>>
>> spi-nor spi7.0: w25q128 (16384 Kbytes)
>>
>> with 64K erase size.
>>
>> Here is some extract of the hex-diffs between out.bin and out2.bin (the 
>> latter being the good one):
>>
>> --- /dev/fd/63  2020-08-24 17:16:58.776409282 +0000
>> +++ /dev/fd/62  2020-08-24 17:16:58.776409282 +0000
>> @@ -6,18 +6,18 @@
>>  00000050  0f 30 0d 06 03 55 04 07  0c 06 44 61 6c 6c 61 73  |.0...U....Dallas|
>>  00000060  31 27 30 25 06 03 55 04  0a 0c 1e 54 65 78 61 73  |1'0%..U....Texas|
>>  00000070  20 49 6e 73 74 72 75 6d  65 6e 74 73 20 49 6e 63  | Instruments Inc|
>> -00000080  84 8b 96 2c 0c 12 18 03  01 05 05 04 01 02 00 00  |...,............|
>> -00000090  07 06 44 45 20 01 0d 14  2a 01 00 32 05 24 30 48  |..DE ...*..2.$0H|
>> -000000a0  60 6c 30 14 01 00 00 0f  ac 04 01 00 00 0f ac 04  |`l0.............|
>> -000000b0  01 00 00 0f ac 02 0c 00  2d 1a 6f 18 17 ff ff ff  |........-.o.....|
>> +00000080  6f 72 70 6f 72 61 74 65  64 31 13 30 11 06 03 55  |orporated1.0...U|
>> +00000090  04 0b 0c 0a 50 72 6f 63  65 73 73 6f 72 73 31 13  |....Processors1.|
>> +000000a0  30 11 06 03 55 04 03 0c  0a 54 49 20 73 75 70 70  |0...U....TI supp|
>> +000000b0  6f 72 74 31 1d 30 1b 06  09 2a 86 48 86 f7 0d 01  |ort1.0...*.H....|
>>  000000c0  09 01 16 0e 73 75 70 70  6f 72 74 40 74 69 2e 63  |....support@ti.c|
>>  000000d0  6f 6d 30 1e 17 0d 32 30  30 37 32 32 31 31 30 30  |om0...2007221100|
>>  000000e0  30 30 5a 17 0d 32 30 30  38 32 31 31 31 30 30 30  |00Z..20082111000|
>>  000000f0  30 5a 30 81 9d 31 0b 30  09 06 03 55 04 06 13 02  |0Z0..1.0...U....|
>> -00000100  00 00 27 a4 00 00 42 43  5e 00 62 32 2f 00 b4 96  |..'...BC^.b2/...|
>> -00000110  24 44 0c 00 c6 00 43 0a  00 00 0b f0 43 a5 2a 01  |$D....C.....C.*.|
>> -00000120  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
>> -*
>> +00000100  55 53 31 0b 30 09 06 03  55 04 08 0c 02 54 58 31  |US1.0...U....TX1|
>> +00000110  0f 30 0d 06 03 55 04 07  0c 06 44 61 6c 6c 61 73  |.0...U....Dallas|
>> +00000120  31 27 30 25 06 03 55 04  0a 0c 1e 54 65 78 61 73  |1'0%..U....Texas|
>> +00000130  20 49 6e 73 74 72 75 6d  65 6e 74 73 20 49 6e 63  | Instruments Inc|
>>  00000140  6f 72 70 6f 72 61 74 65  64 31 13 30 11 06 03 55  |orporated1.0...U|
>>  00000150  04 0b 0c 0a 50 72 6f 63  65 73 73 6f 72 73 31 13  |....Processors1.|
>>  00000160  30 11 06 03 55 04 03 0c  0a 54 49 20 73 75 70 70  |0...U....TI supp|
>>
>> [...]
>>
>>  000017a0  02 8a e5 06 c8 8c e2 14  c2 8a e5 7c 01 00 ea ed  |...........|....|
>>  000017b0  1f 8f e2 66 02 00 ea 5b  45 72 72 6f 72 5d 20 52  |...f...[Error] R|
>> -000017c0  69 64 20 55 54 43 20 49  44 21 21 21 0a 00 00 5b  |id UTC ID!!!...[|
>> -000017d0  45 72 72 6f 72 5d 20 49  6e 76 61 6c 69 64 20 50  |Error] Invalid P|
>> -000017e0  65 65 72 20 43 68 61 6e  6e 65 6c 20 4e 75 6d 62  |eer Channel Numb|
>> -000017f0  65 72 21 21 21 0a 00 41  73 73 65 72 74 69 6f 6e  |er!!!..Assertion|
>> +000017c0  4d 20 41 6c 6c 6f 63 20  54 58 20 43 68 20 66 61  |M Alloc TX Ch fa|
>> +000017d0  69 6c 65 64 21 21 21 0a  00 00 00 73 72 63 2f 75  |iled!!!....src/u|
>> +000017e0  64 6d 61 5f 63 68 2e 63  00 00 00 75 74 63 49 6e  |dma_ch.c...utcIn|
>> +000017f0  66 6f 20 21 3d 20 4e 55  4c 4c 5f 50 54 52 00 75  |fo != NULL_PTR.u|
>>  00001800  74 63 49 64 20 3c 3d 20  55 44 4d 41 5f 4e 55 4d  |tcId <= UDMA_NUM|
>>  00001810  5f 55 54 43 5f 49 4e 53  54 41 4e 43 45 00 00 72  |_UTC_INSTANCE..r|
>>
> 
> I've done [1] for now in order to make the OSPI usable again here. It
> looks like reading an mtd device in one chunk (single read syscall) is
> fine, ie. "dd if=/dev/mtd3 of=content2 bs=<sizeof-mtd3>", while reading
> it in multiple chunks is problematic, e.g. "dd if=/dev/mtd3 of=content2
> bs=4096". Interestingly, the deviation is already on the first block,
> which may speak against a setup issue for a second transfer.

I cannot think of a reasonable explanation for this. I wonder how the first chunk 
gets affected when reading in multiple chunks.

> 
> The content I've seen in the corrupted output may come from other parts
> of the memory. I've found my WIFI SSID there, which is definitely not
> part of our OSPI image.
> 

Hmm, one guess it that QSPI on IoT board is 16MB and hence 
does not support 4 byte addressing vs the OSPI on AM654 EVM. 
There could be bug around that case.

So, could you apply diff [1] on linux-next and then execute 
falling testcase and post the register dump printed?

[1]:


---><8---


diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 508b219eabf80..b9739ae919340 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -907,6 +907,10 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
        struct dma_async_tx_descriptor *tx;
        dma_cookie_t cookie;
        dma_addr_t dma_dst;
+       int i;
+
+       for (i = 0; i < 10; i++)
+               dev_err(dev, "REG off %x:  val %x\n", i, readl(cqspi->iobase + (i << 2)));
 
        if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
                memcpy_fromio(buf, cqspi->ahb_base + from, len);

 
Also, there seems to be DMA mapping related issue, that was always present in 
older driver as well. Could you see if diff [2] fixes the issue?

[2] Use DMA device for mapping:

---><8---


diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index b9739ae919340..a546aa4598758 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -901,6 +901,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
 {
        struct cqspi_st *cqspi = f_pdata->cqspi;
        struct device *dev = &cqspi->pdev->dev;
+       struct device *ddev = cqspi->rx_chan->device->dev;
        enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
        dma_addr_t dma_src = (dma_addr_t)cqspi->mmap_phys_base + from;
        int ret = 0;
@@ -917,8 +918,8 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
                return 0;
        }
 
-       dma_dst = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
-       if (dma_mapping_error(dev, dma_dst)) {
+       dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
+       if (dma_mapping_error(ddev, dma_dst)) {
                dev_err(dev, "dma mapping failed\n");
                return -ENOMEM;
        }
@@ -952,7 +953,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
        }
 
 err_unmap:
-       dma_unmap_single(dev, dma_dst, len, DMA_FROM_DEVICE);
+       dma_unmap_single(ddev, dma_dst, len, DMA_FROM_DEVICE);
 
        return ret;
 }


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
  2020-08-26 12:18             ` Vignesh Raghavendra
@ 2020-08-26 13:31               ` Jan Kiszka
  2020-08-27  7:04                 ` Vignesh Raghavendra
  0 siblings, 1 reply; 28+ messages in thread
From: Jan Kiszka @ 2020-08-26 13:31 UTC (permalink / raw)
  To: Vignesh Raghavendra, Tudor Ambarus, Mark Brown, Jin,
	Le (RC-CN DF FA R&D)
  Cc: Boris Brezillon, Ramuthevar Vadivel Murugan, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex

On 26.08.20 14:18, Vignesh Raghavendra wrote:
> 
> 
> On 8/26/20 3:42 PM, Jan Kiszka wrote:
>> On 24.08.20 19:20, Jan Kiszka wrote:
>>> On 24.08.20 14:49, Jan Kiszka wrote:
>>>> On 24.08.20 13:45, Vignesh Raghavendra wrote:
>>>>>
>>>>>
>>>>> On 8/22/20 11:35 PM, Jan Kiszka wrote:
>>>>>> On 01.06.20 09:04, Vignesh Raghavendra wrote:
>>>>>>> dma_request_chan_by_mask() can throw EPROBE_DEFER if DMA provider
>>>>>>> is not yet probed. Currently driver just falls back to using PIO mode
>>>>>>> (which is less efficient) in this case. Instead return probe deferral
>>>>>>> error as is so that driver will be re probed once DMA provider is
>>>>>>> available.
>>>>>>>
>>>>>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>>>>>>> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>>>>>>> ---
>>>>> [...]
>>>>>>>
>>>>>>>  static const struct spi_nor_controller_ops cqspi_controller_ops = {
>>>>>>> @@ -1269,8 +1274,11 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>>>>>>>  			dev_dbg(nor->dev, "using direct mode for %s\n",
>>>>>>>  				mtd->name);
>>>>>>>
>>>>>>> -			if (!cqspi->rx_chan)
>>>>>>> -				cqspi_request_mmap_dma(cqspi);
>>>>>>> +			if (!cqspi->rx_chan) {
>>>>>>> +				ret = cqspi_request_mmap_dma(cqspi);
>>>>>>> +				if (ret == -EPROBE_DEFER)
>>>>>>> +					goto err;
>>>>>>> +			}
>>>>>>>  		}
>>>>>>>  	}
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> This seem to break reading the SPI flash on our IOT2050 [1] (didn't test
>>>>>> the eval board yet).
>>>>>>
>>>>>> Without that commit, read happens via PIO, and that works. With the
>>>>>> commit, the pattern
>>>>>>
>>>>>> with open("out.bin", "wb") as out:
>>>>>>     pos = 0
>>>>>>     while pos < 2:
>>>>>>         with open("/dev/mtd0", "rb") as mtd:
>>>>>>            mtd.seek(pos * 0x10000)
>>>>>>            out.write(mtd.read(0x10000))
>>>>>>         pos += 1
>>>>>>
>>>>>> gives the wrong result for the second block while
>>>>>
>>>>> Interesting... Could you please explain wrong result? Is the data move
>>>>> around or completely garbage?
>>>>
>>>> It looks like some stripes contain data from other parts of the flash or
>>>> kernel RAM. It's not just garbage, there are readable strings included.
>>>>
>>>>>
>>>>> Does this fail even on AM654 EVM? Could you share full script for me to
>>>>> test locally?
>>>>
>>>> The scripts are complete (python). Just binary-diff the outputs.
>>>>
>>>> I'll try on the EVM later.
>>>
>>> Done so now, could reproduce.
>>
>> ..."could *not* reproduce" there. Sorry if that caused confusion.
>>
> 
> Oh, thanks! I was wondering why I cannot see this issue on AM654 EVM at my end...
> 
>>>
>>> But the OSPIs are definitely different. EVM reports
>>>
>>> spi-nor spi0.0: mt35xu512aba (65536 Kbytes)
>>>
>>> with 4K erase size. Our our board, we have
>>>
>>> spi-nor spi7.0: w25q128 (16384 Kbytes)
>>>
>>> with 64K erase size.
>>>
>>> Here is some extract of the hex-diffs between out.bin and out2.bin (the 
>>> latter being the good one):
>>>
>>> --- /dev/fd/63  2020-08-24 17:16:58.776409282 +0000
>>> +++ /dev/fd/62  2020-08-24 17:16:58.776409282 +0000
>>> @@ -6,18 +6,18 @@
>>>  00000050  0f 30 0d 06 03 55 04 07  0c 06 44 61 6c 6c 61 73  |.0...U....Dallas|
>>>  00000060  31 27 30 25 06 03 55 04  0a 0c 1e 54 65 78 61 73  |1'0%..U....Texas|
>>>  00000070  20 49 6e 73 74 72 75 6d  65 6e 74 73 20 49 6e 63  | Instruments Inc|
>>> -00000080  84 8b 96 2c 0c 12 18 03  01 05 05 04 01 02 00 00  |...,............|
>>> -00000090  07 06 44 45 20 01 0d 14  2a 01 00 32 05 24 30 48  |..DE ...*..2.$0H|
>>> -000000a0  60 6c 30 14 01 00 00 0f  ac 04 01 00 00 0f ac 04  |`l0.............|
>>> -000000b0  01 00 00 0f ac 02 0c 00  2d 1a 6f 18 17 ff ff ff  |........-.o.....|
>>> +00000080  6f 72 70 6f 72 61 74 65  64 31 13 30 11 06 03 55  |orporated1.0...U|
>>> +00000090  04 0b 0c 0a 50 72 6f 63  65 73 73 6f 72 73 31 13  |....Processors1.|
>>> +000000a0  30 11 06 03 55 04 03 0c  0a 54 49 20 73 75 70 70  |0...U....TI supp|
>>> +000000b0  6f 72 74 31 1d 30 1b 06  09 2a 86 48 86 f7 0d 01  |ort1.0...*.H....|
>>>  000000c0  09 01 16 0e 73 75 70 70  6f 72 74 40 74 69 2e 63  |....support@ti.c|
>>>  000000d0  6f 6d 30 1e 17 0d 32 30  30 37 32 32 31 31 30 30  |om0...2007221100|
>>>  000000e0  30 30 5a 17 0d 32 30 30  38 32 31 31 31 30 30 30  |00Z..20082111000|
>>>  000000f0  30 5a 30 81 9d 31 0b 30  09 06 03 55 04 06 13 02  |0Z0..1.0...U....|
>>> -00000100  00 00 27 a4 00 00 42 43  5e 00 62 32 2f 00 b4 96  |..'...BC^.b2/...|
>>> -00000110  24 44 0c 00 c6 00 43 0a  00 00 0b f0 43 a5 2a 01  |$D....C.....C.*.|
>>> -00000120  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
>>> -*
>>> +00000100  55 53 31 0b 30 09 06 03  55 04 08 0c 02 54 58 31  |US1.0...U....TX1|
>>> +00000110  0f 30 0d 06 03 55 04 07  0c 06 44 61 6c 6c 61 73  |.0...U....Dallas|
>>> +00000120  31 27 30 25 06 03 55 04  0a 0c 1e 54 65 78 61 73  |1'0%..U....Texas|
>>> +00000130  20 49 6e 73 74 72 75 6d  65 6e 74 73 20 49 6e 63  | Instruments Inc|
>>>  00000140  6f 72 70 6f 72 61 74 65  64 31 13 30 11 06 03 55  |orporated1.0...U|
>>>  00000150  04 0b 0c 0a 50 72 6f 63  65 73 73 6f 72 73 31 13  |....Processors1.|
>>>  00000160  30 11 06 03 55 04 03 0c  0a 54 49 20 73 75 70 70  |0...U....TI supp|
>>>
>>> [...]
>>>
>>>  000017a0  02 8a e5 06 c8 8c e2 14  c2 8a e5 7c 01 00 ea ed  |...........|....|
>>>  000017b0  1f 8f e2 66 02 00 ea 5b  45 72 72 6f 72 5d 20 52  |...f...[Error] R|
>>> -000017c0  69 64 20 55 54 43 20 49  44 21 21 21 0a 00 00 5b  |id UTC ID!!!...[|
>>> -000017d0  45 72 72 6f 72 5d 20 49  6e 76 61 6c 69 64 20 50  |Error] Invalid P|
>>> -000017e0  65 65 72 20 43 68 61 6e  6e 65 6c 20 4e 75 6d 62  |eer Channel Numb|
>>> -000017f0  65 72 21 21 21 0a 00 41  73 73 65 72 74 69 6f 6e  |er!!!..Assertion|
>>> +000017c0  4d 20 41 6c 6c 6f 63 20  54 58 20 43 68 20 66 61  |M Alloc TX Ch fa|
>>> +000017d0  69 6c 65 64 21 21 21 0a  00 00 00 73 72 63 2f 75  |iled!!!....src/u|
>>> +000017e0  64 6d 61 5f 63 68 2e 63  00 00 00 75 74 63 49 6e  |dma_ch.c...utcIn|
>>> +000017f0  66 6f 20 21 3d 20 4e 55  4c 4c 5f 50 54 52 00 75  |fo != NULL_PTR.u|
>>>  00001800  74 63 49 64 20 3c 3d 20  55 44 4d 41 5f 4e 55 4d  |tcId <= UDMA_NUM|
>>>  00001810  5f 55 54 43 5f 49 4e 53  54 41 4e 43 45 00 00 72  |_UTC_INSTANCE..r|
>>>
>>
>> I've done [1] for now in order to make the OSPI usable again here. It
>> looks like reading an mtd device in one chunk (single read syscall) is
>> fine, ie. "dd if=/dev/mtd3 of=content2 bs=<sizeof-mtd3>", while reading
>> it in multiple chunks is problematic, e.g. "dd if=/dev/mtd3 of=content2
>> bs=4096". Interestingly, the deviation is already on the first block,
>> which may speak against a setup issue for a second transfer.
> 
> I cannot think of a reasonable explanation for this. I wonder how the first chunk 
> gets affected when reading in multiple chunks.
> 
>>
>> The content I've seen in the corrupted output may come from other parts
>> of the memory. I've found my WIFI SSID there, which is definitely not
>> part of our OSPI image.
>>
> 
> Hmm, one guess it that QSPI on IoT board is 16MB and hence 

It is 16M, true.

> does not support 4 byte addressing vs the OSPI on AM654 EVM. 
> There could be bug around that case.
> 
> So, could you apply diff [1] on linux-next and then execute 
> falling testcase and post the register dump printed?
> 
> [1]:
> 

Apparently always this:

[  197.602226] cadence-qspi 47040000.spi: REG off 0:  val 80083881
[  197.608181] cadence-qspi 47040000.spi: REG off 1:  val 3
[  197.613512] cadence-qspi 47040000.spi: REG off 2:  val 2
[  197.618833] cadence-qspi 47040000.spi: REG off 3:  val a0a0a0a
[  197.624677] cadence-qspi 47040000.spi: REG off 4:  val 5
[  197.629998] cadence-qspi 47040000.spi: REG off 5:  val 101002
[  197.635759] cadence-qspi 47040000.spi: REG off 6:  val 80
[  197.641165] cadence-qspi 47040000.spi: REG off 7:  val 0
[  197.646488] cadence-qspi 47040000.spi: REG off 8:  val 0
[  197.651809] cadence-qspi 47040000.spi: REG off 9:  val 0

> 
> ---><8---
> 
> 
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 508b219eabf80..b9739ae919340 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -907,6 +907,10 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>         struct dma_async_tx_descriptor *tx;
>         dma_cookie_t cookie;
>         dma_addr_t dma_dst;
> +       int i;
> +
> +       for (i = 0; i < 10; i++)
> +               dev_err(dev, "REG off %x:  val %x\n", i, readl(cqspi->iobase + (i << 2)));
>  
>         if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
>                 memcpy_fromio(buf, cqspi->ahb_base + from, len);
> 
>  
> Also, there seems to be DMA mapping related issue, that was always present in 
> older driver as well. Could you see if diff [2] fixes the issue?
> 
> [2] Use DMA device for mapping:
> 
> ---><8---
> 
> 
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index b9739ae919340..a546aa4598758 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -901,6 +901,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>  {
>         struct cqspi_st *cqspi = f_pdata->cqspi;
>         struct device *dev = &cqspi->pdev->dev;
> +       struct device *ddev = cqspi->rx_chan->device->dev;
>         enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
>         dma_addr_t dma_src = (dma_addr_t)cqspi->mmap_phys_base + from;
>         int ret = 0;
> @@ -917,8 +918,8 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>                 return 0;
>         }
>  
> -       dma_dst = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
> -       if (dma_mapping_error(dev, dma_dst)) {
> +       dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
> +       if (dma_mapping_error(ddev, dma_dst)) {
>                 dev_err(dev, "dma mapping failed\n");
>                 return -ENOMEM;
>         }
> @@ -952,7 +953,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>         }
>  
>  err_unmap:
> -       dma_unmap_single(dev, dma_dst, len, DMA_FROM_DEVICE);
> +       dma_unmap_single(ddev, dma_dst, len, DMA_FROM_DEVICE);
>  
>         return ret;
>  }
> 

That seems to help! Wasn't able to reproduce the issue with this applied
so far.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel
  2020-08-26 13:31               ` Jan Kiszka
@ 2020-08-27  7:04                 ` Vignesh Raghavendra
  0 siblings, 0 replies; 28+ messages in thread
From: Vignesh Raghavendra @ 2020-08-27  7:04 UTC (permalink / raw)
  To: Jan Kiszka, Tudor Ambarus, Mark Brown, Jin, Le (RC-CN DF FA R&D)
  Cc: Boris Brezillon, Ramuthevar Vadivel Murugan, linux-mtd,
	linux-kernel, linux-spi, simon.k.r.goldschmidt, dinguyen, marex



On 8/26/20 7:01 PM, Jan Kiszka wrote:
> On 26.08.20 14:18, Vignesh Raghavendra wrote:
>> On 8/26/20 3:42 PM, Jan Kiszka wrote:
>>> On 24.08.20 19:20, Jan Kiszka wrote:
>>>> On 24.08.20 14:49, Jan Kiszka wrote:
>>>>> On 24.08.20 13:45, Vignesh Raghavendra wrote:
>>>>>>
[...]
>> Also, there seems to be DMA mapping related issue, that was always present in 
>> older driver as well. Could you see if diff [2] fixes the issue?
>>
>> [2] Use DMA device for mapping:
>>
>> ---><8---
>>
>>
>> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
>> index b9739ae919340..a546aa4598758 100644
>> --- a/drivers/spi/spi-cadence-quadspi.c
>> +++ b/drivers/spi/spi-cadence-quadspi.c
>> @@ -901,6 +901,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>>  {
>>         struct cqspi_st *cqspi = f_pdata->cqspi;
>>         struct device *dev = &cqspi->pdev->dev;
>> +       struct device *ddev = cqspi->rx_chan->device->dev;
>>         enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
>>         dma_addr_t dma_src = (dma_addr_t)cqspi->mmap_phys_base + from;
>>         int ret = 0;
>> @@ -917,8 +918,8 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>>                 return 0;
>>         }
>>  
>> -       dma_dst = dma_map_single(dev, buf, len, DMA_FROM_DEVICE);
>> -       if (dma_mapping_error(dev, dma_dst)) {
>> +       dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
>> +       if (dma_mapping_error(ddev, dma_dst)) {
>>                 dev_err(dev, "dma mapping failed\n");
>>                 return -ENOMEM;
>>         }
>> @@ -952,7 +953,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>>         }
>>  
>>  err_unmap:
>> -       dma_unmap_single(dev, dma_dst, len, DMA_FROM_DEVICE);
>> +       dma_unmap_single(ddev, dma_dst, len, DMA_FROM_DEVICE);
>>  
>>         return ret;
>>  }
>>
> 
> That seems to help! Wasn't able to reproduce the issue with this applied
> so far.
> 

OK, great... I will post this patch soon once I finish a bit more
testing... Thanks!


Regards
Vignesh

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2020-08-27  7:04 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-01  7:04 [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Vignesh Raghavendra
2020-06-01  7:04 ` [RESEND PATCH v3 1/8] mtd: spi-nor: cadence-quadspi: Make driver independent of flash geometry Vignesh Raghavendra
2020-06-01  7:04 ` [RESEND PATCH v3 2/8] mtd: spi-nor: cadence-quadspi: Provide a way to disable DAC mode Vignesh Raghavendra
2020-06-01  7:04 ` [RESEND PATCH v3 3/8] mtd: spi-nor: cadence-quadspi: Don't initialize rx_dma_complete on failure Vignesh Raghavendra
2020-06-01  7:04 ` [RESEND PATCH v3 4/8] mtd: spi-nor: cadence-quadspi: Fix error path on failure to acquire reset lines Vignesh Raghavendra
2020-06-01  7:04 ` [RESEND PATCH v3 5/8] mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel Vignesh Raghavendra
2020-08-22 18:05   ` Jan Kiszka
2020-08-24 11:45     ` Vignesh Raghavendra
2020-08-24 12:49       ` Jan Kiszka
2020-08-24 17:20         ` Jan Kiszka
2020-08-26 10:12           ` Jan Kiszka
2020-08-26 12:18             ` Vignesh Raghavendra
2020-08-26 13:31               ` Jan Kiszka
2020-08-27  7:04                 ` Vignesh Raghavendra
2020-08-25  3:12         ` Jin, Le
2020-06-01  7:04 ` [RESEND PATCH v3 6/8] mtd: spi-nor: cadence-quadspi: Drop redundant WREN in erase path Vignesh Raghavendra
2020-06-01  7:04 ` [RESEND PATCH v3 7/8] mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework Vignesh Raghavendra
2020-08-24  5:55   ` Jan Kiszka
2020-08-24 11:44     ` Vignesh Raghavendra
2020-08-24 12:04       ` Boris Brezillon
2020-08-24 13:52         ` Jan Kiszka
2020-08-24 12:06       ` Jan Kiszka
2020-06-01  7:04 ` [RESEND PATCH v3 8/8] spi: Move cadence-quadspi driver to drivers/spi/ Vignesh Raghavendra
2020-06-19 10:57 ` [RESEND PATCH v3 0/8] mtd: spi-nor: Move cadence-qaudspi to spi-mem framework Mark Brown
2020-06-19 11:47   ` Tudor.Ambarus
2020-06-19 15:17     ` Mark Brown
2020-06-26  9:25       ` Tudor.Ambarus
2020-06-19 13:28 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).