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

["NG, TZE YEE" <tze.yee.ng@altera.com>]

On 24/9/2026 4:49 pm, Kathpalia, Tanmay wrote:
> 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.
> 

Hi Tanmay,

Agreed. The two paths don't do the same work, so justifying the delay
by analogy to phy_init() was wrong. I'll drop that claim from the commit
message.

>> 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.
> 

Yes, Agilex5 eMMC in HS400 selection path. The failing command is the 
HS200->HS CM6 (R1b) the core issues in mmc_select_hs400():

   mmc0: switch to high-speed from hs200 failed, err:-110
   mmc0: error -110 whilst initialising MMC card
   mmc0: Failed to initialize a non-removable card

It reproduces on every init on this board without the settle delay. SD 
SDR104 on the same board is unaffected.


>> 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.
> 

I will fix the comment in v2 to remove the phy_init() or "configuration 
references.

>> +    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.
> 

Agreed. The scan issues only data read commands, which sample reliably 
once PHY_INIT_COMPLETE is set.  The command that fails is the first one 
the caller issues after tuning, which in the HS400 path is the R1b busy 
CMD6 rather than a data read. So the settle should be done once, after 
the final tap.

In v2, the settle delay will sits after the set_tune_val() error return, 
so it only runs when tuning succeeds; a tuning failure returns before it.

This also answers Adrian's cost observation: it's a single 5 ms after
the winning tap now, not ~40x during the scan.

Thanks,
Tze Yee

>>       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.