* [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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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
2026-08-21 11:41 ` [PATCH v5 3/6] iio: dac: ad5504: strictly separate ACPI and DT probe paths Taha Ed-Dafili
0 siblings, 1 reply; 12+ 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] 12+ messages in thread* Re: [PATCH v5 3/6] iio: dac: ad5504: strictly separate ACPI and DT probe paths
2026-08-21 11:20 ` Joshua Crofts
@ 2026-08-21 11:41 ` Taha Ed-Dafili
2026-08-21 11:58 ` Joshua Crofts
0 siblings, 1 reply; 12+ messages in thread
From: Taha Ed-Dafili @ 2026-08-21 11:41 UTC (permalink / raw)
To: joshua.crofts1
Cc: 0rayn.dev, Michael.Hennerich, andy, conor+dt, devicetree,
dlechner, jic23, krzk+dt, lars, linux-iio, linux-kernel, linux,
nuno.sa, robh, skhan
> ret / (MICRO / MILLI)
This exact line was discussed with Andy on v4. Patch 6 in this series
replaces this whole branch with the output-range-microvolt property
parsing, which does use (MICRO / MILLI) correctly. We agreed it wasn't
worth changing here to avoid an unnecessary intermediate diff. Let me
know if it would be better to fix it at this patch regardless.
Best Regards,
Taha Ed-Dafili
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/6] iio: dac: ad5504: strictly separate ACPI and DT probe paths
2026-08-21 11:41 ` [PATCH v5 3/6] iio: dac: ad5504: strictly separate ACPI and DT probe paths Taha Ed-Dafili
@ 2026-08-21 11:58 ` Joshua Crofts
0 siblings, 0 replies; 12+ messages in thread
From: Joshua Crofts @ 2026-08-21 11:58 UTC (permalink / raw)
To: Taha Ed-Dafili
Cc: Michael.Hennerich, andy, conor+dt, devicetree, dlechner, jic23,
krzk+dt, lars, linux-iio, linux-kernel, linux, nuno.sa, robh,
skhan
On Fri, 21 Aug 2026 07:41:53 -0400
Taha Ed-Dafili <0rayn.dev@gmail.com> wrote:
> > ret / (MICRO / MILLI)
>
> This exact line was discussed with Andy on v4. Patch 6 in this series
> replaces this whole branch with the output-range-microvolt property
> parsing, which does use (MICRO / MILLI) correctly. We agreed it wasn't
> worth changing here to avoid an unnecessary intermediate diff. Let me
> know if it would be better to fix it at this patch regardless.
>
Ah, hadn't gotten to the v4 discussion, in that case it's fine!
--
Kind regards,
Joshua Crofts
^ permalink raw reply [flat|nested] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ messages in thread