Re: [PATCH v2 09/11] mmc: fsl_esdhc_imx: simplify esdhc_setup_data()

[Jaehoon Chung]

On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit 7e48a028a42c111ba38a90b86e5f57dace980fa0 ]
> 
> First, we need the waterlevel setting for PIO mode only. Secondy, both DMA
> setup code is identical for both directions, except for the data pointer.
> Thus, unify them.
> 
> Signed-off-by: Michael Walle <mich...@walle.cc>
> Signed-off-by: Sean Anderson <sean.ander...@seco.com>


Reviewed-by: Jaehoon Chung <jh80.ch...@samsung.com>

Best Regards,
Jaehoon Chung

> ---
> 
> (no changes since v1)
> 
>  drivers/mmc/fsl_esdhc_imx.c | 89 ++++++++++++++++++++++---------------
>  1 file changed, 52 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index afc8259323..aa3d0877cb 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -279,59 +279,74 @@ static void esdhc_pio_read_write(struct fsl_esdhc_priv 
> *priv,
>  }
>  #endif
>  
> -static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
> -                         struct mmc_data *data)
> +#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
> +static void esdhc_setup_watermark_level(struct fsl_esdhc_priv *priv,
> +                                     struct mmc_data *data)
>  {
> -     int timeout;
> -     uint trans_bytes = data->blocksize * data->blocks;
>       struct fsl_esdhc *regs = priv->esdhc_regs;
> -     uint wml_value;
> -
> -     wml_value = data->blocksize/4;
> +     uint wml_value = data->blocksize / 4;
>  
>       if (data->flags & MMC_DATA_READ) {
>               if (wml_value > WML_RD_WML_MAX)
>                       wml_value = WML_RD_WML_MAX_VAL;
>  
>               esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
> -#ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -             priv->dma_addr = dma_map_single(data->dest, trans_bytes,
> -                                             mmc_get_dma_dir(data));
> -             if (upper_32_bits(priv->dma_addr))
> -                     printf("Cannot use 64 bit addresses with SDMA\n");
> -             esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
> -#endif
>       } else {
>               if (wml_value > WML_WR_WML_MAX)
>                       wml_value = WML_WR_WML_MAX_VAL;
> -             if (priv->wp_enable) {
> -                     if ((esdhc_read32(&regs->prsstat) &
> -                         PRSSTAT_WPSPL) == 0) {
> -                             printf("\nThe SD card is locked. Can not write 
> to a locked card.\n\n");
> -                             return -ETIMEDOUT;
> -                     }
> -             } else {
> -#if CONFIG_IS_ENABLED(DM_GPIO)
> -                     if (dm_gpio_is_valid(&priv->wp_gpio) &&
> -                         dm_gpio_get_value(&priv->wp_gpio)) {
> -                             printf("\nThe SD card is locked. Can not write 
> to a locked card.\n\n");
> -                             return -ETIMEDOUT;
> -                     }
> -#endif
> -             }
>  
>               esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
> -                                     wml_value << 16);
> -#ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -             priv->dma_addr = dma_map_single((void *)data->src, trans_bytes,
> -                                             mmc_get_dma_dir(data));
> -             if (upper_32_bits(priv->dma_addr))
> -                     printf("Cannot use 64 bit addresses with SDMA\n");
> -             esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
> -#endif
> +                                wml_value << 16);
>       }
> +}
> +#endif
>  
> +static void esdhc_setup_dma(struct fsl_esdhc_priv *priv, struct mmc_data 
> *data)
> +{
> +     uint trans_bytes = data->blocksize * data->blocks;
> +     struct fsl_esdhc *regs = priv->esdhc_regs;
> +     void *buf;
> +
> +     if (data->flags & MMC_DATA_WRITE)
> +             buf = (void *)data->src;
> +     else
> +             buf = data->dest;
> +
> +     priv->dma_addr = dma_map_single(buf, trans_bytes,
> +                                     mmc_get_dma_dir(data));
> +     if (upper_32_bits(priv->dma_addr))
> +             printf("Cannot use 64 bit addresses with SDMA\n");
> +     esdhc_write32(&regs->dsaddr, lower_32_bits(priv->dma_addr));
>       esdhc_write32(&regs->blkattr, data->blocks << 16 | data->blocksize);
> +}
> +
> +static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
> +                         struct mmc_data *data)
> +{
> +     int timeout;
> +     bool is_write = data->flags & MMC_DATA_WRITE;
> +     struct fsl_esdhc *regs = priv->esdhc_regs;
> +
> +     if (is_write) {
> +             if (priv->wp_enable && !(esdhc_read32(&regs->prsstat) & 
> PRSSTAT_WPSPL)) {
> +                     printf("Cannot write to locked SD card.\n");
> +                     return -EINVAL;
> +             } else {
> +#if CONFIG_IS_ENABLED(DM_GPIO)
> +                     if (dm_gpio_is_valid(&priv->wp_gpio) &&
> +                         dm_gpio_get_value(&priv->wp_gpio)) {
> +                             printf("Cannot write to locked SD card.\n");
> +                             return -EINVAL;
> +                     }
> +#endif
> +             }
> +     }
> +
> +#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
> +     esdhc_setup_watermark_level(priv, data);
> +#else
> +     esdhc_setup_dma(priv, data);
> +#endif
>  
>       /* Calculate the timeout period for data transactions */
>       /*
>