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

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

+ 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@starfivetech.com>
> ---
>  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?


Going forward, it would probably be simpler to put 'cit' in
struct cqhci_host and write it, if it is not 0, in __cqhci_enable()
e.g.

diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c
index a466cd88b043..d5f8b427e1f8 100644
--- a/drivers/mmc/host/cqhci-core.c
+++ b/drivers/mmc/host/cqhci-core.c
@@ -279,6 +279,14 @@ static void __cqhci_enable(struct cqhci_host *cq_host)
 	cqhci_writel(cq_host, upper_32_bits(cq_host->desc_dma_base),
 		     CQHCI_TDLBAU);
 
+	if (cq_host->cit) {
+		u32 ssc1 = cqhci_readl(cq_host, CQHCI_SSC1);
+
+		ssc1 &= ~CQHCI_SSC1_CIT_MASK;
+		ssc1 |= FIELD_PREP(CQHCI_SSC1_CIT_MASK, cq_host->cit);
+		cqhci_writel(cq_host, ssc1, CQHCI_SSC1);
+	}
+
 	cqhci_writel(cq_host, cq_host->rca, CQHCI_SSC2);
 
 	cqhci_set_irqs(cq_host, 0);

> +	if (ret)
> +		dev_warn_once(mmc_dev(mmc),
> +			      "failed to set CQE command idle time: %d\n", ret);
> +}
> +
> +static const struct cqhci_host_ops starfive_jhb100_cqhci_ops = {
> +	.enable			= dwcmshc_sdhci_cqe_enable,
> +	.disable		= sdhci_cqe_disable,
> +	.pre_enable		= dwcmshc_starfive_jhb100_pre_enable,
> +	.dumpregs		= dwcmshc_cqhci_dumpregs,
> +	.set_tran_desc		= dwcmshc_set_tran_desc,
> +};
> +
>  static int dwcmshc_starfive_jhb100_init(struct device *dev, struct sdhci_host *host,
>  					struct dwcmshc_priv *dwc_priv)
>  {
> @@ -2412,6 +2433,7 @@ static const struct dwcmshc_pltfm_data sdhci_dwcmshc_jhb100_pdata = {
>  		.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
>  		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
>  	},
> +	.cqhci_host_ops = &starfive_jhb100_cqhci_ops,
>  	.init = dwcmshc_starfive_jhb100_init,
>  	.deinit = dwcmshc_starfive_jhb100_deinit,
>  };