Re: [PATCH v2 07/11] mmc: fsl_esdhc_imx: simplify 64bit check for SDMA transfers

[Jaehoon Chung]

On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit da86e8cfcb03ed5c1d8e0718bc8bc8583e60ced8 ]
> 
> SDMA can only do DMA with 32 bit addresses. This is true for all
> architectures (just doesn't apply to 32 bit ones). Simplify the code and
> remove unnecessary CONFIG_FSL_LAYERSCAPE.
> 
> Also make the error message more concise.
> 
> 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>

Just add minor comments.

> ---
> 
> (no changes since v1)
> 
>  drivers/mmc/fsl_esdhc_imx.c | 33 ++++++---------------------------
>  1 file changed, 6 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index 95cde9e410..3588056759 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -282,10 +282,7 @@ static int esdhc_setup_data(struct fsl_esdhc_priv *priv, 
> struct mmc *mmc,
>  {
>       int timeout;
>       struct fsl_esdhc *regs = priv->esdhc_regs;
> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) 
> || \
> -     defined(CONFIG_IMX8ULP)
>       dma_addr_t addr;
> -#endif
>       uint wml_value;
>  
>       wml_value = data->blocksize/4;
> @@ -296,16 +293,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv 
> *priv, struct mmc *mmc,
>  
>               esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) 
> || \
> -     defined(CONFIG_IMX8ULP)
>               addr = virt_to_phys((void *)(data->dest));
>               if (upper_32_bits(addr))
> -                     printf("Error found for upper 32 bits\n");
> -             else
> -                     esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
> -#else
> -             esdhc_write32(&regs->dsaddr, (u32)data->dest);
> -#endif
> +                     printf("Cannot use 64 bit addresses with SDMA\n");
> +             esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>  #endif
>       } else {
>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> @@ -334,16 +325,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv 
> *priv, struct mmc *mmc,
>               esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
>                                       wml_value << 16);
>  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) 
> || \
> -             defined(CONFIG_IMX8ULP)
>               addr = virt_to_phys((void *)(data->src));
>               if (upper_32_bits(addr))
> -                     printf("Error found for upper 32 bits\n");
> -             else
> -                     esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
> -#else
> -             esdhc_write32(&regs->dsaddr, (u32)data->src);
> -#endif
> +                     printf("Cannot use 64 bit addresses with SDMA\n");

Fix indentation?

> +             esdhc_write32(&regs->dsaddr, lower_32_bits(addr));
>  #endif
>       }
>  
> @@ -400,18 +385,12 @@ static void check_and_invalidate_dcache_range
>       unsigned end = 0;
>       unsigned size = roundup(ARCH_DMA_MINALIGN,
>                               data->blocks*data->blocksize);
> -#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) || defined(CONFIG_IMX8M) 
> || \
> -     defined(CONFIG_IMX8ULP)
>       dma_addr_t addr;
>  
>       addr = virt_to_phys((void *)(data->dest));
>       if (upper_32_bits(addr))
> -             printf("Error found for upper 32 bits\n");
> -     else
> -             start = lower_32_bits(addr);
> -#else
> -     start = (unsigned)data->dest;
> -#endif
> +             printf("Cannot use 64 bit addresses with SDMA\n");

Ditto.

> +     start = lower_32_bits(addr);
>       end = start + size;
>       invalidate_dcache_range(start, end);
>  }
>