[PATCH v4 0/6] iio: dac: ad5504: bindings, cleanups, locking, and scale fixes

[PATCH v4 0/6] iio: dac: ad5504: bindings, cleanups, locking, and scale fixes

[Taha Ed-Dafili]

This v4 series continues the cleanup and scale-fix work for the AD5504
high-voltage DAC driver, addressing all outstanding feedback from the v3 review.

Jonathan Cameron applied patches 2 (sort headers alphabetically) and 4
(introduce local dev pointer) from v3 directly to iio/testing. This series
rebases on top of that state and does not re-send those patches.

Changes in v4:

* Split the formerly monolithic v3 patch 5 into four focused commits:
  pdata removal, locking, ACPI/DT separation, and output-range support.
  Each commit leaves the driver in a clean, bisectable state.
* Fix ACPI probe path by forking on ACPI_COMPANION() instead of relying
  on the -ENODEV sentinel from the regulator framework, following the
  pattern established in adc/ti-ads7950.c (David Lechner).
* Introduce a mutex via devm_mutex_init() and guard(mutex) to fix three
  concurrency issues: shared DMA SPI buffers, non-atomic CTRL+NOOP write
  sequences, and unprotected pwr_down_mask/pwr_down_mode state.
* Use device_property_present() to cleanly distinguish "property absent"
  from "property present but malformed" when parsing
  output-range-microvolt, replacing the ambiguous -EINVAL sentinel.
* Add backward compatibility fallback via
  devm_regulator_get_enable_read_voltage() for old DTs that predate the
  output-range-microvolt property.
* Replace <linux/device.h> with <linux/dev_printk.h> in IWYU patch
  (Jonathan Cameron).

Testing:
Verified on ARM64 Raspberry Pi 5 (BCM2712) using a custom Device Tree
overlay on spi10, without physical AD5504 hardware.

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

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

Invalid range (output-range-microvolt = <0 70000000>):
[   40.646862] ad5504 spi10.0: error -EINVAL: Invalid output-range-microvolt
[   40.646869] ad5504 spi10.0: probe with driver ad5504 failed with error -22

Backward compatibility (no output-range-microvolt, vcc-supply = 60V):
$ cat /sys/bus/iio/devices/iio:device0/out_voltage_scale
14.648437500  (60000 mV / 4096 via regulator fallback)

Powerdown sysfs read/write and powerdown mode switching (20kohm_to_gnd,
three_state) verified on all four channels without errors.

Previous versions:
v3: https://lore.kernel.org/linux-iio/20260509142047.30302-1-0rayn.dev@gmail.com/
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/

Thank you to Jonathan Cameron, David Lechner, Andy Shevchenko, Nuno Sá,
Krzysztof Kozlowski, and Conor Dooley for the continued reviews.

Taha Ed-Dafili (6):
  dt-bindings: iio: dac: ad5504: add output-range and missing gpios
  iio: dac: ad5504: Align headers with IWYU principle
  iio: dac: ad5504: remove legacy platform data support
  iio: dac: ad5504: introduce local lock to protect state and spi
    transfers
  iio: dac: ad5504: strictly separate ACPI and DT probe paths
  iio: dac: ad5504: support scale via output-range-microvolt property

 .../bindings/iio/dac/adi,ad5504.yaml          | 39 +++++++++-
 drivers/iio/dac/ad5504.c                      | 76 ++++++++++++++-----
 include/linux/iio/dac/ad5504.h                | 15 ----
 3 files changed, 97 insertions(+), 33 deletions(-)
 delete mode 100644 include/linux/iio/dac/ad5504.h

-- 
2.55.0

[PATCH v4 1/6] 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>
Acked-by: Conor Dooley <conor.dooley@microchip.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.55.0

[PATCH v4 2/6] 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.
- Replaced <linux/device.h> with <linux/dev_printk.h>

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

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index ad1a74678cf2..689147cdaa14 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -5,22 +5,26 @@
  * Copyright 2011 Analog Devices Inc.
  */
 
-#include <linux/bitops.h>
-#include <linux/device.h>
-#include <linux/fs.h>
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/dev_printk.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>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
+#include <asm/byteorder.h>
+
 #define AD5504_RES_MASK			GENMASK(11, 0)
 #define AD5504_CMD_READ			BIT(15)
 #define AD5504_CMD_WRITE		0
-- 
2.55.0

Re: [PATCH v4 2/6] iio: dac: ad5504: Align headers with IWYU principle

[Andy Shevchenko]

On Mon, Aug 17, 2026 at 05:11:11PM -0400, Taha Ed-Dafili wrote:
> 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.

Not anymore, now spi.h is responsible for that.

> - 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.
> - Replaced <linux/device.h> with <linux/dev_printk.h>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 2/6] iio: dac: ad5504: Align headers with IWYU principle

[Uwe Kleine-König]

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

Hello,

On Mon, Aug 17, 2026 at 05:11:11PM -0400, Taha Ed-Dafili wrote:
> - Add <linux/mod_devicetable.h> for struct spi_device_id.

no, please rely on linux/spi/spi.h to provide that or use
<linux/device-id/spi.h> if you insist.

Best regards
Uwe

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

[PATCH v4 3/6] iio: dac: ad5504: remove legacy platform data support

[Taha Ed-Dafili]

The AD5504 driver contains fallback logic to use legacy pdata for
configuration. Since modern systems configure hardware exclusively
using device tree or ACPI, and there are no in tree users for it.
It can be safely removed.

Drop the ad5504_platform_data structure, remove the fallback logic
from the probe function, and delete the dedicated
include/linux/iio/dac/ad5504.h header file.

Signed-off-by: Taha Ed-Dafili <0rayn.dev@gmail.com>
---
 drivers/iio/dac/ad5504.c       | 14 +++-----------
 include/linux/iio/dac/ad5504.h | 15 ---------------
 2 files changed, 3 insertions(+), 26 deletions(-)
 delete mode 100644 include/linux/iio/dac/ad5504.h

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 689147cdaa14..55ce7e49e0e0 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -18,7 +18,6 @@
 #include <linux/sysfs.h>
 #include <linux/types.h>
 
-#include <linux/iio/dac/ad5504.h>
 #include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -275,7 +274,6 @@ 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;
 	int ret;
@@ -287,16 +285,10 @@ 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)
+	if (ret < 0)
 		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 = ret / 1000;
 
 	st->spi = spi;
 	indio_dev->name = spi_get_device_id(st->spi)->name;
diff --git a/include/linux/iio/dac/ad5504.h b/include/linux/iio/dac/ad5504.h
deleted file mode 100644
index 9f23c90486ee..000000000000
--- a/include/linux/iio/dac/ad5504.h
+++ /dev/null
@@ -1,15 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * AD5504 SPI DAC driver
- *
- * Copyright 2011 Analog Devices Inc.
- */
-
-#ifndef SPI_AD5504_H_
-#define SPI_AD5504_H_
-
-struct ad5504_platform_data {
-	u16				vref_mv;
-};
-
-#endif /* SPI_AD5504_H_ */
-- 
2.55.0

Re: [PATCH v4 3/6] iio: dac: ad5504: remove legacy platform data support

[Andy Shevchenko]

On Mon, Aug 17, 2026 at 05:11:12PM -0400, Taha Ed-Dafili wrote:
> The AD5504 driver contains fallback logic to use legacy pdata for
> configuration. Since modern systems configure hardware exclusively
> using device tree or ACPI, and there are no in tree users for it.
> It can be safely removed.
> 
> Drop the ad5504_platform_data structure, remove the fallback logic
> from the probe function, and delete the dedicated
> include/linux/iio/dac/ad5504.h header file.

This breaks the case with dummy regulators as far as I can see.
Has this been discussed in the previous rounds?

...

> -	if (ret == -ENODEV) {

> -		else
> -			dev_warn(dev, "reference voltage unspecified\n");


This one.

...

In general it's a good change.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v4 4/6] iio: dac: ad5504: introduce local lock to protect state and spi transfers

[Taha Ed-Dafili]

The AD5504 driver currently lacks locking, exposing it to several
multi-threading race conditions:
1. The shared DMA-safe SPI transfer buffers (st->data) can be
   corrupted if multiple threads trigger read_raw or write_raw
   simultaneously.
2. The ad5504_write_dac_powerdown() routine executes a sequence of
   back-to-back SPI writes (a CTRL register update followed by a
   mandatory NOOP). This entire sequence must be atomic.
3. Internal state variables like pwr_down_mask and pwr_down_mode
   can be read and modified concurrently.

Introduce a mutex in the ad5504_state structure and initialize it via
devm_mutex_init() in probe. Use the modern scoped guard(mutex) macro
at the top-level public IIO callbacks (read_raw, write_raw, and the
powerdown attributes) to safely serialize access to the device state
and the SPI bus.

In ad5504_read_raw() and ad5504_write_raw(), guard(mutex) is scoped to
the IIO_CHAN_INFO_RAW case only, since IIO_CHAN_INFO_SCALE merely reads
vref_mv, which is fixed at probe time and never modified afterward and
therefore needs no serialization. Because guard(mutex) declares a
cleanup-scoped variable, it cannot appear directly after a case label;
wrap the case body in a compound statement (case IIO_CHAN_INFO_RAW: {
... }) to give it the block scope it requires.

Signed-off-by: Taha Ed-Dafili <0rayn.dev@gmail.com>
---
Note: The concurrency race conditions addressed in this patch were 
originally reported by the Sashiko bot:
https://sashiko.dev/#/patchset/20260509142047.30302-1-0rayn.dev@gmail.com

 drivers/iio/dac/ad5504.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 55ce7e49e0e0..87946cdf69e7 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -7,12 +7,14 @@
 
 #include <linux/array_size.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/dev_printk.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
 #include <linux/kstrtox.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 #include <linux/sysfs.h>
@@ -49,10 +51,12 @@
  * @pwr_down_mask:	power down mask
  * @pwr_down_mode:	current power down mode
  * @data:		transfer buffer
+ * @lock:		lock to protect state and spi transfers
  */
 struct ad5504_state {
 	struct spi_device		*spi;
 	struct regulator		*reg;
+	struct mutex			lock;
 	unsigned short			vref_mv;
 	unsigned			pwr_down_mask;
 	unsigned			pwr_down_mode;
@@ -103,7 +107,8 @@ static int ad5504_read_raw(struct iio_dev *indio_dev,
 	int ret;
 
 	switch (m) {
-	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_RAW: {
+		guard(mutex)(&st->lock);
 		ret = ad5504_spi_read(st, chan->address);
 		if (ret < 0)
 			return ret;
@@ -111,6 +116,7 @@ static int ad5504_read_raw(struct iio_dev *indio_dev,
 		*val = ret;
 
 		return IIO_VAL_INT;
+	}
 	case IIO_CHAN_INFO_SCALE:
 		*val = st->vref_mv;
 		*val2 = chan->scan_type.realbits;
@@ -128,11 +134,13 @@ static int ad5504_write_raw(struct iio_dev *indio_dev,
 	struct ad5504_state *st = iio_priv(indio_dev);
 
 	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_RAW: {
+		guard(mutex)(&st->lock);
 		if (val >= (1 << chan->scan_type.realbits) || val < 0)
 			return -EINVAL;
 
 		return ad5504_spi_write(st, chan->address, val);
+	}
 	default:
 		return -EINVAL;
 	}
@@ -148,6 +156,7 @@ static int ad5504_get_powerdown_mode(struct iio_dev *indio_dev,
 {
 	struct ad5504_state *st = iio_priv(indio_dev);
 
+	guard(mutex)(&st->lock);
 	return st->pwr_down_mode;
 }
 
@@ -156,6 +165,7 @@ static int ad5504_set_powerdown_mode(struct iio_dev *indio_dev,
 {
 	struct ad5504_state *st = iio_priv(indio_dev);
 
+	guard(mutex)(&st->lock);
 	st->pwr_down_mode = mode;
 
 	return 0;
@@ -173,6 +183,7 @@ static ssize_t ad5504_read_dac_powerdown(struct iio_dev *indio_dev,
 {
 	struct ad5504_state *st = iio_priv(indio_dev);
 
+	guard(mutex)(&st->lock);
 	return sysfs_emit(buf, "%d\n",
 			  !(st->pwr_down_mask & (1 << chan->channel)));
 }
@@ -185,6 +196,7 @@ static ssize_t ad5504_write_dac_powerdown(struct iio_dev *indio_dev,
 	int ret;
 	struct ad5504_state *st = iio_priv(indio_dev);
 
+	guard(mutex)(&st->lock);
 	ret = kstrtobool(buf, &pwr_down);
 	if (ret)
 		return ret;
@@ -284,6 +296,10 @@ static int ad5504_probe(struct spi_device *spi)
 
 	st = iio_priv(indio_dev);
 
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		return ret;
+
 	ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
 	if (ret < 0)
 		return ret;
-- 
2.55.0

Re: [PATCH v4 4/6] iio: dac: ad5504: introduce local lock to protect state and spi transfers

[Andy Shevchenko]

On Mon, Aug 17, 2026 at 05:11:13PM -0400, Taha Ed-Dafili wrote:
> The AD5504 driver currently lacks locking, exposing it to several
> multi-threading race conditions:
> 1. The shared DMA-safe SPI transfer buffers (st->data) can be
>    corrupted if multiple threads trigger read_raw or write_raw
>    simultaneously.
> 2. The ad5504_write_dac_powerdown() routine executes a sequence of
>    back-to-back SPI writes (a CTRL register update followed by a
>    mandatory NOOP). This entire sequence must be atomic.
> 3. Internal state variables like pwr_down_mask and pwr_down_mode
>    can be read and modified concurrently.
> 
> Introduce a mutex in the ad5504_state structure and initialize it via
> devm_mutex_init() in probe. Use the modern scoped guard(mutex) macro
> at the top-level public IIO callbacks (read_raw, write_raw, and the
> powerdown attributes) to safely serialize access to the device state
> and the SPI bus.
> 
> In ad5504_read_raw() and ad5504_write_raw(), guard(mutex) is scoped to
> the IIO_CHAN_INFO_RAW case only, since IIO_CHAN_INFO_SCALE merely reads
> vref_mv, which is fixed at probe time and never modified afterward and
> therefore needs no serialization. Because guard(mutex) declares a
> cleanup-scoped variable, it cannot appear directly after a case label;
> wrap the case body in a compound statement (case IIO_CHAN_INFO_RAW: {
> ... }) to give it the block scope it requires.

Was this message written with a help with AI?

...

> +	case IIO_CHAN_INFO_RAW: {
> +		guard(mutex)(&st->lock);

Always use blank line(s) to separate the guard()() from the rest of the code.

>  		if (val >= (1 << chan->scan_type.realbits) || val < 0)
>  			return -EINVAL;

...

>  	struct ad5504_state *st = iio_priv(indio_dev);
>  
> +	guard(mutex)(&st->lock);

This is even stronger as we also require the blank line before return.

>  	return st->pwr_down_mode;

...

And so on...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 4/6] iio: dac: ad5504: introduce local lock to protect state and spi transfers

[Taha Ed-Dafili]

> Was this message written with a help with AI?

Yes, I used AI to help with clarity and kernel mailing list
conventions. The technical content and decisions are mine, learned
from reading other drivers and review feedback. I'll keep the
messages more concise going forward.

> Always use blank line(s) to separate the guard()() from the rest of
> the code.

Will fix all guard() call sites in v5.

Best Regards,
Taha Ed-Dafili

[PATCH v4 5/6] iio: dac: ad5504: strictly separate ACPI and DT probe paths

[Taha Ed-Dafili]

Refactor the ad5504_probe() function to explicitly separate the ACPI
and Device Tree execution paths. Previously, the driver relied on a
fragile -ENODEV return value check from the regulator framework to
bypass the voltage check on ACPI platforms.

Following modern IIO subsystem design patterns (such as those found in
adc/ti-ads7950.c), fork the logic using ACPI_COMPANION(). On ACPI
systems, where dedicated voltage regulators are typically omitted from
the firmware description, bypass the regulator subsystem entirely and
initialize the reference voltage to the hardware default 60V scale via
a new macro AD5504_VA_MV_ACPI_DEFAULT.

For Device Tree platforms, treat the VCC regulator as mandatory and
wrap the allocation in dev_err_probe() to cleanly handle potential
deferrals and error propagation.

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

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 87946cdf69e7..546420f9afc0 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -19,6 +19,7 @@
 #include <linux/spi/spi.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
+#include <linux/units.h>
 
 #include <linux/iio/events.h>
 #include <linux/iio/iio.h>
@@ -26,6 +27,10 @@
 
 #include <asm/byteorder.h>
 
+/*
+ * In case of ACPI, we use the 60 V as default voltage reference.
+ */
+#define AD5504_VA_MV_ACPI_DEFAULT	(60 * MILLI)
 #define AD5504_RES_MASK			GENMASK(11, 0)
 #define AD5504_CMD_READ			BIT(15)
 #define AD5504_CMD_WRITE		0
@@ -300,11 +305,16 @@ static int ad5504_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
-	if (ret < 0)
-		return ret;
+	if (ACPI_COMPANION(dev)) {
+		st->vref_mv = AD5504_VA_MV_ACPI_DEFAULT;
+	} else {
+		ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
+		if (ret < 0)
+			return dev_err_probe(dev, ret,
+					     "Failed to get vcc regulator\n");
 
-	st->vref_mv = ret / 1000;
+		st->vref_mv = ret / MILLI;
+	}
 
 	st->spi = spi;
 	indio_dev->name = spi_get_device_id(st->spi)->name;
-- 
2.55.0

Re: [PATCH v4 5/6] iio: dac: ad5504: strictly separate ACPI and DT probe paths

[Andy Shevchenko]

On Mon, Aug 17, 2026 at 05:11:14PM -0400, Taha Ed-Dafili wrote:
> Refactor the ad5504_probe() function to explicitly separate the ACPI
> and Device Tree execution paths. Previously, the driver relied on a
> fragile -ENODEV return value check from the regulator framework to
> bypass the voltage check on ACPI platforms.
> 
> Following modern IIO subsystem design patterns (such as those found in
> adc/ti-ads7950.c), fork the logic using ACPI_COMPANION(). On ACPI
> systems, where dedicated voltage regulators are typically omitted from
> the firmware description, bypass the regulator subsystem entirely and
> initialize the reference voltage to the hardware default 60V scale via
> a new macro AD5504_VA_MV_ACPI_DEFAULT.
> 
> For Device Tree platforms, treat the VCC regulator as mandatory and
> wrap the allocation in dev_err_probe() to cleanly handle potential
> deferrals and error propagation.

...

> +/*
> + * In case of ACPI, we use the 60 V as default voltage reference.
> + */
> +#define AD5504_VA_MV_ACPI_DEFAULT	(60 * MILLI)

Name it

#define AD5504_VA_ACPI_DEFAULT_mV	(60 * MILLI)

What does VA stand for?

...

> +	if (ACPI_COMPANION(dev)) {

It's better to use has_acpi_companion() or is_acpi_device_node().
I prefer to see the latter as that one unifies the style of checking
across the drivers and subsystems.

For that you will need to use dev_fwnode() from property.h and acpi.h
for the macro itself.

> +		st->vref_mv = AD5504_VA_MV_ACPI_DEFAULT;

...

> -	st->vref_mv = ret / 1000;
> +		st->vref_mv = ret / MILLI;

This should be  (MICRO / MILLI) instead of MILLI.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v4 6/6] iio: dac: ad5504: support scale via output-range-microvolt property

[Taha Ed-Dafili]

The AD5504/AD5501 output range (0-30V or 0-60V) is determined by the
hardware R_SEL pin and is required to compute IIO_CHAN_INFO_SCALE
correctly. Previously the driver derived this solely from the vcc
regulator's configured voltage, which conflated the supply voltage
with the DAC's actual output range and offered no way to express the
range explicitly in firmware.

Add support for the standard 'output-range-microvolt' device property,
validating that it specifies one of the two supported ranges (0-30V
or 0-60V) and using it to set st->vref_mv directly. When this property
is present, the vcc regulator is only enabled (not read) via
devm_regulator_get_enable(), since the regulator's actual voltage is
no longer the source of truth for the output range.

For backward compatibility with older device trees that predate this
property, fall back to reading the vcc regulator's configured voltage
via devm_regulator_get_enable_read_voltage() when
'output-range-microvolt' is absent.

Use device_property_present() to explicitly distinguish "property
absent" from "property present but malformed", rather than relying on
the -EINVAL return from device_property_read_u32_array() as an
absence sentinel. That return code is ambiguous: it is also returned
when the property exists but the parsed array size does not match the
expected length, which would have silently and incorrectly routed a
malformed property through the legacy regulator-voltage fallback
instead of surfacing a clear validation error.

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

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 546420f9afc0..25c0744545f0 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -15,6 +15,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 #include <linux/sysfs.h>
@@ -293,6 +294,7 @@ static int ad5504_probe(struct spi_device *spi)
 	struct device *dev = &spi->dev;
 	struct iio_dev *indio_dev;
 	struct ad5504_state *st;
+	u32 range[2];
 	int ret;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
@@ -308,12 +310,30 @@ static int ad5504_probe(struct spi_device *spi)
 	if (ACPI_COMPANION(dev)) {
 		st->vref_mv = AD5504_VA_MV_ACPI_DEFAULT;
 	} else {
-		ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
-		if (ret < 0)
-			return dev_err_probe(dev, ret,
-					     "Failed to get vcc regulator\n");
-
-		st->vref_mv = ret / MILLI;
+		if (device_property_present(dev, "output-range-microvolt")) {
+			ret = device_property_read_u32_array(dev, "output-range-microvolt",
+							     range, ARRAY_SIZE(range));
+			if (ret)
+				return dev_err_probe(dev, ret,
+							"Error parsing output-range-microvolt\n");
+
+			if (range[0] != 0 || (range[1] != 30 * MICRO && range[1] != 60 * MICRO))
+				return dev_err_probe(dev, -EINVAL,
+					"Invalid output-range-microvolt\n");
+
+			st->vref_mv = range[1] / MILLI;
+
+			ret = devm_regulator_get_enable(dev, "vcc");
+			if (ret < 0)
+				return dev_err_probe(dev, ret, "Failed to enable vcc regulator\n");
+		} else {
+			/* Backward compat: old DTs without output-range-microvolt */
+			ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
+			if (ret < 0)
+				return dev_err_probe(dev, ret, "Failed to get vcc regulator\n");
+
+			st->vref_mv = ret / MILLI;
+		}
 	}
 
 	st->spi = spi;
-- 
2.55.0

Re: [PATCH v4 6/6] iio: dac: ad5504: support scale via output-range-microvolt property

[Andy Shevchenko]

On Mon, Aug 17, 2026 at 05:11:15PM -0400, Taha Ed-Dafili wrote:
> The AD5504/AD5501 output range (0-30V or 0-60V) is determined by the
> hardware R_SEL pin and is required to compute IIO_CHAN_INFO_SCALE
> correctly. Previously the driver derived this solely from the vcc
> regulator's configured voltage, which conflated the supply voltage
> with the DAC's actual output range and offered no way to express the
> range explicitly in firmware.
> 
> Add support for the standard 'output-range-microvolt' device property,
> validating that it specifies one of the two supported ranges (0-30V
> or 0-60V) and using it to set st->vref_mv directly. When this property
> is present, the vcc regulator is only enabled (not read) via
> devm_regulator_get_enable(), since the regulator's actual voltage is
> no longer the source of truth for the output range.
> 
> For backward compatibility with older device trees that predate this
> property, fall back to reading the vcc regulator's configured voltage
> via devm_regulator_get_enable_read_voltage() when
> 'output-range-microvolt' is absent.
> 
> Use device_property_present() to explicitly distinguish "property
> absent" from "property present but malformed", rather than relying on
> the -EINVAL return from device_property_read_u32_array() as an
> absence sentinel. That return code is ambiguous: it is also returned
> when the property exists but the parsed array size does not match the
> expected length, which would have silently and incorrectly routed a
> malformed property through the legacy regulator-voltage fallback
> instead of surfacing a clear validation error.

We do not need a Dostoevsky novel in the commit message. Please, make it
straight to the point. Don't blindly use AI, you should understand what
the code is doing. Same comment to all commit messages in the series.

...

> -		st->vref_mv = ret / MILLI;

So, the change from 1000 to MILLI is in the line that is changed again in this
patch. So, no need to change it earlier. And again, it should be (MICRO / MILLI).

> +			st->vref_mv = range[1] / MILLI;

> +			st->vref_mv = ret / MILLI;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 6/6] iio: dac: ad5504: support scale via output-range-microvolt property

[Taha Ed-Dafili]

On Mon, Aug 18, 2026 at 06:30:00AM +0300, Andy Shevchenko wrote:
> We do not need a Dostoevsky novel in the commit message. Please, make
> it straight to the point. Don't blindly use AI, you should understand
> what the code is doing. Same comment to all commit messages in the
> series.

Understood, will trim all commit messages in v5. My workflow is to
draft the messages myself then use AI to check clarity and kernel
mailing list conventions. I'll make sure that doesn't result in
over-explained prose going forward.

> So, the change from 1000 to MILLI is in the line that is changed
> again in this patch. So, no need to change it earlier. And again, it
> should be (MICRO / MILLI).

I started the code with a bare / 1000, then replaced it with MILLI
thinking it was the more readable form. I wasn't aware of the
(MICRO / MILLI) idiom for expressing unit conversions explicitly.
Will fix in v5 and drop the intermediate change from patch 5.

Best Regards,
Taha Ed-dafili

Re: [PATCH v4 6/6] iio: dac: ad5504: support scale via output-range-microvolt property

[Andy Shevchenko]

On Tue, Aug 18, 2026 at 04:23:47AM -0400, Taha Ed-Dafili wrote:
> On Mon, Aug 18, 2026 at 06:30:00AM +0300, Andy Shevchenko wrote:

...

> > So, the change from 1000 to MILLI is in the line that is changed
> > again in this patch. So, no need to change it earlier. And again, it
> > should be (MICRO / MILLI).
> 
> I started the code with a bare / 1000, then replaced it with MILLI
> thinking it was the more readable form. I wasn't aware of the
> (MICRO / MILLI) idiom for expressing unit conversions explicitly.
> Will fix in v5 and drop the intermediate change from patch 5.

Since you want to get a millivolts from microvolts the 1000 is semantically
(MICRO / MILLI).

-- 
With Best Regards,
Andy Shevchenko