[PATCH 0/7] iio: convert GPIO chips to using new value setters

[PATCH 0/7] iio: convert GPIO chips to using new value setters

[Bartosz Golaszewski]

struct gpio_chip now has callbacks for setting line values that return
an integer, allowing to indicate failures. We're in the process of
converting all GPIO drivers to using the new API. This series converts
all the IIO GPIO controllers and also contains some additional
refactoring patches for ad5592r in preparation for the conversion.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
Bartosz Golaszewski (7):
      iio: dac: ad5592r: destroy mutexes in detach paths
      iio: dac: ad5592r: use lock guards
      iio: dac: ad5592r: use new GPIO line value setter callbacks
      iio: adc: ti-ads7950: use new GPIO line value setter callbacks
      iio: adc: ad4130: use new GPIO line value setter callbacks
      iio: addac: ad74413r: use new GPIO line value setter callbacks
      iio: addac: ad74115: use new GPIO line value setter callbacks

 drivers/iio/adc/ad4130.c       |  10 ++--
 drivers/iio/adc/ti-ads7950.c   |  17 ++++--
 drivers/iio/addac/ad74115.c    |  18 +++---
 drivers/iio/addac/ad74413r.c   |  28 ++++-----
 drivers/iio/dac/ad5592r-base.c | 132 +++++++++++++++++------------------------
 5 files changed, 95 insertions(+), 110 deletions(-)
---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250401-gpiochip-set-rv-iio-b064ce43791d

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

[PATCH 1/7] iio: dac: ad5592r: destroy mutexes in detach paths

[Bartosz Golaszewski]

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

The locks used here are initialized but never released which causes
resource leaks with mutex debugging enabled. Add missing calls to
mutex_destroy() or use devres if applicable.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/iio/dac/ad5592r-base.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
index 50d19304bacb..fe4c35689d4d 100644
--- a/drivers/iio/dac/ad5592r-base.c
+++ b/drivers/iio/dac/ad5592r-base.c
@@ -155,6 +155,8 @@ static void ad5592r_gpio_cleanup(struct ad5592r_state *st)
 {
 	if (st->gpio_map)
 		gpiochip_remove(&st->gpiochip);
+
+	mutex_destroy(&st->gpio_lock);
 }
 
 static int ad5592r_reset(struct ad5592r_state *st)
@@ -622,7 +624,9 @@ int ad5592r_probe(struct device *dev, const char *name,
 	iio_dev->info = &ad5592r_info;
 	iio_dev->modes = INDIO_DIRECT_MODE;
 
-	mutex_init(&st->lock);
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		goto error_disable_reg;
 
 	ad5592r_init_scales(st, ad5592r_get_vref(st));
 

-- 
2.45.2

[PATCH 2/7] iio: dac: ad5592r: use lock guards

[Bartosz Golaszewski]

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

Use lock guards from linux/cleanup.h to simplify the code and remove
some labels.

Note that we need to initialize some variables even though it's not
technically required as scoped_guards() are implemented as for loops.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/iio/dac/ad5592r-base.c | 119 ++++++++++++++++-------------------------
 1 file changed, 47 insertions(+), 72 deletions(-)

diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
index fe4c35689d4d..bbe3b52c6a12 100644
--- a/drivers/iio/dac/ad5592r-base.c
+++ b/drivers/iio/dac/ad5592r-base.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/iio/iio.h>
 #include <linux/module.h>
@@ -24,16 +25,14 @@ static int ad5592r_gpio_get(struct gpio_chip *chip, unsigned offset)
 {
 	struct ad5592r_state *st = gpiochip_get_data(chip);
 	int ret = 0;
-	u8 val;
+	u8 val = 0;
 
-	mutex_lock(&st->gpio_lock);
-
-	if (st->gpio_out & BIT(offset))
-		val = st->gpio_val;
-	else
-		ret = st->ops->gpio_read(st, &val);
-
-	mutex_unlock(&st->gpio_lock);
+	scoped_guard(mutex, &st->gpio_lock) {
+		if (st->gpio_out & BIT(offset))
+			val = st->gpio_val;
+		else
+			ret = st->ops->gpio_read(st, &val);
+	}
 
 	if (ret < 0)
 		return ret;
@@ -45,7 +44,7 @@ static void ad5592r_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 {
 	struct ad5592r_state *st = gpiochip_get_data(chip);
 
-	mutex_lock(&st->gpio_lock);
+	guard(mutex)(&st->gpio_lock);
 
 	if (value)
 		st->gpio_val |= BIT(offset);
@@ -53,8 +52,6 @@ static void ad5592r_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 		st->gpio_val &= ~BIT(offset);
 
 	st->ops->reg_write(st, AD5592R_REG_GPIO_SET, st->gpio_val);
-
-	mutex_unlock(&st->gpio_lock);
 }
 
 static int ad5592r_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
@@ -62,21 +59,16 @@ static int ad5592r_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
 	struct ad5592r_state *st = gpiochip_get_data(chip);
 	int ret;
 
-	mutex_lock(&st->gpio_lock);
+	guard(mutex)(&st->gpio_lock);
 
 	st->gpio_out &= ~BIT(offset);
 	st->gpio_in |= BIT(offset);
 
 	ret = st->ops->reg_write(st, AD5592R_REG_GPIO_OUT_EN, st->gpio_out);
 	if (ret < 0)
-		goto err_unlock;
+		return ret;
 
-	ret = st->ops->reg_write(st, AD5592R_REG_GPIO_IN_EN, st->gpio_in);
-
-err_unlock:
-	mutex_unlock(&st->gpio_lock);
-
-	return ret;
+	return st->ops->reg_write(st, AD5592R_REG_GPIO_IN_EN, st->gpio_in);
 }
 
 static int ad5592r_gpio_direction_output(struct gpio_chip *chip,
@@ -85,7 +77,7 @@ static int ad5592r_gpio_direction_output(struct gpio_chip *chip,
 	struct ad5592r_state *st = gpiochip_get_data(chip);
 	int ret;
 
-	mutex_lock(&st->gpio_lock);
+	guard(mutex)(&st->gpio_lock);
 
 	if (value)
 		st->gpio_val |= BIT(offset);
@@ -97,18 +89,13 @@ static int ad5592r_gpio_direction_output(struct gpio_chip *chip,
 
 	ret = st->ops->reg_write(st, AD5592R_REG_GPIO_SET, st->gpio_val);
 	if (ret < 0)
-		goto err_unlock;
+		return ret;
 
 	ret = st->ops->reg_write(st, AD5592R_REG_GPIO_OUT_EN, st->gpio_out);
 	if (ret < 0)
-		goto err_unlock;
+		return ret;
 
-	ret = st->ops->reg_write(st, AD5592R_REG_GPIO_IN_EN, st->gpio_in);
-
-err_unlock:
-	mutex_unlock(&st->gpio_lock);
-
-	return ret;
+	return st->ops->reg_write(st, AD5592R_REG_GPIO_IN_EN, st->gpio_in);
 }
 
 static int ad5592r_gpio_request(struct gpio_chip *chip, unsigned offset)
@@ -171,10 +158,9 @@ static int ad5592r_reset(struct ad5592r_state *st)
 		udelay(1);
 		gpiod_set_value(gpio, 1);
 	} else {
-		mutex_lock(&st->lock);
-		/* Writing this magic value resets the device */
-		st->ops->reg_write(st, AD5592R_REG_RESET, 0xdac);
-		mutex_unlock(&st->lock);
+		scoped_guard(mutex, &st->lock)
+			/* Writing this magic value resets the device */
+			st->ops->reg_write(st, AD5592R_REG_RESET, 0xdac);
 	}
 
 	udelay(250);
@@ -249,45 +235,43 @@ static int ad5592r_set_channel_modes(struct ad5592r_state *st)
 		}
 	}
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	/* Pull down unused pins to GND */
 	ret = ops->reg_write(st, AD5592R_REG_PULLDOWN, pulldown);
 	if (ret)
-		goto err_unlock;
+		return ret;
 
 	ret = ops->reg_write(st, AD5592R_REG_TRISTATE, tristate);
 	if (ret)
-		goto err_unlock;
+		return ret;
 
 	/* Configure pins that we use */
 	ret = ops->reg_write(st, AD5592R_REG_DAC_EN, dac);
 	if (ret)
-		goto err_unlock;
+		return ret;
 
 	ret = ops->reg_write(st, AD5592R_REG_ADC_EN, adc);
 	if (ret)
-		goto err_unlock;
+		return ret;
 
 	ret = ops->reg_write(st, AD5592R_REG_GPIO_SET, st->gpio_val);
 	if (ret)
-		goto err_unlock;
+		return ret;
 
 	ret = ops->reg_write(st, AD5592R_REG_GPIO_OUT_EN, st->gpio_out);
 	if (ret)
-		goto err_unlock;
+		return ret;
 
 	ret = ops->reg_write(st, AD5592R_REG_GPIO_IN_EN, st->gpio_in);
 	if (ret)
-		goto err_unlock;
+		return ret;
 
 	/* Verify that we can read back at least one register */
 	ret = ops->reg_read(st, AD5592R_REG_ADC_EN, &read_back);
 	if (!ret && (read_back & 0xff) != adc)
 		ret = -EIO;
 
-err_unlock:
-	mutex_unlock(&st->lock);
 	return ret;
 }
 
@@ -316,11 +300,10 @@ static int ad5592r_write_raw(struct iio_dev *iio_dev,
 		if (!chan->output)
 			return -EINVAL;
 
-		mutex_lock(&st->lock);
+		guard(mutex)(&st->lock);
 		ret = st->ops->write_dac(st, chan->channel, val);
 		if (!ret)
 			st->cached_dac[chan->channel] = val;
-		mutex_unlock(&st->lock);
 		return ret;
 	case IIO_CHAN_INFO_SCALE:
 		if (chan->type == IIO_VOLTAGE) {
@@ -335,7 +318,7 @@ static int ad5592r_write_raw(struct iio_dev *iio_dev,
 			else
 				return -EINVAL;
 
-			mutex_lock(&st->lock);
+			guard(mutex)(&st->lock);
 
 			ret = st->ops->reg_read(st, AD5592R_REG_CTRL,
 						&st->cached_gp_ctrl);
@@ -360,11 +343,8 @@ static int ad5592r_write_raw(struct iio_dev *iio_dev,
 						~AD5592R_REG_CTRL_ADC_RANGE;
 			}
 
-			ret = st->ops->reg_write(st, AD5592R_REG_CTRL,
-						 st->cached_gp_ctrl);
-			mutex_unlock(&st->lock);
-
-			return ret;
+			return st->ops->reg_write(st, AD5592R_REG_CTRL,
+						  st->cached_gp_ctrl);
 		}
 		break;
 	default:
@@ -379,15 +359,15 @@ static int ad5592r_read_raw(struct iio_dev *iio_dev,
 			   int *val, int *val2, long m)
 {
 	struct ad5592r_state *st = iio_priv(iio_dev);
-	u16 read_val;
-	int ret, mult;
+	u16 read_val = 0;
+	int ret = 0, mult = 0;
 
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
 		if (!chan->output) {
-			mutex_lock(&st->lock);
-			ret = st->ops->read_adc(st, chan->channel, &read_val);
-			mutex_unlock(&st->lock);
+			scoped_guard(mutex, &st->lock)
+				ret = st->ops->read_adc(st, chan->channel,
+							&read_val);
 			if (ret)
 				return ret;
 
@@ -400,9 +380,8 @@ static int ad5592r_read_raw(struct iio_dev *iio_dev,
 			read_val &= GENMASK(11, 0);
 
 		} else {
-			mutex_lock(&st->lock);
-			read_val = st->cached_dac[chan->channel];
-			mutex_unlock(&st->lock);
+			scoped_guard(mutex, &st->lock)
+				read_val = st->cached_dac[chan->channel];
 		}
 
 		dev_dbg(st->dev, "Channel %u read: 0x%04hX\n",
@@ -420,16 +399,14 @@ static int ad5592r_read_raw(struct iio_dev *iio_dev,
 			return IIO_VAL_INT_PLUS_NANO;
 		}
 
-		mutex_lock(&st->lock);
-
-		if (chan->output)
-			mult = !!(st->cached_gp_ctrl &
-				AD5592R_REG_CTRL_DAC_RANGE);
-		else
-			mult = !!(st->cached_gp_ctrl &
-				AD5592R_REG_CTRL_ADC_RANGE);
-
-		mutex_unlock(&st->lock);
+		scoped_guard(mutex, &st->lock) {
+			if (chan->output)
+				mult = !!(st->cached_gp_ctrl &
+					AD5592R_REG_CTRL_DAC_RANGE);
+			else
+				mult = !!(st->cached_gp_ctrl &
+					AD5592R_REG_CTRL_ADC_RANGE);
+		}
 
 		*val *= ++mult;
 
@@ -439,15 +416,13 @@ static int ad5592r_read_raw(struct iio_dev *iio_dev,
 	case IIO_CHAN_INFO_OFFSET:
 		ret = ad5592r_get_vref(st);
 
-		mutex_lock(&st->lock);
+		guard(mutex)(&st->lock);
 
 		if (st->cached_gp_ctrl & AD5592R_REG_CTRL_ADC_RANGE)
 			*val = (-34365 * 25) / ret;
 		else
 			*val = (-75365 * 25) / ret;
 
-		mutex_unlock(&st->lock);
-
 		return IIO_VAL_INT;
 	default:
 		return -EINVAL;

-- 
2.45.2

[PATCH 3/7] iio: dac: ad5592r: use new GPIO 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/iio/dac/ad5592r-base.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
index bbe3b52c6a12..ad650dcd73a0 100644
--- a/drivers/iio/dac/ad5592r-base.c
+++ b/drivers/iio/dac/ad5592r-base.c
@@ -40,7 +40,8 @@ static int ad5592r_gpio_get(struct gpio_chip *chip, unsigned offset)
 	return !!(val & BIT(offset));
 }
 
-static void ad5592r_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+static int ad5592r_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			    int value)
 {
 	struct ad5592r_state *st = gpiochip_get_data(chip);
 
@@ -51,7 +52,7 @@ static void ad5592r_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 	else
 		st->gpio_val &= ~BIT(offset);
 
-	st->ops->reg_write(st, AD5592R_REG_GPIO_SET, st->gpio_val);
+	return st->ops->reg_write(st, AD5592R_REG_GPIO_SET, st->gpio_val);
 }
 
 static int ad5592r_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
@@ -128,7 +129,7 @@ static int ad5592r_gpio_init(struct ad5592r_state *st)
 	st->gpiochip.direction_input = ad5592r_gpio_direction_input;
 	st->gpiochip.direction_output = ad5592r_gpio_direction_output;
 	st->gpiochip.get = ad5592r_gpio_get;
-	st->gpiochip.set = ad5592r_gpio_set;
+	st->gpiochip.set_rv = ad5592r_gpio_set;
 	st->gpiochip.request = ad5592r_gpio_request;
 	st->gpiochip.owner = THIS_MODULE;
 	st->gpiochip.names = ad5592r_gpio_names;

-- 
2.45.2

[PATCH 4/7] iio: adc: ti-ads7950: use new GPIO 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/iio/adc/ti-ads7950.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index af28672aa803..0356ccf23fea 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -403,10 +403,11 @@ static const struct iio_info ti_ads7950_info = {
 	.update_scan_mode	= ti_ads7950_update_scan_mode,
 };
 
-static void ti_ads7950_set(struct gpio_chip *chip, unsigned int offset,
-			   int value)
+static int ti_ads7950_set(struct gpio_chip *chip, unsigned int offset,
+			  int value)
 {
 	struct ti_ads7950_state *st = gpiochip_get_data(chip);
+	int ret;
 
 	mutex_lock(&st->slock);
 
@@ -416,9 +417,11 @@ static void ti_ads7950_set(struct gpio_chip *chip, unsigned int offset,
 		st->cmd_settings_bitmask &= ~BIT(offset);
 
 	st->single_tx = TI_ADS7950_MAN_CMD_SETTINGS(st);
-	spi_sync(st->spi, &st->scan_single_msg);
+	ret = spi_sync(st->spi, &st->scan_single_msg);
 
 	mutex_unlock(&st->slock);
+
+	return ret;
 }
 
 static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
@@ -499,7 +502,11 @@ static int ti_ads7950_direction_input(struct gpio_chip *chip,
 static int ti_ads7950_direction_output(struct gpio_chip *chip,
 				       unsigned int offset, int value)
 {
-	ti_ads7950_set(chip, offset, value);
+	int ret;
+
+	ret = ti_ads7950_set(chip, offset, value);
+	if (ret)
+		return ret;
 
 	return _ti_ads7950_set_direction(chip, offset, 0);
 }
@@ -641,7 +648,7 @@ static int ti_ads7950_probe(struct spi_device *spi)
 	st->chip.direction_input = ti_ads7950_direction_input;
 	st->chip.direction_output = ti_ads7950_direction_output;
 	st->chip.get = ti_ads7950_get;
-	st->chip.set = ti_ads7950_set;
+	st->chip.set_rv = ti_ads7950_set;
 
 	ret = gpiochip_add_data(&st->chip, st);
 	if (ret) {

-- 
2.45.2

[PATCH 5/7] iio: adc: ad4130: use new GPIO 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/iio/adc/ad4130.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/ad4130.c b/drivers/iio/adc/ad4130.c
index 0f4c9cd6c102..6cf790ff3eb5 100644
--- a/drivers/iio/adc/ad4130.c
+++ b/drivers/iio/adc/ad4130.c
@@ -522,15 +522,15 @@ static int ad4130_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
 	return GPIO_LINE_DIRECTION_OUT;
 }
 
-static void ad4130_gpio_set(struct gpio_chip *gc, unsigned int offset,
-			    int value)
+static int ad4130_gpio_set(struct gpio_chip *gc, unsigned int offset,
+			   int value)
 {
 	struct ad4130_state *st = gpiochip_get_data(gc);
 	unsigned int mask = FIELD_PREP(AD4130_IO_CONTROL_GPIO_DATA_MASK,
 				       BIT(offset));
 
-	regmap_update_bits(st->regmap, AD4130_IO_CONTROL_REG, mask,
-			   value ? mask : 0);
+	return regmap_update_bits(st->regmap, AD4130_IO_CONTROL_REG, mask,
+				  value ? mask : 0);
 }
 
 static int ad4130_set_mode(struct ad4130_state *st, enum ad4130_mode mode)
@@ -2064,7 +2064,7 @@ static int ad4130_probe(struct spi_device *spi)
 	st->gc.can_sleep = true;
 	st->gc.init_valid_mask = ad4130_gpio_init_valid_mask;
 	st->gc.get_direction = ad4130_gpio_get_direction;
-	st->gc.set = ad4130_gpio_set;
+	st->gc.set_rv = ad4130_gpio_set;
 
 	ret = devm_gpiochip_add_data(dev, &st->gc, st);
 	if (ret)

-- 
2.45.2

[PATCH 6/7] iio: addac: ad74413r: use new GPIO 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/iio/addac/ad74413r.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
index f14d12b03da6..adfa14c4b06f 100644
--- a/drivers/iio/addac/ad74413r.c
+++ b/drivers/iio/addac/ad74413r.c
@@ -276,8 +276,8 @@ static int ad74413r_set_comp_drive_strength(struct ad74413r_state *st,
 }
 
 
-static void ad74413r_gpio_set(struct gpio_chip *chip,
-			      unsigned int offset, int val)
+static int ad74413r_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			     int val)
 {
 	struct ad74413r_state *st = gpiochip_get_data(chip);
 	unsigned int real_offset = st->gpo_gpio_offsets[offset];
@@ -286,16 +286,16 @@ static void ad74413r_gpio_set(struct gpio_chip *chip,
 	ret = ad74413r_set_gpo_config(st, real_offset,
 				      AD74413R_GPO_CONFIG_LOGIC);
 	if (ret)
-		return;
+		return ret;
 
-	regmap_update_bits(st->regmap, AD74413R_REG_GPO_CONFIG_X(real_offset),
-			   AD74413R_GPO_CONFIG_DATA_MASK,
-			   val ? AD74413R_GPO_CONFIG_DATA_MASK : 0);
+	return regmap_update_bits(st->regmap,
+				  AD74413R_REG_GPO_CONFIG_X(real_offset),
+				  AD74413R_GPO_CONFIG_DATA_MASK,
+				  val ? AD74413R_GPO_CONFIG_DATA_MASK : 0);
 }
 
-static void ad74413r_gpio_set_multiple(struct gpio_chip *chip,
-				       unsigned long *mask,
-				       unsigned long *bits)
+static int ad74413r_gpio_set_multiple(struct gpio_chip *chip,
+				      unsigned long *mask, unsigned long *bits)
 {
 	struct ad74413r_state *st = gpiochip_get_data(chip);
 	unsigned long real_mask = 0;
@@ -309,15 +309,15 @@ static void ad74413r_gpio_set_multiple(struct gpio_chip *chip,
 		ret = ad74413r_set_gpo_config(st, real_offset,
 			AD74413R_GPO_CONFIG_LOGIC_PARALLEL);
 		if (ret)
-			return;
+			return ret;
 
 		real_mask |= BIT(real_offset);
 		if (*bits & offset)
 			real_bits |= BIT(real_offset);
 	}
 
-	regmap_update_bits(st->regmap, AD74413R_REG_GPO_PAR_DATA,
-			   real_mask, real_bits);
+	return regmap_update_bits(st->regmap, AD74413R_REG_GPO_PAR_DATA,
+				  real_mask, real_bits);
 }
 
 static int ad74413r_gpio_get(struct gpio_chip *chip, unsigned int offset)
@@ -1424,8 +1424,8 @@ static int ad74413r_probe(struct spi_device *spi)
 		st->gpo_gpiochip.ngpio = st->num_gpo_gpios;
 		st->gpo_gpiochip.parent = st->dev;
 		st->gpo_gpiochip.can_sleep = true;
-		st->gpo_gpiochip.set = ad74413r_gpio_set;
-		st->gpo_gpiochip.set_multiple = ad74413r_gpio_set_multiple;
+		st->gpo_gpiochip.set_rv = ad74413r_gpio_set;
+		st->gpo_gpiochip.set_multiple_rv = ad74413r_gpio_set_multiple;
 		st->gpo_gpiochip.set_config = ad74413r_gpio_set_gpo_config;
 		st->gpo_gpiochip.get_direction =
 			ad74413r_gpio_get_gpo_direction;

-- 
2.45.2

[PATCH 7/7] iio: addac: ad74115: use new GPIO 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/iio/addac/ad74115.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/addac/ad74115.c b/drivers/iio/addac/ad74115.c
index a7e480f2472d..2a809e07526b 100644
--- a/drivers/iio/addac/ad74115.c
+++ b/drivers/iio/addac/ad74115.c
@@ -542,18 +542,16 @@ static int ad74115_gpio_get(struct gpio_chip *gc, unsigned int offset)
 	return FIELD_GET(AD74115_GPIO_CONFIG_GPI_DATA, val);
 }
 
-static void ad74115_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
+static int ad74115_gpio_set(struct gpio_chip *gc, unsigned int offset,
+			    int value)
 {
 	struct ad74115_state *st = gpiochip_get_data(gc);
-	struct device *dev = &st->spi->dev;
-	int ret;
 
-	ret = regmap_update_bits(st->regmap, AD74115_GPIO_CONFIG_X_REG(offset),
-				 AD74115_GPIO_CONFIG_GPO_DATA,
-				 FIELD_PREP(AD74115_GPIO_CONFIG_GPO_DATA, value));
-	if (ret)
-		dev_err(dev, "Failed to set GPIO %u output value, err: %d\n",
-			offset, ret);
+	return regmap_update_bits(st->regmap,
+				  AD74115_GPIO_CONFIG_X_REG(offset),
+				  AD74115_GPIO_CONFIG_GPO_DATA,
+				  FIELD_PREP(AD74115_GPIO_CONFIG_GPO_DATA,
+					     value));
 }
 
 static int ad74115_set_comp_debounce(struct ad74115_state *st, unsigned int val)
@@ -1580,7 +1578,7 @@ static int ad74115_setup_gpio_chip(struct ad74115_state *st)
 		.direction_input = ad74115_gpio_direction_input,
 		.direction_output = ad74115_gpio_direction_output,
 		.get = ad74115_gpio_get,
-		.set = ad74115_gpio_set,
+		.set_rv = ad74115_gpio_set,
 	};
 
 	return devm_gpiochip_add_data(dev, &st->gc, st);

-- 
2.45.2

Re: [PATCH 1/7] iio: dac: ad5592r: destroy mutexes in detach paths

[Jonathan Cameron]

On Mon, 07 Apr 2025 09:18:09 +0200
Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> The locks used here are initialized but never released which causes
> resource leaks with mutex debugging enabled. Add missing calls to
> mutex_destroy() or use devres if applicable.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Hi Bartosz,

> ---
>  drivers/iio/dac/ad5592r-base.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
> index 50d19304bacb..fe4c35689d4d 100644
> --- a/drivers/iio/dac/ad5592r-base.c
> +++ b/drivers/iio/dac/ad5592r-base.c
> @@ -155,6 +155,8 @@ static void ad5592r_gpio_cleanup(struct ad5592r_state *st)
>  {
>  	if (st->gpio_map)
>  		gpiochip_remove(&st->gpiochip);
> +
> +	mutex_destroy(&st->gpio_lock);
>  }
>  
>  static int ad5592r_reset(struct ad5592r_state *st)
> @@ -622,7 +624,9 @@ int ad5592r_probe(struct device *dev, const char *name,
>  	iio_dev->info = &ad5592r_info;
>  	iio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	mutex_init(&st->lock);
> +	ret = devm_mutex_init(dev, &st->lock);
> +	if (ret)
> +		goto error_disable_reg;

Please don't mix devm and gotos.  That tends to make for complex
logic to review for limited gain.   Devm then gotos is fine though
(i.e. a transition from all devm to gotos only mid way through probe).

Easiest solution would be to move this mutex init before the regulator
is enabled (so up about 10 lines).

Then we finish the devm stuff before starting the non devm part.

A more complex cleanup would be to drop the support for dynamic vref
(as that is almost certainly copied and pasted from elsewhere rather than
a real thing) and then use devm to manage the vref.  That wouldn't
be related to this series though so I'd just move that mutex init up.


>  
>  	ad5592r_init_scales(st, ad5592r_get_vref(st));
>  
> 

Re: [PATCH 2/7] iio: dac: ad5592r: use lock guards

[Jonathan Cameron]

On Mon, 07 Apr 2025 09:18:10 +0200
Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Use lock guards from linux/cleanup.h to simplify the code and remove
> some labels.
> 
> Note that we need to initialize some variables even though it's not
> technically required as scoped_guards() are implemented as for loops.
That is always a tiny bit irritating. Thanks for calling it out.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/iio/dac/ad5592r-base.c | 119 ++++++++++++++++-------------------------
>  1 file changed, 47 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
> index fe4c35689d4d..bbe3b52c6a12 100644
> --- a/drivers/iio/dac/ad5592r-base.c
> +++ b/drivers/iio/dac/ad5592r-base.c

Just one case I'd prefer done a tiny bit differently so as to alway do
early returns rather than very nearly always.


>  	udelay(250);
> @@ -249,45 +235,43 @@ static int ad5592r_set_channel_modes(struct ad5592r_state *st)
>  		}
>  	}
>  
> -	mutex_lock(&st->lock);
> +	guard(mutex)(&st->lock);
>  
>  	/* Pull down unused pins to GND */
>  	ret = ops->reg_write(st, AD5592R_REG_PULLDOWN, pulldown);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	ret = ops->reg_write(st, AD5592R_REG_TRISTATE, tristate);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	/* Configure pins that we use */
>  	ret = ops->reg_write(st, AD5592R_REG_DAC_EN, dac);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	ret = ops->reg_write(st, AD5592R_REG_ADC_EN, adc);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	ret = ops->reg_write(st, AD5592R_REG_GPIO_SET, st->gpio_val);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	ret = ops->reg_write(st, AD5592R_REG_GPIO_OUT_EN, st->gpio_out);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	ret = ops->reg_write(st, AD5592R_REG_GPIO_IN_EN, st->gpio_in);
>  	if (ret)
> -		goto err_unlock;
> +		return ret;
>  
>  	/* Verify that we can read back at least one register */
>  	ret = ops->reg_read(st, AD5592R_REG_ADC_EN, &read_back);
>  	if (!ret && (read_back & 0xff) != adc)
>  		ret = -EIO;
		return -EIO; 
for this one.

>  
> -err_unlock:
> -	mutex_unlock(&st->lock);
>  	return ret;
return 0; here

>  }

Re: [PATCH 0/7] iio: convert GPIO chips to using new value setters

[Jonathan Cameron]

On Mon, 07 Apr 2025 09:18:08 +0200
Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. We're in the process of
> converting all GPIO drivers to using the new API. This series converts
> all the IIO GPIO controllers and also contains some additional
> refactoring patches for ad5592r in preparation for the conversion.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Nice.  All the _rv patches look good to me. Just a few minor things
on the precursors.  One other driver crossed with this, but Matti has
posted a patch updating that one as well.

https://lore.kernel.org/all/Z_N_J52IZ2IaWawl@mva-rohm/

Thanks,

Jonathan

> ---
> Bartosz Golaszewski (7):
>       iio: dac: ad5592r: destroy mutexes in detach paths
>       iio: dac: ad5592r: use lock guards
>       iio: dac: ad5592r: use new GPIO line value setter callbacks
>       iio: adc: ti-ads7950: use new GPIO line value setter callbacks
>       iio: adc: ad4130: use new GPIO line value setter callbacks
>       iio: addac: ad74413r: use new GPIO line value setter callbacks
>       iio: addac: ad74115: use new GPIO line value setter callbacks
> 
>  drivers/iio/adc/ad4130.c       |  10 ++--
>  drivers/iio/adc/ti-ads7950.c   |  17 ++++--
>  drivers/iio/addac/ad74115.c    |  18 +++---
>  drivers/iio/addac/ad74413r.c   |  28 ++++-----
>  drivers/iio/dac/ad5592r-base.c | 132 +++++++++++++++++------------------------
>  5 files changed, 95 insertions(+), 110 deletions(-)
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250401-gpiochip-set-rv-iio-b064ce43791d
> 
> Best regards,

Re: [PATCH 2/7] iio: dac: ad5592r: use lock guards

[kernel test robot]

Hi Bartosz,

kernel test robot noticed the following build errors:

[auto build test ERROR on 0af2f6be1b4281385b618cb86ad946eded089ac8]

url:    https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/iio-dac-ad5592r-destroy-mutexes-in-detach-paths/20250407-152721
base:   0af2f6be1b4281385b618cb86ad946eded089ac8
patch link:    https://lore.kernel.org/r/20250407-gpiochip-set-rv-iio-v1-2-8431b003a145%40linaro.org
patch subject: [PATCH 2/7] iio: dac: ad5592r: use lock guards
config: arm-randconfig-001-20250408 (https://download.01.org/0day-ci/archive/20250408/202504081058.aukPDkTg-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 92c93f5286b9ff33f27ff694d2dc33da1c07afdd)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250408/202504081058.aukPDkTg-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504081058.aukPDkTg-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/iio/dac/ad5592r-base.c:350:2: error: cannot jump from switch statement to this case label
     350 |         default:
         |         ^
   drivers/iio/dac/ad5592r-base.c:303:3: note: jump bypasses initialization of variable with __attribute__((cleanup))
     303 |                 guard(mutex)(&st->lock);
         |                 ^
   include/linux/cleanup.h:319:15: note: expanded from macro 'guard'
     319 |         CLASS(_name, __UNIQUE_ID(guard))
         |                      ^
   include/linux/compiler.h:166:29: note: expanded from macro '__UNIQUE_ID'
     166 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^
   include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE'
      84 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^
   include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE'
      83 | #define ___PASTE(a,b) a##b
         |                       ^
   <scratch space>:104:1: note: expanded from here
     104 | __UNIQUE_ID_guard492
         | ^
   drivers/iio/dac/ad5592r-base.c:308:2: error: cannot jump from switch statement to this case label
     308 |         case IIO_CHAN_INFO_SCALE:
         |         ^
   drivers/iio/dac/ad5592r-base.c:303:3: note: jump bypasses initialization of variable with __attribute__((cleanup))
     303 |                 guard(mutex)(&st->lock);
         |                 ^
   include/linux/cleanup.h:319:15: note: expanded from macro 'guard'
     319 |         CLASS(_name, __UNIQUE_ID(guard))
         |                      ^
   include/linux/compiler.h:166:29: note: expanded from macro '__UNIQUE_ID'
     166 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^
   include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE'
      84 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^
   include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE'
      83 | #define ___PASTE(a,b) a##b
         |                       ^
   <scratch space>:104:1: note: expanded from here
     104 | __UNIQUE_ID_guard492
         | ^
   drivers/iio/dac/ad5592r-base.c:427:2: error: cannot jump from switch statement to this case label
     427 |         default:
         |         ^
   drivers/iio/dac/ad5592r-base.c:419:3: note: jump bypasses initialization of variable with __attribute__((cleanup))
     419 |                 guard(mutex)(&st->lock);
         |                 ^
   include/linux/cleanup.h:319:15: note: expanded from macro 'guard'
     319 |         CLASS(_name, __UNIQUE_ID(guard))
         |                      ^
   include/linux/compiler.h:166:29: note: expanded from macro '__UNIQUE_ID'
     166 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^
   include/linux/compiler_types.h:84:22: note: expanded from macro '__PASTE'
      84 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^
   include/linux/compiler_types.h:83:23: note: expanded from macro '___PASTE'
      83 | #define ___PASTE(a,b) a##b
         |                       ^
   <scratch space>:169:1: note: expanded from here
     169 | __UNIQUE_ID_guard497
         | ^
   3 errors generated.


vim +350 drivers/iio/dac/ad5592r-base.c

56ca9db862bf3d7 Paul Cercueil       2016-04-05  287  
56ca9db862bf3d7 Paul Cercueil       2016-04-05  288  static int ad5592r_write_raw(struct iio_dev *iio_dev,
56ca9db862bf3d7 Paul Cercueil       2016-04-05  289  	struct iio_chan_spec const *chan, int val, int val2, long mask)
56ca9db862bf3d7 Paul Cercueil       2016-04-05  290  {
56ca9db862bf3d7 Paul Cercueil       2016-04-05  291  	struct ad5592r_state *st = iio_priv(iio_dev);
56ca9db862bf3d7 Paul Cercueil       2016-04-05  292  	int ret;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  293  
56ca9db862bf3d7 Paul Cercueil       2016-04-05  294  	switch (mask) {
56ca9db862bf3d7 Paul Cercueil       2016-04-05  295  	case IIO_CHAN_INFO_RAW:
56ca9db862bf3d7 Paul Cercueil       2016-04-05  296  
56ca9db862bf3d7 Paul Cercueil       2016-04-05  297  		if (val >= (1 << chan->scan_type.realbits) || val < 0)
56ca9db862bf3d7 Paul Cercueil       2016-04-05  298  			return -EINVAL;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  299  
56ca9db862bf3d7 Paul Cercueil       2016-04-05  300  		if (!chan->output)
56ca9db862bf3d7 Paul Cercueil       2016-04-05  301  			return -EINVAL;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  302  
299c6ede9f0343c Bartosz Golaszewski 2025-04-07  303  		guard(mutex)(&st->lock);
56ca9db862bf3d7 Paul Cercueil       2016-04-05  304  		ret = st->ops->write_dac(st, chan->channel, val);
56ca9db862bf3d7 Paul Cercueil       2016-04-05  305  		if (!ret)
56ca9db862bf3d7 Paul Cercueil       2016-04-05  306  			st->cached_dac[chan->channel] = val;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  307  		return ret;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  308  	case IIO_CHAN_INFO_SCALE:
56ca9db862bf3d7 Paul Cercueil       2016-04-05  309  		if (chan->type == IIO_VOLTAGE) {
56ca9db862bf3d7 Paul Cercueil       2016-04-05  310  			bool gain;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  311  
56ca9db862bf3d7 Paul Cercueil       2016-04-05  312  			if (val == st->scale_avail[0][0] &&
56ca9db862bf3d7 Paul Cercueil       2016-04-05  313  				val2 == st->scale_avail[0][1])
56ca9db862bf3d7 Paul Cercueil       2016-04-05  314  				gain = false;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  315  			else if (val == st->scale_avail[1][0] &&
56ca9db862bf3d7 Paul Cercueil       2016-04-05  316  				 val2 == st->scale_avail[1][1])
56ca9db862bf3d7 Paul Cercueil       2016-04-05  317  				gain = true;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  318  			else
56ca9db862bf3d7 Paul Cercueil       2016-04-05  319  				return -EINVAL;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  320  
299c6ede9f0343c Bartosz Golaszewski 2025-04-07  321  			guard(mutex)(&st->lock);
56ca9db862bf3d7 Paul Cercueil       2016-04-05  322  
56ca9db862bf3d7 Paul Cercueil       2016-04-05  323  			ret = st->ops->reg_read(st, AD5592R_REG_CTRL,
56ca9db862bf3d7 Paul Cercueil       2016-04-05  324  						&st->cached_gp_ctrl);
56ca9db862bf3d7 Paul Cercueil       2016-04-05  325  			if (ret < 0) {
33c53cbf8f7bc8d Sergiu Cuciurean    2020-05-20  326  				mutex_unlock(&st->lock);
56ca9db862bf3d7 Paul Cercueil       2016-04-05  327  				return ret;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  328  			}
56ca9db862bf3d7 Paul Cercueil       2016-04-05  329  
56ca9db862bf3d7 Paul Cercueil       2016-04-05  330  			if (chan->output) {
56ca9db862bf3d7 Paul Cercueil       2016-04-05  331  				if (gain)
56ca9db862bf3d7 Paul Cercueil       2016-04-05  332  					st->cached_gp_ctrl |=
56ca9db862bf3d7 Paul Cercueil       2016-04-05  333  						AD5592R_REG_CTRL_DAC_RANGE;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  334  				else
56ca9db862bf3d7 Paul Cercueil       2016-04-05  335  					st->cached_gp_ctrl &=
56ca9db862bf3d7 Paul Cercueil       2016-04-05  336  						~AD5592R_REG_CTRL_DAC_RANGE;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  337  			} else {
56ca9db862bf3d7 Paul Cercueil       2016-04-05  338  				if (gain)
56ca9db862bf3d7 Paul Cercueil       2016-04-05  339  					st->cached_gp_ctrl |=
56ca9db862bf3d7 Paul Cercueil       2016-04-05  340  						AD5592R_REG_CTRL_ADC_RANGE;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  341  				else
56ca9db862bf3d7 Paul Cercueil       2016-04-05  342  					st->cached_gp_ctrl &=
56ca9db862bf3d7 Paul Cercueil       2016-04-05  343  						~AD5592R_REG_CTRL_ADC_RANGE;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  344  			}
56ca9db862bf3d7 Paul Cercueil       2016-04-05  345  
299c6ede9f0343c Bartosz Golaszewski 2025-04-07  346  			return st->ops->reg_write(st, AD5592R_REG_CTRL,
56ca9db862bf3d7 Paul Cercueil       2016-04-05  347  						  st->cached_gp_ctrl);
56ca9db862bf3d7 Paul Cercueil       2016-04-05  348  		}
56ca9db862bf3d7 Paul Cercueil       2016-04-05  349  		break;
56ca9db862bf3d7 Paul Cercueil       2016-04-05 @350  	default:
56ca9db862bf3d7 Paul Cercueil       2016-04-05  351  		return -EINVAL;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  352  	}
56ca9db862bf3d7 Paul Cercueil       2016-04-05  353  
56ca9db862bf3d7 Paul Cercueil       2016-04-05  354  	return 0;
56ca9db862bf3d7 Paul Cercueil       2016-04-05  355  }
56ca9db862bf3d7 Paul Cercueil       2016-04-05  356  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki