On 12/06/17 16:27, Hans de Goede wrote: > Hi, > > On 12-06-17 14:11, Adrian Hunter wrote: >> On 08/06/17 21:55, Hans de Goede wrote: >>> Add a DMI based blacklist for systems where probing some sdio interfaces >>> is harmful (e.g. causes pci-e based wifi to not work). >>> >>> BugLink: https://bbs.archlinux.org/viewtopic.php?id=224086 >>> Fixes: db52d4f8a4bd ("mmc: sdhci-acpi: support 80860F14 UID 2 SDIO bus") >>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >>> --- >>> Changes in v2: >>> -Adjust for changes in mmc: sdhci-acpi: Add fix_up_power_blacklist module >>> option >>> -Only use a single fix_up_power_dmi_blacklist for the GPDwin further testing >>> has shown that the DMI strings are unique enough that we do not need the >>> bios-date in there >>> >>> Changes in v3: >>> -Adjust for changes to "mmc: sdhci-acpi: Add blacklist module option" >>> >>> Changes in v4: >>> -Rename blacklist to dmi_probe_blacklist as it now blacklists probing, >>> rather then calling acpi_device_fix_up_power. >>> -Also check bios-date against known bios-dates for the GPD win, to avoid >>> possible false positives due to the use of quite generic DMI strings >>> -Add Fixes and BugLink tags >>> --- >>> drivers/mmc/host/sdhci-acpi.c | 64 >>> +++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 64 insertions(+) >>> >>> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c >>> index ecc3aefd4643..3e12a6a8ad99 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,6 +84,11 @@ struct sdhci_acpi_host { >>> bool use_runtime_pm; >>> }; >>> +struct dmi_probe_blacklist_data { >>> + const char *hid_uid; >>> + const char * const *bios_dates; >>> +}; >>> + >>> static char *blacklist; >>> static bool sdhci_acpi_compare_hid_uid(const char *match, const char >>> *hid, >>> @@ -116,6 +122,34 @@ static bool sdhci_acpi_compare_hid_uid(const char >>> *match, const char *hid, >>> return false; >>> } >>> +static const char *sdhci_acpi_get_dmi_blacklist(const struct >>> dmi_system_id *bl) >>> +{ >>> + const struct dmi_system_id *dmi_id; >>> + const struct dmi_probe_blacklist_data *bl_data; >>> + const char *bios_date; >>> + int i; >>> + >>> + dmi_id = dmi_first_match(bl); >>> + if (!dmi_id) >>> + return NULL; >>> + >>> + bl_data = dmi_id->driver_data; >>> + >>> + if (!bl_data->bios_dates) >>> + return bl_data->hid_uid; >>> + >>> + bios_date = dmi_get_system_info(DMI_BIOS_DATE); >>> + if (!bios_date) >>> + return NULL; >>> + >>> + for (i = 0; bl_data->bios_dates[i]; i++) { >>> + if (strcmp(bl_data->bios_dates[i], bios_date) == 0) >>> + return bl_data->hid_uid; >>> + } >>> + >>> + return NULL; >>> +} >>> + >>> static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned >>> int flag) >>> { >>> return c->slot && (c->slot->flags & flag); >>> @@ -391,6 +425,33 @@ static const struct acpi_device_id sdhci_acpi_ids[] = { >>> }; >>> MODULE_DEVICE_TABLE(acpi, sdhci_acpi_ids); >>> +const struct dmi_probe_blacklist_data gpd_win_bl_data = { >>> + .hid_uid = "80860F14:2", >>> + .bios_dates = (const char * const []){ >>> + "10/25/2016", "11/18/2016", "02/21/2017", NULL }, >>> +}; >>> + >>> +static const struct dmi_system_id dmi_probe_blacklist[] = { >>> + { >>> + /* >>> + * Match for the GPDwin which unfortunately uses somewhat >>> + * generic dmi strings, which is why we test for 4 strings >>> + * and a known BIOS date. >>> + * Comparing against 29 other byt/cht boards, board_name is >>> + * unique to the GPDwin, where as only 2 other boards have the >>> + * same board_serial and 3 others have the same board_vendor >>> + */ >>> + .driver_data = (void *)&gpd_win_bl_data, >>> + .matches = { >>> + DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"), >>> + DMI_MATCH(DMI_BOARD_NAME, "Default string"), >>> + DMI_MATCH(DMI_BOARD_SERIAL, "Default string"), >>> + DMI_MATCH(DMI_PRODUCT_NAME, "Default string"), >>> + }, >> >> To me this is matching by accident rather than by design, which is not >> acceptable. > > I already explained why we need this dmi quirk in your reply of v3, > it would have been nice if you replied there. I understand what you are saying, but that doesn't make the patch acceptable, so I cannot Ack it. > > As I already mentioned when I first submitted this patch-set this > patch-set fixes a regression. When I first installed Linux on this > system, the wifi just worked, until this commit got merged: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=db52d4f8a4bde36263a7cc9d46ff20b243562ac9 > > > So that gives us 3 options: In the absence of another solution, the options are: 1. get the BIOS fixed 2. use the module option to blacklist the bad device > > 1) Revert the commit causing the regressions > 2) Do nothing, live with the regression. > 3) Add a DMI based quirk > > 1. is not an option since that commit is necessary to make wifi work > on other devices > > 2. is what you seem to be advocating, but since the kernel has a clear > no regressions policy that is not an option either > > 3. is thus the only option left. > > So unless you see a 4th option we really need to go with this patch, > note that in this version I've made the chance of false positives > for the DMI match even smaller then it was before because it now > needs to match a know bios-date too. > > Also note that this is being hit be actual users, not just by me, see: > > https://bbs.archlinux.org/viewtopic.php?id=224086 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
