[PATCH 0/3] iio: addac: ad74413r: minor improvements

[PATCH 0/3] iio: addac: ad74413r: minor improvements

[Nuno Sa via B4 Relay]

Minor improvements handling locking, reset gpio and vref voltage. No
functional changes intended.

---
Nuno Sa (3):
      iio: addac: ad74413r: drop reset_gpio from struct ad74413r_state
      iio: addac: ad74413r: use devm_regulator_get_enable_read_voltage()
      iio: addac: ad74413r: simplify with cleanup.h

 drivers/iio/addac/ad74413r.c | 81 ++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 51 deletions(-)
---
base-commit: c3e9df514041ec6c46be83801b1891392f4522f7
change-id: 20241016-dev-ad74413r-minor-improv-d8b061d44348
--

Thanks!
- Nuno Sá

[PATCH 1/3] iio: addac: ad74413r: drop reset_gpio from struct ad74413r_state

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

We just need the reset gpio during probe so there's no need to keep it
in our state struct. Hence, move devm_gpiod_get_optional() into
ad74413r_reset() and use a local struct gpio_desc.

While at it, request the gpio in the asserted state (GPIOD_OUT_HIGH) so
that we already perform the reset while requesting the gpio saving us a
call to gpiod_set_value_cansleep().

Also, explicitly include <gpio/consumer.h> for
devm_gpiod_get_optional().

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/addac/ad74413r.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
index e50c896a07668..550e2460e29ca 100644
--- a/drivers/iio/addac/ad74413r.c
+++ b/drivers/iio/addac/ad74413r.c
@@ -9,6 +9,7 @@
 #include <linux/crc8.h>
 #include <linux/device.h>
 #include <linux/err.h>
+#include <linux/gpio/consumer.h>
 #include <linux/gpio/driver.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
@@ -72,7 +73,6 @@ struct ad74413r_state {
 	struct regmap			*regmap;
 	struct device			*dev;
 	struct iio_trigger		*trig;
-	struct gpio_desc		*reset_gpio;
 
 	size_t			adc_active_channels;
 	struct spi_message	adc_samples_msg;
@@ -407,12 +407,16 @@ static int ad74413r_gpio_set_comp_config(struct gpio_chip *chip,
 
 static int ad74413r_reset(struct ad74413r_state *st)
 {
+	struct gpio_desc *reset_gpio;
 	int ret;
 
-	if (st->reset_gpio) {
-		gpiod_set_value_cansleep(st->reset_gpio, 1);
+	reset_gpio = devm_gpiod_get_optional(st->dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(reset_gpio))
+		return PTR_ERR(reset_gpio);
+
+	if (reset_gpio) {
 		fsleep(50);
-		gpiod_set_value_cansleep(st->reset_gpio, 0);
+		gpiod_set_value_cansleep(reset_gpio, 0);
 		return 0;
 	}
 
@@ -1378,10 +1382,6 @@ static int ad74413r_probe(struct spi_device *spi)
 	if (IS_ERR(st->regmap))
 		return PTR_ERR(st->regmap);
 
-	st->reset_gpio = devm_gpiod_get_optional(st->dev, "reset", GPIOD_OUT_LOW);
-	if (IS_ERR(st->reset_gpio))
-		return PTR_ERR(st->reset_gpio);
-
 	st->refin_reg = devm_regulator_get(st->dev, "refin");
 	if (IS_ERR(st->refin_reg))
 		return dev_err_probe(st->dev, PTR_ERR(st->refin_reg),

-- 
2.46.1

[PATCH 2/3] iio: addac: ad74413r: use devm_regulator_get_enable_read_voltage()

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

It's highly unlikely for the converter ref voltage to change at runtime.
Hence, let's read the voltage and save it (instead of the regulator
struct). While at it, simplify the code by using
devm_regulator_get_enable_read_voltage().

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/addac/ad74413r.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
index 550e2460e29c..cfe26a394465 100644
--- a/drivers/iio/addac/ad74413r.c
+++ b/drivers/iio/addac/ad74413r.c
@@ -60,7 +60,7 @@ struct ad74413r_state {
 	unsigned int			num_gpo_gpios;
 	unsigned int			num_comparator_gpios;
 	u32				sense_resistor_ohms;
-
+	int				refin_reg_uv;
 	/*
 	 * Synchronize consecutive operations when doing a one-shot
 	 * conversion and when updating the ADC samples SPI message.
@@ -69,7 +69,6 @@ struct ad74413r_state {
 
 	const struct ad74413r_chip_info	*chip_info;
 	struct spi_device		*spi;
-	struct regulator		*refin_reg;
 	struct regmap			*regmap;
 	struct device			*dev;
 	struct iio_trigger		*trig;
@@ -664,7 +663,7 @@ static int ad74413r_get_output_voltage_scale(struct ad74413r_state *st,
 static int ad74413r_get_output_current_scale(struct ad74413r_state *st,
 					     int *val, int *val2)
 {
-	*val = regulator_get_voltage(st->refin_reg);
+	*val = st->refin_reg_uv;
 	*val2 = st->sense_resistor_ohms * AD74413R_DAC_CODE_MAX * 1000;
 
 	return IIO_VAL_FRACTIONAL;
@@ -1351,11 +1350,6 @@ static int ad74413r_setup_gpios(struct ad74413r_state *st)
 	return 0;
 }
 
-static void ad74413r_regulator_disable(void *regulator)
-{
-	regulator_disable(regulator);
-}
-
 static int ad74413r_probe(struct spi_device *spi)
 {
 	struct ad74413r_state *st;
@@ -1382,19 +1376,11 @@ static int ad74413r_probe(struct spi_device *spi)
 	if (IS_ERR(st->regmap))
 		return PTR_ERR(st->regmap);
 
-	st->refin_reg = devm_regulator_get(st->dev, "refin");
-	if (IS_ERR(st->refin_reg))
-		return dev_err_probe(st->dev, PTR_ERR(st->refin_reg),
-				     "Failed to get refin regulator\n");
-
-	ret = regulator_enable(st->refin_reg);
-	if (ret)
-		return ret;
-
-	ret = devm_add_action_or_reset(st->dev, ad74413r_regulator_disable,
-				       st->refin_reg);
-	if (ret)
-		return ret;
+	ret = devm_regulator_get_enable_read_voltage(st->dev, "refin");
+	if (ret < 0)
+		return dev_err_probe(st->dev, ret,
+				     "Failed to get refin regulator voltage\n");
+	st->refin_reg_uv = ret;
 
 	st->sense_resistor_ohms = 100000000;
 	device_property_read_u32(st->dev, "shunt-resistor-micro-ohms",

-- 
2.46.1

[PATCH 3/3] iio: addac: ad74413r: simplify with cleanup.h

[Nuno Sa via B4 Relay]

From: Nuno Sa <nuno.sa@analog.com>

Make use of mutex guard() and IIO iio_device_claim_direct_scoped() to
simplify code and error handling.

While at it, use devm_mutex_init() to initialize the mutex.

Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
 drivers/iio/addac/ad74413r.c | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
index cfe26a394465..daea2bde7acf 100644
--- a/drivers/iio/addac/ad74413r.c
+++ b/drivers/iio/addac/ad74413r.c
@@ -6,6 +6,7 @@
 
 #include <linux/unaligned.h>
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/crc8.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -864,19 +865,12 @@ static int ad74413r_get_single_adc_result(struct iio_dev *indio_dev,
 					  unsigned int channel, int *val)
 {
 	struct ad74413r_state *st = iio_priv(indio_dev);
-	int ret;
 
-	ret = iio_device_claim_direct_mode(indio_dev);
-	if (ret)
-		return ret;
-
-	mutex_lock(&st->lock);
-	ret = _ad74413r_get_single_adc_result(st, channel, val);
-	mutex_unlock(&st->lock);
-
-	iio_device_release_direct_mode(indio_dev);
-
-	return ret;
+	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
+		guard(mutex)(&st->lock);
+		return _ad74413r_get_single_adc_result(st, channel, val);
+	}
+	unreachable();
 }
 
 static void ad74413r_adc_to_resistance_result(int adc_result, int *val)
@@ -898,7 +892,7 @@ static int ad74413r_update_scan_mode(struct iio_dev *indio_dev,
 	unsigned int channel;
 	int ret = -EINVAL;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	spi_message_init(&st->adc_samples_msg);
 	st->adc_active_channels = 0;
@@ -906,11 +900,11 @@ static int ad74413r_update_scan_mode(struct iio_dev *indio_dev,
 	for_each_clear_bit(channel, active_scan_mask, AD74413R_CHANNEL_MAX) {
 		ret = ad74413r_set_adc_channel_enable(st, channel, false);
 		if (ret)
-			goto out;
+			return ret;
 	}
 
 	if (*active_scan_mask == 0)
-		goto out;
+		return ret;
 
 	/*
 	 * The read select register is used to select which register's value
@@ -928,7 +922,7 @@ static int ad74413r_update_scan_mode(struct iio_dev *indio_dev,
 	for_each_set_bit(channel, active_scan_mask, AD74413R_CHANNEL_MAX) {
 		ret = ad74413r_set_adc_channel_enable(st, channel, true);
 		if (ret)
-			goto out;
+			return ret;
 
 		st->adc_active_channels++;
 
@@ -959,11 +953,7 @@ static int ad74413r_update_scan_mode(struct iio_dev *indio_dev,
 	xfer->cs_change = 0;
 
 	spi_message_add_tail(xfer, &st->adc_samples_msg);
-
-out:
-	mutex_unlock(&st->lock);
-
-	return ret;
+	return 0;
 }
 
 static int ad74413r_buffer_postenable(struct iio_dev *indio_dev)
@@ -1368,7 +1358,10 @@ static int ad74413r_probe(struct spi_device *spi)
 	if (!st->chip_info)
 		return -EINVAL;
 
-	mutex_init(&st->lock);
+	ret = devm_mutex_init(st->dev, &st->lock);
+	if (ret)
+		return ret;
+
 	init_completion(&st->adc_data_completion);
 
 	st->regmap = devm_regmap_init(st->dev, NULL, st,

-- 
2.46.1

Re: [PATCH 0/3] iio: addac: ad74413r: minor improvements

[Jonathan Cameron]

On Wed, 16 Oct 2024 16:21:58 +0200
Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

> Minor improvements handling locking, reset gpio and vref voltage. No
> functional changes intended.
> 
Applied.
> ---
> Nuno Sa (3):
>       iio: addac: ad74413r: drop reset_gpio from struct ad74413r_state
>       iio: addac: ad74413r: use devm_regulator_get_enable_read_voltage()
>       iio: addac: ad74413r: simplify with cleanup.h
> 
>  drivers/iio/addac/ad74413r.c | 81 ++++++++++++++++----------------------------
>  1 file changed, 30 insertions(+), 51 deletions(-)
> ---
> base-commit: c3e9df514041ec6c46be83801b1891392f4522f7
> change-id: 20241016-dev-ad74413r-minor-improv-d8b061d44348
> --
> 
> Thanks!
> - Nuno Sá
> 
>