Re: [PATCH v2 06/12] iio: st_sensors: Stop abusing mlock to ensure internal coherency

[Jonathan Cameron <jic23@kernel.org>]

On Wed,  2 Feb 2022 15:02:02 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> An odr_lock has been introduced to protect local accesses to the odr
> internal cache and ensure the cached value always reflected the actual
> value. Using the mlock() for this purpose is no longer needed, so let's
> drop these extra mutex_lock/unlock() calls.
> 
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Cc: Denis Ciocca <denis.ciocca@st.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Obviously a different issue but all the write_raw() functions should
use the pattern used in st_pressure_core.c and do early
returns seeing as there doesn't seem to be any cleanup to do.

We can tidy that up in a follow up patch as I'm sure there are other
areas in these drivers where direct returns would be nicer than
what is there currently!

Otherwise looks good to me.

> ---
>  drivers/iio/accel/st_accel_core.c       | 5 ++---
>  drivers/iio/gyro/st_gyro_core.c         | 5 ++---
>  drivers/iio/magnetometer/st_magn_core.c | 5 ++---
>  drivers/iio/pressure/st_pressure_core.c | 8 ++------
>  4 files changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> index 31ea19d0ba71..d314125269e4 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -1139,10 +1139,9 @@ static int st_accel_write_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SAMP_FREQ:
>  		if (val2)
>  			return -EINVAL;
> -		mutex_lock(&indio_dev->mlock);
> +
>  		err = st_sensors_set_odr(indio_dev, val);
> -		mutex_unlock(&indio_dev->mlock);
> -		return err;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
> index 201050b76fe5..46e3df1bfacb 100644
> --- a/drivers/iio/gyro/st_gyro_core.c
> +++ b/drivers/iio/gyro/st_gyro_core.c
> @@ -415,10 +415,9 @@ static int st_gyro_write_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SAMP_FREQ:
>  		if (val2)
>  			return -EINVAL;
> -		mutex_lock(&indio_dev->mlock);
> +
>  		err = st_sensors_set_odr(indio_dev, val);
> -		mutex_unlock(&indio_dev->mlock);
> -		return err;
> +		break;
>  	default:
>  		err = -EINVAL;
>  	}
> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
> index 0806a1e65ce4..7b48e7a29cee 100644
> --- a/drivers/iio/magnetometer/st_magn_core.c
> +++ b/drivers/iio/magnetometer/st_magn_core.c
> @@ -549,10 +549,9 @@ static int st_magn_write_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_SAMP_FREQ:
>  		if (val2)
>  			return -EINVAL;
> -		mutex_lock(&indio_dev->mlock);
> +
>  		err = st_sensors_set_odr(indio_dev, val);
> -		mutex_unlock(&indio_dev->mlock);
> -		return err;
> +		break;
>  	default:
>  		err = -EINVAL;
>  	}
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index 26a1ee43d56e..05a909eeaff0 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -560,16 +560,12 @@ static int st_press_write_raw(struct iio_dev *indio_dev,
>  			      int val2,
>  			      long mask)
>  {
> -	int err;
> -
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SAMP_FREQ:
>  		if (val2)
>  			return -EINVAL;
> -		mutex_lock(&indio_dev->mlock);
> -		err = st_sensors_set_odr(indio_dev, val);
> -		mutex_unlock(&indio_dev->mlock);
> -		return err;
> +
> +		return st_sensors_set_odr(indio_dev, val);
>  	default:
>  		return -EINVAL;
>  	}