| Message ID | 20170404085013.19837-4-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/04/17 11:50, Hans de Goede wrote: > Add a DMI based blacklist for systems where calling > acpi_device_fix_up_power() is harmful. > > Note as the comment added above the GPDwin entry explains unfortunately > the GPDwin dmi strings are somewhat generic, so I've added the bios-date > string for a more unique match, which means we need 3 entries for the > GPDwin for the 3 known BIOS versions out there. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/mmc/host/sdhci-acpi.c | 46 ++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 45 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c > index 8fe46ba..826e34f 100644 > --- a/drivers/mmc/host/sdhci-acpi.c > +++ b/drivers/mmc/host/sdhci-acpi.c > @@ -36,6 +36,7 @@ > #include <linux/pm.h> > #include <linux/pm_runtime.h> > #include <linux/delay.h> > +#include <linux/dmi.h> > > #include <linux/mmc/host.h> > #include <linux/mmc/pm.h> > @@ -83,7 +84,7 @@ struct sdhci_acpi_host { > bool use_runtime_pm; > }; > > -static int fix_up_power = 1; > +static int fix_up_power = -1; > > static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag) > { > @@ -363,6 +364,42 @@ static const struct acpi_device_id sdhci_acpi_ids[] = { > }; > MODULE_DEVICE_TABLE(acpi, sdhci_acpi_ids); > > +static const struct dmi_system_id fix_up_power_blacklist[] = { > + { > + /* > + * Match for the GPDwin which unfortunately uses somewhat > + * generic dmi strings, which is why the bios-date match is > + * included and we need multiple entries :| These strings have > + * been checked against 6 other byt/cht boards and board_vendor > + * and board_name are unique to the GPDwin (in the test set) > + * where as only one other board has the same board_version. > + */ > + .matches = { > + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), > + DMI_MATCH(DMI_BOARD_NAME, "Default string"), > + DMI_MATCH(DMI_BOARD_VERSION, "Default string"), > + DMI_MATCH(DMI_BIOS_DATE, "10/25/2016"), > + }, > + }, > + { > + .matches = { > + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), > + DMI_MATCH(DMI_BOARD_NAME, "Default string"), > + DMI_MATCH(DMI_BOARD_VERSION, "Default string"), > + DMI_MATCH(DMI_BIOS_DATE, "11/18/2016"), > + }, > + }, > + { > + .matches = { > + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), > + DMI_MATCH(DMI_BOARD_NAME, "Default string"), > + DMI_MATCH(DMI_BOARD_VERSION, "Default string"), > + DMI_MATCH(DMI_BIOS_DATE, "02/21/2017"), And if there are more versions? I would prefer to add a module parameter to blacklist the offending HID:UID > + }, > + }, > + { } > +}; > + > static const struct sdhci_acpi_slot *sdhci_acpi_get_slot(const char *hid, > const char *uid) > { > @@ -395,6 +432,13 @@ static int sdhci_acpi_probe(struct platform_device *pdev) > if (acpi_bus_get_device(handle, &device)) > return -ENODEV; > > + if (fix_up_power == -1) { > + if (dmi_check_system(fix_up_power_blacklist)) > + fix_up_power = 0; > + else > + fix_up_power = 1; > + } > + > /* Power on the SDHCI controller and its children */ > if (fix_up_power) { > acpi_device_fix_up_power(device); > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 05-04-17 09:29, Adrian Hunter wrote: > On 04/04/17 11:50, Hans de Goede wrote: >> Add a DMI based blacklist for systems where calling >> acpi_device_fix_up_power() is harmful. >> >> Note as the comment added above the GPDwin entry explains unfortunately >> the GPDwin dmi strings are somewhat generic, so I've added the bios-date >> string for a more unique match, which means we need 3 entries for the >> GPDwin for the 3 known BIOS versions out there. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/mmc/host/sdhci-acpi.c | 46 ++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 45 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c >> index 8fe46ba..826e34f 100644 >> --- a/drivers/mmc/host/sdhci-acpi.c >> +++ b/drivers/mmc/host/sdhci-acpi.c >> @@ -36,6 +36,7 @@ >> #include <linux/pm.h> >> #include <linux/pm_runtime.h> >> #include <linux/delay.h> >> +#include <linux/dmi.h> >> >> #include <linux/mmc/host.h> >> #include <linux/mmc/pm.h> >> @@ -83,7 +84,7 @@ struct sdhci_acpi_host { >> bool use_runtime_pm; >> }; >> >> -static int fix_up_power = 1; >> +static int fix_up_power = -1; >> >> static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag) >> { >> @@ -363,6 +364,42 @@ static const struct acpi_device_id sdhci_acpi_ids[] = { >> }; >> MODULE_DEVICE_TABLE(acpi, sdhci_acpi_ids); >> >> +static const struct dmi_system_id fix_up_power_blacklist[] = { >> + { >> + /* >> + * Match for the GPDwin which unfortunately uses somewhat >> + * generic dmi strings, which is why the bios-date match is >> + * included and we need multiple entries :| These strings have >> + * been checked against 6 other byt/cht boards and board_vendor >> + * and board_name are unique to the GPDwin (in the test set) >> + * where as only one other board has the same board_version. >> + */ >> + .matches = { >> + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), >> + DMI_MATCH(DMI_BOARD_NAME, "Default string"), >> + DMI_MATCH(DMI_BOARD_VERSION, "Default string"), >> + DMI_MATCH(DMI_BIOS_DATE, "10/25/2016"), >> + }, >> + }, >> + { >> + .matches = { >> + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), >> + DMI_MATCH(DMI_BOARD_NAME, "Default string"), >> + DMI_MATCH(DMI_BOARD_VERSION, "Default string"), >> + DMI_MATCH(DMI_BIOS_DATE, "11/18/2016"), >> + }, >> + }, >> + { >> + .matches = { >> + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), >> + DMI_MATCH(DMI_BOARD_NAME, "Default string"), >> + DMI_MATCH(DMI_BOARD_VERSION, "Default string"), >> + DMI_MATCH(DMI_BIOS_DATE, "02/21/2017"), > > And if there are more versions? True, I already sort of expected this, so I've asked the internets to send me dmi strings for other Cherry Trail / Bay Trail devices, I now have a sample set of 15 devices. Among these the GPDwin is the only one to set DMI_BOARD_VENDOR to "AMI Corporation", as well as the only one to set DMI_BOARD_NAME to "Default string", one of only 2 to set DMI_BOARD_SERIAL to "Default string" and as one of only 3 to set DMI_PRODUCT_NAME to "Default string", so I think that if me on those 4 (the latter 2 to be on the save side) that we can then remove the BIOS date check and use only one entry, I will do so in the next version. I do believe that we really need a DMI quirk and not just a module option as this is something which should just work (and wifi not working actually is a recent regression, see below). > I would prefer to add a module parameter to blacklist the offending HID:UID I think that applying the module_param to only a single HID:UID is a good idea, this will also make the DMI match more narrow, esp. since the GPDwin uses the somewhat rare 80860F14:2 HID:UID which only got added to the kernel recently (and promptly regressed wifi support on the GPDwin). I will do so in the next version. When you say "black_list" do you mean not apply the acpi_power_fix_up calls to the selected HID:UID, or do you mean completely ignoring the device (return -ENODEV from probe()) ? I think only skipping the acpi_power_fix_up calls is better, because that I've the feeling that might help on some boards which do have sdio-wifi too, where as returning -ENODEV although it will work for the GPDwin case, is less likely to help in other cases (although I must admit I guess it may also help in some cases where only skipping the power-fixup might not). Regards, Hans > >> + }, >> + }, >> + { } >> +}; >> + >> static const struct sdhci_acpi_slot *sdhci_acpi_get_slot(const char *hid, >> const char *uid) >> { >> @@ -395,6 +432,13 @@ static int sdhci_acpi_probe(struct platform_device *pdev) >> if (acpi_bus_get_device(handle, &device)) >> return -ENODEV; >> >> + if (fix_up_power == -1) { >> + if (dmi_check_system(fix_up_power_blacklist)) >> + fix_up_power = 0; >> + else >> + fix_up_power = 1; >> + } >> + >> /* Power on the SDHCI controller and its children */ >> if (fix_up_power) { >> acpi_device_fix_up_power(device); >> > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c index 8fe46ba..826e34f 100644 --- a/drivers/mmc/host/sdhci-acpi.c +++ b/drivers/mmc/host/sdhci-acpi.c @@ -36,6 +36,7 @@ #include <linux/pm.h> #include <linux/pm_runtime.h> #include <linux/delay.h> +#include <linux/dmi.h> #include <linux/mmc/host.h> #include <linux/mmc/pm.h> @@ -83,7 +84,7 @@ struct sdhci_acpi_host { bool use_runtime_pm; }; -static int fix_up_power = 1; +static int fix_up_power = -1; static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag) { @@ -363,6 +364,42 @@ static const struct acpi_device_id sdhci_acpi_ids[] = { }; MODULE_DEVICE_TABLE(acpi, sdhci_acpi_ids); +static const struct dmi_system_id fix_up_power_blacklist[] = { + { + /* + * Match for the GPDwin which unfortunately uses somewhat + * generic dmi strings, which is why the bios-date match is + * included and we need multiple entries :| These strings have + * been checked against 6 other byt/cht boards and board_vendor + * and board_name are unique to the GPDwin (in the test set) + * where as only one other board has the same board_version. + */ + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), + DMI_MATCH(DMI_BOARD_NAME, "Default string"), + DMI_MATCH(DMI_BOARD_VERSION, "Default string"), + DMI_MATCH(DMI_BIOS_DATE, "10/25/2016"), + }, + }, + { + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), + DMI_MATCH(DMI_BOARD_NAME, "Default string"), + DMI_MATCH(DMI_BOARD_VERSION, "Default string"), + DMI_MATCH(DMI_BIOS_DATE, "11/18/2016"), + }, + }, + { + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), + DMI_MATCH(DMI_BOARD_NAME, "Default string"), + DMI_MATCH(DMI_BOARD_VERSION, "Default string"), + DMI_MATCH(DMI_BIOS_DATE, "02/21/2017"), + }, + }, + { } +}; + static const struct sdhci_acpi_slot *sdhci_acpi_get_slot(const char *hid, const char *uid) { @@ -395,6 +432,13 @@ static int sdhci_acpi_probe(struct platform_device *pdev) if (acpi_bus_get_device(handle, &device)) return -ENODEV; + if (fix_up_power == -1) { + if (dmi_check_system(fix_up_power_blacklist)) + fix_up_power = 0; + else + fix_up_power = 1; + } + /* Power on the SDHCI controller and its children */ if (fix_up_power) { acpi_device_fix_up_power(device);
Add a DMI based blacklist for systems where calling acpi_device_fix_up_power() is harmful. Note as the comment added above the GPDwin entry explains unfortunately the GPDwin dmi strings are somewhat generic, so I've added the bios-date string for a more unique match, which means we need 3 entries for the GPDwin for the 3 known BIOS versions out there. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/mmc/host/sdhci-acpi.c | 46 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-)