linux-kernel - Re: [PATCH] reset: gpio: add a devlink between reset-gpio and its consumer

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMRc=Mf7CMeZSUJLnteC9Xk5WvG_EKn3vx+jzcWNaWAvpvT00Q@mail.gmail.com>
Date: Wed, 26 Nov 2025 09:56:45 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Philipp Zabel <p.zabel@...gutronix.de>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, Val Packett <val@...kett.cool>, linux-arm-msm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, 
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH] reset: gpio: add a devlink between reset-gpio and its consumer

On Tue, Nov 25, 2025 at 3:33 PM Philipp Zabel <p.zabel@...gutronix.de> wrote:
>
> Hi Bartosz,
>
> On Di, 2025-11-25 at 13:55 +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> >
> > The device that requests the reset control managed by the reset-gpio
> > device is effectively its consumer but the devlink is only established
> > between it and the GPIO controller exposing the reset pin. Add a devlink
> > between the consumer of the reset control and its supplier. This will
> > allow us to simplify the GPIOLIB code managing shared GPIOs when
> > handling the corner case of reset-gpio and gpiolib-shared interacting.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> > ---
> > This change will allow us to simplify the code in gpiolib-shared.c in
> > the next cycle, so it would be awesome if it could make v6.19.
> >
> > Val: I'm Cc'ing you as you're already playing with audio on hamoa. If v2
> > of the reset-gpios fix works for you, could you please also test this
> > and make sure it doesn't break anything with soundwire?
> >
> > Thanks in advance,
> > Bart
> > ---
> >  drivers/reset/core.c | 73 +++++++++++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 58 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> >
> > -static int reset_add_gpio_aux_device(struct device *parent,
> > -                                  struct fwnode_handle *swnode,
> > -                                  int id, void *pdata)
> > +static struct auxiliary_device *
> > +reset_add_gpio_aux_device(struct device *parent, struct fwnode_handle *swnode,
> > +                       int id, void *pdata)
>
> This function grows ever more similar to auxiliary_device_create().
>

I could have used it if not for the fact that it calls
device_set_of_node_from_dev() and we don't want it.

> s/add/create/?

I don't mind but would prefer to not overload this patch.

> > +
> > +static void reset_gpio_add_devlink(struct device_node *np,
> > +                                struct reset_gpio_lookup *rgpio_dev)
> > +{
> > +     struct device *consumer;
> > +
> > +     /*
> > +      * We must use get_dev_from_fwnode() and not of_find_device_by_node()
> > +      * because the latter only considers the platform bus while we want to
> > +      * get consumers of any kind that can be associated with firmware
> > +      * nodes: auxiliary, soundwire, etc.
> > +      */
> > +     consumer = get_dev_from_fwnode(of_fwnode_handle(np));
>
> If called via __reset_control_get(), this just reconstructs the device
> from dev->of_node. I think it would be better to move the linking into
> __reset_control_get() and use the passed in consumer device directly.
>

That would affect all users, do we want this? In most cases they will
already have a link with different flags. I don't think this is
correct.

> > +     if (consumer) {
> > +             if (!device_link_add(consumer, &rgpio_dev->adev->dev, DL_FLAG_STATELESS))
>
> Who removes this link when the consumer puts the reset control, or if
> we error out later?
>

Nobody. Here's why: the reset-gpio device never gets removed. If we're
here, this means it's already registered. If the consumer is unbound,
the devlink stays in place. When we rebind, device_link_add() will do
nothing. If the consumer device is released, we purge all links
anyway. Let me know if I'm wrong, but seems to me, it's fine to leave
it and make it stateless.

Bart