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?

                                                                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);
>               goto failwr;
>       }
>  
> @@ -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)

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to