[PATCH v2 1/2] Input: soc_button_array - support AMD variant Surface devices

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

From: Sachi King <nakato@nakato.io>

The power button on the AMD variants of the Surface Laptop 3 and 4 uses
the same MSHW0040 device ID as the 5th and later generations of Surface
devices, so soc_button_array matches it. The device check then rejects
it, and surfacepro3_button binds instead, which is the wrong driver.

soc_device_check_MSHW0040() evaluates function 2 of the MSHW0040 _DSM,
the OEM Platform Revision, and treats a returned 0 as proof that the
_DSM is not there. That is wrong: 0 is a valid revision. The AMD
variants implement the function and return 0, so the check rejects
exactly the devices it is meant to accept.

What separates the two groups is whether the device implements that
function at all, which function 0 of the same _DSM advertises. Of the 32
Surface models carrying MSHW0040 in the ACPI dumps at [1]:

  fn 0 mask   fn 2 advertised   models
  no _DSM     no                Surface Pro 4
  0x03        no                Surface Book 1
  0x05        yes               Surface Laptop 1, 2
  0x0d        yes               Surface Book 2, Laptop SE, Studio 2
  0x0f        yes               the remaining 25

Only two models do not advertise it, and they are exactly the two that
need surfacepro3_button: the Surface Pro 4, whose MSHW0040 device
carries no _DSM at all, and the Surface Book 1, whose _DSM advertises
functions 0 and 1 only, function 1 there being dGPU base attach
handling.

Use acpi_check_dsm() to make the decision, and keep evaluating the
revision afterwards so that it remains available through dev_dbg().

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

Fixes: c394159310d0 ("Input: soc_button_array - add support for newer surface devices")
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/input/misc/soc_button_array.c | 31 +++++++++++++++------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index dbf86ea778e6..75946e18cc80 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -546,9 +546,17 @@ static const struct soc_device_data soc_device_MSHW0028 = {
  * Special device check for Surface Book 2 and Surface Pro (2017).
  * Both, the Surface Pro 4 (surfacepro3_button.c) and the above mentioned
  * devices use MSHW0040 for power and volume buttons, however the way they
- * have to be addressed differs. Make sure that we only load this drivers
- * for the correct devices by checking the OEM Platform Revision provided by
- * the _DSM method.
+ * have to be addressed differs. Make sure that we only load this driver for
+ * the correct devices.
+ *
+ * The OEM Platform Revision _DSM function tells them apart, but its return
+ * value cannot: it is a revision number and 0 is a valid one, which the AMD
+ * variants of the Surface Laptop 3 and 4 report while still needing this
+ * driver. Check whether the function is implemented at all instead, which
+ * function 0 of the same _DSM advertises. The devices that need
+ * surfacepro3_button.c do not advertise it: the Surface Pro 4 has no _DSM on
+ * this device, and the Surface Book 1 has one that advertises functions 0
+ * and 1 only.
  */
 #define MSHW0040_DSM_REVISION		0x01
 #define MSHW0040_DSM_GET_OMPR		0x02	// get OEM Platform Revision
@@ -560,27 +568,22 @@ static int soc_device_check_MSHW0040(struct device *dev)
 {
 	acpi_handle handle = ACPI_HANDLE(dev);
 	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 -ENODEV;
 
-	// 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 (result) {
 		oem_platform_rev = result->integer.value;
 		ACPI_FREE(result);
 	}
 
-	/*
-	 * If the revision is zero here, the _DSM evaluation has failed. This
-	 * indicates that we have a Pro 4 or Book 1 and this driver should not
-	 * be used.
-	 */
-	if (oem_platform_rev == 0)
-		return -ENODEV;
-
 	dev_dbg(dev, "OEM Platform Revision %llu\n", oem_platform_rev);
 
 	return 0;
-- 
2.56.0