On 16/06/17 15:33, Hans de Goede wrote: > Hi, > > On 14-06-17 15:20, Hans de Goede wrote: >> Hi, >> >> On 14-06-17 09:43, Adrian Hunter wrote: >>> 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 >> >> a. That is not going to happen (I've already contacted the vendor). >> b. Even if that were to happen, almost no-one will update the BIOS, so >> this does not help >> >>> 2. use the module option to blacklist the bad device >> >> Needing to use a module-option, where before none was necessary >> is still a regression. I've personally had a commit of mine >> reverted by Torvalds himself because I changed something which >> would require the use a of a kernel cmdline option in certain >> corner-cases where no cmdline option was needed before. >> >> Basically your solutions boil down to my: >> >>>> 2) Do nothing, live with the regression. >> >>>> 2. is what you seem to be advocating, but since the kernel has a clear >>>> no regressions policy that is not an option either >> >> So your advocating we just live with the REGRESSION, because that >> is what this is a REGRESSION and nothing else. That is simply >> not acceptable (and clearly against kernel policy). >> >> I've compared DMI data to 29 other boards using the same chipset >> to prove the DMI match is unique, then since you are still worried >> about the match being too generic I also added BIOS date checking, >> which certainly makes the match more then unique enough, something to >> which you've not even responded... >> >> In the mean time users have been suffering from this regression >> for 3 months now: >> https://bbs.archlinux.org/viewtopic.php?id=224086 >> >> I've no words for this, other then that your blocking of fixing >> this REGRESSION, without you even addressing my factual arguments >> why this match is not too generic, vs you're feeling that it is >> too generic, simply is unacceptable. > > To be clear, I understand that needing DMI quirks in the first place > is undesirable, and that this vendor using way too generic strings > is adding extra ugliness to the ugliness of needing DMI quirks in > the first place, so I understand your reluctance here. > > But to me making this "just" work for users trumps my desire to > avoid ugliness like this. I really want to see Linux used by as much > users as possible and in order for that to happen we need to have > Ubunutu / Fedora just work with their hardware, if users first need > to google a kernel cmdline option, then they will just stop using > Linux. Perhaps there is something else we can match on, like the presence of the PCIe wifi device since we only use SDIO for wifi. Can you send a copy of the ACPI DSDT table, or an acpidump file. Also lspci output. -- 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
