[U-Boot] [PATCH 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses
This series reverts use of bounce_buf.c for non-DMA related alignment restriction and replaces it with local bounce buffer to handle problems with non 32 bit aligned writes on TI platforms. Based on top of Jason's series: https://patchwork.ozlabs.org/cover/856431/
Tested on K2G EVM.
Goldschmidt Simon (1): Revert "spi: cadence_qspi_apb: Use 32 bit indirect read transaction when possible"
Vignesh R (2): Revert "spi: cadence_qspi_apb: Use 32 bit indirect write transaction when possible" spi: cadence_qspi_apb: Make flash writes 32 bit aligned
drivers/spi/cadence_qspi.c | 13 ++++++++++++- drivers/spi/cadence_qspi.h | 1 + drivers/spi/cadence_qspi_apb.c | 42 +++++++++------------------------------- include/configs/k2g_evm.h | 1 - include/configs/socfpga_common.h | 1 - include/configs/stv0991.h | 1 - 6 files changed, 22 insertions(+), 37 deletions(-)
From: Goldschmidt Simon sgoldschmidt@de.pepperl-fuchs.com
This reverts commit b63b46313ed29e9b0c36b3d6b9407f6eade40c8f.
This commit changed cadence_qspi_apb to use bouncebuf.c, which invalidates the data cache after reading. This is meant for dma transfers only and breaks the cadence_qspi driver which copies via cpu only: data that is copied by the cpu is in cache only and the cache invalidation at the end throws away this data.
Signed-off-by: Simon Goldschmidt sgoldschmidt@de.pepperl-fuchs.com Signed-off-by: Vignesh R vigneshr@ti.com --- drivers/spi/cadence_qspi_apb.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 8309ab87940c..c7cb33aa8fc3 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -627,8 +627,6 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, { unsigned int remaining = n_rx; unsigned int bytes_to_read = 0; - struct bounce_buffer bb; - u8 *bb_rxbuf; int ret;
writel(n_rx, plat->regbase + CQSPI_REG_INDIRECTRDBYTES); @@ -637,11 +635,6 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, writel(CQSPI_REG_INDIRECTRD_START, plat->regbase + CQSPI_REG_INDIRECTRD);
- ret = bounce_buffer_start(&bb, (void *)rxbuf, n_rx, GEN_BB_WRITE); - if (ret) - return ret; - bb_rxbuf = bb.bounce_buffer; - while (remaining > 0) { ret = cadence_qspi_wait_for_data(plat); if (ret < 0) { @@ -655,13 +648,12 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, bytes_to_read *= plat->fifo_width; bytes_to_read = bytes_to_read > remaining ? remaining : bytes_to_read; - readsl(plat->ahbbase, bb_rxbuf, bytes_to_read >> 2); - if (bytes_to_read % 4) - readsb(plat->ahbbase, - bb_rxbuf + rounddown(bytes_to_read, 4), - bytes_to_read % 4); - - bb_rxbuf += bytes_to_read; + /* Handle non-4-byte aligned access to avoid data abort. */ + if (((uintptr_t)rxbuf % 4) || (bytes_to_read % 4)) + readsb(plat->ahbbase, rxbuf, bytes_to_read); + else + readsl(plat->ahbbase, rxbuf, bytes_to_read >> 2); + rxbuf += bytes_to_read; remaining -= bytes_to_read; bytes_to_read = cadence_qspi_get_rd_sram_level(plat); } @@ -678,7 +670,6 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, /* Clear indirect completion status */ writel(CQSPI_REG_INDIRECTRD_DONE, plat->regbase + CQSPI_REG_INDIRECTRD); - bounce_buffer_stop(&bb);
return 0;
@@ -686,7 +677,6 @@ failrd: /* Cancel the indirect read */ writel(CQSPI_REG_INDIRECTRD_CANCEL, plat->regbase + CQSPI_REG_INDIRECTRD); - bounce_buffer_stop(&bb); return ret; }
This reverts commit 57897c13de03ac0136d64641a3eab526c6810387.
Using bounce_buf.c to handle non-DMA alignment problems is bad as bounce_buf.c does cache manipulations which is not required. Therefore revert this patch in favour of local bounce buffer solution in the next patch.
Signed-off-by: Vignesh R vigneshr@ti.com --- drivers/spi/cadence_qspi_apb.c | 26 ++++++-------------------- include/configs/k2g_evm.h | 1 - include/configs/socfpga_common.h | 1 - include/configs/stv0991.h | 1 - 4 files changed, 6 insertions(+), 23 deletions(-)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index c7cb33aa8fc3..fa8af33ae19b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -30,7 +30,6 @@ #include <linux/errno.h> #include <wait_bit.h> #include <spi.h> -#include <bouncebuf.h> #include "cadence_qspi.h"
#define CQSPI_REG_POLL_US 1 /* 1us */ @@ -718,17 +717,6 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, unsigned int remaining = n_tx; unsigned int write_bytes; int ret; - struct bounce_buffer bb; - u8 *bb_txbuf; - - /* - * Handle non-4-byte aligned accesses via bounce buffer to - * avoid data abort. - */ - ret = bounce_buffer_start(&bb, (void *)txbuf, n_tx, GEN_BB_READ); - if (ret) - return ret; - bb_txbuf = bb.bounce_buffer;
/* Configure the indirect read transfer bytes */ writel(n_tx, plat->regbase + CQSPI_REG_INDIRECTWRBYTES); @@ -739,11 +727,11 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
while (remaining > 0) { write_bytes = remaining > page_size ? page_size : remaining; - writesl(plat->ahbbase, bb_txbuf, write_bytes >> 2); - if (write_bytes % 4) - writesb(plat->ahbbase, - bb_txbuf + rounddown(write_bytes, 4), - write_bytes % 4); + /* Handle non-4-byte aligned access to avoid data abort. */ + if (((uintptr_t)txbuf % 4) || (write_bytes % 4)) + writesb(plat->ahbbase, txbuf, write_bytes); + else + writesl(plat->ahbbase, txbuf, write_bytes >> 2);
ret = wait_for_bit("QSPI", plat->regbase + CQSPI_REG_SDRAMLEVEL, CQSPI_REG_SDRAMLEVEL_WR_MASK << @@ -753,7 +741,7 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, goto failwr; }
- bb_txbuf += write_bytes; + txbuf += write_bytes; remaining -= write_bytes; }
@@ -764,7 +752,6 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, printf("Indirect write completion error (%i)\n", ret); goto failwr; } - bounce_buffer_stop(&bb);
/* Clear indirect completion status */ writel(CQSPI_REG_INDIRECTWR_DONE, @@ -775,7 +762,6 @@ failwr: /* Cancel the indirect write */ writel(CQSPI_REG_INDIRECTWR_CANCEL, plat->regbase + CQSPI_REG_INDIRECTWR); - bounce_buffer_stop(&bb); return ret; }
diff --git a/include/configs/k2g_evm.h b/include/configs/k2g_evm.h index 535e7124fc80..0a38922a519e 100644 --- a/include/configs/k2g_evm.h +++ b/include/configs/k2g_evm.h @@ -93,7 +93,6 @@ #ifndef CONFIG_SPL_BUILD #define CONFIG_CADENCE_QSPI #define CONFIG_CQSPI_REF_CLK 384000000 -#define CONFIG_BOUNCE_BUFFER #endif
#define SPI_MTD_PARTS KEYSTONE_SPI1_MTD_PARTS diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index ec8bb500504a..f6607b101ec5 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -184,7 +184,6 @@ unsigned int cm_get_l4_sp_clk_hz(void); unsigned int cm_get_qspi_controller_clk_hz(void); #define CONFIG_CQSPI_REF_CLK cm_get_qspi_controller_clk_hz() #endif -#define CONFIG_BOUNCE_BUFFER
/* * Designware SPI support diff --git a/include/configs/stv0991.h b/include/configs/stv0991.h index fd96979bf897..beb8f1ae9a92 100644 --- a/include/configs/stv0991.h +++ b/include/configs/stv0991.h @@ -64,7 +64,6 @@ + */ #ifdef CONFIG_OF_CONTROL /* QSPI is controlled via DT */ #define CONFIG_CQSPI_REF_CLK ((30/4)/2)*1000*1000 -#define CONFIG_BOUNCE_BUFFER
#endif
Make flash writes 32 bit aligned by using bounce buffers to deal with non 32 bit aligned buffers. Allocate a 512 byte bounce buffer (max known page size currently) for this case. This is required because as per TI K2G TRM[1], the external master is only permitted to issue 32-bit data interface writes until the last word of an indirect transfer. Otherwise indirect writes is known to fail sometimes.
[1] http://www.ti.com/lit/ug/spruhy8g/spruhy8g.pdf
Signed-off-by: Vignesh R vigneshr@ti.com --- drivers/spi/cadence_qspi.c | 13 ++++++++++++- drivers/spi/cadence_qspi.h | 1 + drivers/spi/cadence_qspi_apb.c | 10 +++++----- 3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 991a7160bdb8..49c9b477e678 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -158,6 +158,10 @@ static int cadence_spi_probe(struct udevice *bus)
priv->regbase = plat->regbase; priv->ahbbase = plat->ahbbase; + /* Bounce buf to handle unaligned writes. 512B is max pagesize */ + priv->bounce_buf = malloc(512); + if (!priv->bounce_buf) + return -ENOMEM;
if (!priv->qspi_is_init) { cadence_qspi_apb_controller_init(plat); @@ -259,8 +263,15 @@ static int cadence_spi_xfer(struct udevice *dev, unsigned int bitlen, err = cadence_qspi_apb_indirect_write_setup (plat, priv->cmd_len, cmd_buf); if (!err) { + const u8 *txbuf = dout; + + if ((uintptr_t)txbuf % 4) { + memcpy(priv->bounce_buf, dout, + data_bytes); + txbuf = priv->bounce_buf; + } err = cadence_qspi_apb_indirect_write_execute - (plat, data_bytes, dout); + (plat, data_bytes, txbuf); } break; default: diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index 83154210bd95..c97419af3de3 100644 --- a/drivers/spi/cadence_qspi.h +++ b/drivers/spi/cadence_qspi.h @@ -43,6 +43,7 @@ struct cadence_spi_priv { unsigned int qspi_calibrated_hz; unsigned int qspi_calibrated_cs; unsigned int previous_hz; + void *bounce_buf; };
/* Functions call declaration */ diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index fa8af33ae19b..fd3ece4cb129 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -727,11 +727,11 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
while (remaining > 0) { write_bytes = remaining > page_size ? page_size : remaining; - /* Handle non-4-byte aligned access to avoid data abort. */ - if (((uintptr_t)txbuf % 4) || (write_bytes % 4)) - writesb(plat->ahbbase, txbuf, write_bytes); - else - writesl(plat->ahbbase, txbuf, write_bytes >> 2); + writesl(plat->ahbbase, txbuf, write_bytes >> 2); + if (write_bytes % 4) + writesb(plat->ahbbase, + txbuf + rounddown(write_bytes, 4), + write_bytes % 4);
ret = wait_for_bit("QSPI", plat->regbase + CQSPI_REG_SDRAMLEVEL, CQSPI_REG_SDRAMLEVEL_WR_MASK <<
participants (1)
-
Vignesh R