Re: [PATCH v1 2/2] mmc: sdhci-of-dwcmshc: Set CQHCI command idle time for StarFive JHB100

[Adrian Hunter]

On 04/11/2026 03:40, Shawn Lin wrote:
> 在 2026/10/31 星期六 0:45, Adrian Hunter 写道:
>> + Shawn
>>
>> On 14/10/2026 15:12, Changhuang Liang wrote:
>>> Add a cqhci_host_ops for the StarFive JHB100 platform that sets the
>>> CQHCI command idle time (CIT) to 320 ns during pre_enable.
>>>
>>> Setting the command idle time improves eMMC performance on this
>>> platform. Before/after dd results:
>>>
>>> before:
>>> dd if=/dev/mmcblk0 of=/dev/null bs=1M count=800
>>> 838860800 bytes (839 MB, 800 MiB) copied, 9.38511 s, 89.4 MB/s
>>> dd if=/dev/zero of=/dev/mmcblk0 bs=1M count=800
>>> 838860800 bytes (839 MB, 800 MiB) copied, 12.2127 s, 68.7 MB/s
>>>
>>> after:
>>> dd if=/dev/mmcblk0 of=/dev/null bs=1M count=800
>>> 838860800 bytes (839 MB, 800 MiB) copied, 3.19472 s, 263 MB/s
>>> dd if=/dev/zero of=/dev/mmcblk0 bs=1M count=800
>>> 838860800 bytes (839 MB, 800 MiB) copied, 10.7587 s, 78.0 MB/s
>>>
>>> Signed-off-by: Changhuang Liang <changhuang.liang@xxxxxxxxxxxxxxxx>
>>> ---
>>>   drivers/mmc/host/sdhci-of-dwcmshc.c | 22 ++++++++++++++++++++++
>>>   1 file changed, 22 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
>>> index 249055f14162..36628b44f78b 100644
>>> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
>>> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
>>> @@ -283,6 +283,8 @@
>>>   #define PHY_DELAY_CODE_SD        0x55
>>>   #define PHY_DELAY_CODE_SDIO        0x29
>>>   +/* CIT for the JHB100 SoC */
>>> +#define JHB100_CQHCI_CIT_NS    320
>>>   struct rk35xx_priv {
>>>       struct reset_control *reset;
>>>       u8 txclk_tapnum;
>>> @@ -2340,6 +2342,25 @@ static const struct sdhci_ops starfive_jhb100_ops = {
>>>       .platform_execute_tuning = th1520_execute_tuning,
>>>   };
>>>   +static void dwcmshc_starfive_jhb100_pre_enable(struct mmc_host *mmc)
>>> +{
>>> +    struct cqhci_host *cq_host = mmc->cqe_private;
>>> +    int ret;
>>> +
>>> +    ret = cqhci_set_cmd_idle_time(cq_host, JHB100_CQHCI_CIT_NS);
>>
>> Currently doing this from ->pre_enable() leaves a gap.  It needs:
>>
>> diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
>> index a466cd88b043..e1bf6d50e8eb 100644
>> --- a/drivers/mmc/host/cqhci-core.c
>> +++ b/drivers/mmc/host/cqhci-core.c
>> @@ -283,6 +283,9 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
>>         cqhci_set_irqs(cq_host, 0);
>>   +    if (cq_host->ops->pre_enable)
>> +        cq_host->ops->pre_enable(mmc);
>> +
>>       cqcfg |= CQHCI_ENABLE;
>>         cqhci_writel(cq_host, cqcfg, CQHCI_CFG);
>>
>> which is probably:
>>
>> Fixes: 79cf71c0b177 ("mmc: sdhci-of-dwcmshc: reduce CIT for better performance")
>>
>> Shawn?
>>
> 
> The window between CQHCI_ENABLE and the first request has no queued
> commands for the idle timer to apply to, and pre_enable() re-runs on
> every cqe_off -> cqe_on transition (recovery, resume), so the value gets
> rewritten even if the registers were reset in between.

Might be worth checking.

At card init, cqhci_enable() calls __cqhci_enable(), which sets mmc->cqe_on = 
true (cqhci-core.c) and never calls ->pre_enable(). cqhci_request() only calls 
->pre_enable() when !mmc->cqe_on (cqhci-core.c). So the first request skips it, 
and so do all the requests after it. CIT is first written only after something 
runs a non-CQE command, because mmc_start_request() then calls cqe_off() 
(core.c). Examples: a CMD6 partition switch, a discard, an ioctl, or a flush 
without DCMD. Until that happens, real CQE I/O runs with the default CIT.

For Resume: cqhci_suspend() → cqhci_deactivate() → __cqhci_disable() clears both 
cqe_on and activated. The first request after resume takes the !activated path 
into __cqhci_enable(), which sets cqe_on = true again. The !cqe_on branch is 
then skipped and ->pre_enable() is not called.

RESET_ALL: same problem. rk35xx_sdhci_reset() on SDHCI_RESET_ALL calls 
cqhci_deactivate() and also asserts the reset_control, so SSC1 is certainly back 
to its default. CIT then stays at the default after resume until the next 
cqe_off.