The ad7476 driver supports variants with different amount of supply
regulators. On some variants there is only VCC, which is used as a
reference voltage. Others have separate VREF regulator, and some rely on
internal VREF. Some have both internal VREF and option to connect
external one.

The ad7476 driver reads the regulator voltage only when the user asks to
get the scale. This means the driver needs to do some dancing while
picking the correct reference regulator (or internal reference), and
store it for the later use.

According to the discussion:
https://lore.kernel.org/linux-iio/20250331122247.05c6b09d@jic23-huawei/
variable voltage references are rare, making it hard to justify the
added complexity for supporting those.

Drop the support for the variable voltage references and simplify things
by using the managed regulator get and enable interfaces.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
NOTE: This is 100% untested as I lack of the hardware. I have tried to
maintain the existing logic, but there is always some room for an error.
All testing and logic reviewing is greatly appreciated.
---
 drivers/iio/adc/ad7476.c | 84 ++++++++++------------------------------
 1 file changed, 21 insertions(+), 63 deletions(-)

diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
index bdfffc4f5724..7b1d6a0fd941 100644
--- a/drivers/iio/adc/ad7476.c
+++ b/drivers/iio/adc/ad7476.c
@@ -39,10 +39,10 @@ struct ad7476_chip_info {
 struct ad7476_state {
 	struct spi_device		*spi;
 	const struct ad7476_chip_info	*chip_info;
-	struct regulator		*ref_reg;
 	struct gpio_desc		*convst_gpio;
 	struct spi_transfer		xfer;
 	struct spi_message		msg;
+	int				scale_mv;
 	/*
 	 * DMA (thus cache coherency maintenance) may require the
 	 * transfer buffers to live in their own cache lines.
@@ -111,7 +111,6 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
 {
 	int ret;
 	struct ad7476_state *st = iio_priv(indio_dev);
-	int scale_uv;
 
 	switch (m) {
 	case IIO_CHAN_INFO_RAW:
@@ -126,14 +125,7 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
 			GENMASK(st->chip_info->channel[0].scan_type.realbits - 1, 0);
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		if (st->ref_reg) {
-			scale_uv = regulator_get_voltage(st->ref_reg);
-			if (scale_uv < 0)
-				return scale_uv;
-		} else {
-			scale_uv = st->chip_info->int_vref_uv;
-		}
-		*val = scale_uv / 1000;
+		*val = st->scale_mv;
 		*val2 = chan->scan_type.realbits;
 		return IIO_VAL_FRACTIONAL_LOG2;
 	}
@@ -286,18 +278,10 @@ static const struct iio_info ad7476_info = {
 	.read_raw = &ad7476_read_raw,
 };
 
-static void ad7476_reg_disable(void *data)
-{
-	struct regulator *reg = data;
-
-	regulator_disable(reg);
-}
-
 static int ad7476_probe(struct spi_device *spi)
 {
 	struct ad7476_state *st;
 	struct iio_dev *indio_dev;
-	struct regulator *reg;
 	int ret;
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
@@ -308,58 +292,32 @@ static int ad7476_probe(struct spi_device *spi)
 	st->chip_info =
 		(struct ad7476_chip_info *)spi_get_device_id(spi)->driver_data;
 
-	reg = devm_regulator_get(&spi->dev, "vcc");
-	if (IS_ERR(reg))
-		return PTR_ERR(reg);
-
-	ret = regulator_enable(reg);
-	if (ret)
-		return ret;
-
-	ret = devm_add_action_or_reset(&spi->dev, ad7476_reg_disable, reg);
-	if (ret)
-		return ret;
-
-	/* Either vcc or vref (below) as appropriate */
-	if (!st->chip_info->int_vref_uv)
-		st->ref_reg = reg;
+	/* Use VCC for reference voltage if vref / internal vref aren't used */
+	if (!st->chip_info->int_vref_uv && !st->chip_info->has_vref) {
+		ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vcc");
+		if (ret < 0)
+			return ret;
+		st->scale_mv = ret / 1000;
+	} else {
+		ret = devm_regulator_get_enable(&spi->dev, "vcc");
+		if (ret < 0)
+			return ret;
+	}
 
 	if (st->chip_info->has_vref) {
-
-		/* If a device has an internal reference vref is optional */
-		if (st->chip_info->int_vref_uv) {
-			reg = devm_regulator_get_optional(&spi->dev, "vref");
-			if (IS_ERR(reg) && (PTR_ERR(reg) != -ENODEV))
-				return PTR_ERR(reg);
-		} else {
-			reg = devm_regulator_get(&spi->dev, "vref");
-			if (IS_ERR(reg))
-				return PTR_ERR(reg);
-		}
-
-		if (!IS_ERR(reg)) {
-			ret = regulator_enable(reg);
-			if (ret)
+		ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vref");
+		if (ret < 0) {
+			/* Vref is optional if a device has an internal reference */
+			if (!st->chip_info->int_vref_uv || ret != -ENODEV)
 				return ret;
-
-			ret = devm_add_action_or_reset(&spi->dev,
-						       ad7476_reg_disable,
-						       reg);
-			if (ret)
-				return ret;
-			st->ref_reg = reg;
 		} else {
-			/*
-			 * Can only get here if device supports both internal
-			 * and external reference, but the regulator connected
-			 * to the external reference is not connected.
-			 * Set the reference regulator pointer to NULL to
-			 * indicate this.
-			 */
-			st->ref_reg = NULL;
+			st->scale_mv = ret / 1000;
 		}
 	}
 
+	if (!st->scale_mv)
+		st->scale_mv = st->chip_info->int_vref_uv / 100;
+
 	if (st->chip_info->has_vdrive) {
 		ret = devm_regulator_get_enable(&spi->dev, "vdrive");
 		if (ret)
-- 
2.50.1

On Fri, 1 Aug 2025 13:07:39 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> The ad7476 driver supports variants with different amount of supply
> regulators. On some variants there is only VCC, which is used as a
> reference voltage. Others have separate VREF regulator, and some rely on
> internal VREF. Some have both internal VREF and option to connect
> external one.
> 
> The ad7476 driver reads the regulator voltage only when the user asks to
> get the scale. This means the driver needs to do some dancing while
> picking the correct reference regulator (or internal reference), and
> store it for the later use.
> 
> According to the discussion:
> https://lore.kernel.org/linux-iio/20250331122247.05c6b09d@jic23-huawei/
> variable voltage references are rare, making it hard to justify the
> added complexity for supporting those.
> 
> Drop the support for the variable voltage references and simplify things
> by using the managed regulator get and enable interfaces.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

So there is risk of regression in doing this to an existing driver.
I'm not that worried about it because as you note, we almost never
see variable reference voltages.  So this is the whole, if no one notices
it's not a regression exception to Linus' rules on regressions.
 
Looks good to me.

Jonathan

On 8/1/25 5:07 AM, Matti Vaittinen wrote:
> The ad7476 driver supports variants with different amount of supply
> regulators. On some variants there is only VCC, which is used as a
> reference voltage. Others have separate VREF regulator, and some rely on
> internal VREF. Some have both internal VREF and option to connect
> external one.
> 

...

> +	if (!st->scale_mv)
> +		st->scale_mv = st->chip_info->int_vref_uv / 100;
> +

Shouldn't this be 1000 rather than 100 to go from microvolts to millivolts?

Also, I would just change the chip info to `int_vref_mv` to avoid needing
to do the division at all.

Hi David,

Thanks for taking a look at this :)

On 05/08/2025 19:09, David Lechner wrote:
> On 8/1/25 5:07 AM, Matti Vaittinen wrote:
>> The ad7476 driver supports variants with different amount of supply
>> regulators. On some variants there is only VCC, which is used as a
>> reference voltage. Others have separate VREF regulator, and some rely on
>> internal VREF. Some have both internal VREF and option to connect
>> external one.
>>
> 
> ...
> 
>> +	if (!st->scale_mv)
>> +		st->scale_mv = st->chip_info->int_vref_uv / 100;
>> +
> 
> Shouldn't this be 1000 rather than 100 to go from microvolts to millivolts?

Yes, Thanks!

> Also, I would just change the chip info to `int_vref_mv` to avoid needing
> to do the division at all.

Yep. I'll do that.

Yours,
	-- Matti