Re: [PATCH 18/18] gpio: mvebu: Make use of devm_pwmchip_alloc() function

[Andy Shevchenko <andy@kernel.org>]

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