* [PATCH v1 0/2] Improve eMMC performance for the StarFive JHB100 SoC @ 2026-10-14 12:12 Changhuang Liang 2026-10-14 12:12 ` [PATCH v1 1/2] mmc: cqhci: Add support for setting command idle time in nanoseconds Changhuang Liang 2026-10-14 12:12 ` [PATCH v1 2/2] mmc: sdhci-of-dwcmshc: Set CQHCI command idle time for StarFive JHB100 Changhuang Liang 0 siblings, 2 replies; 8+ messages in thread From: Changhuang Liang @ 2026-10-14 12:12 UTC (permalink / raw) To: Asutosh Das, Ritesh Harjani, Ulf Hansson, Adrian Hunter Cc: linux-mmc, linux-kernel, Changhuang Liang This series implements a generic interface to configure CIT based on time (ns). Different platforms can improve MMC performance by configuring their respective CIT. This series has been tested on the JHB100 SoC. 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 This series depends on series: https://lore.kernel.org/all/20260930103258.13375-1-changhuang.liang@starfivetech.com/ Changhuang Liang (2): mmc: cqhci: Add support for setting command idle time in nanoseconds mmc: sdhci-of-dwcmshc: Set CQHCI command idle time for StarFive JHB100 drivers/mmc/host/cqhci-core.c | 60 +++++++++++++++++++++++++++++ drivers/mmc/host/cqhci.h | 6 +++ drivers/mmc/host/sdhci-of-dwcmshc.c | 22 +++++++++++ 3 files changed, 88 insertions(+) -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v1 1/2] mmc: cqhci: Add support for setting command idle time in nanoseconds 2026-10-14 12:12 [PATCH v1 0/2] Improve eMMC performance for the StarFive JHB100 SoC Changhuang Liang @ 2026-10-14 12:12 ` Changhuang Liang 2026-10-14 12:12 ` [PATCH v1 2/2] mmc: sdhci-of-dwcmshc: Set CQHCI command idle time for StarFive JHB100 Changhuang Liang 1 sibling, 0 replies; 8+ messages in thread From: Changhuang Liang @ 2026-10-14 12:12 UTC (permalink / raw) To: Asutosh Das, Ritesh Harjani, Ulf Hansson, Adrian Hunter Cc: linux-mmc, linux-kernel, Changhuang Liang Add a helper that derives the internal timer clock frequency (ITCF) of the CQE from the ITCFMUL and ITCFVAL fields of the CQHCI capabilities register, and export cqhci_set_cmd_idle_time() to let host drivers program the Send Status Command Idle Timer (CIT) in CQHCI_SSC1 using a value in nanoseconds. The CIT defines how long the bus has to be idle, in internal timer clock cycles, before the CQE issues CMD13 to poll the Queue Status Register. Its reset value is controller specific and can be far from optimal, so host drivers may need to tune it for their platform. Doing the conversion in the core avoids every host driver re-implementing the ITCFMUL/ITCFVAL decoding. The requested time is rounded up to whole clock cycles so that the programmed idle time is never shorter than requested. The helper returns -EOPNOTSUPP when the controller does not report a usable ITCF and -ERANGE when the result does not fit the 16-bit CIT field, so callers can report the failure instead of silently running with the reset default. Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com> --- drivers/mmc/host/cqhci-core.c | 60 +++++++++++++++++++++++++++++++++++ drivers/mmc/host/cqhci.h | 6 ++++ 2 files changed, 66 insertions(+) diff --git a/drivers/mmc/host/cqhci-core.c b/drivers/mmc/host/cqhci-core.c index a466cd88b043..aff8d6619ea9 100644 --- a/drivers/mmc/host/cqhci-core.c +++ b/drivers/mmc/host/cqhci-core.c @@ -12,6 +12,7 @@ #include <linux/scatterlist.h> #include <linux/platform_device.h> #include <linux/ktime.h> +#include <linux/units.h> #include <linux/mmc/mmc.h> #include <linux/mmc/host.h> @@ -1163,6 +1164,63 @@ struct cqhci_host *cqhci_pltfm_init(struct platform_device *pdev) } EXPORT_SYMBOL(cqhci_pltfm_init); +static u64 cqhci_itcfmul_to_hz(u32 itcfmul) +{ + /* CQHCI_CAP.ITCFMUL: 0 = 1 kHz, 1 = 10 kHz, ..., 4 = 10 MHz, others reserved */ + if (itcfmul > 4) + return 0; + + return HZ_PER_KHZ * int_pow(10, itcfmul); +} + +static void cqhci_itcf_init(struct cqhci_host *cq_host) +{ + u32 cap = cqhci_readl(cq_host, CQHCI_CAP); + u64 itcf; + + itcf = cqhci_itcfmul_to_hz(CQHCI_ITCFMUL(cap)) * CQHCI_ITCFVAL(cap); + if (!itcf || itcf > U32_MAX) { + pr_debug("%s: cqhci: internal timer clock frequency not available (CAP 0x%08x)\n", + mmc_hostname(cq_host->mmc), cap); + return; + } + + cq_host->itcf_hz = itcf; +} + +/** + * cqhci_set_cmd_idle_time - program the Send Status Command Idle Timer (CIT) + * @cq_host: CQHCI host + * @ns: idle time in nanoseconds + * + * Set the number of internal timer clock cycles the CQE waits on an idle bus + * before issuing CMD13 to poll the Queue Status Register. The cycle count is + * rounded up so that the resulting idle time is never shorter than @ns. + * + * Return: 0 on success, -EOPNOTSUPP if the controller does not report its + * internal timer clock frequency, or -ERANGE if @ns does not fit the CIT field. + */ +int cqhci_set_cmd_idle_time(struct cqhci_host *cq_host, u32 ns) +{ + u32 ssc1; + u64 cit; + + if (!cq_host->itcf_hz) + return -EOPNOTSUPP; + + cit = DIV_ROUND_UP_ULL((u64)cq_host->itcf_hz * ns, NSEC_PER_SEC); + if (!cit || cit > FIELD_MAX(CQHCI_SSC1_CIT_MASK)) + return -ERANGE; + + ssc1 = cqhci_readl(cq_host, CQHCI_SSC1); + ssc1 &= ~CQHCI_SSC1_CIT_MASK; + ssc1 |= FIELD_PREP(CQHCI_SSC1_CIT_MASK, cit); + cqhci_writel(cq_host, ssc1, CQHCI_SSC1); + + return 0; +} +EXPORT_SYMBOL(cqhci_set_cmd_idle_time); + static unsigned int cqhci_ver_major(struct cqhci_host *cq_host) { return CQHCI_VER_MAJOR(cqhci_readl(cq_host, CQHCI_VER)); @@ -1207,6 +1265,8 @@ int cqhci_init(struct cqhci_host *cq_host, struct mmc_host *mmc, goto out_err; } + cqhci_itcf_init(cq_host); + spin_lock_init(&cq_host->lock); init_completion(&cq_host->halt_comp); diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h index 8fbbc48c3f85..07eb753f9586 100644 --- a/drivers/mmc/host/cqhci.h +++ b/drivers/mmc/host/cqhci.h @@ -26,6 +26,8 @@ #define CQHCI_CAP_CS 0x10000000 /* Crypto Support */ #define CQHCI_CAP_ITCFMUL GENMASK(15, 12) #define CQHCI_ITCFMUL(x) FIELD_GET(CQHCI_CAP_ITCFMUL, (x)) +#define CQHCI_CAP_ITCFVAL GENMASK(9, 0) +#define CQHCI_ITCFVAL(x) FIELD_GET(CQHCI_CAP_ITCFVAL, (x)) /* configuration */ #define CQHCI_CFG 0x08 @@ -234,6 +236,9 @@ struct cqhci_host { int num_slots; int qcnt; + /* Internal Timer Clock Frequency (Hz), 0 if not reported by CQHCI_CAP */ + u32 itcf_hz; + u32 dcmd_slot; u32 caps; #define CQHCI_TASK_DESC_SZ_128 0x1 @@ -318,6 +323,7 @@ struct platform_device; irqreturn_t cqhci_irq(struct mmc_host *mmc, int cmd_error, int data_error); int cqhci_init(struct cqhci_host *cq_host, struct mmc_host *mmc, bool dma64); struct cqhci_host *cqhci_pltfm_init(struct platform_device *pdev); +int cqhci_set_cmd_idle_time(struct cqhci_host *cq_host, u32 ns); int cqhci_deactivate(struct mmc_host *mmc); void cqhci_set_tran_desc(u8 *desc, dma_addr_t addr, int len, bool end, bool dma64); static inline int cqhci_suspend(struct mmc_host *mmc) -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v1 2/2] mmc: sdhci-of-dwcmshc: Set CQHCI command idle time for StarFive JHB100 2026-10-14 12:12 [PATCH v1 0/2] Improve eMMC performance for the StarFive JHB100 SoC Changhuang Liang 2026-10-14 12:12 ` [PATCH v1 1/2] mmc: cqhci: Add support for setting command idle time in nanoseconds Changhuang Liang @ 2026-10-14 12:12 ` Changhuang Liang 2026-10-30 16:45 ` Adrian Hunter 1 sibling, 1 reply; 8+ messages in thread From: Changhuang Liang @ 2026-10-14 12:12 UTC (permalink / raw) To: Asutosh Das, Ritesh Harjani, Ulf Hansson, Adrian Hunter Cc: linux-mmc, linux-kernel, Changhuang Liang 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); + 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, }; -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/2] mmc: sdhci-of-dwcmshc: Set CQHCI command idle time for StarFive JHB100 2026-10-14 12:12 ` [PATCH v1 2/2] mmc: sdhci-of-dwcmshc: Set CQHCI command idle time for StarFive JHB100 Changhuang Liang @ 2026-10-30 16:45 ` Adrian Hunter 2026-11-04 1:07 ` Changhuang Liang 2026-11-04 1:40 ` Shawn Lin 0 siblings, 2 replies; 8+ messages in thread From: Adrian Hunter @ 2026-10-30 16:45 UTC (permalink / raw) To: Changhuang Liang, Asutosh Das, Ritesh Harjani, Ulf Hansson, Shawn Lin Cc: linux-mmc, linux-kernel + 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, > }; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/2] mmc: sdhci-of-dwcmshc: Set CQHCI command idle time for StarFive JHB100 2026-10-30 16:45 ` Adrian Hunter @ 2026-11-04 1:07 ` Changhuang Liang 2026-11-04 1:40 ` Shawn Lin 1 sibling, 0 replies; 8+ messages in thread From: Changhuang Liang @ 2026-11-04 1:07 UTC (permalink / raw) To: Adrian Hunter, Asutosh Das, Ritesh Harjani, Ulf Hansson, Shawn Lin Cc: linux-mmc, linux-kernel Hi, Adrian Thanks for the review. > + 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); Okay, I'll try this modification plan. Best Regards, Changhuang ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/2] mmc: sdhci-of-dwcmshc: Set CQHCI command idle time for StarFive JHB100 2026-10-30 16:45 ` Adrian Hunter 2026-11-04 1:07 ` Changhuang Liang @ 2026-11-04 1:40 ` Shawn Lin 2026-11-04 7:29 ` Adrian Hunter 1 sibling, 1 reply; 8+ messages in thread From: Shawn Lin @ 2026-11-04 1:40 UTC (permalink / raw) To: Adrian Hunter, Changhuang Liang, Asutosh Das, Ritesh Harjani, Ulf Hansson Cc: shawn.lin, linux-mmc, linux-kernel 在 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@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? > 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. You are right that __cqhci_enable() does not touch CQHCI_SSC1. It only programs CQHCI_CFG, CQHCI_TDLBA/TDLBAU, CQHCI_SSC2 and CQHCI_CTL, so today the CIT write lands just before the first request and the default value in the idle window is harmless. I agree the value belongs in struct cqhci_host and should be programmed from __cqhci_enable() when it is non-zero, as you suggested. > > 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, >> }; > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/2] mmc: sdhci-of-dwcmshc: Set CQHCI command idle time for StarFive JHB100 2026-11-04 1:40 ` Shawn Lin @ 2026-11-04 7:29 ` Adrian Hunter 2026-11-04 8:08 ` Shawn Lin 0 siblings, 1 reply; 8+ messages in thread From: Adrian Hunter @ 2026-11-04 7:29 UTC (permalink / raw) To: Shawn Lin, Changhuang Liang, Asutosh Das, Ritesh Harjani, Ulf Hansson Cc: linux-mmc, linux-kernel 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@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? >> > > 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 2/2] mmc: sdhci-of-dwcmshc: Set CQHCI command idle time for StarFive JHB100 2026-11-04 7:29 ` Adrian Hunter @ 2026-11-04 8:08 ` Shawn Lin 0 siblings, 0 replies; 8+ messages in thread From: Shawn Lin @ 2026-11-04 8:08 UTC (permalink / raw) To: Adrian Hunter, Changhuang Liang, Asutosh Das, Ritesh Harjani, Ulf Hansson Cc: shawn.lin, linux-mmc, linux-kernel 在 2026/11/04 星期三 15:29, 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@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? >>> >> >> 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. > Indeed. If Changhuang is going to configure cit in __cqhci_enable, I could prepare a patch on top to improve it for rockchip platforms. > 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. > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-11-04 8:08 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-14 12:12 [PATCH v1 0/2] Improve eMMC performance for the StarFive JHB100 SoC Changhuang Liang 2026-10-14 12:12 ` [PATCH v1 1/2] mmc: cqhci: Add support for setting command idle time in nanoseconds Changhuang Liang 2026-10-14 12:12 ` [PATCH v1 2/2] mmc: sdhci-of-dwcmshc: Set CQHCI command idle time for StarFive JHB100 Changhuang Liang 2026-10-30 16:45 ` Adrian Hunter 2026-11-04 1:07 ` Changhuang Liang 2026-11-04 1:40 ` Shawn Lin 2026-11-04 7:29 ` Adrian Hunter 2026-11-04 8:08 ` Shawn Lin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®