All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Lechner <dlechner@baylibre.com>
To: linux-iio@vger.kernel.org, linux-staging@lists.linux.dev
Cc: "David Lechner" <dlechner@baylibre.com>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"Michael Hennerich" <Michael.Hennerich@analog.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Axel Haslam" <ahaslam@baylibre.com>,
	"Philip Molloy" <pmolloy@baylibre.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v4 17/17] staging: iio: resolver: ad2s1210: simplify code with guard(mutex)
Date: Thu,  5 Oct 2023 19:50:34 -0500	[thread overview]
Message-ID: <20231005-ad2s1210-mainline-v4-17-ec00746840fc@baylibre.com> (raw)
In-Reply-To: <20231005-ad2s1210-mainline-v4-0-ec00746840fc@baylibre.com>

We can simplify the code and get rid of most of the gotos by using
guard(mutex) from cleanup.h.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---

v4 changes: New patch in v4.

 drivers/staging/iio/resolver/ad2s1210.c | 157 ++++++++++----------------------
 1 file changed, 50 insertions(+), 107 deletions(-)

diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index c4e1bc22e8b0..c4e0ffa92dc2 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -47,6 +47,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -404,11 +405,13 @@ static int ad2s1210_single_conversion(struct iio_dev *indio_dev,
 	s64 timestamp;
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
+
 	gpiod_set_value(st->sample_gpio, 1);
 	timestamp = iio_get_time_ns(indio_dev);
 	/* delay (6 * tck + 20) nano seconds */
 	udelay(1);
+	gpiod_set_value(st->sample_gpio, 0);
 
 	switch (chan->type) {
 	case IIO_ANGL:
@@ -418,14 +421,13 @@ static int ad2s1210_single_conversion(struct iio_dev *indio_dev,
 		ret = ad2s1210_set_mode(st, MOD_VEL);
 		break;
 	default:
-		ret = -EINVAL;
-		break;
+		return -EINVAL;
 	}
 	if (ret < 0)
-		goto error_ret;
+		return ret;
 	ret = spi_read(st->sdev, &st->sample, 3);
 	if (ret < 0)
-		goto error_ret;
+		return ret;
 
 	switch (chan->type) {
 	case IIO_ANGL:
@@ -437,17 +439,11 @@ static int ad2s1210_single_conversion(struct iio_dev *indio_dev,
 		ret = IIO_VAL_INT;
 		break;
 	default:
-		ret = -EINVAL;
-		break;
+		return -EINVAL;
 	}
 
 	ad2s1210_push_events(indio_dev, st->sample.fault, timestamp);
 
-error_ret:
-	gpiod_set_value(st->sample_gpio, 0);
-	/* delay (2 * tck + 20) nano seconds */
-	udelay(1);
-	mutex_unlock(&st->lock);
 	return ret;
 }
 
@@ -455,11 +451,9 @@ static int ad2s1210_get_hysteresis(struct ad2s1210_state *st, int *val)
 {
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	ret = regmap_test_bits(st->regmap, AD2S1210_REG_CONTROL,
 			       AD2S1210_ENABLE_HYSTERESIS);
-	mutex_unlock(&st->lock);
-
 	if (ret < 0)
 		return ret;
 
@@ -469,15 +463,10 @@ static int ad2s1210_get_hysteresis(struct ad2s1210_state *st, int *val)
 
 static int ad2s1210_set_hysteresis(struct ad2s1210_state *st, int val)
 {
-	int ret;
-
-	mutex_lock(&st->lock);
-	ret = regmap_update_bits(st->regmap, AD2S1210_REG_CONTROL,
-				 AD2S1210_ENABLE_HYSTERESIS,
-				 val ? AD2S1210_ENABLE_HYSTERESIS : 0);
-	mutex_unlock(&st->lock);
-
-	return ret;
+	guard(mutex)(&st->lock);
+	return regmap_update_bits(st->regmap, AD2S1210_REG_CONTROL,
+				  AD2S1210_ENABLE_HYSTERESIS,
+				  val ? AD2S1210_ENABLE_HYSTERESIS : 0);
 }
 
 static int ad2s1210_get_phase_lock_range(struct ad2s1210_state *st,
@@ -485,11 +474,9 @@ static int ad2s1210_get_phase_lock_range(struct ad2s1210_state *st,
 {
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	ret = regmap_test_bits(st->regmap, AD2S1210_REG_CONTROL,
 			       AD2S1210_PHASE_LOCK_RANGE_44);
-	mutex_unlock(&st->lock);
-
 	if (ret < 0)
 		return ret;
 
@@ -509,7 +496,7 @@ static int ad2s1210_get_phase_lock_range(struct ad2s1210_state *st,
 static int ad2s1210_set_phase_lock_range(struct ad2s1210_state *st,
 					 int val, int val2)
 {
-	int deg, ret;
+	int deg;
 
 	/* convert radians to degrees - only two allowable values */
 	if (val == PHASE_44_DEG_TO_RAD_INT && val2 == PHASE_44_DEG_TO_RAD_MICRO)
@@ -520,12 +507,10 @@ static int ad2s1210_set_phase_lock_range(struct ad2s1210_state *st,
 	else
 		return -EINVAL;
 
-	mutex_lock(&st->lock);
-	ret = regmap_update_bits(st->regmap, AD2S1210_REG_CONTROL,
-				 AD2S1210_PHASE_LOCK_RANGE_44,
-				 deg == 44 ? AD2S1210_PHASE_LOCK_RANGE_44 : 0);
-	mutex_unlock(&st->lock);
-	return ret;
+	guard(mutex)(&st->lock);
+	return regmap_update_bits(st->regmap, AD2S1210_REG_CONTROL,
+				  AD2S1210_PHASE_LOCK_RANGE_44,
+				  deg == 44 ? AD2S1210_PHASE_LOCK_RANGE_44 : 0);
 }
 
 /* map resolution to microradians/LSB for LOT registers */
@@ -542,10 +527,8 @@ static int ad2s1210_get_voltage_threshold(struct ad2s1210_state *st,
 	unsigned int reg_val;
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	ret = regmap_read(st->regmap, reg, &reg_val);
-	mutex_unlock(&st->lock);
-
 	if (ret < 0)
 		return ret;
 
@@ -557,15 +540,11 @@ static int ad2s1210_set_voltage_threshold(struct ad2s1210_state *st,
 					  unsigned int reg, int val)
 {
 	unsigned int reg_val;
-	int ret;
 
 	reg_val = val / THRESHOLD_MILLIVOLT_PER_LSB;
 
-	mutex_lock(&st->lock);
-	ret = regmap_write(st->regmap, reg, reg_val);
-	mutex_unlock(&st->lock);
-
-	return ret;
+	guard(mutex)(&st->lock);
+	return regmap_write(st->regmap, reg, reg_val);
 }
 
 static int ad2s1210_get_lot_high_threshold(struct ad2s1210_state *st,
@@ -574,10 +553,8 @@ static int ad2s1210_get_lot_high_threshold(struct ad2s1210_state *st,
 	unsigned int reg_val;
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	ret = regmap_read(st->regmap, AD2S1210_REG_LOT_HIGH_THRD, &reg_val);
-	mutex_unlock(&st->lock);
-
 	if (ret < 0)
 		return ret;
 
@@ -596,18 +573,18 @@ static int ad2s1210_set_lot_high_threshold(struct ad2s1210_state *st,
 	if (val != 0)
 		return -EINVAL;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	/*
 	 * We need to read both high and low registers first so we can preserve
 	 * the hysteresis.
 	 */
 	ret = regmap_read(st->regmap, AD2S1210_REG_LOT_HIGH_THRD, &high_reg_val);
 	if (ret < 0)
-		goto error_ret;
+		return ret;
 
 	ret = regmap_read(st->regmap, AD2S1210_REG_LOT_LOW_THRD, &low_reg_val);
 	if (ret < 0)
-		goto error_ret;
+		return ret;
 
 	hysteresis = high_reg_val - low_reg_val;
 	high_reg_val = val2 / ad2s1210_lot_threshold_urad_per_lsb[st->resolution];
@@ -615,14 +592,9 @@ static int ad2s1210_set_lot_high_threshold(struct ad2s1210_state *st,
 
 	ret = regmap_write(st->regmap, AD2S1210_REG_LOT_HIGH_THRD, high_reg_val);
 	if (ret < 0)
-		goto error_ret;
-
-	ret = regmap_write(st->regmap, AD2S1210_REG_LOT_LOW_THRD, low_reg_val);
-
-error_ret:
-	mutex_unlock(&st->lock);
+		return ret;
 
-	return ret;
+	return regmap_write(st->regmap, AD2S1210_REG_LOT_LOW_THRD, low_reg_val);
 }
 
 static int ad2s1210_get_lot_low_threshold(struct ad2s1210_state *st,
@@ -631,16 +603,13 @@ static int ad2s1210_get_lot_low_threshold(struct ad2s1210_state *st,
 	unsigned int high_reg_val, low_reg_val;
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
+
 	ret = regmap_read(st->regmap, AD2S1210_REG_LOT_HIGH_THRD, &high_reg_val);
 	if (ret < 0)
-		goto error_ret;
+		return ret;
 
 	ret = regmap_read(st->regmap, AD2S1210_REG_LOT_LOW_THRD, &low_reg_val);
-
-error_ret:
-	mutex_unlock(&st->lock);
-
 	if (ret < 0)
 		return ret;
 
@@ -663,18 +632,14 @@ static int ad2s1210_set_lot_low_threshold(struct ad2s1210_state *st,
 
 	hysteresis = val2 / ad2s1210_lot_threshold_urad_per_lsb[st->resolution];
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
+
 	ret = regmap_read(st->regmap, AD2S1210_REG_LOT_HIGH_THRD, &reg_val);
 	if (ret < 0)
-		goto error_ret;
+		return ret;
 
-	ret = regmap_write(st->regmap, AD2S1210_REG_LOT_LOW_THRD,
+	return regmap_write(st->regmap, AD2S1210_REG_LOT_LOW_THRD,
 			   reg_val - hysteresis);
-
-error_ret:
-	mutex_unlock(&st->lock);
-
-	return ret;
 }
 
 static int ad2s1210_get_excitation_frequency(struct ad2s1210_state *st, int *val)
@@ -682,31 +647,23 @@ static int ad2s1210_get_excitation_frequency(struct ad2s1210_state *st, int *val
 	unsigned int reg_val;
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
+
 	ret = regmap_read(st->regmap, AD2S1210_REG_EXCIT_FREQ, &reg_val);
 	if (ret < 0)
-		goto error_ret;
+		return ret;
 
 	*val = reg_val * st->clkin_hz / (1 << 15);
-	ret = IIO_VAL_INT;
-
-error_ret:
-	mutex_unlock(&st->lock);
-	return ret;
+	return IIO_VAL_INT;
 }
 
 static int ad2s1210_set_excitation_frequency(struct ad2s1210_state *st, int val)
 {
-	int ret;
-
 	if (val < AD2S1210_MIN_EXCIT || val > AD2S1210_MAX_EXCIT)
 		return -EINVAL;
 
-	mutex_lock(&st->lock);
-	ret = ad2s1210_reinit_excitation_frequency(st, val);
-	mutex_unlock(&st->lock);
-
-	return ret;
+	guard(mutex)(&st->lock);
+	return ad2s1210_reinit_excitation_frequency(st, val);
 }
 
 static const int ad2s1210_velocity_scale[] = {
@@ -982,10 +939,8 @@ static ssize_t event_attr_voltage_reg_show(struct device *dev,
 	unsigned int value;
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	ret = regmap_read(st->regmap, iattr->address, &value);
-	mutex_unlock(&st->lock);
-
 	if (ret < 0)
 		return ret;
 
@@ -1005,11 +960,9 @@ static ssize_t event_attr_voltage_reg_store(struct device *dev,
 	if (ret)
 		return -EINVAL;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 	ret = regmap_write(st->regmap, iattr->address,
 			   data / THRESHOLD_MILLIVOLT_PER_LSB);
-	mutex_unlock(&st->lock);
-
 	if (ret < 0)
 		return ret;
 
@@ -1083,7 +1036,7 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
 	unsigned int data;
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	/* Use default config register value plus resolution from devicetree. */
 	data = FIELD_PREP(AD2S1210_PHASE_LOCK_RANGE_44, 1);
@@ -1093,13 +1046,9 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
 
 	ret = regmap_write(st->regmap, AD2S1210_REG_CONTROL, data);
 	if (ret < 0)
-		goto error_ret;
-
-	ret = ad2s1210_reinit_excitation_frequency(st, AD2S1210_DEF_EXCIT);
+		return ret;
 
-error_ret:
-	mutex_unlock(&st->lock);
-	return ret;
+	return ad2s1210_reinit_excitation_frequency(st, AD2S1210_DEF_EXCIT);
 }
 
 static int ad2s1210_read_label(struct iio_dev *indio_dev,
@@ -1243,18 +1192,13 @@ static int ad2s1210_debugfs_reg_access(struct iio_dev *indio_dev,
 				       unsigned int *readval)
 {
 	struct ad2s1210_state *st = iio_priv(indio_dev);
-	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	if (readval)
-		ret = regmap_read(st->regmap, reg, readval);
-	else
-		ret = regmap_write(st->regmap, reg, writeval);
-
-	mutex_unlock(&st->lock);
+		return regmap_read(st->regmap, reg, readval);
 
-	return ret;
+	return regmap_write(st->regmap, reg, writeval);
 }
 
 static irqreturn_t ad2s1210_trigger_handler(int irq, void *p)
@@ -1265,7 +1209,7 @@ static irqreturn_t ad2s1210_trigger_handler(int irq, void *p)
 	size_t chan = 0;
 	int ret;
 
-	mutex_lock(&st->lock);
+	guard(mutex)(&st->lock);
 
 	memset(&st->scan, 0, sizeof(st->scan));
 	gpiod_set_value(st->sample_gpio, 1);
@@ -1299,7 +1243,6 @@ static irqreturn_t ad2s1210_trigger_handler(int irq, void *p)
 
 error_ret:
 	gpiod_set_value(st->sample_gpio, 0);
-	mutex_unlock(&st->lock);
 	iio_trigger_notify_done(indio_dev->trig);
 
 	return IRQ_HANDLED;

-- 
2.42.0


  parent reply	other threads:[~2023-10-06  0:52 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-06  0:50 [PATCH v4 00/17] iio: resolver: move ad2s1210 out of staging David Lechner
2023-10-06  0:50 ` [PATCH v4 01/17] staging: iio: resolver: ad2s1210: do not use fault register for dummy read David Lechner
2023-10-10 15:38   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 02/17] staging: iio: resolver: ad2s1210: implement hysteresis as channel attr David Lechner
2023-10-10 15:40   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 03/17] staging: iio: resolver: ad2s1210: convert fexcit to channel attribute David Lechner
2023-10-10 15:41   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 04/17] staging: iio: resolver: ad2s1210: convert resolution to devicetree property David Lechner
2023-10-10 15:43   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 05/17] staging: iio: resolver: ad2s1210: add phase lock range support David Lechner
2023-10-10 15:46   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 06/17] staging: iio: resolver: ad2s1210: add triggered buffer support David Lechner
2023-10-10 15:47   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 07/17] staging: iio: resolver: ad2s1210: convert LOT threshold attrs to event attrs David Lechner
2023-10-10 15:54   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 08/17] staging: iio: resolver: ad2s1210: convert LOS threshold to event attr David Lechner
2023-10-10 15:52   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 09/17] staging: iio: resolver: ad2s1210: convert DOS overrange " David Lechner
2023-10-10 15:55   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 10/17] staging: iio: resolver: ad2s1210: convert DOS mismatch " David Lechner
2023-10-10 15:56   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 11/17] staging: iio: resolver: ad2s1210: rename DOS reset min/max attrs David Lechner
2023-10-10 15:57   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 12/17] iio: event: add optional event label support David Lechner
2023-10-10 15:59   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 13/17] staging: iio: resolver: ad2s1210: implement fault events David Lechner
2023-10-10 16:05   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 14/17] staging: iio: resolver: ad2s1210: add register/fault support summary David Lechner
2023-10-10 16:07   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 15/17] staging: iio: resolver: ad2s1210: add label attribute support David Lechner
2023-10-10 16:08   ` Jonathan Cameron
2023-10-06  0:50 ` [PATCH v4 16/17] staging: iio: resolver: ad2s1210: remove fault attribute David Lechner
2023-10-10 16:09   ` Jonathan Cameron
2023-10-06  0:50 ` David Lechner [this message]
2023-10-10 16:17   ` [PATCH v4 17/17] staging: iio: resolver: ad2s1210: simplify code with guard(mutex) Jonathan Cameron
2023-10-10 17:40     ` David Lechner
2023-10-10 17:46       ` Jonathan Cameron

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=20231005-ad2s1210-mainline-v4-17-ec00746840fc@baylibre.com \
    --to=dlechner@baylibre.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=ahaslam@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=nuno.sa@analog.com \
    --cc=pmolloy@baylibre.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.