sdhci-pci voltage switch fixes
A message that is not in the archive (sent off-list or not yet received): 20181022134026.GB18413@jaya
v2 cover sdhci-pci voltage switch fixes
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(+)
Signature · 1 line
2.17.2
PATCH v2 1/3 mmc: sdhci-pci: disable 1.8V with dmi quirk
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.
--- drivers/mmc/host/sdhci-pci-core.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
drivers/mmc/host/sdhci-pci-core.c +17 -0
@@ -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; }
Signature · 1 line
2.17.2
PATCH v2 2/3 mmc: sdhci-pci: add new machine with 1.8V dmi quirk
In reply to Anisse AstierThis machine also displays the same issue that is fixed by the quirk.
--- drivers/mmc/host/sdhci-pci-core.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/mmc/host/sdhci-pci-core.c +6 -0
@@ -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"), + }, + }, { } };
Signature · 1 line
2.17.2
PATCH v2 3/3 mmc: sdhci-pci: only install voltage switch method when useful
In reply to Anisse AstierIf 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.
--- drivers/mmc/host/sdhci-pci-core.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
drivers/mmc/host/sdhci-pci-core.c +11 -0
@@ -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; }
Signature · 1 line
2.17.2
Re: [PATCH v2 3/3] mmc: sdhci-pci: only install voltage switch method when useful
Hi Adrian,
Quoted 40 lines · Anisse Astier
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(+)drivers/mmc/host/sdhci-pci-core.c +11
-0@@ -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; }Signature · 1 line
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
Adrian Hunter
maintainerRe: [PATCH v2 3/3] mmc: sdhci-pci: only install voltage switch method when useful
Quoted 40 lines · Anisse Astier
Hi Adrian,
Quoted 37 lines · Anisse Astier
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(+)drivers/mmc/host/sdhci-pci-core.c +11
-0@@ -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; }
Quoted 6 lines
Quoted 3 lines
Signature · 1 line
2.17.2What 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.
Re: [PATCH v2 3/3] mmc: sdhci-pci: only install voltage switch method when useful
Quoted 48 lines · Adrian Hunter
Quoted 40 lines · Anisse Astier
Hi Adrian,
Quoted 37 lines · Anisse Astier
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(+)drivers/mmc/host/sdhci-pci-core.c +11
-0@@ -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; }Quoted 6 lines
Quoted 3 lines
Signature · 1 line
2.17.2What do you think of picking this last patch ? Or maybe you had different cleanups in mind when you said you wanted to rework this ?
Quoted 2 lines
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