[PATCH v2 2/2] platform/surface: surfacepro3_button: don't load on amd variant

["Antonio Vázquez Blanco" <antoniovazquezblanco@gmail.com>]

From: Sachi King <nakato@nakato.io>

surfacepro3_button binds to MSHW0040 on the AMD variants of the Surface
Laptop 3 and 4, where soc_button_array is the right driver.

surface_button_check_MSHW0040() evaluates function 2 of the MSHW0040
_DSM, the OEM Platform Revision, and binds when the result is 0,
assuming that only a failed evaluation yields 0. The AMD variants
implement that function and legitimately return 0, so this driver binds
there as well.

Decide instead on whether the device implements the function at all,
which function 0 of the same _DSM advertises and acpi_check_dsm()
reports. Of the 32 Surface models carrying MSHW0040 in the ACPI dumps at
[1], only two do not advertise it, and they are exactly the two this
driver is for: the Surface Pro 4, whose MSHW0040 device carries no _DSM,
and the Surface Book 1, whose _DSM advertises functions 0 and 1 only.

The driver currently reaches the right answer on the Surface Book 1 by
accident rather than by that test. Function 2 is not implemented there,
so the _DSM falls through to a trailing Return (Buffer), and
acpi_evaluate_dsm_typed() rejects the type and returns NULL rather than
an integer. The revision is left at 0 and the driver binds.

Keep evaluating the revision when it is advertised, so that it remains
available through dev_dbg().

[1] https://github.com/linux-surface/acpidumps

Fixes: 64dd243d7356 ("platform/x86: surfacepro3_button: Fix device check")
Link: https://lore.kernel.org/all/20211109081125.41410-1-nakato@nakato.io/
Co-developed-by: Maximilian Luz <luzmaximilian@gmail.com>
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Signed-off-by: Sachi King <nakato@nakato.io>
Co-developed-by: Antonio Vázquez Blanco <antoniovazquezblanco@gmail.com>
Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco@gmail.com>
---
 drivers/platform/surface/surfacepro3_button.c | 31 ++++++++++---------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/surface/surfacepro3_button.c b/drivers/platform/surface/surfacepro3_button.c
index b38aa010053e..2d95a8ef3e23 100644
--- a/drivers/platform/surface/surfacepro3_button.c
+++ b/drivers/platform/surface/surfacepro3_button.c
@@ -148,7 +148,14 @@ static int surface_button_resume(struct device *dev)
 /*
  * Surface Pro 4 and Surface Book 2 / Surface Pro 2017 use the same device
  * ID (MSHW0040) for the power/volume buttons. Make sure this is the right
- * device by checking for the _DSM method and OEM Platform Revision.
+ * device by checking whether the OEM Platform Revision _DSM function is
+ * implemented, which function 0 of that _DSM advertises.
+ *
+ * The returned revision cannot be used for this: 0 is a valid revision, and
+ * the AMD variants of the Surface Laptop 3 and 4 report it while needing
+ * soc_button_array instead. The devices handled here do not advertise the
+ * function at all: the Surface Pro 4 has no _DSM on this device, and the
+ * Surface Book 1 has one that advertises functions 0 and 1 only.
  *
  * Returns true if the driver should bind to this device, i.e. the device is
  * either MSWH0028 (Pro 3) or MSHW0040 on a Pro 4 or Book 1.
@@ -156,21 +163,17 @@ static int surface_button_resume(struct device *dev)
 static bool surface_button_check_MSHW0040(struct device *dev, acpi_handle handle)
 {
 	union acpi_object *result;
-	u64 oem_platform_rev = 0;	// valid revisions are nonzero
+	u64 oem_platform_rev = 0;
+
+	if (!acpi_check_dsm(handle, &MSHW0040_DSM_UUID, MSHW0040_DSM_REVISION,
+			    BIT(MSHW0040_DSM_GET_OMPR)))
+		return true;
 
-	// get OEM platform revision
+	/* Fetch the revision separately, it is only used for debugging. */
 	result = acpi_evaluate_dsm_typed(handle, &MSHW0040_DSM_UUID,
 					 MSHW0040_DSM_REVISION,
-					 MSHW0040_DSM_GET_OMPR,
-					 NULL, ACPI_TYPE_INTEGER);
-
-	/*
-	 * If evaluating the _DSM fails, the method is not present. This means
-	 * that we have either MSHW0028 or MSHW0040 on Pro 4 or Book 1, so we
-	 * should use this driver. We use revision 0 indicating it is
-	 * unavailable.
-	 */
-
+					 MSHW0040_DSM_GET_OMPR, NULL,
+					 ACPI_TYPE_INTEGER);
 	if (result) {
 		oem_platform_rev = result->integer.value;
 		ACPI_FREE(result);
@@ -178,7 +181,7 @@ static bool surface_button_check_MSHW0040(struct device *dev, acpi_handle handle
 
 	dev_dbg(dev, "OEM Platform Revision %llu\n", oem_platform_rev);
 
-	return oem_platform_rev == 0;
+	return false;
 }
 
 
-- 
2.56.0