> -----Original Message-----
> From: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> Sent: 2022年11月25日 1:07
> To: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> Cc: Sarthak Garg <quic_sartgarg@xxxxxxxxxxx>; Florian Fainelli
> <f.fainelli@xxxxxxxxx>; Al Cooper <alcooperx@xxxxxxxxx>; Bough Chen
> <haibo.chen@xxxxxxx>; Andrew Jeffery <andrew@xxxxxxxx>; Eugen Hristev
> <eugen.hristev@xxxxxxxxxxxxx>; Vignesh Raghavendra <vigneshr@xxxxxx>;
> Prabu Thangamuthu <prabu.t@xxxxxxxxxxxx>; Manjunath M B
> <manjumb@xxxxxxxxxxxx>; Ben Dooks <ben-linux@xxxxxxxxx>; Jaehoon Chung
> <jh80.chung@xxxxxxxxxxx>; Viresh Kumar <vireshk@xxxxxxxxxx>; Thierry
> Reding <thierry.reding@xxxxxxxxx>; Hu Ziji <huziji@xxxxxxxxxxx>; Wolfram
> Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>; Sascha Hauer
> <s.hauer@xxxxxxxxxxxxxx>; Brian Norris <briannorris@xxxxxxxxxxxx>;
> Wenchao Chen <wenchao.chen@xxxxxxxxxx>; Chevron Li
> <chevron.li@xxxxxxxxxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx
> Subject: [PATCH 1/4] mmc: sdhci: Fix voltage switch delay
>
> Commit 20b92a30b561 ("mmc: sdhci: update signal voltage switch code")
> removed voltage switch delays from sdhci because mmc core had been
> enhanced to support them. However that assumed that sdhci_set_ios() did a
> single clock change, which it did not, and so the delays in mmc core, which should
> have come after the first clock change, were not effective.
>
> Fix by avoiding re-configuring UHS and preset settings when the clock is turning
> on and the settings have not changed. That then also avoids the associated clock
> changes, so that then sdhci_set_ios() does a single clock change when voltage
> switching, and the mmc core delays become effective.
>
> To do that has meant keeping track of driver strength (host->drv_type), and
> cases of reinitialization (host->reinit_uhs).
>
> Note also, the 'turning_on_clk' restriction should not be necessary but is done to
> minimize the impact of the change on stable kernels.
>
> Fixes: 20b92a30b561 ("mmc: sdhci: update signal voltage switch code")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Hi Adrian,
These patch set take care of the card clock on/off by CARD_EN bit, but for the USDHC on i.MX series, it do not implement these bits. Instead, it use another bit named force_clk_on to gate on/off the card clock.
So logically, USDHC do not has this issue.
I apply your patch and did a quick test on i.MX platform, do not find any side effect.
So for the patch set:
Tested-by: Haibo Chen <haibo.chen@xxxxxxx>
Best Regards
Haibo Chen
> ---
> drivers/mmc/host/sdhci.c | 51 ++++++++++++++++++++++++++++++++++------
> drivers/mmc/host/sdhci.h | 2 ++
> 2 files changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
> 2108e8075609..351b09b90d76 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -373,6 +373,7 @@ static void sdhci_init(struct sdhci_host *host, int soft)
> if (soft) {
> /* force clock reconfiguration */
> host->clock = 0;
> + host->reinit_uhs = true;
> mmc->ops->set_ios(mmc, &mmc->ios);
> }
> }
> @@ -2279,11 +2280,35 @@ void sdhci_set_uhs_signaling(struct sdhci_host
> *host, unsigned timing) } EXPORT_SYMBOL_GPL(sdhci_set_uhs_signaling);
>
> +static bool sdhci_timing_has_preset(unsigned char timing) {
> + switch (timing) {
> + case MMC_TIMING_UHS_SDR12:
> + case MMC_TIMING_UHS_SDR25:
> + case MMC_TIMING_UHS_SDR50:
> + case MMC_TIMING_UHS_SDR104:
> + case MMC_TIMING_UHS_DDR50:
> + case MMC_TIMING_MMC_DDR52:
> + return true;
> + };
> + return false;
> +}
> +
> +static bool sdhci_preset_needed(struct sdhci_host *host, unsigned char
> +timing) {
> + return !(host->quirks2 & SDHCI_QUIRK2_PRESET_VALUE_BROKEN) &&
> + sdhci_timing_has_preset(timing); }
> +
> void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) {
> struct sdhci_host *host = mmc_priv(mmc);
> + bool reinit_uhs = host->reinit_uhs;
> + bool turning_on_clk = false;
> u8 ctrl;
>
> + host->reinit_uhs = false;
> +
> if (ios->power_mode == MMC_POWER_UNDEFINED)
> return;
>
> @@ -2309,6 +2334,8 @@ void sdhci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
> sdhci_enable_preset_value(host, false);
>
> if (!ios->clock || ios->clock != host->clock) {
> + turning_on_clk = ios->clock && !host->clock;
> +
> host->ops->set_clock(host, ios->clock);
> host->clock = ios->clock;
>
> @@ -2335,6 +2362,18 @@ void sdhci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
>
> host->ops->set_bus_width(host, ios->bus_width);
>
> + /*
> + * Special case to avoid multiple clock changes during voltage
> + * switching.
> + */
> + if (!reinit_uhs &&
> + turning_on_clk &&
> + host->timing == ios->timing &&
> + host->version >= SDHCI_SPEC_300 &&
> + (host->preset_enabled || host->drv_type == ios->drv_type) &&
> + (host->preset_enabled || !sdhci_preset_needed(host, ios->timing)))
> + return;
> +
> ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
>
> if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT)) { @@ -2378,6 +2417,7
> @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> }
>
> sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> + host->drv_type = ios->drv_type;
> } else {
> /*
> * According to SDHC Spec v3.00, if the Preset Value @@
> -2405,19 +2445,14 @@ void sdhci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
> host->ops->set_uhs_signaling(host, ios->timing);
> host->timing = ios->timing;
>
> - if (!(host->quirks2 & SDHCI_QUIRK2_PRESET_VALUE_BROKEN) &&
> - ((ios->timing == MMC_TIMING_UHS_SDR12) ||
> - (ios->timing == MMC_TIMING_UHS_SDR25) ||
> - (ios->timing == MMC_TIMING_UHS_SDR50) ||
> - (ios->timing == MMC_TIMING_UHS_SDR104) ||
> - (ios->timing == MMC_TIMING_UHS_DDR50) ||
> - (ios->timing == MMC_TIMING_MMC_DDR52))) {
> + if (sdhci_preset_needed(host, ios->timing)) {
> u16 preset;
>
> sdhci_enable_preset_value(host, true);
> preset = sdhci_get_preset_value(host);
> ios->drv_type = FIELD_GET(SDHCI_PRESET_DRV_MASK,
> preset);
> + host->drv_type = ios->drv_type;
> }
>
> /* Re-enable SD Clock */
> @@ -3754,6 +3789,7 @@ int sdhci_resume_host(struct sdhci_host *host)
> sdhci_init(host, 0);
> host->pwr = 0;
> host->clock = 0;
> + host->reinit_uhs = true;
> mmc->ops->set_ios(mmc, &mmc->ios);
> } else {
> sdhci_init(host, (mmc->pm_flags & MMC_PM_KEEP_POWER)); @@
> -3816,6 +3852,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host,
> int soft_reset)
> /* Force clock and power re-program */
> host->pwr = 0;
> host->clock = 0;
> + host->reinit_uhs = true;
> mmc->ops->start_signal_voltage_switch(mmc, &mmc->ios);
> mmc->ops->set_ios(mmc, &mmc->ios);
>
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index
> 908da47ac5ba..b6f31a7d6152 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -524,6 +524,8 @@ struct sdhci_host {
>
> unsigned int clock; /* Current clock (MHz) */
> u8 pwr; /* Current voltage */
> + u8 drv_type; /* Current UHS-I driver type */
> + bool reinit_uhs; /* Force UHS-related re-initialization */
>
> bool runtime_suspended; /* Host is runtime suspended */
> bool bus_on; /* Bus power prevents runtime suspend */
> --
> 2.34.1
