[PATCH 0/3] gpio: deprecate and track the removal of the GPIOD_FLAGS_BIT_NONEXCLUSIVE flag

[PATCH 0/3] gpio: deprecate and track the removal of the GPIOD_FLAGS_BIT_NONEXCLUSIVE flag

[Bartosz Golaszewski]

This feature is another pet-peeve of mine. It's a hack that people
started using and now it's in all kinds of drivers. It doesn't really
explain what it actually does, and it implements it badly.

Let's deprecate it officially, add it to MAINTAINERS keywords so that it
pops up on our radars when used again, add a task to track it and I plan
to use the power sequencing subsystem to handle the cases where
non-exclusive access to GPIOs is required.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
Bartosz Golaszewski (3):
      gpio: deprecate the GPIOD_FLAGS_BIT_NONEXCLUSIVE flag
      MAINTAINERS: add another keyword for the GPIO subsystem
      gpio: TODO: track the removal of GPIOD_FLAGS_BIT_NONEXCLUSIVE

 MAINTAINERS                   |  1 +
 drivers/gpio/TODO             | 14 ++++++++++++++
 include/linux/gpio/consumer.h |  1 +
 3 files changed, 16 insertions(+)
---
base-commit: 405e2241def89c88f008dcb899eb5b6d4be8b43c
change-id: 20250331-gpio-todo-remove-nonexclusive-ed875467eb56

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

[PATCH 1/3] gpio: deprecate the GPIOD_FLAGS_BIT_NONEXCLUSIVE flag

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

