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

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

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