* [PATCH v4 1/4] gpio: shared-proxy: Fix race between concurrent first users
2026-10-17 4:36 [PATCH v4 0/4] gpio: shared-proxy: Fix line accounting Shawn Guo
@ 2026-10-17 4:36 ` Shawn Guo
2026-10-20 10:06 ` Bartosz Golaszewski
2026-10-17 4:36 ` [PATCH v4 2/4] gpio: shared-proxy: Drop the vote of a released proxy on write failure Shawn Guo
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Shawn Guo @ 2026-10-17 4:36 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski
Cc: linux-gpio, linux-arm-msm, linux-kernel, Shawn Guo
The proxy lets a consumer set the line direction and reset the
default value and vote tally only when usecnt == 1, i.e. when it is
the sole requester. But request() and direction_output() take the
descriptor mutex separately. When two consumers probe concurrently,
both request the line before either configures it, both see
usecnt == 2, both fall into the shared path, read back the still
unconfigured direction (input) and fail with -EPERM.
On Nord RRD the two DAC supplies are reg-fixed-voltage regulators
sharing tlmm GPIO 41. They probe asynchronously and intermittently
both fail, leaving the codecs without supplies:
reg-fixed-voltage regulator-3p3-dac: setup of GPIO (default) failed: -1
reg-fixed-voltage regulator-5p0-dac: setup of GPIO (default) failed: -1
Requesting the line is not evidence that a consumer has claimed it.
Instead, count proxies that have actually configured the line (set its
direction or voted for a value) in a new cfgcnt, with a per-proxy
configured flag, and allow (re)initialization only while no other
proxy has done so. A consumer obtained with GPIOD_ASIS that votes via
gpiod_set_value() without ever setting the direction counts as
configured, so a later user can never reset def_val/votecnt behind its
vote. The count is dropped when the proxy frees the line.
Fixes: e992d54c6f97 ("gpio: shared-proxy: implement the shared GPIO proxy driver")
Assisted-by: LLM
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
drivers/gpio/gpio-shared-proxy.c | 74 ++++++++++++++++++++++++++++----
drivers/gpio/gpiolib-shared.h | 1 +
2 files changed, 66 insertions(+), 9 deletions(-)
diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
index bc69b8729d19..0d1d724a6a37 100644
--- a/drivers/gpio/gpio-shared-proxy.c
+++ b/drivers/gpio/gpio-shared-proxy.c
@@ -21,9 +21,33 @@ struct gpio_shared_proxy_data {
struct gpio_chip gc;
struct gpio_shared_desc *shared_desc;
struct device *dev;
+ bool configured;
bool voted_change;
};
+/*
+ * A proxy may (re)initialize the line - set its direction and reset the
+ * default value and the vote tally - only while no other proxy has set the
+ * direction or voted for a value. Requesting the line is not enough: consumers
+ * may request concurrently, so counting requests lets every one of them
+ * conclude that it is not first.
+ */
+static bool gpio_shared_proxy_may_init(struct gpio_shared_proxy_data *proxy)
+{
+ unsigned int cfgcnt = proxy->shared_desc->cfgcnt;
+
+ return !cfgcnt || (cfgcnt == 1 && proxy->configured);
+}
+
+static void gpio_shared_proxy_mark_configured(struct gpio_shared_proxy_data *proxy)
+{
+ if (proxy->configured)
+ return;
+
+ proxy->configured = true;
+ proxy->shared_desc->cfgcnt++;
+}
+
static int
gpio_shared_proxy_set_unlocked(struct gpio_shared_proxy_data *proxy, int value)
{
@@ -117,6 +141,11 @@ static void gpio_shared_proxy_free(struct gpio_chip *gc, unsigned int offset)
proxy->shared_desc->usecnt--;
+ if (proxy->configured) {
+ proxy->configured = false;
+ shared_desc->cfgcnt--;
+ }
+
dev_dbg(proxy->dev, "Shared GPIO freed, number of users: %u\n",
proxy->shared_desc->usecnt);
}
@@ -131,7 +160,7 @@ static int gpio_shared_proxy_set_config(struct gpio_chip *gc,
guard(mutex)(&shared_desc->mutex);
- if (shared_desc->usecnt > 1) {
+ if (!gpio_shared_proxy_may_init(proxy)) {
if (shared_desc->cfg != cfg) {
dev_dbg(proxy->dev,
"Shared GPIO's configuration already set, accepting changes but users may conflict!!\n");
@@ -155,15 +184,21 @@ static int gpio_shared_proxy_direction_input(struct gpio_chip *gc,
struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
struct gpio_shared_desc *shared_desc = proxy->shared_desc;
struct gpio_desc *desc = shared_desc->desc;
- int dir;
+ int ret, dir;
guard(mutex)(&shared_desc->mutex);
- if (shared_desc->usecnt == 1) {
+ if (gpio_shared_proxy_may_init(proxy)) {
dev_dbg(proxy->dev,
- "Only one user of this shared GPIO, allowing to set direction to input\n");
+ "No other user configured this shared GPIO, allowing to set direction to input\n");
- return gpiod_direction_input(desc);
+ ret = gpiod_direction_input(desc);
+ if (ret)
+ return ret;
+
+ gpio_shared_proxy_mark_configured(proxy);
+
+ return 0;
}
dir = gpiod_get_direction(desc);
@@ -176,6 +211,8 @@ static int gpio_shared_proxy_direction_input(struct gpio_chip *gc,
return -EPERM;
}
+ gpio_shared_proxy_mark_configured(proxy);
+
return 0;
}
@@ -189,18 +226,20 @@ static int gpio_shared_proxy_direction_output(struct gpio_chip *gc,
guard(mutex)(&shared_desc->mutex);
- if (shared_desc->usecnt == 1) {
+ if (gpio_shared_proxy_may_init(proxy)) {
dev_dbg(proxy->dev,
- "Only one user of this shared GPIO, allowing to set direction to output with value '%s'\n",
+ "No other user configured this shared GPIO, allowing to set direction to output with value '%s'\n",
str_high_low(value));
ret = gpiod_direction_output(desc, value);
if (ret)
return ret;
+ /* Only this proxy can have voted, so the tally is ours to reset */
shared_desc->def_val = value;
shared_desc->votecnt = 0;
proxy->voted_change = false;
+ gpio_shared_proxy_mark_configured(proxy);
return 0;
}
@@ -215,7 +254,13 @@ static int gpio_shared_proxy_direction_output(struct gpio_chip *gc,
return -EPERM;
}
- return gpio_shared_proxy_set_unlocked(proxy, value);
+ ret = gpio_shared_proxy_set_unlocked(proxy, value);
+ if (ret)
+ return ret;
+
+ gpio_shared_proxy_mark_configured(proxy);
+
+ return 0;
}
static int gpio_shared_proxy_get_cansleep(struct gpio_chip *gc,
@@ -230,10 +275,21 @@ static int gpio_shared_proxy_set_cansleep(struct gpio_chip *gc,
unsigned int offset, int value)
{
struct gpio_shared_proxy_data *proxy = gpiochip_get_data(gc);
+ int ret;
guard(mutex)(&proxy->shared_desc->mutex);
- return gpio_shared_proxy_set_unlocked(proxy, value);
+ /*
+ * A GPIOD_ASIS consumer may vote without ever setting the direction;
+ * count it as configured so a later user can't reset its vote.
+ */
+ ret = gpio_shared_proxy_set_unlocked(proxy, value);
+ if (ret)
+ return ret;
+
+ gpio_shared_proxy_mark_configured(proxy);
+
+ return 0;
}
static int gpio_shared_proxy_get_direction(struct gpio_chip *gc,
diff --git a/drivers/gpio/gpiolib-shared.h b/drivers/gpio/gpiolib-shared.h
index 618756f6c6aa..ae10506b7029 100644
--- a/drivers/gpio/gpiolib-shared.h
+++ b/drivers/gpio/gpiolib-shared.h
@@ -41,6 +41,7 @@ struct gpio_shared_desc {
struct gpio_desc *desc;
unsigned long cfg;
unsigned int usecnt;
+ unsigned int cfgcnt;
unsigned int votecnt;
int def_val;
struct mutex mutex; /* serializes all proxy operations on this descriptor */
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v4 1/4] gpio: shared-proxy: Fix race between concurrent first users
2026-10-17 4:36 ` [PATCH v4 1/4] gpio: shared-proxy: Fix race between concurrent first users Shawn Guo
@ 2026-10-20 10:06 ` Bartosz Golaszewski
2026-10-28 6:57 ` Shawn Guo
0 siblings, 1 reply; 11+ messages in thread
From: Bartosz Golaszewski @ 2026-10-20 10:06 UTC (permalink / raw)
To: Shawn Guo
Cc: linux-gpio, linux-arm-msm, linux-kernel, Linus Walleij,
Bartosz Golaszewski
On Sat, 17 Oct 2026 06:36:02 +0200, Shawn Guo
<shengchao.guo@oss.qualcomm.com> said:
> The proxy lets a consumer set the line direction and reset the
> default value and vote tally only when usecnt == 1, i.e. when it is
> the sole requester. But request() and direction_output() take the
> descriptor mutex separately. When two consumers probe concurrently,
> both request the line before either configures it, both see
> usecnt == 2, both fall into the shared path, read back the still
> unconfigured direction (input) and fail with -EPERM.
>
> On Nord RRD the two DAC supplies are reg-fixed-voltage regulators
> sharing tlmm GPIO 41. They probe asynchronously and intermittently
> both fail, leaving the codecs without supplies:
>
> reg-fixed-voltage regulator-3p3-dac: setup of GPIO (default) failed: -1
> reg-fixed-voltage regulator-5p0-dac: setup of GPIO (default) failed: -1
>
> Requesting the line is not evidence that a consumer has claimed it.
> Instead, count proxies that have actually configured the line (set its
> direction or voted for a value) in a new cfgcnt, with a per-proxy
> configured flag, and allow (re)initialization only while no other
> proxy has done so. A consumer obtained with GPIOD_ASIS that votes via
> gpiod_set_value() without ever setting the direction counts as
> configured, so a later user can never reset def_val/votecnt behind its
> vote. The count is dropped when the proxy frees the line.
>
> Fixes: e992d54c6f97 ("gpio: shared-proxy: implement the shared GPIO proxy driver")
> Assisted-by: LLM
> Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> ---
> drivers/gpio/gpio-shared-proxy.c | 74 ++++++++++++++++++++++++++++----
> drivers/gpio/gpiolib-shared.h | 1 +
> 2 files changed, 66 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
> index bc69b8729d19..0d1d724a6a37 100644
> --- a/drivers/gpio/gpio-shared-proxy.c
> +++ b/drivers/gpio/gpio-shared-proxy.c
> @@ -21,9 +21,33 @@ struct gpio_shared_proxy_data {
> struct gpio_chip gc;
> struct gpio_shared_desc *shared_desc;
> struct device *dev;
> + bool configured;
> bool voted_change;
> };
>
> +/*
> + * A proxy may (re)initialize the line - set its direction and reset the
> + * default value and the vote tally - only while no other proxy has set the
> + * direction or voted for a value. Requesting the line is not enough: consumers
> + * may request concurrently, so counting requests lets every one of them
> + * conclude that it is not first.
> + */
> +static bool gpio_shared_proxy_may_init(struct gpio_shared_proxy_data *proxy)
> +{
> + unsigned int cfgcnt = proxy->shared_desc->cfgcnt;
> +
We expect the shared lock to be taken, so can you add lockdep assert here ...
> + return !cfgcnt || (cfgcnt == 1 && proxy->configured);
> +}
> +
> +static void gpio_shared_proxy_mark_configured(struct gpio_shared_proxy_data *proxy)
> +{
> + if (proxy->configured)
> + return;
... and here?
Thanks,
Bartosz
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v4 1/4] gpio: shared-proxy: Fix race between concurrent first users
2026-10-20 10:06 ` Bartosz Golaszewski
@ 2026-10-28 6:57 ` Shawn Guo
0 siblings, 0 replies; 11+ messages in thread
From: Shawn Guo @ 2026-10-28 6:57 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-gpio, linux-arm-msm, linux-kernel, Linus Walleij
On Tue, Oct 20, 2026 at 11:06:23AM +0100, Bartosz Golaszewski wrote:
> On Sat, 17 Oct 2026 06:36:02 +0200, Shawn Guo
> <shengchao.guo@oss.qualcomm.com> said:
> > The proxy lets a consumer set the line direction and reset the
> > default value and vote tally only when usecnt == 1, i.e. when it is
> > the sole requester. But request() and direction_output() take the
> > descriptor mutex separately. When two consumers probe concurrently,
> > both request the line before either configures it, both see
> > usecnt == 2, both fall into the shared path, read back the still
> > unconfigured direction (input) and fail with -EPERM.
> >
> > On Nord RRD the two DAC supplies are reg-fixed-voltage regulators
> > sharing tlmm GPIO 41. They probe asynchronously and intermittently
> > both fail, leaving the codecs without supplies:
> >
> > reg-fixed-voltage regulator-3p3-dac: setup of GPIO (default) failed: -1
> > reg-fixed-voltage regulator-5p0-dac: setup of GPIO (default) failed: -1
> >
> > Requesting the line is not evidence that a consumer has claimed it.
> > Instead, count proxies that have actually configured the line (set its
> > direction or voted for a value) in a new cfgcnt, with a per-proxy
> > configured flag, and allow (re)initialization only while no other
> > proxy has done so. A consumer obtained with GPIOD_ASIS that votes via
> > gpiod_set_value() without ever setting the direction counts as
> > configured, so a later user can never reset def_val/votecnt behind its
> > vote. The count is dropped when the proxy frees the line.
> >
> > Fixes: e992d54c6f97 ("gpio: shared-proxy: implement the shared GPIO proxy driver")
> > Assisted-by: LLM
> > Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> > ---
> > drivers/gpio/gpio-shared-proxy.c | 74 ++++++++++++++++++++++++++++----
> > drivers/gpio/gpiolib-shared.h | 1 +
> > 2 files changed, 66 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
> > index bc69b8729d19..0d1d724a6a37 100644
> > --- a/drivers/gpio/gpio-shared-proxy.c
> > +++ b/drivers/gpio/gpio-shared-proxy.c
> > @@ -21,9 +21,33 @@ struct gpio_shared_proxy_data {
> > struct gpio_chip gc;
> > struct gpio_shared_desc *shared_desc;
> > struct device *dev;
> > + bool configured;
> > bool voted_change;
> > };
> >
> > +/*
> > + * A proxy may (re)initialize the line - set its direction and reset the
> > + * default value and the vote tally - only while no other proxy has set the
> > + * direction or voted for a value. Requesting the line is not enough: consumers
> > + * may request concurrently, so counting requests lets every one of them
> > + * conclude that it is not first.
> > + */
> > +static bool gpio_shared_proxy_may_init(struct gpio_shared_proxy_data *proxy)
> > +{
> > + unsigned int cfgcnt = proxy->shared_desc->cfgcnt;
> > +
>
> We expect the shared lock to be taken, so can you add lockdep assert here ...
>
> > + return !cfgcnt || (cfgcnt == 1 && proxy->configured);
> > +}
> > +
> > +static void gpio_shared_proxy_mark_configured(struct gpio_shared_proxy_data *proxy)
> > +{
> > + if (proxy->configured)
> > + return;
>
> ... and here?
Sure, will add lockdep_assert_held() to both in v5.
Shawn
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 2/4] gpio: shared-proxy: Drop the vote of a released proxy on write failure
2026-10-17 4:36 [PATCH v4 0/4] gpio: shared-proxy: Fix line accounting Shawn Guo
2026-10-17 4:36 ` [PATCH v4 1/4] gpio: shared-proxy: Fix race between concurrent first users Shawn Guo
@ 2026-10-17 4:36 ` Shawn Guo
2026-10-17 4:36 ` [PATCH v4 3/4] gpio: shared-proxy: Drive the default value when no votes are cast Shawn Guo
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Shawn Guo @ 2026-10-17 4:36 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski
Cc: linux-gpio, linux-arm-msm, linux-kernel, Shawn Guo
When a proxy that voted for a value change frees the line, it calls
gpio_shared_proxy_set_unlocked() to withdraw its vote. If it is the
last voter and restoring the default value fails, that function
returns early, leaving votecnt incremented and voted_change set.
The consumer is gone, but its vote is not. The line is held at the
non-default value for all remaining users, since votecnt can never
drop to zero again. And the proxy data outlives the request, so
voted_change is still set when the line is requested again. If
another user has reinitialized the line in between, resetting
votecnt to 0, the stale proxy's next vote for the default value
decrements votecnt from 0 and underflows it.
Drop the vote unconditionally on release, after reporting the
failure, since the consumer is going away regardless.
Fixes: e992d54c6f97 ("gpio: shared-proxy: implement the shared GPIO proxy driver")
Assisted-by: LLM
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
drivers/gpio/gpio-shared-proxy.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
index 0d1d724a6a37..24241c981df1 100644
--- a/drivers/gpio/gpio-shared-proxy.c
+++ b/drivers/gpio/gpio-shared-proxy.c
@@ -134,9 +134,13 @@ static void gpio_shared_proxy_free(struct gpio_chip *gc, unsigned int offset)
if (proxy->voted_change) {
ret = gpio_shared_proxy_set_unlocked(proxy, shared_desc->def_val);
- if (ret)
+ if (ret) {
dev_err(proxy->dev,
"Failed to unset the shared GPIO value on release: %d\n", ret);
+ /* The consumer is going away regardless, drop its vote */
+ shared_desc->votecnt--;
+ proxy->voted_change = false;
+ }
}
proxy->shared_desc->usecnt--;
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v4 3/4] gpio: shared-proxy: Drive the default value when no votes are cast
2026-10-17 4:36 [PATCH v4 0/4] gpio: shared-proxy: Fix line accounting Shawn Guo
2026-10-17 4:36 ` [PATCH v4 1/4] gpio: shared-proxy: Fix race between concurrent first users Shawn Guo
2026-10-17 4:36 ` [PATCH v4 2/4] gpio: shared-proxy: Drop the vote of a released proxy on write failure Shawn Guo
@ 2026-10-17 4:36 ` Shawn Guo
2026-10-20 13:17 ` Bartosz Golaszewski
2026-10-17 4:36 ` [PATCH v4 4/4] gpio: shared-proxy: Drop the vote when switching to input Shawn Guo
2026-10-20 13:18 ` [PATCH v4 0/4] gpio: shared-proxy: Fix line accounting Bartosz Golaszewski
4 siblings, 1 reply; 11+ messages in thread
From: Shawn Guo @ 2026-10-17 4:36 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski
Cc: linux-gpio, linux-arm-msm, linux-kernel, Shawn Guo
A consumer that requests a shared line with GPIOD_ASIS never calls
direction_output(), so def_val keeps its initial value of 0. If the
line was left high, for instance by the bootloader, a later
gpiod_set_value(0) from that consumer matches def_val and the proxy
treats it as withdrawing a vote it never cast. It returns success
without writing the hardware, and the line stays high.
The consumer has no way to tell. Its request to drive the line low is
silently ignored until it first votes for high and then withdraws
that vote.
When the requested value is the default and no votes are
outstanding, write it to the hardware instead of returning early.
For lines configured via direction_output() this is a redundant
write of the value the line already has. With votes outstanding the
request remains a no-op, as the line must stay at the voted value.
Fixes: e992d54c6f97 ("gpio: shared-proxy: implement the shared GPIO proxy driver")
Assisted-by: LLM
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
drivers/gpio/gpio-shared-proxy.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
index 24241c981df1..456a3c3dbf71 100644
--- a/drivers/gpio/gpio-shared-proxy.c
+++ b/drivers/gpio/gpio-shared-proxy.c
@@ -81,9 +81,16 @@ gpio_shared_proxy_set_unlocked(struct gpio_shared_proxy_data *proxy, int value)
}
/* Desired value is the default. */
- if (!proxy->voted_change)
- /* We didn't vote for change previously, nothing to do. */
+ if (!proxy->voted_change) {
+ /*
+ * With no votes outstanding the line should be at the
+ * default, but a GPIOD_ASIS line may have been left at the
+ * opposite value. Drive it there.
+ */
+ if (!shared_desc->votecnt)
+ ret = gpiod_set_value_cansleep(desc, shared_desc->def_val);
goto out;
+ }
/* We previously voted for change. */
if (shared_desc->votecnt == 1) {
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v4 3/4] gpio: shared-proxy: Drive the default value when no votes are cast
2026-10-17 4:36 ` [PATCH v4 3/4] gpio: shared-proxy: Drive the default value when no votes are cast Shawn Guo
@ 2026-10-20 13:17 ` Bartosz Golaszewski
2026-10-28 7:01 ` Shawn Guo
0 siblings, 1 reply; 11+ messages in thread
From: Bartosz Golaszewski @ 2026-10-20 13:17 UTC (permalink / raw)
To: Shawn Guo
Cc: linux-gpio, linux-arm-msm, linux-kernel, Linus Walleij,
Bartosz Golaszewski
On Sat, 17 Oct 2026 06:36:04 +0200, Shawn Guo
<shengchao.guo@oss.qualcomm.com> said:
> A consumer that requests a shared line with GPIOD_ASIS never calls
> direction_output(), so def_val keeps its initial value of 0. If the
> line was left high, for instance by the bootloader, a later
> gpiod_set_value(0) from that consumer matches def_val and the proxy
> treats it as withdrawing a vote it never cast. It returns success
> without writing the hardware, and the line stays high.
>
> The consumer has no way to tell. Its request to drive the line low is
> silently ignored until it first votes for high and then withdraws
> that vote.
>
> When the requested value is the default and no votes are
> outstanding, write it to the hardware instead of returning early.
> For lines configured via direction_output() this is a redundant
> write of the value the line already has. With votes outstanding the
> request remains a no-op, as the line must stay at the voted value.
>
Wouldn't this only matter for a line requested as-is? We could avoid the
redundant write if we only did it for such lines instead of for all
unconditionally.
Bart
> Fixes: e992d54c6f97 ("gpio: shared-proxy: implement the shared GPIO proxy driver")
> Assisted-by: LLM
> Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> ---
> drivers/gpio/gpio-shared-proxy.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
> index 24241c981df1..456a3c3dbf71 100644
> --- a/drivers/gpio/gpio-shared-proxy.c
> +++ b/drivers/gpio/gpio-shared-proxy.c
> @@ -81,9 +81,16 @@ gpio_shared_proxy_set_unlocked(struct gpio_shared_proxy_data *proxy, int value)
> }
>
> /* Desired value is the default. */
> - if (!proxy->voted_change)
> - /* We didn't vote for change previously, nothing to do. */
> + if (!proxy->voted_change) {
> + /*
> + * With no votes outstanding the line should be at the
> + * default, but a GPIOD_ASIS line may have been left at the
> + * opposite value. Drive it there.
> + */
> + if (!shared_desc->votecnt)
> + ret = gpiod_set_value_cansleep(desc, shared_desc->def_val);
> goto out;
> + }
>
> /* We previously voted for change. */
> if (shared_desc->votecnt == 1) {
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v4 3/4] gpio: shared-proxy: Drive the default value when no votes are cast
2026-10-20 13:17 ` Bartosz Golaszewski
@ 2026-10-28 7:01 ` Shawn Guo
2026-10-28 9:22 ` Bartosz Golaszewski
0 siblings, 1 reply; 11+ messages in thread
From: Shawn Guo @ 2026-10-28 7:01 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-gpio, linux-arm-msm, linux-kernel, Linus Walleij
On Tue, Oct 20, 2026 at 09:17:00AM -0400, Bartosz Golaszewski wrote:
> On Sat, 17 Oct 2026 06:36:04 +0200, Shawn Guo
> <shengchao.guo@oss.qualcomm.com> said:
> > A consumer that requests a shared line with GPIOD_ASIS never calls
> > direction_output(), so def_val keeps its initial value of 0. If the
> > line was left high, for instance by the bootloader, a later
> > gpiod_set_value(0) from that consumer matches def_val and the proxy
> > treats it as withdrawing a vote it never cast. It returns success
> > without writing the hardware, and the line stays high.
> >
> > The consumer has no way to tell. Its request to drive the line low is
> > silently ignored until it first votes for high and then withdraws
> > that vote.
> >
> > When the requested value is the default and no votes are
> > outstanding, write it to the hardware instead of returning early.
> > For lines configured via direction_output() this is a redundant
> > write of the value the line already has. With votes outstanding the
> > request remains a no-op, as the line must stay at the voted value.
> >
>
> Wouldn't this only matter for a line requested as-is? We could avoid the
> redundant write if we only did it for such lines instead of for all
> unconditionally.
It also matters for a line configured via direction_output(), after
patch 2. When a proxy releases the line and restoring the default value
fails, gpio_shared_proxy_free() still drops its vote. The line is then
at the non-default value with votecnt == 0, so "no votes outstanding"
no longer implies "line is at the default" either way. The unconditional
write lets any remaining user that sets the default bring the line back.
Gating it on as-is lines would leave it stuck there.
The proxy also has no direct notion of as-is; it would need a flag in
gpio_shared_desc tracking whether def_val has ever been written to the
hardware, set and cleared in the init paths of direction_output() and
direction_input().
The write itself is cheap and rare: it only happens when a proxy that
hasn't voted sets the default while no votes are outstanding.
If you still prefer the gating, I can do that in v5 and keep the
free-failure case recoverable some other way. Otherwise I'll add the
reasoning above to the commit message.
Shawn
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 3/4] gpio: shared-proxy: Drive the default value when no votes are cast
2026-10-28 7:01 ` Shawn Guo
@ 2026-10-28 9:22 ` Bartosz Golaszewski
0 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2026-10-28 9:22 UTC (permalink / raw)
To: Shawn Guo
Cc: linux-gpio, linux-arm-msm, linux-kernel, Linus Walleij,
Bartosz Golaszewski
On Wed, 28 Oct 2026 08:01:01 +0100, Shawn Guo
<shengchao.guo@oss.qualcomm.com> said:
> On Tue, Oct 20, 2026 at 09:17:00AM -0400, Bartosz Golaszewski wrote:
>> On Sat, 17 Oct 2026 06:36:04 +0200, Shawn Guo
>> <shengchao.guo@oss.qualcomm.com> said:
>> > A consumer that requests a shared line with GPIOD_ASIS never calls
>> > direction_output(), so def_val keeps its initial value of 0. If the
>> > line was left high, for instance by the bootloader, a later
>> > gpiod_set_value(0) from that consumer matches def_val and the proxy
>> > treats it as withdrawing a vote it never cast. It returns success
>> > without writing the hardware, and the line stays high.
>> >
>> > The consumer has no way to tell. Its request to drive the line low is
>> > silently ignored until it first votes for high and then withdraws
>> > that vote.
>> >
>> > When the requested value is the default and no votes are
>> > outstanding, write it to the hardware instead of returning early.
>> > For lines configured via direction_output() this is a redundant
>> > write of the value the line already has. With votes outstanding the
>> > request remains a no-op, as the line must stay at the voted value.
>> >
>>
>> Wouldn't this only matter for a line requested as-is? We could avoid the
>> redundant write if we only did it for such lines instead of for all
>> unconditionally.
>
> It also matters for a line configured via direction_output(), after
> patch 2. When a proxy releases the line and restoring the default value
> fails, gpio_shared_proxy_free() still drops its vote. The line is then
> at the non-default value with votecnt == 0, so "no votes outstanding"
> no longer implies "line is at the default" either way. The unconditional
> write lets any remaining user that sets the default bring the line back.
> Gating it on as-is lines would leave it stuck there.
>
> The proxy also has no direct notion of as-is; it would need a flag in
> gpio_shared_desc tracking whether def_val has ever been written to the
> hardware, set and cleared in the init paths of direction_output() and
> direction_input().
>
> The write itself is cheap and rare: it only happens when a proxy that
> hasn't voted sets the default while no votes are outstanding.
>
> If you still prefer the gating, I can do that in v5 and keep the
> free-failure case recoverable some other way. Otherwise I'll add the
> reasoning above to the commit message.
>
No, that's alright, let's keep the write.
Bart
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 4/4] gpio: shared-proxy: Drop the vote when switching to input
2026-10-17 4:36 [PATCH v4 0/4] gpio: shared-proxy: Fix line accounting Shawn Guo
` (2 preceding siblings ...)
2026-10-17 4:36 ` [PATCH v4 3/4] gpio: shared-proxy: Drive the default value when no votes are cast Shawn Guo
@ 2026-10-17 4:36 ` Shawn Guo
2026-10-20 13:18 ` [PATCH v4 0/4] gpio: shared-proxy: Fix line accounting Bartosz Golaszewski
4 siblings, 0 replies; 11+ messages in thread
From: Shawn Guo @ 2026-10-17 4:36 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski
Cc: linux-gpio, linux-arm-msm, linux-kernel, Shawn Guo
A proxy that is the only one to have configured a shared line may
reinitialize it. When it sets the direction to output, the proxy
resets the vote tally, but when it sets the direction to input, it
leaves any vote it cast in place, although an input line carries no
value.
That vote outlives its meaning. When the consumer releases the line,
gpio_shared_proxy_free() tries to withdraw it by writing the default
value, which gpiolib refuses on an input line:
Failed to unset the shared GPIO value on release: -1
This happens, for instance, when a consumer votes for a change with
gpiod_set_value() after setting the line to output and then switches
it to input.
Clear the vote when the proxy reinitializes the line as input, as
the output path already does. No other proxy can hold a vote at that
point: voting marks a proxy configured, and a proxy that frees the
line drops its vote.
Fixes: e992d54c6f97 ("gpio: shared-proxy: implement the shared GPIO proxy driver")
Assisted-by: LLM
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
drivers/gpio/gpio-shared-proxy.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
index 456a3c3dbf71..d4680b35fac3 100644
--- a/drivers/gpio/gpio-shared-proxy.c
+++ b/drivers/gpio/gpio-shared-proxy.c
@@ -207,6 +207,9 @@ static int gpio_shared_proxy_direction_input(struct gpio_chip *gc,
if (ret)
return ret;
+ /* An input carries no value, drop any vote this proxy cast */
+ shared_desc->votecnt = 0;
+ proxy->voted_change = false;
gpio_shared_proxy_mark_configured(proxy);
return 0;
--
2.43.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v4 0/4] gpio: shared-proxy: Fix line accounting
2026-10-17 4:36 [PATCH v4 0/4] gpio: shared-proxy: Fix line accounting Shawn Guo
` (3 preceding siblings ...)
2026-10-17 4:36 ` [PATCH v4 4/4] gpio: shared-proxy: Drop the vote when switching to input Shawn Guo
@ 2026-10-20 13:18 ` Bartosz Golaszewski
4 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2026-10-20 13:18 UTC (permalink / raw)
To: Shawn Guo
Cc: linux-gpio, linux-arm-msm, linux-kernel, Linus Walleij,
Bartosz Golaszewski
On Sat, 17 Oct 2026 06:36:01 +0200, Shawn Guo
<shengchao.guo@oss.qualcomm.com> said:
> Four fixes for how the shared GPIO proxy accounts for users and
> votes.
>
> Patch 1 fixes a race between consumers that request a shared line
> concurrently. Each of them sees usecnt > 1, assumes another user has
> already configured the line, and fails with -EPERM. On Nord RRD this
> intermittently leaves both DAC supply regulators without their enable
> GPIO.
>
> Patch 2 fixes a stale vote left behind when a proxy releases the line
> and restoring the default value fails. The line stays at the
> non-default value for the remaining users, and votecnt can underflow
> when the line is requested again.
>
> Patch 3 fixes gpiod_set_value() to the default value doing nothing on
> a line that was requested with GPIOD_ASIS and left at the opposite
> value, e.g. by the bootloader.
>
> Patch 4 drops a proxy's vote when it switches the line to input, so
> releasing the line no longer fails trying to write the default value
> to an input
>
Thanks! Looks good but I have a comment on one patch and a question on
another.
Bart
^ permalink raw reply [flat|nested] 11+ messages in thread