Re: [PATCH] Input: soc_button_array - fix MS Surface Pro 11 probe failure
From: sashiko-bot@kernel.org
Date: 2026-09-08 21:54:25
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 [off-list ref] 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/ (local)
quoted hunk ↗ jump to hunk
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. [ ... ]
quoted hunk ↗ jump to hunk
@@ -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