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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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