[PATCH v3 0/5] iio: dac: ad5504: bindings, cleanups, and scale fixes

[PATCH v3 0/5] iio: dac: ad5504: bindings, cleanups, and scale fixes

[Taha Ed-Dafili]

Hi everyone,

First, I want to sincerely apologize for the long delay between v2 and v3.
I had to step away to focus on finishing my last year studies, but I am
back now to get this series over the finish line!

This v3 series addresses the feedbacks from the v2 review,
specifically focusing on strict DT validation, dropping unnecessary
driver code, and making sure the driver works perfectly at every
individual commit.

Testing & Hardware Proof:
The series has been physically verified on an ARM64 Raspberry Pi 5 using
a custom Device Tree overlay.

The AD5504 is a 12-bit DAC. The hardware scale outputs
now perfectly match the DT inputs:

Testing the 60V path (output-range-microvolt = <0 60000000>):
$ cat /sys/bus/iio/devices/iio:device0/out_voltage_scale
14.648437500  (60000mV / 4096)

Testing the 30V path (output-range-microvolt = <0 30000000>):
$ cat /sys/bus/iio/devices/iio:device0/out_voltage_scale
7.324218750   (30000mV / 4096)

Additionally, providing invalid DT configurations (e.g., 70V) successfully
triggers the strict bounds checking and cleanly aborts the probe with
-EINVAL.

Changes in v3:

* Dropped the patch adding GPIO control for /CLR and /LDAC. As pointed out
  by Nuno Sá, since the driver does not actively handle or toggle these
  pins, requesting them in the driver is dead code. They remain documented
  in the bindings.
* Combined the pdata removal, regulator _enable swap, and DT parsing into
  a single atomic commit (Patch 5) so the driver compiles and runs cleanly
  at every step of the git history.
* Restored the -ENODEV check for the regulator to maintain ACPI
  compatibility.
* Implemented strict min/max array bounds validation for the
  'output-range-microvolt' property.
* Included missing <linux/array_size.h>, <asm/byteorder.h>, and
  <linux/kstrtox.h> headers for full IWYU compliance.
* Reordered commits to group cleanups before functional changes.
* Dropped Suggested-by tags for standard review feedback.

Changes in v2:

* Replaced vendor-specific adi,output-range-volts with the standard
  output-range-microvolt property array.
* Replaced adi,output-range-gpios with standard range-sel-gpios.
* Split header updates into two patches: alphabetical sorting and IWYU
  compliance.
* Used MILLI and MICRO macros to replace hardcoded values.
* Introduced a local dev pointer to respect the 80-character limit and
  realigned function arguments.

Previous versions:
v2: https://lore.kernel.org/linux-iio/20260310174835.24209-1-0rayn.dev@gmail.com/
v1: https://lore.kernel.org/linux-iio/20260212181955.42724-1-0rayn.dev@gmail.com/
RFC: https://lore.kernel.org/linux-iio/20260210121032.4630-1-0rayn.dev@gmail.com/

Thank you to Jonathan Cameron, Krzysztof Kozlowski, Andy Shevchenko,
Nuno Sá, and David Lechner for the extensive reviews, schema guidance,
and C optimizations throughout this series.

Taha Ed-Dafili

Taha Ed-Dafili (5):
  dt-bindings: iio: dac: ad5504: add output-range and missing gpios
  iio: dac: ad5504: sort headers alphabetically
  iio: dac: ad5504: Align headers with IWYU principle
  iio: dac: ad5504: introduce local dev pointer
  iio: dac: ad5504: fix scale via output-range-microvolt

 .../bindings/iio/dac/adi,ad5504.yaml          | 39 +++++++++++-
 drivers/iio/dac/ad5504.c                      | 63 +++++++++++--------
 2 files changed, 74 insertions(+), 28 deletions(-)

-- 
2.47.3

[PATCH v3 1/5] dt-bindings: iio: dac: ad5504: add output-range and missing gpios

[Taha Ed-Dafili]

The AD5504 output range (0-30V or 0-60V) is determined by the R_SEL pin.

Use standard output-range-microvolt and range-sel-gpios properties to
describe the hardware configuration of the R_SEL pin. Ensure mutual
exclusivity using the not/required logic. Additionally, add missing
vlogic-supply, clr-gpios, ldac-gpios and datasheet links, and provide
a complete usage example.

