All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio/adc/ad7291: Improve unlocking of a mutex in ad7291_read_raw()
@ 2018-03-13 19:16 ` SF Markus Elfring
  0 siblings, 0 replies; 3+ messages in thread
From: SF Markus Elfring @ 2018-03-13 19:16 UTC (permalink / raw)
  To: linux-iio, Hartmut Knaack, Jonathan Cameron, Lars-Peter Clausen,
	Michael Hennerich, Peter Meerwald-Stadler
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 13 Mar 2018 20:08:40 +0100

* Add a jump target so that a call of the function "mutex_unlock" is stored
  only once in this function implementation.

* Replace three calls by goto statements.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/iio/adc/ad7291.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/adc/ad7291.c b/drivers/iio/adc/ad7291.c
index a862b5d8fb4b..b2c3d4c79909 100644
--- a/drivers/iio/adc/ad7291.c
+++ b/drivers/iio/adc/ad7291.c
@@ -335,27 +335,27 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
 			mutex_lock(&chip->state_lock);
 			/* If in autocycle mode drop through */
 			if (chip->command & AD7291_AUTOCYCLE) {
-				mutex_unlock(&chip->state_lock);
-				return -EBUSY;
+				ret = -EBUSY;
+				goto unlock;
 			}
 			/* Enable this channel alone */
 			regval = chip->command & (~AD7291_VOLTAGE_MASK);
 			regval |= BIT(15 - chan->channel);
 			ret = ad7291_i2c_write(chip, AD7291_COMMAND, regval);
-			if (ret < 0) {
-				mutex_unlock(&chip->state_lock);
-				return ret;
-			}
+			if (ret < 0)
+				goto unlock;
+
 			/* Read voltage */
 			ret = i2c_smbus_read_word_swapped(chip->client,
 							  AD7291_VOLTAGE);
-			if (ret < 0) {
-				mutex_unlock(&chip->state_lock);
-				return ret;
-			}
+			if (ret < 0)
+				goto unlock;
+
 			*val = ret & AD7291_VALUE_MASK;
+			ret = IIO_VAL_INT;
+unlock:
 			mutex_unlock(&chip->state_lock);
-			return IIO_VAL_INT;
+			return ret;
 		case IIO_TEMP:
 			/* Assumes tsense bit of command register always set */
 			ret = i2c_smbus_read_word_swapped(chip->client,
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* [PATCH] iio/adc/ad7291: Improve unlocking of a mutex in ad7291_read_raw()
@ 2018-03-13 19:16 ` SF Markus Elfring
  0 siblings, 0 replies; 3+ messages in thread
From: SF Markus Elfring @ 2018-03-13 19:16 UTC (permalink / raw)
  To: linux-iio, Hartmut Knaack, Jonathan Cameron, Lars-Peter Clausen,
	Michael Hennerich, Peter Meerwald-Stadler
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 13 Mar 2018 20:08:40 +0100

* Add a jump target so that a call of the function "mutex_unlock" is stored
  only once in this function implementation.

* Replace three calls by goto statements.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/iio/adc/ad7291.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/adc/ad7291.c b/drivers/iio/adc/ad7291.c
index a862b5d8fb4b..b2c3d4c79909 100644
--- a/drivers/iio/adc/ad7291.c
+++ b/drivers/iio/adc/ad7291.c
@@ -335,27 +335,27 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
 			mutex_lock(&chip->state_lock);
 			/* If in autocycle mode drop through */
 			if (chip->command & AD7291_AUTOCYCLE) {
-				mutex_unlock(&chip->state_lock);
-				return -EBUSY;
+				ret = -EBUSY;
+				goto unlock;
 			}
 			/* Enable this channel alone */
 			regval = chip->command & (~AD7291_VOLTAGE_MASK);
 			regval |= BIT(15 - chan->channel);
 			ret = ad7291_i2c_write(chip, AD7291_COMMAND, regval);
