* FAILED: patch "[PATCH] mmc: sdhci: Fix voltage switch delay" failed to apply to 5.4-stable tree
@ 2022-12-03 10:58 gregkh
2022-12-05 14:07 ` Adrian Hunter
0 siblings, 1 reply; 3+ messages in thread
From: gregkh @ 2022-12-03 10:58 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson; +Cc: stable
The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
Possible dependencies:
c981cdfb9925 ("mmc: sdhci: Fix voltage switch delay")
fa0910107a9f ("mmc: sdhci: use FIELD_GET for preset value bit masks")
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From c981cdfb9925f64a364f13c2b4f98f877308a408 Mon Sep 17 00:00:00 2001
From: Adrian Hunter <adrian.hunter@intel.com>
Date: Mon, 28 Nov 2022 15:32:56 +0200
Subject: [PATCH] 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@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Link: https://lore.kernel.org/r/20221128133259.38305-2-adrian.hunter@intel.com
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fef03de85b99..c7ad32a75b57 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);
}
}
@@ -2293,11 +2294,46 @@ 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);
+}
+
+static bool sdhci_presetable_values_change(struct sdhci_host *host, struct mmc_ios *ios)
+{
+ /*
+ * Preset Values are: Driver Strength, Clock Generator and SDCLK/RCLK
+ * Frequency. Check if preset values need to be enabled, or the Driver
+ * Strength needs updating. Note, clock changes are handled separately.
+ */
+ return !host->preset_enabled &&
+ (sdhci_preset_needed(host, ios->timing) || host->drv_type != ios->drv_type);
+}
+
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;
@@ -2323,6 +2359,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;
@@ -2349,6 +2387,17 @@ 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 &&
+ !sdhci_presetable_values_change(host, ios))
+ return;
+
ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT)) {
@@ -2392,6 +2441,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
@@ -2419,19 +2469,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 */
@@ -3768,6 +3813,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));
@@ -3830,6 +3876,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 d750c464bd1e..87a3aaa07438 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 */
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: FAILED: patch "[PATCH] mmc: sdhci: Fix voltage switch delay" failed to apply to 5.4-stable tree
2022-12-03 10:58 FAILED: patch "[PATCH] mmc: sdhci: Fix voltage switch delay" failed to apply to 5.4-stable tree gregkh
@ 2022-12-05 14:07 ` Adrian Hunter
2022-12-05 16:35 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: Adrian Hunter @ 2022-12-05 14:07 UTC (permalink / raw)
To: gregkh; +Cc: stable, ulf.hansson
On 3/12/22 12:58, gregkh@linuxfoundation.org wrote:
>
> The patch below does not apply to the 5.4-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
>
> Possible dependencies:
>
> c981cdfb9925 ("mmc: sdhci: Fix voltage switch delay")
> fa0910107a9f ("mmc: sdhci: use FIELD_GET for preset value bit masks")
Yes please cherry-pick fa0910107a9f then c981cdfb9925
Ditto for 4.19 and 4.14 too please.
>
> thanks,
>
> greg k-h
>
> ------------------ original commit in Linus's tree ------------------
>
> From c981cdfb9925f64a364f13c2b4f98f877308a408 Mon Sep 17 00:00:00 2001
> From: Adrian Hunter <adrian.hunter@intel.com>
> Date: Mon, 28 Nov 2022 15:32:56 +0200
> Subject: [PATCH] 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@vger.kernel.org
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> Link: https://lore.kernel.org/r/20221128133259.38305-2-adrian.hunter@intel.com
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index fef03de85b99..c7ad32a75b57 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);
> }
> }
> @@ -2293,11 +2294,46 @@ 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);
> +}
> +
> +static bool sdhci_presetable_values_change(struct sdhci_host *host, struct mmc_ios *ios)
> +{
> + /*
> + * Preset Values are: Driver Strength, Clock Generator and SDCLK/RCLK
> + * Frequency. Check if preset values need to be enabled, or the Driver
> + * Strength needs updating. Note, clock changes are handled separately.
> + */
> + return !host->preset_enabled &&
> + (sdhci_preset_needed(host, ios->timing) || host->drv_type != ios->drv_type);
> +}
> +
> 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;
>
> @@ -2323,6 +2359,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;
>
> @@ -2349,6 +2387,17 @@ 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 &&
> + !sdhci_presetable_values_change(host, ios))
> + return;
> +
> ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
>
> if (!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT)) {
> @@ -2392,6 +2441,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
> @@ -2419,19 +2469,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 */
> @@ -3768,6 +3813,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));
> @@ -3830,6 +3876,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 d750c464bd1e..87a3aaa07438 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 */
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: FAILED: patch "[PATCH] mmc: sdhci: Fix voltage switch delay" failed to apply to 5.4-stable tree
2022-12-05 14:07 ` Adrian Hunter
@ 2022-12-05 16:35 ` Greg KH
0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2022-12-05 16:35 UTC (permalink / raw)
To: Adrian Hunter; +Cc: stable, ulf.hansson
On Mon, Dec 05, 2022 at 04:07:12PM +0200, Adrian Hunter wrote:
> On 3/12/22 12:58, gregkh@linuxfoundation.org wrote:
> >
> > The patch below does not apply to the 5.4-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> >
> > Possible dependencies:
> >
> > c981cdfb9925 ("mmc: sdhci: Fix voltage switch delay")
> > fa0910107a9f ("mmc: sdhci: use FIELD_GET for preset value bit masks")
>
> Yes please cherry-pick fa0910107a9f then c981cdfb9925
>
> Ditto for 4.19 and 4.14 too please.
All now done, and for 4.9 too.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-05 16:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-03 10:58 FAILED: patch "[PATCH] mmc: sdhci: Fix voltage switch delay" failed to apply to 5.4-stable tree gregkh
2022-12-05 14:07 ` Adrian Hunter
2022-12-05 16:35 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox