[PATCH v4 0/4] ti-ads7950: fix gpio handling and facelift

[PATCH v4 0/4] 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.

---
Changes in v4:
- rebased on top of latest next
- dropped the 2 already applied patches
- restored inadvertently removed mutex_init() (Andy)
- stopped calling devm_regulator_get_enable_read_voltage() on ACPI
  systems as it is guaranteed to fail (David, Jonathan)
- removed call to spi_set_drvdata() since it is not needed anymore (David)
- Link to v3: https://patch.msgid.link/20260305-ti-ads7950-facelift-v3-0-a23fdd1a079e@gmail.com

Changes in 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)

Changes in v2:

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

---
Dmitry Torokhov (4):
      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 | 195 ++++++++++++++-----------------------------
 1 file changed, 62 insertions(+), 133 deletions(-)
---
base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
change-id: 20260220-ti-ads7950-facelift-ad8b5390a654

Thanks.

-- 
Dmitry

[PATCH v4 1/4] 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.

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

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index 028acd42741f..6e9ea9cc33bf 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -299,18 +299,19 @@ 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;
-
-	iio_push_to_buffers_with_ts_unaligned(indio_dev, &st->rx_buf[2],
-					      sizeof(*st->rx_buf) *
-					      TI_ADS7950_MAX_CHAN,
-					      iio_get_time_ns(indio_dev));
-
-out:
-	mutex_unlock(&st->slock);
+	do {
+		guard(mutex)(&st->slock);
+
+		ret = spi_sync(st->spi, &st->ring_msg);
+		if (ret)
+			break;
+
+		iio_push_to_buffers_with_ts_unaligned(indio_dev, &st->rx_buf[2],
+						      sizeof(*st->rx_buf) *
+						      TI_ADS7950_MAX_CHAN,
+						      iio_get_time_ns(indio_dev));
+	} while (0);
+
 	iio_trigger_notify_done(indio_dev->trig);
 
 	return IRQ_HANDLED;
@@ -321,20 +322,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)
@@ -400,9 +397,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);
@@ -410,11 +406,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)
@@ -423,13 +416,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 */
@@ -437,7 +429,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);
 
@@ -446,12 +438,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,
@@ -467,9 +456,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)))
@@ -477,15 +465,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,
@@ -508,9 +492,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 */
@@ -518,17 +502,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.1018.g2bb0e51243-goog

Re: [PATCH v4 1/4] iio: adc: ti-ads7950: switch to using guard() notation

[Bartosz Golaszewski]

On Mon, 30 Mar 2026 00:47:06 +0200, Dmitry Torokhov
<dmitry.torokhov@gmail.com> said:
> guard() notation allows early returns when encountering errors, making
> control flow more obvious. Use it.
>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/iio/adc/ti-ads7950.c | 83 +++++++++++++++++---------------------------
>  1 file changed, 31 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
> index 028acd42741f..6e9ea9cc33bf 100644
> --- a/drivers/iio/adc/ti-ads7950.c
> +++ b/drivers/iio/adc/ti-ads7950.c
> @@ -299,18 +299,19 @@ 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;
> -
> -	iio_push_to_buffers_with_ts_unaligned(indio_dev, &st->rx_buf[2],
> -					      sizeof(*st->rx_buf) *
> -					      TI_ADS7950_MAX_CHAN,
> -					      iio_get_time_ns(indio_dev));
> -
> -out:
> -	mutex_unlock(&st->slock);
> +	do {
> +		guard(mutex)(&st->slock);

Am I missing something? Why isn't it just a:

	scoped_guard(mutex, &st->slock) {
		...
	}

?

Bart

Re: [PATCH v4 1/4] iio: adc: ti-ads7950: switch to using guard() notation

[Dmitry Torokhov]

On March 30, 2026 2:15:42 AM PDT, Bartosz Golaszewski <brgl@kernel.org> wrote:
>On Mon, 30 Mar 2026 00:47:06 +0200, Dmitry Torokhov
><dmitry.torokhov@gmail.com> said:
>> guard() notation allows early returns when encountering errors, making
>> control flow more obvious. Use it.
>>
>> Reviewed-by: David Lechner <dlechner@baylibre.com>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> ---
>>  drivers/iio/adc/ti-ads7950.c | 83 +++++++++++++++++---------------------------
>>  1 file changed, 31 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
>> index 028acd42741f..6e9ea9cc33bf 100644
>> --- a/drivers/iio/adc/ti-ads7950.c
>> +++ b/drivers/iio/adc/ti-ads7950.c
>> @@ -299,18 +299,19 @@ 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;
>> -
>> -	iio_push_to_buffers_with_ts_unaligned(indio_dev, &st->rx_buf[2],
>> -					      sizeof(*st->rx_buf) *
>> -					      TI_ADS7950_MAX_CHAN,
>> -					      iio_get_time_ns(indio_dev));
>> -
>> -out:
>> -	mutex_unlock(&st->slock);
>> +	do {
>> +		guard(mutex)(&st->slock);
>
>Am I missing something? Why isn't it just a:
>
>	scoped_guard(mutex, &st->slock) {
>		...
>	}

Maintainer's preference. It was a scoped guard in the first iteration. 

Thanks. 

-- 
Dmitry

Re: [PATCH v4 1/4] iio: adc: ti-ads7950: switch to using guard() notation

[Bartosz Golaszewski]

On Mon, 30 Mar 2026 11:18:14 +0200, Dmitry Torokhov
<dmitry.torokhov@gmail.com> said:
> On March 30, 2026 2:15:42 AM PDT, Bartosz Golaszewski <brgl@kernel.org> wrote:
>>On Mon, 30 Mar 2026 00:47:06 +0200, Dmitry Torokhov
>><dmitry.torokhov@gmail.com> said:
>>> guard() notation allows early returns when encountering errors, making
>>> control flow more obvious. Use it.
>>>
>>> Reviewed-by: David Lechner <dlechner@baylibre.com>
>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>> ---
>>>  drivers/iio/adc/ti-ads7950.c | 83 +++++++++++++++++---------------------------
>>>  1 file changed, 31 insertions(+), 52 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
>>> index 028acd42741f..6e9ea9cc33bf 100644
>>> --- a/drivers/iio/adc/ti-ads7950.c
>>> +++ b/drivers/iio/adc/ti-ads7950.c
>>> @@ -299,18 +299,19 @@ 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;
>>> -
>>> -	iio_push_to_buffers_with_ts_unaligned(indio_dev, &st->rx_buf[2],
>>> -					      sizeof(*st->rx_buf) *
>>> -					      TI_ADS7950_MAX_CHAN,
>>> -					      iio_get_time_ns(indio_dev));
>>> -
>>> -out:
>>> -	mutex_unlock(&st->slock);
>>> +	do {
>>> +		guard(mutex)(&st->slock);
>>
>>Am I missing something? Why isn't it just a:
>>
>>	scoped_guard(mutex, &st->slock) {
>>		...
>>	}
>
> Maintainer's preference. It was a scoped guard in the first iteration.
>

Fair enough, though I don't really understand that. It looks less readable this
way IMO.

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH v4 1/4] iio: adc: ti-ads7950: switch to using guard() notation

[Jonathan Cameron]

On Mon, 30 Mar 2026 05:21:25 -0400
Bartosz Golaszewski <brgl@kernel.org> wrote:

> On Mon, 30 Mar 2026 11:18:14 +0200, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> said:
> > On March 30, 2026 2:15:42 AM PDT, Bartosz Golaszewski <brgl@kernel.org> wrote:  
> >>On Mon, 30 Mar 2026 00:47:06 +0200, Dmitry Torokhov
> >><dmitry.torokhov@gmail.com> said:  
> >>> guard() notation allows early returns when encountering errors, making
> >>> control flow more obvious. Use it.
> >>>
> >>> Reviewed-by: David Lechner <dlechner@baylibre.com>
> >>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >>> ---
> >>>  drivers/iio/adc/ti-ads7950.c | 83 +++++++++++++++++---------------------------
> >>>  1 file changed, 31 insertions(+), 52 deletions(-)
> >>>
> >>> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
> >>> index 028acd42741f..6e9ea9cc33bf 100644
> >>> --- a/drivers/iio/adc/ti-ads7950.c
> >>> +++ b/drivers/iio/adc/ti-ads7950.c
> >>> @@ -299,18 +299,19 @@ 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;
> >>> -
> >>> -	iio_push_to_buffers_with_ts_unaligned(indio_dev, &st->rx_buf[2],
> >>> -					      sizeof(*st->rx_buf) *
> >>> -					      TI_ADS7950_MAX_CHAN,
> >>> -					      iio_get_time_ns(indio_dev));
> >>> -
> >>> -out:
> >>> -	mutex_unlock(&st->slock);
> >>> +	do {
> >>> +		guard(mutex)(&st->slock);  
> >>
> >>Am I missing something? Why isn't it just a:
> >>
> >>	scoped_guard(mutex, &st->slock) {
> >>		...
> >>	}  
> >
> > Maintainer's preference. It was a scoped guard in the first iteration.
> >  
> 
> Fair enough, though I don't really understand that. It looks less readable this
> way IMO.

The reasoning is the subtle nature of what a break means in scoped_guard().
Perhaps one day we'll get used to treating those as loops (which they are
under the hood).  The do {} while() pattern makes the loop visible.

Jonathan

> 
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> 

[PATCH v4 2/4] 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.

Reviewed-by: David Lechner <dlechner@baylibre.com>
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 6e9ea9cc33bf..c31c706c92a9 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -520,7 +520,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.1018.g2bb0e51243-goog

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

[Bartosz Golaszewski]

On Mon, 30 Mar 2026 00:47:07 +0200, Dmitry Torokhov
<dmitry.torokhov@gmail.com> said:
> 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.
>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> 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 6e9ea9cc33bf..c31c706c92a9 100644
> --- a/drivers/iio/adc/ti-ads7950.c
> +++ b/drivers/iio/adc/ti-ads7950.c
> @@ -520,7 +520,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.1018.g2bb0e51243-goog
>
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

[PATCH v4 3/4] 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 | 48 ++++++++++++--------------------------------
 1 file changed, 13 insertions(+), 35 deletions(-)

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index c31c706c92a9..0b98c8e7385d 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -334,19 +334,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;
@@ -375,11 +365,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;
@@ -573,30 +559,25 @@ static int ti_ads7950_probe(struct spi_device *spi)
 	spi_message_init_with_transfers(&st->scan_single_msg,
 					st->scan_single_xfer, 3);
 
-	/* Use hard coded value for reference voltage in ACPI case */
-	if (ACPI_COMPANION(&spi->dev))
-		st->vref_mv = TI_ADS7950_VA_MV_ACPI_DEFAULT;
-
 	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;
-	}
+	/* 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 {
+		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");
 
-	ret = regulator_enable(st->reg);
-	if (ret) {
-		dev_err(&spi->dev, "Failed to enable regulator \"vref\"\n");
-		goto error_destroy_mutex;
+		st->vref_mv = ret / 1000;
 	}
 
 	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);
@@ -636,8 +617,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);
 
@@ -652,7 +631,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.1018.g2bb0e51243-goog

Re: [PATCH v4 3/4] iio: adc: ti-ads7950: switch to using devm_regulator_get_enable_read_voltage()

[Bartosz Golaszewski]

On Mon, 30 Mar 2026 00:47:08 +0200, Dmitry Torokhov
<dmitry.torokhov@gmail.com> said:
> 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

These multi-function helpers are getting out of control. :)

> simplifies probing and unbinding code.
>
> Suggested-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

[PATCH v4 4/4] 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().

Reviewed-by: David Lechner <dlechner@baylibre.com>
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 0b98c8e7385d..234249ab76d8 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -506,10 +506,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)
@@ -517,8 +515,6 @@ static int ti_ads7950_probe(struct spi_device *spi)
 
 	st = iio_priv(indio_dev);
 
-	spi_set_drvdata(spi, indio_dev);
-
 	st->spi = spi;
 
 	info = spi_get_device_match_data(spi);
@@ -573,24 +569,22 @@ static int ti_ads7950_probe(struct spi_device *spi)
 		st->vref_mv = ret / 1000;
 	}
 
-	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);
@@ -605,33 +599,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[] = {
@@ -674,7 +647,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.1018.g2bb0e51243-goog

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

[Bartosz Golaszewski]

On Mon, 30 Mar 2026 00:47:09 +0200, Dmitry Torokhov
<dmitry.torokhov@gmail.com> said:
> All resources that the driver needs have managed API now. Switch to
> using them to make code clearer and drop ti_ads7950_remove().
>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> -

[snip]

> -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);

That's a functional change, there's no corresponding conversion to using
devm_mutex_init().

Bart

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

[Dmitry Torokhov]

On March 30, 2026 2:20:33 AM PDT, Bartosz Golaszewski <brgl@kernel.org> wrote:
>On Mon, 30 Mar 2026 00:47:09 +0200, Dmitry Torokhov
><dmitry.torokhov@gmail.com> said:
>> All resources that the driver needs have managed API now. Switch to
>> using them to make code clearer and drop ti_ads7950_remove().
>>
>> Reviewed-by: David Lechner <dlechner@baylibre.com>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> ---
>> -
>
>[snip]
>
>> -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);
>
>That's a functional change, there's no corresponding conversion to using
>devm_mutex_init().

devm_mutex_init() makes absolutely no sense, as well as having mutex_destroy() in remove(). The memory containing the mutex will be released right after. 

-- 
Dmitry

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

[Andy Shevchenko]

On Mon, Mar 30, 2026 at 02:24:52AM -0700, Dmitry Torokhov wrote:
> On March 30, 2026 2:20:33 AM PDT, Bartosz Golaszewski <brgl@kernel.org> wrote:
> >On Mon, 30 Mar 2026 00:47:09 +0200, Dmitry Torokhov
> ><dmitry.torokhov@gmail.com> said:

...

> >> -	mutex_destroy(&st->slock);
> >
> >That's a functional change, there's no corresponding conversion to using
> >devm_mutex_init().
> 
> devm_mutex_init() makes absolutely no sense, as well as having
> mutex_destroy() in remove(). The memory containing the mutex will be released
> right after. 

For somebody who debugs mutexes it might make sense. So, I agree with Bart here,
this needs to be addressed (and this is a used patter in IIO).

-- 
With Best Regards,
Andy Shevchenko