[PATCH] Input: soc_button_array - fix MS Surface Pro 11 probe failure

[PATCH] Input: soc_button_array - fix MS Surface Pro 11 probe failure

[Hans de Goede]

On the MS Surface Pro 11 soc_button_array probing races with the GPIO
driver probing. If soc_button_array wins the race then gpiod_get() returns
EPROBE_DEFER, which should normally take care of retrying later, but
the soc_button_array code deliberately ignores EPROBE_DEFER causing it
to fail its probe() which causes the volume and power buttons to now work.

The ignoring of EPROBE_DEFER is there to deal with a problem specific to
older Bay Trail (BYT) and Cherry Trail (CHT) tablets which often use this
driver. Modify the error handling to only ignore EPROBE_DEFER on BYT and
CHT platforms and propagate EPROBE_DEFER normally on other platforms.

Reported-by: Sergey Lebedev <lsa.uz@pm.me>
Closes: https://lore.kernel.org/lkml/20260830141355.55898-1-lsa.uz@pm.me/
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 drivers/input/misc/soc_button_array.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index b8cad415c62c..6ed32c622d53 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -149,6 +149,16 @@ static int soc_button_lookup_gpio(struct device *dev, int acpi_index,
 	return 0;
 }
 
+#ifdef CONFIG_X86
+#include <linux/platform_data/x86/soc.h>
+static bool soc_intel_is_byt_or_cht(void)
+{
+	return soc_intel_is_byt() || soc_intel_is_cht();
+}
+#else
+static bool soc_intel_is_byt_or_cht(void) { return false; }
+#endif
+
 static struct platform_device *
 soc_button_device_create(struct platform_device *pdev,
 			 const struct soc_button_info *button_info,
@@ -191,8 +201,9 @@ soc_button_device_create(struct platform_device *pdev,
 		error = soc_button_lookup_gpio(&pdev->dev, info->acpi_index, &gpio, &irq);
 		if (error || irq < 0) {
 			/*
-			 * Skip GPIO if not present. Note we deliberately
-			 * ignore -EPROBE_DEFER errors here. On some devices
+			 * Propagate -EPROBE_DEFER, skip button on other errors.
+			 *
+			 * -EPROBE_DEFER is ignored on Bay & Cherry Trail. Here
 			 * Intel is using so called virtual GPIOs which are not
 			 * GPIOs at all but some way for AML code to check some
 			 * random status bits without need a custom opregion.
@@ -201,6 +212,9 @@ soc_button_device_create(struct platform_device *pdev,
 			 * we do not have a driver for these so they will never
 			 * show up, therefore we ignore -EPROBE_DEFER.
 			 */
+			if (error == -EPROBE_DEFER && !soc_intel_is_byt_or_cht())
+				goto err_free_mem;
+
 			continue;
 		}
 
-- 
2.55.0

Re: [PATCH] Input: soc_button_array - fix MS Surface Pro 11 probe failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Failure to propagate -EPROBE_DEFER when gpiod_to_irq() defers probe.
- [Low] Redundant #ifdef wrapping and misplaced #include directive in the middle of a .c file.
--

commit 96cf5f0eb3e99312a52ef035fbd723de02a88be5
Author: Hans de Goede <johannes.goede@oss.qualcomm.com>

Input: soc_button_array - fix MS Surface Pro 11 probe failure

This patch modifies error handling in soc_button_array to propagate 
-EPROBE_DEFER on non-BYT/CHT platforms. This resolves a probe race with the 
GPIO driver on the MS Surface Pro 11.

Closes: https://lore.kernel.org/lkml/20260830141355.55898-1-lsa.uz@pm.me/

> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> index b8cad415c62ca..6ed32c622d530 100644
> --- a/drivers/input/misc/soc_button_array.c
> +++ b/drivers/input/misc/soc_button_array.c
> @@ -149,6 +149,16 @@ static int soc_button_lookup_gpio(struct device *dev, int acpi_index,
>  	return 0;
>  }
>  
> +#ifdef CONFIG_X86
> +#include <linux/platform_data/x86/soc.h>
> +static bool soc_intel_is_byt_or_cht(void)
> +{
> +	return soc_intel_is_byt() || soc_intel_is_cht();
> +}
> +#else
> +static bool soc_intel_is_byt_or_cht(void) { return false; }
> +#endif

[Severity: Low]
This isn't a bug, but is this #ifdef CONFIG_X86 block and mid-file include
necessary?

The <linux/platform_data/x86/soc.h> header already contains
#if IS_ENABLED(CONFIG_X86) internally and provides fallback stubs for other
architectures, so the wrapping appears redundant. Placing #include directives
in the middle of a .c file is also generally discouraged.

[ ... ]

> @@ -201,6 +212,9 @@ soc_button_device_create(struct platform_device *pdev,
>  			 * we do not have a driver for these so they will never
>  			 * show up, therefore we ignore -EPROBE_DEFER.
>  			 */
> +			if (error == -EPROBE_DEFER && !soc_intel_is_byt_or_cht())
> +				goto err_free_mem;
> +

[Severity: Medium]
Does this check properly handle a deferred probe from the irqchip?

When soc_button_lookup_gpio() is called, gpiod_get_index() might succeed
(leaving error = 0), but gpiod_to_irq() can return -EPROBE_DEFER if the
irqchip is not yet initialized.

In this scenario, irq will be -517 (-EPROBE_DEFER), but error remains 0.
Since this condition only checks if error == -EPROBE_DEFER, the check
evaluates to false, the code falls through to continue, and the button
is silently skipped instead of propagating the deferral as intended.

>  			continue;
>  		}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908214410.70422-1-johannes.goede@oss.qualcomm.com?part=1