| Message ID | 20220202140208.391394-7-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Miscellaneous IIO core enhancements | expand |
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; > }
Hi Jonathan, jic23@kernel.org wrote on Sun, 6 Feb 2022 15:45:33 +0000: > 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. I honestly hesitated when dropping the mlocks there. I've done it in v3, so that we don't hurt our eyes on this anymore :) > > 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! Thanks, Miquèl
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; }
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> --- 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(-)