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

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

[Greg Malysa]

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>
Signed-off-by: Greg Malysa <greg.malysa@timesys.com>
Signed-off-by: Ian Roberts <ian.roberts@timesys.com>

---


---
 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..4bde7db5c7 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);
+
+#if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
+	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 */
-- 
2.43.2

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

[Jaehoon Chung]

> -----Original Message-----
> From: Greg Malysa <greg.malysa@timesys.com>
> Sent: Tuesday, March 26, 2024 11:18 AM
> To: u-boot@lists.denx.de; Peng Fan <peng.fan@nxp.com>; Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Ian Roberts <ian.roberts@timesys.com>; Nathan Barrett-Morrison <nathan.morrison@timesys.com>; Greg
> Malysa <greg.malysa@timesys.com>; Jonas Karlman <jonas@kwiboo.se>; Kever Yang <kever.yang@rock-
> chips.com>; Peter Geis <pgwipeout@gmail.com>; Sean Anderson <sean.anderson@seco.com>; Simon Glass
> <sjg@chromium.org>; Tom Rini <trini@konsulko.com>
> Subject: [PATCH] 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>
> Signed-off-by: Greg Malysa <greg.malysa@timesys.com>
> Signed-off-by: Ian Roberts <ian.roberts@timesys.com>

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

Best Regards,
Jaehoon Chung

> 
> ---
> 
> 
> ---
>  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..4bde7db5c7 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);
> +
> +#if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
> +	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 */
> --
> 2.43.2

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

[Jaehoon Chung]

Hi,

> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Jaehoon Chung
> Sent: Wednesday, April 17, 2024 9:26 AM
> To: 'Greg Malysa' <greg.malysa@timesys.com>; u-boot@lists.denx.de; 'Peng Fan' <peng.fan@nxp.com>
> Cc: 'Ian Roberts' <ian.roberts@timesys.com>; 'Nathan Barrett-Morrison' <nathan.morrison@timesys.com>;
> 'Jonas Karlman' <jonas@kwiboo.se>; 'Kever Yang' <kever.yang@rock-chips.com>; 'Peter Geis'
> <pgwipeout@gmail.com>; 'Sean Anderson' <sean.anderson@seco.com>; 'Simon Glass' <sjg@chromium.org>;
> 'Tom Rini' <trini@konsulko.com>
> Subject: RE: [PATCH] mmc: sdhci: introduce adma_write_desc() hook to struct sdhci_ops
> 
> 
> 
> > -----Original Message-----
> > From: Greg Malysa <greg.malysa@timesys.com>
> > Sent: Tuesday, March 26, 2024 11:18 AM
> > To: u-boot@lists.denx.de; Peng Fan <peng.fan@nxp.com>; Jaehoon Chung <jh80.chung@samsung.com>
> > Cc: Ian Roberts <ian.roberts@timesys.com>; Nathan Barrett-Morrison <nathan.morrison@timesys.com>;
> Greg
> > Malysa <greg.malysa@timesys.com>; Jonas Karlman <jonas@kwiboo.se>; Kever Yang <kever.yang@rock-
> > chips.com>; Peter Geis <pgwipeout@gmail.com>; Sean Anderson <sean.anderson@seco.com>; Simon Glass
> > <sjg@chromium.org>; Tom Rini <trini@konsulko.com>
> > Subject: [PATCH] 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>
> > Signed-off-by: Greg Malysa <greg.malysa@timesys.com>
> > Signed-off-by: Ian Roberts <ian.roberts@timesys.com>
> 
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Some target are failed to build. (e.g, j721e_beagleboneai64_r5)

+drivers/mmc/sdhci-adma.c: In function '__sdhci_adma_write_desc':
+drivers/mmc/sdhci-adma.c:37:43: error: 'const struct sdhci_ops' has no member named 'adma_write_desc'
+   37 |         if (host && host->ops && host->ops->adma_write_desc)
+      |                                           ^~
+drivers/mmc/sdhci-adma.c:38:26: error: 'const struct sdhci_ops' has no member named 'adma_write_desc'
+   38 |                 host->ops->adma_write_desc(host, desc, addr, len, end);
+      |                          ^~
+make[3]: *** [scripts/Makefile.build:257: drivers/mmc/sdhci-adma.o] Error 1
+make[2]: *** [scripts/Makefile.build:397: drivers/mmc] Error 2

Best Regards,
Jaehoon Chung

> 
> Best Regards,
> Jaehoon Chung
> 
> >
> > ---
> >
> >
> > ---
> >  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..4bde7db5c7 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);
> > +
> > +#if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
> > +	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 */
> > --
> > 2.43.2
> 

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

[Greg Malysa]

Hi,

> Some target are failed to build. (e.g, j721e_beagleboneai64_r5)
>
> +drivers/mmc/sdhci-adma.c: In function '__sdhci_adma_write_desc':
> +drivers/mmc/sdhci-adma.c:37:43: error: 'const struct sdhci_ops' has no member named 'adma_write_desc'
> +   37 |         if (host && host->ops && host->ops->adma_write_desc)
> +      |                                           ^~
> +drivers/mmc/sdhci-adma.c:38:26: error: 'const struct sdhci_ops' has no member named 'adma_write_desc'
> +   38 |                 host->ops->adma_write_desc(host, desc, addr, len, end);
> +      |                          ^~
> +make[3]: *** [scripts/Makefile.build:257: drivers/mmc/sdhci-adma.o] Error 1
> +make[2]: *** [scripts/Makefile.build:397: drivers/mmc] Error 2

I will test v2 with CI before resubmitting so that this issue is
fixed. It is caused by the change and explanation below:

> > > diff --git a/include/sdhci.h b/include/sdhci.h
> > > index a1b74e3bd7..4bde7db5c7 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);
> > > +
> > > +#if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
> > > +   void    (*adma_write_desc)(struct sdhci_host *host, void **desc,
> > > +                              dma_addr_t addr, int len, bool end);
> > > +#endif
> > >  };

We got a little too excited about following checkpatch's
recommendations (no #ifdef CONFIG_xyz, prefer #if
CONFIG_IS_ENABLED(xyz)), which breaks down in this case:

CONFIG_IS_ENABLED(xyz) checks if:
- regular build and CONFIG_xyz is enabled (this portion succeeds)
- SPL build and CONFIG_SPL_xyz is enabled (this portion fails)
drivers/mmc/Makefile builds sdhci-adma.o based on
CONFIG_SDHCI_ADMA_HELPERS only.

There is no CONFIG_SPL_SDHCI_ADMA_HELPERS so CONFIG_IS_ENABLED fails
while building the SPL version of sdhci-adma.o as the structure
definition is different. This only appears on platforms which have
CONFIG_SPL_MMC enabled, which our platform did not, so I missed this
interaction earlier. I apologize for this mistake.

This will be fixed in v2 by changing the #if back to #ifdef
CONFIG_MMC_SDHCI_ADMA_HELPERS, which I will submit after CI finishes
running to verify on all platforms.

Thanks,
Greg

-- 
Greg Malysa
Timesys Corporation