[U-Boot] [PATCH v5 13/21] mmc: dw_mmc: support fifo mode in dwc mmc driver

[Simon Glass <sjg@chromium.org>]

+Pantelis (mmc maintainer)

Hi Lin,

On 10 November 2015 at 03:24, Lin Huang <hl@rock-chips.com> wrote:
> some soc(rk3036 etc) use dw_mmc but do not have internal dma,
> so we implement fifo mode to read and write data.
>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
>  drivers/mmc/dw_mmc.c | 81 +++++++++++++++++++++++++++++++++++++++++++---------
>  include/dwmmc.h      |  5 ++++
>  2 files changed, 72 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> index 26d34ae..18be055 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -118,6 +118,8 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>         u32 mask, ctrl;
>         ulong start = get_timer(0);
>         struct bounce_buffer bbstate;
> +       unsigned int fifo_len, fifo_depth, size;
> +       unsigned int *buf = NULL;
>
>         while (dwmci_readl(host, DWMCI_STATUS) & DWMCI_BUSY) {
>                 if (get_timer(start) > timeout) {
> @@ -129,17 +131,28 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>         dwmci_writel(host, DWMCI_RINTSTS, DWMCI_INTMSK_ALL);
>
>         if (data) {
> -               if (data->flags == MMC_DATA_READ) {
> -                       bounce_buffer_start(&bbstate, (void*)data->dest,
> -                                           data->blocksize *
> -                                           data->blocks, GEN_BB_WRITE);
> +               if (host->fifo_mode) {
> +                       if (data->flags == MMC_DATA_READ)
> +                               buf = (unsigned int *)data->dest;
> +                       else
> +                               buf = (unsigned int *)data->src;
> +                       dwmci_writel(host, DWMCI_BLKSIZ, data->blocksize);
> +                       dwmci_writel(host, DWMCI_BYTCNT,
> +                                    data->blocksize * data->blocks);
> +                       dwmci_wait_reset(host, DWMCI_CTRL_FIFO_RESET);
>                 } else {
> -                       bounce_buffer_start(&bbstate, (void*)data->src,
> -                                           data->blocksize *
> -                                           data->blocks, GEN_BB_READ);
> +                       if (data->flags == MMC_DATA_READ) {
> +                               bounce_buffer_start(&bbstate, (void*)data->dest,
> +                                               data->blocksize *
> +                                               data->blocks, GEN_BB_WRITE);
> +                       } else {
> +                               bounce_buffer_start(&bbstate, (void*)data->src,
> +                                               data->blocksize *
> +                                               data->blocks, GEN_BB_READ);
> +                       }
> +                       dwmci_prepare_data(host, data, cur_idmac,
> +                                          bbstate.bounce_buffer);
>                 }
> -               dwmci_prepare_data(host, data, cur_idmac,
> -                                  bbstate.bounce_buffer);
>         }
>
>         dwmci_writel(host, DWMCI_CMDARG, cmd->cmdarg);
> @@ -215,6 +228,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>         if (data) {
>                 start = get_timer(0);
>                 timeout = 240000;
> +               size = data->blocksize * data->blocks / 4;
>                 for (;;) {
>                         mask = dwmci_readl(host, DWMCI_RINTSTS);
>                         /* Error during data transfer. */
> @@ -224,6 +238,44 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>                                 break;
>                         }
>

This looks too long to me. Can you first add a patch to move
everything inside the 'if (data)' block to a separate static function?

> +                       /* only fifo mode need it */
> +                       if (data->flags == MMC_DATA_READ && host->fifo_mode) {

I think both of these blocks should be in the same 'if (host->fifo_mode)'

In fact, can it be 'if (host->fifo_mode && size)' ?

> +                               if ((dwmci_readl(host, DWMCI_RINTSTS) &&
> +                                    DWMCI_INTMSK_RXDR) && size) {
> +                                       fifo_len = dwmci_readl(host,
> +                                                              DWMCI_STATUS);
> +                                       fifo_len = (fifo_len >> DWMCI_FIFO_SHIFT)
> +                                                   & DWMCI_FIFO_MASK;
> +                                       for (i = 0; i < fifo_len; i++)
> +                                               *buf++ = dwmci_readl(host,
> +                                                               DWMCI_DATA);
> +                                       dwmci_writel(host, DWMCI_RINTSTS,
> +                                                    DWMCI_INTMSK_RXDR);
> +                                       size = size > fifo_len ?
> +                                                       (size - fifo_len) : 0;

Then the above line can be common at the end of the if ()/

> +                               }
> +                       } else if (data->flags == MMC_DATA_WRITE &&
> +                                  host->fifo_mode) {
> +                               fifo_depth = (((host->fifoth_val &
> +                                               RX_WMARK_MASK) >>
> +                                               RX_WMARK_SHIFT) + 1) * 2;
> +                               if ((dwmci_readl(host, DWMCI_RINTSTS) &&
> +                                    DWMCI_INTMSK_TXDR) && size) {
> +                                       fifo_len = dwmci_readl(host,
> +                                                       DWMCI_STATUS);
> +                                       fifo_len = fifo_depth -
> +                                               ((fifo_len >> DWMCI_FIFO_SHIFT)
> +                                               & DWMCI_FIFO_MASK);
> +                                       for (i = 0; i < fifo_len; i++)
> +                                               dwmci_writel(host, DWMCI_DATA,
> +                                                            *buf++);
> +                                       dwmci_writel(host, DWMCI_RINTSTS,
> +                                                    DWMCI_INTMSK_TXDR);
> +                                       size = size > fifo_len ?
> +                                                       (size - fifo_len) : 0;
> +                               }
> +                       }
> +
>                         /* Data arrived correctly. */
>                         if (mask & DWMCI_INTMSK_DTO) {
>                                 ret = 0;
> @@ -241,11 +293,12 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>
>                 dwmci_writel(host, DWMCI_RINTSTS, mask);
>
> -               ctrl = dwmci_readl(host, DWMCI_CTRL);
> -               ctrl &= ~(DWMCI_DMA_EN);
> -               dwmci_writel(host, DWMCI_CTRL, ctrl);
> -
> -               bounce_buffer_stop(&bbstate);
> +               if (!host->fifo_mode) {
> +                       ctrl = dwmci_readl(host, DWMCI_CTRL);
> +                       ctrl &= ~(DWMCI_DMA_EN);
> +                       dwmci_writel(host, DWMCI_CTRL, ctrl);
> +                       bounce_buffer_stop(&bbstate);
> +               }
>         }
>
>         udelay(100);
> diff --git a/include/dwmmc.h b/include/dwmmc.h
> index 25cf42c..8d26e5e 100644
> --- a/include/dwmmc.h
> +++ b/include/dwmmc.h
> @@ -105,6 +105,8 @@
>
>  /* Status Register */
>  #define DWMCI_BUSY             (1 << 9)
> +#define DWMCI_FIFO_MASK                0x1ff
> +#define DWMCI_FIFO_SHIFT       17
>
>  /* FIFOTH Register */
>  #define MSIZE(x)               ((x) << 28)
> @@ -180,6 +182,9 @@ struct dwmci_host {
>         unsigned int (*get_mmc_clk)(struct dwmci_host *host, uint freq);
>
>         struct mmc_config cfg;
> +
> +       /* use fifo mode to read and write data */
> +       u32 fifo_mode;

Can this be bool?

>  };
>
>  struct dwmci_idmac {
> --
> 1.9.1
>

Regards,
Simon