public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* 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