* [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