Re: [PATCH v2 3/4] mmc: sdhci_am654: Clear ITAPDLY on tuning failure

Hi Diogo,

Thanks for the fix.

On 8/3/2026 8:47 AM, Diogo Ivo (Schneider Electric) wrote:
> When tuning fails, stale ITAPDLY values can persist and interfere with
> subsequent I/O accesses, for example in DDR50 mode in cards with no tuning
> support. Move the ITAPDLY enable setting out of the tuning loop to after
> successful tuning, and explicitly clear ITAPDLY (delay and enable) when
> tuning fails so that we are sure only working values are actually left in
> hardware.
> 
> Fixes: 901d16e46296 ("mmc: sdhci_am654: Add retry tuning")
> Cc: <[email protected]>
> Signed-off-by: Diogo Ivo (Schneider Electric) <[email protected]>

Reviewed-by: Judith Mendez <[email protected]>

> ---
> Changes in v2:
>   - As suggested by Sashiko clear the values in itap_del_sel[] and itap_del_ena[]
>     so that they agree with what is written in HW and subsequent runtime_suspend()
>     and runtime_resume() can never write stale values.
>   - Remove now unused timing variable
> ---
>   drivers/mmc/host/sdhci_am654.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index 0db587e702ac..8ce10422bba0 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -527,7 +527,6 @@ static int sdhci_am654_do_tuning(struct sdhci_host *host,
>   {
>   	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>   	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> -	unsigned char timing = host->mmc->ios.timing;
>   	struct window fail_window[ITAPDLY_LENGTH];
>   	struct device *dev = mmc_dev(host->mmc);
>   	u8 curr_pass, itap;
> @@ -536,11 +535,8 @@ static int sdhci_am654_do_tuning(struct sdhci_host *host,
>   
>   	memset(fail_window, 0, sizeof(fail_window));
>   
> -	/* Enable ITAPDLY */
> -	sdhci_am654->itap_del_ena[timing] = 0x1;
> -
>   	for (itap = 0; itap < ITAPDLY_LENGTH; itap++) {
> -		sdhci_am654_write_itapdly(sdhci_am654, itap, sdhci_am654->itap_del_ena[timing]);
> +		sdhci_am654_write_itapdly(sdhci_am654, itap, 0x1);
>   
>   		curr_pass = !mmc_send_tuning(host->mmc, opcode, NULL);
>   
> @@ -584,10 +580,16 @@ static int sdhci_am654_platform_execute_tuning(struct sdhci_host *host,
>   
>   	if (itapdly < 0) {
>   		dev_err(dev, "Failed to find itapdly, fail tuning\n");
> +		sdhci_am654_write_itapdly(sdhci_am654, 0, 0);
> +		sdhci_am654->itap_del_ena[timing] = 0;
> +		sdhci_am654->itap_del_sel[timing] = 0;
>   		return -1;
>   	}
>   
>   	dev_dbg(dev, "Passed tuning, final itapdly=%d\n", itapdly);
> +
> +	/* Enable ITAPDLY */
> +	sdhci_am654->itap_del_ena[timing] = 0x1;
>   	sdhci_am654_write_itapdly(sdhci_am654, itapdly, sdhci_am654->itap_del_ena[timing]);
>   	/* Save ITAPDLY */
>   	sdhci_am654->itap_del_sel[timing] = itapdly;
>