[PATCH v2 00/10] spi: dw: Add support for DUAL/QUAD/OCTAL modes
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. Given that there has been no testing of v2 over the past month, I don't think lack of testing should hold up this series.
Changes in v3: - Dropped merged patches - Rebased on u-boot/master
Changes in v2: - Add more information to exec_op debug message - Actually mask interrupts - Merge CAP_{DUAL,QUAD,OCTAL} into CAP_ENHANCED - Fix some inconsistencies in register naming and usage - Moved some hunks between commits so things make more sense
Sean Anderson (10): 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: Actually mask interrupts spi: dw: Switch to capabilities spi: dw: Rewrite poll_transfer logic spi: dw: Add ENHANCED cap spi: dw: Define registers for enhanced mode spi: dw: Support enhanced SPI spi: dw: Support clock stretching
drivers/spi/designware_spi.c | 647 ++++++++++++++++++++++++----------- drivers/spi/spi-mem.c | 7 + include/spi-mem.h | 3 + 3 files changed, 451 insertions(+), 206 deletions(-)
This is useful for extending the default functionality. This mirrors the change in Linux commit 46109648052f ("spi: spi-mem: export spi_mem_default_supports_op()").
Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Pratyush Yadav p.yadav@ti.com ---
(no changes since v1)
include/spi-mem.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/spi-mem.h b/include/spi-mem.h index 8be3e2bf6b..a404d3bbee 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);
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 Reviewed-by: Pratyush Yadav p.yadav@ti.com ---
(no changes since v2)
Changes in v2: - Add more information to exec_op debug message
drivers/spi/spi-mem.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c index c095ae9505..6772367ef7 100644 --- a/drivers/spi/spi-mem.c +++ b/drivers/spi/spi-mem.c @@ -220,6 +220,13 @@ 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 dummy cycles=%u data bytes=%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;
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 ---
(no changes since v1)
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; } }
Writing 1s to this register *unmasks* interrupts. Mask them instead.
Signed-off-by: Sean Anderson seanga2@gmail.com ---
(no changes since v2)
Changes in v2: - Actually mask interrupts
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..4ef948a0b9 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, 0); dw_write(priv, DW_SPI_SSIENR, 1);
/*
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 ---
(no changes since v1)
drivers/spi/designware_spi.c | 151 +++++++++++++++++------------------ 1 file changed, 73 insertions(+), 78 deletions(-)
diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index 4ef948a0b9..6375e6d778 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, 0); + + /* + * 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);
/* @@ -271,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); }
/* @@ -337,11 +336,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,25 +354,21 @@ static int dw_spi_probe(struct udevice *bus) if (ret) return ret;
- if (!init) - return -EINVAL; - ret = init(bus, priv); - if (ret) - return ret; - - 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); - /* 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; }
@@ -510,7 +502,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 +574,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 +734,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 +751,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" }, { } };
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 ---
(no changes since v1)
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 6375e6d778..72cca14887 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 */ @@ -372,14 +368,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 @@ -388,67 +394,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); }
/* @@ -474,19 +544,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); @@ -496,30 +568,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 @@ -531,8 +595,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. @@ -565,9 +636,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; @@ -597,47 +669,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);
On 2021/04/03 8:05, Sean Anderson wrote:
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.
That sounds like the long fight I had on Linux side... Did you account for the fact that the K210 has a buggy fifo depth (it reports 32 but is in fact 31) ?
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
(no changes since v1)
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 6375e6d778..72cca14887 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 */
@@ -372,14 +368,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
@@ -388,67 +394,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);elsetxw = *(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);
}
/* @@ -474,19 +544,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);
@@ -496,30 +568,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
@@ -531,8 +595,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.
@@ -565,9 +636,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;
@@ -597,47 +669,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);
On 4/4/21 8:47 PM, Damien Le Moal wrote:
On 2021/04/03 8:05, Sean Anderson wrote:
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.
That sounds like the long fight I had on Linux side... Did you account for the fact that the K210 has a buggy fifo depth (it reports 32 but is in fact 31) ?
Hm, I forgot about that. I think that's a good line of investigation.
--Sean
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
(no changes since v1)
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 6375e6d778..72cca14887 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 */
@@ -372,14 +368,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
@@ -388,67 +394,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);elsetxw = *(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); }
/*
@@ -474,19 +544,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);
@@ -496,30 +568,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
@@ -531,8 +595,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.
@@ -565,9 +636,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;
@@ -597,47 +669,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);
This capability corresponds to an SSIC_SPI_MODE of 1, 2, or 3. 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 just pretend to have DUAL+QUAD+OCTAL and let the slave determine what we actually use.
Signed-off-by: Sean Anderson seanga2@gmail.com ---
(no changes since v2)
Changes in v2: - Merge CAP_{DUAL,QUAD,OCTAL} into CAP_ENHANCED
drivers/spi/designware_spi.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index 72cca14887..c2639141c6 100644 --- a/drivers/spi/designware_spi.c +++ b/drivers/spi/designware_spi.c @@ -129,6 +129,7 @@ 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_ENHANCED BIT(4) unsigned long caps; unsigned long bus_clk_rate; unsigned int freq; /* Default frequency */ @@ -141,6 +142,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) @@ -249,6 +251,18 @@ 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 a general ENHANCED cap and let the + * slave decide what to use. + */ + if (priv->caps & DW_SPI_CAP_DWC_SSI) { + if (FIELD_GET(DWC_SSI_CTRLR0_SPI_FRF_MASK, cr0)) + priv->caps |= DW_SPI_CAP_ENHANCED; + } else if (FIELD_GET(CTRLR0_SPI_FRF_MASK, cr0)) { + priv->caps |= DW_SPI_CAP_ENHANCED; + } + dw_write(priv, DW_SPI_SSIENR, 1);
/* @@ -746,13 +760,19 @@ 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_ENHANCED) && + (mode & (SPI_RX_DUAL | SPI_TX_DUAL | + SPI_RX_QUAD | SPI_TX_QUAD | + 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; }
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 ---
(no changes since v2)
Changes in v2: - Fix some inconsistencies in register naming and usage
drivers/spi/designware_spi.c | 60 ++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index c2639141c6..e110c5bca1 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,56 @@ #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_MASK 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 length 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 length */ +#define SPI_CTRLR0_INST_L_MASK GENMASK(9, 8) +#define INST_L_0 0x0 +#define INST_L_4 0x1 +#define INST_L_8 0x2 +#define INST_L_16 0x3 +/* Number of "dummy" cycles */ +#define SPI_CTRLR0_WAIT_CYCLES_MASK GENMASK(15, 11) +/* Stretch the clock if the FIFO over/underflows */ +#define SPI_CTRLR0_CLK_STRETCH_EN BIT(30) + #define RX_TIMEOUT 1000 /* timeout in ms */
struct dw_spi_plat {
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 ---
(no changes since v1)
drivers/spi/designware_spi.c | 136 ++++++++++++++++++++++++++++++----- 1 file changed, 119 insertions(+), 17 deletions(-)
diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index e110c5bca1..64a3a8556b 100644 --- a/drivers/spi/designware_spi.c +++ b/drivers/spi/designware_spi.c @@ -218,7 +218,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, @@ -229,12 +230,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_MASK, trans_type) + | FIELD_PREP(SPI_CTRLR0_ADDR_L_MASK, op->addr.nbytes * 2) + | FIELD_PREP(SPI_CTRLR0_INST_L_MASK, INST_L_8) + | 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) @@ -619,6 +644,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"); @@ -697,6 +729,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) { @@ -707,37 +742,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)); @@ -771,6 +841,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) { @@ -781,6 +882,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, };
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 ---
(no changes since v1)
drivers/spi/designware_spi.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c index 64a3a8556b..44fb679fdb 100644 --- a/drivers/spi/designware_spi.c +++ b/drivers/spi/designware_spi.c @@ -258,6 +258,7 @@ static u32 dw_spi_update_spi_cr0(const struct spi_mem_op *op) | FIELD_PREP(SPI_CTRLR0_ADDR_L_MASK, op->addr.nbytes * 2) | FIELD_PREP(SPI_CTRLR0_INST_L_MASK, INST_L_8) | FIELD_PREP(SPI_CTRLR0_WAIT_CYCLES_MASK, wait_cycles) + | SPI_CTRLR0_CLK_STRETCH_EN; }
static int request_gpio_cs(struct udevice *bus) @@ -360,6 +361,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); }
/* @@ -782,8 +786,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);
On 4/2/21 7:05 PM, Sean Anderson wrote:
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. Given that there has been no testing of v2 over the past month, I don't think lack of testing should hold up this series.
Changes in v3:
- Dropped merged patches
- Rebased on u-boot/master
Changes in v2:
- Add more information to exec_op debug message
- Actually mask interrupts
- Merge CAP_{DUAL,QUAD,OCTAL} into CAP_ENHANCED
- Fix some inconsistencies in register naming and usage
- Moved some hunks between commits so things make more sense
Sean Anderson (10): 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: Actually mask interrupts spi: dw: Switch to capabilities spi: dw: Rewrite poll_transfer logic spi: dw: Add ENHANCED cap spi: dw: Define registers for enhanced mode spi: dw: Support enhanced SPI spi: dw: Support clock stretching
drivers/spi/designware_spi.c | 647 ++++++++++++++++++++++++----------- drivers/spi/spi-mem.c | 7 + include/spi-mem.h | 3 + 3 files changed, 451 insertions(+), 206 deletions(-)
Looks like I forgot to bump the version. This should be v3. I can resend if necessary.
--Sean
On Sat, Apr 3, 2021 at 4:37 AM Sean Anderson seanga2@gmail.com wrote:
On 4/2/21 7:05 PM, Sean Anderson wrote:
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. Given that there has been no testing of v2 over the past month, I don't think lack of testing should hold up this series.
Changes in v3:
- Dropped merged patches
- Rebased on u-boot/master
Changes in v2:
- Add more information to exec_op debug message
- Actually mask interrupts
- Merge CAP_{DUAL,QUAD,OCTAL} into CAP_ENHANCED
- Fix some inconsistencies in register naming and usage
- Moved some hunks between commits so things make more sense
Sean Anderson (10): 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: Actually mask interrupts spi: dw: Switch to capabilities spi: dw: Rewrite poll_transfer logic spi: dw: Add ENHANCED cap spi: dw: Define registers for enhanced mode spi: dw: Support enhanced SPI spi: dw: Support clock stretching
drivers/spi/designware_spi.c | 647 ++++++++++++++++++++++++----------- drivers/spi/spi-mem.c | 7 + include/spi-mem.h | 3 + 3 files changed, 451 insertions(+), 206 deletions(-)
Looks like I forgot to bump the version. This should be v3. I can resend if necessary.
Yes, Please.
There are few comments,
1. Patch "spi: spi-mem: Add debug message for spi-mem ops"
As we discussed in the previous version, drop the unnecessary debug statements after ops execution as this patch trying to add more verbose to before ops execution.
2. Comments in Patch v2,06/10
Jagan.
On 4/19/21 3:06 AM, Jagan Teki wrote:
On Sat, Apr 3, 2021 at 4:37 AM Sean Anderson seanga2@gmail.com wrote:
On 4/2/21 7:05 PM, Sean Anderson wrote:
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. Given that there has been no testing of v2 over the past month, I don't think lack of testing should hold up this series.
Changes in v3:
- Dropped merged patches
- Rebased on u-boot/master
Changes in v2:
- Add more information to exec_op debug message
- Actually mask interrupts
- Merge CAP_{DUAL,QUAD,OCTAL} into CAP_ENHANCED
- Fix some inconsistencies in register naming and usage
- Moved some hunks between commits so things make more sense
Sean Anderson (10): 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: Actually mask interrupts spi: dw: Switch to capabilities spi: dw: Rewrite poll_transfer logic spi: dw: Add ENHANCED cap spi: dw: Define registers for enhanced mode spi: dw: Support enhanced SPI spi: dw: Support clock stretching
drivers/spi/designware_spi.c | 647 ++++++++++++++++++++++++----------- drivers/spi/spi-mem.c | 7 + include/spi-mem.h | 3 + 3 files changed, 451 insertions(+), 206 deletions(-)
Looks like I forgot to bump the version. This should be v3. I can resend if necessary.
Yes, Please.
There are few comments,
- Patch "spi: spi-mem: Add debug message for spi-mem ops"
As we discussed in the previous version, drop the unnecessary debug statements after ops execution as this patch trying to add more verbose to before ops execution.
From last time:
dev_dbg(slave->dev,"exec %02Xh %u-%u-%u addr=%llx dummy cycles=%u data bytes=%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);This looks unnecessary to me, we have debug prints at the end of the function, which indeed sufficient.
This is insufficient. First, that information is printed after the op is executed. This is too late if one is trying to debug (e.g.) a hang in the spi driver. It's also missing opcode, widths, address, and dummy bytes. The only duplicated info here is nbytes. Since this is debugging information, I think we should print what is most useful for people debugging this subsystem.
I would like if you could respond to my response before I remove this.
- Comments in Patch v2,06/10
I will look into that before resending.
--Sean
Jagan.
On Mon, Apr 19, 2021 at 6:49 PM Sean Anderson seanga2@gmail.com wrote:
On 4/19/21 3:06 AM, Jagan Teki wrote:
On Sat, Apr 3, 2021 at 4:37 AM Sean Anderson seanga2@gmail.com wrote:
On 4/2/21 7:05 PM, Sean Anderson wrote:
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. Given that there has been no testing of v2 over the past month, I don't think lack of testing should hold up this series.
Changes in v3:
- Dropped merged patches
- Rebased on u-boot/master
Changes in v2:
- Add more information to exec_op debug message
- Actually mask interrupts
- Merge CAP_{DUAL,QUAD,OCTAL} into CAP_ENHANCED
- Fix some inconsistencies in register naming and usage
- Moved some hunks between commits so things make more sense
Sean Anderson (10): 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: Actually mask interrupts spi: dw: Switch to capabilities spi: dw: Rewrite poll_transfer logic spi: dw: Add ENHANCED cap spi: dw: Define registers for enhanced mode spi: dw: Support enhanced SPI spi: dw: Support clock stretching
drivers/spi/designware_spi.c | 647 ++++++++++++++++++++++++----------- drivers/spi/spi-mem.c | 7 + include/spi-mem.h | 3 + 3 files changed, 451 insertions(+), 206 deletions(-)
Looks like I forgot to bump the version. This should be v3. I can resend if necessary.
Yes, Please.
There are few comments,
- Patch "spi: spi-mem: Add debug message for spi-mem ops"
As we discussed in the previous version, drop the unnecessary debug statements after ops execution as this patch trying to add more verbose to before ops execution.
From last time:
dev_dbg(slave->dev,"exec %02Xh %u-%u-%u addr=%llx dummy cycles=%u data bytes=%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);This looks unnecessary to me, we have debug prints at the end of the function, which indeed sufficient.
This is insufficient. First, that information is printed after the op is executed. This is too late if one is trying to debug (e.g.) a hang in the spi driver. It's also missing opcode, widths, address, and dummy bytes. The only duplicated info here is nbytes. Since this is debugging information, I think we should print what is most useful for people debugging this subsystem.
I would like if you could respond to my response before I remove this.
Yes, I have commented based on your previous version comments. What I'm saying here is to adjust even the exiting debug statements in order to not duplicate if your patch changes.
Jagan.
participants (3)
-
Damien Le Moal -
Jagan Teki -
Sean Anderson