[PATCH 0/7] spi: cadence-quadspi: Add Octal DTR support

[PATCH 0/7] spi: cadence-quadspi: Add Octal DTR support

[Pratyush Yadav]

Hi,

This series adds support for Octal DTR mode now that SPI NOR supports
these flashes. Patches 1/7 to 4/7 and 6/7 fix some minor bugs and issues.
Patche 5/7 lays some groundwork by implementing the supports_op() hook.
Patch 7/7 adds the Octal DTR mode support.

While the main aim of this series is to support 8D-8D-8D mode, other
modes like 4D-4D-4D or 2S-2S-2S should also now be supported, though
they have not been tested.

Tested on J721E with Micron MT35XU512ABA and on J7200 with Cypress
S28HS512T. Tested on J721E with Micron MT25QU512A (1S-1S-4S) for
regressions.

Pratyush Yadav (7):
  spi: cadence-quadspi: Set master max_speed_hz
  spi: cadence-quadspi: Abort read if dummy cycles required are too many
  spi: cadence-quadspi: Set dummy cycles from STIG commands
  spi: cadence-quadspi: Fix dummy cycle calculation when buswidth > 1
  spi: cadence-quadspi: Implement a simple supports_op hook
  spi: cadence-quadspi: Wait at least 500 ms for direct reads
  spi: cadence-quadspi: Add DTR support

 drivers/spi/spi-cadence-quadspi.c | 364 ++++++++++++++++++++++++++----
 1 file changed, 325 insertions(+), 39 deletions(-)

--
2.28.0

[PATCH 1/7] spi: cadence-quadspi: Set master max_speed_hz

[Pratyush Yadav]

As of commit 9326e4f1e5dd ("spi: Limit the spi device max speed to
controller's max speed"), the SPI device max speed is set to the
controller's max speed if it is larger. The Cadence QSPI controller does
not set the controller's max speed so it is left at its initial value of
0. This means the SPI device max speed is always set to 0.

The SPI device max speed is used to calculate the baud rate divider when
performing an operation. If this speed is 0, the default divider of 32
is used. No matter what speed is specified by the device tree property
'spi-max-frequency', the device will always operate at ref_clk / 32.

Fix this by setting master->max_speed_hz to the ref clock speed so the
correct divider can be calculated.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
 drivers/spi/spi-cadence-quadspi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index ba7d40c2922f..ea3890c7d9ff 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1279,6 +1279,7 @@ static int cqspi_probe(struct platform_device *pdev)
 	reset_control_deassert(rstc_ocp);
 
 	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
+	master->max_speed_hz = cqspi->master_ref_clk_hz;
 	ddata  = of_device_get_match_data(dev);
 	if (ddata) {
 		if (ddata->quirks & CQSPI_NEEDS_WR_DELAY)
-- 
2.28.0

[PATCH 2/7] spi: cadence-quadspi: Abort read if dummy cycles required are too many

[Pratyush Yadav]

The controller can only support up to 31 dummy cycles. If the command
requires more it falls back to using 31. This command is likely to fail
because the correct number of cycles are not waited upon. Rather than
silently issuing an incorrect command, fail loudly so the caller can get
a chance to find out the command can't be supported by the controller.

Fixes: 140623410536 ("mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller")
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
 drivers/spi/spi-cadence-quadspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index ea3890c7d9ff..5efb1f929be0 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -461,7 +461,7 @@ static int cqspi_read_setup(struct cqspi_flash_pdata *f_pdata,
 	/* Setup dummy clock cycles */
 	dummy_clk = op->dummy.nbytes * 8;
 	if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
-		dummy_clk = CQSPI_DUMMY_CLKS_MAX;
+		return -EOPNOTSUPP;
 
 	if (dummy_clk)
 		reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
-- 
2.28.0

[PATCH 3/7] spi: cadence-quadspi: Set dummy cycles from STIG commands

[Pratyush Yadav]

If a command does not have an address phase it goes via the STIG path.
The dummy cycles are not initialized for the STIG commands. As a result,
STIG commands with dummy cycles will not work.

Initialize the dummy cycle field before issuing the STIG command to make
sure it is sent correctly. Move the code to calculate dummy cycle value
to a separate function so it is not repeated twice. DTR support will add
some more logic here to it is worth it to extract it out in a function.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
 drivers/spi/spi-cadence-quadspi.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 5efb1f929be0..6a778014ff60 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -188,6 +188,7 @@ struct cqspi_driver_platdata {
 #define CQSPI_REG_CMDCTRL			0x90
 #define CQSPI_REG_CMDCTRL_EXECUTE_MASK		BIT(0)
 #define CQSPI_REG_CMDCTRL_INPROGRESS_MASK	BIT(1)
+#define CQSPI_REG_CMDCTRL_DUMMY_LSB		7
 #define CQSPI_REG_CMDCTRL_WR_BYTES_LSB		12
 #define CQSPI_REG_CMDCTRL_WR_EN_LSB		15
 #define CQSPI_REG_CMDCTRL_ADD_BYTES_LSB		16
@@ -198,6 +199,7 @@ struct cqspi_driver_platdata {
 #define CQSPI_REG_CMDCTRL_WR_BYTES_MASK		0x7
 #define CQSPI_REG_CMDCTRL_ADD_BYTES_MASK	0x3
 #define CQSPI_REG_CMDCTRL_RD_BYTES_MASK		0x7
+#define CQSPI_REG_CMDCTRL_DUMMY_MASK		0x1F
 
 #define CQSPI_REG_INDIRECTWR			0x70
 #define CQSPI_REG_INDIRECTWR_START_MASK		BIT(0)
@@ -288,6 +290,15 @@ static unsigned int cqspi_calc_rdreg(struct cqspi_flash_pdata *f_pdata)
 	return rdreg;
 }
 
+static unsigned int cqspi_calc_dummy(const struct spi_mem_op *op)
+{
+	unsigned int dummy_clk;
+
+	dummy_clk = op->dummy.nbytes * 8;
+
+	return dummy_clk;
+}
+
 static int cqspi_wait_idle(struct cqspi_st *cqspi)
 {
 	const unsigned int poll_idle_retry = 3;
@@ -355,6 +366,7 @@ static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
 	size_t n_rx = op->data.nbytes;
 	unsigned int rdreg;
 	unsigned int reg;
+	unsigned int dummy_clk;
 	size_t read_len;
 	int status;
 
@@ -370,6 +382,14 @@ static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
 	rdreg = cqspi_calc_rdreg(f_pdata);
 	writel(rdreg, reg_base + CQSPI_REG_RD_INSTR);
 
+	dummy_clk = cqspi_calc_dummy(op);
+	if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
+		return -EOPNOTSUPP;
+
+	if (dummy_clk)
+		reg |= (dummy_clk & CQSPI_REG_CMDCTRL_DUMMY_MASK)
+		     << CQSPI_REG_CMDCTRL_DUMMY_LSB;
+
 	reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB);
 
 	/* 0 means 1 byte. */
@@ -459,7 +479,8 @@ static int cqspi_read_setup(struct cqspi_flash_pdata *f_pdata,
 	reg |= cqspi_calc_rdreg(f_pdata);
 
 	/* Setup dummy clock cycles */
-	dummy_clk = op->dummy.nbytes * 8;
+	dummy_clk = cqspi_calc_dummy(op);
+
 	if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
 		return -EOPNOTSUPP;
 
-- 
2.28.0

[PATCH 4/7] spi: cadence-quadspi: Fix dummy cycle calculation when buswidth > 1

[Pratyush Yadav]

SPI MEM deals with dummy bytes but the controller deals with dummy
cycles. Multiplying bytes by 8 is correct if the dummy phase uses 1S
mode since 1 byte will be sent in 8 cycles. But if the dummy phase uses
4S mode then 1 byte will be sent in 2 cycles.

To correctly translate dummy bytes to dummy cycles, the dummy buswidth
also needs to be taken into account. Divide 8 by the buswidth to get the
correct multiplier for getting the number of cycles.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
 drivers/spi/spi-cadence-quadspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 6a778014ff60..376abef43530 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -294,7 +294,7 @@ static unsigned int cqspi_calc_dummy(const struct spi_mem_op *op)
 {
 	unsigned int dummy_clk;
 
-	dummy_clk = op->dummy.nbytes * 8;
+	dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth);
 
 	return dummy_clk;
 }
-- 
2.28.0

[PATCH 5/7] spi: cadence-quadspi: Implement a simple supports_op hook

[Pratyush Yadav]

The default SPI MEM supports_op hook rejects DTR ops by default. Add a
simple supports_op hook that very closely imitates the SPI MEM one. It
will be extended in later commits to allow DTR ops.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
 drivers/spi/spi-cadence-quadspi.c | 61 +++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 376abef43530..1781d4e94ebd 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1031,6 +1031,66 @@ static int cqspi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 	return ret;
 }
 
+static int cqspi_check_buswidth_req(struct spi_mem *mem, u8 buswidth, bool tx)
+{
+	u32 mode = mem->spi->mode;
+
+	switch (buswidth) {
+	case 1:
+		return 0;
+
+	case 2:
+		if ((tx &&
+		     (mode & (SPI_TX_DUAL | SPI_TX_QUAD | SPI_TX_OCTAL))) ||
+		    (!tx &&
+		     (mode & (SPI_RX_DUAL | SPI_RX_QUAD | SPI_RX_OCTAL))))
+			return 0;
+
+		break;
+
+	case 4:
+		if ((tx && (mode & (SPI_TX_QUAD | SPI_TX_OCTAL))) ||
+		    (!tx && (mode & (SPI_RX_QUAD | SPI_RX_OCTAL))))
+			return 0;
+
+		break;
+
+	case 8:
+		if ((tx && (mode & SPI_TX_OCTAL)) ||
+		    (!tx && (mode & SPI_RX_OCTAL)))
+			return 0;
+
+		break;
+
+	default:
+		break;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static bool cqspi_supports_mem_op(struct spi_mem *mem,
+				  const struct spi_mem_op *op)
+{
+	if (cqspi_check_buswidth_req(mem, op->cmd.buswidth, true))
+		return false;
+
+	if (op->addr.nbytes &&
+	    cqspi_check_buswidth_req(mem, op->addr.buswidth, true))
+		return false;
+
+	if (op->dummy.nbytes &&
+	    cqspi_check_buswidth_req(mem, op->dummy.buswidth, true))
+		return false;
+
+	if (op->data.nbytes &&
+	    cqspi_check_buswidth_req(mem, op->data.buswidth,
+				     op->data.dir == SPI_MEM_DATA_OUT))
+		return false;
+
+	return true;
+}
+
 static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
 				    struct cqspi_flash_pdata *f_pdata,
 				    struct device_node *np)
@@ -1159,6 +1219,7 @@ static const char *cqspi_get_name(struct spi_mem *mem)
 static const struct spi_controller_mem_ops cqspi_mem_ops = {
 	.exec_op = cqspi_exec_mem_op,
 	.get_name = cqspi_get_name,
+	.supports_op = cqspi_supports_mem_op,
 };
 
 static int cqspi_setup_flash(struct cqspi_st *cqspi)
-- 
2.28.0

[PATCH 6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads

[Pratyush Yadav]

When performing a direct read via DMA the timeout for completion is set
equal to the read length. This is fine for larger reads. For a small
read like the Read Status Register command, the timeout would be 1 or 2
milliseconds. This is not enough to cover the overhead needed in setting
up DMA.

Make sure the timeout is at least 500 ms to allow DMA ample time to
finish. For reads larger than 500 bytes, the timeout will continue to be
equal to the read length.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
 drivers/spi/spi-cadence-quadspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 1781d4e94ebd..90040664e1b9 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -963,7 +963,7 @@ static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
 
 	dma_async_issue_pending(cqspi->rx_chan);
 	if (!wait_for_completion_timeout(&cqspi->rx_dma_complete,
-					 msecs_to_jiffies(len))) {
+					 msecs_to_jiffies(max(len, 500UL)))) {
 		dmaengine_terminate_sync(cqspi->rx_chan);
 		dev_err(dev, "DMA wait_for_completion_timeout\n");
 		ret = -ETIMEDOUT;
-- 
2.28.0

Re: [PATCH 6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads

[kernel test robot]

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

Hi Pratyush,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on spi/for-next]
[also build test WARNING on v5.11-rc1 next-20201223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: arm-randconfig-r006-20201221 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cee1e7d14f4628d6174b33640d502bff3b54ae45)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/04a7bcbc449363e5d6f498376c69116567b49d7d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
        git checkout 04a7bcbc449363e5d6f498376c69116567b49d7d
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/spi/spi-cadence-quadspi.c:966:24: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (500UL) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
                                            msecs_to_jiffies(max(len, 500UL)))) {
                                                             ^~~~~~~~~~~~~~~
   include/linux/minmax.h:58:19: note: expanded from macro 'max'
   #define max(x, y)       __careful_cmp(x, y, >)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:42:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:32:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:18:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
   1 warning generated.


vim +966 drivers/spi/spi-cadence-quadspi.c

   919	
   920	static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
   921					     u_char *buf, loff_t from, size_t len)
   922	{
   923		struct cqspi_st *cqspi = f_pdata->cqspi;
   924		struct device *dev = &cqspi->pdev->dev;
   925		enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
   926		dma_addr_t dma_src = (dma_addr_t)cqspi->mmap_phys_base + from;
   927		int ret = 0;
   928		struct dma_async_tx_descriptor *tx;
   929		dma_cookie_t cookie;
   930		dma_addr_t dma_dst;
   931		struct device *ddev;
   932	
   933		if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
   934			memcpy_fromio(buf, cqspi->ahb_base + from, len);
   935			return 0;
   936		}
   937	
   938		ddev = cqspi->rx_chan->device->dev;
   939		dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
   940		if (dma_mapping_error(ddev, dma_dst)) {
   941			dev_err(dev, "dma mapping failed\n");
   942			return -ENOMEM;
   943		}
   944		tx = dmaengine_prep_dma_memcpy(cqspi->rx_chan, dma_dst, dma_src,
   945					       len, flags);
   946		if (!tx) {
   947			dev_err(dev, "device_prep_dma_memcpy error\n");
   948			ret = -EIO;
   949			goto err_unmap;
   950		}
   951	
   952		tx->callback = cqspi_rx_dma_callback;
   953		tx->callback_param = cqspi;
   954		cookie = tx->tx_submit(tx);
   955		reinit_completion(&cqspi->rx_dma_complete);
   956	
   957		ret = dma_submit_error(cookie);
   958		if (ret) {
   959			dev_err(dev, "dma_submit_error %d\n", cookie);
   960			ret = -EIO;
   961			goto err_unmap;
   962		}
   963	
   964		dma_async_issue_pending(cqspi->rx_chan);
   965		if (!wait_for_completion_timeout(&cqspi->rx_dma_complete,
 > 966						 msecs_to_jiffies(max(len, 500UL)))) {
   967			dmaengine_terminate_sync(cqspi->rx_chan);
   968			dev_err(dev, "DMA wait_for_completion_timeout\n");
   969			ret = -ETIMEDOUT;
   970			goto err_unmap;
   971		}
   972	
   973	err_unmap:
   974		dma_unmap_single(ddev, dma_dst, len, DMA_FROM_DEVICE);
   975	
   976		return ret;
   977	}
   978	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32412 bytes --]

Re: [PATCH 6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads

[kernel test robot]

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

Hi Pratyush,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on spi/for-next]
[also build test WARNING on v5.11-rc1 next-20201223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: arm-randconfig-r006-20201221 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cee1e7d14f4628d6174b33640d502bff3b54ae45)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/04a7bcbc449363e5d6f498376c69116567b49d7d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
        git checkout 04a7bcbc449363e5d6f498376c69116567b49d7d
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/spi/spi-cadence-quadspi.c:966:24: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (500UL) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
                                            msecs_to_jiffies(max(len, 500UL)))) {
                                                             ^~~~~~~~~~~~~~~
   include/linux/minmax.h:58:19: note: expanded from macro 'max'
   #define max(x, y)       __careful_cmp(x, y, >)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:42:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:32:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:18:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
   1 warning generated.


vim +966 drivers/spi/spi-cadence-quadspi.c

   919	
   920	static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
   921					     u_char *buf, loff_t from, size_t len)
   922	{
   923		struct cqspi_st *cqspi = f_pdata->cqspi;
   924		struct device *dev = &cqspi->pdev->dev;
   925		enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
   926		dma_addr_t dma_src = (dma_addr_t)cqspi->mmap_phys_base + from;
   927		int ret = 0;
   928		struct dma_async_tx_descriptor *tx;
   929		dma_cookie_t cookie;
   930		dma_addr_t dma_dst;
   931		struct device *ddev;
   932	
   933		if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
   934			memcpy_fromio(buf, cqspi->ahb_base + from, len);
   935			return 0;
   936		}
   937	
   938		ddev = cqspi->rx_chan->device->dev;
   939		dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
   940		if (dma_mapping_error(ddev, dma_dst)) {
   941			dev_err(dev, "dma mapping failed\n");
   942			return -ENOMEM;
   943		}
   944		tx = dmaengine_prep_dma_memcpy(cqspi->rx_chan, dma_dst, dma_src,
   945					       len, flags);
   946		if (!tx) {
   947			dev_err(dev, "device_prep_dma_memcpy error\n");
   948			ret = -EIO;
   949			goto err_unmap;
   950		}
   951	
   952		tx->callback = cqspi_rx_dma_callback;
   953		tx->callback_param = cqspi;
   954		cookie = tx->tx_submit(tx);
   955		reinit_completion(&cqspi->rx_dma_complete);
   956	
   957		ret = dma_submit_error(cookie);
   958		if (ret) {
   959			dev_err(dev, "dma_submit_error %d\n", cookie);
   960			ret = -EIO;
   961			goto err_unmap;
   962		}
   963	
   964		dma_async_issue_pending(cqspi->rx_chan);
   965		if (!wait_for_completion_timeout(&cqspi->rx_dma_complete,
 > 966						 msecs_to_jiffies(max(len, 500UL)))) {
   967			dmaengine_terminate_sync(cqspi->rx_chan);
   968			dev_err(dev, "DMA wait_for_completion_timeout\n");
   969			ret = -ETIMEDOUT;
   970			goto err_unmap;
   971		}
   972	
   973	err_unmap:
   974		dma_unmap_single(ddev, dma_dst, len, DMA_FROM_DEVICE);
   975	
   976		return ret;
   977	}
   978	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32412 bytes --]

