[PATCH v3 0/6] ti-ads7950: fix gpio handling and facelift

[PATCH v3 0/6] ti-ads7950: fix gpio handling and facelift

[Dmitry Torokhov]

The original reason for this series is to make sure ti_ads7950_get()
returns result in range [0, 1] or a negative error code to comply with
gpiolib requirement.

During review David noticed the fact that the function also clobbers
gpio state. Fixing that lead to adding cleanups using guard() and devm.

v3:

- Added "Fixes" tags
- Picked up reviewed-by tags for #1 and #2
- Dropped the conversion to "error" variables
- Avoid using scoped_guard (per David)
- directly return results of function calls at the end of function (David)
- Split spi_sync() error handling into a new patxch (Andy)
- Switch to using devm_regulator_get_enable_read_voltage() (David)

v2:

- style fixes in #1 (per David)
- fix to not clobber state (#2, new)
- #3 and #4 (new)

---
Dmitry Torokhov (6):
      iio: adc: ti-ads7950: normalize return value of gpio_get
      iio: adc: ti-ads7950: do not clobber gpio state in ti_ads7950_get()
      iio: adc: ti-ads7950: switch to using guard() notation
      iio: adc: ti-ads7950: simplify check for spi_setup() failures
      iio: adc: ti-ads7950: switch to using devm_regulator_get_enable_read_voltage()
      iio: adc: ti-ads7950: complete conversion to using managed resources

 drivers/iio/adc/ti-ads7950.c | 188 ++++++++++++++-----------------------------
 1 file changed, 59 insertions(+), 129 deletions(-)
---
base-commit: c025f6cf4209e1542ec2afebe49f42bbaf1a5c7b
change-id: 20260220-ti-ads7950-facelift-ad8b5390a654

Thanks.

-- 
Dmitry

[PATCH v3 1/6] iio: adc: ti-ads7950: normalize return value of gpio_get

[Dmitry Torokhov]

The GPIO get callback is expected to return 0 or 1 (or a negative error
code). Ensure that the value returned by ti_ads7950_get() for output
pins is normalized to the [0, 1] range.

Fixes: 86ef402d805d ("gpiolib: sanitize the return value of gpio_chip::get()")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/iio/adc/ti-ads7950.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index bbe1ce577789..b8cc39fc39fb 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -433,7 +433,7 @@ static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
 
 	/* If set as output, return the output */
 	if (st->gpio_cmd_settings_bitmask & BIT(offset)) {
-		ret = st->cmd_settings_bitmask & BIT(offset);
+		ret = (st->cmd_settings_bitmask & BIT(offset)) ? 1 : 0;
 		goto out;
 	}
 

-- 
2.53.0.473.g4a7958ca14-goog

[PATCH v3 2/6] iio: adc: ti-ads7950: do not clobber gpio state in ti_ads7950_get()

[Dmitry Torokhov]

GPIO state was inadvertently overwritten by the result of spi_sync(),
resulting in ti_ads7950_get() only returning 0 as GPIO state (or error).

Fix this by introducing a separate variable to hold the state.

Fixes: c97dce792dc8 ("iio: adc: ti-ads7950: add GPIO support")
Reported-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/iio/adc/ti-ads7950.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index b8cc39fc39fb..cdc624889559 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -427,13 +427,15 @@ static int ti_ads7950_set(struct gpio_chip *chip, unsigned int offset,
 static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
 {
 	struct ti_ads7950_state *st = gpiochip_get_data(chip);
+	bool state;
 	int ret;
 
 	mutex_lock(&st->slock);
 
 	/* If set as output, return the output */
 	if (st->gpio_cmd_settings_bitmask & BIT(offset)) {
-		ret = (st->cmd_settings_bitmask & BIT(offset)) ? 1 : 0;
+		state = st->cmd_settings_bitmask & BIT(offset);
+		ret = 0;
 		goto out;
 	}
 
@@ -444,7 +446,7 @@ static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
 	if (ret)
 		goto out;
 
-	ret = ((st->single_rx >> 12) & BIT(offset)) ? 1 : 0;
+	state = (st->single_rx >> 12) & BIT(offset);
 
 	/* Revert back to original settings */
 	st->cmd_settings_bitmask &= ~TI_ADS7950_CR_GPIO_DATA;
@@ -456,7 +458,7 @@ static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
 out:
 	mutex_unlock(&st->slock);
 
-	return ret;
+	return ret ?: state;
 }
 
 static int ti_ads7950_get_direction(struct gpio_chip *chip,

-- 
2.53.0.473.g4a7958ca14-goog

[PATCH v3 3/6] iio: adc: ti-ads7950: switch to using guard() notation

[Dmitry Torokhov]

guard() notation allows early returns when encountering errors, making
control flow more obvious. Use it.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/iio/adc/ti-ads7950.c | 75 ++++++++++++++++----------------------------
 1 file changed, 27 insertions(+), 48 deletions(-)

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index cdc624889559..1515089dd759 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -308,16 +308,17 @@ static irqreturn_t ti_ads7950_trigger_handler(int irq, void *p)
 	struct ti_ads7950_state *st = iio_priv(indio_dev);
 	int ret;
 
-	mutex_lock(&st->slock);
-	ret = spi_sync(st->spi, &st->ring_msg);
-	if (ret < 0)
-		goto out;
+	do {
+		guard(mutex)(&st->slock);
 
-	iio_push_to_buffers_with_timestamp(indio_dev, &st->rx_buf[2],
-					   iio_get_time_ns(indio_dev));
+		ret = spi_sync(st->spi, &st->ring_msg);
+		if (ret)
+			break;
+
+		iio_push_to_buffers_with_timestamp(indio_dev, &st->rx_buf[2],
+						   iio_get_time_ns(indio_dev));
+	} while (0);
 
-out:
-	mutex_unlock(&st->slock);
 	iio_trigger_notify_done(indio_dev->trig);
 
 	return IRQ_HANDLED;
@@ -328,20 +329,16 @@ static int ti_ads7950_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
 	struct ti_ads7950_state *st = iio_priv(indio_dev);
 	int ret, cmd;
 
-	mutex_lock(&st->slock);
+	guard(mutex)(&st->slock);
+
 	cmd = TI_ADS7950_MAN_CMD(TI_ADS7950_CR_CHAN(ch));
 	st->single_tx = cmd;
 
 	ret = spi_sync(st->spi, &st->scan_single_msg);
 	if (ret)
-		goto out;
-
-	ret = st->single_rx;
-
-out:
-	mutex_unlock(&st->slock);
+		return ret;
 
-	return ret;
+	return st->single_rx;
 }
 
 static int ti_ads7950_get_range(struct ti_ads7950_state *st)
@@ -407,9 +404,8 @@ 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);
+	guard(mutex)(&st->slock);
 
 	if (value)
 		st->cmd_settings_bitmask |= BIT(offset);
@@ -417,11 +413,8 @@ static int 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);
-	ret = spi_sync(st->spi, &st->scan_single_msg);
-
-	mutex_unlock(&st->slock);
 
-	return ret;
+	return spi_sync(st->spi, &st->scan_single_msg);
 }
 
 static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
@@ -430,13 +423,12 @@ static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
 	bool state;
 	int ret;
 
-	mutex_lock(&st->slock);
+	guard(mutex)(&st->slock);
 
 	/* If set as output, return the output */
 	if (st->gpio_cmd_settings_bitmask & BIT(offset)) {
 		state = st->cmd_settings_bitmask & BIT(offset);
-		ret = 0;
-		goto out;
+		return state;
 	}
 
 	/* GPIO data bit sets SDO bits 12-15 to GPIO input */
@@ -444,7 +436,7 @@ static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
 	st->single_tx = TI_ADS7950_MAN_CMD_SETTINGS(st);
 	ret = spi_sync(st->spi, &st->scan_single_msg);
 	if (ret)
-		goto out;
+		return ret;
 
 	state = (st->single_rx >> 12) & BIT(offset);
 
@@ -453,12 +445,9 @@ static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
 	st->single_tx = TI_ADS7950_MAN_CMD_SETTINGS(st);
 	ret = spi_sync(st->spi, &st->scan_single_msg);
 	if (ret)
-		goto out;
-
-out:
-	mutex_unlock(&st->slock);
+		return ret;
 
-	return ret ?: state;
+	return state;
 }
 
 static int ti_ads7950_get_direction(struct gpio_chip *chip,
@@ -474,9 +463,8 @@ static int _ti_ads7950_set_direction(struct gpio_chip *chip, int offset,
 				     int input)
 {
 	struct ti_ads7950_state *st = gpiochip_get_data(chip);
-	int ret = 0;
 
-	mutex_lock(&st->slock);
+	guard(mutex)(&st->slock);
 
 	/* Only change direction if needed */
 	if (input && (st->gpio_cmd_settings_bitmask & BIT(offset)))
@@ -484,15 +472,11 @@ static int _ti_ads7950_set_direction(struct gpio_chip *chip, int offset,
 	else if (!input && !(st->gpio_cmd_settings_bitmask & BIT(offset)))
 		st->gpio_cmd_settings_bitmask |= BIT(offset);
 	else
-		goto out;
+		return 0;
 
 	st->single_tx = TI_ADS7950_GPIO_CMD_SETTINGS(st);
-	ret = spi_sync(st->spi, &st->scan_single_msg);
 
-out:
-	mutex_unlock(&st->slock);
-
-	return ret;
+	return spi_sync(st->spi, &st->scan_single_msg);
 }
 
 static int ti_ads7950_direction_input(struct gpio_chip *chip,
@@ -515,9 +499,9 @@ static int ti_ads7950_direction_output(struct gpio_chip *chip,
 
 static int ti_ads7950_init_hw(struct ti_ads7950_state *st)
 {
-	int ret = 0;
+	int ret;
 
-	mutex_lock(&st->slock);
+	guard(mutex)(&st->slock);
 
 	/* Settings for Manual/Auto1/Auto2 commands */
 	/* Default to 5v ref */
@@ -525,17 +509,12 @@ static int ti_ads7950_init_hw(struct ti_ads7950_state *st)
 	st->single_tx = TI_ADS7950_MAN_CMD_SETTINGS(st);
 	ret = spi_sync(st->spi, &st->scan_single_msg);
 	if (ret)
-		goto out;
+		return ret;
 
 	/* Settings for GPIO command */
 	st->gpio_cmd_settings_bitmask = 0x0;
 	st->single_tx = TI_ADS7950_GPIO_CMD_SETTINGS(st);
-	ret = spi_sync(st->spi, &st->scan_single_msg);
-
-out:
-	mutex_unlock(&st->slock);
-
-	return ret;
+	return spi_sync(st->spi, &st->scan_single_msg);
 }
 
 static int ti_ads7950_probe(struct spi_device *spi)

-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH v3 3/6] iio: adc: ti-ads7950: switch to using guard() notation

[David Lechner]

On 3/5/26 1:21 PM, Dmitry Torokhov wrote:
> guard() notation allows early returns when encountering errors, making
> control flow more obvious. Use it.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---

Reviewed-by: David Lechner <dlechner@baylibre.com>

[PATCH v3 4/6] iio: adc: ti-ads7950: simplify check for spi_setup() failures

[Dmitry Torokhov]

spi_setup() specifies that it returns 0 on success or negative error on
failure. Therefore we can simply check for the return code being 0 or
not.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/iio/adc/ti-ads7950.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index 1515089dd759..273c35e03185 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -527,7 +527,7 @@ static int ti_ads7950_probe(struct spi_device *spi)
 	spi->bits_per_word = 16;
 	spi->mode |= SPI_CS_WORD;
 	ret = spi_setup(spi);
-	if (ret < 0) {
+	if (ret) {
 		dev_err(&spi->dev, "Error in spi setup\n");
 		return ret;
 	}

-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH v3 4/6] iio: adc: ti-ads7950: simplify check for spi_setup() failures

[Jonathan Cameron]

On Thu, 05 Mar 2026 11:21:55 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> spi_setup() specifies that it returns 0 on success or negative error on
> failure. Therefore we can simply check for the return code being 0 or
> not.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/iio/adc/ti-ads7950.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
> index 1515089dd759..273c35e03185 100644
> --- a/drivers/iio/adc/ti-ads7950.c
> +++ b/drivers/iio/adc/ti-ads7950.c
> @@ -527,7 +527,7 @@ static int ti_ads7950_probe(struct spi_device *spi)
>  	spi->bits_per_word = 16;
>  	spi->mode |= SPI_CS_WORD;
>  	ret = spi_setup(spi);
> -	if (ret < 0) {
Given it doesn't really fit better in patch 6 than it does here
maybe drag the
	if (ret)
		return dev_err_probe();
here instead just to avoid tiny it of churn around brackets.

Not important though.

> +	if (ret) {
>  		dev_err(&spi->dev, "Error in spi setup\n");
>  		return ret;
>  	}
> 

Re: [PATCH v3 4/6] iio: adc: ti-ads7950: simplify check for spi_setup() failures

[David Lechner]

On 3/5/26 1:21 PM, Dmitry Torokhov wrote:
> spi_setup() specifies that it returns 0 on success or negative error on
> failure. Therefore we can simply check for the return code being 0 or
> not.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
Reviewed-by: David Lechner <dlechner@baylibre.com>

[PATCH v3 5/6] iio: adc: ti-ads7950: switch to using devm_regulator_get_enable_read_voltage()

[Dmitry Torokhov]

The regulator is enabled for the entire time the driver is bound to the
device, and we only need to access it to fetch voltage, which can be
done at probe time.

Switch to using devm_regulator_get_enable_read_voltage() which
simplifies probing and unbinding code.

Suggested-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/iio/adc/ti-ads7950.c | 45 +++++++++++---------------------------------
 1 file changed, 11 insertions(+), 34 deletions(-)

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index 273c35e03185..847e83baa876 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -341,19 +341,9 @@ static int ti_ads7950_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
 	return st->single_rx;
 }
 
-static int ti_ads7950_get_range(struct ti_ads7950_state *st)
+static unsigned int ti_ads7950_get_range(struct ti_ads7950_state *st)
 {
-	int vref;
-
-	if (st->vref_mv) {
-		vref = st->vref_mv;
-	} else {
-		vref = regulator_get_voltage(st->reg);
-		if (vref < 0)
-			return vref;
-
-		vref /= 1000;
-	}
+	unsigned int vref = st->vref_mv;
 
 	if (st->cmd_settings_bitmask & TI_ADS7950_CR_RANGE_5V)
 		vref *= 2;
@@ -382,11 +372,7 @@ static int ti_ads7950_read_raw(struct iio_dev *indio_dev,
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		ret = ti_ads7950_get_range(st);
-		if (ret < 0)
-			return ret;
-
-		*val = ret;
+		*val = ti_ads7950_get_range(st);
 		*val2 = (1 << chan->scan_type.realbits) - 1;
 
 		return IIO_VAL_FRACTIONAL;
@@ -580,30 +566,24 @@ static int ti_ads7950_probe(struct spi_device *spi)
 	spi_message_init_with_transfers(&st->scan_single_msg,
 					st->scan_single_xfer, 3);
 
+	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref");
+	if (ret < 0)
+		return dev_err_probe(&spi->dev, ret,
+				     "Failed to get regulator \"vref\"\n");
+
 	/* Use hard coded value for reference voltage in ACPI case */
 	if (ACPI_COMPANION(&spi->dev))
 		st->vref_mv = TI_ADS7950_VA_MV_ACPI_DEFAULT;
+	else
+		st->vref_mv = ret / 1000;
 
 	mutex_init(&st->slock);
 
-	st->reg = devm_regulator_get(&spi->dev, "vref");
-	if (IS_ERR(st->reg)) {
-		ret = dev_err_probe(&spi->dev, PTR_ERR(st->reg),
-				     "Failed to get regulator \"vref\"\n");
-		goto error_destroy_mutex;
-	}
-
-	ret = regulator_enable(st->reg);
-	if (ret) {
-		dev_err(&spi->dev, "Failed to enable regulator \"vref\"\n");
-		goto error_destroy_mutex;
-	}
-
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
 					 &ti_ads7950_trigger_handler, NULL);
 	if (ret) {
 		dev_err(&spi->dev, "Failed to setup triggered buffer\n");
-		goto error_disable_reg;
+		goto error_destroy_mutex;
 	}
 
 	ret = ti_ads7950_init_hw(st);
@@ -643,8 +623,6 @@ static int ti_ads7950_probe(struct spi_device *spi)
 	iio_device_unregister(indio_dev);
 error_cleanup_ring:
 	iio_triggered_buffer_cleanup(indio_dev);
-error_disable_reg:
-	regulator_disable(st->reg);
 error_destroy_mutex:
 	mutex_destroy(&st->slock);
 
@@ -659,7 +637,6 @@ static void ti_ads7950_remove(struct spi_device *spi)
 	gpiochip_remove(&st->chip);
 	iio_device_unregister(indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
-	regulator_disable(st->reg);
 	mutex_destroy(&st->slock);
 }
 

-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH v3 5/6] iio: adc: ti-ads7950: switch to using devm_regulator_get_enable_read_voltage()

[Jonathan Cameron]

On Thu, 05 Mar 2026 11:21:56 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> The regulator is enabled for the entire time the driver is bound to the
> device, and we only need to access it to fetch voltage, which can be
> done at probe time.
> 
> Switch to using devm_regulator_get_enable_read_voltage() which
> simplifies probing and unbinding code.
> 
> Suggested-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Hi.

I think this broke the ACPI case (where regulator isn't available).

Jonathan

> ---
>  drivers/iio/adc/ti-ads7950.c | 45 +++++++++++---------------------------------
>  1 file changed, 11 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
> index 273c35e03185..847e83baa876 100644
> --- a/drivers/iio/adc/ti-ads7950.c
> +++ b/drivers/iio/adc/ti-ads7950.c
> @@ -341,19 +341,9 @@ static int ti_ads7950_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
>  	return st->single_rx;
>  }
>  
> -static int ti_ads7950_get_range(struct ti_ads7950_state *st)
> +static unsigned int ti_ads7950_get_range(struct ti_ads7950_state *st)
>  {
> -	int vref;
> -
> -	if (st->vref_mv) {
> -		vref = st->vref_mv;
> -	} else {
> -		vref = regulator_get_voltage(st->reg);
> -		if (vref < 0)
> -			return vref;
> -
> -		vref /= 1000;
> -	}
> +	unsigned int vref = st->vref_mv;
>  
>  	if (st->cmd_settings_bitmask & TI_ADS7950_CR_RANGE_5V)
>  		vref *= 2;
> @@ -382,11 +372,7 @@ static int ti_ads7950_read_raw(struct iio_dev *indio_dev,
>  
>  		return IIO_VAL_INT;
>  	case IIO_CHAN_INFO_SCALE:
> -		ret = ti_ads7950_get_range(st);
> -		if (ret < 0)
> -			return ret;
> -
> -		*val = ret;
> +		*val = ti_ads7950_get_range(st);
>  		*val2 = (1 << chan->scan_type.realbits) - 1;
>  
>  		return IIO_VAL_FRACTIONAL;
> @@ -580,30 +566,24 @@ static int ti_ads7950_probe(struct spi_device *spi)
>  	spi_message_init_with_transfers(&st->scan_single_msg,
>  					st->scan_single_xfer, 3);
>  
> +	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref");
> +	if (ret < 0)

I think you need to check for -ENODEV and if you see than then
see if the acpi route below applies.  Otherwise on ACPI this will fail
and we'll fail the probe.


> +		return dev_err_probe(&spi->dev, ret,
> +				     "Failed to get regulator \"vref\"\n");
> +
>  	/* Use hard coded value for reference voltage in ACPI case */
>  	if (ACPI_COMPANION(&spi->dev))
>  		st->vref_mv = TI_ADS7950_VA_MV_ACPI_DEFAULT;
> +	else
> +		st->vref_mv = ret / 1000;

Re: [PATCH v3 5/6] iio: adc: ti-ads7950: switch to using devm_regulator_get_enable_read_voltage()

[David Lechner]

On 3/7/26 5:49 AM, Jonathan Cameron wrote:
> On Thu, 05 Mar 2026 11:21:56 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
>> The regulator is enabled for the entire time the driver is bound to the
>> device, and we only need to access it to fetch voltage, which can be
>> done at probe time.
>>
>> Switch to using devm_regulator_get_enable_read_voltage() which
>> simplifies probing and unbinding code.
>>
>> Suggested-by: David Lechner <dlechner@baylibre.com>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Hi.
> 
> I think this broke the ACPI case (where regulator isn't available).
> 
> Jonathan
> 
>> ---
>>  drivers/iio/adc/ti-ads7950.c | 45 +++++++++++---------------------------------
>>  1 file changed, 11 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
>> index 273c35e03185..847e83baa876 100644
>> --- a/drivers/iio/adc/ti-ads7950.c
>> +++ b/drivers/iio/adc/ti-ads7950.c
>> @@ -341,19 +341,9 @@ static int ti_ads7950_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
>>  	return st->single_rx;
>>  }
>>  
>> -static int ti_ads7950_get_range(struct ti_ads7950_state *st)
>> +static unsigned int ti_ads7950_get_range(struct ti_ads7950_state *st)
>>  {
>> -	int vref;
>> -
>> -	if (st->vref_mv) {
>> -		vref = st->vref_mv;
>> -	} else {
>> -		vref = regulator_get_voltage(st->reg);
>> -		if (vref < 0)
>> -			return vref;
>> -
>> -		vref /= 1000;
>> -	}
>> +	unsigned int vref = st->vref_mv;
>>  
>>  	if (st->cmd_settings_bitmask & TI_ADS7950_CR_RANGE_5V)
>>  		vref *= 2;
>> @@ -382,11 +372,7 @@ static int ti_ads7950_read_raw(struct iio_dev *indio_dev,
>>  
>>  		return IIO_VAL_INT;
>>  	case IIO_CHAN_INFO_SCALE:
>> -		ret = ti_ads7950_get_range(st);
>> -		if (ret < 0)
>> -			return ret;
>> -
>> -		*val = ret;
>> +		*val = ti_ads7950_get_range(st);
>>  		*val2 = (1 << chan->scan_type.realbits) - 1;
>>  
>>  		return IIO_VAL_FRACTIONAL;
>> @@ -580,30 +566,24 @@ static int ti_ads7950_probe(struct spi_device *spi)
>>  	spi_message_init_with_transfers(&st->scan_single_msg,
>>  					st->scan_single_xfer, 3);
>>  
>> +	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref");
>> +	if (ret < 0)
> 
> I think you need to check for -ENODEV and if you see than then
> see if the acpi route below applies.  Otherwise on ACPI this will fail
> and we'll fail the probe.
> 

Or do something like:

	if (ACPI_COMPANION(&spi->dev)) {
		ret = devm_regulator_get_enable(&spi->dev, "vref");
		if (ret)
			return dev_err_probe(&spi->dev, ret,
					     "Failed to get regulator \"vref\"\n");

 		st->vref_mv = TI_ADS7950_VA_MV_ACPI_DEFAULT;
	} else {

		ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref");
		if (ret < 0)
			return dev_err_probe(&spi->dev, ret,
					     "Failed to get regulator \"vref\"\n");

		st->vref_mv = ret / 1000;
	}

> 
>> +		return dev_err_probe(&spi->dev, ret,
>> +				     "Failed to get regulator \"vref\"\n");
>> +
>>  	/* Use hard coded value for reference voltage in ACPI case */
>>  	if (ACPI_COMPANION(&spi->dev))
>>  		st->vref_mv = TI_ADS7950_VA_MV_ACPI_DEFAULT;
>> +	else
>> +		st->vref_mv = ret / 1000;
> 
> 

Re: [PATCH v3 5/6] iio: adc: ti-ads7950: switch to using devm_regulator_get_enable_read_voltage()

[Dmitry Torokhov]

On Sat, Mar 07, 2026 at 11:43:32AM -0600, David Lechner wrote:
> On 3/7/26 5:49 AM, Jonathan Cameron wrote:
> > On Thu, 05 Mar 2026 11:21:56 -0800
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > 
> >> The regulator is enabled for the entire time the driver is bound to the
> >> device, and we only need to access it to fetch voltage, which can be
> >> done at probe time.
> >>
> >> Switch to using devm_regulator_get_enable_read_voltage() which
> >> simplifies probing and unbinding code.
> >>
> >> Suggested-by: David Lechner <dlechner@baylibre.com>
> >> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Hi.
> > 
> > I think this broke the ACPI case (where regulator isn't available).

Oh, you're right.

> > 
> > Jonathan
> > 
> >> ---
> >>  drivers/iio/adc/ti-ads7950.c | 45 +++++++++++---------------------------------
> >>  1 file changed, 11 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
> >> index 273c35e03185..847e83baa876 100644
> >> --- a/drivers/iio/adc/ti-ads7950.c
> >> +++ b/drivers/iio/adc/ti-ads7950.c
> >> @@ -341,19 +341,9 @@ static int ti_ads7950_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
> >>  	return st->single_rx;
> >>  }
> >>  
> >> -static int ti_ads7950_get_range(struct ti_ads7950_state *st)
> >> +static unsigned int ti_ads7950_get_range(struct ti_ads7950_state *st)
> >>  {
> >> -	int vref;
> >> -
> >> -	if (st->vref_mv) {
> >> -		vref = st->vref_mv;
> >> -	} else {
> >> -		vref = regulator_get_voltage(st->reg);
> >> -		if (vref < 0)
> >> -			return vref;
> >> -
> >> -		vref /= 1000;
> >> -	}
> >> +	unsigned int vref = st->vref_mv;
> >>  
> >>  	if (st->cmd_settings_bitmask & TI_ADS7950_CR_RANGE_5V)
> >>  		vref *= 2;
> >> @@ -382,11 +372,7 @@ static int ti_ads7950_read_raw(struct iio_dev *indio_dev,
> >>  
> >>  		return IIO_VAL_INT;
> >>  	case IIO_CHAN_INFO_SCALE:
> >> -		ret = ti_ads7950_get_range(st);
> >> -		if (ret < 0)
> >> -			return ret;
> >> -
> >> -		*val = ret;
> >> +		*val = ti_ads7950_get_range(st);
> >>  		*val2 = (1 << chan->scan_type.realbits) - 1;
> >>  
> >>  		return IIO_VAL_FRACTIONAL;
> >> @@ -580,30 +566,24 @@ static int ti_ads7950_probe(struct spi_device *spi)
> >>  	spi_message_init_with_transfers(&st->scan_single_msg,
> >>  					st->scan_single_xfer, 3);
> >>  
> >> +	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref");
> >> +	if (ret < 0)
> > 
> > I think you need to check for -ENODEV and if you see than then
> > see if the acpi route below applies.  Otherwise on ACPI this will fail
> > and we'll fail the probe.
> > 
> 
> Or do something like:
> 
> 	if (ACPI_COMPANION(&spi->dev)) {
> 		ret = devm_regulator_get_enable(&spi->dev, "vref");
> 		if (ret)
> 			return dev_err_probe(&spi->dev, ret,
> 					     "Failed to get regulator \"vref\"\n");
> 
>  		st->vref_mv = TI_ADS7950_VA_MV_ACPI_DEFAULT;

We know that concept of regulators is not exposed on ACPI systems, and
we'd get a dummy here, so maybe just store st->vref_mv and not bother
with acquiring and enabling the dummy regulator?

Thanks.

-- 
Dmitry

Re: [PATCH v3 5/6] iio: adc: ti-ads7950: switch to using devm_regulator_get_enable_read_voltage()

[David Lechner]

On 3/7/26 12:04 PM, Dmitry Torokhov wrote:
> On Sat, Mar 07, 2026 at 11:43:32AM -0600, David Lechner wrote:
>> On 3/7/26 5:49 AM, Jonathan Cameron wrote:
>>> On Thu, 05 Mar 2026 11:21:56 -0800
>>> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>>>
>>>> The regulator is enabled for the entire time the driver is bound to the
>>>> device, and we only need to access it to fetch voltage, which can be
>>>> done at probe time.
>>>>
>>>> Switch to using devm_regulator_get_enable_read_voltage() which
>>>> simplifies probing and unbinding code.
>>>>
>>>> Suggested-by: David Lechner <dlechner@baylibre.com>
>>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>> Hi.
>>>
>>> I think this broke the ACPI case (where regulator isn't available).
> 
> Oh, you're right.
> 
>>>
>>> Jonathan
>>>
>>>> ---
>>>>  drivers/iio/adc/ti-ads7950.c | 45 +++++++++++---------------------------------
>>>>  1 file changed, 11 insertions(+), 34 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
>>>> index 273c35e03185..847e83baa876 100644
>>>> --- a/drivers/iio/adc/ti-ads7950.c
>>>> +++ b/drivers/iio/adc/ti-ads7950.c
>>>> @@ -341,19 +341,9 @@ static int ti_ads7950_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
>>>>  	return st->single_rx;
>>>>  }
>>>>  
>>>> -static int ti_ads7950_get_range(struct ti_ads7950_state *st)
>>>> +static unsigned int ti_ads7950_get_range(struct ti_ads7950_state *st)
>>>>  {
>>>> -	int vref;
>>>> -
>>>> -	if (st->vref_mv) {
>>>> -		vref = st->vref_mv;
>>>> -	} else {
>>>> -		vref = regulator_get_voltage(st->reg);
>>>> -		if (vref < 0)
>>>> -			return vref;
>>>> -
>>>> -		vref /= 1000;
>>>> -	}
>>>> +	unsigned int vref = st->vref_mv;
>>>>  
>>>>  	if (st->cmd_settings_bitmask & TI_ADS7950_CR_RANGE_5V)
>>>>  		vref *= 2;
>>>> @@ -382,11 +372,7 @@ static int ti_ads7950_read_raw(struct iio_dev *indio_dev,
>>>>  
>>>>  		return IIO_VAL_INT;
>>>>  	case IIO_CHAN_INFO_SCALE:
>>>> -		ret = ti_ads7950_get_range(st);
>>>> -		if (ret < 0)
>>>> -			return ret;
>>>> -
>>>> -		*val = ret;
>>>> +		*val = ti_ads7950_get_range(st);
>>>>  		*val2 = (1 << chan->scan_type.realbits) - 1;
>>>>  
>>>>  		return IIO_VAL_FRACTIONAL;
>>>> @@ -580,30 +566,24 @@ static int ti_ads7950_probe(struct spi_device *spi)
>>>>  	spi_message_init_with_transfers(&st->scan_single_msg,
>>>>  					st->scan_single_xfer, 3);
>>>>  
>>>> +	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref");
>>>> +	if (ret < 0)
>>>
>>> I think you need to check for -ENODEV and if you see than then
>>> see if the acpi route below applies.  Otherwise on ACPI this will fail
>>> and we'll fail the probe.
>>>
>>
>> Or do something like:
>>
>> 	if (ACPI_COMPANION(&spi->dev)) {
>> 		ret = devm_regulator_get_enable(&spi->dev, "vref");
>> 		if (ret)
>> 			return dev_err_probe(&spi->dev, ret,
>> 					     "Failed to get regulator \"vref\"\n");
>>
>>  		st->vref_mv = TI_ADS7950_VA_MV_ACPI_DEFAULT;
> 
> We know that concept of regulators is not exposed on ACPI systems, and
> we'd get a dummy here, so maybe just store st->vref_mv and not bother
> with acquiring and enabling the dummy regulator?
> 
> Thanks.
> 

Sounds OK to me.

Re: [PATCH v3 5/6] iio: adc: ti-ads7950: switch to using devm_regulator_get_enable_read_voltage()

[Jonathan Cameron]

On Sat, 7 Mar 2026 12:07:40 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 3/7/26 12:04 PM, Dmitry Torokhov wrote:
> > On Sat, Mar 07, 2026 at 11:43:32AM -0600, David Lechner wrote:  
> >> On 3/7/26 5:49 AM, Jonathan Cameron wrote:  
> >>> On Thu, 05 Mar 2026 11:21:56 -0800
> >>> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> >>>  
> >>>> The regulator is enabled for the entire time the driver is bound to the
> >>>> device, and we only need to access it to fetch voltage, which can be
> >>>> done at probe time.
> >>>>
> >>>> Switch to using devm_regulator_get_enable_read_voltage() which
> >>>> simplifies probing and unbinding code.
> >>>>
> >>>> Suggested-by: David Lechner <dlechner@baylibre.com>
> >>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>  
> >>> Hi.
> >>>
> >>> I think this broke the ACPI case (where regulator isn't available).  
> > 
> > Oh, you're right.
> >   
> >>>
> >>> Jonathan
> >>>  
> >>>> ---
> >>>>  drivers/iio/adc/ti-ads7950.c | 45 +++++++++++---------------------------------
> >>>>  1 file changed, 11 insertions(+), 34 deletions(-)
> >>>>
> >>>> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
> >>>> index 273c35e03185..847e83baa876 100644
> >>>> --- a/drivers/iio/adc/ti-ads7950.c
> >>>> +++ b/drivers/iio/adc/ti-ads7950.c
> >>>> @@ -341,19 +341,9 @@ static int ti_ads7950_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
> >>>>  	return st->single_rx;
> >>>>  }
> >>>>  
> >>>> -static int ti_ads7950_get_range(struct ti_ads7950_state *st)
> >>>> +static unsigned int ti_ads7950_get_range(struct ti_ads7950_state *st)
> >>>>  {
> >>>> -	int vref;
> >>>> -
> >>>> -	if (st->vref_mv) {
> >>>> -		vref = st->vref_mv;
> >>>> -	} else {
> >>>> -		vref = regulator_get_voltage(st->reg);
> >>>> -		if (vref < 0)
> >>>> -			return vref;
> >>>> -
> >>>> -		vref /= 1000;
> >>>> -	}
> >>>> +	unsigned int vref = st->vref_mv;
> >>>>  
> >>>>  	if (st->cmd_settings_bitmask & TI_ADS7950_CR_RANGE_5V)
> >>>>  		vref *= 2;
> >>>> @@ -382,11 +372,7 @@ static int ti_ads7950_read_raw(struct iio_dev *indio_dev,
> >>>>  
> >>>>  		return IIO_VAL_INT;
> >>>>  	case IIO_CHAN_INFO_SCALE:
> >>>> -		ret = ti_ads7950_get_range(st);
> >>>> -		if (ret < 0)
> >>>> -			return ret;
> >>>> -
> >>>> -		*val = ret;
> >>>> +		*val = ti_ads7950_get_range(st);
> >>>>  		*val2 = (1 << chan->scan_type.realbits) - 1;
> >>>>  
> >>>>  		return IIO_VAL_FRACTIONAL;
> >>>> @@ -580,30 +566,24 @@ static int ti_ads7950_probe(struct spi_device *spi)
> >>>>  	spi_message_init_with_transfers(&st->scan_single_msg,
> >>>>  					st->scan_single_xfer, 3);
> >>>>  
> >>>> +	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref");
> >>>> +	if (ret < 0)  
> >>>
> >>> I think you need to check for -ENODEV and if you see than then
> >>> see if the acpi route below applies.  Otherwise on ACPI this will fail
> >>> and we'll fail the probe.
> >>>  
> >>
> >> Or do something like:
> >>
> >> 	if (ACPI_COMPANION(&spi->dev)) {
> >> 		ret = devm_regulator_get_enable(&spi->dev, "vref");
> >> 		if (ret)
> >> 			return dev_err_probe(&spi->dev, ret,
> >> 					     "Failed to get regulator \"vref\"\n");
> >>
> >>  		st->vref_mv = TI_ADS7950_VA_MV_ACPI_DEFAULT;  
> > 
> > We know that concept of regulators is not exposed on ACPI systems, and
> > we'd get a dummy here, so maybe just store st->vref_mv and not bother
> > with acquiring and enabling the dummy regulator?
> > 
> > Thanks.
> >   
> 
> Sounds OK to me.
Likewise.

J

Re: [PATCH v3 5/6] iio: adc: ti-ads7950: switch to using devm_regulator_get_enable_read_voltage()

[Andy Shevchenko]

On Sat, Mar 07, 2026 at 11:49:47AM +0000, Jonathan Cameron wrote:
> On Thu, 05 Mar 2026 11:21:56 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

...

> I think this broke the ACPI case (where regulator isn't available).

Right, and I even have an HW to test (if required).

...

> > +	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref");
> > +	if (ret < 0)
> 
> I think you need to check for -ENODEV and if you see than then
> see if the acpi route below applies.  Otherwise on ACPI this will fail
> and we'll fail the probe.

Yep, just make it default without even checking the fwnode type.
No need to bring the whole acpi.h into the mix.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 5/6] iio: adc: ti-ads7950: switch to using devm_regulator_get_enable_read_voltage()

[Dmitry Torokhov]

On Sun, Mar 08, 2026 at 10:32:13PM +0200, Andy Shevchenko wrote:
> On Sat, Mar 07, 2026 at 11:49:47AM +0000, Jonathan Cameron wrote:
> > On Thu, 05 Mar 2026 11:21:56 -0800
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> ...
> 
> > I think this broke the ACPI case (where regulator isn't available).
> 
> Right, and I even have an HW to test (if required).
> 
> ...
> 
> > > +	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref");
> > > +	if (ret < 0)
> > 
> > I think you need to check for -ENODEV and if you see than then
> > see if the acpi route below applies.  Otherwise on ACPI this will fail
> > and we'll fail the probe.
> 
> Yep, just make it default without even checking the fwnode type.
> No need to bring the whole acpi.h into the mix.

I do not think this would be correct behavior. On non-ACPI systems the
regulator is mandatory, and the driver should not blindly apply scale
from ACPI systems just because regulator is missing in the DTS.

Thanks.

-- 
Dmitry

Re: [PATCH v3 5/6] iio: adc: ti-ads7950: switch to using devm_regulator_get_enable_read_voltage()

[Andy Shevchenko]

On Mon, Mar 09, 2026 at 05:35:58AM +0000, Dmitry Torokhov wrote:
> On Sun, Mar 08, 2026 at 10:32:13PM +0200, Andy Shevchenko wrote:
> > On Sat, Mar 07, 2026 at 11:49:47AM +0000, Jonathan Cameron wrote:
> > > On Thu, 05 Mar 2026 11:21:56 -0800
> > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

...

> > > > +	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref");
> > > > +	if (ret < 0)
> > > 
> > > I think you need to check for -ENODEV and if you see than then
> > > see if the acpi route below applies.  Otherwise on ACPI this will fail
> > > and we'll fail the probe.
> > 
> > Yep, just make it default without even checking the fwnode type.
> > No need to bring the whole acpi.h into the mix.
> 
> I do not think this would be correct behavior. On non-ACPI systems the
> regulator is mandatory, and the driver should not blindly apply scale
> from ACPI systems just because regulator is missing in the DTS.

I mistakenly thought that original code doesn't have that cneck. Yes, the
ACPI_COMPANION() should be preserved.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 6/6] iio: adc: ti-ads7950: complete conversion to using managed resources

[Dmitry Torokhov]

All resources that the driver needs have managed API now. Switch to
using them to make code clearer and drop ti_ads7950_remove().

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/iio/adc/ti-ads7950.c | 66 +++++++++++++-------------------------------
 1 file changed, 19 insertions(+), 47 deletions(-)

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index 847e83baa876..a1197e9cff93 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -513,10 +513,8 @@ static int ti_ads7950_probe(struct spi_device *spi)
 	spi->bits_per_word = 16;
 	spi->mode |= SPI_CS_WORD;
 	ret = spi_setup(spi);
-	if (ret) {
-		dev_err(&spi->dev, "Error in spi setup\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&spi->dev, ret, "Error in spi setup\n");
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
 	if (!indio_dev)
@@ -577,26 +575,22 @@ static int ti_ads7950_probe(struct spi_device *spi)
 	else
 		st->vref_mv = ret / 1000;
 
-	mutex_init(&st->slock);
-
-	ret = iio_triggered_buffer_setup(indio_dev, NULL,
-					 &ti_ads7950_trigger_handler, NULL);
-	if (ret) {
-		dev_err(&spi->dev, "Failed to setup triggered buffer\n");
-		goto error_destroy_mutex;
-	}
+	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev, NULL,
+					      &ti_ads7950_trigger_handler,
+					      NULL);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret,
+				     "Failed to setup triggered buffer\n");
 
 	ret = ti_ads7950_init_hw(st);
-	if (ret) {
-		dev_err(&spi->dev, "Failed to init adc chip\n");
-		goto error_cleanup_ring;
-	}
+	if (ret)
+		return dev_err_probe(&spi->dev, ret,
+				     "Failed to init adc chip\n");
 
-	ret = iio_device_register(indio_dev);
-	if (ret) {
-		dev_err(&spi->dev, "Failed to register iio device\n");
-		goto error_cleanup_ring;
-	}
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret,
+				     "Failed to register iio device\n");
 
 	/* Add GPIO chip */
 	st->chip.label = dev_name(&st->spi->dev);
@@ -611,33 +605,12 @@ static int ti_ads7950_probe(struct spi_device *spi)
 	st->chip.get = ti_ads7950_get;
 	st->chip.set = ti_ads7950_set;
 
-	ret = gpiochip_add_data(&st->chip, st);
-	if (ret) {
-		dev_err(&spi->dev, "Failed to init GPIOs\n");
-		goto error_iio_device;
-	}
+	ret = devm_gpiochip_add_data(&spi->dev, &st->chip, st);
+	if (ret)
+		return dev_err_probe(&spi->dev, ret,
+				     "Failed to init GPIOs\n");
 
 	return 0;
-
-error_iio_device:
-	iio_device_unregister(indio_dev);
-error_cleanup_ring:
-	iio_triggered_buffer_cleanup(indio_dev);
-error_destroy_mutex:
-	mutex_destroy(&st->slock);
-
-	return ret;
-}
-
-static void ti_ads7950_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct ti_ads7950_state *st = iio_priv(indio_dev);
-
-	gpiochip_remove(&st->chip);
-	iio_device_unregister(indio_dev);
-	iio_triggered_buffer_cleanup(indio_dev);
-	mutex_destroy(&st->slock);
 }
 
 static const struct spi_device_id ti_ads7950_id[] = {
@@ -680,7 +653,6 @@ static struct spi_driver ti_ads7950_driver = {
 		.of_match_table = ads7950_of_table,
 	},
 	.probe		= ti_ads7950_probe,
-	.remove		= ti_ads7950_remove,
 	.id_table	= ti_ads7950_id,
 };
 module_spi_driver(ti_ads7950_driver);

-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH v3 6/6] iio: adc: ti-ads7950: complete conversion to using managed resources

[Andy Shevchenko]

On Thu, Mar 05, 2026 at 11:21:57AM -0800, Dmitry Torokhov wrote:
> All resources that the driver needs have managed API now. Switch to
> using them to make code clearer and drop ti_ads7950_remove().

...

> -	mutex_init(&st->slock);

I missed seeing the + lines WRT the above removal.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 6/6] iio: adc: ti-ads7950: complete conversion to using managed resources

[Dmitry Torokhov]

On Fri, Mar 06, 2026 at 02:21:00PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 05, 2026 at 11:21:57AM -0800, Dmitry Torokhov wrote:
> > All resources that the driver needs have managed API now. Switch to
> > using them to make code clearer and drop ti_ads7950_remove().
> 
> ...
> 
> > -	mutex_init(&st->slock);
> 
> I missed seeing the + lines WRT the above removal.
> 

Oops, will fix it up.

-- 
Dmitry

Re: [PATCH v3 6/6] iio: adc: ti-ads7950: complete conversion to using managed resources

[David Lechner]

On 3/5/26 1:21 PM, Dmitry Torokhov wrote:
> All resources that the driver needs have managed API now. Switch to
> using them to make code clearer and drop ti_ads7950_remove().
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---

...

> @@ -577,26 +575,22 @@ static int ti_ads7950_probe(struct spi_device *spi)
>  	else
>  		st->vref_mv = ret / 1000;
>  
> -	mutex_init(&st->slock);

With this accidental removal fixed:

Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH v3 6/6] iio: adc: ti-ads7950: complete conversion to using managed resources

[David Lechner]

On 3/5/26 1:21 PM, Dmitry Torokhov wrote:
> All resources that the driver needs have managed API now. Switch to
> using them to make code clearer and drop ti_ads7950_remove().
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---

One more small detail...

> -static void ti_ads7950_remove(struct spi_device *spi)
> -{
> -	struct iio_dev *indio_dev = spi_get_drvdata(spi);

Since we are removing the only instance of spi_get_drvdata(), we can
also remove spi_set_drvdata() in the probe function

> -	struct ti_ads7950_state *st = iio_priv(indio_dev);
> -
> -	gpiochip_remove(&st->chip);
> -	iio_device_unregister(indio_dev);
> -	iio_triggered_buffer_cleanup(indio_dev);
> -	mutex_destroy(&st->slock);
>  }
>  

Re: [PATCH v3 0/6] ti-ads7950: fix gpio handling and facelift

[Jonathan Cameron]

On Thu, 05 Mar 2026 11:21:51 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> The original reason for this series is to make sure ti_ads7950_get()
> returns result in range [0, 1] or a negative error code to comply with
> gpiolib requirement.
> 
> During review David noticed the fact that the function also clobbers
> gpio state. Fixing that lead to adding cleanups using guard() and devm.
> 
Applied patches 1 and 2 to the fixes-togreg branch of iio.git and marked
them for stable.

The rest will need to wait for those to be upstream.  Given I have the
memory of a goldfish and there will it seems be a v4, please mention
that in the cover letter.

Thanks,

Jonathan

> v3:
> 
> - Added "Fixes" tags
> - Picked up reviewed-by tags for #1 and #2
> - Dropped the conversion to "error" variables
> - Avoid using scoped_guard (per David)
> - directly return results of function calls at the end of function (David)
> - Split spi_sync() error handling into a new patxch (Andy)
> - Switch to using devm_regulator_get_enable_read_voltage() (David)
> 
> v2:
> 
> - style fixes in #1 (per David)
> - fix to not clobber state (#2, new)
> - #3 and #4 (new)
> 
> ---
> Dmitry Torokhov (6):
>       iio: adc: ti-ads7950: normalize return value of gpio_get
>       iio: adc: ti-ads7950: do not clobber gpio state in ti_ads7950_get()
>       iio: adc: ti-ads7950: switch to using guard() notation
>       iio: adc: ti-ads7950: simplify check for spi_setup() failures
>       iio: adc: ti-ads7950: switch to using devm_regulator_get_enable_read_voltage()
>       iio: adc: ti-ads7950: complete conversion to using managed resources
> 
>  drivers/iio/adc/ti-ads7950.c | 188 ++++++++++++++-----------------------------
>  1 file changed, 59 insertions(+), 129 deletions(-)
> ---
> base-commit: c025f6cf4209e1542ec2afebe49f42bbaf1a5c7b
> change-id: 20260220-ti-ads7950-facelift-ad8b5390a654
> 
> Thanks.
>