Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 1/3] Input: soc_button_array - use resources to set up IRQ info for gpio-keys
@ 2026-10-21  1:49 Dmitry Torokhov
  2026-10-21  1:50 ` [PATCH 2/3] Input: soc_button_array - use __free() cleanup facilities to free memory Dmitry Torokhov
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2026-10-21  1:49 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Bartosz Golaszewski, Arnd Bergmann, Andy Shevchenko,
	Linus Walleij, linux-input, linux-gpio, linux-kernel

In preparation for transitioning the driver to use static device
properties instead of platform data to configure gpio-keys driver,
switch to using IRQ resources attached to the gpio-keys platform device
to communicate the IRQ selection.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/misc/soc_button_array.c | 33 +++++++++++++++++++++------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index dbf86ea778e6..d4dc4e7116a7 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -17,6 +17,7 @@
 #include <linux/gpio_keys.h>
 #include <linux/platform_data/x86/soc.h>
 #include <linux/platform_device.h>
+#include <linux/slab.h>
 
 static bool use_low_level_irq;
 module_param(use_low_level_irq, bool, 0444);
@@ -154,6 +155,11 @@ soc_button_device_create(struct platform_device *pdev,
 			 const struct soc_button_info *button_info,
 			 bool autorepeat)
 {
+	struct platform_device_info pdevinfo = {
+		.parent = &pdev->dev,
+		.name = "gpio-keys",
+		.id = PLATFORM_DEVID_AUTO,
+	};
 	const struct soc_button_info *info;
 	struct platform_device *pd;
 	struct gpio_keys_button *gpio_keys;
@@ -167,6 +173,15 @@ soc_button_device_create(struct platform_device *pdev,
 		if (info->autorepeat == autorepeat)
 			n_buttons++;
 
+	struct resource *irq_resources __free(kfree) = NULL;
+	/* See dmi_use_low_level_irq[] comment */
+	if (!autorepeat && (use_low_level_irq ||
+			    dmi_check_system(dmi_use_low_level_irq))) {
+		irq_resources = kzalloc_objs(*irq_resources, n_buttons);
+		if (!irq_resources)
+			return ERR_PTR(-ENOMEM);
+	}
+
 	gpio_keys_pdata = devm_kzalloc(&pdev->dev,
 				       sizeof(*gpio_keys_pdata) +
 					sizeof(*gpio_keys) * n_buttons,
@@ -174,6 +189,9 @@ soc_button_device_create(struct platform_device *pdev,
 	if (!gpio_keys_pdata)
 		return ERR_PTR(-ENOMEM);
 
+	pdevinfo.data = gpio_keys_pdata;
+	pdevinfo.size_data = sizeof(*gpio_keys_pdata);
+
 	gpio_keys = (void *)(gpio_keys_pdata + 1);
 	n_buttons = 0;
 
@@ -212,10 +230,9 @@ soc_button_device_create(struct platform_device *pdev,
 		}
 
 		/* See dmi_use_low_level_irq[] comment */
-		if (!autorepeat && (use_low_level_irq ||
-				    dmi_check_system(dmi_use_low_level_irq))) {
+		if (irq_resources) {
 			irq_set_irq_type(irq, IRQ_TYPE_LEVEL_LOW);
-			gpio_keys[n_buttons].irq = irq;
+			irq_resources[n_buttons] = DEFINE_RES_IRQ(irq);
 			gpio_keys[n_buttons].gpio = -ENOENT;
 		} else {
 			gpio_keys[n_buttons].gpio = gpio;
@@ -240,10 +257,12 @@ soc_button_device_create(struct platform_device *pdev,
 	gpio_keys_pdata->nbuttons = n_buttons;
 	gpio_keys_pdata->rep = autorepeat;
 
-	pd = platform_device_register_resndata(&pdev->dev, "gpio-keys",
-					       PLATFORM_DEVID_AUTO, NULL, 0,
-					       gpio_keys_pdata,
-					       sizeof(*gpio_keys_pdata));
+	if (irq_resources) {
+		pdevinfo.res = irq_resources;
+		pdevinfo.num_res = n_buttons;
+	}
+
+	pd = platform_device_register_full(&pdevinfo);
 	error = PTR_ERR_OR_ZERO(pd);
 	if (error) {
 		dev_err(&pdev->dev,
-- 
2.56.0.212.g129ede20d9-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] Input: soc_button_array - use __free() cleanup facilities to free memory
  2026-10-21  1:49 [PATCH 1/3] Input: soc_button_array - use resources to set up IRQ info for gpio-keys Dmitry Torokhov
@ 2026-10-21  1:50 ` Dmitry Torokhov
  2026-10-21 10:06   ` Bartosz Golaszewski
  2026-10-21  1:50 ` [PATCH 3/3] Input: soc_button_array - convert to software nodes Dmitry Torokhov
  2026-10-21  1:59 ` [PATCH 1/3] Input: soc_button_array - use resources to set up IRQ info for gpio-keys sashiko-bot
  2 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2026-10-21  1:50 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Bartosz Golaszewski, Arnd Bergmann, Andy Shevchenko,
	Linus Walleij, linux-input, linux-gpio, linux-kernel

Switch to using __free(kfree) cleanup facilities instead of doing manual
cleanup or abusing devm for allocations that are only needed during
probing.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/misc/soc_button_array.c | 52 ++++++++++++---------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index d4dc4e7116a7..8d782ef4f78b 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -366,10 +366,10 @@ static const u8 btns_desc_uuid[16] = {
 static struct soc_button_info *soc_button_get_button_info(struct device *dev)
 {
 	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
-	const union acpi_object *desc, *el0, *uuid, *btns_desc = NULL;
-	struct soc_button_info *button_info;
+	const union acpi_object *el0, *uuid, *btns_desc = NULL;
 	acpi_status status;
 	int i, btn, collection_uid = -1;
+	int error;
 
 	status = acpi_evaluate_object_typed(ACPI_HANDLE(dev), "_DSD", NULL,
 					    &buf, ACPI_TYPE_PACKAGE);
@@ -378,8 +378,9 @@ static struct soc_button_info *soc_button_get_button_info(struct device *dev)
 		return ERR_PTR(-ENODEV);
 	}
 
+	const union acpi_object *desc __free(kfree) = buf.pointer;
+
 	/* Look for the Button Descriptors UUID */
-	desc = buf.pointer;
 	for (i = 0; (i + 1) < desc->package.count; i += 2) {
 		uuid = &desc->package.elements[i];
 
@@ -397,8 +398,7 @@ static struct soc_button_info *soc_button_get_button_info(struct device *dev)
 
 	if (!btns_desc || !btns_desc->package.count) {
 		dev_err(dev, "ACPI Button Descriptors not found\n");
-		button_info = ERR_PTR(-ENODEV);
-		goto out;
+		return ERR_PTR(-ENODEV);
 	}
 
 	/* The first package describes the collection */
@@ -414,32 +414,26 @@ static struct soc_button_info *soc_button_get_button_info(struct device *dev)
 	}
 	if (collection_uid == -1) {
 		dev_err(dev, "Invalid Button Collection Descriptor\n");
-		button_info = ERR_PTR(-ENODEV);
-		goto out;
+		return ERR_PTR(-ENODEV);
 	}
 
 	/* There are package.count - 1 buttons + 1 terminating empty entry */
-	button_info = devm_kcalloc(dev, btns_desc->package.count,
-				   sizeof(*button_info), GFP_KERNEL);
-	if (!button_info) {
-		button_info = ERR_PTR(-ENOMEM);
-		goto out;
-	}
+	struct soc_button_info *button_info __free(kfree) =
+		kzalloc_objs(*button_info, btns_desc->package.count);
+	if (!button_info)
+		return ERR_PTR(-ENOMEM);
 
 	/* Parse the button descriptors */
 	for (i = 1, btn = 0; i < btns_desc->package.count; i++, btn++) {
-		if (soc_button_parse_btn_desc(dev,
-					      &btns_desc->package.elements[i],
-					      collection_uid,
-					      &button_info[btn])) {
-			button_info = ERR_PTR(-ENODEV);
-			goto out;
-		}
+		error = soc_button_parse_btn_desc(dev,
+						  &btns_desc->package.elements[i],
+						  collection_uid,
+						  &button_info[btn]);
+		if (error)
+			return ERR_PTR(error);
 	}
 
-out:
-	kfree(buf.pointer);
-	return button_info;
+	return_ptr(button_info);
 }
 
 static void soc_button_remove(struct platform_device *pdev)
@@ -458,6 +452,7 @@ static int soc_button_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	const struct soc_device_data *device_data;
 	const struct soc_button_info *button_info;
+	const struct soc_button_info *dynamic_button_info __free(kfree) = NULL;
 	struct soc_button_data *priv;
 	struct platform_device *pd;
 	int i;
@@ -473,9 +468,11 @@ static int soc_button_probe(struct platform_device *pdev)
 	if (device_data && device_data->button_info) {
 		button_info = device_data->button_info;
 	} else {
-		button_info = soc_button_get_button_info(dev);
-		if (IS_ERR(button_info))
-			return PTR_ERR(button_info);
+		dynamic_button_info = soc_button_get_button_info(dev);
+		if (IS_ERR(dynamic_button_info))
+			return PTR_ERR(dynamic_button_info);
+
+		button_info = dynamic_button_info;
 	}
 
 	error = gpiod_count(dev, NULL);
@@ -507,9 +504,6 @@ static int soc_button_probe(struct platform_device *pdev)
 	if (!priv->children[0] && !priv->children[1])
 		return -ENODEV;
 
-	if (!device_data || !device_data->button_info)
-		devm_kfree(dev, button_info);
-
 	return 0;
 }
 
-- 
2.56.0.212.g129ede20d9-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] Input: soc_button_array - convert to software nodes
  2026-10-21  1:49 [PATCH 1/3] Input: soc_button_array - use resources to set up IRQ info for gpio-keys Dmitry Torokhov
  2026-10-21  1:50 ` [PATCH 2/3] Input: soc_button_array - use __free() cleanup facilities to free memory Dmitry Torokhov
@ 2026-10-21  1:50 ` Dmitry Torokhov
  2026-10-21 10:11   ` Bartosz Golaszewski
  2026-10-21  1:59 ` [PATCH 1/3] Input: soc_button_array - use resources to set up IRQ info for gpio-keys sashiko-bot
  2 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2026-10-21  1:50 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Bartosz Golaszewski, Arnd Bergmann, Andy Shevchenko,
	Linus Walleij, linux-input, linux-gpio, linux-kernel

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@gmail.com>
---
 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);
 
-	*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);
 	*irq_ret = gpiod_to_irq(desc);
 
 	gpiod_put(desc);
@@ -150,11 +165,28 @@ static int soc_button_lookup_gpio(struct device *dev, int acpi_index,
 	return 0;
 }
 
+static void soc_button_remove_swnodes(struct fwnode_handle *parent)
+{
+	struct fwnode_handle *child;
+
+	if (IS_ERR_OR_NULL(parent))
+		return;
+
+	fwnode_for_each_child_node(parent, child)
+		fwnode_remove_software_node(child);
+
+	fwnode_remove_software_node(parent);
+}
+
 static struct platform_device *
 soc_button_device_create(struct platform_device *pdev,
 			 const struct soc_button_info *button_info,
 			 bool autorepeat)
 {
+	const struct property_entry parent_props[] = {
+		PROPERTY_ENTRY_BOOL("autorepeat"),
+		{ }
+	};
 	struct platform_device_info pdevinfo = {
 		.parent = &pdev->dev,
 		.name = "gpio-keys",
@@ -162,17 +194,19 @@ soc_button_device_create(struct platform_device *pdev,
 	};
 	const struct soc_button_info *info;
 	struct platform_device *pd;
-	struct gpio_keys_button *gpio_keys;
-	struct gpio_keys_platform_data *gpio_keys_pdata;
+	struct fwnode_handle *parent_node;
 	const struct dmi_system_id *dmi_id;
 	int invalid_acpi_index = -1;
-	int error, gpio, irq = 0;
+	int error, irq = 0;
 	int n_buttons = 0;
 
 	for (info = button_info; info->name; info++)
 		if (info->autorepeat == autorepeat)
 			n_buttons++;
 
+	if (n_buttons == 0)
+		return ERR_PTR(-ENODEV);
+
 	struct resource *irq_resources __free(kfree) = NULL;
 	/* See dmi_use_low_level_irq[] comment */
 	if (!autorepeat && (use_low_level_irq ||
@@ -182,17 +216,10 @@ soc_button_device_create(struct platform_device *pdev,
 			return ERR_PTR(-ENOMEM);
 	}
 
-	gpio_keys_pdata = devm_kzalloc(&pdev->dev,
-				       sizeof(*gpio_keys_pdata) +
-					sizeof(*gpio_keys) * n_buttons,
-				       GFP_KERNEL);
-	if (!gpio_keys_pdata)
-		return ERR_PTR(-ENOMEM);
-
-	pdevinfo.data = gpio_keys_pdata;
-	pdevinfo.size_data = sizeof(*gpio_keys_pdata);
+	parent_node = fwnode_create_software_node(autorepeat ? parent_props : NULL, NULL);
+	if (IS_ERR(parent_node))
+		return ERR_CAST(parent_node);
 
-	gpio_keys = (void *)(gpio_keys_pdata + 1);
 	n_buttons = 0;
 
 	dmi_id = dmi_first_match(dmi_invalid_acpi_index);
@@ -200,13 +227,19 @@ soc_button_device_create(struct platform_device *pdev,
 		invalid_acpi_index = (long)dmi_id->driver_data;
 
 	for (info = button_info; info->name; info++) {
+		struct software_node_ref_args gpio_ref;
+		struct property_entry child_props[7] = { };
+		struct fwnode_handle *child_node;
+		int prop_idx = 0;
+
 		if (info->autorepeat != autorepeat)
 			continue;
 
 		if (info->acpi_index == invalid_acpi_index)
 			continue;
 
-		error = soc_button_lookup_gpio(&pdev->dev, info->acpi_index, &gpio, &irq);
+		error = soc_button_lookup_gpio(&pdev->dev, info->acpi_index,
+					       info->active_low, &gpio_ref, &irq);
 		if (error || irq < 0) {
 			/*
 			 * Propagate -EPROBE_DEFER, skip button on other errors.
@@ -223,57 +256,68 @@ soc_button_device_create(struct platform_device *pdev,
 			if ((error == -EPROBE_DEFER || irq == -EPROBE_DEFER) &&
 			    !(soc_intel_is_byt() || soc_intel_is_cht())) {
 				error = -EPROBE_DEFER;
-				goto err_free_mem;
+				goto err_remove_swnodes;
 			}
 
 			continue;
 		}
 
+		child_props[prop_idx++] = PROPERTY_ENTRY_U32("linux,code", info->event_code);
+		child_props[prop_idx++] = PROPERTY_ENTRY_STRING("label", info->name);
+
+		if (info->event_type != EV_KEY)
+			child_props[prop_idx++] =
+				PROPERTY_ENTRY_U32("linux,input-type",
+						   info->event_type);
+
+		if (info->wakeup)
+			child_props[prop_idx++] = PROPERTY_ENTRY_BOOL("wakeup-source");
+
+		/* These devices often use cheap buttons, use 50 ms debounce */
+		child_props[prop_idx++] = PROPERTY_ENTRY_U32("debounce-interval", 50);
+
 		/* See dmi_use_low_level_irq[] comment */
 		if (irq_resources) {
 			irq_set_irq_type(irq, IRQ_TYPE_LEVEL_LOW);
 			irq_resources[n_buttons] = DEFINE_RES_IRQ(irq);
-			gpio_keys[n_buttons].gpio = -ENOENT;
 		} else {
-			gpio_keys[n_buttons].gpio = gpio;
+			child_props[prop_idx++] =
+				PROPERTY_ENTRY_REF("gpios", &gpio_ref);
+		}
+
+		child_node = fwnode_create_software_node(child_props, parent_node);
+		if (IS_ERR(child_node)) {
+			error = PTR_ERR(child_node);
+			goto err_remove_swnodes;
 		}
 
-		gpio_keys[n_buttons].type = info->event_type;
-		gpio_keys[n_buttons].code = info->event_code;
-		gpio_keys[n_buttons].active_low = info->active_low;
-		gpio_keys[n_buttons].desc = info->name;
-		gpio_keys[n_buttons].wakeup = info->wakeup;
-		/* These devices often use cheap buttons, use 50 ms debounce */
-		gpio_keys[n_buttons].debounce_interval = 50;
 		n_buttons++;
 	}
 
 	if (n_buttons == 0) {
 		error = -ENODEV;
-		goto err_free_mem;
+		goto err_remove_swnodes;
 	}
 
-	gpio_keys_pdata->buttons = gpio_keys;
-	gpio_keys_pdata->nbuttons = n_buttons;
-	gpio_keys_pdata->rep = autorepeat;
-
 	if (irq_resources) {
 		pdevinfo.res = irq_resources;
 		pdevinfo.num_res = n_buttons;
 	}
 
+	pdevinfo.fwnode = parent_node;
+
 	pd = platform_device_register_full(&pdevinfo);
 	error = PTR_ERR_OR_ZERO(pd);
 	if (error) {
 		dev_err(&pdev->dev,
 			"failed registering gpio-keys: %d\n", error);
-		goto err_free_mem;
+		goto err_remove_swnodes;
 	}
 
 	return pd;
 
-err_free_mem:
-	devm_kfree(&pdev->dev, gpio_keys_pdata);
+err_remove_swnodes:
+	soc_button_remove_swnodes(parent_node);
 	return ERR_PTR(error);
 }
 
@@ -439,12 +483,18 @@ static struct soc_button_info *soc_button_get_button_info(struct device *dev)
 static void soc_button_remove(struct platform_device *pdev)
 {
 	struct soc_button_data *priv = platform_get_drvdata(pdev);
-
+	struct platform_device *pd;
+	struct fwnode_handle *fwnode;
 	int i;
 
-	for (i = 0; i < BUTTON_TYPES; i++)
-		if (priv->children[i])
-			platform_device_unregister(priv->children[i]);
+	for (i = 0; i < BUTTON_TYPES; i++) {
+		pd = priv->children[i];
+		if (pd) {
+			fwnode = dev_fwnode(&pd->dev);
+			platform_device_unregister(pd);
+			soc_button_remove_swnodes(fwnode);
+		}
+	}
 }
 
 static int soc_button_probe(struct platform_device *pdev)
-- 
2.56.0.212.g129ede20d9-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] Input: soc_button_array - use resources to set up IRQ info for gpio-keys
  2026-10-21  1:49 [PATCH 1/3] Input: soc_button_array - use resources to set up IRQ info for gpio-keys Dmitry Torokhov
  2026-10-21  1:50 ` [PATCH 2/3] Input: soc_button_array - use __free() cleanup facilities to free memory Dmitry Torokhov
  2026-10-21  1:50 ` [PATCH 3/3] Input: soc_button_array - convert to software nodes Dmitry Torokhov
@ 2026-10-21  1:59 ` sashiko-bot
  2 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-21  1:59 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-gpio

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Input: mix of __free() and goto cleanup in soc_button_device_create()
--

commit 135ab963100f947c993746f76110c2ca70438cec
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: soc_button_array - use resources to set up IRQ info for gpio-keys

This commit switches soc_button_device_create() to pass IRQ information to
the gpio-keys platform device via platform device IRQ resources instead of
platform data, in preparation for converting the driver to static device
properties.

> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> index dbf86ea778e67..d4dc4e7116a7d 100644
> --- a/drivers/input/misc/soc_button_array.c
> +++ b/drivers/input/misc/soc_button_array.c
[ ... ]
> @@ -167,6 +173,15 @@ soc_button_device_create(struct platform_device *pdev,
>  		if (info->autorepeat == autorepeat)
>  			n_buttons++;
>  
> +	struct resource *irq_resources __free(kfree) = NULL;
> +	/* See dmi_use_low_level_irq[] comment */
> +	if (!autorepeat && (use_low_level_irq ||
> +			    dmi_check_system(dmi_use_low_level_irq))) {
> +		irq_resources = kzalloc_objs(*irq_resources, n_buttons);
> +		if (!irq_resources)
> +			return ERR_PTR(-ENOMEM);
> +	}
> +

[Severity: Low]
In drivers/input/misc/soc_button_array.c, soc_button_device_create()
introduces irq_resources with __free(kfree) while still using goto-based
cleanup via err_free_mem to free gpio_keys_pdata on later failure paths:

drivers/input/misc/soc_button_array.c:soc_button_device_create() {
	...
	if (n_buttons == 0) {
		error = -ENODEV;
		goto err_free_mem;
	}
	...
	pd = platform_device_register_full(&pdevinfo);
	error = PTR_ERR_OR_ZERO(pd);
	if (error) {
		dev_err(&pdev->dev,
			"failed registering gpio-keys: %d\n", error);
		goto err_free_mem;
	}

	return pd;

err_free_mem:
	devm_kfree(&pdev->dev, gpio_keys_pdata);
	return ERR_PTR(error);
}

Although irq_resources is declared and initialized before all of the
goto err_free_mem jumps, include/linux/cleanup.h recommends against mixing
__free() scope-based cleanup and goto cleanup labels within the same
function. Later in the series (in commit "Input: soc_button_array - convert
to software nodes"), err_free_mem is replaced by goto err_remove_swnodes
while keeping irq_resources __free(kfree).

Should soc_button_device_create() avoid mixing __free(kfree) with goto
cleanup here?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021015004.1537738-1-dmitry.torokhov@gmail.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3] Input: soc_button_array - use __free() cleanup facilities to free memory
  2026-10-21  1:50 ` [PATCH 2/3] Input: soc_button_array - use __free() cleanup facilities to free memory Dmitry Torokhov
@ 2026-10-21 10:06   ` Bartosz Golaszewski
  0 siblings, 0 replies; 6+ messages in thread
From: Bartosz Golaszewski @ 2026-10-21 10:06 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Bartosz Golaszewski, Arnd Bergmann, Andy Shevchenko,
	Linus Walleij, linux-input, linux-gpio, linux-kernel,
	Hans de Goede

On Wed, 21 Oct 2026 03:50:00 +0200, Dmitry Torokhov
<dmitry.torokhov@gmail.com> said:
> Switch to using __free(kfree) cleanup facilities instead of doing manual
> cleanup or abusing devm for allocations that are only needed during
> probing.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] Input: soc_button_array - convert to software nodes
  2026-10-21  1:50 ` [PATCH 3/3] Input: soc_button_array - convert to software nodes Dmitry Torokhov
@ 2026-10-21 10:11   ` Bartosz Golaszewski
  0 siblings, 0 replies; 6+ messages in thread
From: Bartosz Golaszewski @ 2026-10-21 10:11 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Bartosz Golaszewski, Arnd Bergmann, Andy Shevchenko,
	Linus Walleij, linux-input, linux-gpio, linux-kernel,
	Hans de Goede

On Wed, 21 Oct 2026 03:50:01 +0200, Dmitry Torokhov
<dmitry.torokhov@gmail.com> 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@gmail.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-21 10:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-21  1:49 [PATCH 1/3] Input: soc_button_array - use resources to set up IRQ info for gpio-keys Dmitry Torokhov
2026-10-21  1:50 ` [PATCH 2/3] Input: soc_button_array - use __free() cleanup facilities to free memory Dmitry Torokhov
2026-10-21 10:06   ` Bartosz Golaszewski
2026-10-21  1:50 ` [PATCH 3/3] Input: soc_button_array - convert to software nodes Dmitry Torokhov
2026-10-21 10:11   ` Bartosz Golaszewski
2026-10-21  1:59 ` [PATCH 1/3] Input: soc_button_array - use resources to set up IRQ info for gpio-keys sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox