linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] mmc: sdhci-acpi: Fix HS400 tuning for AMDI0040
@ 2020-08-19 19:00 Raul E Rangel
  2020-08-21  0:24 ` Nicolas Boichat
  0 siblings, 1 reply; 4+ messages in thread
From: Raul E Rangel @ 2020-08-19 19:00 UTC (permalink / raw)
  To: adrian.hunter
  Cc: Akshu.Agrawal, chris.wang, Nehal-bakulchandra.Shah,
	Raul E Rangel, Ulf Hansson, linux-kernel, linux-mmc

The AMD eMMC Controller can only use the tuned clock while in HS200 and
HS400 mode. If we switch to a different mode, we need to disable the
tuned clock. If we have previously performed tuning and switch back to
HS200 or HS400, we can re-enable the tuned clock.

Previously the tuned clock was not getting disabled when switching to
DDR52 which is part of the HS400 tuning sequence.

Fixes: 34597a3f60b1 ("mmc: sdhci-acpi: Add support for ACPI HID of AMD Controller with HS400")
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---

Changes in v2:
- Added static to amd_sdhci_execute_tuning

 drivers/mmc/host/sdhci-acpi.c | 68 +++++++++++++++++++++++++++++------
 1 file changed, 58 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 48ecbd0b180d8..a832d917e2fe3 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -535,6 +535,11 @@ static const struct sdhci_acpi_slot sdhci_acpi_slot_qcom_sd = {
 	.caps    = MMC_CAP_NONREMOVABLE,
 };
 
+struct amd_sdhci_host {
+	bool	tuned_clock;
+	bool	dll_enabled;
+};
+
 /* AMD sdhci reset dll register. */
 #define SDHCI_AMD_RESET_DLL_REGISTER    0x908
 
@@ -555,26 +560,67 @@ static void sdhci_acpi_amd_hs400_dll(struct sdhci_host *host)
 }
 
 /*
- * For AMD Platform it is required to disable the tuning
- * bit first controller to bring to HS Mode from HS200
- * mode, later enable to tune to HS400 mode.
+ * The initialization sequence for HS400 is:
+ *     HS->HS200->Perform Tuning->HS->HS400
+ *
+ * The re-tuning sequence is:
+ *     HS400->DDR52->HS->HS200->Perform Tuning->HS->HS400
+ *
+ * The AMD eMMC Controller can only use the tuned clock while in HS200 and HS400
+ * mode. If we switch to a different mode, we need to disable the tuned clock.
+ * If we have previously performed tuning and switch back to HS200 or
+ * HS400, we can re-enable the tuned clock.
+ *
  */
 static void amd_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct sdhci_host *host = mmc_priv(mmc);
+	struct sdhci_acpi_host *acpi_host = sdhci_priv(host);
+	struct amd_sdhci_host *amd_host = sdhci_acpi_priv(acpi_host);
 	unsigned int old_timing = host->timing;
+	u16 val;
 
 	sdhci_set_ios(mmc, ios);
-	if (old_timing == MMC_TIMING_MMC_HS200 &&
-	    ios->timing == MMC_TIMING_MMC_HS)
-		sdhci_writew(host, 0x9, SDHCI_HOST_CONTROL2);
-	if (old_timing != MMC_TIMING_MMC_HS400 &&
-	    ios->timing == MMC_TIMING_MMC_HS400) {
-		sdhci_writew(host, 0x80, SDHCI_HOST_CONTROL2);
-		sdhci_acpi_amd_hs400_dll(host);
+
+	if (old_timing != host->timing && amd_host->tuned_clock) {
+		if (host->timing == MMC_TIMING_MMC_HS400 ||
+		    host->timing == MMC_TIMING_MMC_HS200) {
+			val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+			val |= SDHCI_CTRL_TUNED_CLK;
+			sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
+		} else {
+			val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+			val &= ~SDHCI_CTRL_TUNED_CLK;
+			sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
+		}
+
+		/* DLL is only required for HS400 */
+		if (host->timing == MMC_TIMING_MMC_HS400 &&
+		    !amd_host->dll_enabled) {
+			trace_printk("%s: Enabling DLL\n", __func__);
+			sdhci_acpi_amd_hs400_dll(host);
+			amd_host->dll_enabled = true;
+		}
 	}
 }
 
+static int amd_sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
+{
+	int err;
+	struct sdhci_host *host = mmc_priv(mmc);
+	struct sdhci_acpi_host *acpi_host = sdhci_priv(host);
+	struct amd_sdhci_host *amd_host = sdhci_acpi_priv(acpi_host);
+
+	amd_host->tuned_clock = false;
+
+	err = sdhci_execute_tuning(mmc, opcode);
+
+	if (!err && !host->tuning_err)
+		amd_host->tuned_clock = true;
+
+	return err;
+}
+
 static const struct sdhci_ops sdhci_acpi_ops_amd = {
 	.set_clock	= sdhci_set_clock,
 	.set_bus_width	= sdhci_set_bus_width,
@@ -602,6 +648,7 @@ static int sdhci_acpi_emmc_amd_probe_slot(struct platform_device *pdev,
 
 	host->mmc_host_ops.select_drive_strength = amd_select_drive_strength;
 	host->mmc_host_ops.set_ios = amd_set_ios;
+	host->mmc_host_ops.execute_tuning = amd_sdhci_execute_tuning;
 	return 0;
 }
 
@@ -613,6 +660,7 @@ static const struct sdhci_acpi_slot sdhci_acpi_slot_amd_emmc = {
 			  SDHCI_QUIRK_32BIT_ADMA_SIZE,
 	.quirks2	= SDHCI_QUIRK2_BROKEN_64_BIT_DMA,
 	.probe_slot     = sdhci_acpi_emmc_amd_probe_slot,
+	.priv_size	= sizeof(struct amd_sdhci_host),
 };
 
 struct sdhci_acpi_uid_slot {
-- 
2.28.0.220.ged08abb693-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] mmc: sdhci-acpi: Fix HS400 tuning for AMDI0040
  2020-08-19 19:00 [PATCH v2] mmc: sdhci-acpi: Fix HS400 tuning for AMDI0040 Raul E Rangel
@ 2020-08-21  0:24 ` Nicolas Boichat
  2020-08-21  9:03   ` Ulf Hansson
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Boichat @ 2020-08-21  0:24 UTC (permalink / raw)
  To: Raul E Rangel
  Cc: adrian.hunter, Akshu.Agrawal, chris.wang,
	Nehal-bakulchandra.Shah, Ulf Hansson, lkml, linux-mmc,
	Steven Rostedt

On Thu, Aug 20, 2020 at 3:00 AM Raul E Rangel <rrangel@chromium.org> wrote:
>
> The AMD eMMC Controller can only use the tuned clock while in HS200 and
> HS400 mode. If we switch to a different mode, we need to disable the
> tuned clock. If we have previously performed tuning and switch back to
> HS200 or HS400, we can re-enable the tuned clock.
>
> Previously the tuned clock was not getting disabled when switching to
> DDR52 which is part of the HS400 tuning sequence.
>
> Fixes: 34597a3f60b1 ("mmc: sdhci-acpi: Add support for ACPI HID of AMD Controller with HS400")
> Signed-off-by: Raul E Rangel <rrangel@chromium.org>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>
> Changes in v2:
> - Added static to amd_sdhci_execute_tuning
>
>  drivers/mmc/host/sdhci-acpi.c | 68 +++++++++++++++++++++++++++++------
>  1 file changed, 58 insertions(+), 10 deletions(-)
[snip]
> +               /* DLL is only required for HS400 */
> +               if (host->timing == MMC_TIMING_MMC_HS400 &&
> +                   !amd_host->dll_enabled) {
> +                       trace_printk("%s: Enabling DLL\n", __func__);

Please do not use trace_printk in production code [1,2], it is only
meant for debug use. Consider using dev_dbg.

[1] https://elixir.bootlin.com/linux/v5.8/source/kernel/trace/trace.c#L3158
[2] https://elixir.bootlin.com/linux/v5.8/source/include/linux/kernel.h#L766

> +                       sdhci_acpi_amd_hs400_dll(host);
> +                       amd_host->dll_enabled = true;
> +               }
>         }

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] mmc: sdhci-acpi: Fix HS400 tuning for AMDI0040
  2020-08-21  0:24 ` Nicolas Boichat
