Devicetree
 help / color / mirror / Atom feed
* [PATCH v5 0/6] iio: dac: ad5504: bindings, cleanups, locking, and scale fixes
@ 2026-08-21 10:24 Taha Ed-Dafili
  2026-08-21 10:24 ` [PATCH v5 1/6] dt-bindings: iio: dac: ad5504: add output-range and missing gpios Taha Ed-Dafili
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Taha Ed-Dafili @ 2026-08-21 10:24 UTC (permalink / raw)
  To: jic23, lars
  Cc: Michael.Hennerich, robh, krzk+dt, conor+dt, dlechner, nuno.sa,
	andy, skhan, linux, linux-iio, devicetree, linux-kernel,
	Taha Ed-Dafili

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.

This v5 series addresses feedback from the v4 review by Jonathan
Cameron, Andy Shevchenko and Uwe Kleine-König:

* Drop <linux/mod_devicetable.h> — now covered by spi.h (Andy, Uwe)
* Add blank lines after all guard() call sites (Andy)
* Rename macro to AD5504_VREF_ACPI_DEFAULT_mV (Andy)
* Switch to is_acpi_device_node(dev_fwnode(dev)) (Andy)
* Use (MICRO / MILLI) for microvolt-to-millivolt conversions (Andy)
* Trim all commit messages (Andy, Jonathan)
* Reorder ACPI/DT separation before pdata removal to avoid an
  intermediate commit that hard-fails probe on regulator-less
  systems (issue raised by Andy)

Testing:
Re-verified on ARM64 Raspberry Pi 5 (BCM2712) using a custom Device
Tree overlay on spi10, without physical AD5504 hardware. All test
cases from v4 pass unchanged with the v5 code:

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>):
[   94.762850] ad5504 spi10.0: error -EINVAL: Invalid output-range-microvolt
[   94.762857] 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:
v4: https://lore.kernel.org/linux-iio/20260817211118.21833-1-0rayn.dev@gmail.com/
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, Conor Dooley and Uwe Kleine-König 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: separate ACPI and DT paths
  iio: dac: ad5504: remove legacy platform data support
  iio: dac: ad5504: introduce local lock to protect state and spi
    transfers
  iio: dac: ad5504: support scale via output-range-microvolt property

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

-- 
2.55.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 1/6] dt-bindings: iio: dac: ad5504: add output-range and missing gpios
  2026-08-21 10:24 [PATCH v5 0/6] iio: dac: ad5504: bindings, cleanups, locking, and scale fixes Taha Ed-Dafili
@ 2026-08-21 10:24 ` Taha Ed-Dafili
  2026-08-21 10:36   ` sashiko-bot
  2026-08-21 10:24 ` [PATCH v5 2/6] iio: dac: ad5504: Align headers with IWYU principle Taha Ed-Dafili
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Taha Ed-Dafili @ 2026-08-21 10:24 UTC (permalink / raw)
  To: jic23, lars
  Cc: Michael.Hennerich, robh, krzk+dt, conor+dt, dlechner, nuno.sa,
	andy, skhan, linux, linux-iio, devicetree, linux-kernel,
	Taha Ed-Dafili, Conor Dooley

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v5 2/6] iio: dac: ad5504: Align headers with IWYU principle
  2026-08-21 10:24 [PATCH v5 0/6] iio: dac: ad5504: bindings, cleanups, locking, and scale fixes Taha Ed-Dafili
  2026-08-21 10:24 ` [PATCH v5 1/6] dt-bindings: iio: dac: ad5504: add output-range and missing gpios Taha Ed-Dafili
@ 2026-08-21 10:24 ` Taha Ed-Dafili
  2026-08-21 10:24 ` [PATCH v5 3/6] iio: dac: ad5504: separate ACPI and DT paths Taha Ed-Dafili
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Taha Ed-Dafili @ 2026-08-21 10:24 UTC (permalink / raw)
  To: jic23, lars
  Cc: Michael.Hennerich, robh, krzk+dt, conor+dt, dlechner, nuno.sa,
	andy, skhan, linux, linux-iio, devicetree, linux-kernel,
	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/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 | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index ad1a74678cf2..5c9c66b04154 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -5,22 +5,25 @@
  * 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/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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v5 3/6] iio: dac: ad5504: separate ACPI and DT paths
  2026-08-21 10:24 [PATCH v5 0/6] iio: dac: ad5504: bindings, cleanups, locking, and scale fixes Taha Ed-Dafili
  2026-08-21 10:24 ` [PATCH v5 1/6] dt-bindings: iio: dac: ad5504: add output-range and missing gpios Taha Ed-Dafili
  2026-08-21 10:24 ` [PATCH v5 2/6] iio: dac: ad5504: Align headers with IWYU principle Taha Ed-Dafili
@ 2026-08-21 10:24 ` Taha Ed-Dafili
  2026-08-21 11:20   ` Joshua Crofts
  2026-08-21 10:24 ` [PATCH v5 4/6] iio: dac: ad5504: remove legacy platform data support Taha Ed-Dafili
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Taha Ed-Dafili @ 2026-08-21 10:24 UTC (permalink / raw)
  To: jic23, lars
  Cc: Michael.Hennerich, robh, krzk+dt, conor+dt, dlechner, nuno.sa,
	andy, skhan, linux, linux-iio, devicetree, linux-kernel,
	Taha Ed-Dafili

The driver relied on a fragile -ENODEV check from the regulator
framework to bypass voltage configuration on ACPI platforms, sharing
the same fallback path as legacy pdata.

Fork probe on is_acpi_device_node() to explicitly handle each path.
On ACPI, skip the regulator and use a fixed 60V default via
AD5504_VREF_ACPI_DEFAULT_mV. The pdata fallback remains for non-ACPI
systems in this commit; it is removed in the next.

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

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 5c9c66b04154..6f70c57c7a82 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -5,6 +5,7 @@
  * Copyright 2011 Analog Devices Inc.
  */
 
+#include <linux/acpi.h>
 #include <linux/array_size.h>
 #include <linux/bits.h>
 #include <linux/dev_printk.h>
@@ -12,10 +13,12 @@
 #include <linux/interrupt.h>
 #include <linux/kstrtox.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>
@@ -24,6 +27,10 @@
 
 #include <asm/byteorder.h>
 
+/*
+ * In case of ACPI, we use the 60 V as default voltage reference.
+ */
+#define AD5504_VREF_ACPI_DEFAULT_mV	(60 * MILLI)
 #define AD5504_RES_MASK			GENMASK(11, 0)
 #define AD5504_CMD_READ			BIT(15)
 #define AD5504_CMD_WRITE		0
@@ -285,16 +292,20 @@ 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)
-		return ret;
-	if (ret == -ENODEV) {
-		if (pdata->vref_mv)
-			st->vref_mv = pdata->vref_mv;
-		else
-			dev_warn(dev, "reference voltage unspecified\n");
+	if (is_acpi_device_node(dev_fwnode(dev))) {
+		st->vref_mv = AD5504_VREF_ACPI_DEFAULT_mV;
 	} else {
-		st->vref_mv = ret / 1000;
+		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(dev, "reference voltage unspecified\n");
+		} else {
+			st->vref_mv = ret / 1000;
+		}
 	}
 
 	st->spi = spi;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v5 4/6] iio: dac: ad5504: remove legacy platform data support
  2026-08-21 10:24 [PATCH v5 0/6] iio: dac: ad5504: bindings, cleanups, locking, and scale fixes Taha Ed-Dafili
                   ` (2 preceding siblings ...)
  2026-08-21 10:24 ` [PATCH v5 3/6] iio: dac: ad5504: separate ACPI and DT paths Taha Ed-Dafili
@ 2026-08-21 10:24 ` Taha Ed-Dafili
  2026-08-21 10:36   ` sashiko-bot
  2026-08-21 10:24 ` [PATCH v5 5/6] iio: dac: ad5504: introduce local lock to protect state and spi transfers Taha Ed-Dafili
  2026-08-21 10:24 ` [PATCH v5 6/6] iio: dac: ad5504: support scale via output-range-microvolt property Taha Ed-Dafili
  5 siblings, 1 reply; 10+ messages in thread
From: Taha Ed-Dafili @ 2026-08-21 10:24 UTC (permalink / raw)
  To: jic23, lars
  Cc: Michael.Hennerich, robh, krzk+dt, conor+dt, dlechner, nuno.sa,
	andy, skhan, linux, linux-iio, devicetree, linux-kernel,
	Taha Ed-Dafili

The AD5504 driver has no in-tree users of its legacy platform data
support. Now that ACPI has its own explicit path, non-ACPI systems can
safely treat the vcc regulator as mandatory.

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

Signed-off-by: Taha Ed-Dafili <0rayn.dev@gmail.com>
---
 drivers/iio/dac/ad5504.c       | 17 +++++------------
 include/linux/iio/dac/ad5504.h | 15 ---------------
 2 files changed, 5 insertions(+), 27 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 6f70c57c7a82..799726eafdf2 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -20,7 +20,6 @@
 #include <linux/types.h>
 #include <linux/units.h>
 
-#include <linux/iio/dac/ad5504.h>
 #include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -281,7 +280,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;
@@ -296,16 +294,11 @@ static int ad5504_probe(struct spi_device *spi)
 		st->vref_mv = AD5504_VREF_ACPI_DEFAULT_mV;
 	} else {
 		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(dev, "reference voltage unspecified\n");
-		} else {
-			st->vref_mv = ret / 1000;
-		}
+		if (ret < 0)
+			return dev_err_probe(dev, ret,
+					     "Failed to get vcc regulator\n");
+
+		st->vref_mv = ret / 1000;
 	}
 
 	st->spi = spi;
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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v5 5/6] iio: dac: ad5504: introduce local lock to protect state and spi transfers
  2026-08-21 10:24 [PATCH v5 0/6] iio: dac: ad5504: bindings, cleanups, locking, and scale fixes Taha Ed-Dafili
                   ` (3 preceding siblings ...)
  2026-08-21 10:24 ` [PATCH v5 4/6] iio: dac: ad5504: remove legacy platform data support Taha Ed-Dafili
@ 2026-08-21 10:24 ` Taha Ed-Dafili
  2026-08-21 10:24 ` [PATCH v5 6/6] iio: dac: ad5504: support scale via output-range-microvolt property Taha Ed-Dafili
  5 siblings, 0 replies; 10+ messages in thread
From: Taha Ed-Dafili @ 2026-08-21 10:24 UTC (permalink / raw)
  To: jic23, lars
  Cc: Michael.Hennerich, robh, krzk+dt, conor+dt, dlechner, nuno.sa,
	andy, skhan, linux, linux-iio, devicetree, linux-kernel,
	Taha Ed-Dafili

The driver lacks locking, exposing concurrent access to the shared DMA
SPI buffers, the non-atomic CTRL+NOOP write sequence in powerdown, and
the pwr_down_mask/pwr_down_mode state variables.

Introduce a mutex via devm_mutex_init() and guard(mutex) at the IIO
callbacks to serialize access. In read_raw() and write_raw(), scope the
lock to IIO_CHAN_INFO_RAW only since IIO_CHAN_INFO_SCALE reads vref_mv
which is immutable after probe.

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

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 799726eafdf2..489d2e03e9f6 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -8,11 +8,13 @@
 #include <linux/acpi.h>
 #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/module.h>
+#include <linux/mutex.h>
 #include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
@@ -55,10 +57,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;
@@ -109,7 +113,9 @@ 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;
@@ -117,6 +123,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;
@@ -134,11 +141,14 @@ 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;
 	}
@@ -154,6 +164,8 @@ 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;
 }
 
@@ -162,6 +174,8 @@ 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;
@@ -179,6 +193,8 @@ 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)));
 }
@@ -191,6 +207,8 @@ 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;
@@ -290,6 +308,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;
+
 	if (is_acpi_device_node(dev_fwnode(dev))) {
 		st->vref_mv = AD5504_VREF_ACPI_DEFAULT_mV;
 	} else {
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v5 6/6] iio: dac: ad5504: support scale via output-range-microvolt property
  2026-08-21 10:24 [PATCH v5 0/6] iio: dac: ad5504: bindings, cleanups, locking, and scale fixes Taha Ed-Dafili
                   ` (4 preceding siblings ...)
  2026-08-21 10:24 ` [PATCH v5 5/6] iio: dac: ad5504: introduce local lock to protect state and spi transfers Taha Ed-Dafili
@ 2026-08-21 10:24 ` Taha Ed-Dafili
  5 siblings, 0 replies; 10+ messages in thread
From: Taha Ed-Dafili @ 2026-08-21 10:24 UTC (permalink / raw)
  To: jic23, lars
  Cc: Michael.Hennerich, robh, krzk+dt, conor+dt, dlechner, nuno.sa,
	andy, skhan, linux, linux-iio, devicetree, linux-kernel,
	Taha Ed-Dafili

The AD5504 output range is set by the R_SEL pin, not the VCC supply.
Add support for 'output-range-microvolt' to set vref_mv directly from
firmware. Use device_property_present() to distinguish absent from
malformed property rather than relying on the ambiguous -EINVAL from
device_property_read_u32_array(). Fall back to the vcc regulator
voltage for old DTs that predate this property.

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

diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 489d2e03e9f6..19ece5a2af58 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -300,6 +300,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));
@@ -315,12 +316,30 @@ static int ad5504_probe(struct spi_device *spi)
 	if (is_acpi_device_node(dev_fwnode(dev))) {
 		st->vref_mv = AD5504_VREF_ACPI_DEFAULT_mV;
 	} 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;
+		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] / (MICRO / 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 / (MICRO / MILLI);
+		}
 	}
 
 	st->spi = spi;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/6] dt-bindings: iio: dac: ad5504: add output-range and missing gpios
  2026-08-21 10:24 ` [PATCH v5 1/6] dt-bindings: iio: dac: ad5504: add output-range and missing gpios Taha Ed-Dafili
@ 2026-08-21 10:36   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-21 10:36 UTC (permalink / raw)
  To: Taha Ed-Dafili; +Cc: conor+dt, devicetree, robh

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `output-range-microvolt` property is defined with an `items` array but lacks `minItems: 2` and `maxItems: 2` constraints.
- [Medium] The `vlogic-supply` property is missing from the devicetree example, contradicting the commit message claim.
--

--- Patch [1]: [PATCH v5 1/6] dt-bindings: iio: dac: ad5504: add output-range and missing gpios ---
commit fae484c36ed84e118c97518d409f24eed9d5eadd
Author: Taha Ed-Dafili <0rayn.dev@gmail.com>
Subject: dt-bindings: iio: dac: ad5504: add output-range and missing gpios

This commit updates the AD5504 device tree binding to support output voltage
range selection via the R_SEL pin. It adds output-range-microvolt and missing
GPIO properties, enforces mutual exclusivity for range selection, and updates
the documentation with a 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
[ ... ]
> @@ -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]

[Severity: Medium]
Should this property include minItems and maxItems constraints? Without 
explicit bounds like maxItems: 2, will arrays of arbitrary length or missing 
elements silently pass schema validation?

> +
> +  range-sel-gpios:
> +    description:
> +      GPIO connected to the R_SEL pin to select the output voltage range.
> +    maxItems: 1
[ ... ]
> @@ -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>;
>          };
>      };
>  ...

[Severity: Medium]
The commit message states this patch will "add missing vlogic-supply...
and provide a complete usage example." Is vlogic-supply intentionally omitted
from this example block?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 4/6] iio: dac: ad5504: remove legacy platform data support
  2026-08-21 10:24 ` [PATCH v5 4/6] iio: dac: ad5504: remove legacy platform data support Taha Ed-Dafili
@ 2026-08-21 10:36   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-21 10:36 UTC (permalink / raw)
  To: Taha Ed-Dafili; +Cc: devicetree, robh, conor+dt

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver unconditionally dereferences `spi_get_device_id()` without checking for NULL.
--

--- Patch [4]: [PATCH v5 4/6] iio: dac: ad5504: remove legacy platform data support ---
commit 71e5a82418860f4a272cfaab189a444dee114159
Author: Taha Ed-Dafili <0rayn.dev@gmail.com>

iio: dac: ad5504: remove legacy platform data support

This commit removes the legacy platform data support from the AD5504 driver
since there are no in-tree users. It drops the ad5504_platform_data
structure and updates the probe logic to treat the vcc regulator as
mandatory on non-ACPI systems.

> diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
> index 830b8ea8eaebf..96ab6d456d80e 100644
> --- a/drivers/iio/dac/ad5504.c
> +++ b/drivers/iio/dac/ad5504.c
> @@ -296,16 +294,11 @@ static int ad5504_probe(struct spi_device *spi)
[ ... ]
>  	}
>  
>  	st->spi = spi;
>  	indio_dev->name = spi_get_device_id(st->spi)->name;
>  	indio_dev->info = &ad5504_info;
>  	if (spi_get_device_id(st->spi)->driver_data == ID_AD5501)

[Severity: High]
This is a pre-existing issue, but does this code unconditionally dereference
the return value of spi_get_device_id() in ad5504_probe()?

If a privileged user writes an unmatched device name to the driver's sysfs
bind or driver_override interface, it forces a driver match via
device_match_driver_override(). Because the device's modalias might not be
in the driver's id_table, spi_match_id() would return NULL.

Could this sequence lead to spi_get_device_id() returning NULL and causing
a kernel panic when its name or driver_data fields are accessed here?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 3/6] iio: dac: ad5504: separate ACPI and DT paths
  2026-08-21 10:24 ` [PATCH v5 3/6] iio: dac: ad5504: separate ACPI and DT paths Taha Ed-Dafili
@ 2026-08-21 11:20   ` Joshua Crofts
  0 siblings, 0 replies; 10+ messages in thread
From: Joshua Crofts @ 2026-08-21 11:20 UTC (permalink / raw)
  To: Taha Ed-Dafili
  Cc: jic23, lars, Michael.Hennerich, robh, krzk+dt, conor+dt, dlechner,
	nuno.sa, andy, skhan, linux, linux-iio, devicetree, linux-kernel

On Fri, 21 Aug 2026 06:24:45 -0400
Taha Ed-Dafili <0rayn.dev@gmail.com> wrote:
> @@ -285,16 +292,20 @@ 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)
> -		return ret;
> -	if (ret == -ENODEV) {
> -		if (pdata->vref_mv)
> -			st->vref_mv = pdata->vref_mv;
> -		else
> -			dev_warn(dev, "reference voltage unspecified\n");
> +	if (is_acpi_device_node(dev_fwnode(dev))) {
> +		st->vref_mv = AD5504_VREF_ACPI_DEFAULT_mV;
>  	} else {
> -		st->vref_mv = ret / 1000;
> +		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(dev, "reference voltage unspecified\n");
> +		} else {
> +			st->vref_mv = ret / 1000;

ret / (MICRO / MILLI)

-- 
Kind regards,
Joshua Crofts

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-08-21 11:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-21 10:24 [PATCH v5 0/6] iio: dac: ad5504: bindings, cleanups, locking, and scale fixes Taha Ed-Dafili
2026-08-21 10:24 ` [PATCH v5 1/6] dt-bindings: iio: dac: ad5504: add output-range and missing gpios Taha Ed-Dafili
2026-08-21 10:36   ` sashiko-bot
2026-08-21 10:24 ` [PATCH v5 2/6] iio: dac: ad5504: Align headers with IWYU principle Taha Ed-Dafili
2026-08-21 10:24 ` [PATCH v5 3/6] iio: dac: ad5504: separate ACPI and DT paths Taha Ed-Dafili
2026-08-21 11:20   ` Joshua Crofts
2026-08-21 10:24 ` [PATCH v5 4/6] iio: dac: ad5504: remove legacy platform data support Taha Ed-Dafili
2026-08-21 10:36   ` sashiko-bot
2026-08-21 10:24 ` [PATCH v5 5/6] iio: dac: ad5504: introduce local lock to protect state and spi transfers Taha Ed-Dafili
2026-08-21 10:24 ` [PATCH v5 6/6] iio: dac: ad5504: support scale via output-range-microvolt property Taha Ed-Dafili

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox