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

[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

[sashiko-bot]

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

New issues:
- [High] The driver deletes legacy platform data for configuring the output range but fails to implement Device Tree parsing to replace it, leading to incorrect IIO scale reporting.
- [High] Silent failure to probe if the optional 'vcc-supply' regulator is omitted from the device tree.

Pre-existing issues:
- [High] Missing mutex lock protecting shared SPI transfer buffers and device state during concurrent sysfs accesses.
--

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

This commit removes legacy platform data support from the AD5504 driver,
deleting the ad5504_platform_data structure and the fallback configuration
logic in the probe function.

> diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
> index 1793af0781dc7..79db277b776e2 100644
> --- a/drivers/iio/dac/ad5504.c
> +++ b/drivers/iio/dac/ad5504.c
[ ... ]
> @@ -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;

[Severity: High]
Will this result in a silent probe failure if the optional vcc-supply
regulator is omitted from the device tree?

Since devm_regulator_get_enable_read_voltage() internally uses
devm_regulator_get_optional(), it returns -ENODEV if the regulator is
absent.

Propagating this error directly without logging causes the driver core to
treat it as a silent match rejection, whereas before it gracefully continued.

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

[Severity: High]
Does this missing implementation of device tree parsing lead to incorrect IIO
scale reporting?

The code currently hardcodes the reference voltage to the vcc supply
voltage without providing a fallback to parse the newly added
output-range-microvolt property.

If a system has a 60V VCC supply but the R_SEL pin is configured for a
30V output range, the device will now incorrectly report a 60V scale to
userspace since the fallback logic was removed.

[ ... ]

[Severity: High]
This is a pre-existing issue, but is it possible for concurrent sysfs
accesses to corrupt memory or disrupt hardware state?

Looking at ad5504_spi_write() and ad5504_spi_read(), the shared spi buffers
are accessed without locking:

drivers/iio/dac/ad5504.c:ad5504_spi_write() {
    ...
    st->data[0] = cpu_to_be16(AD5504_CMD_WRITE | AD5504_ADDR(addr) |
                              (val & AD5504_RES_MASK));

    return spi_write(st->spi, &st->data[0], 2);
}

Additionally, in ad5504_write_dac_powerdown(), multiple SPI writes are
performed and st->pwr_down_mask is modified without serialization:

drivers/iio/dac/ad5504.c:ad5504_write_dac_powerdown() {
    ...
    if (pwr_down)
        st->pwr_down_mask &= ~(1 << chan->channel);
    else
        st->pwr_down_mask |= (1 << chan->channel);
    
    ret = ad5504_spi_write(st, AD5504_ADDR_CTRL,
                           AD5504_DAC_PWRDWN_MODE(st->pwr_down_mode) |
                           AD5504_DAC_PWR(st->pwr_down_mask));
    
    /* writes to the CTRL register must be followed by a NOOP */
    ad5504_spi_write(st, AD5504_ADDR_NOOP, 0);
    ...
}

Could concurrent reads or writes by userspace processes to sysfs attributes
interleave SPI commands or corrupt the DMA buffer in st->data?

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

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

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

[Taha Ed-Dafili]

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

Yes, David Lechner suggested in v3 to follow the pattern in
adc/ti-ads7950.c. Patch 3 is an intermediate state. Patch 5 replaces
this block with ACPI_COMPANION() fork.

Best Regards,
Taha Ed-dafili

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

[Andy Shevchenko]

On Wed, Aug 19, 2026 at 04:22:21AM -0400, Taha Ed-Dafili wrote:
> > This breaks the case with dummy regulators as far as I can see.
> > Has this been discussed in the previous rounds?
> 
> Yes, David Lechner suggested in v3 to follow the pattern in
> adc/ti-ads7950.c. Patch 3 is an intermediate state. Patch 5 replaces
> this block with ACPI_COMPANION() fork.

But this is still a runtime bisectability issue.
Not critical most likely, but still...

-- 
With Best Regards,
Andy Shevchenko

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

[Taha Ed-Dafili]

> But this is still a runtime bisectability issue.
> Not critical most likely, but still...

I see your point. My plan for v5 is to reorder: introduce the ACPI/DT
fork first, then remove pdata on top of it. That way the -ENODEV case
is already handled by the ACPI fork when pdata is removed. What do you
think of this approach?

Best Regards,
Taha Ed-dafili