[v2] mmc: sdhci: introduce adma_write_desc() hook to struct sdhci_ops

From: Ian Roberts <ian.roberts@timesys.com>

Add this hook so that it can be overridden with driver specific
implementations. We also let the original sdhci_adma_write_desc()
accept &desc so that the function can set its new value. Then export
the function so that it could be reused by driver's specific
implementations.

The above is a port of Linux kernel commit 54552e4948cbf

In addition, allow drivers to allocate their own ADMA descriptor
tables if additional space is required.

Finally, fix the assignment of adma_addr to fix compiler warning
on 64-bit platforms that still use 32-bit DMA addressing.

Co-developed-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
Co-developed-by: Greg Malysa <greg.malysa@timesys.com>
Signed-off-by: Greg Malysa <greg.malysa@timesys.com>
Signed-off-by: Ian Roberts <ian.roberts@timesys.com>

---

Changes in v2:
- Switch from #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA) to #ifdef
  CONFIG_MMC_SDHCI_ADMA_HELPERS, as CONFIG_IS_ENABLED() causes a build
  failure during SPL builds when CONFIG_SPL_MMC is set.
- Passed CI before submitting this time

---
 drivers/mmc/fsl_esdhc.c  |  2 +-
 drivers/mmc/sdhci-adma.c | 41 +++++++++++++++++++++++++++-------------
 drivers/mmc/sdhci.c      |  8 +++++---
 include/sdhci.h          | 12 ++++++++++--
 4 files changed, 44 insertions(+), 19 deletions(-)

Hello Greg, Ian, Jaehoon!

On Mon, 2024-04-22 at 15:00 -0400, Greg Malysa wrote:
> From: Ian Roberts <ian.roberts@timesys.com>
> 
> Add this hook so that it can be overridden with driver specific
> implementations. We also let the original sdhci_adma_write_desc()
> accept &desc so that the function can set its new value. Then export
> the function so that it could be reused by driver's specific
> implementations.
> 
> The above is a port of Linux kernel commit 54552e4948cbf
> 
> In addition, allow drivers to allocate their own ADMA descriptor
> tables if additional space is required.
> 
> Finally, fix the assignment of adma_addr to fix compiler warning
> on 64-bit platforms that still use 32-bit DMA addressing.
> 
> Co-developed-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
> Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
> Co-developed-by: Greg Malysa <greg.malysa@timesys.com>
> Signed-off-by: Greg Malysa <greg.malysa@timesys.com>
> Signed-off-by: Ian Roberts <ian.roberts@timesys.com>

I've bisected this patch (74755c1fed1b) to brake MMC flashing over DFU
on TI AM62x, which manifests as aborted transfer on dfu-util side:

---
DFU state(2) = dfuIDLE, status(0) = No error condition is present
DFU mode device DFU version 0110
Device returned transfer size 4096
Copying data from PC to DFU device
Download	[=========                ]  36%       118784 bytes failed!
DFU state(10) = dfuERROR, status(14) = Something went wrong, but the device does not know what it was
---

But not detected as any error condition on U-Boot side.

I'll try to debug this further, but one detail I can tell for now is that
my DFU transfer buffer size is 0x20000. Number of transferred bytes before
failure is always the same.

> ---
> 
> Changes in v2:
> - Switch from #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA) to #ifdef
>   CONFIG_MMC_SDHCI_ADMA_HELPERS, as CONFIG_IS_ENABLED() causes a build
>   failure during SPL builds when CONFIG_SPL_MMC is set.
> - Passed CI before submitting this time
> 
> ---
>  drivers/mmc/fsl_esdhc.c  |  2 +-
>  drivers/mmc/sdhci-adma.c | 41 +++++++++++++++++++++++++++-------------
>  drivers/mmc/sdhci.c      |  8 +++++---
>  include/sdhci.h          | 12 ++++++++++--
>  4 files changed, 44 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index d506666669..bd0671cc52 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -252,7 +252,7 @@ static void esdhc_setup_dma(struct fsl_esdhc_priv *priv, struct mmc_data *data)
>             priv->adma_desc_table) {
>                 debug("Using ADMA2\n");
>                 /* prefer ADMA2 if it is available */
> -               sdhci_prepare_adma_table(priv->adma_desc_table, data,
> +               sdhci_prepare_adma_table(NULL, priv->adma_desc_table, data,
>                                          priv->dma_addr);
>  
>                 adma_addr = virt_to_phys(priv->adma_desc_table);
> diff --git a/drivers/mmc/sdhci-adma.c b/drivers/mmc/sdhci-adma.c
> index 8213223d3f..8c38448b6a 100644
> --- a/drivers/mmc/sdhci-adma.c
> +++ b/drivers/mmc/sdhci-adma.c
> @@ -9,9 +9,10 @@
>  #include <malloc.h>
>  #include <asm/cache.h>
>  
> -static void sdhci_adma_desc(struct sdhci_adma_desc *desc,
> -                           dma_addr_t addr, u16 len, bool end)
> +void sdhci_adma_write_desc(struct sdhci_host *host, void **next_desc,
> +                          dma_addr_t addr, int len, bool end)
>  {
> +       struct sdhci_adma_desc *desc = *next_desc;
>         u8 attr;
>  
>         attr = ADMA_DESC_ATTR_VALID | ADMA_DESC_TRANSFER_DATA;
> @@ -19,17 +20,30 @@ static void sdhci_adma_desc(struct sdhci_adma_desc *desc,
>                 attr |= ADMA_DESC_ATTR_END;
>  
>         desc->attr = attr;
> -       desc->len = len;
> +       desc->len = len & 0xffff;
>         desc->reserved = 0;
>         desc->addr_lo = lower_32_bits(addr);
>  #ifdef CONFIG_DMA_ADDR_T_64BIT
>         desc->addr_hi = upper_32_bits(addr);
>  #endif
> +
> +       *next_desc += ADMA_DESC_LEN;
> +}
> +
> +static inline void __sdhci_adma_write_desc(struct sdhci_host *host,
> +                                          void **desc, dma_addr_t addr,
> +                                          int len, bool end)
> +{
> +       if (host && host->ops && host->ops->adma_write_desc)
> +               host->ops->adma_write_desc(host, desc, addr, len, end);
> +       else
> +               sdhci_adma_write_desc(host, desc, addr, len, end);
>  }
>  
>  /**
>   * sdhci_prepare_adma_table() - Populate the ADMA table
>   *
> + * @host:      Pointer to the sdhci_host
>   * @table:     Pointer to the ADMA table
>   * @data:      Pointer to MMC data
>   * @addr:      DMA address to write to or read from
> @@ -39,25 +53,26 @@ static void sdhci_adma_desc(struct sdhci_adma_desc *desc,
>   * Please note, that the table size depends on CONFIG_SYS_MMC_MAX_BLK_COUNT and
>   * we don't have to check for overflow.
>   */
> -void sdhci_prepare_adma_table(struct sdhci_adma_desc *table,
> -                             struct mmc_data *data, dma_addr_t addr)
> +void sdhci_prepare_adma_table(struct sdhci_host *host,
> +                             struct sdhci_adma_desc *table,
> +                             struct mmc_data *data, dma_addr_t start_addr)
>  {
> +       dma_addr_t addr = start_addr;
>         uint trans_bytes = data->blocksize * data->blocks;
> -       uint desc_count = DIV_ROUND_UP(trans_bytes, ADMA_MAX_LEN);
> -       struct sdhci_adma_desc *desc = table;
> -       int i = desc_count;
> +       void *next_desc = table;
> +       int i = DIV_ROUND_UP(trans_bytes, ADMA_MAX_LEN);
>  
>         while (--i) {
> -               sdhci_adma_desc(desc, addr, ADMA_MAX_LEN, false);
> +               __sdhci_adma_write_desc(host, &next_desc, addr,
> +                                       ADMA_MAX_LEN, false);
>                 addr += ADMA_MAX_LEN;
>                 trans_bytes -= ADMA_MAX_LEN;
> -               desc++;
>         }
>  
> -       sdhci_adma_desc(desc, addr, trans_bytes, true);
> +       __sdhci_adma_write_desc(host, &next_desc, addr, trans_bytes, true);
>  
> -       flush_cache((dma_addr_t)table,
> -                   ROUND(desc_count * sizeof(struct sdhci_adma_desc),
> +       flush_cache((phys_addr_t)table,
> +                   ROUND(next_desc - (void *)table,
>                           ARCH_DMA_MINALIGN));
>  }
>  
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 0178ed8a11..65090348ae 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -111,7 +111,7 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
>         }
>  #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
>         else if (host->flags & (USE_ADMA | USE_ADMA64)) {
> -               sdhci_prepare_adma_table(host->adma_desc_table, data,
> +               sdhci_prepare_adma_table(host, host->adma_desc_table, data,
>                                          host->start_addr);
>  
>                 sdhci_writel(host, lower_32_bits(host->adma_addr),
> @@ -897,8 +897,10 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,
>                        __func__);
>                 return -EINVAL;
>         }
> -       host->adma_desc_table = sdhci_adma_init();
> -       host->adma_addr = (dma_addr_t)host->adma_desc_table;
> +       if (!host->adma_desc_table) {
> +               host->adma_desc_table = sdhci_adma_init();
> +               host->adma_addr = virt_to_phys(host->adma_desc_table);
> +       }
>  
>  #ifdef CONFIG_DMA_ADDR_T_64BIT
>         host->flags |= USE_ADMA64;
> diff --git a/include/sdhci.h b/include/sdhci.h
> index a1b74e3bd7..d73a725609 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -291,6 +291,11 @@ struct sdhci_ops {
>          * Return: 0 if successful, -ve on error
>          */
>         int     (*set_enhanced_strobe)(struct sdhci_host *host);
> +
> +#ifdef CONFIG_MMC_SDHCI_ADMA_HELPERS
> +       void    (*adma_write_desc)(struct sdhci_host *host, void **desc,
> +                                  dma_addr_t addr, int len, bool end);
> +#endif
>  };
>  
>  #define ADMA_MAX_LEN   65532
> @@ -526,8 +531,11 @@ extern const struct dm_mmc_ops sdhci_ops;
>  #else
>  #endif
>  
> +void sdhci_adma_write_desc(struct sdhci_host *host, void **next_desc,
> +                          dma_addr_t addr, int len, bool end);
>  struct sdhci_adma_desc *sdhci_adma_init(void);
> -void sdhci_prepare_adma_table(struct sdhci_adma_desc *table,
> -                             struct mmc_data *data, dma_addr_t addr);
> +void sdhci_prepare_adma_table(struct sdhci_host *host,
> +                             struct sdhci_adma_desc *table,
> +                             struct mmc_data *data, dma_addr_t start_addr);
>  
>  #endif /* __SDHCI_HW_H */