Re: [PATCH v5 1/4] gpio: shared-proxy: Fix race between concurrent first users

[Marek Szyprowski]

On 28.10.2026 13:34, Shawn Guo wrote:
> 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")
> Signed-off-by: Shawn Guo <shengchao.guo@xxxxxxxxxxxxxxxx>

I've hit the same issue on RUBIK Pi3 board, see:

https://lore.kernel.org/all/20261028222803.2682808-1-m.szyprowski@xxxxxxxxxxx/

This patch fixes it, so feel free to add:

Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>


> ---
>  drivers/gpio/gpio-shared-proxy.c | 78 ++++++++++++++++++++++++++++----
>  drivers/gpio/gpiolib-shared.h    |  1 +
>  2 files changed, 70 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpio/gpio-shared-proxy.c b/drivers/gpio/gpio-shared-proxy.c
> index bc69b8729d19..2e9ccd8a8d83 100644
> --- a/drivers/gpio/gpio-shared-proxy.c
> +++ b/drivers/gpio/gpio-shared-proxy.c
> @@ -21,9 +21,37 @@ 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;
> +
> +	lockdep_assert_held(&proxy->shared_desc->mutex);
> +
> +	return !cfgcnt || (cfgcnt == 1 && proxy->configured);
> +}
> +
> +static void gpio_shared_proxy_mark_configured(struct gpio_shared_proxy_data *proxy)
> +{
> +	lockdep_assert_held(&proxy->shared_desc->mutex);
> +
> +	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 +145,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 +164,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 +188,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 +215,8 @@ static int gpio_shared_proxy_direction_input(struct gpio_chip *gc,
>  		return -EPERM;
>  	}
>  
> +	gpio_shared_proxy_mark_configured(proxy);
> +
>  	return 0;
>  }
>  
> @@ -189,18 +230,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 +258,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 +279,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 */

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland