Hi,
This new patch series changes the approach to enabling the no-1.8V
quirk. We determined that not having the DSM method might not be a good
enough signal for the board/eMMC not supporting 1.8V.
It now uses a DMI quirk; I've also added another machine which benefited
from the previous patch, but for which the issue is harder to reproduce.
I've kept the previous cleanup of the voltage_switch callback in an
optional third patch, but it does not enable any quirk.
Anisse Astier (3):
mmc: sdhci-pci: disable 1.8V with dmi quirk
mmc: sdhci-pci: add new machine with 1.8V dmi quirk
mmc: sdhci-pci: only install voltage switch method when useful
drivers/mmc/host/sdhci-pci-core.c | 34 +++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
--
2.17.2
If there's no ACPI DSM for voltage switch, it will just cause a lot of
debug info down the line, we only need one at startup.
Signed-off-by: Anisse Astier <[email protected]>
---
drivers/mmc/host/sdhci-pci-core.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index f2c1fb339d66..95fdbb883c7e 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -723,6 +723,7 @@ static const struct dmi_system_id board_no_1_8v[] = {
static void byt_probe_slot(struct sdhci_pci_slot *slot)
{
struct mmc_host_ops *ops = &slot->host->mmc_host_ops;
+ struct intel_host *intel_host = sdhci_pci_priv(slot);
byt_read_dsm(slot);
@@ -733,6 +734,16 @@ static void byt_probe_slot(struct sdhci_pci_slot *slot)
mmc_hostname(slot->host->mmc));
slot->host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
}
+ /* Check if we have the appropriate voltage switch DSM methods */
+ if (!(intel_host->dsm_fns & (1 << INTEL_DSM_V18_SWITCH)) &&
+ !(intel_host->dsm_fns & (1 << INTEL_DSM_V33_SWITCH))) {
+ /* No voltage switching supported at all, there's no
+ * point in installing the callback: return.
+ */
+ pr_debug("%s: No voltage switching ACPI DSM helper\n",
+ mmc_hostname(slot->host->mmc));
+ return;
+ }
ops->start_signal_voltage_switch = intel_start_signal_voltage_switch;
}
--
2.17.2
If the motherboard is known not to support 1.8V properly, add the
necessary quirk on probe.
This fixes an issue on a Gemini Lake (GLK) laptop : eMMC driver will
timeout on boot (from 60seconds to 10minutes ) as the cqhci attempts CQE
recovery after a failed voltage switch. In earlier kernels, the problem
existed, but only delayed boot for about 10 seconds after an I/O error,
allowing booting on the eMMC (almost) unnoticed.
Signed-off-by: Anisse Astier <[email protected]>
---
drivers/mmc/host/sdhci-pci-core.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 7bfd366d970d..396413f7c854 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -29,6 +29,7 @@
#include <linux/mmc/slot-gpio.h>
#include <linux/mmc/sdhci-pci-data.h>
#include <linux/acpi.h>
+#include <linux/dmi.h>
#include "cqhci.h"
@@ -703,6 +704,16 @@ static int intel_execute_tuning(struct mmc_host *mmc, u32 opcode)
return 0;
}
+static const struct dmi_system_id board_no_1_8v[] = {
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Notebook"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "N75_77GU"),
+ },
+ },
+ { }
+};
+
static void byt_probe_slot(struct sdhci_pci_slot *slot)
{
struct mmc_host_ops *ops = &slot->host->mmc_host_ops;
@@ -710,6 +721,12 @@ static void byt_probe_slot(struct sdhci_pci_slot *slot)
byt_read_dsm(slot);
ops->execute_tuning = intel_execute_tuning;
+
+ if (dmi_check_system(board_no_1_8v)) {
+ pr_debug("%s: motherboard does not support 1.8V\n",
+ mmc_hostname(slot->host->mmc));
+ slot->host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
+ }
ops->start_signal_voltage_switch = intel_start_signal_voltage_switch;
}
--
2.17.2
This machine also displays the same issue that is fixed by the quirk.
Signed-off-by: Anisse Astier <[email protected]>
---
drivers/mmc/host/sdhci-pci-core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 396413f7c854..f2c1fb339d66 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -711,6 +711,12 @@ static const struct dmi_system_id board_no_1_8v[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "N75_77GU"),
},
},
+ {
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Ordissimo"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Agathe3"),
+ },
+ },
{ }
};
--
2.17.2
Hi Adrian,
On Tue, Oct 23, 2018 at 12:07:29PM +0200, Anisse Astier wrote:
> If there's no ACPI DSM for voltage switch, it will just cause a lot of
> debug info down the line, we only need one at startup.
>
> Signed-off-by: Anisse Astier <[email protected]>
> ---
> drivers/mmc/host/sdhci-pci-core.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index f2c1fb339d66..95fdbb883c7e 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -723,6 +723,7 @@ static const struct dmi_system_id board_no_1_8v[] = {
> static void byt_probe_slot(struct sdhci_pci_slot *slot)
> {
> struct mmc_host_ops *ops = &slot->host->mmc_host_ops;
> + struct intel_host *intel_host = sdhci_pci_priv(slot);
>
> byt_read_dsm(slot);
>
> @@ -733,6 +734,16 @@ static void byt_probe_slot(struct sdhci_pci_slot *slot)
> mmc_hostname(slot->host->mmc));
> slot->host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
> }
> + /* Check if we have the appropriate voltage switch DSM methods */
> + if (!(intel_host->dsm_fns & (1 << INTEL_DSM_V18_SWITCH)) &&
> + !(intel_host->dsm_fns & (1 << INTEL_DSM_V33_SWITCH))) {
> + /* No voltage switching supported at all, there's no
> + * point in installing the callback: return.
> + */
> + pr_debug("%s: No voltage switching ACPI DSM helper\n",
> + mmc_hostname(slot->host->mmc));
> + return;
> + }
> ops->start_signal_voltage_switch = intel_start_signal_voltage_switch;
> }
>
> --
> 2.17.2
>
What do you think of picking this last patch ? Or maybe you had
different cleanups in mind when you said you wanted to rework this ?
Regards,
Anisse
On 16/11/18 6:58 PM, Anisse Astier wrote:
> Hi Adrian,
>
> On Tue, Oct 23, 2018 at 12:07:29PM +0200, Anisse Astier wrote:
>> If there's no ACPI DSM for voltage switch, it will just cause a lot of
>> debug info down the line, we only need one at startup.
>>
>> Signed-off-by: Anisse Astier <[email protected]>
>> ---
>> drivers/mmc/host/sdhci-pci-core.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
>> index f2c1fb339d66..95fdbb883c7e 100644
>> --- a/drivers/mmc/host/sdhci-pci-core.c
>> +++ b/drivers/mmc/host/sdhci-pci-core.c
>> @@ -723,6 +723,7 @@ static const struct dmi_system_id board_no_1_8v[] = {
>> static void byt_probe_slot(struct sdhci_pci_slot *slot)
>> {
>> struct mmc_host_ops *ops = &slot->host->mmc_host_ops;
>> + struct intel_host *intel_host = sdhci_pci_priv(slot);
>>
>> byt_read_dsm(slot);
>>
>> @@ -733,6 +734,16 @@ static void byt_probe_slot(struct sdhci_pci_slot *slot)
>> mmc_hostname(slot->host->mmc));
>> slot->host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
>> }
>> + /* Check if we have the appropriate voltage switch DSM methods */
>> + if (!(intel_host->dsm_fns & (1 << INTEL_DSM_V18_SWITCH)) &&
>> + !(intel_host->dsm_fns & (1 << INTEL_DSM_V33_SWITCH))) {
>> + /* No voltage switching supported at all, there's no
>> + * point in installing the callback: return.
>> + */
>> + pr_debug("%s: No voltage switching ACPI DSM helper\n",
>> + mmc_hostname(slot->host->mmc));
>> + return;
>> + }
>> ops->start_signal_voltage_switch = intel_start_signal_voltage_switch;
>> }
>>
>> --
>> 2.17.2
>>
>
> What do you think of picking this last patch ? Or maybe you had
> different cleanups in mind when you said you wanted to rework this ?
Voltage switches are relatively rare, and dynamic debug allows control over
exactly which debug messages display, so I am not sure this patch is needed.
On Mon, Nov 19, 2018 at 09:45:03AM +0200, Adrian Hunter wrote:
> On 16/11/18 6:58 PM, Anisse Astier wrote:
> > Hi Adrian,
> >
> > On Tue, Oct 23, 2018 at 12:07:29PM +0200, Anisse Astier wrote:
> >> If there's no ACPI DSM for voltage switch, it will just cause a lot of
> >> debug info down the line, we only need one at startup.
> >>
> >> Signed-off-by: Anisse Astier <[email protected]>
> >> ---
> >> drivers/mmc/host/sdhci-pci-core.c | 11 +++++++++++
> >> 1 file changed, 11 insertions(+)
> >>
> >> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> >> index f2c1fb339d66..95fdbb883c7e 100644
> >> --- a/drivers/mmc/host/sdhci-pci-core.c
> >> +++ b/drivers/mmc/host/sdhci-pci-core.c
> >> @@ -723,6 +723,7 @@ static const struct dmi_system_id board_no_1_8v[] = {
> >> static void byt_probe_slot(struct sdhci_pci_slot *slot)
> >> {
> >> struct mmc_host_ops *ops = &slot->host->mmc_host_ops;
> >> + struct intel_host *intel_host = sdhci_pci_priv(slot);
> >>
> >> byt_read_dsm(slot);
> >>
> >> @@ -733,6 +734,16 @@ static void byt_probe_slot(struct sdhci_pci_slot *slot)
> >> mmc_hostname(slot->host->mmc));
> >> slot->host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
> >> }
> >> + /* Check if we have the appropriate voltage switch DSM methods */
> >> + if (!(intel_host->dsm_fns & (1 << INTEL_DSM_V18_SWITCH)) &&
> >> + !(intel_host->dsm_fns & (1 << INTEL_DSM_V33_SWITCH))) {
> >> + /* No voltage switching supported at all, there's no
> >> + * point in installing the callback: return.
> >> + */
> >> + pr_debug("%s: No voltage switching ACPI DSM helper\n",
> >> + mmc_hostname(slot->host->mmc));
> >> + return;
> >> + }
> >> ops->start_signal_voltage_switch = intel_start_signal_voltage_switch;
> >> }
> >>
> >> --
> >> 2.17.2
> >>
> >
> > What do you think of picking this last patch ? Or maybe you had
> > different cleanups in mind when you said you wanted to rework this ?
>
> Voltage switches are relatively rare, and dynamic debug allows control over
> exactly which debug messages display, so I am not sure this patch is needed.
I just thought this message was a bit clearer than:
mmc0: intel_start_signal_voltage_switch DSM fn 4 error -95 result 0
But you're correct, with dynamic debug it shouldn't be an issue, and the
original message is easily searchable in the code. It only happens every
two seconds.
I'm ok to drop this patch.
Regards,
Anisse