* [PATCH v2 0/7] iio: convert GPIO chips to using new value setters
@ 2025-04-09 8:40 Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 1/7] iio: dac: ad5592r: destroy mutexes in detach paths Bartosz Golaszewski
` (7 more replies)
0 siblings, 8 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2025-04-09 8:40 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Linus Walleij, Bartosz Golaszewski, Cosmin Tanislav
Cc: linux-iio, linux-kernel, linux-gpio, 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>
---
Changes in v2:
- move devm_mutex_init() earlier in probe() to avoid using a goto
- rework returning on error in ad5592r_set_channel_modes(): return
immediately instead of saving the return value and going to the bottom
of the function
- use scoped_guard() in one more place to fix a build warning reported
by the build bot
- Link to v1: https://lore.kernel.org/r/20250407-gpiochip-set-rv-iio-v1-0-8431b003a145@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 | 147 ++++++++++++++++++-----------------------
5 files changed, 103 insertions(+), 117 deletions(-)
---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250401-gpiochip-set-rv-iio-b064ce43791d
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/7] iio: dac: ad5592r: destroy mutexes in detach paths
2025-04-09 8:40 [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Bartosz Golaszewski
@ 2025-04-09 8:40 ` Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 2/7] iio: dac: ad5592r: use lock guards Bartosz Golaszewski
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2025-04-09 8:40 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Linus Walleij, Bartosz Golaszewski, Cosmin Tanislav
Cc: linux-iio, linux-kernel, linux-gpio, 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 | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
index 50d19304bacb..eb85907f61ae 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)
@@ -606,6 +608,10 @@ int ad5592r_probe(struct device *dev, const char *name,
st->num_channels = 8;
dev_set_drvdata(dev, iio_dev);
+ ret = devm_mutex_init(dev, &st->lock);
+ if (ret)
+ return ret;
+
st->reg = devm_regulator_get_optional(dev, "vref");
if (IS_ERR(st->reg)) {
if ((PTR_ERR(st->reg) != -ENODEV) && dev_fwnode(dev))
@@ -622,8 +628,6 @@ int ad5592r_probe(struct device *dev, const char *name,
iio_dev->info = &ad5592r_info;
iio_dev->modes = INDIO_DIRECT_MODE;
- mutex_init(&st->lock);
-
ad5592r_init_scales(st, ad5592r_get_vref(st));
ret = ad5592r_reset(st);
--
2.45.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/7] iio: dac: ad5592r: use lock guards
2025-04-09 8:40 [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 1/7] iio: dac: ad5592r: destroy mutexes in detach paths Bartosz Golaszewski
@ 2025-04-09 8:40 ` Bartosz Golaszewski
2025-04-12 11:31 ` Jonathan Cameron
2025-04-09 8:40 ` [PATCH v2 3/7] iio: dac: ad5592r: use new GPIO line value setter callbacks Bartosz Golaszewski
` (5 subsequent siblings)
7 siblings, 1 reply; 11+ messages in thread
From: Bartosz Golaszewski @ 2025-04-09 8:40 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Linus Walleij, Bartosz Golaszewski, Cosmin Tanislav
Cc: linux-iio, linux-kernel, linux-gpio, 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 | 132 +++++++++++++++++------------------------
1 file changed, 54 insertions(+), 78 deletions(-)
diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
index eb85907f61ae..ada60f5ff1b6 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,46 +235,44 @@ 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;
-err_unlock:
- mutex_unlock(&st->lock);
- return ret;
+ return 0;
}
static int ad5592r_reset_channel_modes(struct ad5592r_state *st)
@@ -305,7 +289,7 @@ static int ad5592r_write_raw(struct iio_dev *iio_dev,
struct iio_chan_spec const *chan, int val, int val2, long mask)
{
struct ad5592r_state *st = iio_priv(iio_dev);
- int ret;
+ int ret = 0;
switch (mask) {
case IIO_CHAN_INFO_RAW:
@@ -316,11 +300,11 @@ static int ad5592r_write_raw(struct iio_dev *iio_dev,
if (!chan->output)
return -EINVAL;
- mutex_lock(&st->lock);
- ret = st->ops->write_dac(st, chan->channel, val);
- if (!ret)
- st->cached_dac[chan->channel] = val;
- mutex_unlock(&st->lock);
+ scoped_guard(mutex, &st->lock) {
+ ret = st->ops->write_dac(st, chan->channel, val);
+ if (!ret)
+ st->cached_dac[chan->channel] = val;
+ }
return ret;
case IIO_CHAN_INFO_SCALE:
if (chan->type == IIO_VOLTAGE) {
@@ -335,7 +319,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 +344,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 +360,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 +381,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 +400,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 +417,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/7] iio: dac: ad5592r: use new GPIO line value setter callbacks
2025-04-09 8:40 [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 1/7] iio: dac: ad5592r: destroy mutexes in detach paths Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 2/7] iio: dac: ad5592r: use lock guards Bartosz Golaszewski
@ 2025-04-09 8:40 ` Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 4/7] iio: adc: ti-ads7950: " Bartosz Golaszewski
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2025-04-09 8:40 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Linus Walleij, Bartosz Golaszewski, Cosmin Tanislav
Cc: linux-iio, linux-kernel, linux-gpio, 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 ada60f5ff1b6..c4c3453da823 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/7] iio: adc: ti-ads7950: use new GPIO line value setter callbacks
2025-04-09 8:40 [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Bartosz Golaszewski
` (2 preceding siblings ...)
2025-04-09 8:40 ` [PATCH v2 3/7] iio: dac: ad5592r: use new GPIO line value setter callbacks Bartosz Golaszewski
@ 2025-04-09 8:40 ` Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 5/7] iio: adc: ad4130: " Bartosz Golaszewski
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2025-04-09 8:40 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Linus Walleij, Bartosz Golaszewski, Cosmin Tanislav
Cc: linux-iio, linux-kernel, linux-gpio, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 5/7] iio: adc: ad4130: use new GPIO line value setter callbacks
2025-04-09 8:40 [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Bartosz Golaszewski
` (3 preceding siblings ...)
2025-04-09 8:40 ` [PATCH v2 4/7] iio: adc: ti-ads7950: " Bartosz Golaszewski
@ 2025-04-09 8:40 ` Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 6/7] iio: addac: ad74413r: " Bartosz Golaszewski
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2025-04-09 8:40 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Linus Walleij, Bartosz Golaszewski, Cosmin Tanislav
Cc: linux-iio, linux-kernel, linux-gpio, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 6/7] iio: addac: ad74413r: use new GPIO line value setter callbacks
2025-04-09 8:40 [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Bartosz Golaszewski
` (4 preceding siblings ...)
2025-04-09 8:40 ` [PATCH v2 5/7] iio: adc: ad4130: " Bartosz Golaszewski
@ 2025-04-09 8:40 ` Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 7/7] iio: addac: ad74115: " Bartosz Golaszewski
2025-04-09 14:08 ` [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Nuno Sá
7 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2025-04-09 8:40 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Linus Walleij, Bartosz Golaszewski, Cosmin Tanislav
Cc: linux-iio, linux-kernel, linux-gpio, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 7/7] iio: addac: ad74115: use new GPIO line value setter callbacks
2025-04-09 8:40 [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Bartosz Golaszewski
` (5 preceding siblings ...)
2025-04-09 8:40 ` [PATCH v2 6/7] iio: addac: ad74413r: " Bartosz Golaszewski
@ 2025-04-09 8:40 ` Bartosz Golaszewski
2025-04-09 14:08 ` [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Nuno Sá
7 siblings, 0 replies; 11+ messages in thread
From: Bartosz Golaszewski @ 2025-04-09 8:40 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Linus Walleij, Bartosz Golaszewski, Cosmin Tanislav
Cc: linux-iio, linux-kernel, linux-gpio, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/7] iio: convert GPIO chips to using new value setters
2025-04-09 8:40 [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Bartosz Golaszewski
` (6 preceding siblings ...)
2025-04-09 8:40 ` [PATCH v2 7/7] iio: addac: ad74115: " Bartosz Golaszewski
@ 2025-04-09 14:08 ` Nuno Sá
2025-04-12 11:32 ` Jonathan Cameron
7 siblings, 1 reply; 11+ messages in thread
From: Nuno Sá @ 2025-04-09 14:08 UTC (permalink / raw)
To: Bartosz Golaszewski, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Linus Walleij, Cosmin Tanislav
Cc: linux-iio, linux-kernel, linux-gpio, Bartosz Golaszewski
On Wed, 2025-04-09 at 10:40 +0200, Bartosz Golaszewski 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>
> ---
LGTM,
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> Changes in v2:
> - move devm_mutex_init() earlier in probe() to avoid using a goto
> - rework returning on error in ad5592r_set_channel_modes(): return
> immediately instead of saving the return value and going to the bottom
> of the function
> - use scoped_guard() in one more place to fix a build warning reported
> by the build bot
> - Link to v1:
> https://lore.kernel.org/r/20250407-gpiochip-set-rv-iio-v1-0-8431b003a145@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 | 147 ++++++++++++++++++----------------------
> -
> 5 files changed, 103 insertions(+), 117 deletions(-)
> ---
> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> change-id: 20250401-gpiochip-set-rv-iio-b064ce43791d
>
> Best regards,
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/7] iio: dac: ad5592r: use lock guards
2025-04-09 8:40 ` [PATCH v2 2/7] iio: dac: ad5592r: use lock guards Bartosz Golaszewski
@ 2025-04-12 11:31 ` Jonathan Cameron
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2025-04-12 11:31 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Lars-Peter Clausen, Michael Hennerich, Linus Walleij,
Cosmin Tanislav, linux-iio, linux-kernel, linux-gpio,
Bartosz Golaszewski
On Wed, 09 Apr 2025 10:40:40 +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.
Needed a tweak for my llvm test build to work.
diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
index ada60f5ff1b6..32e63a159d86 100644
--- a/drivers/iio/dac/ad5592r-base.c
+++ b/drivers/iio/dac/ad5592r-base.c
@@ -414,7 +414,7 @@ static int ad5592r_read_raw(struct iio_dev *iio_dev,
*val2 = chan->scan_type.realbits;
return IIO_VAL_FRACTIONAL_LOG2;
- case IIO_CHAN_INFO_OFFSET:
+ case IIO_CHAN_INFO_OFFSET: {
ret = ad5592r_get_vref(st);
guard(mutex)(&st->lock);
@@ -425,6 +425,7 @@ static int ad5592r_read_raw(struct iio_dev *iio_dev,
*val = (-75365 * 25) / ret;
return IIO_VAL_INT;
+ }
default:
return -EINVAL;
}
Scope gets messy in switch statements.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> drivers/iio/dac/ad5592r-base.c | 132 +++++++++++++++++------------------------
> 1 file changed, 54 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
> index eb85907f61ae..ada60f5ff1b6 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,46 +235,44 @@ 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;
>
> -err_unlock:
> - mutex_unlock(&st->lock);
> - return ret;
> + return 0;
> }
>
> static int ad5592r_reset_channel_modes(struct ad5592r_state *st)
> @@ -305,7 +289,7 @@ static int ad5592r_write_raw(struct iio_dev *iio_dev,
> struct iio_chan_spec const *chan, int val, int val2, long mask)
> {
> struct ad5592r_state *st = iio_priv(iio_dev);
> - int ret;
> + int ret = 0;
>
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> @@ -316,11 +300,11 @@ static int ad5592r_write_raw(struct iio_dev *iio_dev,
> if (!chan->output)
> return -EINVAL;
>
> - mutex_lock(&st->lock);
> - ret = st->ops->write_dac(st, chan->channel, val);
> - if (!ret)
> - st->cached_dac[chan->channel] = val;
> - mutex_unlock(&st->lock);
> + scoped_guard(mutex, &st->lock) {
> + ret = st->ops->write_dac(st, chan->channel, val);
> + if (!ret)
> + st->cached_dac[chan->channel] = val;
> + }
> return ret;
> case IIO_CHAN_INFO_SCALE:
> if (chan->type == IIO_VOLTAGE) {
> @@ -335,7 +319,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 +344,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 +360,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 +381,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 +400,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 +417,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;
>
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/7] iio: convert GPIO chips to using new value setters
2025-04-09 14:08 ` [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Nuno Sá
@ 2025-04-12 11:32 ` Jonathan Cameron
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2025-04-12 11:32 UTC (permalink / raw)
To: Nuno Sá
Cc: Bartosz Golaszewski, Lars-Peter Clausen, Michael Hennerich,
Linus Walleij, Cosmin Tanislav, linux-iio, linux-kernel,
linux-gpio, Bartosz Golaszewski
On Wed, 09 Apr 2025 15:08:24 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:
> On Wed, 2025-04-09 at 10:40 +0200, Bartosz Golaszewski 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>
> > ---
>
> LGTM,
>
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
>
Applied with that tweak to patch 2.
Pushed out initially as testing for the build bots to poke at it some more.
Jonathan
> > Changes in v2:
> > - move devm_mutex_init() earlier in probe() to avoid using a goto
> > - rework returning on error in ad5592r_set_channel_modes(): return
> > immediately instead of saving the return value and going to the bottom
> > of the function
> > - use scoped_guard() in one more place to fix a build warning reported
> > by the build bot
> > - Link to v1:
> > https://lore.kernel.org/r/20250407-gpiochip-set-rv-iio-v1-0-8431b003a145@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 | 147 ++++++++++++++++++----------------------
> > -
> > 5 files changed, 103 insertions(+), 117 deletions(-)
> > ---
> > base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
> > change-id: 20250401-gpiochip-set-rv-iio-b064ce43791d
> >
> > Best regards,
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-04-12 11:33 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-09 8:40 [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 1/7] iio: dac: ad5592r: destroy mutexes in detach paths Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 2/7] iio: dac: ad5592r: use lock guards Bartosz Golaszewski
2025-04-12 11:31 ` Jonathan Cameron
2025-04-09 8:40 ` [PATCH v2 3/7] iio: dac: ad5592r: use new GPIO line value setter callbacks Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 4/7] iio: adc: ti-ads7950: " Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 5/7] iio: adc: ad4130: " Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 6/7] iio: addac: ad74413r: " Bartosz Golaszewski
2025-04-09 8:40 ` [PATCH v2 7/7] iio: addac: ad74115: " Bartosz Golaszewski
2025-04-09 14:08 ` [PATCH v2 0/7] iio: convert GPIO chips to using new value setters Nuno Sá
2025-04-12 11:32 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).