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@xxxxxxxxx> 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@xxxxxxxxxxxx>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> 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;
> 
>