Use guard(mutex) for cleaner lock handling and simpler error paths.

Signed-off-by: Tomas Borquez <tomasborquez13@gmail.com>
---
 drivers/staging/iio/frequency/ad9832.c | 41 +++++++++++---------------
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c b/drivers/staging/iio/frequency/ad9832.c
index a5523e2210bf..35d8d51d5c2b 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -9,6 +9,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -179,20 +180,20 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 
 	ret = kstrtoul(buf, 10, &val);
 	if (ret)
-		goto error_ret;
+		return ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	switch ((u32)this_attr->address) {
 	case AD9832_FREQ0HM:
 	case AD9832_FREQ1HM:
 		ret = ad9832_write_frequency(st, this_attr->address, val);
-		break;
+		return ret ?: len;
 	case AD9832_PHASE0H:
 	case AD9832_PHASE1H:
 	case AD9832_PHASE2H:
 	case AD9832_PHASE3H:
 		ret = ad9832_write_phase(st, this_attr->address, val);
-		break;
+		return ret ?: len;
 	case AD9832_PINCTRL_EN:
 		st->ctrl_ss &= ~AD9832_SELSRC;
 		st->ctrl_ss |= FIELD_PREP(AD9832_SELSRC, val ? 0 : 1);
@@ -200,24 +201,20 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_SYNCSELSRC) |
 						  st->ctrl_ss);
 		ret = spi_sync(st->spi, &st->msg);
-		break;
+		return ret ?: len;
 	case AD9832_FREQ_SYM:
-		if (val == 1 || val == 0) {
-			st->ctrl_fp &= ~AD9832_FREQ;
-			st->ctrl_fp |= FIELD_PREP(AD9832_FREQ, val ? 1 : 0);
-		} else {
-			ret = -EINVAL;
-			break;
-		}
+		if (val != 1 && val != 0)
+			return -EINVAL;
+
+		st->ctrl_fp &= ~AD9832_FREQ;
+		st->ctrl_fp |= FIELD_PREP(AD9832_FREQ, val ? 1 : 0);
 		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_FPSELECT) |
 						  st->ctrl_fp);
 		ret = spi_sync(st->spi, &st->msg);
-		break;
+		return ret ?: len;
 	case AD9832_PHASE_SYM:
-		if (val > 3) {
-			ret = -EINVAL;
-			break;
-		}
+		if (val > 3)
+			return -EINVAL;
 
 		st->ctrl_fp &= ~AD9832_PHASE_MASK;
 		st->ctrl_fp |= FIELD_PREP(AD9832_PHASE_MASK, val);
@@ -225,7 +222,7 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_FPSELECT) |
 						  st->ctrl_fp);
 		ret = spi_sync(st->spi, &st->msg);
-		break;
+		return ret ?: len;
 	case AD9832_OUTPUT_EN:
 		if (val)
 			st->ctrl_src &= ~(AD9832_RESET | AD9832_SLEEP | AD9832_CLR);
@@ -235,14 +232,10 @@ static ssize_t ad9832_write(struct device *dev, struct device_attribute *attr,
 		st->data = cpu_to_be16(FIELD_PREP(AD9832_CMD_MSK, AD9832_CMD_SLEEPRESCLR) |
 						  st->ctrl_src);
 		ret = spi_sync(st->spi, &st->msg);
-		break;
+		return ret ?: len;
 	default:
-		ret = -ENODEV;
+		return -ENODEV;
 	}
-	mutex_unlock(&st->lock);
-
-error_ret:
-	return ret ? ret : len;
 }
 
 /*
-- 
2.43.0

On Tue, Dec 30, 2025 at 10:35 PM Tomas Borquez <tomasborquez13@gmail.com> wrote:
>
> Use guard(mutex) for cleaner lock handling and simpler error paths.

...

Don't remember if it was in the previous rounds of review or somewhere
else, but Jonathan (? IIRC) suggestd to use

  ret = foo(...);
  if (ret)
   return ret;
  break;
  ...
  return len;

However, this one just duplicates what is already in use. Have you
checked the bloat-o-meter before and after and see if there is any
difference in the compiled object file?

-- 
With Best Regards,
Andy Shevchenko

On Wed, Dec 31, 2025 at 12:50:54AM +0200, Andy Shevchenko wrote:
> On Tue, Dec 30, 2025 at 10:35 PM Tomas Borquez <tomasborquez13@gmail.com> wrote:
> >
> > Use guard(mutex) for cleaner lock handling and simpler error paths.
> 
> ...
> 
> Don't remember if it was in the previous rounds of review or somewhere
> else, but Jonathan (? IIRC) suggestd to use
> 
>   ret = foo(...);
>   if (ret)
>    return ret;
>   break;
>   ...
>   return len;

You are right, I should have sticked to that one since anyways I ended
up using that pattern on patch 5.

> However, this one just duplicates what is already in use. Have you
> checked the bloat-o-meter before and after and see if there is any
> difference in the compiled object file?

Jonathan's is better by 7 delta, so I'll stick with his

> -- 
> With Best Regards,
> Andy Shevchenko