* [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