The non-exclusive GPIO request flag looks like a functional feature but
is in fact a bad workaround for a corner-case that got out of hand. It
should be removed so deprecate it officially so that nobody uses it
anymore.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 include/linux/gpio/consumer.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 45b651c05b9c8..8adc8e9cb4a7b 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -31,6 +31,7 @@ struct gpio_descs {
 #define GPIOD_FLAGS_BIT_DIR_OUT		BIT(1)
 #define GPIOD_FLAGS_BIT_DIR_VAL		BIT(2)
 #define GPIOD_FLAGS_BIT_OPEN_DRAIN	BIT(3)
+/* GPIOD_FLAGS_BIT_NONEXCLUSIVE is DEPRECATED, don't use in new code. */
 #define GPIOD_FLAGS_BIT_NONEXCLUSIVE	BIT(4)
 
 /**

-- 
2.45.2

[PATCH 2/3] MAINTAINERS: add another keyword for the GPIO subsystem

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Add GPIOD_FLAGS_BIT_NONEXCLUSIVE as a keyword to the GPIO entry so that
we get notified if anybody tries to use it as it's a deprecated symbol.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ce2b64f4568d5..210fbca61ad37 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10151,6 +10151,7 @@ F:	include/linux/gpio.h
 F:	include/linux/gpio/
 F:	include/linux/of_gpio.h
 K:	(devm_)?gpio_(request|free|direction|get|set)
+K:	GPIOD_FLAGS_BIT_NONEXCLUSIVE
 
 GPIO UAPI
 M:	Bartosz Golaszewski <brgl@bgdev.pl>

-- 
2.45.2

[PATCH 3/3] gpio: TODO: track the removal of GPIOD_FLAGS_BIT_NONEXCLUSIVE

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

This flag should be replaced by a better mechanism that counts the users
and properly manages the resources. The pwrseq subsystem is a good
candidate. GPIOs themselves should remain a unique resource. Add a task
for tracking the removal of GPIOD_FLAGS_BIT_NONEXCLUSIVE.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/TODO | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpio/TODO b/drivers/gpio/TODO
index b5f0a7a2e1bf1..5385071901993 100644
--- a/drivers/gpio/TODO
+++ b/drivers/gpio/TODO
@@ -186,3 +186,17 @@ their hardware offsets within the chip.
 
 Encourage users to switch to using them and eventually remove the existing
 global export/unexport attribues.
+
+-------------------------------------------------------------------------------
+
+Remove GPIOD_FLAGS_BIT_NONEXCLUSIVE
+
+This flag is an awful workaround that was created for some regulator
+corner-cases but got out of hand and is now used in at least 33 places
+treewide. Unlike what the intuition may tell users, it's not a reference
+counted mechanisms like what clocks or regulators use but just a raw access
+to the same GPIO descriptor from multiple places with no synchronization (other
+than what the underying driver offers). It doesn't even correctly support
+releasing the supposedly non-exclusive GPIOs. This whole thing should go and be
+replaced with a better solution - for exampe: using the relatively new power
+sequencing subsystem.

-- 
2.45.2

Re: [PATCH 3/3] gpio: TODO: track the removal of GPIOD_FLAGS_BIT_NONEXCLUSIVE

[Linus Walleij]

On Mon, Mar 31, 2025 at 11:00 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> +Remove GPIOD_FLAGS_BIT_NONEXCLUSIVE
> +
> +This flag is an awful workaround that was created for some regulator
> +corner-cases but got out of hand and is now used in at least 33 places
> +treewide. Unlike what the intuition may tell users, it's not a reference
> +counted mechanisms like what clocks or regulators use but just a raw access
> +to the same GPIO descriptor from multiple places with no synchronization (other
> +than what the underying driver offers). It doesn't even correctly support
> +releasing the supposedly non-exclusive GPIOs. This whole thing should go and be
> +replaced with a better solution - for exampe: using the relatively new power
> +sequencing subsystem.

Try to focus on the solution instead of writing so much about the problem.
We mostly get the information that the GPIO maintainer dislikes them,
not so much about why they exist and what can be done about them.

I would describe the actual problem and the actual solution,
something like:

"A problematic usecase for GPIOs is when two consumers want to use
the same descriptor independently, as a consumer (in Linux a struct
device) is generally assumed to have exclusive access to all of its
resources, with a resource being defined as anything obtained behind
a devm_* managed resource API such as devm_gpiod_get().

Providers such as regulators pose a special problem here since
regulators instantiate several struct regulator_dev instances containing
a struct device but using the *same* enable GPIO line: i.e. from a Linux
point of view this enable line has a many-to-one ownership. You can
imagine a 12V and a 5V regulator being turned on by the same GPIO
line for example. The regulator resources need to have two different
devices in Linux because they have different voltages, but they are enabled
by the same GPIO.

This breaks the devres resource assumptions:

If several providers with their own struct device is using one
and the same GPIO line, the devres consumer is unclear: which
struct device should own the GPIO line?

A hack allows GPIO lines to be shared between several consumers
with the flag GPIOD_FLAGS_BIT_NONEXCLUSIVE but this API is
confusing and prone to abuse, as is the related devm_gpiod_unhinge() API.

A better solution to some of the problems is to use approaches such as
the power sequencing subsystem, which avoids having several owners of
a resource by strictly sequencing the order in which they are obtained
and activated.

For example a GPIO turning on the power for both wireless and bluetooth
chips can be obtained and turned on in a power sequence such that this
problem never occurs: it is always active when when it needs to be, it has
just one owner (the power sequence itself, struct pwrseq_device, which
contains a struct device) and the GPIO regulator driver is not used in this
scenario."

Yours,
Linus Walleij

Re: [PATCH 3/3] gpio: TODO: track the removal of GPIOD_FLAGS_BIT_NONEXCLUSIVE

[Bartosz Golaszewski]

On Tue, Apr 1, 2025 at 12:49 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Mar 31, 2025 at 11:00 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > +Remove GPIOD_FLAGS_BIT_NONEXCLUSIVE
> > +
> > +This flag is an awful workaround that was created for some regulator
> > +corner-cases but got out of hand and is now used in at least 33 places
> > +treewide. Unlike what the intuition may tell users, it's not a reference
> > +counted mechanisms like what clocks or regulators use but just a raw access
> > +to the same GPIO descriptor from multiple places with no synchronization (other
> > +than what the underying driver offers). It doesn't even correctly support
> > +releasing the supposedly non-exclusive GPIOs. This whole thing should go and be
> > +replaced with a better solution - for exampe: using the relatively new power
> > +sequencing subsystem.
>
> Try to focus on the solution instead of writing so much about the problem.
> We mostly get the information that the GPIO maintainer dislikes them,
> not so much about why they exist and what can be done about them.
>

I'm sorry, after a second thought this does indeed sound too harsh but
when I realized that people started slapping this non-exclusive flag
on every problem - like when the GPIO ACPI code requests some GPIOs
(possibly erroneously), so driver authors just request it as
non-exclusive instead of investigating the actual problem, or some
codec drivers in sound/ which seem to not even need it and it looks
like a bad copy-paste, I got really frustrated that it's another thing
on the pile of stuff to fix. I will reword this entry.

> I would describe the actual problem and the actual solution,
> something like:
>
> "A problematic usecase for GPIOs is when two consumers want to use
> the same descriptor independently, as a consumer (in Linux a struct
> device) is generally assumed to have exclusive access to all of its
> resources, with a resource being defined as anything obtained behind
> a devm_* managed resource API such as devm_gpiod_get().
>

This is not a devres issue though. I don't think we should mention it
here. The real problem about this flag is that it effectively allows
two users to "fight" over a line's state. It would be better if it
would count the enable operations but this would go against the spirit
of the "gpiod_set_value()" interface, where you expect the value to be
actually set to what you request it to be. GPIOs should remain
exclusive and any "packaging" should happen in a higher layer.

This is one of those pesky resource ownership issues really.

> Providers such as regulators pose a special problem here since
> regulators instantiate several struct regulator_dev instances containing
> a struct device but using the *same* enable GPIO line: i.e. from a Linux
> point of view this enable line has a many-to-one ownership. You can
> imagine a 12V and a 5V regulator being turned on by the same GPIO
> line for example. The regulator resources need to have two different
> devices in Linux because they have different voltages, but they are enabled
> by the same GPIO.
>
> This breaks the devres resource assumptions:
>

The same thing would happen without devres. I think the regulator
framework should have some way of mediating GPIO use. There should
only really be a single gpiod_get() call for all enable-gpios of a
single regulator provider. I have it on my roadmap to look into it.
Whether the right approach is a GPIO quirk or a power sequence
provider binding to the top-level regulator provider, I don't know
yet.

> If several providers with their own struct device is using one
> and the same GPIO line, the devres consumer is unclear: which
> struct device should own the GPIO line?
>

Well, other subsystems just use reference-counted resources in this
case but see above - this is not a good fit for GPIOs.

> A hack allows GPIO lines to be shared between several consumers
> with the flag GPIOD_FLAGS_BIT_NONEXCLUSIVE but this API is
> confusing and prone to abuse, as is the related devm_gpiod_unhinge() API.

This is another thing that should most likely be deprecated and
removed. It's clearly a case of papering over unclear ownership of a
resource. I believe that any "hand-over" of ownership is really asking
for trouble. The entity that does the "get" should also do the "put"
in almost all cases. Fortunately it's only limited to a few use-cases
in drivers/regulator/ so it's not nearly as bad. However, I don't
really see a close relationship between these two issues. How about
adding a task for that as well?

>
> A better solution to some of the problems is to use approaches such as
> the power sequencing subsystem, which avoids having several owners of
> a resource by strictly sequencing the order in which they are obtained
> and activated.
>
> For example a GPIO turning on the power for both wireless and bluetooth
> chips can be obtained and turned on in a power sequence such that this
> problem never occurs: it is always active when when it needs to be, it has
> just one owner (the power sequence itself, struct pwrseq_device, which
> contains a struct device) and the GPIO regulator driver is not used in this
> scenario."
>

So this bit is also unrelated - in the case of the wcn pwrseq driver,
there are two separate GPIOs for wifi and bluetooth. They just need a
delay between toggling one and the other. I would skip it.

Bart

Re: [PATCH 3/3] gpio: TODO: track the removal of GPIOD_FLAGS_BIT_NONEXCLUSIVE

[Linus Walleij]

On Tue, Apr 1, 2025 at 10:57 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> > If several providers with their own struct device is using one
> > and the same GPIO line, the devres consumer is unclear: which
> > struct device should own the GPIO line?
> >
>
> Well, other subsystems just use reference-counted resources in this
> case but see above - this is not a good fit for GPIOs.

So to rehash, for example clocks and regulators are by definition the
equivalent to NONEXCLUSIVE, that is their default behaviour.

Two devices can surely request the same clock.

They can independently issue clk_enable() and clk_disable(),
and the semantics is a reference count increase/decrease.

They can have the same phandle in the device tree.

But GPIOs can not. They can only have one owner.

Technically this is because the only reference count we have in a gpio
descriptor is the boolean flag FLAG_REQUESTED, and it
happens like this in gpiod_request_commit():

        if (test_and_set_bit(FLAG_REQUESTED, &desc->flags))
                return -EBUSY;

This semantic is in a way natural because what would you do when
two owners make something a GPIO cannot do, such as
one does gpiod_set_value(1) and the other does gpiod_set_value(0)?

This issue does not exist in resources such as clocks or
regulators that only do enable/disable and that is why GPIOs
are different from other resources.

Then we can think of solutions to that.

One way would be to add a new type of refcounted GPIO
descriptor for this specific usecase, like (OTOMH):

struct gpio_desc_shared {
    struct gpio_desc *gpiod;
    struct device *devs[MAX_OWNERS];
    u32 use_count;
};

struct gpio_desc_shared *gpiod_shared_get(struct device *dev ...);
void gpiod_shared_put(struct gpio_desc_shared *gds);

int gpiod_shared_enable(struct gpio_desc_shared *gds);
int gpiod_shared_disable(struct gpio_desc_shared *gds);

So this compound struct will not be able to set value
directly.

The gpiod inside that shared descriptor need to be obtained with
some gpiolib-internal quirk, not with gpiod_request().

It will issue gpiod_set_value(1) on the first enable and
gpiod_set_value(0) on last disable its internal descriptor.

If existing valid users are switched over to that then the
NONEXCLUSIVE stuff can be deleted.

Yours,
Linus Walleij

Re: [PATCH 3/3] gpio: TODO: track the removal of GPIOD_FLAGS_BIT_NONEXCLUSIVE

[Bartosz Golaszewski]

On Fri, Apr 4, 2025 at 11:02 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Apr 1, 2025 at 10:57 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > > If several providers with their own struct device is using one
> > > and the same GPIO line, the devres consumer is unclear: which
> > > struct device should own the GPIO line?
> > >
> >
> > Well, other subsystems just use reference-counted resources in this
> > case but see above - this is not a good fit for GPIOs.
>
> So to rehash, for example clocks and regulators are by definition the
> equivalent to NONEXCLUSIVE, that is their default behaviour.
>
> Two devices can surely request the same clock.
>
> They can independently issue clk_enable() and clk_disable(),
> and the semantics is a reference count increase/decrease.
>
> They can have the same phandle in the device tree.
>
> But GPIOs can not. They can only have one owner.
>
> Technically this is because the only reference count we have in a gpio
> descriptor is the boolean flag FLAG_REQUESTED, and it
> happens like this in gpiod_request_commit():
>
>         if (test_and_set_bit(FLAG_REQUESTED, &desc->flags))
>                 return -EBUSY;
>
> This semantic is in a way natural because what would you do when
> two owners make something a GPIO cannot do, such as
> one does gpiod_set_value(1) and the other does gpiod_set_value(0)?
>

Or even better: one goes gpiod_direction_output(desc, 1), the other
goes gpiod_direction_input()!

One goes gpiod_set_config(OPEN_DRAIN), the other gpiod_set_config(OPEN_SOURCE)!!

There's just no good way of allowing multiple users to work with the
same line in a safe and sane way.

> This issue does not exist in resources such as clocks or
> regulators that only do enable/disable and that is why GPIOs
> are different from other resources.
>
> Then we can think of solutions to that.
>
> One way would be to add a new type of refcounted GPIO
> descriptor for this specific usecase, like (OTOMH):
>
> struct gpio_desc_shared {
>     struct gpio_desc *gpiod;
>     struct device *devs[MAX_OWNERS];
>     u32 use_count;
> };
>
> struct gpio_desc_shared *gpiod_shared_get(struct device *dev ...);
> void gpiod_shared_put(struct gpio_desc_shared *gds);
>
> int gpiod_shared_enable(struct gpio_desc_shared *gds);
> int gpiod_shared_disable(struct gpio_desc_shared *gds);
>
> So this compound struct will not be able to set value
> directly.
>
> The gpiod inside that shared descriptor need to be obtained with
> some gpiolib-internal quirk, not with gpiod_request().
>
> It will issue gpiod_set_value(1) on the first enable and
> gpiod_set_value(0) on last disable its internal descriptor.
>
> If existing valid users are switched over to that then the
> NONEXCLUSIVE stuff can be deleted.
>

I don't agree with this. I could possibly live with that being used
exclusively in lower-level core subsystem code (for instance:
regulator/core.c) but, in this form, this still requires drivers - who
have no business knowing whether the GPIO they use is shared - to use
the right API. Not to mention that once you make an interface
available, people will be very eager to abuse it. IMO this should be
approached from the other side.

The closest thing to making the sharing opaque to consumers is
providing a pwrseq-backed, faux GPIO chip that allows a very limited
set of operations on GPIOs - get, get_value, set_value - and return an
error on others. A value set would actually be equivalent to "enable
high" and be refcounted by pwrseq. I have something in mind but this
cycle, I already have a lot on my plate. I will get to it eventually
and come up with some code to back my idea.

In any case: the GPIO sharing logic should be hidden, I just need to
figure out how to make it possible to be notified about when the value
change actually happens as per Mark's requirement.

Let me reiterate: a random ethernet PHY driver should not have to call
gpiod_get_shared() or anything similar - why would it? It can be used
in all kinds of situations, whether the GPIO is shared is none of its
business.

Bartosz

Re: [PATCH 3/3] gpio: TODO: track the removal of GPIOD_FLAGS_BIT_NONEXCLUSIVE

[Linus Walleij]

On Mon, Apr 7, 2025 at 2:38 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> I don't agree with this. I could possibly live with that being used
> exclusively in lower-level core subsystem code (for instance:
> regulator/core.c) but, in this form, this still requires drivers - who
> have no business knowing whether the GPIO they use is shared - to use
> the right API. Not to mention that once you make an interface
> available, people will be very eager to abuse it. IMO this should be
> approached from the other side.
>
> The closest thing to making the sharing opaque to consumers is
> providing a pwrseq-backed, faux GPIO chip that allows a very limited
> set of operations on GPIOs - get, get_value, set_value - and return an
> error on others. A value set would actually be equivalent to "enable
> high" and be refcounted by pwrseq. I have something in mind but this
> cycle, I already have a lot on my plate. I will get to it eventually
> and come up with some code to back my idea.
>
> In any case: the GPIO sharing logic should be hidden, I just need to
> figure out how to make it possible to be notified about when the value
> change actually happens as per Mark's requirement.
>
> Let me reiterate: a random ethernet PHY driver should not have to call
> gpiod_get_shared() or anything similar - why would it? It can be used
> in all kinds of situations, whether the GPIO is shared is none of its
> business.

I get your point, it's just that I don't see how pwrseq solves it, so I
would have to see it.

I think a bit of my problem is with the name, as in how would a
power seqeunce solve the problem of a GPIO that is shared for
something not power or reset for example.

But maybe all the existing cases we have are shared power or
reset :D

I could think of a shared LED GPIO (this LED should be on if
any consumers A...X are active) but I just made that up.

Yours,
Linus Walleij

Re: [PATCH 3/3] gpio: TODO: track the removal of GPIOD_FLAGS_BIT_NONEXCLUSIVE

[Bartosz Golaszewski]

On Tue, Apr 15, 2025 at 10:44 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Apr 7, 2025 at 2:38 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > I don't agree with this. I could possibly live with that being used
> > exclusively in lower-level core subsystem code (for instance:
> > regulator/core.c) but, in this form, this still requires drivers - who
> > have no business knowing whether the GPIO they use is shared - to use
> > the right API. Not to mention that once you make an interface
> > available, people will be very eager to abuse it. IMO this should be
> > approached from the other side.
> >
> > The closest thing to making the sharing opaque to consumers is
> > providing a pwrseq-backed, faux GPIO chip that allows a very limited
> > set of operations on GPIOs - get, get_value, set_value - and return an
> > error on others. A value set would actually be equivalent to "enable
> > high" and be refcounted by pwrseq. I have something in mind but this
> > cycle, I already have a lot on my plate. I will get to it eventually
> > and come up with some code to back my idea.
> >
> > In any case: the GPIO sharing logic should be hidden, I just need to
> > figure out how to make it possible to be notified about when the value
> > change actually happens as per Mark's requirement.
> >
> > Let me reiterate: a random ethernet PHY driver should not have to call
> > gpiod_get_shared() or anything similar - why would it? It can be used
> > in all kinds of situations, whether the GPIO is shared is none of its
> > business.
>
> I get your point, it's just that I don't see how pwrseq solves it, so I
> would have to see it.
>
> I think a bit of my problem is with the name, as in how would a
> power seqeunce solve the problem of a GPIO that is shared for
> something not power or reset for example.
>

Sigh... It may end up being the same story as with BPF. I named it
pwrseq because a power sequence was what I was working with but it
also deals with shared resources.

> But maybe all the existing cases we have are shared power or
> reset :D
>

No, unfortunately not, though for the cases of shared reset-gpios we
now have the reset-gpio.c driver and its implicit sharing of GPIOs.

> I could think of a shared LED GPIO (this LED should be on if
> any consumers A...X are active) but I just made that up.
>

Actually, I have something in mind that may allow me to avoid using
pwrseq. Give me a couple weeks though as I have other priorities ATM.

Bart

Re: [PATCH 0/3] gpio: deprecate and track the removal of the GPIOD_FLAGS_BIT_NONEXCLUSIVE flag

[Linus Walleij]

On Mon, Mar 31, 2025 at 11:00 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> This feature is another pet-peeve of mine. It's a hack that people
> started using and now it's in all kinds of drivers. It doesn't really
> explain what it actually does, and it implements it badly.
>
> Let's deprecate it officially, add it to MAINTAINERS keywords so that it
> pops up on our radars when used again, add a task to track it and I plan
> to use the power sequencing subsystem to handle the cases where
> non-exclusive access to GPIOs is required.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Some like drivers/fsi/fsi-master-aspeed.c seem to be just a bug,
are all non-regulator users bugs?

In general though:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij