Re: [PATCH v3 5/5] iio: dac: ad5504: fix scale via output-range-microvolt

[Jonathan Cameron]

On Sat,  9 May 2026 15:20:43 +0100
Taha Ed-Dafili <0rayn.dev@xxxxxxxxx> wrote:

> The AD5504 full-scale range is hardware-determined by the R_SEL pin,
> not the VCC supply voltage.
> 
> Fix the scaling logic by reading the standard 'output-range-microvolt'
> property from the device tree instead of querying the VCC regulator or
> relying on legacy platform data (pdata).
> 
> As a result of this transition:
> - The 'vcc' regulator is now only enabled, not read.
> - Legacy pdata support is removed, as it is no longer required for
>   fallback voltage calculations.
> - Strict array bounds checking is added for the DT property.
Hi Taha

These could have been broken up more.  Given this patch isn't suitable
for backporting anyway, you could have done a precursor ripping out
the pdata.

> 
> Signed-off-by: Taha Ed-Dafili <0rayn.dev@xxxxxxxxx>

The datasheet is less clear than it might be but I'm far from
sure that the range is as simple as 0-30 vs 0-60.

See for example the output characteristics in table 2.
That lists Output Voltage Range as AGND + 0.5 to VDD - 0.5V
which is not surprising as VDD can be too low to support the fully
0-60V without any nasty things like step up convertors which
aren't present here. 

Now there is a footnote:
"The DAC architecture gives a fixed linear voltage output range
of 0 V to 30 V if R_SEL is held high and 0 V to 60 V if R_SEL is
held low. As the output voltage range is limited by output
amplifier compliance, VDD should be set to at least 0.5 V higher
than the maximum output voltage to ensure compliance."

So I guess we can 'assume' that footnote is obeyed and indeed
do things as you have here.

One other thing inline.

> ---
>  drivers/iio/dac/ad5504.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
> index 9e95da6e49d6..040f580b8282 100644
> --- a/drivers/iio/dac/ad5504.c
> +++ b/drivers/iio/dac/ad5504.c
> @@ -14,10 +14,12 @@
>  #include <linux/kstrtox.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
> +#include <linux/property.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/spi/spi.h>
>  #include <linux/sysfs.h>
>  #include <linux/types.h>
> +#include <linux/units.h>
>  
>  #include <linux/iio/dac/ad5504.h>
>  #include <linux/iio/events.h>
> @@ -274,9 +276,9 @@ static const struct iio_chan_spec ad5504_channels[] = {
>  static int ad5504_probe(struct spi_device *spi)
>  {
>  	struct device *dev = &spi->dev;
> -	const struct ad5504_platform_data *pdata = dev_get_platdata(dev);

Last use of this - remove the header as well.

>  	struct iio_dev *indio_dev;
>  	struct ad5504_state *st;
> +	u32 range[2];
>  	int ret;
>  
>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> @@ -285,16 +287,19 @@ static int ad5504_probe(struct spi_device *spi)
>  
>  	st = iio_priv(indio_dev);
>  
> -	ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
> -	if (ret < 0 && ret != -ENODEV)
> +	ret = devm_regulator_get_enable(dev, "vcc");
> +	if (ret && ret != -ENODEV)
>  		return ret;
> -	if (ret == -ENODEV) {
> -		if (pdata->vref_mv)
> -			st->vref_mv = pdata->vref_mv;
> -		else
> -			dev_warn(dev, "reference voltage unspecified\n");
> -	} else {
> -		st->vref_mv = ret / 1000;
> +
> +	st->vref_mv = 60 * MILLI;
> +	ret = device_property_read_u32_array(dev, "output-range-microvolt",
> +					     range, ARRAY_SIZE(range));
> +	if (!ret) {
> +		if (range[0] != 0 || (range[1] != 30 * MICRO && range[1] != 60 * MICRO))
> +			return -EINVAL;
> +
> +		if (range[1] == 30 * MICRO)
> +			st->vref_mv = 30 * MILLI;
>  	}
>  
>  	st->spi = spi;