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

[Jonathan Cameron <jic23@kernel.org>]

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