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 <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
---
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
On Thu, Aug 20, 2020 at 3:00 AM Raul E Rangel <[email protected]> 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 <[email protected]>
> Acked-by: Adrian Hunter <[email protected]>
> ---
>
> 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;
> + }
> }
On Fri, 21 Aug 2020 at 02:24, Nicolas Boichat <[email protected]> wrote:
>
> On Thu, Aug 20, 2020 at 3:00 AM Raul E Rangel <[email protected]> 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 <[email protected]>
> > Acked-by: Adrian Hunter <[email protected]>
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;
> > + }
> > }
On Fri, 21 Aug 2020 at 16:31, Raul Rangel <[email protected]> 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 <[email protected]> wrote:
>>
>> On Fri, 21 Aug 2020 at 02:24, Nicolas Boichat <[email protected]> wrote:
>> >
>> > On Thu, Aug 20, 2020 at 3:00 AM Raul E Rangel <[email protected]> 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 <[email protected]>
>> > > Acked-by: Adrian Hunter <[email protected]>
>>
>> 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;
>> > > + }
>> > > }