Re: [PATCH v2] iio: adc: mcp3911: simplify code with guard macro

From: Jonathan Cameron
Date: Mon Dec 04 2023 - 07:23:21 EST


On Mon, 27 Nov 2023 08:38:22 +0100
Marcus Folkesson <marcus.folkesson@xxxxxxxxx> wrote:

> Use the guard(mutex) macro for handle mutex lock/unlocks.
>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@xxxxxxxxx>

Oddly this doesn't apply. Also some comments inline.
> ---
> Changes in v2:
> - Return directly instead of goto label
> - Link to v1: https://lore.kernel.org/r/20231125-mcp3911-guard-v1-1-2748d16a3f3f@xxxxxxxxx
> ---
> drivers/iio/adc/mcp3911.c | 27 ++++++++++-----------------
> 1 file changed, 10 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
> index 974c5bd923a6..30836725ef9a 100644
> --- a/drivers/iio/adc/mcp3911.c
> +++ b/drivers/iio/adc/mcp3911.c
> @@ -7,6 +7,7 @@
> */
> #include <linux/bitfield.h>
> #include <linux/bits.h>
> +#include <linux/cleanup.h>
> #include <linux/clk.h>
> #include <linux/delay.h>
> #include <linux/err.h>
> @@ -168,13 +169,13 @@ static int mcp3911_read_raw(struct iio_dev *indio_dev,
> struct mcp3911 *adc = iio_priv(indio_dev);
> int ret = -EINVAL;
>
> - mutex_lock(&adc->lock);
> + guard(mutex)(&adc->lock);
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> ret = mcp3911_read(adc,
> MCP3911_CHANNEL(channel->channel), val, 3);
> if (ret)
> - goto out;
> + return ret;
>
> *val = sign_extend32(*val, 23);
This should go further. Return early in the good paths as well
return IIO_VAL_INT;
a few lines later than this.