On 05/03/2016 12:42 PM, Pavel Machek wrote:
Hi!
This patch replaces the whole unmaintainable indirect write implementation with the one from upcoming Linux CQSPI driver, which went through multiple rounds of thorough review and testing. While this makes the patch look terrifying and violates all best-practices of software development, all
Could we get something less terifying and less violating? :-).
the patch does is it plucks out duplicate ad-hoc code distributed across the driver and replaces it with more compact code doing exactly the same thing.
So this one is just a cleanup, and no behaviour change yet?
Apparently Stefan discovered one behavior change, which I need to look into. Apparently, u-boot can pass unaligned buffer to the driver, which can cause problems.
Pavel
drivers/spi/cadence_qspi_apb.c | 122 +++++++++-------------------------------- 1 file changed, 26 insertions(+), 96 deletions(-)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 7786dd6..00a50cb 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -28,6 +28,7 @@ #include <common.h> #include <asm/io.h> #include <asm/errno.h> +#include <wait_bit.h> #include "cadence_qspi.h"
#define CQSPI_REG_POLL_US (1) /* 1us */ @@ -214,32 +215,6 @@ static void cadence_qspi_apb_read_fifo_data(void *dest, return; }
-static void cadence_qspi_apb_write_fifo_data(const void *dest_ahb_addr,
- const void *src, unsigned int bytes)
-{
- unsigned int temp = 0;
- int i;
- int remaining = bytes;
- unsigned int *dest_ptr = (unsigned int *)dest_ahb_addr;
- unsigned int *src_ptr = (unsigned int *)src;
- while (remaining >= CQSPI_FIFO_WIDTH) {
for (i = CQSPI_FIFO_WIDTH/sizeof(src_ptr) - 1; i >= 0; i--)writel(*(src_ptr+i), dest_ptr+i);src_ptr += CQSPI_FIFO_WIDTH/sizeof(src_ptr);remaining -= CQSPI_FIFO_WIDTH;- }
- if (remaining) {
/* dangling bytes */i = remaining/sizeof(dest_ptr);memcpy(&temp, src_ptr+i, remaining % sizeof(dest_ptr));writel(temp, dest_ptr+i);for (--i; i >= 0; i--)writel(*(src_ptr+i), dest_ptr+i);- }
- return;
-}
/* Read from SRAM FIFO with polling SRAM fill level. */ static int qspi_read_sram_fifo_poll(const void *reg_base, void *dest_addr, const void *src_addr, unsigned int num_bytes) @@ -276,44 +251,6 @@ static int qspi_read_sram_fifo_poll(const void *reg_base, void *dest_addr, return 0; }
-/* Write to SRAM FIFO with polling SRAM fill level. */ -static int qpsi_write_sram_fifo_push(struct cadence_spi_platdata *plat,
const void *src_addr, unsigned int num_bytes)-{
- const void *reg_base = plat->regbase;
- void *dest_addr = plat->ahbbase;
- unsigned int retry = CQSPI_REG_RETRY;
- unsigned int sram_level;
- unsigned int wr_bytes;
- unsigned char *src = (unsigned char *)src_addr;
- int remaining = num_bytes;
- unsigned int page_size = plat->page_size;
- unsigned int sram_threshold_words = CQSPI_REG_SRAM_THRESHOLD_WORDS;
- while (remaining > 0) {
retry = CQSPI_REG_RETRY;while (retry--) {sram_level = CQSPI_GET_WR_SRAM_LEVEL(reg_base);if (sram_level <= sram_threshold_words)break;}if (!retry) {printf("QSPI: SRAM fill level (0x%08x) not hit lower expected level (0x%08x)",sram_level, sram_threshold_words);return -1;}/* Write a page or remaining bytes. */wr_bytes = (remaining > page_size) ?page_size : remaining;cadence_qspi_apb_write_fifo_data(dest_addr, src, wr_bytes);src += wr_bytes;remaining -= wr_bytes;- }
- return 0;
-}
void cadence_qspi_apb_controller_enable(void *reg_base) { unsigned int reg; @@ -810,48 +747,41 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, }
int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
- unsigned int txlen, const u8 *txbuf)
- unsigned int n_tx, const u8 *txbuf)
{
- unsigned int reg = 0;
- unsigned int retry;
unsigned int page_size = plat->page_size;
unsigned int remaining = n_tx;
unsigned int write_bytes;
int ret;
/* Configure the indirect read transfer bytes */
- writel(txlen, plat->regbase + CQSPI_REG_INDIRECTWRBYTES);
writel(n_tx, plat->regbase + CQSPI_REG_INDIRECTWRBYTES);
/* Start the indirect write transfer */ writel(CQSPI_REG_INDIRECTWR_START_MASK, plat->regbase + CQSPI_REG_INDIRECTWR);
- if (qpsi_write_sram_fifo_push(plat, (const void *)txbuf, txlen))
goto failwr;- /* Wait until last write is completed (FIFO empty) */
- retry = CQSPI_REG_RETRY;
- while (retry--) {
reg = CQSPI_GET_WR_SRAM_LEVEL(plat->regbase);if (reg == 0)break;udelay(1);- }
- if (reg != 0) {
printf("QSPI: timeout for indirect write\n");goto failwr;- }
- while (remaining > 0) {
write_bytes = remaining > page_size ? page_size : remaining;writesl(plat->ahbbase, txbuf, DIV_ROUND_UP(write_bytes, 4));ret = wait_for_bit("QSPI", plat->regbase + CQSPI_REG_SDRAMLEVEL,CQSPI_REG_SDRAMLEVEL_WR_MASK <<CQSPI_REG_SDRAMLEVEL_WR_LSB, 0, 10, 0);if (ret) {printf("Indirect write timed out (%i)\n", ret);goto failwr;}
- /* Check flash indirect controller status */
- retry = CQSPI_REG_RETRY;
- while (retry--) {
reg = readl(plat->regbase + CQSPI_REG_INDIRECTWR);if (reg & CQSPI_REG_INDIRECTWR_DONE_MASK)break;udelay(1);
txbuf += write_bytes;remaining -= write_bytes;
- if (!(reg & CQSPI_REG_INDIRECTWR_DONE_MASK)) {
printf("QSPI: indirect completion status error with reg 0x%08x\n",reg);
- /* Check indirect done status */
- ret = wait_for_bit("QSPI", plat->regbase + CQSPI_REG_INDIRECTWR,
CQSPI_REG_INDIRECTWR_DONE_MASK, 1, 10, 0);- if (ret) {
printf("Indirect write completion error (%i)\n", ret);@@ -864,7 +794,7 @@ failwr: /* Cancel the indirect write */ writel(CQSPI_REG_INDIRECTWR_CANCEL_MASK, plat->regbase + CQSPI_REG_INDIRECTWR);
- return -1;
- return ret;
}
void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy)