Re: [PATCH 2/2] mmc: sdhci-esdhc-imx: calclute data timeout value based on clock

[Adrian Hunter <adrian.hunter@intel.com>]

On 24/03/25 11:23, ziniu.wang_1@nxp.com wrote:
> From: Luke Wang <ziniu.wang_1@nxp.com>
> 
> Calclute data timeout value based on clock instead of using max value.

And the subject:

	Calclute -> Calculate

Presumably the driver has been working OK up until now with max value.
Is there any particular reason to change it?

> 
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index ff78a7c6a04c..e7316ecff64e 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -31,6 +31,7 @@
>  #include "cqhci.h"
>  
>  #define ESDHC_SYS_CTRL_DTOCV_MASK	GENMASK(19, 16)
> +#define ESDHC_SYS_CTRL_DTOCV_SHIFT	16
>  #define ESDHC_SYS_CTRL_IPP_RST_N	BIT(23)
>  #define	ESDHC_CTRL_D3CD			0x08
>  #define ESDHC_BURST_LEN_EN_INCR		(1 << 27)
> @@ -1387,12 +1388,16 @@ static unsigned int esdhc_get_max_timeout_count(struct sdhci_host *host)
>  
>  static void esdhc_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>  {
> -	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> -	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
> +	bool too_big = false;
> +	u8 count = sdhci_calc_timeout(host, cmd, &too_big);
>  
> -	/* use maximum timeout counter */
> +	/*
> +	 * ESDHC_SYSTEM_CONTROL bit[23] used to control hardware reset
> +	 * pin of the card. Write 0 to bit[23] will reset the card.
> +	 * Only write DTOCV filed here.

filed -> field ?

> +	 */
>  	esdhc_clrset_le(host, ESDHC_SYS_CTRL_DTOCV_MASK,
> -			esdhc_is_usdhc(imx_data) ? 0xF0000 : 0xE0000,
> +			count << ESDHC_SYS_CTRL_DTOCV_SHIFT,

Could use FIELD_PREP() here

>  			ESDHC_SYSTEM_CONTROL);

Another way to do this could be to add SDHCI_TIMEOUT_CONTROL to
esdhc_writeb_le() and remove esdhc_set_timeout().  That would
avoid having to export sdhci_calc_timeout() and is perhaps
slightly more consistent with other code in this driver.
Probably look something like below:

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index ff78a7c6a04c..66477fc0ba82 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -870,6 +870,16 @@ static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
 
 		esdhc_clrset_le(host, mask, new_val, reg);
 		return;
+	case SDHCI_TIMEOUT_CONTROL:
+		/*
+		 * ESDHC_SYSTEM_CONTROL bit[23] used to control hardware reset
+		 * pin of the card. Write 0 to bit[23] will reset the card.
+		 * Only write DTOCV field here.
+		 */
+		esdhc_clrset_le(host, ESDHC_SYS_CTRL_DTOCV_MASK,
+				FIELD_PREP(ESDHC_SYS_CTRL_DTOCV_MASK, val),
+				ESDHC_SYSTEM_CONTROL);
+		return;
 	case SDHCI_SOFTWARE_RESET:
 		if (val & SDHCI_RESET_DATA)
 			new_val = readl(host->ioaddr + SDHCI_HOST_CONTROL);
@@ -1385,17 +1395,6 @@ static unsigned int esdhc_get_max_timeout_count(struct sdhci_host *host)
 	return esdhc_is_usdhc(imx_data) ? 1 << 29 : 1 << 27;
 }
 
-static void esdhc_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
-{
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
-
-	/* use maximum timeout counter */
-	esdhc_clrset_le(host, ESDHC_SYS_CTRL_DTOCV_MASK,
-			esdhc_is_usdhc(imx_data) ? 0xF0000 : 0xE0000,
-			ESDHC_SYSTEM_CONTROL);
-}
-
 static u32 esdhc_cqhci_irq(struct sdhci_host *host, u32 intmask)
 {
 	int cmd_error = 0;
@@ -1432,7 +1431,6 @@ static struct sdhci_ops sdhci_esdhc_ops = {
 	.get_min_clock = esdhc_pltfm_get_min_clock,
 	.get_max_timeout_count = esdhc_get_max_timeout_count,
 	.get_ro = esdhc_pltfm_get_ro,
-	.set_timeout = esdhc_set_timeout,
 	.set_bus_width = esdhc_pltfm_set_bus_width,
 	.set_uhs_signaling = esdhc_set_uhs_signaling,
 	.reset = esdhc_reset,


>  }
>  
> @@ -1777,6 +1782,8 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  		 * to distinguish the card type.
>  		 */
>  		host->mmc_host_ops.init_card = usdhc_init_card;
> +
> +		host->max_timeout_count = 0xF;
>  	}
>  
>  	if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING)