Re: [PATCH 02/10] [v3] input: gpio-keys: make legacy gpiolib optional

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Mon, Jun 01, 2026 at 08:32:31PM +0200, Arnd Bergmann wrote:
> On Sat, May 30, 2026, at 02:45, sashiko-bot@kernel.org wrote:
> > [Severity: High]
> > Does this change break legacy devices that legitimately provide both a valid
> > GPIO and a valid IRQ?
> >
> > If a legacy platform provides both a valid GPIO and an IRQ, this condition
> > now evaluates to false.
> >
> > By skipping the GPIO request entirely, the driver falls back to treating the
> > device as a pure IRQ button. This forces the driver to lose hardware
> > debouncing and rely on a simulated release timer instead of checking the
> > physical GPIO line.
> 
> There are only three drivers that provide the ->irq field. Two are the
> ones that I'm changing in this patch, the soc_button_array driver
> is the third and none of them set ->irq and ->gpio at the same time.
> 
> >>   */
> >>  struct gpio_keys_button {
> >>  	unsigned int code;
> >> +#ifdef CONFIG_GPIOLIB_LEGACY
> >>  	int gpio;
> >> +#endif
> >>  	int active_low;
> >
> > [Severity: Medium]
> > Will this cause build failures for drivers that assign to this field when
> > CONFIG_GPIOLIB_LEGACY is disabled?
> 
> Yes, that is the intention of the patch: nothing should set the
> ->gpio field unless GPIOLIB_LEGACY is set.
> 
> > Other modern drivers dynamically create the platform device and pass legacy
> > GPIO numbers by directly assigning to this field:
> >
> > drivers/input/misc/soc_button_array.c:soc_button_device_create() {
> >     ...
> >     gpio_keys[n_buttons].gpio = gpio;
> >     ...
> > }
> >
> > When CONFIG_GPIOLIB_LEGACY is disabled, this results in a compilation failure
> > since struct gpio_keys_button no longer has the gpio member.
> 
> I previously included a patch force-enable GPIOLIB_LEGACY in this
> patch series, see
> 
> https://lore.kernel.org/all/npijagtgyad33xxlq46b7kwzydhcgt5tkgd5ihsjl6t4czbqyf@umovipwh73i2/
> 
> I made a mistake during a rebase, so my older patch was still
> included in my randconfig testing but not in the series I
> sent. All the other changes in it are now redundant, bit
> the soc_button_array portion indeed still remains.
> 
> Hans, Dmitry, do you already have plans to deal with the
> soc_button_array driver to move it away from legacy GPIOs?
> 
> So far I can see four possible ways we can deal with it,
> but none that I actually like:
> 
> a) delay the problem, apply my original oneline change
>    to 'select GPIOLIB_LEGACY' and fix it later, so we
>    can make GPIOLIB_LEGACY default-disabled in 7.3.
> 
> b) add a gpiod member to struct gpio_keys_button and skip
>    the intermediate gpio number here. Clean it up later.
> 
> c) always pass the gpio as an interrupt, as the driver
>    already does for some machines
> 
> d) add dynamic device properties that duplicate the
>    information from ACPI/DMI, so the driver can
>    stop using platform data
> 
> e) disconnect gpio_keys_button from gpio-keys.c and
>    register the buttons to the input subsystem
>    directly from soc_button_device_create().

I tried doing e) and while it is possible I ended up re-implementing
most of gpio_keys.c which is not great. We still need both GPIO and
IRQ-only modes, and handling debouncing, both software and hardware, and
so on. So I would like to see if d) is possible. The problem is that we
need to reconstruct GPIO reference from existing gpio_desc. If Bart will
be OK with something like below then I think the conversion should be
achievable.

gpiolib: Introduce gpiod_get_software_node_reference()

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

To support converting drivers (such as soc_button_array) to use static
device properties/software nodes, we need a way to reconstruct a GPIO
reference from an existing gpio_desc.

Introduce gpiod_get_software_node_reference() which populates a
struct software_node_ref_args with the GPIO device's fwnode, the pin
offset, and the appropriate active-low flag.

Assisted-by: Gemini:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpiolib.c        |   35 +++++++++++++++++++++++++++++++++++
 include/linux/gpio/consumer.h |   12 ++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 1e6dce430dca..63cef0fb02b5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -247,6 +247,41 @@ int gpiod_hwgpio(const struct gpio_desc *desc)
 }
 EXPORT_SYMBOL_GPL(gpiod_hwgpio);
 
+/**
+ * gpiod_get_software_node_reference() - Reconstruct a software node reference
+ *                                       from a GPIO descriptor
+ * @desc: GPIO descriptor
+ * @args: Pointer to software node reference arguments structure to populate
+ *
+ * This function can be used to construct a software node reference from
+ * an existing GPIO descriptor. This is useful when a driver wants to pass
+ * a software node to a secondary device, but needs to reconstruct a GPIO
+ * reference dynamically.
+ *
+ * Returns:
+ * 0 on success, or a negative error code on failure.
+ */
+int gpiod_get_software_node_reference(const struct gpio_desc *desc,
+				      struct software_node_ref_args *args)
+{
+	struct gpio_device *gdev;
+
+	if (IS_ERR_OR_NULL(desc) || !args)
+		return -EINVAL;
+
+	gdev = desc->gdev;
+
+	memset(args, 0, sizeof(*args));
+	args->fwnode = dev_fwnode(&gdev->dev);
+	args->nargs = 2;
+	args->args[0] = gpiod_hwgpio(desc);
+	if (test_bit(GPIOD_FLAG_ACTIVE_LOW, &desc->flags))
+		args->args[1] = GPIO_ACTIVE_LOW;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gpiod_get_software_node_reference);
+
 /**
  * gpiod_to_chip - Return the GPIO chip to which a GPIO descriptor belongs
  * @desc:	descriptor to return the chip of
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 3efb5cb1e1d1..c95b715a1d15 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -11,6 +11,7 @@
 struct acpi_device;
 struct device;
 struct fwnode_handle;
+struct software_node_ref_args;
 
 struct gpio_array;
 struct gpio_desc;
@@ -177,6 +178,9 @@ int desc_to_gpio(const struct gpio_desc *desc);
 
 int gpiod_hwgpio(const struct gpio_desc *desc);
 
+int gpiod_get_software_node_reference(const struct gpio_desc *desc,
+				      struct software_node_ref_args *args);
+
 struct gpio_desc *fwnode_gpiod_get_index(struct fwnode_handle *fwnode,
 					 const char *con_id, int index,
 					 enum gpiod_flags flags,
@@ -545,6 +549,14 @@ static inline int desc_to_gpio(const struct gpio_desc *desc)
 	return -EINVAL;
 }
 
+static inline
+int gpiod_get_software_node_reference(const struct gpio_desc *desc,
+				      struct software_node_ref_args *args)
+{
+	WARN_ON(desc);
+	return -EINVAL;
+}
+
 static inline
 struct gpio_desc *fwnode_gpiod_get_index(struct fwnode_handle *fwnode,
 					 const char *con_id, int index,

Thanks.

-- 
Dmitry