Re: [PATCH v2 05/12] iio: dac: ad5686: add support for missing power supplies

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 09 Jun 2026 11:13:00 +0100
Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:

> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> Get and enable regulators for vdd, vlogic and vref input power pins. Vdd
> is the input power supply, while vlogic powers the digital side. vref is
> replacing vcc, which is being deprecated, but still supported. The value
> of vref_mv is checked so that a device without internal voltage reference
> cannot proceed without an explicit supply. For correct operation, vdd and
> vlogic are required, then devm_regulator_get_enable() is used so the
> driver can still work without them by using the stub/dummy regulators.
> Error report uses dev_err_probe(), which helps debugging an init issue.
> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
Possibly the comment below falls into the bikeshed colour category.
I'm not really that fussed either way.

> ---
>  drivers/iio/dac/ad5686.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index 5840fda4b011..fc3863274b29 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c
> @@ -8,6 +8,8 @@
>  #include <linux/array_size.h>
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/dev_printk.h>
>  #include <linux/errno.h>
>  #include <linux/export.h>
>  #include <linux/kstrtox.h>
> @@ -484,12 +486,27 @@ int ad5686_probe(struct device *dev,
>  	st->ops = ops;
>  	st->chip_info = chip_info;
>  
> -	ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
> +	ret = devm_regulator_get_enable(dev, "vdd");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to enable vdd supply\n");
> +
> +	ret = devm_regulator_get_enable(dev, "vlogic");
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to enable vlogic supply\n");
> +
> +	ret = devm_regulator_get_enable_read_voltage(dev, "vref");
> +	if (ret == -ENODEV) /* vcc-supply is deprecated, but supported still */
> +		ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
>  	if (ret < 0 && ret != -ENODEV)
> -		return ret;
> +		return dev_err_probe(dev, ret, "failed to read vref voltage\n");
>  
>  	st->use_internal_vref = ret == -ENODEV;
I think I'd slightly prefer this as
	ret = devm_regulator_get_enable_read_voltage(dev, "vref");
	if (ret == -ENODEV) /* vcc-supply is deprecated, but supported still */
		ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
	if (ret == -ENODEV)
		st->use_internal_vref = true;
	else if (ret < 0)
		return dev_err_probe()...
(which I think is functionally the same).

But if you strongly prefer yours I guess it is readable enough.




>  	st->vref_mv = st->use_internal_vref ? st->chip_info->int_vref_mv : ret / 1000;
> +	if (!st->vref_mv)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "invalid or not provided vref voltage\n");
> +
> +	fsleep(5); /* power-up time */
>  
>  	/* Initialize masks to all ones */
>  	st->pwr_down_mask = ~0;
>