From: Taha Ed-Dafili <0rayn.dev@gmail.com>
To: jic23@kernel.org, lars@metafoo.de
Cc: Michael.Hennerich@analog.com, robh@kernel.org,
krzk+dt@kernel.org, conor+dt@kernel.org, dlechner@baylibre.com,
nuno.sa@analog.com, andy@kernel.org, skhan@linuxfoundation.org,
linux@analog.com, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Taha Ed-Dafili <0rayn.dev@gmail.com>
Subject: [PATCH v4 4/6] iio: dac: ad5504: introduce local lock to protect state and spi transfers
Date: Mon, 17 Aug 2026 17:11:13 -0400 [thread overview]
Message-ID: <20260817211118.21833-5-0rayn.dev@gmail.com> (raw)
In-Reply-To: <20260817211118.21833-1-0rayn.dev@gmail.com>
The AD5504 driver currently lacks locking, exposing it to several
multi-threading race conditions:
1. The shared DMA-safe SPI transfer buffers (st->data) can be
corrupted if multiple threads trigger read_raw or write_raw
simultaneously.
2. The ad5504_write_dac_powerdown() routine executes a sequence of
back-to-back SPI writes (a CTRL register update followed by a
mandatory NOOP). This entire sequence must be atomic.
3. Internal state variables like pwr_down_mask and pwr_down_mode
can be read and modified concurrently.
Introduce a mutex in the ad5504_state structure and initialize it via
devm_mutex_init() in probe. Use the modern scoped guard(mutex) macro
at the top-level public IIO callbacks (read_raw, write_raw, and the
powerdown attributes) to safely serialize access to the device state
and the SPI bus.
In ad5504_read_raw() and ad5504_write_raw(), guard(mutex) is scoped to
the IIO_CHAN_INFO_RAW case only, since IIO_CHAN_INFO_SCALE merely reads
vref_mv, which is fixed at probe time and never modified afterward and
therefore needs no serialization. Because guard(mutex) declares a
cleanup-scoped variable, it cannot appear directly after a case label;
wrap the case body in a compound statement (case IIO_CHAN_INFO_RAW: {
... }) to give it the block scope it requires.
Signed-off-by: Taha Ed-Dafili <0rayn.dev@gmail.com>
---
Note: The concurrency race conditions addressed in this patch were
originally reported by the Sashiko bot:
https://sashiko.dev/#/patchset/20260509142047.30302-1-0rayn.dev@gmail.com
drivers/iio/dac/ad5504.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/dac/ad5504.c b/drivers/iio/dac/ad5504.c
index 55ce7e49e0e0..87946cdf69e7 100644
--- a/drivers/iio/dac/ad5504.c
+++ b/drivers/iio/dac/ad5504.c
@@ -7,12 +7,14 @@
#include <linux/array_size.h>
#include <linux/bits.h>
+#include <linux/cleanup.h>
#include <linux/dev_printk.h>
#include <linux/errno.h>
#include <linux/interrupt.h>
#include <linux/kstrtox.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/regulator/consumer.h>
#include <linux/spi/spi.h>
#include <linux/sysfs.h>
@@ -49,10 +51,12 @@
* @pwr_down_mask: power down mask
* @pwr_down_mode: current power down mode
* @data: transfer buffer
+ * @lock: lock to protect state and spi transfers
*/
struct ad5504_state {
struct spi_device *spi;
struct regulator *reg;
+ struct mutex lock;
unsigned short vref_mv;
unsigned pwr_down_mask;
unsigned pwr_down_mode;
@@ -103,7 +107,8 @@ static int ad5504_read_raw(struct iio_dev *indio_dev,
int ret;
switch (m) {
- case IIO_CHAN_INFO_RAW:
+ case IIO_CHAN_INFO_RAW: {
+ guard(mutex)(&st->lock);
ret = ad5504_spi_read(st, chan->address);
if (ret < 0)
return ret;
@@ -111,6 +116,7 @@ static int ad5504_read_raw(struct iio_dev *indio_dev,
*val = ret;
return IIO_VAL_INT;
+ }
case IIO_CHAN_INFO_SCALE:
*val = st->vref_mv;
*val2 = chan->scan_type.realbits;
@@ -128,11 +134,13 @@ static int ad5504_write_raw(struct iio_dev *indio_dev,
struct ad5504_state *st = iio_priv(indio_dev);
switch (mask) {
- case IIO_CHAN_INFO_RAW:
+ case IIO_CHAN_INFO_RAW: {
+ guard(mutex)(&st->lock);
if (val >= (1 << chan->scan_type.realbits) || val < 0)
return -EINVAL;
return ad5504_spi_write(st, chan->address, val);
+ }
default:
return -EINVAL;
}
@@ -148,6 +156,7 @@ static int ad5504_get_powerdown_mode(struct iio_dev *indio_dev,
{
struct ad5504_state *st = iio_priv(indio_dev);
+ guard(mutex)(&st->lock);
return st->pwr_down_mode;
}
@@ -156,6 +165,7 @@ static int ad5504_set_powerdown_mode(struct iio_dev *indio_dev,
{
struct ad5504_state *st = iio_priv(indio_dev);
+ guard(mutex)(&st->lock);
st->pwr_down_mode = mode;
return 0;
@@ -173,6 +183,7 @@ static ssize_t ad5504_read_dac_powerdown(struct iio_dev *indio_dev,
{
struct ad5504_state *st = iio_priv(indio_dev);
+ guard(mutex)(&st->lock);
return sysfs_emit(buf, "%d\n",
!(st->pwr_down_mask & (1 << chan->channel)));
}
@@ -185,6 +196,7 @@ static ssize_t ad5504_write_dac_powerdown(struct iio_dev *indio_dev,
int ret;
struct ad5504_state *st = iio_priv(indio_dev);
+ guard(mutex)(&st->lock);
ret = kstrtobool(buf, &pwr_down);
if (ret)
return ret;
@@ -284,6 +296,10 @@ static int ad5504_probe(struct spi_device *spi)
st = iio_priv(indio_dev);
+ ret = devm_mutex_init(dev, &st->lock);
+ if (ret)
+ return ret;
+
ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
if (ret < 0)
return ret;
--
2.55.0
next prev parent reply other threads:[~2026-08-17 21:11 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-17 21:11 [PATCH v4 0/6] iio: dac: ad5504: bindings, cleanups, locking, and scale fixes Taha Ed-Dafili
2026-08-17 21:11 ` [PATCH v4 1/6] dt-bindings: iio: dac: ad5504: add output-range and missing gpios Taha Ed-Dafili
2026-08-17 21:11 ` [PATCH v4 2/6] iio: dac: ad5504: Align headers with IWYU principle Taha Ed-Dafili
2026-08-18 4:43 ` Andy Shevchenko
2026-08-17 21:11 ` [PATCH v4 3/6] iio: dac: ad5504: remove legacy platform data support Taha Ed-Dafili
2026-08-18 4:45 ` Andy Shevchenko
2026-08-17 21:11 ` Taha Ed-Dafili [this message]
2026-08-18 6:12 ` [PATCH v4 4/6] iio: dac: ad5504: introduce local lock to protect state and spi transfers Andy Shevchenko
2026-08-17 21:11 ` [PATCH v4 5/6] iio: dac: ad5504: strictly separate ACPI and DT probe paths Taha Ed-Dafili
2026-08-18 6:26 ` Andy Shevchenko
2026-08-17 21:11 ` [PATCH v4 6/6] iio: dac: ad5504: support scale via output-range-microvolt property Taha Ed-Dafili
2026-08-18 6:30 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260817211118.21833-5-0rayn.dev@gmail.com \
--to=0rayn.dev@gmail.com \
--cc=Michael.Hennerich@analog.com \
--cc=andy@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@analog.com \
--cc=nuno.sa@analog.com \
--cc=robh@kernel.org \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox