On Thu, Aug 03, 2023 at 11:42:12AM +0200, Uwe Kleine-König wrote: > On Sun, Jul 30, 2023 at 12:07:33PM +0200, Bartosz Golaszewski wrote: ... > - the locking scheme in gpiod_request_commit() looks strange. gpio_lock > is released and retaken possibly several times. I wonder what it > actually protects there. Maybe doing > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index edab00c9cb3c..496b1cebba58 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -2064,13 +2064,11 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label) > goto out_free_unlock; > } > } > + spin_unlock_irqrestore(&gpio_lock, flags); > if (gc->get_direction) { > /* gc->get_direction may sleep */ > - spin_unlock_irqrestore(&gpio_lock, flags); > gpiod_get_direction(desc); > - spin_lock_irqsave(&gpio_lock, flags); > } > - spin_unlock_irqrestore(&gpio_lock, flags); > return 0; > > out_free_unlock: > > simplifies the code and given that gpiod_get_direction() rechecks > gc->get_direction unlocked I don't think we'd loose anything here. Wouldn't this break sleeping bus accesses (I2C gpio expanders, etc)? -- With Best Regards, Andy Shevchenko