Received-SPF: pass (google.com: domain of linux-kernel+bounces-112646-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Sun, 24 Mar 2024 11:20:02 +0000
From: Jonathan Cameron <jic23@kernel.org>
To: Vasileios Amoiridis <vassilisamir@gmail.com>
Cc: lars@metafoo.de, andriy.shevchenko@linux.intel.com,
 ang.iglesiasg@gmail.com, mazziesaccount@gmail.com, ak@it-klinger.de,
 petre.rodan@subdimension.ro, phil@raspberrypi.com, 579lpy@gmail.com,
 linus.walleij@linaro.org, semen.protsenko@linaro.org,
 linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, Jonathan Cameron
 <Jonathan.Cameron@huawei.com>
Subject: Re: [PATCH v3 2/6] iio: pressure: Introduce new cleanup routines to
 BMP280 driver *_raw() functions
Message-ID: <20240324112002.3173a269@jic23-huawei>
In-Reply-To: <20240319002925.2121016-3-vassilisamir@gmail.com>
References: <20240319002925.2121016-1-vassilisamir@gmail.com>
	<20240319002925.2121016-3-vassilisamir@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Tue, 19 Mar 2024 01:29:21 +0100
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Introduce the new linux/cleanup.h with the guard(mutex) functionality
> in the {read/write}_raw() functions
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  drivers/iio/pressure/bmp280-core.c | 125 +++++++++++++----------------
>  1 file changed, 58 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 871b2214121b..f7a13ff6f26c 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -460,77 +460,74 @@ static int bmp280_read_humid(struct bmp280_data *data, int *val, int *val2)
>  	return IIO_VAL_INT;
>  }
>  
> -static int bmp280_read_raw(struct iio_dev *indio_dev,
> -			   struct iio_chan_spec const *chan,
> -			   int *val, int *val2, long mask)
> +static int bmp280_read_raw_guarded(struct iio_dev *indio_dev,

Why do we need the guarded naming?  It always took the lock, no
change just because we are doing that in a neater fashion.

I don't see a reason for that name.  Better to use something like
_impl, or _internal as the prefix.

I don't want to see people calling every function that uses guard
_guarded().


> +				   struct iio_chan_spec const *chan,
> +				   int *val, int *val2, long mask)
Otherwise, I didn't find anything beyond what Andy already pointed out.
So looks good in general.

Jonathan