Re: [PATCH 3/3] mmc: sdhci-acpi: Add DMI fix_up_power_blacklist

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux