[PATCH v2 0/7] ACPI: acpi_device_override_status() changes

[PATCH v2 0/7] ACPI: acpi_device_override_status() changes

[Hans de Goede]

Hi Rafael,

As requested here is a v2 of my series previously titled:
"ACPI: scan: Skip turning off some unused objects during scan"

Which was a regression fix series for the commit c10383e8ddf4
("ACPI: scan: Release PM resources blocked by unused objects")
change, but that has been reverted now. So as requested here is
a v2 changing the wording of various commit messages since these
changes are still useful to have regardless.

Patch 1/7 is a v2/resend of the "ACPI / x86: Drop PWM2 device on
Lenovo Yoga Book from always present table" patch. You requested
changing the commit message of this one a bit to make it sound
less like a regression fix (which it is not). But you already
have the previous version of this patch in your bleeding-edge
branch, with a "Cc: 5.1+ <stable@vger.kernel.org> # 5.1+"
added ?  So depending on which version you want you can either
skip this patch when applying this series, or replace it with
the version from this series.

Patches 2-4 are the main changes to make the always_present
quirk handling more flexible, changing it into a status_override
mechanism + adding a quirk for the GPD win and pocket to fix
an issue with those in a more elegant matter then the current
kludge in the sdhci-acpi code.

Patch 5 is an unrelated patch which touches the override-status
quirk table, so it needed to be rebased and I decided to add it
to this series to make it clear that its v2 needs to be applied
on top of the other ACPI changes from this series.

Patches 6+7 cleanup the sdhci-acpi code, removing the now no
longer needed ugly kludge for the GPD win/pocket. These can
be merged independently from patches 1-5, through the mmc
tree, as long as they get send to Linus during the same
kernel cycle as the ACPI bits.

Regards,

Hans


Hans de Goede (7):
  ACPI / x86: Drop PWM2 device on Lenovo Yoga Book from always present
    table
  ACPI: Change acpi_device_always_present() into
    acpi_device_override_status()
  ACPI / x86: Allow specifying acpi_device_override_status() quirks by
    path
  ACPI / x86: Add not-present quirk for the PCI0.SDHB.BRC1 device on the
    GPD win
  ACPI / x86: Add PWM2 on the Xiaomi Mi Pad 2 to the always_present list
  mmc: sdhci-acpi: Remove special handling for GPD win/pocket devices
  mmc: sdhci-acpi: Use the new soc_intel_is_byt() helper

 drivers/acpi/bus.c            |   4 +-
 drivers/acpi/x86/utils.c      | 122 +++++++++++++++++++++++-----------
 drivers/mmc/host/sdhci-acpi.c |  78 ++--------------------
 include/acpi/acpi_bus.h       |   5 +-
 4 files changed, 96 insertions(+), 113 deletions(-)

-- 
2.33.1

[PATCH v2 1/7] ACPI / x86: Drop PWM2 device on Lenovo Yoga Book from always present table

[Hans de Goede]

It turns out that there is a WMI object which controls the PWM2 device
used for the keyboard backlight and that WMI object also provides some
other useful functionality.

The upcoming lenovo-yogabook-wmi driver will offer both backlight
control and the other functionality, so there no longer is a need
to have the lpss-pwm driver binding to PWM2 for backlight control;
and this is now actually undesirable because this will cause both
the WMI code and the lpss-pwm driver to poke at the same PWM
controller.

Drop the always-present quirk for the PWM2 ACPI-device, so that the
 lpss-pwm controller will no longer bind to it.

Cc: Yauhen Kharuzhy <jekhor@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Change the wording so that this is not seen as something to
  backport to the stable-series (note backporting won't cause
  issues but it is not really necessary)
---
 drivers/acpi/x86/utils.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