@ 2020-08-21  9:03   ` Ulf Hansson
       [not found]     ` <CAHQZ30BW4YMzK9KUOHP8wV49Yw0UG_PZY6UdaQZdQRdMYFjFwQ@mail.gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Ulf Hansson @ 2020-08-21  9:03 UTC (permalink / raw)
  To: Raul E Rangel
  Cc: Nicolas Boichat, Adrian Hunter, Akshu Agrawal, chris.wang, Shah,
	Nehal-bakulchandra, lkml, linux-mmc, Steven Rostedt

On Fri, 21 Aug 2020 at 02:24, Nicolas Boichat <drinkcat@chromium.org> wrote:
>
> On Thu, Aug 20, 2020 at 3:00 AM Raul E Rangel <rrangel@chromium.org> wrote:
> >
> > The AMD eMMC Controller can only use the tuned clock while in HS200 and
> > HS400 mode. If we switch to a different mode, we need to disable the
> > tuned clock. If we have previously performed tuning and switch back to
> > HS200 or HS400, we can re-enable the tuned clock.
> >
> > Previously the tuned clock was not getting disabled when switching to
> > DDR52 which is part of the HS400 tuning sequence.
> >
> > Fixes: 34597a3f60b1 ("mmc: sdhci-acpi: Add support for ACPI HID of AMD Controller with HS400")
> > Signed-off-by: Raul E Rangel <rrangel@chromium.org>
> > Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Applied for fixes, by dropping the trace_printk below, thanks!

Kind regards
Uffe


> > ---
> >
> > Changes in v2:
> > - Added static to amd_sdhci_execute_tuning
> >
> >  drivers/mmc/host/sdhci-acpi.c | 68 +++++++++++++++++++++++++++++------
> >  1 file changed, 58 insertions(+), 10 deletions(-)
> [snip]
> > +               /* DLL is only required for HS400 */
> > +               if (host->timing == MMC_TIMING_MMC_HS400 &&
> > +                   !amd_host->dll_enabled) {
> > +                       trace_printk("%s: Enabling DLL\n", __func__);
>
> Please do not use trace_printk in production code [1,2], it is only
> meant for debug use. Consider using dev_dbg.
>
> [1] https://elixir.bootlin.com/linux/v5.8/source/kernel/trace/trace.c#L3158
> [2] https://elixir.bootlin.com/linux/v5.8/source/include/linux/kernel.h#L766
>
> > +                       sdhci_acpi_amd_hs400_dll(host);
> > +                       amd_host->dll_enabled = true;
> > +               }
> >         }

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] mmc: sdhci-acpi: Fix HS400 tuning for AMDI0040
       [not found]     ` <CAHQZ30BW4YMzK9KUOHP8wV49Yw0UG_PZY6UdaQZdQRdMYFjFwQ@mail.gmail.com>
@ 2020-08-21 21:56       ` Ulf Hansson
  0 siblings, 0 replies; 4+ messages in thread
From: Ulf Hansson @ 2020-08-21 21:56 UTC (permalink / raw)
  To: Raul Rangel
  Cc: Nicolas Boichat, Adrian Hunter, Akshu Agrawal, Wang, Chris, Shah,
	Nehal-bakulchandra, lkml, linux-mmc, Steven Rostedt

On Fri, 21 Aug 2020 at 16:31, Raul Rangel <rrangel@chromium.org> wrote:
>
> Oops, what was embarrassing! Thanks Ulf for removing it. Thanks Nick for caching that.

No worries, we all make mistakes. The important thing is that we take
good care of fixing them as soon as possible.

Kind regards
Uffe

>
> On Fri, Aug 21, 2020 at 3:04 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> On Fri, 21 Aug 2020 at 02:24, Nicolas Boichat <drinkcat@chromium.org> wrote:
>> >
>> > On Thu, Aug 20, 2020 at 3:00 AM Raul E Rangel <rrangel@chromium.org> wrote:
>> > >
>> > > The AMD eMMC Controller can only use the tuned clock while in HS200 and
>> > > HS400 mode. If we switch to a different mode, we need to disable the
>> > > tuned clock. If we have previously performed tuning and switch back to
>> > > HS200 or HS400, we can re-enable the tuned clock.
>> > >
>> > > Previously the tuned clock was not getting disabled when switching to
>> > > DDR52 which is part of the HS400 tuning sequence.
>> > >
>> > > Fixes: 34597a3f60b1 ("mmc: sdhci-acpi: Add support for ACPI HID of AMD Controller with HS400")
>> > > Signed-off-by: Raul E Rangel <rrangel@chromium.org>
>> > > Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>>
>> Applied for fixes, by dropping the trace_printk below, thanks!
>>
>> Kind regards
>> Uffe
>>
>>
>> > > ---
>> > >
>> > > Changes in v2:
>> > > - Added static to amd_sdhci_execute_tuning
>> > >
>> > >  drivers/mmc/host/sdhci-acpi.c | 68 +++++++++++++++++++++++++++++------
>> > >  1 file changed, 58 insertions(+), 10 deletions(-)
>> > [snip]
>> > > +               /* DLL is only required for HS400 */
>> > > +               if (host->timing == MMC_TIMING_MMC_HS400 &&
>> > > +                   !amd_host->dll_enabled) {
>> > > +                       trace_printk("%s: Enabling DLL\n", __func__);
>> >
>> > Please do not use trace_printk in production code [1,2], it is only
>> > meant for debug use. Consider using dev_dbg.
>> >
>> > [1] https://elixir.bootlin.com/linux/v5.8/source/kernel/trace/trace.c#L3158
>> > [2] https://elixir.bootlin.com/linux/v5.8/source/include/linux/kernel.h#L766
>> >
>> > > +                       sdhci_acpi_amd_hs400_dll(host);
>> > > +                       amd_host->dll_enabled = true;
>> > > +               }
>> > >         }

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-08-21 21:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-19 19:00 [PATCH v2] mmc: sdhci-acpi: Fix HS400 tuning for AMDI0040 Raul E Rangel
2020-08-21  0:24 ` Nicolas Boichat
2020-08-21  9:03   ` Ulf Hansson
     [not found]     ` <CAHQZ30BW4YMzK9KUOHP8wV49Yw0UG_PZY6UdaQZdQRdMYFjFwQ@mail.gmail.com>
2020-08-21 21:56       ` Ulf Hansson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).