-			if (ret < 0) {
-				mutex_unlock(&chip->state_lock);
-				return ret;
-			}
+			if (ret < 0)
+				goto unlock;
+
 			/* Read voltage */
 			ret = i2c_smbus_read_word_swapped(chip->client,
 							  AD7291_VOLTAGE);
-			if (ret < 0) {
-				mutex_unlock(&chip->state_lock);
-				return ret;
-			}
+			if (ret < 0)
+				goto unlock;
+
 			*val = ret & AD7291_VALUE_MASK;
+			ret = IIO_VAL_INT;
+unlock:
 			mutex_unlock(&chip->state_lock);
-			return IIO_VAL_INT;
+			return ret;
 		case IIO_TEMP:
 			/* Assumes tsense bit of command register always set */
 			ret = i2c_smbus_read_word_swapped(chip->client,
-- 
2.16.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] iio/adc/ad7291: Improve unlocking of a mutex in ad7291_read_raw()
  2018-03-13 19:16 ` SF Markus Elfring
  (?)
@ 2018-03-18 10:18 ` Jonathan Cameron
  -1 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2018-03-18 10:18 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-iio, Hartmut Knaack, Lars-Peter Clausen, Michael Hennerich,
	Peter Meerwald-Stadler, LKML, kernel-janitors

On Tue, 13 Mar 2018 20:16:27 +0100
SF Markus Elfring <elfring@users.sourceforge.net> wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 13 Mar 2018 20:08:40 +0100
> 
> * Add a jump target so that a call of the function "mutex_unlock" is stored
>   only once in this function implementation.
> 
> * Replace three calls by goto statements.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/iio/adc/ad7291.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7291.c b/drivers/iio/adc/ad7291.c
> index a862b5d8fb4b..b2c3d4c79909 100644
> --- a/drivers/iio/adc/ad7291.c
> +++ b/drivers/iio/adc/ad7291.c
> @@ -335,27 +335,27 @@ static int ad7291_read_raw(struct iio_dev *indio_dev,
>  			mutex_lock(&chip->state_lock);
>  			/* If in autocycle mode drop through */
>  			if (chip->command & AD7291_AUTOCYCLE) {
> -				mutex_unlock(&chip->state_lock);
> -				return -EBUSY;
> +				ret = -EBUSY;
> +				goto unlock;
>  			}
>  			/* Enable this channel alone */
>  			regval = chip->command & (~AD7291_VOLTAGE_MASK);
>  			regval |= BIT(15 - chan->channel);
>  			ret = ad7291_i2c_write(chip, AD7291_COMMAND, regval);
> -			if (ret < 0) {
> -				mutex_unlock(&chip->state_lock);
> -				return ret;
> -			}
> +			if (ret < 0)
> +				goto unlock;
> +
>  			/* Read voltage */
>  			ret = i2c_smbus_read_word_swapped(chip->client,
>  							  AD7291_VOLTAGE);
> -			if (ret < 0) {
> -				mutex_unlock(&chip->state_lock);
> -				return ret;
> -			}
> +			if (ret < 0)
> +				goto unlock;
> +
>  			*val = ret & AD7291_VALUE_MASK;
> +			ret = IIO_VAL_INT;
> +unlock:
>  			mutex_unlock(&chip->state_lock);
> -			return IIO_VAL_INT;
> +			return ret;
This localized error handling makes for complex code flow.  It does however
suggest that perhaps this block of code would be better in it's own function
with nice clean internal error handling.

>  		case IIO_TEMP:
>  			/* Assumes tsense bit of command register always set */
>  			ret = i2c_smbus_read_word_swapped(chip->client,

Markus, please throttle these patches to one at a time.  If you send lots
of similar patches before anyone has reviewed any of them it wastes both
your and reviewers time.

Thanks,

Jonathan

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-18 10:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-13 19:16 [PATCH] iio/adc/ad7291: Improve unlocking of a mutex in ad7291_read_raw() SF Markus Elfring
2018-03-13 19:16 ` SF Markus Elfring
2018-03-18 10:18 ` Jonathan Cameron

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.