index f22f23933063..3bcac98f6eca 100644
--- a/drivers/acpi/x86/utils.c
+++ b/drivers/acpi/x86/utils.c
@@ -54,10 +54,6 @@ static const struct always_present_id always_present_ids[] = {
 	ENTRY("80860F09", "1", X86_MATCH(ATOM_SILVERMONT), {}),
 	ENTRY("80862288", "1", X86_MATCH(ATOM_AIRMONT), {}),
 
-	/* Lenovo Yoga Book uses PWM2 for keyboard backlight control */
-	ENTRY("80862289", "2", X86_MATCH(ATOM_AIRMONT), {
-			DMI_MATCH(DMI_PRODUCT_NAME, "Lenovo YB1-X9"),
-		}),
 	/*
 	 * The INT0002 device is necessary to clear wakeup interrupt sources
 	 * on Cherry Trail devices, without it we get nobody cared IRQ msgs.
-- 
2.33.1

[PATCH v2 2/7] ACPI: Change acpi_device_always_present() into acpi_device_override_status()

[Hans de Goede]

ATM acpi_bus_get_status() calls acpi_device_always_present() to allow
platform quirks to override the _STA return to report that a device
is present (status = ACPI_STA_DEFAULT) independent of the _STA return.

In some cases it might also be useful to have the opposite functionality
and have a platform quirk which marks a device as not present (status = 0)
to work around ACPI table bugs.

Change acpi_device_always_present() into a more generic
acpi_device_override_status() function to allow this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- No changes in v2 of this patch-series
---
 drivers/acpi/bus.c       |  4 +--
 drivers/acpi/x86/utils.c | 64 +++++++++++++++++++++++-----------------
 include/acpi/acpi_bus.h  |  5 ++--
 3 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index fa923a929224..dd535b4b9a16 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -98,8 +98,8 @@ int acpi_bus_get_status(struct acpi_device *device)
 	acpi_status status;
 	unsigned long long sta;
 
-	if (acpi_device_always_present(device)) {
-		acpi_set_device_status(device, ACPI_STA_DEFAULT);
+	if (acpi_device_override_status(device, &sta)) {
+		acpi_set_device_status(device, sta);
 		return 0;
 	}
 
diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
index 3bcac98f6eca..edb4f3fd93dc 100644
--- a/drivers/acpi/x86/utils.c
+++ b/drivers/acpi/x86/utils.c
@@ -22,54 +22,63 @@
  * Some BIOS-es (temporarily) hide specific APCI devices to work around Windows
  * driver bugs. We use DMI matching to match known cases of this.
  *
- * We work around this by always reporting ACPI_STA_DEFAULT for these
- * devices. Note this MUST only be done for devices where this is safe.
+ * Likewise sometimes some not-actually present devices are sometimes
+ * reported as present, which may cause issues.
  *
- * This forcing of devices to be present is limited to specific CPU (SoC)
- * models both to avoid potentially causing trouble on other models and
- * because some HIDs are re-used on different SoCs for completely
- * different devices.
+ * We work around this by using the below quirk list to override the status
+ * reported by the _STA method with a fixed value (ACPI_STA_DEFAULT or 0).
+ * Note this MUST only be done for devices where this is safe.
+ *
+ * This status overriding is limited to specific CPU (SoC) models both to
+ * avoid potentially causing trouble on other models and because some HIDs
+ * are re-used on different SoCs for completely different devices.
  */
-struct always_present_id {
+struct override_status_id {
 	struct acpi_device_id hid[2];
 	struct x86_cpu_id cpu_ids[2];
 	struct dmi_system_id dmi_ids[2]; /* Optional */
 	const char *uid;
+	unsigned long long status;
 };
 
-#define X86_MATCH(model)	X86_MATCH_INTEL_FAM6_MODEL(model, NULL)
-
-#define ENTRY(hid, uid, cpu_models, dmi...) {				\
+#define ENTRY(status, hid, uid, cpu_model, dmi...) {			\
 	{ { hid, }, {} },						\
-	{ cpu_models, {} },						\
+	{ X86_MATCH_INTEL_FAM6_MODEL(cpu_model, NULL), {} },		\
 	{ { .matches = dmi }, {} },					\
 	uid,								\
+	status,								\
 }
 
-static const struct always_present_id always_present_ids[] = {
+#define PRESENT_ENTRY_HID(hid, uid, cpu_model, dmi...) \
+	ENTRY(ACPI_STA_DEFAULT, hid, uid, cpu_model, dmi)
+
+#define NOT_PRESENT_ENTRY_HID(hid, uid, cpu_model, dmi...) \
+	ENTRY(0, hid, uid, cpu_model, dmi)
+
+static const struct override_status_id override_status_ids[] = {
 	/*
 	 * Bay / Cherry Trail PWM directly poked by GPU driver in win10,
 	 * but Linux uses a separate PWM driver, harmless if not used.
 	 */
-	ENTRY("80860F09", "1", X86_MATCH(ATOM_SILVERMONT), {}),
-	ENTRY("80862288", "1", X86_MATCH(ATOM_AIRMONT), {}),
+	PRESENT_ENTRY_HID("80860F09", "1", ATOM_SILVERMONT, {}),
+	PRESENT_ENTRY_HID("80862288", "1", ATOM_AIRMONT, {}),
 
 	/*
 	 * The INT0002 device is necessary to clear wakeup interrupt sources
 	 * on Cherry Trail devices, without it we get nobody cared IRQ msgs.
 	 */
-	ENTRY("INT0002", "1", X86_MATCH(ATOM_AIRMONT), {}),
+	PRESENT_ENTRY_HID("INT0002", "1", ATOM_AIRMONT, {}),
 	/*
 	 * On the Dell Venue 11 Pro 7130 and 7139, the DSDT hides
 	 * the touchscreen ACPI device until a certain time
 	 * after _SB.PCI0.GFX0.LCD.LCD1._ON gets called has passed
 	 * *and* _STA has been called at least 3 times since.
 	 */
-	ENTRY("SYNA7500", "1", X86_MATCH(HASWELL_L), {
+	PRESENT_ENTRY_HID("SYNA7500", "1", HASWELL_L, {
 		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
 		DMI_MATCH(DMI_PRODUCT_NAME, "Venue 11 Pro 7130"),
 	      }),
-	ENTRY("SYNA7500", "1", X86_MATCH(HASWELL_L), {
+	PRESENT_ENTRY_HID("SYNA7500", "1", HASWELL_L, {
 		DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
 		DMI_MATCH(DMI_PRODUCT_NAME, "Venue 11 Pro 7139"),
 	      }),
@@ -85,19 +94,19 @@ static const struct always_present_id always_present_ids[] = {
 	 * was copy-pasted from the GPD win, so it has a disabled KIOX000A
 	 * node which we should not enable, thus we also check the BIOS date.
 	 */
-	ENTRY("KIOX000A", "1", X86_MATCH(ATOM_AIRMONT), {
+	PRESENT_ENTRY_HID("KIOX000A", "1", ATOM_AIRMONT, {
 		DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
 		DMI_MATCH(DMI_BOARD_NAME, "Default string"),
 		DMI_MATCH(DMI_PRODUCT_NAME, "Default string"),
 		DMI_MATCH(DMI_BIOS_DATE, "02/21/2017")
 	      }),
-	ENTRY("KIOX000A", "1", X86_MATCH(ATOM_AIRMONT), {
+	PRESENT_ENTRY_HID("KIOX000A", "1", ATOM_AIRMONT, {
 		DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
 		DMI_MATCH(DMI_BOARD_NAME, "Default string"),
 		DMI_MATCH(DMI_PRODUCT_NAME, "Default string"),
 		DMI_MATCH(DMI_BIOS_DATE, "03/20/2017")
 	      }),
-	ENTRY("KIOX000A", "1", X86_MATCH(ATOM_AIRMONT), {
+	PRESENT_ENTRY_HID("KIOX000A", "1", ATOM_AIRMONT, {
 		DMI_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
 		DMI_MATCH(DMI_BOARD_NAME, "Default string"),
 		DMI_MATCH(DMI_PRODUCT_NAME, "Default string"),
@@ -105,26 +114,27 @@ static const struct always_present_id always_present_ids[] = {
 	      }),
 };
 
-bool acpi_device_always_present(struct acpi_device *adev)
+bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *status)
 {
 	bool ret = false;
 	unsigned int i;
 
-	for (i = 0; i < ARRAY_SIZE(always_present_ids); i++) {
-		if (acpi_match_device_ids(adev, always_present_ids[i].hid))
+	for (i = 0; i < ARRAY_SIZE(override_status_ids); i++) {
+		if (acpi_match_device_ids(adev, override_status_ids[i].hid))
 			continue;
 
 		if (!adev->pnp.unique_id ||
-		    strcmp(adev->pnp.unique_id, always_present_ids[i].uid))
+		    strcmp(adev->pnp.unique_id, override_status_ids[i].uid))
 			continue;
 
-		if (!x86_match_cpu(always_present_ids[i].cpu_ids))
+		if (!x86_match_cpu(override_status_ids[i].cpu_ids))
 			continue;
 
-		if (always_present_ids[i].dmi_ids[0].matches[0].slot &&
-		    !dmi_check_system(always_present_ids[i].dmi_ids))
+		if (override_status_ids[i].dmi_ids[0].matches[0].slot &&
+		    !dmi_check_system(override_status_ids[i].dmi_ids))
 			continue;
 
+		*status = override_status_ids[i].status;
 		ret = true;
 		break;
 	}
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 480f9207a4c6..d6fe27b695c3 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -613,9 +613,10 @@ int acpi_enable_wakeup_device_power(struct acpi_device *dev, int state);
 int acpi_disable_wakeup_device_power(struct acpi_device *dev);
 
 #ifdef CONFIG_X86
-bool acpi_device_always_present(struct acpi_device *adev);
+bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *status);
 #else
-static inline bool acpi_device_always_present(struct acpi_device *adev)
+static inline bool acpi_device_override_status(struct acpi_device *adev,
+					       unsigned long long *status)
 {
 	return false;
 }
-- 
2.33.1

[PATCH v2 3/7] ACPI / x86: Allow specifying acpi_device_override_status() quirks by path

[Hans de Goede]

Not all ACPI-devices have a HID + UID, allow specifying quirks for
acpi_device_override_status() by path too.

Note this moves the path/HID+UID check to after the CPU + DMI checks
since the path lookup is somewhat costly.

This way this lookup is only done on devices where the other checks
match.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- No changes in v2 of this patch-series
---
 drivers/acpi/x86/utils.c | 42 ++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
index edb4f3fd93dc..190bfc2ab3f2 100644
--- a/drivers/acpi/x86/utils.c
+++ b/drivers/acpi/x86/utils.c
@@ -38,22 +38,30 @@ struct override_status_id {
 	struct x86_cpu_id cpu_ids[2];
 	struct dmi_system_id dmi_ids[2]; /* Optional */
 	const char *uid;
+	const char *path;
 	unsigned long long status;
 };
 
-#define ENTRY(status, hid, uid, cpu_model, dmi...) {			\
+#define ENTRY(status, hid, uid, path, cpu_model, dmi...) {		\
 	{ { hid, }, {} },						\
 	{ X86_MATCH_INTEL_FAM6_MODEL(cpu_model, NULL), {} },		\
 	{ { .matches = dmi }, {} },					\
 	uid,								\
+	path,								\
 	status,								\
 }
 
 #define PRESENT_ENTRY_HID(hid, uid, cpu_model, dmi...) \
-	ENTRY(ACPI_STA_DEFAULT, hid, uid, cpu_model, dmi)
+	ENTRY(ACPI_STA_DEFAULT, hid, uid, NULL, cpu_model, dmi)
 
 #define NOT_PRESENT_ENTRY_HID(hid, uid, cpu_model, dmi...) \
-	ENTRY(0, hid, uid, cpu_model, dmi)
+	ENTRY(0, hid, uid, NULL, cpu_model, dmi)
+
+#define PRESENT_ENTRY_PATH(path, cpu_model, dmi...) \
+	ENTRY(ACPI_STA_DEFAULT, "", NULL, path, cpu_model, dmi)
+
+#define NOT_PRESENT_ENTRY_PATH(path, cpu_model, dmi...) \
+	ENTRY(0, "", NULL, path, cpu_model, dmi)
 
 static const struct override_status_id override_status_ids[] = {
 	/*
@@ -120,13 +128,6 @@ bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *s
 	unsigned int i;
 
 	for (i = 0; i < ARRAY_SIZE(override_status_ids); i++) {
-		if (acpi_match_device_ids(adev, override_status_ids[i].hid))
-			continue;
-
-		if (!adev->pnp.unique_id ||
-		    strcmp(adev->pnp.unique_id, override_status_ids[i].uid))
-			continue;
-
 		if (!x86_match_cpu(override_status_ids[i].cpu_ids))
 			continue;
 
@@ -134,6 +135,27 @@ bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *s
 		    !dmi_check_system(override_status_ids[i].dmi_ids))
 			continue;
 
+		if (override_status_ids[i].path) {
+			struct acpi_buffer path = { ACPI_ALLOCATE_BUFFER, NULL };
+			bool match;
+
+			if (acpi_get_name(adev->handle, ACPI_FULL_PATHNAME, &path))
+				continue;
+
+			match = strcmp((char *)path.pointer, override_status_ids[i].path) == 0;
+			kfree(path.pointer);
+
+			if (!match)
+				continue;
+		} else {
+			if (acpi_match_device_ids(adev, override_status_ids[i].hid))
+				continue;
+
+			if (!adev->pnp.unique_id ||
+			    strcmp(adev->pnp.unique_id, override_status_ids[i].uid))
+				continue;
+		}
+
 		*status = override_status_ids[i].status;
 		ret = true;
 		break;
-- 
2.33.1

[PATCH v2 4/7] ACPI / x86: Add not-present quirk for the PCI0.SDHB.BRC1 device on the GPD win

[Hans de Goede]

The GPD win and its sibling the GPD pocket (99% the same electronics in a
different case) use a PCI wifi card. But the ACPI tables on both variants
contain a bug where the SDIO MMC controller for SDIO wifi cards is enabled
despite this. This SDIO MMC controller has a PCI0.SDHB.BRC1 child-device
which _PS3 method sets a GPIO causing the PCI wifi card to turn off.

At the moment there is a pretty ugly kludge in the sdhci-acpi.c code,
just to work around the bug in the DSDT of this single design. This can
be solved cleaner/simply with a quirk overriding the _STA return of the
broken PCI0.SDHB.BRC1 PCI0.SDHB.BRC1 child with a status value of 0,
so that its power_manageable flag gets cleared, avoiding this problem.

Note that even though it is not used, the _STA method for the MMC
controller is deliberately not overridden. If the status of the MMC
controller were forced to 0 it would never get suspended, which would
cause these mini-laptops to not reach S0i3 level when suspended.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Drop the Fixes: c10383e8ddf4 ("ACPI: scan: Release PM resources blocked by
  unused objects") tag and related wording in the commit message, since
  that patch has been reverted for now
---
 drivers/acpi/x86/utils.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
index 190bfc2ab3f2..b3fb428461c6 100644
--- a/drivers/acpi/x86/utils.c
+++ b/drivers/acpi/x86/utils.c
@@ -94,9 +94,10 @@ static const struct override_status_id override_status_ids[] = {
 	/*
 	 * The GPD win BIOS dated 20170221 has disabled the accelerometer, the
 	 * drivers sometimes cause crashes under Windows and this is how the
-	 * manufacturer has solved this :| Note that the the DMI data is less
-	 * generic then it seems, a board_vendor of "AMI Corporation" is quite
-	 * rare and a board_name of "Default String" also is rare.
+	 * manufacturer has solved this :|  The DMI match may not seem unique,
+	 * but it is. In the 67000+ DMI decode dumps from linux-hardware.org
+	 * only 116 have board_vendor set to "AMI Corporation" and of those 116
+	 * only the GPD win and pocket entries' board_name is "Default string".
 	 *
 	 * Unfortunately the GPD pocket also uses these strings and its BIOS
 	 * was copy-pasted from the GPD win, so it has a disabled KIOX000A
@@ -120,6 +121,19 @@ static const struct override_status_id override_status_ids[] = {
 		DMI_MATCH(DMI_PRODUCT_NAME, "Default string"),
 		DMI_MATCH(DMI_BIOS_DATE, "05/25/2017")
 	      }),
+
+	/*
+	 * The GPD win/pocket have a PCI wifi card, but its DSDT has the SDIO
+	 * mmc controller enabled and that has a child-device which _PS3
+	 * method sets a GPIO causing the PCI wifi card to turn off.
+	 * See above remark about uniqueness of the DMI match.
+	 */
+	NOT_PRESENT_ENTRY_PATH("\\_SB_.PCI0.SDHB.BRC1", ATOM_AIRMONT, {
+		DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
+		DMI_EXACT_MATCH(DMI_BOARD_NAME, "Default string"),
+		DMI_EXACT_MATCH(DMI_BOARD_SERIAL, "Default string"),
+		DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Default string"),
+	      }),
 };
 
 bool acpi_device_override_status(struct acpi_device *adev, unsigned long long *status)
-- 
2.33.1

[PATCH v2 5/7] ACPI / x86: Add PWM2 on the Xiaomi Mi Pad 2 to the always_present list

[Hans de Goede]

The Xiaomi Mi Pad 2 has backlit LEDs behind the capacitive menu, home
and back buttons below the screen which are controlled by the PWM2
controller of the CHT SoC. This PWM2 controller gets hidden by the
firmware, add it to the always_present_ids table so that we can use
the PWM controller to control the backlighting of the buttons.

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Rebase on top of other patches in this series
- Add to this series since it now relies on other patches from this series
---
 drivers/acpi/x86/utils.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
index b3fb428461c6..a2ae1ac41319 100644
--- a/drivers/acpi/x86/utils.c
+++ b/drivers/acpi/x86/utils.c
@@ -71,6 +71,12 @@ static const struct override_status_id override_status_ids[] = {
 	PRESENT_ENTRY_HID("80860F09", "1", ATOM_SILVERMONT, {}),
 	PRESENT_ENTRY_HID("80862288", "1", ATOM_AIRMONT, {}),
 
+	/* The Xiaomi Mi Pad 2 uses PWM2 for touchkeys backlight control */
+	PRESENT_ENTRY_HID("80862289", "2", ATOM_AIRMONT, {
+		DMI_MATCH(DMI_SYS_VENDOR, "Xiaomi Inc"),
+		DMI_MATCH(DMI_PRODUCT_NAME, "Mipad2"),
+	      }),
+
 	/*
 	 * The INT0002 device is necessary to clear wakeup interrupt sources
 	 * on Cherry Trail devices, without it we get nobody cared IRQ msgs.
-- 
2.33.1

[PATCH v2 6/7] mmc: sdhci-acpi: Remove special handling for GPD win/pocket devices

[Hans de Goede]

Remove the special sdhci_acpi_no_fixup_child_power() helper which was
added to avoid triggering an ACPI tables bug on the GPD win/pocket
devices.

The ACPI child-device triggering this bug has now been added to the
acpi_device_override_status() quirk table, so that its status
field is set to all 0 (instead of the wrong return value from the _STA
ACPI method). This removes the need for the special handling in
the sdhci-acpi code.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- No changes in v2 of this patch-series
---
 drivers/mmc/host/sdhci-acpi.c | 61 ++---------------------------------
 1 file changed, 3 insertions(+), 58 deletions(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index f1ef0d28b0dd..1461aae13c19 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -34,7 +34,6 @@
 #include <asm/cpu_device_id.h>
 #include <asm/intel-family.h>
 #include <asm/iosf_mbi.h>
-#include <linux/pci.h>
 #endif
 
 #include "sdhci.h"
@@ -250,16 +249,6 @@ static bool sdhci_acpi_byt(void)
 	return x86_match_cpu(byt);
 }
 
-static bool sdhci_acpi_cht(void)
-{
-	static const struct x86_cpu_id cht[] = {
-		X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT, NULL),
-		{}
-	};
-
-	return x86_match_cpu(cht);
-}
-
 #define BYT_IOSF_SCCEP			0x63
 #define BYT_IOSF_OCP_NETCTRL0		0x1078
 #define BYT_IOSF_OCP_TIMEOUT_BASE	GENMASK(10, 8)
@@ -304,43 +293,6 @@ static bool sdhci_acpi_byt_defer(struct device *dev)
 	return false;
 }
 
-static bool sdhci_acpi_cht_pci_wifi(unsigned int vendor, unsigned int device,
-				    unsigned int slot, unsigned int parent_slot)
-{
-	struct pci_dev *dev, *parent, *from = NULL;
-
-	while (1) {
-		dev = pci_get_device(vendor, device, from);
-		pci_dev_put(from);
-		if (!dev)
-			break;
-		parent = pci_upstream_bridge(dev);
-		if (ACPI_COMPANION(&dev->dev) && PCI_SLOT(dev->devfn) == slot &&
-		    parent && PCI_SLOT(parent->devfn) == parent_slot &&
-		    !pci_upstream_bridge(parent)) {
-			pci_dev_put(dev);
-			return true;
-		}
-		from = dev;
-	}
-
-	return false;
-}
-
-/*
- * GPDwin uses PCI wifi which conflicts with SDIO's use of
- * acpi_device_fix_up_power() on child device nodes. Identifying GPDwin is
- * problematic, but since SDIO is only used for wifi, the presence of the PCI
- * wifi card in the expected slot with an ACPI companion node, is used to
- * indicate that acpi_device_fix_up_power() should be avoided.
- */
-static inline bool sdhci_acpi_no_fixup_child_power(struct acpi_device *adev)
-{
-	return sdhci_acpi_cht() &&
-	       acpi_dev_hid_uid_match(adev, "80860F14", "2") &&
-	       sdhci_acpi_cht_pci_wifi(0x14e4, 0x43ec, 0, 28);
-}
-
 #else
 
 static inline void sdhci_acpi_byt_setting(struct device *dev)
@@ -352,11 +304,6 @@ static inline bool sdhci_acpi_byt_defer(struct device *dev)
 	return false;
 }
 
-static inline bool sdhci_acpi_no_fixup_child_power(struct acpi_device *adev)
-{
-	return false;
-}
-
 #endif
 
 static int bxt_get_cd(struct mmc_host *mmc)
@@ -861,11 +808,9 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 
 	/* Power on the SDHCI controller and its children */
 	acpi_device_fix_up_power(device);
-	if (!sdhci_acpi_no_fixup_child_power(device)) {
-		list_for_each_entry(child, &device->children, node)
-			if (child->status.present && child->status.enabled)
-				acpi_device_fix_up_power(child);
-	}
+	list_for_each_entry(child, &device->children, node)
+		if (child->status.present && child->status.enabled)
+			acpi_device_fix_up_power(child);
 
 	if (sdhci_acpi_byt_defer(dev))
 		return -EPROBE_DEFER;
-- 
2.33.1

Re: [PATCH v2 6/7] mmc: sdhci-acpi: Remove special handling for GPD win/pocket devices

[Adrian Hunter]

On 22/11/2021 19:05, Hans de Goede wrote:
> Remove the special sdhci_acpi_no_fixup_child_power() helper which was
> added to avoid triggering an ACPI tables bug on the GPD win/pocket
> devices.
> 
> The ACPI child-device triggering this bug has now been added to the
> acpi_device_override_status() quirk table, so that its status
> field is set to all 0 (instead of the wrong return value from the _STA
> ACPI method). This removes the need for the special handling in
> the sdhci-acpi code.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> Changes in v2:
> - No changes in v2 of this patch-series
> ---
>  drivers/mmc/host/sdhci-acpi.c | 61 ++---------------------------------
>  1 file changed, 3 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index f1ef0d28b0dd..1461aae13c19 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -34,7 +34,6 @@
>  #include <asm/cpu_device_id.h>
>  #include <asm/intel-family.h>
>  #include <asm/iosf_mbi.h>
> -#include <linux/pci.h>
>  #endif
>  
>  #include "sdhci.h"
> @@ -250,16 +249,6 @@ static bool sdhci_acpi_byt(void)
>  	return x86_match_cpu(byt);
>  }
>  
> -static bool sdhci_acpi_cht(void)
> -{
> -	static const struct x86_cpu_id cht[] = {
> -		X86_MATCH_INTEL_FAM6_MODEL(ATOM_AIRMONT, NULL),
> -		{}
> -	};
> -
> -	return x86_match_cpu(cht);
> -}
> -
>  #define BYT_IOSF_SCCEP			0x63
>  #define BYT_IOSF_OCP_NETCTRL0		0x1078
>  #define BYT_IOSF_OCP_TIMEOUT_BASE	GENMASK(10, 8)
> @@ -304,43 +293,6 @@ static bool sdhci_acpi_byt_defer(struct device *dev)
>  	return false;
>  }
>  
> -static bool sdhci_acpi_cht_pci_wifi(unsigned int vendor, unsigned int device,
> -				    unsigned int slot, unsigned int parent_slot)
> -{
> -	struct pci_dev *dev, *parent, *from = NULL;
> -
> -	while (1) {
> -		dev = pci_get_device(vendor, device, from);
> -		pci_dev_put(from);
> -		if (!dev)
> -			break;
> -		parent = pci_upstream_bridge(dev);
> -		if (ACPI_COMPANION(&dev->dev) && PCI_SLOT(dev->devfn) == slot &&
> -		    parent && PCI_SLOT(parent->devfn) == parent_slot &&
> -		    !pci_upstream_bridge(parent)) {
> -			pci_dev_put(dev);
> -			return true;
> -		}
> -		from = dev;
> -	}
> -
> -	return false;
> -}
> -
> -/*
> - * GPDwin uses PCI wifi which conflicts with SDIO's use of
> - * acpi_device_fix_up_power() on child device nodes. Identifying GPDwin is
> - * problematic, but since SDIO is only used for wifi, the presence of the PCI
> - * wifi card in the expected slot with an ACPI companion node, is used to
> - * indicate that acpi_device_fix_up_power() should be avoided.
> - */
> -static inline bool sdhci_acpi_no_fixup_child_power(struct acpi_device *adev)
> -{
> -	return sdhci_acpi_cht() &&
> -	       acpi_dev_hid_uid_match(adev, "80860F14", "2") &&
> -	       sdhci_acpi_cht_pci_wifi(0x14e4, 0x43ec, 0, 28);
> -}
> -
>  #else
>  
>  static inline void sdhci_acpi_byt_setting(struct device *dev)
> @@ -352,11 +304,6 @@ static inline bool sdhci_acpi_byt_defer(struct device *dev)
>  	return false;
>  }
>  
> -static inline bool sdhci_acpi_no_fixup_child_power(struct acpi_device *adev)
> -{
> -	return false;
> -}
> -
>  #endif
>  
>  static int bxt_get_cd(struct mmc_host *mmc)
> @@ -861,11 +808,9 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
>  
>  	/* Power on the SDHCI controller and its children */
>  	acpi_device_fix_up_power(device);
> -	if (!sdhci_acpi_no_fixup_child_power(device)) {
> -		list_for_each_entry(child, &device->children, node)
> -			if (child->status.present && child->status.enabled)
> -				acpi_device_fix_up_power(child);
> -	}
> +	list_for_each_entry(child, &device->children, node)
> +		if (child->status.present && child->status.enabled)
> +			acpi_device_fix_up_power(child);
>  
>  	if (sdhci_acpi_byt_defer(dev))
>  		return -EPROBE_DEFER;
> 

[PATCH v2 7/7] mmc: sdhci-acpi: Use the new soc_intel_is_byt() helper

[Hans de Goede]

Use the new soc_intel_is_byt() helper function from
include/linux/platform_data/x86/soc.h .

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- No changes in v2 of this patch-series
---
 drivers/mmc/host/sdhci-acpi.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 1461aae13c19..c0350e9c03f3 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -31,8 +31,7 @@
 #include <linux/mmc/slot-gpio.h>
 
 #ifdef CONFIG_X86
-#include <asm/cpu_device_id.h>
-#include <asm/intel-family.h>
+#include <linux/platform_data/x86/soc.h>
 #include <asm/iosf_mbi.h>
 #endif
 
@@ -239,16 +238,6 @@ static const struct sdhci_acpi_chip sdhci_acpi_chip_int = {
 
 #ifdef CONFIG_X86
 
-static bool sdhci_acpi_byt(void)
-{
-	static const struct x86_cpu_id byt[] = {
-		X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT, NULL),
-		{}
-	};
-
-	return x86_match_cpu(byt);
-}
-
 #define BYT_IOSF_SCCEP			0x63
 #define BYT_IOSF_OCP_NETCTRL0		0x1078
 #define BYT_IOSF_OCP_TIMEOUT_BASE	GENMASK(10, 8)
@@ -257,7 +246,7 @@ static void sdhci_acpi_byt_setting(struct device *dev)
 {
 	u32 val = 0;
 
-	if (!sdhci_acpi_byt())
+	if (!soc_intel_is_byt())
 		return;
 
 	if (iosf_mbi_read(BYT_IOSF_SCCEP, MBI_CR_READ, BYT_IOSF_OCP_NETCTRL0,
@@ -282,7 +271,7 @@ static void sdhci_acpi_byt_setting(struct device *dev)
 
 static bool sdhci_acpi_byt_defer(struct device *dev)
 {
-	if (!sdhci_acpi_byt())
+	if (!soc_intel_is_byt())
 		return false;
 
 	if (!iosf_mbi_available())
-- 
2.33.1

Re: [PATCH v2 7/7] mmc: sdhci-acpi: Use the new soc_intel_is_byt() helper

[Adrian Hunter]

On 22/11/2021 19:05, Hans de Goede wrote:
> Use the new soc_intel_is_byt() helper function from
> include/linux/platform_data/x86/soc.h .
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> Changes in v2:
> - No changes in v2 of this patch-series
> ---
>  drivers/mmc/host/sdhci-acpi.c | 17 +++--------------
>  1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index 1461aae13c19..c0350e9c03f3 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -31,8 +31,7 @@
>  #include <linux/mmc/slot-gpio.h>
>  
>  #ifdef CONFIG_X86
> -#include <asm/cpu_device_id.h>
> -#include <asm/intel-family.h>
> +#include <linux/platform_data/x86/soc.h>
>  #include <asm/iosf_mbi.h>
>  #endif
>  
> @@ -239,16 +238,6 @@ static const struct sdhci_acpi_chip sdhci_acpi_chip_int = {
>  
>  #ifdef CONFIG_X86
>  
> -static bool sdhci_acpi_byt(void)
> -{
> -	static const struct x86_cpu_id byt[] = {
> -		X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT, NULL),
> -		{}
> -	};
> -
> -	return x86_match_cpu(byt);
> -}
> -
>  #define BYT_IOSF_SCCEP			0x63
>  #define BYT_IOSF_OCP_NETCTRL0		0x1078
>  #define BYT_IOSF_OCP_TIMEOUT_BASE	GENMASK(10, 8)
> @@ -257,7 +246,7 @@ static void sdhci_acpi_byt_setting(struct device *dev)
>  {
>  	u32 val = 0;
>  
> -	if (!sdhci_acpi_byt())
> +	if (!soc_intel_is_byt())
>  		return;
>  
>  	if (iosf_mbi_read(BYT_IOSF_SCCEP, MBI_CR_READ, BYT_IOSF_OCP_NETCTRL0,
> @@ -282,7 +271,7 @@ static void sdhci_acpi_byt_setting(struct device *dev)
>  
>  static bool sdhci_acpi_byt_defer(struct device *dev)
>  {
> -	if (!sdhci_acpi_byt())
> +	if (!soc_intel_is_byt())
>  		return false;
>  
>  	if (!iosf_mbi_available())
> 

Re: [PATCH v2 0/7] ACPI: acpi_device_override_status() changes

[Ulf Hansson]

On Mon, 22 Nov 2021 at 18:05, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Rafael,
>
> As requested here is a v2 of my series previously titled:
> "ACPI: scan: Skip turning off some unused objects during scan"
>
> Which was a regression fix series for the commit c10383e8ddf4
> ("ACPI: scan: Release PM resources blocked by unused objects")
> change, but that has been reverted now. So as requested here is
> a v2 changing the wording of various commit messages since these
> changes are still useful to have regardless.
>
> Patch 1/7 is a v2/resend of the "ACPI / x86: Drop PWM2 device on
> Lenovo Yoga Book from always present table" patch. You requested
> changing the commit message of this one a bit to make it sound
> less like a regression fix (which it is not). But you already
> have the previous version of this patch in your bleeding-edge
> branch, with a "Cc: 5.1+ <stable@vger.kernel.org> # 5.1+"
> added ?  So depending on which version you want you can either
> skip this patch when applying this series, or replace it with
> the version from this series.
>
> Patches 2-4 are the main changes to make the always_present
> quirk handling more flexible, changing it into a status_override
> mechanism + adding a quirk for the GPD win and pocket to fix
> an issue with those in a more elegant matter then the current
> kludge in the sdhci-acpi code.
>
> Patch 5 is an unrelated patch which touches the override-status
> quirk table, so it needed to be rebased and I decided to add it
> to this series to make it clear that its v2 needs to be applied
> on top of the other ACPI changes from this series.
>
> Patches 6+7 cleanup the sdhci-acpi code, removing the now no
> longer needed ugly kludge for the GPD win/pocket. These can
> be merged independently from patches 1-5, through the mmc
> tree, as long as they get send to Linus during the same
> kernel cycle as the ACPI bits.

This sounds like the mmc changes are really not that independent after
all. What about bisectability?

An option is to funnel the sdhci patches together with the ACPI
patches through Rafael's tree. You have my ack for this, but let's
wait for Adrian's ack too.

[...]

Kind regards
Uffe

Re: [PATCH v2 0/7] ACPI: acpi_device_override_status() changes

[Hans de Goede]

Hi,

On 11/23/21 12:13, Ulf Hansson wrote:
> On Mon, 22 Nov 2021 at 18:05, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Rafael,
>>
>> As requested here is a v2 of my series previously titled:
>> "ACPI: scan: Skip turning off some unused objects during scan"
>>
>> Which was a regression fix series for the commit c10383e8ddf4
>> ("ACPI: scan: Release PM resources blocked by unused objects")
>> change, but that has been reverted now. So as requested here is
>> a v2 changing the wording of various commit messages since these
>> changes are still useful to have regardless.
>>
>> Patch 1/7 is a v2/resend of the "ACPI / x86: Drop PWM2 device on
>> Lenovo Yoga Book from always present table" patch. You requested
>> changing the commit message of this one a bit to make it sound
>> less like a regression fix (which it is not). But you already
>> have the previous version of this patch in your bleeding-edge
>> branch, with a "Cc: 5.1+ <stable@vger.kernel.org> # 5.1+"
>> added ?  So depending on which version you want you can either
>> skip this patch when applying this series, or replace it with
>> the version from this series.
>>
>> Patches 2-4 are the main changes to make the always_present
>> quirk handling more flexible, changing it into a status_override
>> mechanism + adding a quirk for the GPD win and pocket to fix
>> an issue with those in a more elegant matter then the current
>> kludge in the sdhci-acpi code.
>>
>> Patch 5 is an unrelated patch which touches the override-status
>> quirk table, so it needed to be rebased and I decided to add it
>> to this series to make it clear that its v2 needs to be applied
>> on top of the other ACPI changes from this series.
>>
>> Patches 6+7 cleanup the sdhci-acpi code, removing the now no
>> longer needed ugly kludge for the GPD win/pocket. These can
>> be merged independently from patches 1-5, through the mmc
>> tree, as long as they get send to Linus during the same
>> kernel cycle as the ACPI bits.
> 
> This sounds like the mmc changes are really not that independent after
> all. What about bisectability?

Merging the ACPI and mmc bits separately does indeed have a 50% chance
of causing an issue where during a bisect the wifi might stop working
on the GPD win / pocket. But only on those 2 models, so it won't be
a general bisect break; and it will only break wifi, without causing
other side-effects.

So I believe this really is mostly a theoretical issue. With that
said merging the entire set to one tree of course is fine too,
maybe even better because it keeps the related ACPI and sdhci
commit close together in the history.

> An option is to funnel the sdhci patches together with the ACPI
> patches through Rafael's tree. You have my ack for this, but let's
> wait for Adrian's ack too.

Thanks.

Regards,

Hans

Re: [PATCH v2 0/7] ACPI: acpi_device_override_status() changes

[Adrian Hunter]

On 23/11/2021 13:13, Ulf Hansson wrote:
> On Mon, 22 Nov 2021 at 18:05, Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Rafael,
>>
>> As requested here is a v2 of my series previously titled:
>> "ACPI: scan: Skip turning off some unused objects during scan"
>>
>> Which was a regression fix series for the commit c10383e8ddf4
>> ("ACPI: scan: Release PM resources blocked by unused objects")
>> change, but that has been reverted now. So as requested here is
>> a v2 changing the wording of various commit messages since these
>> changes are still useful to have regardless.
>>
>> Patch 1/7 is a v2/resend of the "ACPI / x86: Drop PWM2 device on
>> Lenovo Yoga Book from always present table" patch. You requested
>> changing the commit message of this one a bit to make it sound
>> less like a regression fix (which it is not). But you already
>> have the previous version of this patch in your bleeding-edge
>> branch, with a "Cc: 5.1+ <stable@vger.kernel.org> # 5.1+"
>> added ?  So depending on which version you want you can either
>> skip this patch when applying this series, or replace it with
>> the version from this series.
>>
>> Patches 2-4 are the main changes to make the always_present
>> quirk handling more flexible, changing it into a status_override
>> mechanism + adding a quirk for the GPD win and pocket to fix
>> an issue with those in a more elegant matter then the current
>> kludge in the sdhci-acpi code.
>>
>> Patch 5 is an unrelated patch which touches the override-status
>> quirk table, so it needed to be rebased and I decided to add it
>> to this series to make it clear that its v2 needs to be applied
>> on top of the other ACPI changes from this series.
>>
>> Patches 6+7 cleanup the sdhci-acpi code, removing the now no
>> longer needed ugly kludge for the GPD win/pocket. These can
>> be merged independently from patches 1-5, through the mmc
>> tree, as long as they get send to Linus during the same
>> kernel cycle as the ACPI bits.
> 
> This sounds like the mmc changes are really not that independent after
> all. What about bisectability?
> 
> An option is to funnel the sdhci patches together with the ACPI
> patches through Rafael's tree. You have my ack for this, but let's
> wait for Adrian's ack too.

Looks OK to me.

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Re: [PATCH v2 0/7] ACPI: acpi_device_override_status() changes

[Rafael J. Wysocki]

On Thu, Nov 25, 2021 at 10:53 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 23/11/2021 13:13, Ulf Hansson wrote:
> > On Mon, 22 Nov 2021 at 18:05, Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi Rafael,
> >>
> >> As requested here is a v2 of my series previously titled:
> >> "ACPI: scan: Skip turning off some unused objects during scan"
> >>
> >> Which was a regression fix series for the commit c10383e8ddf4
> >> ("ACPI: scan: Release PM resources blocked by unused objects")
> >> change, but that has been reverted now. So as requested here is
> >> a v2 changing the wording of various commit messages since these
> >> changes are still useful to have regardless.
> >>
> >> Patch 1/7 is a v2/resend of the "ACPI / x86: Drop PWM2 device on
> >> Lenovo Yoga Book from always present table" patch. You requested
> >> changing the commit message of this one a bit to make it sound
> >> less like a regression fix (which it is not). But you already
> >> have the previous version of this patch in your bleeding-edge
> >> branch, with a "Cc: 5.1+ <stable@vger.kernel.org> # 5.1+"
> >> added ?  So depending on which version you want you can either
> >> skip this patch when applying this series, or replace it with
> >> the version from this series.
> >>
> >> Patches 2-4 are the main changes to make the always_present
> >> quirk handling more flexible, changing it into a status_override
> >> mechanism + adding a quirk for the GPD win and pocket to fix
> >> an issue with those in a more elegant matter then the current
> >> kludge in the sdhci-acpi code.
> >>
> >> Patch 5 is an unrelated patch which touches the override-status
> >> quirk table, so it needed to be rebased and I decided to add it
> >> to this series to make it clear that its v2 needs to be applied
> >> on top of the other ACPI changes from this series.
> >>
> >> Patches 6+7 cleanup the sdhci-acpi code, removing the now no
> >> longer needed ugly kludge for the GPD win/pocket. These can
> >> be merged independently from patches 1-5, through the mmc
> >> tree, as long as they get send to Linus during the same
> >> kernel cycle as the ACPI bits.
> >
> > This sounds like the mmc changes are really not that independent after
> > all. What about bisectability?
> >
> > An option is to funnel the sdhci patches together with the ACPI
> > patches through Rafael's tree. You have my ack for this, but let's
> > wait for Adrian's ack too.
>
> Looks OK to me.
>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

All patches in the series applied as 5.17 material, thanks!