All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] gpio: shared-proxy: Fix line accounting
@ 2026-10-17  2:43 Shawn Guo
  2026-10-17  2:43 ` [PATCH v3 1/3] gpio: shared-proxy: Fix race between concurrent first users Shawn Guo
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Shawn Guo @ 2026-10-17  2:43 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski
  Cc: linux-gpio, linux-arm-msm, linux-kernel, Shawn Guo

Three 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.

Changes in v3:
- Patch 3: New. Drive the default value to the hardware when no votes
  are outstanding. It predates patch 1 (Sashiko)
- Link to v2: https://lore.kernel.org/all/20261016144919.1362846-1-shengchao.guo@oss.qualcomm.com/

Changes in v2:
- Patch 1: Also use gpio_shared_proxy_may_init() instead of
  usecnt > 1 in set_config, so concurrent first users no longer skip
  the hardware config or log a spurious conflict (Sashiko)
- Patch 2: New. Fix the votecnt underflow on a failed write in free.
  It predates patch 1 (Sashiko)
- Link to v1: https://lore.kernel.org/all/20261015122740.971792-1-shengchao.guo@oss.qualcomm.com/

Shawn Guo (3):
  gpio: shared-proxy: Fix race between concurrent first users
  gpio: shared-proxy: Drop the vote of a released proxy on write failure
  gpio: shared-proxy: Drive the default value when no votes are cast

 drivers/gpio/gpio-shared-proxy.c | 91 +++++++++++++++++++++++++++-----
 drivers/gpio/gpiolib-shared.h    |  1 +
 2 files changed, 80 insertions(+), 12 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/3] gpio: shared-proxy: Fix race between concurrent first users
  2026-10-17  2:43 [PATCH v3 0/3] gpio: shared-proxy: Fix line accounting Shawn Guo
@ 2026-10-17  2:43 ` Shawn Guo
  2026-10-17  3:00   ` sashiko-bot
  2026-10-17  2:43 ` [PATCH v3 2/3] gpio: shared-proxy: Drop the vote of a released proxy on write failure Shawn Guo
  2026-10-17  2:43 ` [PATCH v3 3/3] gpio: shared-proxy: Drive the default value when no votes are cast Shawn Guo
  2 siblings, 1 reply; 6+ messages in thread
From: Shawn Guo @ 2026-10-17  2:43 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 related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/3] gpio: shared-proxy: Drop the vote of a released proxy on write failure
  2026-10-17  2:43 [PATCH v3 0/3] gpio: shared-proxy: Fix line accounting Shawn Guo
  2026-10-17  2:43 ` [PATCH v3 1/3] gpio: shared-proxy: Fix race between concurrent first users Shawn Guo
@ 2026-10-17  2:43 ` Shawn Guo
  2026-10-17  2:43 ` [PATCH v3 3/3] gpio: shared-proxy: Drive the default value when no votes are cast Shawn Guo
  2 siblings, 0 replies; 6+ messages in thread
From: Shawn Guo @ 2026-10-17  2:43 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 related	[flat|nested] 6+ messages in thread

* [PATCH v3 3/3] gpio: shared-proxy: Drive the default value when no votes are cast
  2026-10-17  2:43 [PATCH v3 0/3] gpio: shared-proxy: Fix line accounting Shawn Guo
  2026-10-17  2:43 ` [PATCH v3 1/3] gpio: shared-proxy: Fix race between concurrent first users Shawn Guo
  2026-10-17  2:43 ` [PATCH v3 2/3] gpio: shared-proxy: Drop the vote of a released proxy on write failure Shawn Guo
@ 2026-10-17  2:43 ` Shawn Guo
  2 siblings, 0 replies; 6+ messages in thread
From: Shawn Guo @ 2026-10-17  2:43 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 related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/3] gpio: shared-proxy: Fix race between concurrent first users
  2026-10-17  2:43 ` [PATCH v3 1/3] gpio: shared-proxy: Fix race between concurrent first users Shawn Guo
@ 2026-10-17  3:00   ` sashiko-bot
  2026-10-17  4:07     ` Shawn Guo
  0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-10-17  3:00 UTC (permalink / raw)
  To: Shawn Guo; +Cc: linux-gpio

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] gpio: shared-proxy: unreset vote state in gpio_shared_proxy_direction_input()
--

commit f02d19c15e193895e840a218407e46f54481d01c
Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>

gpio: shared-proxy: Fix race between concurrent first users

This commit fixes a race between concurrent first users of a shared GPIO
by tracking the number of proxies that have configured the line (via
direction setting or voting) in cfgcnt rather than checking usecnt == 1.

> diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
> index bc69b8729d196..0d1d724a6a376 100644
> --- a/drivers/gpio/gpio-shared-proxy.c
> +++ b/drivers/gpio/gpio-shared-proxy.c

[ ... ]

> @@ -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");
> +
> +		ret = gpiod_direction_input(desc);
> +		if (ret)
> +			return ret;
> +
> +		gpio_shared_proxy_mark_configured(proxy);
>  
> -		return gpiod_direction_input(desc);
> +		return 0;
>  	}

[Severity: Medium]
Should gpio_shared_proxy_direction_input() also reset shared_desc->votecnt
and proxy->voted_change here, just like gpio_shared_proxy_direction_output()
does when gpio_shared_proxy_may_init(proxy) is true?

If a proxy becomes the sole configured user (shared_desc->cfgcnt == 1 and
proxy->configured == true) while holding an active vote (proxy->voted_change
== true and shared_desc->votecnt == 1) -- for example, after calling
gpiod_direction_output() with 0 followed by gpiod_set_value_cansleep() with
1, after voting as a GPIOD_ASIS consumer, or after another consumer that
initially set the direction to output-low releases its descriptor -- and
then reconfigures the line as input via gpio_shared_proxy_direction_input(),
gpiod_direction_input(desc) clears GPIOD_FLAG_IS_OUT on shared_desc->desc
while leaving proxy->voted_change == true and shared_desc->votecnt == 1.

When the proxy is later released in gpio_shared_proxy_free():

drivers/gpio/gpio-shared-proxy.c:gpio_shared_proxy_free() {
	if (proxy->voted_change) {
		ret = gpio_shared_proxy_set_unlocked(proxy, shared_desc->def_val);
		if (ret)
			dev_err(proxy->dev,
				"Failed to unset the shared GPIO value on release: %d\n", ret);
	}
}

gpio_shared_proxy_set_unlocked() -> gpiod_set_value_cansleep() ->
gpiod_set_raw_value_commit() will see GPIOD_FLAG_IS_OUT cleared on the
input descriptor, fail with -EPERM, and log an error (and at this commit,
prior to commit "gpio: shared-proxy: Drop the vote of a released proxy on
write failure", leave shared_desc->votecnt and proxy->voted_change leaked).

[ ... ]

> @@ -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;
>  	}

[ ... ]

>
>
> -- 
> 2.47.3
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017024335.1432020-1-shengchao.guo@oss.qualcomm.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/3] gpio: shared-proxy: Fix race between concurrent first users
  2026-10-17  3:00   ` sashiko-bot
@ 2026-10-17  4:07     ` Shawn Guo
  0 siblings, 0 replies; 6+ messages in thread
From: Shawn Guo @ 2026-10-17  4:07 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-gpio

On Sat, Oct 17, 2026 at 03:00:22AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] gpio: shared-proxy: unreset vote state in gpio_shared_proxy_direction_input()
> --
> 
> commit f02d19c15e193895e840a218407e46f54481d01c
> Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> 
> gpio: shared-proxy: Fix race between concurrent first users
> 
> This commit fixes a race between concurrent first users of a shared GPIO
> by tracking the number of proxies that have configured the line (via
> direction setting or voting) in cfgcnt rather than checking usecnt == 1.
> 
> > diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
> > index bc69b8729d196..0d1d724a6a376 100644
> > --- a/drivers/gpio/gpio-shared-proxy.c
> > +++ b/drivers/gpio/gpio-shared-proxy.c
> 
> [ ... ]
> 
> > @@ -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");
> > +
> > +		ret = gpiod_direction_input(desc);
> > +		if (ret)
> > +			return ret;
> > +
> > +		gpio_shared_proxy_mark_configured(proxy);
> >  
> > -		return gpiod_direction_input(desc);
> > +		return 0;
> >  	}
> 
> [Severity: Medium]
> Should gpio_shared_proxy_direction_input() also reset shared_desc->votecnt
> and proxy->voted_change here, just like gpio_shared_proxy_direction_output()
> does when gpio_shared_proxy_may_init(proxy) is true?
> 
> If a proxy becomes the sole configured user (shared_desc->cfgcnt == 1 and
> proxy->configured == true) while holding an active vote (proxy->voted_change
> == true and shared_desc->votecnt == 1) -- for example, after calling
> gpiod_direction_output() with 0 followed by gpiod_set_value_cansleep() with
> 1, after voting as a GPIOD_ASIS consumer, or after another consumer that
> initially set the direction to output-low releases its descriptor -- and
> then reconfigures the line as input via gpio_shared_proxy_direction_input(),
> gpiod_direction_input(desc) clears GPIOD_FLAG_IS_OUT on shared_desc->desc
> while leaving proxy->voted_change == true and shared_desc->votecnt == 1.
> 
> When the proxy is later released in gpio_shared_proxy_free():
> 
> drivers/gpio/gpio-shared-proxy.c:gpio_shared_proxy_free() {
> 	if (proxy->voted_change) {
> 		ret = gpio_shared_proxy_set_unlocked(proxy, shared_desc->def_val);
> 		if (ret)
> 			dev_err(proxy->dev,
> 				"Failed to unset the shared GPIO value on release: %d\n", ret);
> 	}
> }
> 
> gpio_shared_proxy_set_unlocked() -> gpiod_set_value_cansleep() ->
> gpiod_set_raw_value_commit() will see GPIOD_FLAG_IS_OUT cleared on the
> input descriptor, fail with -EPERM, and log an error (and at this commit,
> prior to commit "gpio: shared-proxy: Drop the vote of a released proxy on
> write failure", leave shared_desc->votecnt and proxy->voted_change leaked).

Agreed, but this predates the patch too. The original driver's
direction_input() also left voted_high/highcnt alone when it was the
sole user; this patch only changes the condition guarding that branch.
With patch 2 applied, the vote no longer leaks on the failed write in
free, so what remains is the spurious error on release.

I'll add a separate patch in v4 that clears the vote when the proxy
reinitializes the line as input, as the output path does. Resetting
votecnt to 0 there is safe: under gpio_shared_proxy_may_init() no other
proxy is configured, voting marks a proxy configured, and a proxy that
frees the line drops its vote.

Shawn

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-17  4:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-17  2:43 [PATCH v3 0/3] gpio: shared-proxy: Fix line accounting Shawn Guo
2026-10-17  2:43 ` [PATCH v3 1/3] gpio: shared-proxy: Fix race between concurrent first users Shawn Guo
2026-10-17  3:00   ` sashiko-bot
2026-10-17  4:07     ` Shawn Guo
2026-10-17  2:43 ` [PATCH v3 2/3] gpio: shared-proxy: Drop the vote of a released proxy on write failure Shawn Guo
2026-10-17  2:43 ` [PATCH v3 3/3] gpio: shared-proxy: Drive the default value when no votes are cast Shawn Guo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.