diff mbox series

mmc: sdhci-acpi: Clear amd_sdhci_host on reset

Message ID 20200831150517.1.I93c78bfc6575771bb653c9d3fca5eb018a08417d@changeid
State New
Headers show
Series mmc: sdhci-acpi: Clear amd_sdhci_host on reset | expand

Commit Message

Raul E Rangel Aug. 31, 2020, 9:10 p.m. UTC
commit 61d7437ed1390 ("mmc: sdhci-acpi: Fix HS400 tuning for AMDI0040")
broke resume for HS400. When the system suspends the eMMC controller is
powered down. So on resume we need to reinitialize the controller.
amd_sdhci_host was not getting cleared, so the DLL was never re-enabled
on resume. This results in HS400 being non-functional.

This change clears the tuned_clock flag, clears the dll_enabled flag and
disables the DLL on reset.

Fixes: 61d7437ed1390 ("mmc: sdhci-acpi: Fix HS400 tuning for AMDI0040")

Signed-off-by: Raul E Rangel <rrangel@chromium.org>
---
- Performed 100+ suspend/resume cycles without issue.
- Also verified tuning continues to work.

 drivers/mmc/host/sdhci-acpi.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

Comments

Adrian Hunter Sept. 1, 2020, 12:19 p.m. UTC | #1
On 1/09/20 12:10 am, Raul E Rangel wrote:
> commit 61d7437ed1390 ("mmc: sdhci-acpi: Fix HS400 tuning for AMDI0040")
> broke resume for HS400. When the system suspends the eMMC controller is
> powered down. So on resume we need to reinitialize the controller.
> amd_sdhci_host was not getting cleared, so the DLL was never re-enabled
> on resume. This results in HS400 being non-functional.
> 
> This change clears the tuned_clock flag, clears the dll_enabled flag and
> disables the DLL on reset.
> 
> Fixes: 61d7437ed1390 ("mmc: sdhci-acpi: Fix HS400 tuning for AMDI0040")
> 
> Signed-off-by: Raul E Rangel <rrangel@chromium.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> - Performed 100+ suspend/resume cycles without issue.
> - Also verified tuning continues to work.
> 
>  drivers/mmc/host/sdhci-acpi.c | 31 ++++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index 962f074ca1742..284cba11e2795 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -551,12 +551,18 @@ static int amd_select_drive_strength(struct mmc_card *card,
>  	return MMC_SET_DRIVER_TYPE_A;
>  }
>  
> -static void sdhci_acpi_amd_hs400_dll(struct sdhci_host *host)
> +static void sdhci_acpi_amd_hs400_dll(struct sdhci_host *host, bool enable)
>  {
> +	struct sdhci_acpi_host *acpi_host = sdhci_priv(host);
> +	struct amd_sdhci_host *amd_host = sdhci_acpi_priv(acpi_host);
> +
>  	/* AMD Platform requires dll setting */
>  	sdhci_writel(host, 0x40003210, SDHCI_AMD_RESET_DLL_REGISTER);
>  	usleep_range(10, 20);
> -	sdhci_writel(host, 0x40033210, SDHCI_AMD_RESET_DLL_REGISTER);
> +	if (enable)
> +		sdhci_writel(host, 0x40033210, SDHCI_AMD_RESET_DLL_REGISTER);
> +
> +	amd_host->dll_enabled = enable;
>  }
>  
>  /*
> @@ -596,10 +602,8 @@ static void amd_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  
>  		/* DLL is only required for HS400 */
>  		if (host->timing == MMC_TIMING_MMC_HS400 &&
> -		    !amd_host->dll_enabled) {
> -			sdhci_acpi_amd_hs400_dll(host);
> -			amd_host->dll_enabled = true;
> -		}
> +		    !amd_host->dll_enabled)
> +			sdhci_acpi_amd_hs400_dll(host, true);
>  	}
>  }
>  
> @@ -620,10 +624,23 @@ static int amd_sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
>  	return err;
>  }
>  
> +static void amd_sdhci_reset(struct sdhci_host *host, u8 mask)
> +{
> +	struct sdhci_acpi_host *acpi_host = sdhci_priv(host);
> +	struct amd_sdhci_host *amd_host = sdhci_acpi_priv(acpi_host);
> +
> +	if (mask & SDHCI_RESET_ALL) {
> +		amd_host->tuned_clock = false;
> +		sdhci_acpi_amd_hs400_dll(host, false);
> +	}
> +
> +	sdhci_reset(host, mask);
> +}
> +
>  static const struct sdhci_ops sdhci_acpi_ops_amd = {
>  	.set_clock	= sdhci_set_clock,
>  	.set_bus_width	= sdhci_set_bus_width,
> -	.reset		= sdhci_reset,
> +	.reset		= amd_sdhci_reset,
>  	.set_uhs_signaling = sdhci_set_uhs_signaling,
>  };
>  
>
Ulf Hansson Sept. 2, 2020, 9:02 a.m. UTC | #2
On Mon, 31 Aug 2020 at 23:10, Raul E Rangel <rrangel@chromium.org> wrote:
>
> commit 61d7437ed1390 ("mmc: sdhci-acpi: Fix HS400 tuning for AMDI0040")
> broke resume for HS400. When the system suspends the eMMC controller is
> powered down. So on resume we need to reinitialize the controller.
> amd_sdhci_host was not getting cleared, so the DLL was never re-enabled
> on resume. This results in HS400 being non-functional.
>
> This change clears the tuned_clock flag, clears the dll_enabled flag and
> disables the DLL on reset.
>
> Fixes: 61d7437ed1390 ("mmc: sdhci-acpi: Fix HS400 tuning for AMDI0040")
>
> Signed-off-by: Raul E Rangel <rrangel@chromium.org>

Applied for fixes, by updating the commit message a bit, thanks!

Kind regards
Uffe


> ---
> - Performed 100+ suspend/resume cycles without issue.
> - Also verified tuning continues to work.
>
>  drivers/mmc/host/sdhci-acpi.c | 31 ++++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index 962f074ca1742..284cba11e2795 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -551,12 +551,18 @@ static int amd_select_drive_strength(struct mmc_card *card,
>         return MMC_SET_DRIVER_TYPE_A;
>  }
>
> -static void sdhci_acpi_amd_hs400_dll(struct sdhci_host *host)
> +static void sdhci_acpi_amd_hs400_dll(struct sdhci_host *host, bool enable)
>  {
> +       struct sdhci_acpi_host *acpi_host = sdhci_priv(host);
> +       struct amd_sdhci_host *amd_host = sdhci_acpi_priv(acpi_host);
> +
>         /* AMD Platform requires dll setting */
>         sdhci_writel(host, 0x40003210, SDHCI_AMD_RESET_DLL_REGISTER);
>         usleep_range(10, 20);
> -       sdhci_writel(host, 0x40033210, SDHCI_AMD_RESET_DLL_REGISTER);
> +       if (enable)
> +               sdhci_writel(host, 0x40033210, SDHCI_AMD_RESET_DLL_REGISTER);
> +
> +       amd_host->dll_enabled = enable;
>  }
>
>  /*
> @@ -596,10 +602,8 @@ static void amd_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>
>                 /* DLL is only required for HS400 */
>                 if (host->timing == MMC_TIMING_MMC_HS400 &&
> -                   !amd_host->dll_enabled) {
> -                       sdhci_acpi_amd_hs400_dll(host);
> -                       amd_host->dll_enabled = true;
> -               }
> +                   !amd_host->dll_enabled)
> +                       sdhci_acpi_amd_hs400_dll(host, true);
>         }
>  }
>
> @@ -620,10 +624,23 @@ static int amd_sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
>         return err;
>  }
>
> +static void amd_sdhci_reset(struct sdhci_host *host, u8 mask)
> +{
> +       struct sdhci_acpi_host *acpi_host = sdhci_priv(host);
> +       struct amd_sdhci_host *amd_host = sdhci_acpi_priv(acpi_host);
> +
> +       if (mask & SDHCI_RESET_ALL) {
> +               amd_host->tuned_clock = false;
> +               sdhci_acpi_amd_hs400_dll(host, false);
> +       }
> +
> +       sdhci_reset(host, mask);
> +}
> +
>  static const struct sdhci_ops sdhci_acpi_ops_amd = {
>         .set_clock      = sdhci_set_clock,
>         .set_bus_width  = sdhci_set_bus_width,
> -       .reset          = sdhci_reset,
> +       .reset          = amd_sdhci_reset,
>         .set_uhs_signaling = sdhci_set_uhs_signaling,
>  };
>
> --
> 2.28.0.402.g5ffc5be6b7-goog
>
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 962f074ca1742..284cba11e2795 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -551,12 +551,18 @@  static int amd_select_drive_strength(struct mmc_card *card,
 	return MMC_SET_DRIVER_TYPE_A;
 }
 
-static void sdhci_acpi_amd_hs400_dll(struct sdhci_host *host)
+static void sdhci_acpi_amd_hs400_dll(struct sdhci_host *host, bool enable)
 {
+	struct sdhci_acpi_host *acpi_host = sdhci_priv(host);
+	struct amd_sdhci_host *amd_host = sdhci_acpi_priv(acpi_host);
+
 	/* AMD Platform requires dll setting */
 	sdhci_writel(host, 0x40003210, SDHCI_AMD_RESET_DLL_REGISTER);
 	usleep_range(10, 20);
-	sdhci_writel(host, 0x40033210, SDHCI_AMD_RESET_DLL_REGISTER);
+	if (enable)
+		sdhci_writel(host, 0x40033210, SDHCI_AMD_RESET_DLL_REGISTER);
+
+	amd_host->dll_enabled = enable;
 }
 
 /*
@@ -596,10 +602,8 @@  static void amd_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 		/* DLL is only required for HS400 */
 		if (host->timing == MMC_TIMING_MMC_HS400 &&
-		    !amd_host->dll_enabled) {
-			sdhci_acpi_amd_hs400_dll(host);
-			amd_host->dll_enabled = true;
-		}
+		    !amd_host->dll_enabled)
+			sdhci_acpi_amd_hs400_dll(host, true);
 	}
 }
 
@@ -620,10 +624,23 @@  static int amd_sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	return err;
 }
 
+static void amd_sdhci_reset(struct sdhci_host *host, u8 mask)
+{
+	struct sdhci_acpi_host *acpi_host = sdhci_priv(host);
+	struct amd_sdhci_host *amd_host = sdhci_acpi_priv(acpi_host);
+
+	if (mask & SDHCI_RESET_ALL) {
+		amd_host->tuned_clock = false;
+		sdhci_acpi_amd_hs400_dll(host, false);
+	}
+
+	sdhci_reset(host, mask);
+}
+
 static const struct sdhci_ops sdhci_acpi_ops_amd = {
 	.set_clock	= sdhci_set_clock,
 	.set_bus_width	= sdhci_set_bus_width,
-	.reset		= sdhci_reset,
+	.reset		= amd_sdhci_reset,
 	.set_uhs_signaling = sdhci_set_uhs_signaling,
 };