[PATCH v2 0/2] gpio: convert more drivers to using the new value setters

[PATCH v2 0/2] gpio: convert more drivers to using the new value setters

[Bartosz Golaszewski]

I don't want to spam people's inboxes so I'm resending just the patches
that caused build warnings. The other ones will be picked up from v1.

New variants of set() and set_multiple() callbacks allow drivers to
indicate failures back to callers. Convert more GPIO drivers to using
them as the old ones are now deprecated.

This series also contains some additional refactoring to the affected
drivers wherever it makes the conversion easier.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
Changes in v2:
- drop patches that don't trigger build bot warnings
- rework the gpio-adnp patch: we need to initialize variables modified
  within scoped_guard() as it's implemented using a for loop and
  triggers warnings due to the compiler thinking we're using variables
  that may be uninitialized
- Link to v1: https://lore.kernel.org/r/20250303-gpiochip-set-conversion-v1-0-1d5cceeebf8b@linaro.org

---
Bartosz Golaszewski (2):
      gpio: adnp: use lock guards for the I2C lock
      gpio: adnp: use new line value setter callbacks

 drivers/gpio/gpio-adnp.c | 134 +++++++++++++++++++----------------------------
 1 file changed, 55 insertions(+), 79 deletions(-)
---
base-commit: 7ec162622e66a4ff886f8f28712ea1b13069e1aa
change-id: 20250303-gpiochip-set-conversion-736fb6548975

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

[PATCH v2 1/2] gpio: adnp: use lock guards for the I2C lock

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reduce the code complexity by using automatic lock guards with the I2C
mutex.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpio-adnp.c | 122 +++++++++++++++++++----------------------------
 1 file changed, 49 insertions(+), 73 deletions(-)

diff --git a/drivers/gpio/gpio-adnp.c b/drivers/gpio/gpio-adnp.c
index 6dafab0cf964..1a282cba8489 100644
--- a/drivers/gpio/gpio-adnp.c
+++ b/drivers/gpio/gpio-adnp.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2011-2012 Avionic Design GmbH
  */
 
+#include <linux/cleanup.h>
 #include <linux/gpio/driver.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
@@ -101,9 +102,9 @@ static void adnp_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 {
 	struct adnp *adnp = gpiochip_get_data(chip);
 
-	mutex_lock(&adnp->i2c_lock);
+	guard(mutex)(&adnp->i2c_lock);
+
 	__adnp_gpio_set(adnp, offset, value);
-	mutex_unlock(&adnp->i2c_lock);
 }
 
 static int adnp_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
@@ -114,32 +115,26 @@ static int adnp_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
 	u8 value;
 	int err;
 
-	mutex_lock(&adnp->i2c_lock);
+	guard(mutex)(&adnp->i2c_lock);
 
 	err = adnp_read(adnp, GPIO_DDR(adnp) + reg, &value);
 	if (err < 0)
-		goto out;
+		return err;
 
 	value &= ~BIT(pos);
 
 	err = adnp_write(adnp, GPIO_DDR(adnp) + reg, value);
 	if (err < 0)
-		goto out;
+		return err;
 
 	err = adnp_read(adnp, GPIO_DDR(adnp) + reg, &value);
 	if (err < 0)
-		goto out;
+		return err;
 
-	if (value & BIT(pos)) {
-		err = -EPERM;
-		goto out;
-	}
+	if (value & BIT(pos))
+		return -EPERM;
 
-	err = 0;
-
-out:
-	mutex_unlock(&adnp->i2c_lock);
-	return err;
+	return 0;
 }
 
 static int adnp_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
@@ -151,33 +146,28 @@ static int adnp_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
 	int err;
 	u8 val;
 
-	mutex_lock(&adnp->i2c_lock);
+	guard(mutex)(&adnp->i2c_lock);
 
 	err = adnp_read(adnp, GPIO_DDR(adnp) + reg, &val);
 	if (err < 0)
-		goto out;
+		return err;
 
 	val |= BIT(pos);
 
 	err = adnp_write(adnp, GPIO_DDR(adnp) + reg, val);
 	if (err < 0)
-		goto out;
+		return err;
 
 	err = adnp_read(adnp, GPIO_DDR(adnp) + reg, &val);
 	if (err < 0)
-		goto out;
+		return err;
 
-	if (!(val & BIT(pos))) {
-		err = -EPERM;
-		goto out;
-	}
+	if (!(val & BIT(pos)))
+		return -EPERM;
 
 	__adnp_gpio_set(adnp, offset, value);
-	err = 0;
 
-out:
-	mutex_unlock(&adnp->i2c_lock);
-	return err;
+	return 0;
 }
 
 static void adnp_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
@@ -187,27 +177,26 @@ static void adnp_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 	int err;
 
 	for (i = 0; i < num_regs; i++) {
-		u8 ddr, plr, ier, isr;
+		u8 ddr = 0, plr = 0, ier = 0, isr = 0;
 
-		mutex_lock(&adnp->i2c_lock);
+		scoped_guard(mutex, &adnp->i2c_lock) {
+			err = adnp_read(adnp, GPIO_DDR(adnp) + i, &ddr);
+			if (err < 0)
+				return;
 
-		err = adnp_read(adnp, GPIO_DDR(adnp) + i, &ddr);
-		if (err < 0)
-			goto unlock;
+			err = adnp_read(adnp, GPIO_PLR(adnp) + i, &plr);
+			if (err < 0)
+				return;
 
-		err = adnp_read(adnp, GPIO_PLR(adnp) + i, &plr);
-		if (err < 0)
-			goto unlock;
+			err = adnp_read(adnp, GPIO_IER(adnp) + i, &ier);
+			if (err < 0)
+				return;
 
-		err = adnp_read(adnp, GPIO_IER(adnp) + i, &ier);
-		if (err < 0)
-			goto unlock;
+			err = adnp_read(adnp, GPIO_ISR(adnp) + i, &isr);
+			if (err < 0)
+				return;
 
-		err = adnp_read(adnp, GPIO_ISR(adnp) + i, &isr);
-		if (err < 0)
-			goto unlock;
-
-		mutex_unlock(&adnp->i2c_lock);
+		}
 
 		for (j = 0; j < 8; j++) {
 			unsigned int bit = (i << adnp->reg_shift) + j;
@@ -232,11 +221,6 @@ static void adnp_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 				   direction, level, interrupt, pending);
 		}
 	}
-
-	return;
-
-unlock:
-	mutex_unlock(&adnp->i2c_lock);
 }
 
 static irqreturn_t adnp_irq(int irq, void *data)
@@ -248,32 +232,24 @@ static irqreturn_t adnp_irq(int irq, void *data)
 
 	for (i = 0; i < num_regs; i++) {
 		unsigned int base = i << adnp->reg_shift, bit;
-		u8 changed, level, isr, ier;
+		u8 changed, level = 0, isr = 0, ier = 0;
 		unsigned long pending;
 		int err;
 
-		mutex_lock(&adnp->i2c_lock);
+		scoped_guard(mutex, &adnp->i2c_lock) {
+			err = adnp_read(adnp, GPIO_PLR(adnp) + i, &level);
+			if (err < 0)
+				continue;
 
-		err = adnp_read(adnp, GPIO_PLR(adnp) + i, &level);
-		if (err < 0) {
-			mutex_unlock(&adnp->i2c_lock);
-			continue;
+			err = adnp_read(adnp, GPIO_ISR(adnp) + i, &isr);
+			if (err < 0)
+				continue;
+
+			err = adnp_read(adnp, GPIO_IER(adnp) + i, &ier);
+			if (err < 0)
+				continue;
 		}
 
-		err = adnp_read(adnp, GPIO_ISR(adnp) + i, &isr);
-		if (err < 0) {
-			mutex_unlock(&adnp->i2c_lock);
-			continue;
-		}
-
-		err = adnp_read(adnp, GPIO_IER(adnp) + i, &ier);
-		if (err < 0) {
-			mutex_unlock(&adnp->i2c_lock);
-			continue;
-		}
-
-		mutex_unlock(&adnp->i2c_lock);
-
 		/* determine pins that changed levels */
 		changed = level ^ adnp->irq_level[i];
 
@@ -365,12 +341,12 @@ static void adnp_irq_bus_unlock(struct irq_data *d)
 	struct adnp *adnp = gpiochip_get_data(gc);
 	unsigned int num_regs = 1 << adnp->reg_shift, i;
 
-	mutex_lock(&adnp->i2c_lock);
+	scoped_guard(mutex, &adnp->i2c_lock) {
+		for (i = 0; i < num_regs; i++)
+			adnp_write(adnp, GPIO_IER(adnp) + i,
+				   adnp->irq_enable[i]);
+	}
 
-	for (i = 0; i < num_regs; i++)
-		adnp_write(adnp, GPIO_IER(adnp) + i, adnp->irq_enable[i]);
-
-	mutex_unlock(&adnp->i2c_lock);
 	mutex_unlock(&adnp->irq_lock);
 }
 

-- 
2.45.2

Re: [PATCH v2 1/2] gpio: adnp: use lock guards for the I2C lock

[Linus Walleij]

On Thu, Mar 6, 2025 at 6:19 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Reduce the code complexity by using automatic lock guards with the I2C
> mutex.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v2 1/2] gpio: adnp: use lock guards for the I2C lock

[David Lechner]

On 3/6/25 11:19 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Reduce the code complexity by using automatic lock guards with the I2C
> mutex.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---

...

> @@ -248,32 +232,24 @@ static irqreturn_t adnp_irq(int irq, void *data)
>  
>  	for (i = 0; i < num_regs; i++) {
>  		unsigned int base = i << adnp->reg_shift, bit;
> -		u8 changed, level, isr, ier;
> +		u8 changed, level = 0, isr = 0, ier = 0;
>  		unsigned long pending;
>  		int err;
>  
> -		mutex_lock(&adnp->i2c_lock);
> +		scoped_guard(mutex, &adnp->i2c_lock) {
> +			err = adnp_read(adnp, GPIO_PLR(adnp) + i, &level);
> +			if (err < 0)
> +				continue;

I think this unintentionally changes the flow here. scoped_guard() is a
for loop, so the continue now breaks out of the scoped_guard() rather
than the outer for loop.

>  
> -		err = adnp_read(adnp, GPIO_PLR(adnp) + i, &level);
> -		if (err < 0) {
> -			mutex_unlock(&adnp->i2c_lock);
> -			continue;
> +			err = adnp_read(adnp, GPIO_ISR(adnp) + i, &isr);
> +			if (err < 0)
> +				continue;
> +
> +			err = adnp_read(adnp, GPIO_IER(adnp) + i, &ier);
> +			if (err < 0)
> +				continue;
>  		}
>  
> -		err = adnp_read(adnp, GPIO_ISR(adnp) + i, &isr);
> -		if (err < 0) {
> -			mutex_unlock(&adnp->i2c_lock);
> -			continue;
> -		}
> -
> -		err = adnp_read(adnp, GPIO_IER(adnp) + i, &ier);
> -		if (err < 0) {
> -			mutex_unlock(&adnp->i2c_lock);
> -			continue;
> -		}
> -
> -		mutex_unlock(&adnp->i2c_lock);
> -
>  		/* determine pins that changed levels */
>  		changed = level ^ adnp->irq_level[i];
>  

Re: [PATCH v2 1/2] gpio: adnp: use lock guards for the I2C lock

[Bartosz Golaszewski]

On Fri, May 22, 2026 at 2:46 AM David Lechner <dlechner@baylibre.com> wrote:
>
> On 3/6/25 11:19 AM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Reduce the code complexity by using automatic lock guards with the I2C
> > mutex.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
>
> ...
>
> > @@ -248,32 +232,24 @@ static irqreturn_t adnp_irq(int irq, void *data)
> >
> >       for (i = 0; i < num_regs; i++) {
> >               unsigned int base = i << adnp->reg_shift, bit;
> > -             u8 changed, level, isr, ier;
> > +             u8 changed, level = 0, isr = 0, ier = 0;
> >               unsigned long pending;
> >               int err;
> >
> > -             mutex_lock(&adnp->i2c_lock);
> > +             scoped_guard(mutex, &adnp->i2c_lock) {
> > +                     err = adnp_read(adnp, GPIO_PLR(adnp) + i, &level);
> > +                     if (err < 0)
> > +                             continue;
>
> I think this unintentionally changes the flow here. scoped_guard() is a
> for loop, so the continue now breaks out of the scoped_guard() rather
> than the outer for loop.
>

Thanks for catching it, I sent a fix.

Bart

[PATCH v2 2/2] gpio: adnp: use new line value setter callbacks

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

struct gpio_chip now has callbacks for setting line values that return
an integer, allowing to indicate failures. Convert the driver to using
them.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpio-adnp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-adnp.c b/drivers/gpio/gpio-adnp.c
index 1a282cba8489..df2710fa2a1a 100644
--- a/drivers/gpio/gpio-adnp.c
+++ b/drivers/gpio/gpio-adnp.c
@@ -79,7 +79,7 @@ static int adnp_gpio_get(struct gpio_chip *chip, unsigned offset)
 	return (value & BIT(pos)) ? 1 : 0;
 }
 
-static void __adnp_gpio_set(struct adnp *adnp, unsigned offset, int value)
+static int __adnp_gpio_set(struct adnp *adnp, unsigned int offset, int value)
 {
 	unsigned int reg = offset >> adnp->reg_shift;
 	unsigned int pos = offset & 7;
@@ -88,23 +88,23 @@ static void __adnp_gpio_set(struct adnp *adnp, unsigned offset, int value)
 
 	err = adnp_read(adnp, GPIO_PLR(adnp) + reg, &val);
 	if (err < 0)
-		return;
+		return err;
 
 	if (value)
 		val |= BIT(pos);
 	else
 		val &= ~BIT(pos);
 
-	adnp_write(adnp, GPIO_PLR(adnp) + reg, val);
+	return adnp_write(adnp, GPIO_PLR(adnp) + reg, val);
 }
 
-static void adnp_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+static int adnp_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
 {
 	struct adnp *adnp = gpiochip_get_data(chip);
 
 	guard(mutex)(&adnp->i2c_lock);
 
-	__adnp_gpio_set(adnp, offset, value);
+	return __adnp_gpio_set(adnp, offset, value);
 }
 
 static int adnp_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
@@ -429,7 +429,7 @@ static int adnp_gpio_setup(struct adnp *adnp, unsigned int num_gpios,
 	chip->direction_input = adnp_gpio_direction_input;
 	chip->direction_output = adnp_gpio_direction_output;
 	chip->get = adnp_gpio_get;
-	chip->set = adnp_gpio_set;
+	chip->set_rv = adnp_gpio_set;
 	chip->can_sleep = true;
 
 	if (IS_ENABLED(CONFIG_DEBUG_FS))

-- 
2.45.2

Re: [PATCH v2 2/2] gpio: adnp: use new line value setter callbacks

[Linus Walleij]

On Thu, Mar 6, 2025 at 6:19 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. Convert the driver to using
> them.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v2 0/2] gpio: convert more drivers to using the new value setters

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Thu, 06 Mar 2025 18:19:25 +0100, Bartosz Golaszewski wrote:
> I don't want to spam people's inboxes so I'm resending just the patches
> that caused build warnings. The other ones will be picked up from v1.
> 
> New variants of set() and set_multiple() callbacks allow drivers to
> indicate failures back to callers. Convert more GPIO drivers to using
> them as the old ones are now deprecated.
> 
> [...]

Applied, thanks!

[1/2] gpio: adnp: use lock guards for the I2C lock
      commit: c7fe19ed39730c121449bdae11e030f02c7071a8
[2/2] gpio: adnp: use new line value setter callbacks
      commit: 21c853ad93097619c7966542e838c54c37f57c90

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>