| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0d251a35a438ebf3e14c6762df7ece079ee1d164.camel@pengutronix.de>
Date: Tue, 18 Nov 2025 17:44:11 +0100
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Bartosz Golaszewski <brgl@...ev.pl>, Linus Walleij
<linus.walleij@...aro.org>, Andy Shevchenko
<andriy.shevchenko@...ux.intel.com>, Daniel Scally <djrscally@...il.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>, Sakari Ailus
<sakari.ailus@...ux.intel.com>, Greg Kroah-Hartman
<gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
Danilo Krummrich <dakr@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>
Cc: linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org, Bartosz Golaszewski
<bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v6 8/8] reset: gpio: use software nodes to setup the
GPIO lookup
On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
>
> GPIO machine lookup is a nice mechanism for associating GPIOs with
> consumers if we don't know what kind of device the GPIO provider is or
> when it will become available. However in the case of the reset-gpio, we
> are already holding a reference to the device and so can reference its
> firmware node. Let's setup a software node that references the relevant
> GPIO and attach it to the auxiliary device we're creating.
>
> Reviewed-by: Philipp Zabel <p.zabel@...gutronix.de>
> Acked-by: Linus Walleij <linus.walleij@...aro.org>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> ---
> drivers/reset/core.c | 126 +++++++++++++++++++++++++++++----------------------
> 1 file changed, 73 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index cefeff10f6c82f5aef269a6d3a58d9d204ed6b7e..8262879e3f0d9ce67683c6baa00d9eea9e3c3ca3 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -14,6 +14,7 @@
> #include <linux/export.h>
> #include <linux/gpio/driver.h>
> #include <linux/gpio/machine.h>
> +#include <linux/gpio/property.h>
> #include <linux/idr.h>
> #include <linux/kernel.h>
> #include <linux/kref.h>
> @@ -77,10 +78,12 @@ struct reset_control_array {
> /**
> * struct reset_gpio_lookup - lookup key for ad-hoc created reset-gpio devices
> * @of_args: phandle to the reset controller with all the args like GPIO number
> + * @swnode: Software node containing the reference to the GPIO provider
> * @list: list entry for the reset_gpio_lookup_list
> */
> struct reset_gpio_lookup {
> struct of_phandle_args of_args;
> + struct fwnode_handle *swnode;
> struct list_head list;
> };
>
> @@ -822,52 +825,45 @@ static void __reset_control_put_internal(struct reset_control *rstc)
> kref_put(&rstc->refcnt, __reset_control_release);
> }
>
> -static int __reset_add_reset_gpio_lookup(struct gpio_device *gdev, int id,
> - struct device_node *np,
> - unsigned int gpio,
> - unsigned int of_flags)
> +static void reset_gpio_aux_device_release(struct device *dev)
> {
> - unsigned int lookup_flags;
> - const char *label_tmp;
> + struct auxiliary_device *adev = to_auxiliary_dev(dev);
>
> - /*
> - * Later we map GPIO flags between OF and Linux, however not all
> - * constants from include/dt-bindings/gpio/gpio.h and
> - * include/linux/gpio/machine.h match each other.
> - */
> - if (of_flags > GPIO_ACTIVE_LOW) {
> - pr_err("reset-gpio code does not support GPIO flags %u for GPIO %u\n",
> - of_flags, gpio);
> - return -EINVAL;
> + kfree(adev);
> +}
> +
> +static int reset_add_gpio_aux_device(struct device *parent,
> + struct fwnode_handle *swnode,
> + int id, void *pdata)
> +{
> + struct auxiliary_device *adev;
> + int ret;
> +
> + adev = kzalloc(sizeof(*adev), GFP_KERNEL);
> + if (!adev)
> + return -ENOMEM;
> +
> + adev->id = id;
> + adev->name = "gpio";
> + adev->dev.parent = parent;
> + adev->dev.platform_data = pdata;
> + adev->dev.release = reset_gpio_aux_device_release;
> + device_set_node(&adev->dev, swnode);
> +
> + ret = auxiliary_device_init(adev);
> + if (ret) {
> + kfree(adev);
> + return ret;
> }
>
> - label_tmp = gpio_device_get_label(gdev);
> - if (!label_tmp)
> - return -EINVAL;
> + ret = __auxiliary_device_add(adev, "reset");
> + if (ret) {
> + auxiliary_device_uninit(adev);
> + kfree(adev);
> + return ret;
> + }
>
> - char *label __free(kfree) = kstrdup(label_tmp, GFP_KERNEL);
> - if (!label)
> - return -ENOMEM;
> -
> - /* Size: one lookup entry plus sentinel */
> - struct gpiod_lookup_table *lookup __free(kfree) = kzalloc(struct_size(lookup, table, 2),
> - GFP_KERNEL);
> - if (!lookup)
> - return -ENOMEM;
> -
> - lookup->dev_id = kasprintf(GFP_KERNEL, "reset.gpio.%d", id);
> - if (!lookup->dev_id)
> - return -ENOMEM;
> -
> - lookup_flags = GPIO_PERSISTENT;
> - lookup_flags |= of_flags & GPIO_ACTIVE_LOW;
> - lookup->table[0] = GPIO_LOOKUP(no_free_ptr(label), gpio, "reset",
> - lookup_flags);
> -
> - /* Not freed on success, because it is persisent subsystem data. */
> - gpiod_add_lookup_table(no_free_ptr(lookup));
> -
> - return 0;
> + return ret;
> }
>
> /*
> @@ -875,8 +871,10 @@ static int __reset_add_reset_gpio_lookup(struct gpio_device *gdev, int id,
> */
> static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
> {
> + struct property_entry properties[2] = { };
> + unsigned int offset, of_flags, lflags;
> struct reset_gpio_lookup *rgpio_dev;
> - struct auxiliary_device *adev;
> + struct device *parent;
> int id, ret;
>
> /*
> @@ -895,6 +893,23 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
> */
> lockdep_assert_not_held(&reset_list_mutex);
>
> + offset = args->args[0];
> + of_flags = args->args[1];
> +
> + /*
> + * Later we map GPIO flags between OF and Linux, however not all
> + * constants from include/dt-bindings/gpio/gpio.h and
> + * include/linux/gpio/machine.h match each other.
> + *
> + * FIXME: Find a better way of translating OF flags to GPIO lookup
> + * flags.
> + */
> + if (of_flags > GPIO_ACTIVE_LOW) {
> + pr_err("reset-gpio code does not support GPIO flags %u for GPIO %u\n",
> + of_flags, offset);
> + return -EINVAL;
> + }
> +
> struct gpio_device *gdev __free(gpio_device_put) =
> gpio_device_find_by_fwnode(of_fwnode_handle(args->np));
> if (!gdev)
> @@ -909,6 +924,10 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
> }
> }
>
> + lflags = GPIO_PERSISTENT | (of_flags & GPIO_ACTIVE_LOW);
> + parent = gpio_device_to_device(gdev);
> + properties[0] = PROPERTY_ENTRY_GPIO("reset-gpios", parent->fwnode, offset, lflags);
> +
> id = ida_alloc(&reset_gpio_ida, GFP_KERNEL);
> if (id < 0)
> return id;
> @@ -920,11 +939,6 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
> goto err_ida_free;
> }
>
> - ret = __reset_add_reset_gpio_lookup(gdev, id, args->np, args->args[0],
> - args->args[1]);
> - if (ret < 0)
> - goto err_kfree;
> -
> rgpio_dev->of_args = *args;
> /*
> * We keep the device_node reference, but of_args.np is put at the end
> @@ -932,19 +946,25 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
> * Hold reference as long as rgpio_dev memory is valid.
> */
> of_node_get(rgpio_dev->of_args.np);
> - adev = auxiliary_device_create(gpio_device_to_device(gdev), "reset",
> - "gpio", &rgpio_dev->of_args, id);
> - ret = PTR_ERR_OR_ZERO(adev);
> +
> + rgpio_dev->swnode = fwnode_create_software_node(properties, NULL);
> + ret = PTR_ERR(rgpio_dev->swnode);
I'll apply this with the following patch squashed in:
diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 3edf04ae8a95..8a7b112a9a77 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -945,7 +945,7 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
of_node_get(rgpio_dev->of_args.np);
rgpio_dev->swnode = fwnode_create_software_node(properties, NULL);
- ret = PTR_ERR(rgpio_dev->swnode);
+ ret = PTR_ERR_OR_ZERO(rgpio_dev->swnode);
if (ret)
goto err_put_of_node;
regards
Philipp
Powered by blists - more mailing lists