Re: [PATCH 1/4] mmc: sdhci-cadence6: add PHY settle delay after tuning DLL re-lock

["Kathpalia, Tanmay" <tanmay.kathpalia@altera.com>]

Hi Tze,

On 9/22/2026 4:42 PM, tze.yee.ng@altera.com wrote:
> From: Tze Yee Ng <tze.yee.ng@altera.com>
>
> After re-locking the DLL, allow the same 5 to 5.5 ms for the clock and
> PHY signals to stabilize as sdhci_cdns6_phy_init() does. Without this

I do not think the comparison with sdhci_cdns6_phy_init() holds. That
5 ms comes after a full PHY and host reprogram, including HRS writes
that happen after PHY_INIT_COMPLETE, and that path is only used after
SDCLK or the speed mode changes. set_tune_val() only updates two
phy_dll_slave_ctrl_reg fields and re-locks the DLL; it does not touch
HRS, clock, or mode.

> settle time the command issued immediately after tuning (e.g. the R1b
> CMD6 that switches eMMC from HS200 down to HS during HS400 selection)
> can time out.

Do you have a log for this? Which mode, which card, and how often it
reproduces. I ran a long regression on eMMC and on SD cards from
several vendors and sizes, and never hit a post-tuning CMD6 timeout
without this delay. I would prefer to see the failure before we add an
unconditional 5 ms.

> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
>   drivers/mmc/host/sdhci-cadence-phy-v6.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> index 22d56bb46d75..35f35ef9c710 100644
> --- a/drivers/mmc/host/sdhci-cadence-phy-v6.c
> +++ b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> @@ -811,6 +811,9 @@ int sdhci_cdns6_set_tune_val(struct sdhci_host *host, unsigned int val)
>   	if (ret)
>   		dev_warn(mmc_dev(host->mmc), "%s: DLL reset release failed: %d\n", __func__, ret);
>   
> +	/* Allow 5 to 5.5 ms for clock and PHY signals to stabilize after configuration */

The comment is copied from phy_init() and says "after configuration",
but here we only reprogrammed the slave delay taps and re-locked the
DLL. Please reword it for this call site.

> +	usleep_range(5000, 5500);
> +

I think the placement also contradicts the rationale. After
sdhci_cdns6_dll_reset(priv, false), PHY_INIT_COMPLETE is already polled,
and per the Cadence DLL PHY user guide section 1.2 that means the master
DLLs have locked and the PHY is ready to accept commands. If a command
still cannot be issued for 5 ms after that, then all 40 scan commands
were sent on an unsettled PHY before this patch, and the real bug is a
wrongly chosen tap rather than a slow CMD6. If the scan was reliable, the
delay is only needed once, after the final tap is programmed.

Also, the sleep is placed after the "DLL reset release failed" warning,
so we also wait 5 ms when the DLL did not re-lock and we are about to
return an error. Skip it when ret is non-zero.

>   	return ret;
>   }
>   
Thanks Adrian - I'd appreciate your view on the comments I posted, given
your experience with this subsystem. I'd like to hold the patch until the
mechanism and the cost are clarified, and happy to go with whatever you
think is right once those points are answered.