[PATCH 00/14] spi: dw: Add support for DUAL/QUAD/OCTAL modes

[PATCH 00/14] spi: dw: Add support for DUAL/QUAD/OCTAL modes

[Sean Anderson]

This series adds support for enhanced SPI modes. It was tested on a K210 (DWC
SSI with QSPI flash).

If anyone has a designware device with QSPI flash attached (especially a DW SSI
APB device), I'd greatly appreciate them testing out this patch series.

Many of the earlier patches in this series are general fixups and can be split
off/merged separately if desired.


Sean Anderson (14):
  cmd: sf: Display errno on erase failure
  cmd: sf: Print error on test failure
  mtd: spi-nor-core: Fix typo in documentation
  mtd: spi-mem: Export spi_mem_default_supports_op
  spi: spi-mem: Add debug message for spi-mem ops
  spi: dw: Log status register on timeout
  spi: dw: Mask all possible interrupts
  spi: dw: Switch to capabilities
  spi: dw: Rewrite poll_transfer logic
  spi: dw: Add DUAL/QUAD/OCTAL caps
  spi: dw: Add registers necessary for DUAL/QUAD/OCTAL
  spi: dw: Support DUAL/QUAD/OCTAL
  spi: dw: Support clock stretching
  riscv: k210: Enable QSPI for spi3

 arch/riscv/dts/k210-maix-bit.dts |   2 +
 cmd/sf.c                         |  29 +-
 drivers/spi/designware_spi.c     | 652 +++++++++++++++++++++----------
 drivers/spi/spi-mem.c            |   6 +
 include/linux/mtd/spi-nor.h      |   2 +-
 include/spi-mem.h                |   3 +
 6 files changed, 476 insertions(+), 218 deletions(-)

-- 
2.29.2

[PATCH 01/14] cmd: sf: Display errno on erase failure

[Sean Anderson]

If there is an error while erasing SPI flash, no errno is displayed. This
makes it difficult to determine the cause of the error. This change mirrors
the logic for write errors above.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 cmd/sf.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/cmd/sf.c b/cmd/sf.c
index c0d6a8f8a0..de80fcd38b 100644
--- a/cmd/sf.c
+++ b/cmd/sf.c
@@ -344,8 +344,11 @@ static int do_spi_flash_erase(int argc, char *const argv[])
 	}
 
 	ret = spi_flash_erase(flash, offset, size);
-	printf("SF: %zu bytes @ %#x Erased: %s\n", (size_t)size, (u32)offset,
-	       ret ? "ERROR" : "OK");
+	printf("SF: %zu bytes @ %#x Erased: ", (size_t)size, (u32)offset);
+	if (ret)
+		printf("ERROR %d\n", ret);
+	else
+		printf("OK\n");
 
 	return ret == 0 ? 0 : 1;
 }
-- 
2.29.2

[PATCH 02/14] cmd: sf: Print error on test failure

[Sean Anderson]

The sf test command is used to test spi flashes (and spi masters). Printing
the exact error code is very helpful to those debugging the spi stack.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 cmd/sf.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/cmd/sf.c b/cmd/sf.c
index de80fcd38b..46346fb9d4 100644
--- a/cmd/sf.c
+++ b/cmd/sf.c
@@ -445,20 +445,22 @@ static int spi_flash_test(struct spi_flash *flash, uint8_t *buf, ulong len,
 			   ulong offset, uint8_t *vbuf)
 {
 	struct test_info test;
-	int i;
+	int err, i;
 
 	printf("SPI flash test:\n");
 	memset(&test, '\0', sizeof(test));
 	test.base_ms = get_timer(0);
 	test.bytes = len;
-	if (spi_flash_erase(flash, offset, len)) {
-		printf("Erase failed\n");
+	err = spi_flash_erase(flash, offset, len);
+	if (err) {
+		printf("Erase failed (err = %d)\n", err);
 		return -1;
 	}
 	spi_test_next_stage(&test);
 
-	if (spi_flash_read(flash, offset, len, vbuf)) {
-		printf("Check read failed\n");
+	err = spi_flash_read(flash, offset, len, vbuf);
+	if (err) {
+		printf("Check read failed (err = %d)\n", err);
 		return -1;
 	}
 	for (i = 0; i < len; i++) {
@@ -471,15 +473,17 @@ static int spi_flash_test(struct spi_flash *flash, uint8_t *buf, ulong len,
 	}
 	spi_test_next_stage(&test);
 
-	if (spi_flash_write(flash, offset, len, buf)) {
-		printf("Write failed\n");
+	err = spi_flash_write(flash, offset, len, buf);
+	if (err) {
+		printf("Write failed (err = %d)\n", err);
 		return -1;
 	}
 	memset(vbuf, '\0', len);
 	spi_test_next_stage(&test);
 
-	if (spi_flash_read(flash, offset, len, vbuf)) {
-		printf("Read failed\n");
+	err = spi_flash_read(flash, offset, len, vbuf);
+	if (err) {
+		printf("Read failed (ret = %d)\n", err);
 		return -1;
 	}
 	spi_test_next_stage(&test);
-- 
2.29.2

[PATCH 03/14] mtd: spi-nor-core: Fix typo in documentation

[Sean Anderson]

This line should come before the docs for the next function.

Fixes: 7aeedac0153 ("mtd: spi: Port SPI NOR framework from Linux")

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 include/linux/mtd/spi-nor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 4a8e19ee4f..39ef872ebf 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -302,8 +302,8 @@ struct spi_flash {
  * @flash_lock:		[FLASH-SPECIFIC] lock a region of the SPI NOR
  * @flash_unlock:	[FLASH-SPECIFIC] unlock a region of the SPI NOR
  * @flash_is_locked:	[FLASH-SPECIFIC] check if a region of the SPI NOR is
- * @quad_enable:	[FLASH-SPECIFIC] enables SPI NOR quad mode
  *			completely locked
+ * @quad_enable:	[FLASH-SPECIFIC] enables SPI NOR quad mode
  * @priv:		the private data
  */
 struct spi_nor {
-- 
2.29.2

[PATCH 04/14] mtd: spi-mem: Export spi_mem_default_supports_op

[Sean Anderson]

This is useful for extending the default functionality.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 include/spi-mem.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/spi-mem.h b/include/spi-mem.h
index ca0f55c8fd..1dd556b6cf 100644
--- a/include/spi-mem.h
+++ b/include/spi-mem.h
@@ -236,6 +236,9 @@ spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
 
 int spi_mem_adjust_op_size(struct spi_slave *slave, struct spi_mem_op *op);
 
+bool spi_mem_default_supports_op(struct spi_slave *slave,
+				 const struct spi_mem_op *op);
+
 bool spi_mem_supports_op(struct spi_slave *slave, const struct spi_mem_op *op);
 
 int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op);
-- 
2.29.2

[PATCH 05/14] spi: spi-mem: Add debug message for spi-mem ops

[Sean Anderson]

This prints some basic metadata about the SPI memory op. This information
may be used to debug SPI drivers (e.g. determining the expected SPI mode).
It is also helpful for verifying that the data on the wire matches the data
intended to be transmitted (e.g. with a logic analyzer). The opcode is
printed with a format of %02Xh to match the notation commonly used in flash
datasheets.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/spi-mem.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index c095ae9505..eb83c11910 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -220,6 +220,12 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 	int ret;
 	int i;
 
+	dev_dbg(slave->dev, "exec %02Xh %u-%u-%u addr=%llx wait=%u len=%u\n",
+		op->cmd.opcode, op->cmd.buswidth, op->addr.buswidth,
+		op->data.buswidth, op->addr.val,
+		op->dummy.buswidth ? op->dummy.nbytes * 8 / op->dummy.buswidth : 0,
+		op->data.nbytes);
+
 	if (!spi_mem_supports_op(slave, op))
 		return -ENOTSUPP;
 
-- 
2.29.2

[PATCH 06/14] spi: dw: Log status register on timeout

[Sean Anderson]

This logs the status register on timeout, so it is easier to determine the
cause of the failure.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/designware_spi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 742121140d..519d6e32bd 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -552,6 +552,7 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
 	if (readl_poll_timeout(priv->regs + DW_SPI_SR, val,
 			       (val & SR_TF_EMPT) && !(val & SR_BUSY),
 			       RX_TIMEOUT * 1000)) {
+		dev_dbg(bus, "timed out; sr=%x\n", dw_read(priv, DW_SPI_SR));
 		ret = -ETIMEDOUT;
 	}
 
@@ -639,6 +640,8 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 		if (readl_poll_timeout(priv->regs + DW_SPI_SR, val,
 				       (val & SR_TF_EMPT) && !(val & SR_BUSY),
 				       RX_TIMEOUT * 1000)) {
+			dev_dbg(bus, "timed out; sr=%x\n",
+				dw_read(priv, DW_SPI_SR));
 			ret = -ETIMEDOUT;
 		}
 	}
-- 
2.29.2

[PATCH 07/14] spi: dw: Mask all possible interrupts

[Sean Anderson]

DWC_SPI has more than 8 interrupts. Write to the whole IMR to mask
everything.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/designware_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 519d6e32bd..29ec1503fd 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -252,7 +252,7 @@ static int dw_spi_of_to_plat(struct udevice *bus)
 static void spi_hw_init(struct udevice *bus, struct dw_spi_priv *priv)
 {
 	dw_write(priv, DW_SPI_SSIENR, 0);
-	dw_write(priv, DW_SPI_IMR, 0xff);
+	dw_write(priv, DW_SPI_IMR, 0xFFFFFFFF);
 	dw_write(priv, DW_SPI_SSIENR, 1);
 
 	/*
-- 
2.29.2

[PATCH 08/14] spi: dw: Switch to capabilities

[Sean Anderson]

Since Linux commit cc760f3143f5 ("spi: dw: Convert CS-override to DW SPI
capabilities"), the Linux driver has used capability flags instead of
using ad-hoc flags and functions. This is a great idea, and we should use
it as well.

The .data field in the compatible array has switched from being an
initialization function to being a set of default capabilities. This is
necessary since some capabilities cannot be determined at runtime.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/designware_spi.c | 141 +++++++++++++++++------------------
 1 file changed, 69 insertions(+), 72 deletions(-)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 29ec1503fd..9a30a677c8 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -123,9 +123,13 @@ struct dw_spi_priv {
 	struct reset_ctl_bulk resets;
 	struct gpio_desc cs_gpio;	/* External chip-select gpio */
 
-	u32 (*update_cr0)(struct dw_spi_priv *priv);
-
 	void __iomem *regs;
+/* DW SPI capabilities */
+#define DW_SPI_CAP_CS_OVERRIDE		BIT(0) /* Unimplemented */
+#define DW_SPI_CAP_KEEMBAY_MST		BIT(1) /* Unimplemented */
+#define DW_SPI_CAP_DWC_SSI		BIT(2)
+#define DW_SPI_CAP_DFS32		BIT(3)
+	unsigned long caps;
 	unsigned long bus_clk_rate;
 	unsigned int freq;		/* Default frequency */
 	unsigned int mode;
@@ -135,7 +139,6 @@ struct dw_spi_priv {
 	void *rx;
 	void *rx_end;
 	u32 fifo_len;			/* depth of the FIFO buffer */
-	u32 max_xfer;			/* Maximum transfer size (in bits) */
 
 	int bits_per_word;
 	int len;
@@ -154,51 +157,30 @@ static inline void dw_write(struct dw_spi_priv *priv, u32 offset, u32 val)
 	__raw_writel(val, priv->regs + offset);
 }
 
-static u32 dw_spi_dw16_update_cr0(struct dw_spi_priv *priv)
+static u32 dw_spi_update_cr0(struct dw_spi_priv *priv)
 {
-	return FIELD_PREP(CTRLR0_DFS_MASK, priv->bits_per_word - 1)
-	     | FIELD_PREP(CTRLR0_FRF_MASK, priv->type)
-	     | FIELD_PREP(CTRLR0_MODE_MASK, priv->mode)
-	     | FIELD_PREP(CTRLR0_TMOD_MASK, priv->tmode);
-}
+	u32 cr0;
 
-static u32 dw_spi_dw32_update_cr0(struct dw_spi_priv *priv)
-{
-	return FIELD_PREP(CTRLR0_DFS_32_MASK, priv->bits_per_word - 1)
-	     | FIELD_PREP(CTRLR0_FRF_MASK, priv->type)
-	     | FIELD_PREP(CTRLR0_MODE_MASK, priv->mode)
-	     | FIELD_PREP(CTRLR0_TMOD_MASK, priv->tmode);
-}
-
-static u32 dw_spi_dwc_update_cr0(struct dw_spi_priv *priv)
-{
-	return FIELD_PREP(DWC_SSI_CTRLR0_DFS_MASK, priv->bits_per_word - 1)
-	     | FIELD_PREP(DWC_SSI_CTRLR0_FRF_MASK, priv->type)
-	     | FIELD_PREP(DWC_SSI_CTRLR0_MODE_MASK, priv->mode)
-	     | FIELD_PREP(DWC_SSI_CTRLR0_TMOD_MASK, priv->tmode);
-}
-
-static int dw_spi_apb_init(struct udevice *bus, struct dw_spi_priv *priv)
-{
-	/* If we read zeros from DFS, then we need to use DFS_32 instead */
-	dw_write(priv, DW_SPI_SSIENR, 0);
-	dw_write(priv, DW_SPI_CTRLR0, 0xffffffff);
-	if (FIELD_GET(CTRLR0_DFS_MASK, dw_read(priv, DW_SPI_CTRLR0))) {
-		priv->max_xfer = 16;
-		priv->update_cr0 = dw_spi_dw16_update_cr0;
+	if (priv->caps & DW_SPI_CAP_DWC_SSI) {
+		cr0 = FIELD_PREP(DWC_SSI_CTRLR0_DFS_MASK,
+				 priv->bits_per_word - 1)
+		    | FIELD_PREP(DWC_SSI_CTRLR0_FRF_MASK, priv->type)
+		    | FIELD_PREP(DWC_SSI_CTRLR0_MODE_MASK, priv->mode)
+		    | FIELD_PREP(DWC_SSI_CTRLR0_TMOD_MASK, priv->tmode);
 	} else {
-		priv->max_xfer = 32;
-		priv->update_cr0 = dw_spi_dw32_update_cr0;
+		if (priv->caps & DW_SPI_CAP_DFS32)
+			cr0 = FIELD_PREP(CTRLR0_DFS_32_MASK,
+					 priv->bits_per_word - 1);
+		else
+			cr0 = FIELD_PREP(CTRLR0_DFS_MASK,
+					 priv->bits_per_word - 1);
+
+		cr0 |= FIELD_PREP(CTRLR0_FRF_MASK, priv->type)
+		    |  FIELD_PREP(CTRLR0_MODE_MASK, priv->mode)
+		    |  FIELD_PREP(CTRLR0_TMOD_MASK, priv->tmode);
 	}
 
-	return 0;
-}
-
-static int dw_spi_dwc_init(struct udevice *bus, struct dw_spi_priv *priv)
-{
-	priv->max_xfer = 32;
-	priv->update_cr0 = dw_spi_dwc_update_cr0;
-	return 0;
+	return cr0;
 }
 
 static int request_gpio_cs(struct udevice *bus)
@@ -251,8 +233,26 @@ static int dw_spi_of_to_plat(struct udevice *bus)
 /* Restart the controller, disable all interrupts, clean rx fifo */
 static void spi_hw_init(struct udevice *bus, struct dw_spi_priv *priv)
 {
+	u32 cr0;
+
 	dw_write(priv, DW_SPI_SSIENR, 0);
 	dw_write(priv, DW_SPI_IMR, 0xFFFFFFFF);
+
+	/*
+	 * Detect features by writing CTRLR0 and seeing which fields remain
+	 * zeroed.
+	 */
+	dw_write(priv, DW_SPI_SSIENR, 0);
+	dw_write(priv, DW_SPI_CTRLR0, 0xffffffff);
+	cr0 = dw_read(priv, DW_SPI_CTRLR0);
+
+	/*
+	 * DWC_SPI always has DFS_32. If we read zeros from DFS, then we need to
+	 * use DFS_32 instead
+	 */
+	if (priv->caps & DW_SPI_CAP_DWC_SSI || !FIELD_GET(CTRLR0_DFS_MASK, cr0))
+		priv->caps |= DW_SPI_CAP_DFS32;
+
 	dw_write(priv, DW_SPI_SSIENR, 1);
 
 	/*
@@ -337,11 +337,8 @@ static int dw_spi_reset(struct udevice *bus)
 	return 0;
 }
 
-typedef int (*dw_spi_init_t)(struct udevice *bus, struct dw_spi_priv *priv);
-
 static int dw_spi_probe(struct udevice *bus)
 {
-	dw_spi_init_t init = (dw_spi_init_t)dev_get_driver_data(bus);
 	struct dw_spi_plat *plat = dev_get_plat(bus);
 	struct dw_spi_priv *priv = dev_get_priv(bus);
 	int ret;
@@ -358,16 +355,13 @@ static int dw_spi_probe(struct udevice *bus)
 	if (ret)
 		return ret;
 
-	if (!init)
-		return -EINVAL;
-	ret = init(bus, priv);
-	if (ret)
-		return ret;
+	priv->caps = dev_get_driver_data(bus);
+	dw_spi_detect_caps(bus, priv);
 
 	version = dw_read(priv, DW_SPI_VERSION);
 	dev_dbg(bus, "ssi_version_id=%c.%c%c%c ssi_max_xfer_size=%u\n",
 		version >> 24, version >> 16, version >> 8, version,
-		priv->max_xfer);
+		priv->caps & DW_SPI_CAP_DFS32 ? 32 : 16);
 
 	/* Currently only bits_per_word == 8 supported */
 	priv->bits_per_word = 8;
@@ -510,7 +504,7 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
 		 */
 		priv->tmode = CTRLR0_TMOD_TR;
 
-	cr0 = priv->update_cr0(priv);
+	cr0 = dw_spi_update_cr0(priv);
 
 	priv->len = bitlen >> 3;
 
@@ -582,7 +576,7 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 	else
 		priv->tmode = CTRLR0_TMOD_TO;
 
-	cr0 = priv->update_cr0(priv);
+	cr0 = dw_spi_update_cr0(priv);
 	dev_dbg(bus, "cr0=%08x buf=%p len=%u [bytes]\n", cr0, op->data.buf.in,
 		op->data.nbytes);
 
@@ -742,15 +736,15 @@ static const struct dm_spi_ops dw_spi_ops = {
 static const struct udevice_id dw_spi_ids[] = {
 	/* Generic compatible strings */
 
-	{ .compatible = "snps,dw-apb-ssi", .data = (ulong)dw_spi_apb_init },
-	{ .compatible = "snps,dw-apb-ssi-3.20a", .data = (ulong)dw_spi_apb_init },
-	{ .compatible = "snps,dw-apb-ssi-3.22a", .data = (ulong)dw_spi_apb_init },
+	{ .compatible = "snps,dw-apb-ssi" },
+	{ .compatible = "snps,dw-apb-ssi-3.20a" },
+	{ .compatible = "snps,dw-apb-ssi-3.22a" },
 	/* First version with SSI_MAX_XFER_SIZE */
-	{ .compatible = "snps,dw-apb-ssi-3.23a", .data = (ulong)dw_spi_apb_init },
-	/* First version with Dual/Quad SPI; unused by this driver */
-	{ .compatible = "snps,dw-apb-ssi-4.00a", .data = (ulong)dw_spi_apb_init },
-	{ .compatible = "snps,dw-apb-ssi-4.01", .data = (ulong)dw_spi_apb_init },
-	{ .compatible = "snps,dwc-ssi-1.01a", .data = (ulong)dw_spi_dwc_init },
+	{ .compatible = "snps,dw-apb-ssi-3.23a" },
+	/* First version with Dual/Quad SPI */
+	{ .compatible = "snps,dw-apb-ssi-4.00a" },
+	{ .compatible = "snps,dw-apb-ssi-4.01" },
+	{ .compatible = "snps,dwc-ssi-1.01a", .data = DW_SPI_CAP_DWC_SSI },
 
 	/* Compatible strings for specific SoCs */
 
@@ -759,16 +753,19 @@ static const struct udevice_id dw_spi_ids[] = {
 	 * version of this device. This compatible string is used for those
 	 * devices, and is not used for sofpgas in general.
 	 */
-	{ .compatible = "altr,socfpga-spi", .data = (ulong)dw_spi_apb_init },
-	{ .compatible = "altr,socfpga-arria10-spi", .data = (ulong)dw_spi_apb_init },
-	{ .compatible = "canaan,kendryte-k210-spi", .data = (ulong)dw_spi_apb_init },
-	{ .compatible = "canaan,kendryte-k210-ssi", .data = (ulong)dw_spi_dwc_init },
-	{ .compatible = "intel,stratix10-spi", .data = (ulong)dw_spi_apb_init },
-	{ .compatible = "intel,agilex-spi", .data = (ulong)dw_spi_apb_init },
-	{ .compatible = "mscc,ocelot-spi", .data = (ulong)dw_spi_apb_init },
-	{ .compatible = "mscc,jaguar2-spi", .data = (ulong)dw_spi_apb_init },
-	{ .compatible = "snps,axs10x-spi", .data = (ulong)dw_spi_apb_init },
-	{ .compatible = "snps,hsdk-spi", .data = (ulong)dw_spi_apb_init },
+	{ .compatible = "altr,socfpga-spi" },
+	{ .compatible = "altr,socfpga-arria10-spi" },
+	{ .compatible = "canaan,kendryte-k210-spi" },
+	{
+		.compatible = "canaan,kendryte-k210-ssi",
+		.data = DW_SPI_CAP_DWC_SSI,
+	},
+	{ .compatible = "intel,stratix10-spi" },
+	{ .compatible = "intel,agilex-spi" },
+	{ .compatible = "mscc,ocelot-spi" },
+	{ .compatible = "mscc,jaguar2-spi" },
+	{ .compatible = "snps,axs10x-spi" },
+	{ .compatible = "snps,hsdk-spi" },
 	{ }
 };
 
-- 
2.29.2

[PATCH 09/14] spi: dw: Rewrite poll_transfer logic

[Sean Anderson]

This rewrites poll_transfer, dw_writer, and dw_reader.

* We now use RO transfers (instead of always using TR). This eliminates the
  need to send out dummy words, and simplifies the transmit logic.
* All parameters (except regs and bits_per_word) are passed explicitly.
* Most parameters have been made explicit (instead of being recalculated on
  every loop).
* Transfers are measured in units of frames instead of bytes. This matches
  the measurements used by the device and eliminates several divisions by
  bits_per_word.
* We now check if we have over-/under-flowed the FIFO. This should help
  prevent hangs when the device stops the transfer and U-Boot doesn't
  realize it (and then waits for the remaining data forever). TXUI is not
  present in DW_APB_SSI, but in the worst case we just don't write data.

  Unfortunately, this doesn't seem to solve all problems, and there are
  some remaining bugs (such as some transfers containing all 1s or all 0s)
  when we have many fifo overflows. This is solved for DWC devices by
  enabling clock stretching.
* poll_transfer is now used by dw_spi_exec_op as well as xfer.
* There are separate inner loops for 8-, 16-, and 32-bit frame sizes. This
  should hopefully reduce the amount of over-/under-flows. However, I
  haven't done exhaustive testing to ensure this is really necessary. In
  particular, I was never able to prevent read overflows at 50MHz clock
  speed.

These changes should probably have been split up into several commits, but
several depend on each other, and it would be difficult to break this up
while preserving bisectability.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/designware_spi.c | 284 +++++++++++++++++++++--------------
 1 file changed, 171 insertions(+), 113 deletions(-)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 9a30a677c8..683394a5a4 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -134,14 +134,10 @@ struct dw_spi_priv {
 	unsigned int freq;		/* Default frequency */
 	unsigned int mode;
 
-	const void *tx;
-	const void *tx_end;
-	void *rx;
-	void *rx_end;
 	u32 fifo_len;			/* depth of the FIFO buffer */
 
 	int bits_per_word;
-	int len;
+	int frames;			/* Number of frames in the transfer */
 	u8 cs;				/* chip select pin */
 	u8 tmode;			/* TR/TO/RO/EEPROM */
 	u8 type;			/* SPI/SSP/MicroWire */
@@ -374,14 +370,24 @@ static int dw_spi_probe(struct udevice *bus)
 	return 0;
 }
 
-/* Return the max entries we can fill into tx fifo */
-static inline u32 tx_max(struct dw_spi_priv *priv)
+/**
+ * dw_writer() - Write data frames to the tx fifo
+ * @priv: Driver private info
+ * @tx: The tx buffer
+ * @idx: The number of data frames already transmitted
+ * @tx_frames: The number of data frames left to transmit
+ * @rx_frames: The number of data frames left to receive (0 if only
+ *             transmitting)
+ * @frame_bytes: The number of bytes taken up by one data frame
+ *
+ * This function writes up to @tx_frames data frames using data from @tx[@idx].
+ *
+ * Return: The number of frames read
+ */
+static uint dw_writer(struct dw_spi_priv *priv, const void *tx, uint idx,
+		      uint tx_frames, uint rx_frames, uint frame_bytes)
 {
-	u32 tx_left, tx_room, rxtx_gap;
-
-	tx_left = (priv->tx_end - priv->tx) / (priv->bits_per_word >> 3);
-	tx_room = priv->fifo_len - dw_read(priv, DW_SPI_TXFLR);
-
+	u32 tx_room = priv->fifo_len - dw_read(priv, DW_SPI_TXFLR);
 	/*
 	 * Another concern is about the tx/rx mismatch, we
 	 * thought about using (priv->fifo_len - rxflr - txflr) as
@@ -390,67 +396,131 @@ static inline u32 tx_max(struct dw_spi_priv *priv)
 	 * shift registers. So a control from sw point of
 	 * view is taken.
 	 */
-	rxtx_gap = ((priv->rx_end - priv->rx) - (priv->tx_end - priv->tx)) /
-		(priv->bits_per_word >> 3);
+	u32 rxtx_gap = rx_frames - tx_frames;
+	u32 count = min3(tx_frames, tx_room, (u32)(priv->fifo_len - rxtx_gap));
+	u32 *dr = priv->regs + DW_SPI_DR;
 
-	return min3(tx_left, tx_room, (u32)(priv->fifo_len - rxtx_gap));
-}
+	if (!count)
+		return 0;
 
-/* Return the max entries we should read out of rx fifo */
-static inline u32 rx_max(struct dw_spi_priv *priv)
-{
-	u32 rx_left = (priv->rx_end - priv->rx) / (priv->bits_per_word >> 3);
-
-	return min_t(u32, rx_left, dw_read(priv, DW_SPI_RXFLR));
-}
-
-static void dw_writer(struct dw_spi_priv *priv)
-{
-	u32 max = tx_max(priv);
-	u32 txw = 0xFFFFFFFF;
-
-	while (max--) {
-		/* Set the tx word if the transfer's original "tx" is not null */
-		if (priv->tx_end - priv->len) {
-			if (priv->bits_per_word == 8)
-				txw = *(u8 *)(priv->tx);
-			else
-				txw = *(u16 *)(priv->tx);
-		}
-		dw_write(priv, DW_SPI_DR, txw);
-		log_content("tx=0x%02x\n", txw);
-		priv->tx += priv->bits_per_word >> 3;
+#define do_write(type) do { \
+	type *start = ((type *)tx) + idx; \
+	type *end = start + count; \
+	do { \
+		__raw_writel(*start++, dr); \
+	} while (start < end); \
+} while (0)
+	switch (frame_bytes) {
+	case 1:
+		do_write(u8);
+		break;
+	case 2:
+		do_write(u16);
+		break;
+	case 3:
+	case 4:
+	default:
+		do_write(u32);
+		break;
 	}
+#undef do_write
+
+	return count;
 }
 
-static void dw_reader(struct dw_spi_priv *priv)
+/**
+ * dw_reader() - Read data frames from the rx fifo
+ * @priv: Driver private data
+ * @rx: The rx buffer
+ * @idx: The number of data frames already received
+ * @frames: The number of data frames left to receive
+ * @frame_bytes: The size of a data frame in bytes
+ *
+ * This function reads up to @frames data frames from @rx[@idx].
+ *
+ * Return: The number of frames read
+ */
+static uint dw_reader(struct dw_spi_priv *priv, void *rx, uint idx, uint frames,
+		      uint frame_bytes)
 {
-	u32 max = rx_max(priv);
-	u16 rxw;
+	u32 rx_lvl = dw_read(priv, DW_SPI_RXFLR);
+	u32 count = min(frames, rx_lvl);
+	u32 *dr = priv->regs + DW_SPI_DR;
 
-	while (max--) {
-		rxw = dw_read(priv, DW_SPI_DR);
-		log_content("rx=0x%02x\n", rxw);
+	if (!count)
+		return 0;
 
-		/* Care about rx if the transfer's original "rx" is not null */
-		if (priv->rx_end - priv->len) {
-			if (priv->bits_per_word == 8)
-				*(u8 *)(priv->rx) = rxw;
-			else
-				*(u16 *)(priv->rx) = rxw;
-		}
-		priv->rx += priv->bits_per_word >> 3;
+#define do_read(type) do { \
+	type *start = ((type *)rx) + idx; \
+	type *end = start + count; \
+	do { \
+		*start++ = __raw_readl(dr); \
+	} while (start < end); \
+} while (0)
+	switch (frame_bytes) {
+	case 1:
+		do_read(u8);
+		break;
+	case 2:
+		do_read(u16);
+		break;
+	case 3:
+	case 4:
+	default:
+		do_read(u32);
+		break;
 	}
+#undef do_read
+
+	return count;
 }
 
-static int poll_transfer(struct dw_spi_priv *priv)
+/**
+ * poll_transfer() - Transmit and receive data frames
+ * @priv: Driver private data
+ * @tx: The tx buffer. May be %NULL to only receive.
+ * @rx: The rx buffer. May be %NULL to discard read data.
+ * @frames: The number of data frames to transfer
+ *
+ * Transmit @tx, while recieving @rx.
+ *
+ * Return: The lesser of the number of frames transmitted or received.
+ */
+static uint poll_transfer(struct dw_spi_priv *priv, const void *tx, void *rx,
+			  uint frames)
 {
-	do {
-		dw_writer(priv);
-		dw_reader(priv);
-	} while (priv->rx_end > priv->rx);
+	uint frame_bytes = priv->bits_per_word >> 3;
+	uint tx_idx = 0;
+	uint rx_idx = 0;
+	uint tx_frames = tx ? frames : 0;
+	uint rx_frames = rx ? frames : 0;
 
-	return 0;
+	while (tx_frames || rx_frames) {
+		if (tx_frames) {
+			uint tx_diff = dw_writer(priv, tx, tx_idx, tx_frames,
+						 rx_frames, frame_bytes);
+
+			tx_idx += tx_diff;
+			tx_frames -= tx_diff;
+		}
+
+		if (rx_frames) {
+			uint rx_diff = dw_reader(priv, rx, rx_idx, rx_frames,
+						 frame_bytes);
+
+			rx_idx += rx_diff;
+			rx_frames -= rx_diff;
+		}
+
+		/*
+		 * If we don't read/write fast enough, the transfer stops.
+		 * Don't bother reading out what's left in the FIFO; it's
+		 * garbage.
+		 */
+		if (dw_read(priv, DW_SPI_RISR) & (ISR_RXOI | ISR_TXUI))
+			break;
+	}
+	return min(tx ? tx_idx : rx_idx, rx ? rx_idx : tx_idx);
 }
 
 /*
@@ -476,19 +546,21 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
 {
 	struct udevice *bus = dev->parent;
 	struct dw_spi_priv *priv = dev_get_priv(bus);
-	const u8 *tx = dout;
+	const void *tx = dout;
 	u8 *rx = din;
 	int ret = 0;
 	u32 cr0 = 0;
-	u32 val;
-	u32 cs;
+	u32 val, cs;
+	uint frames;
 
 	/* spi core configured to do 8 bit transfers */
-	if (bitlen % 8) {
+	if (bitlen % priv->bits_per_word) {
 		dev_err(dev, "Non byte aligned SPI transfer.\n");
 		return -1;
 	}
 
+	frames = bitlen / priv->bits_per_word;
+
 	/* Start the transaction if necessary. */
 	if (flags & SPI_XFER_BEGIN)
 		external_cs_manage(dev, false);
@@ -498,30 +570,22 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
 	else if (rx)
 		priv->tmode = CTRLR0_TMOD_RO;
 	else
-		/*
-		 * In transmit only mode (CTRL0_TMOD_TO) input FIFO never gets
-		 * any data which breaks our logic in poll_transfer() above.
-		 */
-		priv->tmode = CTRLR0_TMOD_TR;
+		priv->tmode = CTRLR0_TMOD_TO;
 
 	cr0 = dw_spi_update_cr0(priv);
 
-	priv->len = bitlen >> 3;
-
-	priv->tx = (void *)tx;
-	priv->tx_end = priv->tx + priv->len;
-	priv->rx = rx;
-	priv->rx_end = priv->rx + priv->len;
-
 	/* Disable controller before writing control registers */
 	dw_write(priv, DW_SPI_SSIENR, 0);
 
-	dev_dbg(dev, "cr0=%08x rx=%p tx=%p len=%d [bytes]\n", cr0, rx, tx,
-		priv->len);
+	dev_dbg(dev, "cr0=%08x rx=%p tx=%p frames=%d\n", cr0, rx, tx,
+		frames);
 	/* Reprogram cr0 only if changed */
 	if (dw_read(priv, DW_SPI_CTRLR0) != cr0)
 		dw_write(priv, DW_SPI_CTRLR0, cr0);
 
+	if (rx)
+		dw_write(priv, DW_SPI_CTRLR1, frames - 1);
+
 	/*
 	 * Configure the desired SS (slave select 0...3) in the controller
 	 * The DW SPI controller will activate and deactivate this CS
@@ -533,8 +597,15 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
 	/* Enable controller after writing control registers */
 	dw_write(priv, DW_SPI_SSIENR, 1);
 
+	/*
+	 * Prime the pump. RO-mode doesn't work unless something gets written to
+	 * the FIFO
+	 */
+	if (rx && !tx)
+		dw_write(priv, DW_SPI_DR, 0xFFFFFFFF);
+
 	/* Start transfer in a polling loop */
-	ret = poll_transfer(priv);
+	poll_transfer(priv, tx, rx, frames);
 
 	/*
 	 * Wait for current transmit operation to complete.
@@ -567,9 +638,10 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 	int pos, i, ret = 0;
 	struct udevice *bus = slave->dev->parent;
 	struct dw_spi_priv *priv = dev_get_priv(bus);
+	struct spi_mem_op *mut_op = (struct spi_mem_op *)op;
 	u8 op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes;
 	u8 op_buf[op_len];
-	u32 cr0;
+	u32 cr0, val;
 
 	if (read)
 		priv->tmode = CTRLR0_TMOD_EPROMREAD;
@@ -599,47 +671,33 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 	if (op->dummy.nbytes)
 		memset(op_buf + pos, 0xff, op->dummy.nbytes);
 
-	external_cs_manage(slave->dev, false);
+	dw_writer(priv, &op_buf, 0, op_len, 0, sizeof(u8));
 
-	priv->tx = &op_buf;
-	priv->tx_end = priv->tx + op_len;
-	priv->rx = NULL;
-	priv->rx_end = NULL;
-	while (priv->tx != priv->tx_end)
-		dw_writer(priv);
+	external_cs_manage(slave->dev, false);
+	dw_write(priv, DW_SPI_SER, 1 << spi_chip_select(slave->dev));
 
 	/*
 	 * XXX: The following are tight loops! Enabling debug messages may cause
 	 * them to fail because we are not reading/writing the fifo fast enough.
 	 */
-	if (read) {
-		priv->rx = op->data.buf.in;
-		priv->rx_end = priv->rx + op->data.nbytes;
+	if (read)
+		mut_op->data.nbytes = poll_transfer(priv, NULL, op->data.buf.in,
+						    op->data.nbytes);
+	else
+		mut_op->data.nbytes = poll_transfer(priv, op->data.buf.out,
+						    NULL, op->data.nbytes);
 
-		dw_write(priv, DW_SPI_SER, 1 << spi_chip_select(slave->dev));
-		while (priv->rx != priv->rx_end)
-			dw_reader(priv);
-	} else {
-		u32 val;
-
-		priv->tx = op->data.buf.out;
-		priv->tx_end = priv->tx + op->data.nbytes;
-
-		/* Fill up the write fifo before starting the transfer */
-		dw_writer(priv);
-		dw_write(priv, DW_SPI_SER, 1 << spi_chip_select(slave->dev));
-		while (priv->tx != priv->tx_end)
-			dw_writer(priv);
-
-		if (readl_poll_timeout(priv->regs + DW_SPI_SR, val,
-				       (val & SR_TF_EMPT) && !(val & SR_BUSY),
-				       RX_TIMEOUT * 1000)) {
-			dev_dbg(bus, "timed out; sr=%x\n",
-				dw_read(priv, DW_SPI_SR));
-			ret = -ETIMEDOUT;
-		}
+	/*
+	 * Ensure the data (or the instruction for zero-data instructions) has
+	 * been transmitted from the fifo/shift register before disabling the
+	 * device.
+	 */
+	if (readl_poll_timeout(priv->regs + DW_SPI_SR, val,
+			       (val & SR_TF_EMPT) && !(val & SR_BUSY),
+			       RX_TIMEOUT * 1000)) {
+		dev_dbg(bus, "timed out; sr=%x\n", dw_read(priv, DW_SPI_SR));
+		ret = -ETIMEDOUT;
 	}
-
 	dw_write(priv, DW_SPI_SER, 0);
 	external_cs_manage(slave->dev, true);
 
-- 
2.29.2

[PATCH 10/14] spi: dw: Add DUAL/QUAD/OCTAL caps

[Sean Anderson]

These capabilities correspond to SSIC_SPI_MODE of 1, 2, or 3, respectively.
This doesn't do much yet, but it does add support for detection and for
disallowing unsupported modes.  Unfortunately, we cannot discriminate
between these modes (only that SSIC_SPI_MODE != 0), so we only assume DUAL
if something sticks to SPI_FRF.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/designware_spi.c | 41 +++++++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 683394a5a4..1dd83e6ca6 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -129,6 +129,9 @@ struct dw_spi_priv {
 #define DW_SPI_CAP_KEEMBAY_MST		BIT(1) /* Unimplemented */
 #define DW_SPI_CAP_DWC_SSI		BIT(2)
 #define DW_SPI_CAP_DFS32		BIT(3)
+#define DW_SPI_CAP_DUAL			BIT(4)
+#define DW_SPI_CAP_QUAD			BIT(5)
+#define DW_SPI_CAP_OCTAL		BIT(5)
 	unsigned long caps;
 	unsigned long bus_clk_rate;
 	unsigned int freq;		/* Default frequency */
@@ -141,6 +144,7 @@ struct dw_spi_priv {
 	u8 cs;				/* chip select pin */
 	u8 tmode;			/* TR/TO/RO/EEPROM */
 	u8 type;			/* SPI/SSP/MicroWire */
+	u8 spi_frf;			/* BYTE/DUAL/QUAD/OCTAL */
 };
 
 static inline u32 dw_read(struct dw_spi_priv *priv, u32 offset)
@@ -267,7 +271,6 @@ static void spi_hw_init(struct udevice *bus, struct dw_spi_priv *priv)
 		priv->fifo_len = (fifo == 1) ? 0 : fifo;
 		dw_write(priv, DW_SPI_TXFTLR, 0);
 	}
-	dev_dbg(bus, "fifo_len=%d\n", priv->fifo_len);
 }
 
 /*
@@ -351,22 +354,21 @@ static int dw_spi_probe(struct udevice *bus)
 	if (ret)
 		return ret;
 
-	priv->caps = dev_get_driver_data(bus);
-	dw_spi_detect_caps(bus, priv);
-
-	version = dw_read(priv, DW_SPI_VERSION);
-	dev_dbg(bus, "ssi_version_id=%c.%c%c%c ssi_max_xfer_size=%u\n",
-		version >> 24, version >> 16, version >> 8, version,
-		priv->caps & DW_SPI_CAP_DFS32 ? 32 : 16);
-
 	/* Currently only bits_per_word == 8 supported */
 	priv->bits_per_word = 8;
 
 	priv->tmode = 0; /* Tx & Rx */
 
 	/* Basic HW init */
+	priv->caps = dev_get_driver_data(bus);
 	spi_hw_init(bus, priv);
 
+	version = dw_read(priv, DW_SPI_VERSION);
+	dev_dbg(bus,
+		"ssi_version_id=%c.%c%c%c ssi_rx_fifo_depth=%u ssi_max_xfer_size=%u\n",
+		version >> 24, version >> 16, version >> 8, version,
+		priv->fifo_len, priv->caps & DW_SPI_CAP_DFS32 ? 32 : 16);
+
 	return 0;
 }
 
@@ -748,13 +750,25 @@ static int dw_spi_set_mode(struct udevice *bus, uint mode)
 {
 	struct dw_spi_priv *priv = dev_get_priv(bus);
 
+	if (!(priv->caps & DW_SPI_CAP_DUAL) &&
+	    (mode & (SPI_RX_DUAL | SPI_TX_DUAL)))
+		return -EINVAL;
+
+	if (!(priv->caps & DW_SPI_CAP_QUAD) &&
+	    (mode & (SPI_RX_QUAD | SPI_TX_QUAD)))
+		return -EINVAL;
+
+	if (!(priv->caps & DW_SPI_CAP_OCTAL) &&
+	    (mode & (SPI_RX_OCTAL | SPI_TX_OCTAL)))
+		return -EINVAL;
+
 	/*
 	 * Can't set mode yet. Since this depends on if rx, tx, or
 	 * rx & tx is requested. So we have to defer this to the
 	 * real transfer function.
 	 */
 	priv->mode = mode;
-	dev_dbg(bus, "mode=%d\n", priv->mode);
+	dev_dbg(bus, "mode=%x\n", mode);
 
 	return 0;
 }
@@ -813,10 +827,13 @@ static const struct udevice_id dw_spi_ids[] = {
 	 */
 	{ .compatible = "altr,socfpga-spi" },
 	{ .compatible = "altr,socfpga-arria10-spi" },
-	{ .compatible = "canaan,kendryte-k210-spi" },
+	{
+		.compatible = "canaan,kendryte-k210-spi",
+		.data = DW_SPI_CAP_QUAD | DW_SPI_CAP_OCTAL,
+	},
 	{
 		.compatible = "canaan,kendryte-k210-ssi",
-		.data = DW_SPI_CAP_DWC_SSI,
+		.data = DW_SPI_CAP_DWC_SSI | DW_SPI_CAP_QUAD,
 	},
 	{ .compatible = "intel,stratix10-spi" },
 	{ .compatible = "intel,agilex-spi" },
-- 
2.29.2

[PATCH 11/14] spi: dw: Add registers necessary for DUAL/QUAD/OCTAL

[Sean Anderson]

This adds some registers needed for DUAL/QUAD/OCTAL modes. It also adds the
fields in (R)ISR so we can check for over-/under-flow.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/designware_spi.c | 55 ++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 1dd83e6ca6..169888a06d 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -56,6 +56,8 @@
 #define DW_SPI_IDR			0x58
 #define DW_SPI_VERSION			0x5c
 #define DW_SPI_DR			0x60
+#define DW_SPI_RX_SAMPLE_DLY		0xf0
+#define DW_SPI_SPI_CTRL0		0xf4
 
 /* Bit fields in CTRLR0 */
 /*
@@ -80,8 +82,8 @@
 #define CTRLR0_TMOD_RO			0x2		/* recv only */
 #define CTRLR0_TMOD_EPROMREAD		0x3		/* eeprom read mode */
 
-#define CTRLR0_SLVOE_OFFSET		10
-#define CTRLR0_SRL_OFFSET		11
+#define CTRLR0_SLVOE_OFFSET		BIT(10)
+#define CTRLR0_SRL			BIT(11)
 #define CTRLR0_CFS_MASK			GENMASK(15, 12)
 
 /* Only present when SSI_MAX_XFER_SIZE=32 */
@@ -92,13 +94,15 @@
 #define CTRLR0_SPI_FRF_BYTE		0x0
 #define	CTRLR0_SPI_FRF_DUAL		0x1
 #define	CTRLR0_SPI_FRF_QUAD		0x2
+#define	CTRLR0_SPI_FRF_OCTAL		0x3
 
 /* Bit fields in CTRLR0 based on DWC_ssi_databook.pdf v1.01a */
 #define DWC_SSI_CTRLR0_DFS_MASK		GENMASK(4, 0)
 #define DWC_SSI_CTRLR0_FRF_MASK		GENMASK(7, 6)
 #define DWC_SSI_CTRLR0_MODE_MASK	GENMASK(9, 8)
 #define DWC_SSI_CTRLR0_TMOD_MASK	GENMASK(11, 10)
-#define DWC_SSI_CTRLR0_SRL_OFFSET	13
+#define DWC_SSI_CTRLR0_SRL		BIT(13)
+#define DWC_SSI_CTRLR0_SSTE		BIT(14)
 #define DWC_SSI_CTRLR0_SPI_FRF_MASK	GENMASK(23, 22)
 
 /* Bit fields in SR, 7 bits */
@@ -111,6 +115,51 @@
 #define SR_TX_ERR			BIT(5)
 #define SR_DCOL				BIT(6)
 
+/* Bit fields in (R)ISR */
+
+/* TX FIFO Empty */
+#define ISR_TXEI			BIT(0)
+/* TX FIFO Overflow */
+#define ISR_TXOI			BIT(1)
+/* RX FIFO Underflow */
+#define ISR_RXUI			BIT(2)
+/* RX FIFO Overflow */
+#define ISR_RXOI			BIT(3)
+/* RX FIFO Full */
+#define ISR_RXFI			BIT(4)
+/* Multi-master contention */
+#define ISR_MSTI			BIT(5)
+/* XIP Receive FIFO Overflow */
+#define ISR_XRXOI			BIT(6)
+/* TX FIFO Underflow */
+#define ISR_TXUI			BIT(7)
+/* AXI Error */
+#define ISR_AXIE			BIT(8)
+/* SPI TX Error */
+#define ISR_SPITE			BIT(10)
+/* SSI Done */
+#define ISR_DONE			BIT(11)
+
+/* Bit fields in SPI_CTRLR0 */
+
+/*
+ * Whether the instruction or address use the value of SPI_FRF or use
+ * FRF_BYTE
+ */
+#define SPI_CTRLR0_TRANS_TYPE		GENMASK(1, 0)
+#define SPI_CTRLR0_TRANS_TYPE_1_1_X	0x0
+#define SPI_CTRLR0_TRANS_TYPE_1_X_X	0x1
+#define SPI_CTRLR0_TRANS_TYPE_X_X_X	0x2
+/* Address width in 4-bit units */
+#define SPI_CTRLR0_ADDR_L_MASK		GENMASK(5, 2)
+/* Enable mode bits after address in XIP mode */
+#define SPI_CTRLR0_XIP_MD_BIT_EN	BIT(7)
+/* Instruction width in 4-bit units */
+#define SPI_CTRLR0_INST_L_MASK		GENMASK(9, 8)
+/* Number of "dummy" cycles */
+#define SPI_CTRLR0_WAIT_CYCLES_MASK	GENMASK(15, 11)
+#define SPI_CTRLR0_CLK_STRETCH_EN	BIT(30)
+
 #define RX_TIMEOUT			1000		/* timeout in ms */
 
 struct dw_spi_plat {
-- 
2.29.2

[PATCH 12/14] spi: dw: Support DUAL/QUAD/OCTAL

[Sean Anderson]

This adds support for DUAL/QUAD/OCTAL transfers. This adds
dw_spi_supports_op to do some sanity checks which would otherwise live in
exec_op. We only support byte transfers, but as far as I could tell only
bytes are supported by mem_ops (e.g. every part of the opcode has nbytes).

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/designware_spi.c | 147 +++++++++++++++++++++++++++++++----
 1 file changed, 130 insertions(+), 17 deletions(-)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 169888a06d..4796d55b20 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -215,7 +215,8 @@ static u32 dw_spi_update_cr0(struct dw_spi_priv *priv)
 				 priv->bits_per_word - 1)
 		    | FIELD_PREP(DWC_SSI_CTRLR0_FRF_MASK, priv->type)
 		    | FIELD_PREP(DWC_SSI_CTRLR0_MODE_MASK, priv->mode)
-		    | FIELD_PREP(DWC_SSI_CTRLR0_TMOD_MASK, priv->tmode);
+		    | FIELD_PREP(DWC_SSI_CTRLR0_TMOD_MASK, priv->tmode)
+		    | FIELD_PREP(DWC_SSI_CTRLR0_SPI_FRF_MASK, priv->spi_frf);
 	} else {
 		if (priv->caps & DW_SPI_CAP_DFS32)
 			cr0 = FIELD_PREP(CTRLR0_DFS_32_MASK,
@@ -226,12 +227,36 @@ static u32 dw_spi_update_cr0(struct dw_spi_priv *priv)
 
 		cr0 |= FIELD_PREP(CTRLR0_FRF_MASK, priv->type)
 		    |  FIELD_PREP(CTRLR0_MODE_MASK, priv->mode)
-		    |  FIELD_PREP(CTRLR0_TMOD_MASK, priv->tmode);
+		    |  FIELD_PREP(CTRLR0_TMOD_MASK, priv->tmode)
+		    |  FIELD_PREP(CTRLR0_SPI_FRF_MASK, priv->spi_frf);
 	}
 
 	return cr0;
 }
 
+static u32 dw_spi_update_spi_cr0(const struct spi_mem_op *op)
+{
+	uint trans_type, wait_cycles;
+
+	/* This assumes support_op has filtered invalid types */
+	if (op->addr.buswidth == 1)
+		trans_type = SPI_CTRLR0_TRANS_TYPE_1_1_X;
+	else if (op->cmd.buswidth == 1)
+		trans_type = SPI_CTRLR0_TRANS_TYPE_1_X_X;
+	else
+		trans_type = SPI_CTRLR0_TRANS_TYPE_X_X_X;
+
+	if (op->dummy.buswidth)
+		wait_cycles = op->dummy.nbytes * 8 / op->dummy.buswidth;
+	else
+		wait_cycles = 0;
+
+	return FIELD_PREP(SPI_CTRLR0_TRANS_TYPE, trans_type)
+	       | FIELD_PREP(SPI_CTRLR0_ADDR_L_MASK, op->addr.nbytes * 2)
+	       | FIELD_PREP(SPI_CTRLR0_INST_L_MASK, 2)
+	       | FIELD_PREP(SPI_CTRLR0_WAIT_CYCLES_MASK, wait_cycles);
+}
+
 static int request_gpio_cs(struct udevice *bus)
 {
 #if CONFIG_IS_ENABLED(DM_GPIO) && !defined(CONFIG_SPL_BUILD)
@@ -302,6 +327,17 @@ static void spi_hw_init(struct udevice *bus, struct dw_spi_priv *priv)
 	if (priv->caps & DW_SPI_CAP_DWC_SSI || !FIELD_GET(CTRLR0_DFS_MASK, cr0))
 		priv->caps |= DW_SPI_CAP_DFS32;
 
+	/*
+	 * If SPI_FRF exists that means we have DUAL, QUAD, or OCTAL. Since we
+	 * can't differentiate, just set DUAL
+	 */
+	if (priv->caps & DW_SPI_CAP_DWC_SSI) {
+		if (FIELD_GET(DWC_SSI_CTRLR0_SPI_FRF_MASK, cr0))
+			priv->caps |= DW_SPI_CAP_DUAL;
+	} else if (FIELD_GET(CTRLR0_SPI_FRF_MASK, cr0)) {
+		priv->caps |= DW_SPI_CAP_DUAL;
+	}
+
 	dw_write(priv, DW_SPI_SSIENR, 1);
 
 	/*
@@ -604,6 +640,13 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
 	u32 val, cs;
 	uint frames;
 
+	/* DUAL/QUAD/OCTAL only supported by exec_op for now */
+	if (priv->mode & (SPI_TX_DUAL | SPI_TX_QUAD | SPI_TX_OCTAL |
+			  SPI_RX_DUAL | SPI_RX_QUAD | SPI_RX_OCTAL))
+		return -1;
+
+	priv->spi_frf = CTRLR0_SPI_FRF_BYTE;
+
 	/* spi core configured to do 8 bit transfers */
 	if (bitlen % priv->bits_per_word) {
 		dev_err(dev, "Non byte aligned SPI transfer.\n");
@@ -682,6 +725,9 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
 /*
  * This function is necessary for reading SPI flash with the native CS
  * c.f. https://lkml.org/lkml/2015/12/23/132
+ *
+ * It also lets us handle DUAL/QUAD/OCTAL transfers in a much more idiomatic
+ * way.
  */
 static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 {
@@ -692,37 +738,72 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 	struct spi_mem_op *mut_op = (struct spi_mem_op *)op;
 	u8 op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes;
 	u8 op_buf[op_len];
-	u32 cr0, val;
+	u32 cr0, spi_cr0, val;
+
+	/* Only bytes are supported for spi-mem transfers */
+	if (priv->bits_per_word != 8)
+		return -EINVAL;
+
+	switch (op->data.buswidth) {
+	case 0:
+	case 1:
+		priv->spi_frf = CTRLR0_SPI_FRF_BYTE;
+		break;
+	case 2:
+		priv->spi_frf = CTRLR0_SPI_FRF_DUAL;
+		break;
+	case 4:
+		priv->spi_frf = CTRLR0_SPI_FRF_QUAD;
+		break;
+	case 8:
+		priv->spi_frf = CTRLR0_SPI_FRF_OCTAL;
+		break;
+	/* BUG: should have been filtered out by supports_op */
+	default:
+		return -EINVAL;
+	}
 
 	if (read)
-		priv->tmode = CTRLR0_TMOD_EPROMREAD;
+		if (priv->spi_frf == CTRLR0_SPI_FRF_BYTE)
+			priv->tmode = CTRLR0_TMOD_EPROMREAD;
+		else
+			priv->tmode = CTRLR0_TMOD_RO;
 	else
 		priv->tmode = CTRLR0_TMOD_TO;
 
 	cr0 = dw_spi_update_cr0(priv);
-	dev_dbg(bus, "cr0=%08x buf=%p len=%u [bytes]\n", cr0, op->data.buf.in,
-		op->data.nbytes);
+	spi_cr0 = dw_spi_update_spi_cr0(op);
+	dev_dbg(bus, "cr0=%08x spi_cr0=%08x buf=%p len=%u [bytes]\n", cr0,
+		spi_cr0, op->data.buf.in, op->data.nbytes);
 
 	dw_write(priv, DW_SPI_SSIENR, 0);
 	dw_write(priv, DW_SPI_CTRLR0, cr0);
 	if (read)
 		dw_write(priv, DW_SPI_CTRLR1, op->data.nbytes - 1);
+	if (priv->spi_frf != CTRLR0_SPI_FRF_BYTE)
+		dw_write(priv, DW_SPI_SPI_CTRL0, spi_cr0);
 	dw_write(priv, DW_SPI_SSIENR, 1);
 
-	/* From spi_mem_exec_op */
-	pos = 0;
-	op_buf[pos++] = op->cmd.opcode;
-	if (op->addr.nbytes) {
-		for (i = 0; i < op->addr.nbytes; i++)
-			op_buf[pos + i] = op->addr.val >>
-				(8 * (op->addr.nbytes - i - 1));
+	/* Write out the instruction */
+	if (priv->spi_frf == CTRLR0_SPI_FRF_BYTE) {
+		/* From spi_mem_exec_op */
+		pos = 0;
+		op_buf[pos++] = op->cmd.opcode;
+		if (op->addr.nbytes) {
+			for (i = 0; i < op->addr.nbytes; i++)
+				op_buf[pos + i] = op->addr.val >>
+					(8 * (op->addr.nbytes - i - 1));
 
-		pos += op->addr.nbytes;
-	}
-	if (op->dummy.nbytes)
+			pos += op->addr.nbytes;
+		}
 		memset(op_buf + pos, 0xff, op->dummy.nbytes);
 
-	dw_writer(priv, &op_buf, 0, op_len, 0, sizeof(u8));
+		dw_writer(priv, &op_buf, 0, op_len, 0, sizeof(u8));
+	} else {
+		/* MUST be written as byte/long; don't ask me why */
+		writeb(op->cmd.opcode, priv->regs + DW_SPI_DR);
+		writel(op->addr.val, priv->regs + DW_SPI_DR);
+	}
 
 	external_cs_manage(slave->dev, false);
 	dw_write(priv, DW_SPI_SER, 1 << spi_chip_select(slave->dev));
@@ -756,6 +837,37 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 	return ret;
 }
 
+bool dw_spi_supports_op(struct spi_slave *slave, const struct spi_mem_op *op)
+{
+	struct dw_spi_priv *priv = dev_get_priv(slave->dev->parent);
+
+	if (!spi_mem_default_supports_op(slave, op))
+		return false;
+
+	/*
+	 * Everything before the data must fit in the fifo.
+	 * In EEPROM mode we also need to fit the dummy.
+	 */
+	if (1 + op->addr.nbytes +
+	    (op->data.buswidth == 1 ? op->dummy.nbytes : 0) > priv->fifo_len)
+		return false;
+
+	/* We only support 1_1_X, 1_X_X, and X_X_X formats */
+	if (op->cmd.buswidth == 1 &&
+	    (!op->addr.nbytes || op->addr.buswidth == 1))
+		return true;
+
+	if (op->cmd.buswidth == 1 &&
+	    (!op->addr.nbytes || op->addr.buswidth == op->data.buswidth))
+		return true;
+
+	if (op->cmd.buswidth == op->data.buswidth &&
+	    (!op->addr.nbytes || op->addr.buswidth == op->data.buswidth))
+		return true;
+
+	return false;
+}
+
 /* The size of ctrl1 limits data transfers to 64K */
 static int dw_spi_adjust_op_size(struct spi_slave *slave, struct spi_mem_op *op)
 {
@@ -766,6 +878,7 @@ static int dw_spi_adjust_op_size(struct spi_slave *slave, struct spi_mem_op *op)
 
 static const struct spi_controller_mem_ops dw_spi_mem_ops = {
 	.exec_op = dw_spi_exec_op,
+	.supports_op = dw_spi_supports_op,
 	.adjust_op_size = dw_spi_adjust_op_size,
 };
 
-- 
2.29.2

[PATCH 13/14] spi: dw: Support clock stretching

[Sean Anderson]

We don't always read/write to the FIFO fast enough. Enable clock stretching
for enhanced SPI transfers. This is only possible with DWC SSI devices more
recent than 1.01a. We also need to set the RXFTLR register to tell the
device when to start reciving again. In particular, the default of 0 will
result in the device never restarting reception if there is an overflow. On
the transmit side, we need to set CTRL1 so that the device knows when to
keep stretching the clock if the FIFO is empty.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/designware_spi.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 4796d55b20..78da490c3e 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -254,7 +254,8 @@ static u32 dw_spi_update_spi_cr0(const struct spi_mem_op *op)
 	return FIELD_PREP(SPI_CTRLR0_TRANS_TYPE, trans_type)
 	       | FIELD_PREP(SPI_CTRLR0_ADDR_L_MASK, op->addr.nbytes * 2)
 	       | FIELD_PREP(SPI_CTRLR0_INST_L_MASK, 2)
-	       | FIELD_PREP(SPI_CTRLR0_WAIT_CYCLES_MASK, wait_cycles);
+	       | FIELD_PREP(SPI_CTRLR0_WAIT_CYCLES_MASK, wait_cycles)
+	       | SPI_CTRLR0_CLK_STRETCH_EN;
 }
 
 static int request_gpio_cs(struct udevice *bus)
@@ -356,6 +357,9 @@ static void spi_hw_init(struct udevice *bus, struct dw_spi_priv *priv)
 		priv->fifo_len = (fifo == 1) ? 0 : fifo;
 		dw_write(priv, DW_SPI_TXFTLR, 0);
 	}
+
+	/* Set receive fifo interrupt level register for clock stretching */
+	dw_write(priv, DW_SPI_RXFTLR, priv->fifo_len - 1);
 }
 
 /*
@@ -778,8 +782,7 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 
 	dw_write(priv, DW_SPI_SSIENR, 0);
 	dw_write(priv, DW_SPI_CTRLR0, cr0);
-	if (read)
-		dw_write(priv, DW_SPI_CTRLR1, op->data.nbytes - 1);
+	dw_write(priv, DW_SPI_CTRLR1, op->data.nbytes - 1);
 	if (priv->spi_frf != CTRLR0_SPI_FRF_BYTE)
 		dw_write(priv, DW_SPI_SPI_CTRL0, spi_cr0);
 	dw_write(priv, DW_SPI_SSIENR, 1);
-- 
2.29.2

[PATCH 14/14] riscv: k210: Enable QSPI for spi3

[Sean Anderson]

This device has four IOs connected to the SPI flash. Add the appropriate
bindings.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 arch/riscv/dts/k210-maix-bit.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/dts/k210-maix-bit.dts b/arch/riscv/dts/k210-maix-bit.dts
index e4dea205b2..902dcfd08a 100644
--- a/arch/riscv/dts/k210-maix-bit.dts
+++ b/arch/riscv/dts/k210-maix-bit.dts
@@ -200,6 +200,8 @@
 		compatible = "jedec,spi-nor";
 		reg = <0>;
 		spi-max-frequency = <50000000>;
+		spi-tx-bus-width = <4>;
+		spi-rx-bus-width = <4>;
 		m25p,fast-read;
 		broken-flash-reset;
 	};
-- 
2.29.2

[PATCH 01/14] cmd: sf: Display errno on erase failure

[Bin Meng]

On Mon, Feb 1, 2021 at 8:35 AM Sean Anderson <seanga2@gmail.com> wrote:
>
> If there is an error while erasing SPI flash, no errno is displayed. This
> makes it difficult to determine the cause of the error. This change mirrors
> the logic for write errors above.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
>  cmd/sf.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH 02/14] cmd: sf: Print error on test failure

[Bin Meng]

On Mon, Feb 1, 2021 at 8:35 AM Sean Anderson <seanga2@gmail.com> wrote:
>
> The sf test command is used to test spi flashes (and spi masters). Printing
> the exact error code is very helpful to those debugging the spi stack.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
>  cmd/sf.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH 03/14] mtd: spi-nor-core: Fix typo in documentation

[Bin Meng]

On Mon, Feb 1, 2021 at 8:35 AM Sean Anderson <seanga2@gmail.com> wrote:
>
> This line should come before the docs for the next function.
>
> Fixes: 7aeedac0153 ("mtd: spi: Port SPI NOR framework from Linux")
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
>  include/linux/mtd/spi-nor.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH 04/14] mtd: spi-mem: Export spi_mem_default_supports_op

[Bin Meng]

On Mon, Feb 1, 2021 at 8:35 AM Sean Anderson <seanga2@gmail.com> wrote:
>
> This is useful for extending the default functionality.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
>  include/spi-mem.h | 3 +++
>  1 file changed, 3 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH 05/14] spi: spi-mem: Add debug message for spi-mem ops

[Bin Meng]

On Mon, Feb 1, 2021 at 8:36 AM Sean Anderson <seanga2@gmail.com> wrote:
>
> This prints some basic metadata about the SPI memory op. This information
> may be used to debug SPI drivers (e.g. determining the expected SPI mode).
> It is also helpful for verifying that the data on the wire matches the data
> intended to be transmitted (e.g. with a logic analyzer). The opcode is
> printed with a format of %02Xh to match the notation commonly used in flash
> datasheets.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
>  drivers/spi/spi-mem.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index c095ae9505..eb83c11910 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -220,6 +220,12 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
>         int ret;
>         int i;
>
> +       dev_dbg(slave->dev, "exec %02Xh %u-%u-%u addr=%llx wait=%u len=%u\n",

dummy cycles=%u, data len=%u to be clearer?

> +               op->cmd.opcode, op->cmd.buswidth, op->addr.buswidth,
> +               op->data.buswidth, op->addr.val,
> +               op->dummy.buswidth ? op->dummy.nbytes * 8 / op->dummy.buswidth : 0,
> +               op->data.nbytes);
> +
>         if (!spi_mem_supports_op(slave, op))
>                 return -ENOTSUPP;
>
> --

Otherwise LGTM
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH 14/14] riscv: k210: Enable QSPI for spi3

[Bin Meng]

On Mon, Feb 1, 2021 at 8:34 AM Sean Anderson <seanga2@gmail.com> wrote:
>
> This device has four IOs connected to the SPI flash. Add the appropriate
> bindings.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
>  arch/riscv/dts/k210-maix-bit.dts | 2 ++
>  1 file changed, 2 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH 04/14] mtd: spi-mem: Export spi_mem_default_supports_op

[Pratyush Yadav]

Hi Sean,

On 31/01/21 07:34PM, Sean Anderson wrote:
> This is useful for extending the default functionality.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>

I saw a similar patch a few days ago [0]. Which one to pick is up to the 
maintainer I guess.

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

[0] https://patchwork.ozlabs.org/project/uboot/patch/20210125035521.23466-2-matt at traverse.com.au/

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

[PATCH 05/14] spi: spi-mem: Add debug message for spi-mem ops

[Pratyush Yadav]

On 31/01/21 07:34PM, Sean Anderson wrote:
> This prints some basic metadata about the SPI memory op. This information
> may be used to debug SPI drivers (e.g. determining the expected SPI mode).
> It is also helpful for verifying that the data on the wire matches the data
> intended to be transmitted (e.g. with a logic analyzer). The opcode is
> printed with a format of %02Xh to match the notation commonly used in flash
> datasheets.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
> 
>  drivers/spi/spi-mem.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index c095ae9505..eb83c11910 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -220,6 +220,12 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
>  	int ret;
>  	int i;
>  
> +	dev_dbg(slave->dev, "exec %02Xh %u-%u-%u addr=%llx wait=%u len=%u\n",
> +		op->cmd.opcode, op->cmd.buswidth, op->addr.buswidth,
> +		op->data.buswidth, op->addr.val,
> +		op->dummy.buswidth ? op->dummy.nbytes * 8 / op->dummy.buswidth : 0,

SPI MEM deals with dummy bytes not cycles [0]. Just print them directly 
instead of converting them to cycles first.

Also agree with Bin that "dummy cycles=" (or dummy bytes if you are 
printing bytes directly) is clearer.

[0] For now. This might change later. See 
https://lore.kernel.org/linux-mtd/6396018a-485f-6eb4-7742-bdb5c4335ed4 at microchip.com/

With these fixed,

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

> +		op->data.nbytes);
> +
>  	if (!spi_mem_supports_op(slave, op))
>  		return -ENOTSUPP;
>  

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

[PATCH 03/14] mtd: spi-nor-core: Fix typo in documentation

[Pratyush Yadav]

+Cc Takahiro

On 31/01/21 07:34PM, Sean Anderson wrote:
> This line should come before the docs for the next function.
> 
> Fixes: 7aeedac0153 ("mtd: spi: Port SPI NOR framework from Linux")
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>

Saw this fix in [0] as well but I think your patch should take priority 
and Takahiro should drop it from his.

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

[0] https://patchwork.ozlabs.org/project/uboot/patch/6442361cd0a3acf98525b475992eb9d0ec014994.1611729896.git.Takahiro.Kuwano at infineon.com/
 
> ---
> 
>  include/linux/mtd/spi-nor.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 4a8e19ee4f..39ef872ebf 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -302,8 +302,8 @@ struct spi_flash {
>   * @flash_lock:		[FLASH-SPECIFIC] lock a region of the SPI NOR
>   * @flash_unlock:	[FLASH-SPECIFIC] unlock a region of the SPI NOR
>   * @flash_is_locked:	[FLASH-SPECIFIC] check if a region of the SPI NOR is
> - * @quad_enable:	[FLASH-SPECIFIC] enables SPI NOR quad mode
>   *			completely locked
> + * @quad_enable:	[FLASH-SPECIFIC] enables SPI NOR quad mode
>   * @priv:		the private data
>   */
>  struct spi_nor {

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

[PATCH 01/14] cmd: sf: Display errno on erase failure

[Pratyush Yadav]

On 31/01/21 07:34PM, Sean Anderson wrote:
> If there is an error while erasing SPI flash, no errno is displayed. This
> makes it difficult to determine the cause of the error. This change mirrors
> the logic for write errors above.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

[PATCH 02/14] cmd: sf: Print error on test failure

[Pratyush Yadav]

On 31/01/21 07:34PM, Sean Anderson wrote:
> The sf test command is used to test spi flashes (and spi masters). Printing
> the exact error code is very helpful to those debugging the spi stack.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

[PATCH 04/14] mtd: spi-mem: Export spi_mem_default_supports_op

[Sean Anderson]

On 2/1/21 7:07 AM, Pratyush Yadav wrote:
> Hi Sean,
> 
> On 31/01/21 07:34PM, Sean Anderson wrote:
>> This is useful for extending the default functionality.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> 
> I saw a similar patch a few days ago [0]. Which one to pick is up to the
> maintainer I guess.
> 
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
> 
> [0] https://patchwork.ozlabs.org/project/uboot/patch/20210125035521.23466-2-matt at traverse.com.au/
> 

It was also done in Linux as commit 46109648052f ("spi: spi-mem: export
spi_mem_default_supports_op()"). Though I wrote this patch before I knew
about that one.

--Sean

[PATCH 05/14] spi: spi-mem: Add debug message for spi-mem ops

[Sean Anderson]

On 2/1/21 7:18 AM, Pratyush Yadav wrote:
> On 31/01/21 07:34PM, Sean Anderson wrote:
>> This prints some basic metadata about the SPI memory op. This information
>> may be used to debug SPI drivers (e.g. determining the expected SPI mode).
>> It is also helpful for verifying that the data on the wire matches the data
>> intended to be transmitted (e.g. with a logic analyzer). The opcode is
>> printed with a format of %02Xh to match the notation commonly used in flash
>> datasheets.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>>   drivers/spi/spi-mem.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
>> index c095ae9505..eb83c11910 100644
>> --- a/drivers/spi/spi-mem.c
>> +++ b/drivers/spi/spi-mem.c
>> @@ -220,6 +220,12 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
>>   	int ret;
>>   	int i;
>>   
>> +	dev_dbg(slave->dev, "exec %02Xh %u-%u-%u addr=%llx wait=%u len=%u\n",
>> +		op->cmd.opcode, op->cmd.buswidth, op->addr.buswidth,
>> +		op->data.buswidth, op->addr.val,
>> +		op->dummy.buswidth ? op->dummy.nbytes * 8 / op->dummy.buswidth : 0,
> 
> SPI MEM deals with dummy bytes not cycles [0]. Just print them directly
> instead of converting them to cycles first.

I chose to do it this way, since as far as I can tell, most quad-spi
drivers (with the exception of mtk_snfi_spi.c, octeon_spi.c, spi-sifive.c,
and ti_qspi.c) deal with cycles and not bytes. The spi-nor system also
specifies dummies in cycles and only converts them to bytes when
creating a struct spi_mem_op. SPI flash datasheets also typically refer
to dummies in cycles; since there is no data being transmitted, they
cannot be truly measured in bytes. Lastly, it is much easier to only
work in cycles when looking at logic analyzer traces. When switching
between 1-1-4 and 1-4-4 instructions, the meaning of "cycles" remains
the same, while "bytes" has a variable amount of time.

> Also agree with Bin that "dummy cycles=" (or dummy bytes if you are
> printing bytes directly) is clearer.

Sure.
  
> 
> [0] For now. This might change later. See
> https://lore.kernel.org/linux-mtd/6396018a-485f-6eb4-7742-bdb5c4335ed4 at microchip.com/

Would be nice :)

--Sean

> 
> With these fixed,
> 
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
> 
>> +		op->data.nbytes);
>> +
>>   	if (!spi_mem_supports_op(slave, op))
>>   		return -ENOTSUPP;
>>   
> 

[PATCH 05/14] spi: spi-mem: Add debug message for spi-mem ops

[Pratyush Yadav]

On 01/02/21 10:33AM, Sean Anderson wrote:
> On 2/1/21 7:18 AM, Pratyush Yadav wrote:
> > On 31/01/21 07:34PM, Sean Anderson wrote:
> > > This prints some basic metadata about the SPI memory op. This information
> > > may be used to debug SPI drivers (e.g. determining the expected SPI mode).
> > > It is also helpful for verifying that the data on the wire matches the data
> > > intended to be transmitted (e.g. with a logic analyzer). The opcode is
> > > printed with a format of %02Xh to match the notation commonly used in flash
> > > datasheets.
> > > 
> > > Signed-off-by: Sean Anderson <seanga2@gmail.com>
> > > ---
> > > 
> > >   drivers/spi/spi-mem.c | 6 ++++++
> > >   1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> > > index c095ae9505..eb83c11910 100644
> > > --- a/drivers/spi/spi-mem.c
> > > +++ b/drivers/spi/spi-mem.c
> > > @@ -220,6 +220,12 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
> > >   	int ret;
> > >   	int i;
> > > +	dev_dbg(slave->dev, "exec %02Xh %u-%u-%u addr=%llx wait=%u len=%u\n",
> > > +		op->cmd.opcode, op->cmd.buswidth, op->addr.buswidth,
> > > +		op->data.buswidth, op->addr.val,
> > > +		op->dummy.buswidth ? op->dummy.nbytes * 8 / op->dummy.buswidth : 0,
> > 
> > SPI MEM deals with dummy bytes not cycles [0]. Just print them directly
> > instead of converting them to cycles first.
> 
> I chose to do it this way, since as far as I can tell, most quad-spi
> drivers (with the exception of mtk_snfi_spi.c, octeon_spi.c, spi-sifive.c,
> and ti_qspi.c) deal with cycles and not bytes. The spi-nor system also
> specifies dummies in cycles and only converts them to bytes when
> creating a struct spi_mem_op. SPI flash datasheets also typically refer
> to dummies in cycles; since there is no data being transmitted, they
> cannot be truly measured in bytes. Lastly, it is much easier to only
> work in cycles when looking at logic analyzer traces. When switching
> between 1-1-4 and 1-4-4 instructions, the meaning of "cycles" remains
> the same, while "bytes" has a variable amount of time.

I agree that dummy cycles is a better unit than dummy bytes, but that is 
not what is currently being used. You are interpreting the dummy.nbytes 
field rather than simply printing it and letting the reader see what it 
means. We don't pass dummy cycles to the controller. We pass dummy 
bytes. So it makes sense to print that in the debug message.

Anyway, I feel like we are getting into bikeshedding territory. So 
either is fine by me. Choose whichever makes more sense to you, keeping 
my arguments in mind.
 
> > Also agree with Bin that "dummy cycles=" (or dummy bytes if you are
> > printing bytes directly) is clearer.
> 
> Sure.
> > 
> > [0] For now. This might change later. See
> > https://lore.kernel.org/linux-mtd/6396018a-485f-6eb4-7742-bdb5c4335ed4 at microchip.com/
> 
> Would be nice :)
> 
> --Sean
> 
> > 
> > With these fixed,
> > 
> > Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
> > 
> > > +		op->data.nbytes);
> > > +
> > >   	if (!spi_mem_supports_op(slave, op))
> > >   		return -ENOTSUPP;
> > 
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

[PATCH 10/14] spi: dw: Add DUAL/QUAD/OCTAL caps

[Sean Anderson]

On 1/31/21 7:34 PM, Sean Anderson wrote:
> These capabilities correspond to SSIC_SPI_MODE of 1, 2, or 3, respectively.
> This doesn't do much yet, but it does add support for detection and for
> disallowing unsupported modes.  Unfortunately, we cannot discriminate
> between these modes (only that SSIC_SPI_MODE != 0), so we only assume DUAL
> if something sticks to SPI_FRF.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
> 
>   drivers/spi/designware_spi.c | 41 +++++++++++++++++++++++++-----------
>   1 file changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
> index 683394a5a4..1dd83e6ca6 100644
> --- a/drivers/spi/designware_spi.c
> +++ b/drivers/spi/designware_spi.c
> @@ -129,6 +129,9 @@ struct dw_spi_priv {
>   #define DW_SPI_CAP_KEEMBAY_MST		BIT(1) /* Unimplemented */
>   #define DW_SPI_CAP_DWC_SSI		BIT(2)
>   #define DW_SPI_CAP_DFS32		BIT(3)
> +#define DW_SPI_CAP_DUAL			BIT(4)
> +#define DW_SPI_CAP_QUAD			BIT(5)
> +#define DW_SPI_CAP_OCTAL		BIT(5)

Looks like these CAPs both use BIT(5). Will fix in next revision.

Actually, it might be better to just have a CAP_EXTENDED and let the SPI
slave determine the bus width...

--Sean

>   	unsigned long caps;
>   	unsigned long bus_clk_rate;
>   	unsigned int freq;		/* Default frequency */
> @@ -141,6 +144,7 @@ struct dw_spi_priv {
>   	u8 cs;				/* chip select pin */
>   	u8 tmode;			/* TR/TO/RO/EEPROM */
>   	u8 type;			/* SPI/SSP/MicroWire */
> +	u8 spi_frf;			/* BYTE/DUAL/QUAD/OCTAL */
>   };
>   
>   static inline u32 dw_read(struct dw_spi_priv *priv, u32 offset)
> @@ -267,7 +271,6 @@ static void spi_hw_init(struct udevice *bus, struct dw_spi_priv *priv)
>   		priv->fifo_len = (fifo == 1) ? 0 : fifo;
>   		dw_write(priv, DW_SPI_TXFTLR, 0);
>   	}
> -	dev_dbg(bus, "fifo_len=%d\n", priv->fifo_len);
>   }
>   
>   /*
> @@ -351,22 +354,21 @@ static int dw_spi_probe(struct udevice *bus)
>   	if (ret)
>   		return ret;
>   
> -	priv->caps = dev_get_driver_data(bus);
> -	dw_spi_detect_caps(bus, priv);
> -
> -	version = dw_read(priv, DW_SPI_VERSION);
> -	dev_dbg(bus, "ssi_version_id=%c.%c%c%c ssi_max_xfer_size=%u\n",
> -		version >> 24, version >> 16, version >> 8, version,
> -		priv->caps & DW_SPI_CAP_DFS32 ? 32 : 16);
> -
>   	/* Currently only bits_per_word == 8 supported */
>   	priv->bits_per_word = 8;
>   
>   	priv->tmode = 0; /* Tx & Rx */
>   
>   	/* Basic HW init */
> +	priv->caps = dev_get_driver_data(bus);
>   	spi_hw_init(bus, priv);
>   
> +	version = dw_read(priv, DW_SPI_VERSION);
> +	dev_dbg(bus,
> +		"ssi_version_id=%c.%c%c%c ssi_rx_fifo_depth=%u ssi_max_xfer_size=%u\n",
> +		version >> 24, version >> 16, version >> 8, version,
> +		priv->fifo_len, priv->caps & DW_SPI_CAP_DFS32 ? 32 : 16);
> +
>   	return 0;
>   }
>   
> @@ -748,13 +750,25 @@ static int dw_spi_set_mode(struct udevice *bus, uint mode)
>   {
>   	struct dw_spi_priv *priv = dev_get_priv(bus);
>   
> +	if (!(priv->caps & DW_SPI_CAP_DUAL) &&
> +	    (mode & (SPI_RX_DUAL | SPI_TX_DUAL)))
> +		return -EINVAL;
> +
> +	if (!(priv->caps & DW_SPI_CAP_QUAD) &&
> +	    (mode & (SPI_RX_QUAD | SPI_TX_QUAD)))
> +		return -EINVAL;
> +
> +	if (!(priv->caps & DW_SPI_CAP_OCTAL) &&
> +	    (mode & (SPI_RX_OCTAL | SPI_TX_OCTAL)))
> +		return -EINVAL;
> +
>   	/*
>   	 * Can't set mode yet. Since this depends on if rx, tx, or
>   	 * rx & tx is requested. So we have to defer this to the
>   	 * real transfer function.
>   	 */
>   	priv->mode = mode;
> -	dev_dbg(bus, "mode=%d\n", priv->mode);
> +	dev_dbg(bus, "mode=%x\n", mode);
>   
>   	return 0;
>   }
> @@ -813,10 +827,13 @@ static const struct udevice_id dw_spi_ids[] = {
>   	 */
>   	{ .compatible = "altr,socfpga-spi" },
>   	{ .compatible = "altr,socfpga-arria10-spi" },
> -	{ .compatible = "canaan,kendryte-k210-spi" },
> +	{
> +		.compatible = "canaan,kendryte-k210-spi",
> +		.data = DW_SPI_CAP_QUAD | DW_SPI_CAP_OCTAL,
> +	},
>   	{
>   		.compatible = "canaan,kendryte-k210-ssi",
> -		.data = DW_SPI_CAP_DWC_SSI,
> +		.data = DW_SPI_CAP_DWC_SSI | DW_SPI_CAP_QUAD,
>   	},
>   	{ .compatible = "intel,stratix10-spi" },
>   	{ .compatible = "intel,agilex-spi" },
> 

[PATCH 14/14] riscv: k210: Enable QSPI for spi3

[Leo Liang]

On Sun, Jan 31, 2021 at 07:34:36PM -0500, Sean Anderson wrote:
> This device has four IOs connected to the SPI flash. Add the appropriate
> bindings.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
> 
>  arch/riscv/dts/k210-maix-bit.dts | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/riscv/dts/k210-maix-bit.dts b/arch/riscv/dts/k210-maix-bit.dts
> index e4dea205b2..902dcfd08a 100644
> --- a/arch/riscv/dts/k210-maix-bit.dts
> +++ b/arch/riscv/dts/k210-maix-bit.dts
> @@ -200,6 +200,8 @@
>  		compatible = "jedec,spi-nor";
>  		reg = <0>;
>  		spi-max-frequency = <50000000>;
> +		spi-tx-bus-width = <4>;
> +		spi-rx-bus-width = <4>;
>  		m25p,fast-read;
>  		broken-flash-reset;
>  	};
> -- 
> 2.29.2
>
Reviewed-by: Leo Liang <ycliang@andestech.com>

[PATCH 07/14] spi: dw: Mask all possible interrupts

[Sean Anderson]

On 1/31/21 7:34 PM, Sean Anderson wrote:
> DWC_SPI has more than 8 interrupts. Write to the whole IMR to mask
> everything.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
> 
>   drivers/spi/designware_spi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
> index 519d6e32bd..29ec1503fd 100644
> --- a/drivers/spi/designware_spi.c
> +++ b/drivers/spi/designware_spi.c
> @@ -252,7 +252,7 @@ static int dw_spi_of_to_plat(struct udevice *bus)
>   static void spi_hw_init(struct udevice *bus, struct dw_spi_priv *priv)
>   {
>   	dw_write(priv, DW_SPI_SSIENR, 0);
> -	dw_write(priv, DW_SPI_IMR, 0xff);
> +	dw_write(priv, DW_SPI_IMR, 0xFFFFFFFF);

After further reviewing the datasheet, it turns out that interrupts are
masked by 0s in this register (and enabled by 1s). I'm not really sure
what this line was originally trying to accomplish. We never enable
interrupts globally, and we can just read from RISR if we need to get an
interrupt. Perhaps this patch should just remove this line?

--Sean

>   	dw_write(priv, DW_SPI_SSIENR, 1);
>   
>   	/*
>