Signed-off-by: Taha Ed-Dafili <0rayn.dev@gmail.com>
---
 .../bindings/iio/dac/adi,ad5504.yaml          | 39 ++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5504.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5504.yaml
index 9c2c038683b4..e0123dceaa33 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5504.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5504.yaml
@@ -10,8 +10,10 @@ maintainers:
   - Lars-Peter Clausen <lars@metafoo.de>
   - Jonathan Cameron <jic23@kernel.org>
 
-description:
+description: |
   High voltage (up to 60V) DACs with temperature sensor alarm function
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad5504.pdf
+  https://www.analog.com/media/en/technical-documentation/data-sheets/ad5501.pdf
 
 properties:
   compatible:
@@ -27,6 +29,29 @@ properties:
     maxItems: 1
 
   vcc-supply: true
+  vlogic-supply: true
+
+  output-range-microvolt:
+    description: |
+      Specify the channel output full scale range. The R_SEL pin
+      determines if the range is 0-30V or 0-60V.
+    items:
+      - const: 0
+      - enum: [30000000, 60000000]
+    default: [0, 60000000]
+
+  range-sel-gpios:
+    description:
+      GPIO connected to the R_SEL pin to select the output voltage range.
+    maxItems: 1
+
+  clr-gpios:
+    description: GPIO that controls the /CLR pin (active low).
+    maxItems: 1
+
+  ldac-gpios:
+    description: GPIO that controls the /LDAC pin (active low).
+    maxItems: 1
 
 additionalProperties: false
 
@@ -34,9 +59,17 @@ required:
   - compatible
   - reg
 
+allOf:
+  - not:
+      required:
+        - range-sel-gpios
+        - output-range-microvolt
+
 examples:
   - |
     #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+
     spi {
         #address-cells = <1>;
         #size-cells = <0>;
@@ -45,6 +78,10 @@ examples:
             compatible = "adi,ad5504";
             vcc-supply = <&dac_vcc>;
             interrupts = <55 IRQ_TYPE_EDGE_FALLING>;
+
+            output-range-microvolt = <0 60000000>;
+            clr-gpios = <&gpio 4 GPIO_ACTIVE_LOW>;
+            ldac-gpios = <&gpio 5 GPIO_ACTIVE_LOW>;
         };
     };
 ...
-- 
2.47.3

[PATCH v3 2/5] iio: dac: ad5504: sort headers alphabetically

[Taha Ed-Dafili]

Rearrange the include headers in alphabetical order to follow the
standard kernel coding style. This is a preparatory cleanup with
no functional changes.

Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Taha Ed-Dafili <0rayn.dev@gmail.com>
---
 drivers/iio/dac/ad5504.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 355bcb6a8ba0..03ce37e2c616 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -5,21 +5,21 @@
  * Copyright 2011 Analog Devices Inc.
  */
 
-#include <linux/interrupt.h>
-#include <linux/fs.h>
+#include <linux/bitops.h>
 #include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/interrupt.h>
 #include <linux/kernel.h>
-#include <linux/spi/spi.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
 #include <linux/slab.h>
+#include <linux/spi/spi.h>
 #include <linux/sysfs.h>
-#include <linux/regulator/consumer.h>
-#include <linux/module.h>
-#include <linux/bitops.h>
 
+#include <linux/iio/dac/ad5504.h>
+#include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
-#include <linux/iio/events.h>
-#include <linux/iio/dac/ad5504.h>
 
 #define AD5504_RES_MASK			GENMASK(11, 0)
 #define AD5504_CMD_READ			BIT(15)
-- 
2.47.3

[PATCH v3 3/5] iio: dac: ad5504: Align headers with IWYU principle

[Taha Ed-Dafili]

Update the header inclusions to follow the IWYU principle and ensure
they are sorted alphabetically:

- Remove <linux/fs.h>, <linux/slab.h>, and <linux/kernel.h> as they
  are unused. The driver relies on devm_ managed allocations, so slab
  is not required.
- Replace <linux/bitops.h> with <linux/bits.h> as only the BIT() and
  GENMASK() macros are used.
- Add <linux/mod_devicetable.h> for struct spi_device_id.
- Add <linux/errno.h> and <linux/types.h> for error codes and data types.
- Add <asm/byteorder.h> for cpu_to_be16().
- Add <linux/array_size.h> for ARRAY_SIZE().
- Add <linux/kstrtox.h> for string to integer conversions.

Signed-off-by: Taha Ed-Dafili <0rayn.dev@gmail.com>
---
 drivers/iio/dac/ad5504.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 03ce37e2c616..57e6eeed26ef 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -5,16 +5,19 @@
  * Copyright 2011 Analog Devices Inc.
  */
 
-#include <linux/bitops.h>
+#include <asm/byteorder.h>
+#include <linux/array_size.h>
+#include <linux/bits.h>
 #include <linux/device.h>
-#include <linux/fs.h>
+#include <linux/errno.h>
 #include <linux/interrupt.h>
-#include <linux/kernel.h>
+#include <linux/kstrtox.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/regulator/consumer.h>
-#include <linux/slab.h>
 #include <linux/spi/spi.h>
 #include <linux/sysfs.h>
+#include <linux/types.h>
 
 #include <linux/iio/dac/ad5504.h>
 #include <linux/iio/events.h>
-- 
2.47.3

[PATCH v3 4/5] iio: dac: ad5504: introduce local dev pointer

[Taha Ed-Dafili]

Replace &spi->dev with a local dev pointer to shorten lines, fix
alignment, and improve overall readability in the probe function.

Signed-off-by: Taha Ed-Dafili <0rayn.dev@gmail.com>
---
 drivers/iio/dac/ad5504.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 57e6eeed26ef..9e95da6e49d6 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -273,25 +273,26 @@ static const struct iio_chan_spec ad5504_channels[] = {
 
 static int ad5504_probe(struct spi_device *spi)
 {
-	const struct ad5504_platform_data *pdata = dev_get_platdata(&spi->dev);
+	struct device *dev = &spi->dev;
+	const struct ad5504_platform_data *pdata = dev_get_platdata(dev);
 	struct iio_dev *indio_dev;
 	struct ad5504_state *st;
 	int ret;
 
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (!indio_dev)
 		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
 
-	ret = devm_regulator_get_enable_read_voltage(&spi->dev, "vcc");
+	ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
 	if (ret < 0 && ret != -ENODEV)
 		return ret;
 	if (ret == -ENODEV) {
 		if (pdata->vref_mv)
 			st->vref_mv = pdata->vref_mv;
 		else
-			dev_warn(&spi->dev, "reference voltage unspecified\n");
+			dev_warn(dev, "reference voltage unspecified\n");
 	} else {
 		st->vref_mv = ret / 1000;
 	}
@@ -307,17 +308,17 @@ static int ad5504_probe(struct spi_device *spi)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	if (spi->irq) {
-		ret = devm_request_threaded_irq(&spi->dev, spi->irq,
-					   NULL,
-					   &ad5504_event_handler,
-					   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-					   spi_get_device_id(st->spi)->name,
-					   indio_dev);
+		ret = devm_request_threaded_irq(dev, spi->irq,
+						NULL,
+						&ad5504_event_handler,
+						IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+						spi_get_device_id(st->spi)->name,
+						indio_dev);
 		if (ret)
 			return ret;
 	}
 
-	return devm_iio_device_register(&spi->dev, indio_dev);
+	return devm_iio_device_register(dev, indio_dev);
 }
 
 static const struct spi_device_id ad5504_id[] = {
-- 
2.47.3

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

[Taha Ed-Dafili]

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.

Signed-off-by: Taha Ed-Dafili <0rayn.dev@gmail.com>
---
 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);
 	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;
-- 
2.47.3

Re: [PATCH v3 1/5] dt-bindings: iio: dac: ad5504: add output-range and missing gpios

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `vlogic-supply` property is missing from the device tree example despite the commit message claiming to provide a complete usage example.
--

commit c85189a6dcd792e207cd5be70833e861a180c568
Author: Taha Ed-Dafili <0rayn.dev@gmail.com>

dt-bindings: iio: dac: ad5504: add output-range and missing gpios

This commit introduces properties for configuring the R_SEL pin on AD5504
devices. It also adds vlogic-supply, clr-gpios, ldac-gpios, datasheet links,
and updates the usage example.

> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5504.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5504.yaml
> index 9c2c038683b4d..e0123dceaa336 100644
> --- a/Documentation/devicetree/bindings/iio/dac/adi,ad5504.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5504.yaml

[ ... ]

> @@ -45,6 +78,10 @@ examples:
>              compatible = "adi,ad5504";
>              vcc-supply = <&dac_vcc>;
>              interrupts = <55 IRQ_TYPE_EDGE_FALLING>;
> +
> +            output-range-microvolt = <0 60000000>;
> +            clr-gpios = <&gpio 4 GPIO_ACTIVE_LOW>;
> +            ldac-gpios = <&gpio 5 GPIO_ACTIVE_LOW>;

This isn't a bug, but the commit message states it provides a complete usage
example including the newly added vlogic-supply. Should vlogic-supply be
included in this example node?

>          };
>      };
>  ...

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260509142047.30302-1-0rayn.dev@gmail.com?part=1

Re: [PATCH v3 3/5] iio: dac: ad5504: Align headers with IWYU principle

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing lock protection for shared SPI DMA buffer 'st->data'.
--

commit 4601fb5b571a0290d2e1fd539e465ce5c954b588
Author: Taha Ed-Dafili <0rayn.dev@gmail.com>

iio: dac: ad5504: Align headers with IWYU principle

This commit updates the header inclusions to follow the Include What You Use
principle and ensures they are sorted alphabetically. Unused headers are
removed, and required ones are added.

> diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
> index 03ce37e2c616a..57e6eeed26efb 100644
> --- a/drivers/iio/dac/ad5504.c
> +++ b/drivers/iio/dac/ad5504.c
> @@ -5,16 +5,19 @@
>   * Copyright 2011 Analog Devices Inc.
>   */
>  
> -#include <linux/bitops.h>
> +#include <asm/byteorder.h>
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
>  #include <linux/device.h>

[ ... ]

>  #include <linux/sysfs.h>
> +#include <linux/types.h>
>  
>  #include <linux/iio/dac/ad5504.h>
>  #include <linux/iio/events.h>

This isn't a regression from this patch, but while reviewing this driver
I noticed a concurrency issue.

The driver allocates a shared tx/rx buffer st->data for SPI transfers.
Multiple sysfs callbacks, such as ad5504_read_raw(), ad5504_write_raw(),
and ad5504_write_dac_powerdown(), directly access and modify st->data
without any driver-level locking.

If thread A calls ad5504_spi_read() and thread B calls ad5504_spi_write()
concurrently, can thread B overwrite st->data[0] while thread A's
spi_sync_transfer() is in progress?

Modifying a DMA buffer while a transfer is in flight violates SPI API
contracts, corrupts the transmitted data, and causes the DAC to receive
incorrect commands.

Additionally, read-modify-write operations on driver state like
st->pwr_down_mask lack locking. Could concurrent writes to different
channels' powerdown files race?

Would it be appropriate to introduce a lock to serialize sysfs operations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260509142047.30302-1-0rayn.dev@gmail.com?part=3

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@gmail.com> 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@gmail.com>

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;

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] UAPI/ABI Breakage: existing Device Trees rely on the 'vcc' regulator to configure the IIO scale, but the driver now silently defaults to 60V.
- [Medium] Silent error suppression for malformed 'output-range-microvolt' device tree properties.
- [Low] Incomplete removal of legacy platform data header.
--

commit b02770a220cb671b4745934fb6d03abfd8d88fd3
Author: Taha Ed-Dafili <0rayn.dev@gmail.com>

iio: dac: ad5504: fix scale via output-range-microvolt

This patch updates the AD5504 driver to read the output-range-microvolt DT
property for setting the scale, rather than querying the vcc regulator. It
also removes legacy pdata support and sets the default scale to 60V if the
property is absent.

> diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
> index 9e95da6e49d61..040f580b82828 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>

The commit message states legacy pdata support is removed. Is it intentional
to leave the #include <linux/iio/dac/ad5504.h> header in place, as it appears
to only contain the unused legacy structs?

[ ... ]

> @@ -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;

Does dropping the regulator voltage read cause compatibility issues with
existing device trees?

Prior to this patch, existing setups could provide a vcc regulator with a
30V voltage, which would properly configure st->vref_mv. By defaulting
directly to 60V when the new output-range-microvolt property is absent, older
device trees might silently change their exposed scale attribute to userspace.

Would it be safer to fallback to reading the regulator voltage and issue a
deprecation warning if the new property is missing?

> +	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;
>  	}

If the device tree property exists but is malformed, such as only containing
a single value instead of an array, device_property_read_u32_array() might
return an error code like -EOVERFLOW or -ENODATA.

Since the code only checks if (!ret), does this silently suppress parsing
errors and assume the 60V default?

Should this explicitly check for ret && ret != -EINVAL and return the error
code instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260509142047.30302-1-0rayn.dev@gmail.com?part=5

Re: [PATCH v3 0/5] iio: dac: ad5504: bindings, cleanups, and scale fixes

[David Lechner]

On 5/9/26 9:20 AM, Taha Ed-Dafili wrote:
> Hi everyone,
> 
> First, I want to sincerely apologize for the long delay between v2 and v3.
> I had to step away to focus on finishing my last year studies, but I am
> back now to get this series over the finish line!
> 
> This v3 series addresses the feedbacks from the v2 review,
> specifically focusing on strict DT validation, dropping unnecessary
> driver code, and making sure the driver works perfectly at every
> individual commit.
> 
> Testing & Hardware Proof:
> The series has been physically verified on an ARM64 Raspberry Pi 5 using
> a custom Device Tree overlay.
> 
> The AD5504 is a 12-bit DAC. The hardware scale outputs
> now perfectly match the DT inputs:
> 
> Testing the 60V path (output-range-microvolt = <0 60000000>):
> $ cat /sys/bus/iio/devices/iio:device0/out_voltage_scale
> 14.648437500  (60000mV / 4096)
> 
> Testing the 30V path (output-range-microvolt = <0 30000000>):
> $ cat /sys/bus/iio/devices/iio:device0/out_voltage_scale
> 7.324218750   (30000mV / 4096)
> 
> Additionally, providing invalid DT configurations (e.g., 70V) successfully
> triggers the strict bounds checking and cleanly aborts the probe with
> -EINVAL.
> 
> Changes in v3:
> 
> * Dropped the patch adding GPIO control for /CLR and /LDAC. As pointed out
>   by Nuno Sá, since the driver does not actively handle or toggle these
>   pins, requesting them in the driver is dead code. They remain documented
>   in the bindings.
> * Combined the pdata removal, regulator _enable swap, and DT parsing into
>   a single atomic commit (Patch 5) so the driver compiles and runs cleanly
>   at every step of the git history.

It should be possible to do this while still having separate commits.

> * Restored the -ENODEV check for the regulator to maintain ACPI
>   compatibility.

While you were studying we discussed this some more in similar cases
and decided that we would like to handle this differently going forward.
For example, see recent changes in adc/ti-ads7950.c which also has
an ACPI case.

> * Implemented strict min/max array bounds validation for the
>   'output-range-microvolt' property.
> * Included missing <linux/array_size.h>, <asm/byteorder.h>, and
>   <linux/kstrtox.h> headers for full IWYU compliance.
> * Reordered commits to group cleanups before functional changes.
> * Dropped Suggested-by tags for standard review feedback.
> 

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

[David Lechner]

On 5/9/26 9:20 AM, Taha Ed-Dafili 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.
> 
> Signed-off-by: Taha Ed-Dafili <0rayn.dev@gmail.com>
> ---
>  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);
>  	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)

I don't think this would ever return -ENODEV, even on ACPI. It will
just get a dummy regulator now.

> +	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;

Re: [PATCH v3 1/5] dt-bindings: iio: dac: ad5504: add output-range and missing gpios

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 600 bytes --]

On Sat, May 09, 2026 at 03:20:39PM +0100, Taha Ed-Dafili wrote:
> The AD5504 output range (0-30V or 0-60V) is determined by the R_SEL pin.
> 
> Use standard output-range-microvolt and range-sel-gpios properties to
> describe the hardware configuration of the R_SEL pin. Ensure mutual
> exclusivity using the not/required logic. Additionally, add missing
> vlogic-supply, clr-gpios, ldac-gpios and datasheet links, and provide
> a complete usage example.
> 
> Signed-off-by: Taha Ed-Dafili <0rayn.dev@gmail.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]