[U-Boot] [PATCH v3 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: [PATCH v6 0/4] spi: cadence_spi: Adopt Linux DT bindings
Tested on K2G EVM.
v3: Rebased on top of latest u-boot-spi/master changes.
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_apb.c | 53 ++++++++++++++++++++-------------------- include/configs/k2g_evm.h | 1 - include/configs/socfpga_common.h | 1 - include/configs/stv0991.h | 1 - 4 files changed, 26 insertions(+), 30 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 Acked-by: Marek Vasut marex@denx.de Reviewed-by: Jason Rush jarush@gmail.com Acked-by: Jason Rush jarush@gmail.com --- drivers/spi/cadence_qspi_apb.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 4b1ddd79b30b..82e8a9ec4d1c 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,16 @@ 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 +674,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 +681,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 Acked-by: Marek Vasut marex@denx.de Acked-by: Simon Goldschmidt sgoldschmidt@de.pepperl-fuchs.com Reviewed-by: Jason Rush jarush@gmail.com Acked-by: Jason Rush jarush@gmail.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 82e8a9ec4d1c..a57109865d29 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 */ @@ -722,17 +721,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); @@ -743,11 +731,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_le32(plat->regbase + CQSPI_REG_SDRAMLEVEL, CQSPI_REG_SDRAMLEVEL_WR_MASK << @@ -757,7 +745,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; }
@@ -768,7 +756,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, @@ -779,7 +766,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. 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 Acked-by: Marek Vasut marex@denx.de Acked-by: Simon Goldschmidt sgoldschmidt@de.pepperl-fuchs.com Reviewed-by: Jason Rush jarush@gmail.com Acked-by: Jason Rush jarush@gmail.com --- drivers/spi/cadence_qspi_apb.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index a57109865d29..6d9a7941b470 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -30,6 +30,7 @@ #include <linux/errno.h> #include <wait_bit.h> #include <spi.h> +#include <malloc.h> #include "cadence_qspi.h"
#define CQSPI_REG_POLL_US 1 /* 1us */ @@ -719,9 +720,23 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, { unsigned int page_size = plat->page_size; unsigned int remaining = n_tx; + const u8 *bb_txbuf = txbuf; + void *bounce_buf = NULL; unsigned int write_bytes; int ret;
+ /* + * Use bounce buffer for non 32 bit aligned txbuf to avoid data + * aborts + */ + if ((uintptr_t)txbuf % 4) { + bounce_buf = malloc(n_tx); + if (!bounce_buf) + return -ENOMEM; + memcpy(bounce_buf, txbuf, n_tx); + bb_txbuf = bounce_buf; + } + /* Configure the indirect read transfer bytes */ writel(n_tx, plat->regbase + CQSPI_REG_INDIRECTWRBYTES);
@@ -731,11 +746,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, bb_txbuf, write_bytes >> 2); + if (write_bytes % 4) + writesb(plat->ahbbase, + bb_txbuf + rounddown(write_bytes, 4), + write_bytes % 4);
ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_SDRAMLEVEL, CQSPI_REG_SDRAMLEVEL_WR_MASK << @@ -745,7 +760,7 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, goto failwr; }
- txbuf += write_bytes; + bb_txbuf += write_bytes; remaining -= write_bytes; }
@@ -760,12 +775,16 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat, /* Clear indirect completion status */ writel(CQSPI_REG_INDIRECTWR_DONE, plat->regbase + CQSPI_REG_INDIRECTWR); + if (bounce_buf) + free(bounce_buf); return 0;
failwr: /* Cancel the indirect write */ writel(CQSPI_REG_INDIRECTWR_CANCEL, plat->regbase + CQSPI_REG_INDIRECTWR); + if (bounce_buf) + free(bounce_buf); return ret; }

On Wed, Jan 24, 2018 at 10:44 AM, Vignesh R vigneshr@ti.com wrote:
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: [PATCH v6 0/4] spi: cadence_spi: Adopt Linux DT bindings
Tested on K2G EVM.
v3: Rebased on top of latest u-boot-spi/master changes.
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
Applied to u-boot-spi/master, thanks!
participants (2)
-
Jagan Teki
-
Vignesh R