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

[Shawn Guo <shengchao.guo@oss.qualcomm.com>]

On Fri, Oct 16, 2026 at 02:58:22PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] gpio: shared: set_value(0) is a no-op on ASIS lines driven HIGH
> --
> 
> commit b2e913f9e7adbb02eb24728f50149f4b95e20857
> Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>
> 
> gpio: shared-proxy: Fix race between concurrent first users
> 
> This commit fixes a race condition where multiple consumers probing concurrently
> can both request a shared GPIO before either configures it, causing both to fail.
> It tracks the number of proxies that have actually configured the line, allowing
> initialization only when no other proxy has done so.
> 
> > 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
> 
> [ ... ]
> 
> > @@ -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);
> 
> [Severity: Medium]
> If a consumer requests a shared GPIO with GPIOD_ASIS and the physical line
> is already driven HIGH, does shared_desc->def_val still default to 0?
> 
> When the consumer later calls gpiod_set_value(0) to drive the line LOW,
> gpio_shared_proxy_set_unlocked() checks if value != shared_desc->def_val.
> Since both value and def_val are 0, it skips calling
> gpiod_set_value_cansleep() and returns success without changing the
> hardware state.
> 
> Does this result in set_value(0) acting as a no-op on ASIS lines that are
> initially driven HIGH?

Yes, set_value(0) is a no-op on an ASIS line that starts out high, but
the behaviour isn't new. def_val is only set by direction_output(), so
an ASIS consumer always sees the zero-initialized default. Its
set_value(0) then looks like withdrawing a vote it never cast. That
was already the case before this patch: the original driver treated
low as the default unconditionally, and b30973e8c392 ("gpio: shared:
make the voting mechanism adaptable") kept that for ASIS consumers.
This patch only marks the proxy as configured after the vote and
doesn't change which values reach the hardware.

I'll fix it in a separate patch: when the requested value is the
default and no votes are outstanding, write it to the hardware instead
of returning early. With votes outstanding the request stays a no-op,
as it should.

Shawn