[PATCH] staging: iio: impedance-analyzer: ad5933: use guard(mutex) in ad5933_show()

[PATCH] staging: iio: impedance-analyzer: ad5933: use guard(mutex) in ad5933_show()

[Nithishvelan M]

Use guard(mutex) to automatically manage the state lock in
ad5933_show(). This simplifies the function by allowing direct returns
from each switch case, eliminating the mutable return variables and
manual mutex_unlock() calls.

Signed-off-by: Nithishvelan M <mnithishvelan@gmail.com>
---
 .../staging/iio/impedance-analyzer/ad5933.c   | 38 ++++++++-----------
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 798e62bae2..024c6b5a06 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -333,39 +334,30 @@ static ssize_t ad5933_show(struct device *dev,
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct ad5933_state *st = iio_priv(indio_dev);
 	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
-	int ret = 0, len = 0;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
+
 	switch ((u32)this_attr->address) {
 	case AD5933_OUT_RANGE:
-		len = sysfs_emit(buf, "%u\n",
-				 st->range_avail[(st->ctrl_hb >> 1) & 0x3]);
-		break;
+		return sysfs_emit(buf, "%u\n",
+				  st->range_avail[(st->ctrl_hb >> 1) & 0x3]);
 	case AD5933_OUT_RANGE_AVAIL:
-		len = sysfs_emit(buf, "%u %u %u %u\n", st->range_avail[0],
-				 st->range_avail[3], st->range_avail[2],
-				 st->range_avail[1]);
-		break;
+		return sysfs_emit(buf, "%u %u %u %u\n", st->range_avail[0],
+				  st->range_avail[3], st->range_avail[2],
+				  st->range_avail[1]);
 	case AD5933_OUT_SETTLING_CYCLES:
-		len = sysfs_emit(buf, "%d\n", st->settling_cycles);
-		break;
+		return sysfs_emit(buf, "%d\n", st->settling_cycles);
 	case AD5933_IN_PGA_GAIN:
-		len = sysfs_emit(buf, "%s\n",
-				 (st->ctrl_hb & AD5933_CTRL_PGA_GAIN_1) ?
-				 "1" : "0.2");
-		break;
+		return sysfs_emit(buf, "%s\n",
+				  (st->ctrl_hb & AD5933_CTRL_PGA_GAIN_1) ?
+				  "1" : "0.2");
 	case AD5933_IN_PGA_GAIN_AVAIL:
-		len = sysfs_emit(buf, "1 0.2\n");
-		break;
+		return sysfs_emit(buf, "1 0.2\n");
 	case AD5933_FREQ_POINTS:
-		len = sysfs_emit(buf, "%d\n", st->freq_points);
-		break;
+		return sysfs_emit(buf, "%d\n", st->freq_points);
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-
-	mutex_unlock(&st->lock);
-	return ret ? ret : len;
 }
 
 static ssize_t ad5933_store(struct device *dev,
-- 
2.53.0

Re: [PATCH] staging: iio: impedance-analyzer: ad5933: use guard(mutex) in ad5933_show()

[Andy Shevchenko]

On Sun, Oct 18, 2026 at 08:01:27PM +0000, Nithishvelan M wrote:
> Use guard(mutex) to automatically manage the state lock in
> ad5933_show(). This simplifies the function by allowing direct returns
> from each switch case, eliminating the mutable return variables and
> manual mutex_unlock() calls.

...

>  	switch ((u32)this_attr->address) {
>  	case AD5933_OUT_RANGE:
> -		len = sysfs_emit(buf, "%u\n",
> -				 st->range_avail[(st->ctrl_hb >> 1) & 0x3]);
> -		break;
> +		return sysfs_emit(buf, "%u\n",
> +				  st->range_avail[(st->ctrl_hb >> 1) & 0x3]);

Keep it on a single (not so long) line.

		return sysfs_emit(buf, "%u\n", st->range_avail[(st->ctrl_hb >> 1) & 0x3]);

Most likely Sashiko might warn of use ctrl_hb variable as an index without
checking. So it might make sense to be prepared for that by adding a local
variable:

		unsigned int ridx = (st->ctrl_hb >> 1) & 0x03;

		return sysfs_emit(buf, "%u\n", st->range_avail[ridx]);


>  	case AD5933_OUT_RANGE_AVAIL:
> -		len = sysfs_emit(buf, "%u %u %u %u\n", st->range_avail[0],
> -				 st->range_avail[3], st->range_avail[2],
> -				 st->range_avail[1]);
> -		break;
> +		return sysfs_emit(buf, "%u %u %u %u\n", st->range_avail[0],
> +				  st->range_avail[3], st->range_avail[2],
> +				  st->range_avail[1]);

Split logically

		return sysfs_emit(buf, "%u %u %u %u\n",
				  st->range_avail[0], st->range_avail[3],
				  st->range_avail[2], st->range_avail[1]);

>  	case AD5933_OUT_SETTLING_CYCLES:
> -		len = sysfs_emit(buf, "%d\n", st->settling_cycles);
> -		break;
> +		return sysfs_emit(buf, "%d\n", st->settling_cycles);
>  	case AD5933_IN_PGA_GAIN:
> -		len = sysfs_emit(buf, "%s\n",
> -				 (st->ctrl_hb & AD5933_CTRL_PGA_GAIN_1) ?
> -				 "1" : "0.2");
> -		break;
> +		return sysfs_emit(buf, "%s\n",

> +				  (st->ctrl_hb & AD5933_CTRL_PGA_GAIN_1) ?
> +				  "1" : "0.2");

A single line

				  (st->ctrl_hb & AD5933_CTRL_PGA_GAIN_1) ?  "1" : "0.2");

and, IIRC the operator precedences, the parentheses can be dropped.

>  	case AD5933_IN_PGA_GAIN_AVAIL:
> -		len = sysfs_emit(buf, "1 0.2\n");
> -		break;
> +		return sysfs_emit(buf, "1 0.2\n");
>  	case AD5933_FREQ_POINTS:
> -		len = sysfs_emit(buf, "%d\n", st->freq_points);
> -		break;
> +		return sysfs_emit(buf, "%d\n", st->freq_points);
>  	default:
> -		ret = -EINVAL;
> +		return -EINVAL;
>  	}

-- 
With Best Regards,
Andy Shevchenko