* [PATCH v3] staging: iio: frequency: ad9834: use guard(mutex)
@ 2026-10-19 14:15 Amir Atef via B4 Relay
2026-10-19 19:03 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Amir Atef via B4 Relay @ 2026-10-19 14:15 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Michael Hennerich, David Lechner, Nuno Sa,
Andy Shevchenko, Greg Kroah-Hartman, linux-iio, linux-staging,
linux-kernel
From: Amir Atef <amiratefabdallah1122@gmail.com>
Use guard(mutex) to manage the state lock in ad9834_write() and
ad9834_store_wavetype(). This simplifies error handling by allowing
early returns instead of setting the return code and breaking out of the
switch statement to manually unlock the mutex.
Signed-off-by: Amir Atef <amiratefabdallah1122@gmail.com>
---
Changes in v3:
- Handle errors immediately at the point where they originate in
ad9834_write() using 'if (ret) return ret;' followed by 'break;', and
replace 'return ret ? ret : len;' with 'return len;'.
- Link to v2: https://lore.kernel.org/r/179219435216.322.13409717511306957730.20261017-ad9834-guard-mutex-v2@gmail.com
Changes in v2:
- Add blank line after guard(mutex) in ad9834_write().
- Move 'int ret;' to the end of the variable declaration block in
ad9834_store_wavetype().
- Drop redundant 'else' in ad9834_store_wavetype() case 1 by inverting
the condition (kept 'else' in ad9834_write() and case 0 where they are
part of if-else-if-else chains).
- Replace ternary return with explicit 'if (ret) return ret;' in
ad9834_store_wavetype().
- Link to v1: https://lore.kernel.org/r/179214943270.333.5921549521929682753.20261016-ad9834-guard-mutex@gmail.com
drivers/staging/iio/frequency/ad9834.c | 70 +++++++++++++++-----------
1 file changed, 41 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index 0d20f796e0f0..8686a22f3fc1 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -6,6 +6,7 @@
*/
#include <linux/bits.h>
+#include <linux/cleanup.h>
#include <linux/clk.h>
#include <linux/dev_printk.h>
#include <linux/err.h>
@@ -154,21 +155,26 @@ static ssize_t ad9834_write(struct device *dev,
if (ret)
return ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
+
switch ((u32)this_attr->address) {
case AD9834_REG_FREQ0:
case AD9834_REG_FREQ1:
ret = ad9834_write_frequency(st, this_attr->address, val);
+ if (ret)
+ return ret;
+
break;
case AD9834_REG_PHASE0:
case AD9834_REG_PHASE1:
ret = ad9834_write_phase(st, this_attr->address, val);
+ if (ret)
+ return ret;
+
break;
case AD9834_OPBITEN:
- if (st->control & AD9834_MODE) {
- ret = -EINVAL; /* AD9834 reserved mode */
- break;
- }
+ if (st->control & AD9834_MODE)
+ return -EINVAL; /* AD9834 reserved mode */
if (val)
st->control |= AD9834_OPBITEN;
@@ -177,6 +183,9 @@ static ssize_t ad9834_write(struct device *dev,
st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
ret = spi_sync(st->spi, &st->msg);
+ if (ret)
+ return ret;
+
break;
case AD9834_PIN_SW:
if (val)
@@ -185,6 +194,9 @@ static ssize_t ad9834_write(struct device *dev,
st->control &= ~AD9834_PIN_SW;
st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
ret = spi_sync(st->spi, &st->msg);
+ if (ret)
+ return ret;
+
break;
case AD9834_FSEL:
case AD9834_PSEL:
@@ -194,11 +206,13 @@ static ssize_t ad9834_write(struct device *dev,
st->control |= this_attr->address;
st->control &= ~AD9834_PIN_SW;
} else {
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
ret = spi_sync(st->spi, &st->msg);
+ if (ret)
+ return ret;
+
break;
case AD9834_RESET:
if (val)
@@ -208,13 +222,15 @@ static ssize_t ad9834_write(struct device *dev,
st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
ret = spi_sync(st->spi, &st->msg);
+ if (ret)
+ return ret;
+
break;
default:
- ret = -ENODEV;
+ return -ENODEV;
}
- mutex_unlock(&st->lock);
- return ret ? ret : len;
+ return len;
}
static ssize_t ad9834_store_wavetype(struct device *dev,
@@ -225,10 +241,10 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct ad9834_state *st = iio_priv(indio_dev);
struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
- int ret = 0;
bool is_ad9833_7 = (st->devid == ID_AD9833) || (st->devid == ID_AD9837);
+ int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
switch ((u32)this_attr->address) {
case 0:
@@ -241,7 +257,7 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
st->control &= ~AD9834_OPBITEN;
st->control |= AD9834_MODE;
} else if (st->control & AD9834_OPBITEN) {
- ret = -EINVAL; /* AD9834 reserved mode */
+ return -EINVAL; /* AD9834 reserved mode */
} else {
st->control |= AD9834_MODE;
}
@@ -249,31 +265,27 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
st->control &= ~AD9834_MODE;
st->control |= AD9834_OPBITEN;
} else {
- ret = -EINVAL;
+ return -EINVAL;
}
break;
case 1:
- if (sysfs_streq(buf, "square") &&
- !(st->control & AD9834_MODE)) {
- st->control &= ~AD9834_MODE;
- st->control |= AD9834_OPBITEN;
- } else {
- ret = -EINVAL;
- }
+ if (!sysfs_streq(buf, "square") || (st->control & AD9834_MODE))
+ return -EINVAL;
+
+ st->control &= ~AD9834_MODE;
+ st->control |= AD9834_OPBITEN;
break;
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
- if (!ret) {
- st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
- ret = spi_sync(st->spi, &st->msg);
- }
- mutex_unlock(&st->lock);
+ st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
+ ret = spi_sync(st->spi, &st->msg);
+ if (ret)
+ return ret;
- return ret ? ret : len;
+ return len;
}
static
--
2.51.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] staging: iio: frequency: ad9834: use guard(mutex)
2026-10-19 14:15 [PATCH v3] staging: iio: frequency: ad9834: use guard(mutex) Amir Atef via B4 Relay
@ 2026-10-19 19:03 ` Andy Shevchenko
0 siblings, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-10-19 19:03 UTC (permalink / raw)
To: amiratefabdallah1122
Cc: Jonathan Cameron, Lars-Peter Clausen, Michael Hennerich,
David Lechner, Nuno Sa, Andy Shevchenko, Greg Kroah-Hartman,
linux-iio, linux-staging, linux-kernel
On Mon, Oct 19, 2026 at 05:15:45PM +0300, Amir Atef via B4 Relay wrote:
> Use guard(mutex) to manage the state lock in ad9834_write() and
> ad9834_store_wavetype(). This simplifies error handling by allowing
> early returns instead of setting the return code and breaking out of the
> switch statement to manually unlock the mutex.
If you want to continue improving the code, you can research the use of
sysfs_match_string() and rather use it as an outer check.
static const char * const wavetypes[] = { "sine", "triangle", "square" };
int wavetype;
...
wavetype = sysfs_match_string(wavetypes);
if (wavetype < 0)
return wavetype;
but it might be actually worse than existing. Again, it needs a research to be
conducted.
As for this change, it's good enough.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v3] staging: iio: frequency: ad9834: use guard(mutex)
@ 2026-10-17 21:12 Amir Atef via B4 Relay
0 siblings, 0 replies; 3+ messages in thread
From: Amir Atef via B4 Relay @ 2026-10-17 21:12 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Michael Hennerich, David Lechner, Nuno Sa,
Andy Shevchenko, Greg Kroah-Hartman, linux-iio, linux-staging,
linux-kernel
From: Amir Atef <amiratefabdallah1122@gmail.com>
Use guard(mutex) to manage the state lock in ad9834_write() and
ad9834_store_wavetype(). This simplifies error handling by allowing
early returns instead of setting the return code and breaking out of the
switch statement to manually unlock the mutex.
Signed-off-by: Amir Atef <amiratefabdallah1122@gmail.com>
---
Changes in v3:
- Check 'ret' and return early at each call site in ad9834_write(), and
replace the final ternary return with 'return len;'.
- Link to v2: https://lore.kernel.org/r/179219435216.322.13409717511306957730.20261017-ad9834-guard-mutex-v2@gmail.com
Changes in v2:
- Add blank line after guard(mutex) in ad9834_write().
- Move 'int ret;' to the end of the variable declaration block in
ad9834_store_wavetype().
- Drop redundant 'else' in ad9834_store_wavetype() case 1 by inverting
the condition (kept 'else' in ad9834_write() and case 0 where they are
part of if-else-if-else chains).
- Replace ternary return with explicit 'if (ret) return ret;' in
ad9834_store_wavetype().
- Link to v1: https://lore.kernel.org/r/179214943270.333.5921549521929682753.20261016-ad9834-guard-mutex@gmail.com
drivers/staging/iio/frequency/ad9834.c | 70 +++++++++++++++-----------
1 file changed, 41 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index 0d20f796e0f0..8686a22f3fc1 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -6,6 +6,7 @@
*/
#include <linux/bits.h>
+#include <linux/cleanup.h>
#include <linux/clk.h>
#include <linux/dev_printk.h>
#include <linux/err.h>
@@ -154,21 +155,26 @@ static ssize_t ad9834_write(struct device *dev,
if (ret)
return ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
+
switch ((u32)this_attr->address) {
case AD9834_REG_FREQ0:
case AD9834_REG_FREQ1:
ret = ad9834_write_frequency(st, this_attr->address, val);
+ if (ret)
+ return ret;
+
break;
case AD9834_REG_PHASE0:
case AD9834_REG_PHASE1:
ret = ad9834_write_phase(st, this_attr->address, val);
+ if (ret)
+ return ret;
+
break;
case AD9834_OPBITEN:
- if (st->control & AD9834_MODE) {
- ret = -EINVAL; /* AD9834 reserved mode */
- break;
- }
+ if (st->control & AD9834_MODE)
+ return -EINVAL; /* AD9834 reserved mode */
if (val)
st->control |= AD9834_OPBITEN;
@@ -177,6 +183,9 @@ static ssize_t ad9834_write(struct device *dev,
st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
ret = spi_sync(st->spi, &st->msg);
+ if (ret)
+ return ret;
+
break;
case AD9834_PIN_SW:
if (val)
@@ -185,6 +194,9 @@ static ssize_t ad9834_write(struct device *dev,
st->control &= ~AD9834_PIN_SW;
st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
ret = spi_sync(st->spi, &st->msg);
+ if (ret)
+ return ret;
+
break;
case AD9834_FSEL:
case AD9834_PSEL:
@@ -194,11 +206,13 @@ static ssize_t ad9834_write(struct device *dev,
st->control |= this_attr->address;
st->control &= ~AD9834_PIN_SW;
} else {
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
ret = spi_sync(st->spi, &st->msg);
+ if (ret)
+ return ret;
+
break;
case AD9834_RESET:
if (val)
@@ -208,13 +222,15 @@ static ssize_t ad9834_write(struct device *dev,
st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
ret = spi_sync(st->spi, &st->msg);
+ if (ret)
+ return ret;
+
break;
default:
- ret = -ENODEV;
+ return -ENODEV;
}
- mutex_unlock(&st->lock);
- return ret ? ret : len;
+ return len;
}
static ssize_t ad9834_store_wavetype(struct device *dev,
@@ -225,10 +241,10 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct ad9834_state *st = iio_priv(indio_dev);
struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
- int ret = 0;
bool is_ad9833_7 = (st->devid == ID_AD9833) || (st->devid == ID_AD9837);
+ int ret;
- mutex_lock(&st->lock);
+ guard(mutex)(&st->lock);
switch ((u32)this_attr->address) {
case 0:
@@ -241,7 +257,7 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
st->control &= ~AD9834_OPBITEN;
st->control |= AD9834_MODE;
} else if (st->control & AD9834_OPBITEN) {
- ret = -EINVAL; /* AD9834 reserved mode */
+ return -EINVAL; /* AD9834 reserved mode */
} else {
st->control |= AD9834_MODE;
}
@@ -249,31 +265,27 @@ static ssize_t ad9834_store_wavetype(struct device *dev,
st->control &= ~AD9834_MODE;
st->control |= AD9834_OPBITEN;
} else {
- ret = -EINVAL;
+ return -EINVAL;
}
break;
case 1:
- if (sysfs_streq(buf, "square") &&
- !(st->control & AD9834_MODE)) {
- st->control &= ~AD9834_MODE;
- st->control |= AD9834_OPBITEN;
- } else {
- ret = -EINVAL;
- }
+ if (!sysfs_streq(buf, "square") || (st->control & AD9834_MODE))
+ return -EINVAL;
+
+ st->control &= ~AD9834_MODE;
+ st->control |= AD9834_OPBITEN;
break;
default:
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
- if (!ret) {
- st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
- ret = spi_sync(st->spi, &st->msg);
- }
- mutex_unlock(&st->lock);
+ st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
+ ret = spi_sync(st->spi, &st->msg);
+ if (ret)
+ return ret;
- return ret ? ret : len;
+ return len;
}
static
--
2.51.0
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-19 19:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-19 14:15 [PATCH v3] staging: iio: frequency: ad9834: use guard(mutex) Amir Atef via B4 Relay
2026-10-19 19:03 ` Andy Shevchenko
-- strict thread matches above, loose matches on Subject: below --
2026-10-17 21:12 Amir Atef via B4 Relay
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®