Re: [PATCH 6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads

[Pratyush Yadav]

On 29/12/20 11:29AM, kernel test robot wrote:
> Hi Pratyush,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on spi/for-next]
> [also build test WARNING on v5.11-rc1 next-20201223]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> config: arm-randconfig-r006-20201221 (attached as .config)
> compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cee1e7d14f4628d6174b33640d502bff3b54ae45)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install arm cross compiling tool for clang build
>         # apt-get install binutils-arm-linux-gnueabi
>         # https://github.com/0day-ci/linux/commit/04a7bcbc449363e5d6f498376c69116567b49d7d
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
>         git checkout 04a7bcbc449363e5d6f498376c69116567b49d7d
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
> >> drivers/spi/spi-cadence-quadspi.c:966:24: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (500UL) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
>                                             msecs_to_jiffies(max(len, 500UL)))) {
>                                                              ^~~~~~~~~~~~~~~
>    include/linux/minmax.h:58:19: note: expanded from macro 'max'
>    #define max(x, y)       __careful_cmp(x, y, >)
>                            ^~~~~~~~~~~~~~~~~~~~~~
>    include/linux/minmax.h:42:24: note: expanded from macro '__careful_cmp'
>            __builtin_choose_expr(__safe_cmp(x, y), \
>                                  ^~~~~~~~~~~~~~~~
>    include/linux/minmax.h:32:4: note: expanded from macro '__safe_cmp'
>                    (__typecheck(x, y) && __no_side_effects(x, y))
>                     ^~~~~~~~~~~~~~~~~
>    include/linux/minmax.h:18:28: note: expanded from macro '__typecheck'
>            (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>                       ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
>    1 warning generated.

On arm64 size_t is defined as unsigned long and on arm is it defined as 
unsigned int. So using 500U would generate the same warning on 64-bit 
platforms. Maybe the fix is to do something like: max(len, (size_t)500). 
Any better ideas?

> 
> 
> vim +966 drivers/spi/spi-cadence-quadspi.c
> 
>    919	
>    920	static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>    921					     u_char *buf, loff_t from, size_t len)
>    922	{
>    923		struct cqspi_st *cqspi = f_pdata->cqspi;
>    924		struct device *dev = &cqspi->pdev->dev;
>    925		enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
>    926		dma_addr_t dma_src = (dma_addr_t)cqspi->mmap_phys_base + from;
>    927		int ret = 0;
>    928		struct dma_async_tx_descriptor *tx;
>    929		dma_cookie_t cookie;
>    930		dma_addr_t dma_dst;
>    931		struct device *ddev;
>    932	
>    933		if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
>    934			memcpy_fromio(buf, cqspi->ahb_base + from, len);
>    935			return 0;
>    936		}
>    937	
>    938		ddev = cqspi->rx_chan->device->dev;
>    939		dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
>    940		if (dma_mapping_error(ddev, dma_dst)) {
>    941			dev_err(dev, "dma mapping failed\n");
>    942			return -ENOMEM;
>    943		}
>    944		tx = dmaengine_prep_dma_memcpy(cqspi->rx_chan, dma_dst, dma_src,
>    945					       len, flags);
>    946		if (!tx) {
>    947			dev_err(dev, "device_prep_dma_memcpy error\n");
>    948			ret = -EIO;
>    949			goto err_unmap;
>    950		}
>    951	
>    952		tx->callback = cqspi_rx_dma_callback;
>    953		tx->callback_param = cqspi;
>    954		cookie = tx->tx_submit(tx);
>    955		reinit_completion(&cqspi->rx_dma_complete);
>    956	
>    957		ret = dma_submit_error(cookie);
>    958		if (ret) {
>    959			dev_err(dev, "dma_submit_error %d\n", cookie);
>    960			ret = -EIO;
>    961			goto err_unmap;
>    962		}
>    963	
>    964		dma_async_issue_pending(cqspi->rx_chan);
>    965		if (!wait_for_completion_timeout(&cqspi->rx_dma_complete,
>  > 966						 msecs_to_jiffies(max(len, 500UL)))) {
>    967			dmaengine_terminate_sync(cqspi->rx_chan);
>    968			dev_err(dev, "DMA wait_for_completion_timeout\n");
>    969			ret = -ETIMEDOUT;
>    970			goto err_unmap;
>    971		}
>    972	
>    973	err_unmap:
>    974		dma_unmap_single(ddev, dma_dst, len, DMA_FROM_DEVICE);
>    975	
>    976		return ret;
>    977	}
>    978	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org



-- 
Regards,
Pratyush Yadav
Texas Instruments India

Re: [PATCH 6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads

[Pratyush Yadav]

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

On 29/12/20 11:29AM, kernel test robot wrote:
> Hi Pratyush,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on spi/for-next]
> [also build test WARNING on v5.11-rc1 next-20201223]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> config: arm-randconfig-r006-20201221 (attached as .config)
> compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cee1e7d14f4628d6174b33640d502bff3b54ae45)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install arm cross compiling tool for clang build
>         # apt-get install binutils-arm-linux-gnueabi
>         # https://github.com/0day-ci/linux/commit/04a7bcbc449363e5d6f498376c69116567b49d7d
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
>         git checkout 04a7bcbc449363e5d6f498376c69116567b49d7d
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
> >> drivers/spi/spi-cadence-quadspi.c:966:24: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (500UL) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
>                                             msecs_to_jiffies(max(len, 500UL)))) {
>                                                              ^~~~~~~~~~~~~~~
>    include/linux/minmax.h:58:19: note: expanded from macro 'max'
>    #define max(x, y)       __careful_cmp(x, y, >)
>                            ^~~~~~~~~~~~~~~~~~~~~~
>    include/linux/minmax.h:42:24: note: expanded from macro '__careful_cmp'
>            __builtin_choose_expr(__safe_cmp(x, y), \
>                                  ^~~~~~~~~~~~~~~~
>    include/linux/minmax.h:32:4: note: expanded from macro '__safe_cmp'
>                    (__typecheck(x, y) && __no_side_effects(x, y))
>                     ^~~~~~~~~~~~~~~~~
>    include/linux/minmax.h:18:28: note: expanded from macro '__typecheck'
>            (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>                       ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
>    1 warning generated.

On arm64 size_t is defined as unsigned long and on arm is it defined as 
unsigned int. So using 500U would generate the same warning on 64-bit 
platforms. Maybe the fix is to do something like: max(len, (size_t)500). 
Any better ideas?

> 
> 
> vim +966 drivers/spi/spi-cadence-quadspi.c
> 
>    919	
>    920	static int cqspi_direct_read_execute(struct cqspi_flash_pdata *f_pdata,
>    921					     u_char *buf, loff_t from, size_t len)
>    922	{
>    923		struct cqspi_st *cqspi = f_pdata->cqspi;
>    924		struct device *dev = &cqspi->pdev->dev;
>    925		enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
>    926		dma_addr_t dma_src = (dma_addr_t)cqspi->mmap_phys_base + from;
>    927		int ret = 0;
>    928		struct dma_async_tx_descriptor *tx;
>    929		dma_cookie_t cookie;
>    930		dma_addr_t dma_dst;
>    931		struct device *ddev;
>    932	
>    933		if (!cqspi->rx_chan || !virt_addr_valid(buf)) {
>    934			memcpy_fromio(buf, cqspi->ahb_base + from, len);
>    935			return 0;
>    936		}
>    937	
>    938		ddev = cqspi->rx_chan->device->dev;
>    939		dma_dst = dma_map_single(ddev, buf, len, DMA_FROM_DEVICE);
>    940		if (dma_mapping_error(ddev, dma_dst)) {
>    941			dev_err(dev, "dma mapping failed\n");
>    942			return -ENOMEM;
>    943		}
>    944		tx = dmaengine_prep_dma_memcpy(cqspi->rx_chan, dma_dst, dma_src,
>    945					       len, flags);
>    946		if (!tx) {
>    947			dev_err(dev, "device_prep_dma_memcpy error\n");
>    948			ret = -EIO;
>    949			goto err_unmap;
>    950		}
>    951	
>    952		tx->callback = cqspi_rx_dma_callback;
>    953		tx->callback_param = cqspi;
>    954		cookie = tx->tx_submit(tx);
>    955		reinit_completion(&cqspi->rx_dma_complete);
>    956	
>    957		ret = dma_submit_error(cookie);
>    958		if (ret) {
>    959			dev_err(dev, "dma_submit_error %d\n", cookie);
>    960			ret = -EIO;
>    961			goto err_unmap;
>    962		}
>    963	
>    964		dma_async_issue_pending(cqspi->rx_chan);
>    965		if (!wait_for_completion_timeout(&cqspi->rx_dma_complete,
>  > 966						 msecs_to_jiffies(max(len, 500UL)))) {
>    967			dmaengine_terminate_sync(cqspi->rx_chan);
>    968			dev_err(dev, "DMA wait_for_completion_timeout\n");
>    969			ret = -ETIMEDOUT;
>    970			goto err_unmap;
>    971		}
>    972	
>    973	err_unmap:
>    974		dma_unmap_single(ddev, dma_dst, len, DMA_FROM_DEVICE);
>    975	
>    976		return ret;
>    977	}
>    978	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org



-- 
Regards,
Pratyush Yadav
Texas Instruments India

Re: [PATCH 6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads

[Nick Desaulniers]

On Tue, Dec 29, 2020 at 1:18 AM 'Pratyush Yadav' via Clang Built Linux
<clang-built-linux@googlegroups.com> wrote:
>
> On 29/12/20 11:29AM, kernel test robot wrote:
> > Hi Pratyush,
> >
> > Thank you for the patch! Perhaps something to improve:
> >
> > [auto build test WARNING on spi/for-next]
> > [also build test WARNING on v5.11-rc1 next-20201223]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch]
> >
> > url:    https://github.com/0day-ci/linux/commits/Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> > config: arm-randconfig-r006-20201221 (attached as .config)
> > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cee1e7d14f4628d6174b33640d502bff3b54ae45)
> > reproduce (this is a W=1 build):
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # install arm cross compiling tool for clang build
> >         # apt-get install binutils-arm-linux-gnueabi
> >         # https://github.com/0day-ci/linux/commit/04a7bcbc449363e5d6f498376c69116567b49d7d
> >         git remote add linux-review https://github.com/0day-ci/linux
> >         git fetch --no-tags linux-review Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
> >         git checkout 04a7bcbc449363e5d6f498376c69116567b49d7d
> >         # save the attached .config to linux build tree
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All warnings (new ones prefixed by >>):
> >
> > >> drivers/spi/spi-cadence-quadspi.c:966:24: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (500UL) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
> >                                             msecs_to_jiffies(max(len, 500UL)))) {
> >                                                              ^~~~~~~~~~~~~~~
> >    include/linux/minmax.h:58:19: note: expanded from macro 'max'
> >    #define max(x, y)       __careful_cmp(x, y, >)
> >                            ^~~~~~~~~~~~~~~~~~~~~~
> >    include/linux/minmax.h:42:24: note: expanded from macro '__careful_cmp'
> >            __builtin_choose_expr(__safe_cmp(x, y), \
> >                                  ^~~~~~~~~~~~~~~~
> >    include/linux/minmax.h:32:4: note: expanded from macro '__safe_cmp'
> >                    (__typecheck(x, y) && __no_side_effects(x, y))
> >                     ^~~~~~~~~~~~~~~~~
> >    include/linux/minmax.h:18:28: note: expanded from macro '__typecheck'
> >            (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> >                       ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
> >    1 warning generated.
>
> On arm64 size_t is defined as unsigned long and on arm is it defined as
> unsigned int. So using 500U would generate the same warning on 64-bit
> platforms. Maybe the fix is to do something like: max(len, (size_t)500).
> Any better ideas?

SGTM

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads

[Nick Desaulniers]

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

On Tue, Dec 29, 2020 at 1:18 AM 'Pratyush Yadav' via Clang Built Linux
<clang-built-linux@googlegroups.com> wrote:
>
> On 29/12/20 11:29AM, kernel test robot wrote:
> > Hi Pratyush,
> >
> > Thank you for the patch! Perhaps something to improve:
> >
> > [auto build test WARNING on spi/for-next]
> > [also build test WARNING on v5.11-rc1 next-20201223]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch]
> >
> > url:    https://github.com/0day-ci/linux/commits/Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> > config: arm-randconfig-r006-20201221 (attached as .config)
> > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cee1e7d14f4628d6174b33640d502bff3b54ae45)
> > reproduce (this is a W=1 build):
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # install arm cross compiling tool for clang build
> >         # apt-get install binutils-arm-linux-gnueabi
> >         # https://github.com/0day-ci/linux/commit/04a7bcbc449363e5d6f498376c69116567b49d7d
> >         git remote add linux-review https://github.com/0day-ci/linux
> >         git fetch --no-tags linux-review Pratyush-Yadav/spi-cadence-quadspi-Add-Octal-DTR-support/20201223-025328
> >         git checkout 04a7bcbc449363e5d6f498376c69116567b49d7d
> >         # save the attached .config to linux build tree
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All warnings (new ones prefixed by >>):
> >
> > >> drivers/spi/spi-cadence-quadspi.c:966:24: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (500UL) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
> >                                             msecs_to_jiffies(max(len, 500UL)))) {
> >                                                              ^~~~~~~~~~~~~~~
> >    include/linux/minmax.h:58:19: note: expanded from macro 'max'
> >    #define max(x, y)       __careful_cmp(x, y, >)
> >                            ^~~~~~~~~~~~~~~~~~~~~~
> >    include/linux/minmax.h:42:24: note: expanded from macro '__careful_cmp'
> >            __builtin_choose_expr(__safe_cmp(x, y), \
> >                                  ^~~~~~~~~~~~~~~~
> >    include/linux/minmax.h:32:4: note: expanded from macro '__safe_cmp'
> >                    (__typecheck(x, y) && __no_side_effects(x, y))
> >                     ^~~~~~~~~~~~~~~~~
> >    include/linux/minmax.h:18:28: note: expanded from macro '__typecheck'
> >            (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> >                       ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
> >    1 warning generated.
>
> On arm64 size_t is defined as unsigned long and on arm is it defined as
> unsigned int. So using 500U would generate the same warning on 64-bit
> platforms. Maybe the fix is to do something like: max(len, (size_t)500).
> Any better ideas?

SGTM

-- 
Thanks,
~Nick Desaulniers

[PATCH 7/7] spi: cadence-quadspi: Add DTR support

[Pratyush Yadav]

Double Transfer Rate (DTR) mode transfers data twice per clock cycle.
Add support for parsing DTR ops and set up the registers to allow it.

Most SPI NOR flashes expect 2 byte commands. Parse the 2-byte opcode
from SPI MEM and set it up in the CQSPI_REG_OP_EXT_LOWER register.

Increment the delay needed before issuing indirect writes because larger
delay is needed for DTR mode. With the current delay some writes end up
missing.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
 drivers/spi/spi-cadence-quadspi.c | 281 +++++++++++++++++++++++++-----
 1 file changed, 242 insertions(+), 39 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 90040664e1b9..06a65e9a8a60 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -52,6 +52,7 @@ struct cqspi_flash_pdata {
 	u8		inst_width;
 	u8		addr_width;
 	u8		data_width;
+	bool		dtr;
 	u8		cs;
 };
 
@@ -111,6 +112,8 @@ struct cqspi_driver_platdata {
 #define CQSPI_REG_CONFIG_CHIPSELECT_LSB		10
 #define CQSPI_REG_CONFIG_DMA_MASK		BIT(15)
 #define CQSPI_REG_CONFIG_BAUD_LSB		19
+#define CQSPI_REG_CONFIG_DTR_PROTO		BIT(24)
+#define CQSPI_REG_CONFIG_DUAL_OPCODE		BIT(30)
 #define CQSPI_REG_CONFIG_IDLE_LSB		31
 #define CQSPI_REG_CONFIG_CHIPSELECT_MASK	0xF
 #define CQSPI_REG_CONFIG_BAUD_MASK		0xF
@@ -173,6 +176,9 @@ struct cqspi_driver_platdata {
 #define CQSPI_REG_SDRAMLEVEL_RD_MASK		0xFFFF
 #define CQSPI_REG_SDRAMLEVEL_WR_MASK		0xFFFF
 
+#define CQSPI_REG_WR_COMPLETION_CTRL		0x38
+#define CQSPI_REG_WR_DISABLE_AUTO_POLL		BIT(14)
+
 #define CQSPI_REG_IRQSTATUS			0x40
 #define CQSPI_REG_IRQMASK			0x44
 
@@ -216,6 +222,14 @@ struct cqspi_driver_platdata {
 #define CQSPI_REG_CMDWRITEDATALOWER		0xA8
 #define CQSPI_REG_CMDWRITEDATAUPPER		0xAC
 
+#define CQSPI_REG_POLLING_STATUS		0xB0
+#define CQSPI_REG_POLLING_STATUS_DUMMY_LSB	16
+
+#define CQSPI_REG_OP_EXT_LOWER			0xE0
+#define CQSPI_REG_OP_EXT_READ_LSB		24
+#define CQSPI_REG_OP_EXT_WRITE_LSB		16
+#define CQSPI_REG_OP_EXT_STIG_LSB		0
+
 /* Interrupt status bits */
 #define CQSPI_REG_IRQ_MODE_ERR			BIT(0)
 #define CQSPI_REG_IRQ_UNDERFLOW			BIT(1)
@@ -290,15 +304,80 @@ static unsigned int cqspi_calc_rdreg(struct cqspi_flash_pdata *f_pdata)
 	return rdreg;
 }
 
-static unsigned int cqspi_calc_dummy(const struct spi_mem_op *op)
+static unsigned int cqspi_calc_dummy(const struct spi_mem_op *op, bool dtr)
 {
 	unsigned int dummy_clk;
 
 	dummy_clk = op->dummy.nbytes * (8 / op->dummy.buswidth);
+	if (dtr)
+		dummy_clk /= 2;
 
 	return dummy_clk;
 }
 
+static int cqspi_set_protocol(struct cqspi_flash_pdata *f_pdata,
+			      const struct spi_mem_op *op)
+{
+	f_pdata->inst_width = CQSPI_INST_TYPE_SINGLE;
+	f_pdata->addr_width = CQSPI_INST_TYPE_SINGLE;
+	f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
+	f_pdata->dtr = op->data.dtr && op->cmd.dtr && op->addr.dtr;
+
+	switch (op->data.buswidth) {
+	case 0:
+		break;
+	case 1:
+		f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
+		break;
+	case 2:
+		f_pdata->data_width = CQSPI_INST_TYPE_DUAL;
+		break;
+	case 4:
+		f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
+		break;
+	case 8:
+		f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Right now we only support 8-8-8 DTR mode. */
+	if (f_pdata->dtr) {
+		switch (op->cmd.buswidth) {
+		case 0:
+			break;
+		case 8:
+			f_pdata->inst_width = CQSPI_INST_TYPE_OCTAL;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		switch (op->addr.buswidth) {
+		case 0:
+			break;
+		case 8:
+			f_pdata->addr_width = CQSPI_INST_TYPE_OCTAL;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		switch (op->data.buswidth) {
+		case 0:
+			break;
+		case 8:
+			f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int cqspi_wait_idle(struct cqspi_st *cqspi)
 {
 	const unsigned int poll_idle_retry = 3;
@@ -356,13 +435,69 @@ static int cqspi_exec_flash_cmd(struct cqspi_st *cqspi, unsigned int reg)
 	return cqspi_wait_idle(cqspi);
 }
 
+static int cqspi_setup_opcode_ext(struct cqspi_flash_pdata *f_pdata,
+				  const struct spi_mem_op *op,
+				  unsigned int shift)
+{
+	struct cqspi_st *cqspi = f_pdata->cqspi;
+	void __iomem *reg_base = cqspi->iobase;
+	unsigned int reg;
+	u8 ext;
+
+	if (op->cmd.nbytes != 2)
+		return -EINVAL;
+
+	/* Opcode extension is the LSB. */
+	ext = op->cmd.opcode & 0xff;
+
+	reg = readl(reg_base + CQSPI_REG_OP_EXT_LOWER);
+	reg &= ~(0xff << shift);
+	reg |= ext << shift;
+	writel(reg, reg_base + CQSPI_REG_OP_EXT_LOWER);
+
+	return 0;
+}
+
+static int cqspi_enable_dtr(struct cqspi_flash_pdata *f_pdata,
+			    const struct spi_mem_op *op, unsigned int shift,
+			    bool enable)
+{
+	struct cqspi_st *cqspi = f_pdata->cqspi;
+	void __iomem *reg_base = cqspi->iobase;
+	unsigned int reg;
+	int ret;
+
+	reg = readl(reg_base + CQSPI_REG_CONFIG);
+
+	/*
+	 * We enable dual byte opcode here. The callers have to set up the
+	 * extension opcode based on which type of operation it is.
+	 */
+	if (enable) {
+		reg |= CQSPI_REG_CONFIG_DTR_PROTO;
+		reg |= CQSPI_REG_CONFIG_DUAL_OPCODE;
+
+		/* Set up command opcode extension. */
+		ret = cqspi_setup_opcode_ext(f_pdata, op, shift);
+		if (ret)
+			return ret;
+	} else {
+		reg &= ~CQSPI_REG_CONFIG_DTR_PROTO;
+		reg &= ~CQSPI_REG_CONFIG_DUAL_OPCODE;
+	}
+
+	writel(reg, reg_base + CQSPI_REG_CONFIG);
+
+	return cqspi_wait_idle(cqspi);
+}
+
 static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
 			      const struct spi_mem_op *op)
 {
 	struct cqspi_st *cqspi = f_pdata->cqspi;
 	void __iomem *reg_base = cqspi->iobase;
 	u8 *rxbuf = op->data.buf.in;
-	u8 opcode = op->cmd.opcode;
+	u8 opcode;
 	size_t n_rx = op->data.nbytes;
 	unsigned int rdreg;
 	unsigned int reg;
@@ -370,6 +505,15 @@ static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
 	size_t read_len;
 	int status;
 
+	status = cqspi_set_protocol(f_pdata, op);
+	if (status)
+		return status;
+
+	status = cqspi_enable_dtr(f_pdata, op, CQSPI_REG_OP_EXT_STIG_LSB,
+				  f_pdata->dtr);
+	if (status)
+		return status;
+
 	if (!n_rx || n_rx > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) {
 		dev_err(&cqspi->pdev->dev,
 			"Invalid input argument, len %zu rxbuf 0x%p\n",
@@ -377,12 +521,17 @@ static int cqspi_command_read(struct cqspi_flash_pdata *f_pdata,
 		return -EINVAL;
 	}
 
+	if (f_pdata->dtr)
+		opcode = op->cmd.opcode >> 8;
+	else
+		opcode = op->cmd.opcode;
+
 	reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;
 
 	rdreg = cqspi_calc_rdreg(f_pdata);
 	writel(rdreg, reg_base + CQSPI_REG_RD_INSTR);
 
-	dummy_clk = cqspi_calc_dummy(op);
+	dummy_clk = cqspi_calc_dummy(op, f_pdata->dtr);
 	if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
 		return -EOPNOTSUPP;
 
@@ -421,12 +570,22 @@ static int cqspi_command_write(struct cqspi_flash_pdata *f_pdata,
 {
 	struct cqspi_st *cqspi = f_pdata->cqspi;
 	void __iomem *reg_base = cqspi->iobase;
-	const u8 opcode = op->cmd.opcode;
+	u8 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;
+
+	ret = cqspi_set_protocol(f_pdata, op);
+	if (ret)
+		return ret;
+
+	ret = cqspi_enable_dtr(f_pdata, op, CQSPI_REG_OP_EXT_STIG_LSB,
+			       f_pdata->dtr);
+	if (ret)
+		return ret;
 
 	if (n_tx > CQSPI_STIG_DATA_LEN_MAX || (n_tx && !txbuf)) {
 		dev_err(&cqspi->pdev->dev,
@@ -435,6 +594,14 @@ static int cqspi_command_write(struct cqspi_flash_pdata *f_pdata,
 		return -EINVAL;
 	}
 
+	reg = cqspi_calc_rdreg(f_pdata);
+	writel(reg, reg_base + CQSPI_REG_RD_INSTR);
+
+	if (f_pdata->dtr)
+		opcode = op->cmd.opcode >> 8;
+	else
+		opcode = op->cmd.opcode;
+
 	reg = opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB;
 
 	if (op->addr.nbytes) {
@@ -474,12 +641,24 @@ static int cqspi_read_setup(struct cqspi_flash_pdata *f_pdata,
 	void __iomem *reg_base = cqspi->iobase;
 	unsigned int dummy_clk = 0;
 	unsigned int reg;
+	int ret;
+	u8 opcode;
 
-	reg = op->cmd.opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB;
+	ret = cqspi_enable_dtr(f_pdata, op, CQSPI_REG_OP_EXT_READ_LSB,
+			       f_pdata->dtr);
+	if (ret)
+		return ret;
+
+	if (f_pdata->dtr)
+		opcode = op->cmd.opcode >> 8;
+	else
+		opcode = op->cmd.opcode;
+
+	reg = opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB;
 	reg |= cqspi_calc_rdreg(f_pdata);
 
 	/* Setup dummy clock cycles */
-	dummy_clk = cqspi_calc_dummy(op);
+	dummy_clk = cqspi_calc_dummy(op, f_pdata->dtr);
 
 	if (dummy_clk > CQSPI_DUMMY_CLKS_MAX)
 		return -EOPNOTSUPP;
@@ -594,15 +773,43 @@ static int cqspi_write_setup(struct cqspi_flash_pdata *f_pdata,
 			     const struct spi_mem_op *op)
 {
 	unsigned int reg;
+	int ret;
 	struct cqspi_st *cqspi = f_pdata->cqspi;
 	void __iomem *reg_base = cqspi->iobase;
+	u8 opcode;
+
+	ret = cqspi_enable_dtr(f_pdata, op, CQSPI_REG_OP_EXT_WRITE_LSB,
+			       f_pdata->dtr);
+	if (ret)
+		return ret;
+
+	if (f_pdata->dtr)
+		opcode = op->cmd.opcode >> 8;
+	else
+		opcode = op->cmd.opcode;
 
 	/* Set opcode. */
-	reg = op->cmd.opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
+	reg = opcode << CQSPI_REG_WR_INSTR_OPCODE_LSB;
+	reg |= f_pdata->data_width << CQSPI_REG_WR_INSTR_TYPE_DATA_LSB;
+	reg |= f_pdata->addr_width << CQSPI_REG_WR_INSTR_TYPE_ADDR_LSB;
 	writel(reg, reg_base + CQSPI_REG_WR_INSTR);
 	reg = cqspi_calc_rdreg(f_pdata);
 	writel(reg, reg_base + CQSPI_REG_RD_INSTR);
 
+	if (f_pdata->dtr) {
+		/*
+		 * Some flashes like the cypress Semper flash expect a 4-byte
+		 * dummy address with the Read SR command in DTR mode, but this
+		 * controller does not support sending address with the Read SR
+		 * command. So, disable write completion polling on the
+		 * controller's side. spi-nor will take care of polling the
+		 * status register.
+		 */
+		reg = readl(reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
+		reg |= CQSPI_REG_WR_DISABLE_AUTO_POLL;
+		writel(reg, reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
+	}
+
 	reg = readl(reg_base + CQSPI_REG_SIZE);
 	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
 	reg |= (op->addr.nbytes - 1);
@@ -856,35 +1063,6 @@ static void cqspi_configure(struct cqspi_flash_pdata *f_pdata,
 		cqspi_controller_enable(cqspi, 1);
 }
 
-static int cqspi_set_protocol(struct cqspi_flash_pdata *f_pdata,
-			      const struct spi_mem_op *op)
-{
-	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 (op->data.dir == SPI_MEM_DATA_IN) {
-		switch (op->data.buswidth) {
-		case 1:
-			f_pdata->data_width = CQSPI_INST_TYPE_SINGLE;
-			break;
-		case 2:
-			f_pdata->data_width = CQSPI_INST_TYPE_DUAL;
-			break;
-		case 4:
-			f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
-			break;
-		case 8:
-			f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
-			break;
-		default:
-			return -EINVAL;
-		}
-	}
-
-	return 0;
-}
-
 static ssize_t cqspi_write(struct cqspi_flash_pdata *f_pdata,
 			   const struct spi_mem_op *op)
 {
@@ -902,7 +1080,16 @@ static ssize_t cqspi_write(struct cqspi_flash_pdata *f_pdata,
 	if (ret)
 		return ret;
 
-	if (cqspi->use_direct_mode && ((to + len) <= cqspi->ahb_size)) {
+	/*
+	 * Some flashes like the Cypress Semper flash expect a dummy 4-byte
+	 * address (all 0s) with the read status register command in DTR mode.
+	 * But this controller does not support sending dummy address bytes to
+	 * the flash when it is polling the write completion register in DTR
+	 * mode. So, we can not use direct mode when in DTR mode for writing
+	 * data.
+	 */
+	if (!f_pdata->dtr && cqspi->use_direct_mode &&
+	    ((to + len) <= cqspi->ahb_size)) {
 		memcpy_toio(cqspi->ahb_base + to, buf, len);
 		return cqspi_wait_idle(cqspi);
 	}
@@ -1072,6 +1259,8 @@ static int cqspi_check_buswidth_req(struct spi_mem *mem, u8 buswidth, bool tx)
 static bool cqspi_supports_mem_op(struct spi_mem *mem,
 				  const struct spi_mem_op *op)
 {
+	bool all_true, all_false;
+
 	if (cqspi_check_buswidth_req(mem, op->cmd.buswidth, true))
 		return false;
 
@@ -1088,6 +1277,19 @@ static bool cqspi_supports_mem_op(struct spi_mem *mem,
 				     op->data.dir == SPI_MEM_DATA_OUT))
 		return false;
 
+	all_true = op->cmd.dtr && op->addr.dtr && op->dummy.dtr &&
+		   op->data.dtr;
+	all_false = !op->cmd.dtr && !op->addr.dtr && !op->dummy.dtr &&
+		    !op->data.dtr;
+
+	/* Mixed DTR modes not supported. */
+	if (!(all_true || all_false))
+		return false;
+
+	/* DTR mode opcodes should be 2 bytes. */
+	if (all_true && op->cmd.nbytes != 2)
+		return false;
+
 	return true;
 }
 
@@ -1365,10 +1567,10 @@ static int cqspi_probe(struct platform_device *pdev)
 	ddata  = of_device_get_match_data(dev);
 	if (ddata) {
 		if (ddata->quirks & CQSPI_NEEDS_WR_DELAY)
-			cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
+			cqspi->wr_delay = 50 * DIV_ROUND_UP(NSEC_PER_SEC,
 						cqspi->master_ref_clk_hz);
 		if (ddata->hwcaps_mask & CQSPI_SUPPORTS_OCTAL)
-			master->mode_bits |= SPI_RX_OCTAL;
+			master->mode_bits |= SPI_RX_OCTAL | SPI_TX_OCTAL;
 		if (!(ddata->quirks & CQSPI_DISABLE_DAC_MODE))
 			cqspi->use_direct_mode = true;
 	}
@@ -1510,3 +1712,4 @@ 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>");
+MODULE_AUTHOR("Pratyush Yadav <p.yadav@ti.com>");
-- 
2.28.0

Re: [PATCH 0/7] spi: cadence-quadspi: Add Octal DTR support

[Mark Brown]

On Wed, 23 Dec 2020 00:14:18 +0530, Pratyush Yadav wrote:
> This series adds support for Octal DTR mode now that SPI NOR supports
> these flashes. Patches 1/7 to 4/7 and 6/7 fix some minor bugs and issues.
> Patche 5/7 lays some groundwork by implementing the supports_op() hook.
> Patch 7/7 adds the Octal DTR mode support.
> 
> While the main aim of this series is to support 8D-8D-8D mode, other
> modes like 4D-4D-4D or 2S-2S-2S should also now be supported, though
> they have not been tested.
> 
> [...]

Applied to

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

Thanks!

[1/7] spi: cadence-quadspi: Set master max_speed_hz
      commit: 3a5c09c8d1ed9a7323f0e5c435021531f0865c16
[2/7] spi: cadence-quadspi: Abort read if dummy cycles required are too many
      commit: ceeda328edeeeeac7579e9dbf0610785a3b83d39
[3/7] spi: cadence-quadspi: Set dummy cycles from STIG commands
      commit: 888d517b992532df2b6115fbdc9620673ca7c651
[4/7] spi: cadence-quadspi: Fix dummy cycle calculation when buswidth > 1
      commit: 7512eaf54190e4cc9247f18439c008d44b15022c
[5/7] spi: cadence-quadspi: Implement a simple supports_op hook
      commit: a273596b9b50c76a9cc1f65d3eb7f8ab5c3eb3e3
[6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads
      commit: 0920a32cf6f20467aa133a47b776ee782daa889f
[7/7] spi: cadence-quadspi: Add DTR support
      commit: f453f293979fb648d2e505c132887811acb6bde6

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

Re: [PATCH 0/7] spi: cadence-quadspi: Add Octal DTR support

[Pratyush Yadav]

Hi Mark,

On 06/01/21 02:59PM, Mark Brown wrote:
> On Wed, 23 Dec 2020 00:14:18 +0530, Pratyush Yadav wrote:
> > This series adds support for Octal DTR mode now that SPI NOR supports
> > these flashes. Patches 1/7 to 4/7 and 6/7 fix some minor bugs and issues.
> > Patche 5/7 lays some groundwork by implementing the supports_op() hook.
> > Patch 7/7 adds the Octal DTR mode support.
> > 
> > While the main aim of this series is to support 8D-8D-8D mode, other
> > modes like 4D-4D-4D or 2S-2S-2S should also now be supported, though
> > they have not been tested.
> > 
> > [...]
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
> 
> Thanks!

:-)
 
> [1/7] spi: cadence-quadspi: Set master max_speed_hz
>       commit: 3a5c09c8d1ed9a7323f0e5c435021531f0865c16
> [2/7] spi: cadence-quadspi: Abort read if dummy cycles required are too many
>       commit: ceeda328edeeeeac7579e9dbf0610785a3b83d39
> [3/7] spi: cadence-quadspi: Set dummy cycles from STIG commands
>       commit: 888d517b992532df2b6115fbdc9620673ca7c651
> [4/7] spi: cadence-quadspi: Fix dummy cycle calculation when buswidth > 1
>       commit: 7512eaf54190e4cc9247f18439c008d44b15022c
> [5/7] spi: cadence-quadspi: Implement a simple supports_op hook
>       commit: a273596b9b50c76a9cc1f65d3eb7f8ab5c3eb3e3
> [6/7] spi: cadence-quadspi: Wait at least 500 ms for direct reads
>       commit: 0920a32cf6f20467aa133a47b776ee782daa889f

The kernel test robot reported some build warnings on this patch on 
32-bit platforms and I was planning on re-rolling it soon. Now that it 
is in your tree, I will send a follow-up patch to fix it.

> [7/7] spi: cadence-quadspi: Add DTR support
>       commit: f453f293979fb648d2e505c132887811acb6bde6
> 
> 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

-- 
Regards,
Pratyush Yadav
Texas Instruments India