Re: [PATCH 3/3] Input: soc_button_array - convert to software nodes

[Bartosz Golaszewski]

On Wed, 21 Oct 2026 03:50:01 +0200, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> said:
> Convert the driver from using legacy platform data to using software
> nodes for registering 'gpio-keys' devices. This allows us to
> transition away from platform data and use generic device properties.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> ---
>  drivers/input/misc/Kconfig            |   1 -
>  drivers/input/misc/soc_button_array.c | 134 ++++++++++++++++++--------
>  2 files changed, 92 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 169768e3d5bb..a2984c1048f3 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -906,7 +906,6 @@ config INPUT_SOC_BUTTON_ARRAY
>  	depends on KEYBOARD_GPIO && ACPI
>  	depends on X86
>  	depends on GPIOLIB
> -	select GPIOLIB_LEGACY
>  	help
>  	  Say Y here if you have a SoC-based tablet that originally runs
>  	  Windows 8 or a Microsoft Surface Book 2, Pro 5, Laptop 1 or later.
> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> index 8d782ef4f78b..aa7838b864c2 100644
> --- a/drivers/input/misc/soc_button_array.c
> +++ b/drivers/input/misc/soc_button_array.c
> @@ -14,9 +14,11 @@
>  #include <linux/acpi.h>
>  #include <linux/dmi.h>
>  #include <linux/gpio/consumer.h>
> -#include <linux/gpio_keys.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/gpio/machine.h>
>  #include <linux/platform_data/x86/soc.h>
>  #include <linux/platform_device.h>
> +#include <linux/property.h>
>  #include <linux/slab.h>
>
>  static bool use_low_level_irq;
> @@ -59,10 +61,10 @@ struct soc_button_data {
>   * gets called with the lid open (e.g. on boot and on resume), it sets the
>   * irq-type to IRQ_TYPE_LEVEL_LOW. Where as the gpio-keys driver programs the
>   * type to, and expects it to be, IRQ_TYPE_EDGE_BOTH.
> - * To work around this we don't set gpio_keys_button.gpio on these 2-in-1s,
> + * To work around this we don't set "gpios" property on these 2-in-1s,
>   * instead we get the irq for the GPIO ourselves, configure it as
>   * IRQ_TYPE_LEVEL_LOW (to match how the _LID AML code configures it) and pass
> - * the irq in gpio_keys_button.irq. Below is a list of affected devices.
> + * the irq as a device resource. Below is a list of affected devices.
>   */
>  static const struct dmi_system_id dmi_use_low_level_irq[] = {
>  	{
> @@ -131,18 +133,31 @@ static const struct dmi_system_id dmi_invalid_acpi_index[] = {
>  };
>
>  /*
> - * Get the Nth GPIO number from the ACPI object.
> + * Get the Nth GPIO information from the ACPI object.
>   */
>  static int soc_button_lookup_gpio(struct device *dev, int acpi_index,
> -				  int *gpio_ret, int *irq_ret)
> +				  bool active_low,
> +				  struct software_node_ref_args *gpio_ref,
> +				  int *irq_ret)
>  {
> +	struct gpio_device *gdev;
> +	struct fwnode_handle *fwnode;
>  	struct gpio_desc *desc;
>
>  	desc = gpiod_get_index(dev, NULL, acpi_index, GPIOD_ASIS);
>  	if (IS_ERR(desc))
>  		return PTR_ERR(desc);

This GPIO is requested for a short time and then released. Do we run into
a risk of the actual target user requesting this pin at the same time and
failing? We don't support shared GPIO pins for ACPI yet (though I want to
add that eventually).

I know this has always been like this but I'm wondering how it works exactly.

>
> -	*gpio_ret = desc_to_gpio(desc);
> +	gdev = gpiod_to_gpio_device(desc);
> +	fwnode = dev_fwnode(gpio_device_to_device(gdev));
> +	if (!fwnode) {
> +		gpiod_put(desc);
> +		return -ENODEV;
> +	}
> +
> +	*gpio_ref = SOFTWARE_NODE_REFERENCE(fwnode, gpiod_hwgpio(desc),
> +					    active_low ? GPIO_ACTIVE_LOW :
> +							 GPIO_ACTIVE_HIGH);

Sigh... I hate it and it looks hacky but I also don't have a better idea. :(

Bart