| Message ID | 20240407172920.264282-4-vassilisamir@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Driver cleanup and series to add triggered buffer | expand |
On Sun, 7 Apr 2024 19:29:17 +0200 Vasileios Amoiridis <vassilisamir@gmail.com> wrote: > Introduce 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> A could of corners of dead code that you can remove. In general looks good. Jonathan > --- > drivers/iio/pressure/bmp280-core.c | 129 +++++++++++++---------------- > 1 file changed, 58 insertions(+), 71 deletions(-) > > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c > index 50bdf79011bc..51bcdf8cede6 100644 > --- a/drivers/iio/pressure/bmp280-core.c > +++ b/drivers/iio/pressure/bmp280-core.c > @@ -27,6 +27,7 @@ > > #include <linux/bitops.h> > #include <linux/bitfield.h> > +#include <linux/cleanup.h> > #include <linux/completion.h> > #include <linux/delay.h> > #include <linux/device.h> > @@ -499,77 +500,69 @@ static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2) > return IIO_VAL_INT; > } > > -static int bmp_read_raw(struct iio_dev *indio_dev, > - struct iio_chan_spec const *chan, > - int *val, int *val2, long mask) > +static int bmp_read_raw_impl(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > { > struct bmp280_data *data = iio_priv(indio_dev); > - int ret; > > - pm_runtime_get_sync(data->dev); > - mutex_lock(&data->lock); > + guard(mutex)(&data->lock); > > switch (mask) { > case IIO_CHAN_INFO_PROCESSED: > switch (chan->type) { > case IIO_HUMIDITYRELATIVE: > - ret = data->chip_info->read_humid(data, val, val2); > - break; > + return data->chip_info->read_humid(data, val, val2); > case IIO_PRESSURE: > - ret = data->chip_info->read_press(data, val, val2); > - break; > + return data->chip_info->read_press(data, val, val2); > case IIO_TEMP: > - ret = data->chip_info->read_temp(data, val, val2); > - break; > + return data->chip_info->read_temp(data, val, val2); > default: > - ret = -EINVAL; > - break; > + return -EINVAL; > } > - break; > case IIO_CHAN_INFO_OVERSAMPLING_RATIO: > switch (chan->type) { > case IIO_HUMIDITYRELATIVE: > *val = 1 << data->oversampling_humid; > - ret = IIO_VAL_INT; > - break; > + return IIO_VAL_INT; > case IIO_PRESSURE: > *val = 1 << data->oversampling_press; > - ret = IIO_VAL_INT; > - break; > + return IIO_VAL_INT; > case IIO_TEMP: > *val = 1 << data->oversampling_temp; > - ret = IIO_VAL_INT; > - break; > + return IIO_VAL_INT; > default: > - ret = -EINVAL; > - break; > + return -EINVAL; > } > - break; > case IIO_CHAN_INFO_SAMP_FREQ: > - if (!data->chip_info->sampling_freq_avail) { > - ret = -EINVAL; > - break; > - } > + if (!data->chip_info->sampling_freq_avail) > + return -EINVAL; > > *val = data->chip_info->sampling_freq_avail[data->sampling_freq][0]; > *val2 = data->chip_info->sampling_freq_avail[data->sampling_freq][1]; > - ret = IIO_VAL_INT_PLUS_MICRO; > - break; > + return IIO_VAL_INT_PLUS_MICRO; > case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: > - if (!data->chip_info->iir_filter_coeffs_avail) { > - ret = -EINVAL; > - break; > - } > + if (!data->chip_info->iir_filter_coeffs_avail) > + return -EINVAL; > > *val = (1 << data->iir_filter_coeff) - 1; > - ret = IIO_VAL_INT; > - break; > + return IIO_VAL_INT; > default: > - ret = -EINVAL; > - break; > + return -EINVAL; > } > > - mutex_unlock(&data->lock); > + return 0; As below. I don't think you can get here. I hope all paths have already returned. > +} > + > +static int bmp_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int *val, int *val2, long mask) > +{ > + struct bmp280_data *data = iio_priv(indio_dev); > + int ret; > + > + pm_runtime_get_sync(data->dev); > + ret = bmp_read_raw_impl(indio_dev, chan, val, val2, mask); > pm_runtime_mark_last_busy(data->dev); > pm_runtime_put_autosuspend(data->dev); > > @@ -697,12 +690,13 @@ static int bmp_write_iir_filter_coeffs(struct bmp280_data *data, int val) > return -EINVAL; > } > > -static int bmp_write_raw(struct iio_dev *indio_dev, > - struct iio_chan_spec const *chan, > - int val, int val2, long mask) > +static int bmp_write_raw_impl(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, > + int val, int val2, long mask) > { > struct bmp280_data *data = iio_priv(indio_dev); > - int ret = 0; > + > + guard(mutex)(&data->lock); > > /* > * Helper functions to update sensor running configuration. > @@ -712,46 +706,39 @@ static int bmp_write_raw(struct iio_dev *indio_dev, > */ > switch (mask) { > case IIO_CHAN_INFO_OVERSAMPLING_RATIO: > - pm_runtime_get_sync(data->dev); > - mutex_lock(&data->lock); > switch (chan->type) { > case IIO_HUMIDITYRELATIVE: > - ret = bmp_write_oversampling_ratio_humid(data, val); > - break; > + return bmp_write_oversampling_ratio_humid(data, val); > case IIO_PRESSURE: > - ret = bmp_write_oversampling_ratio_press(data, val); > - break; > + return bmp_write_oversampling_ratio_press(data, val); > case IIO_TEMP: > - ret = bmp_write_oversampling_ratio_temp(data, val); > - break; > + return bmp_write_oversampling_ratio_temp(data, val); > default: > - ret = -EINVAL; > - break; > + return -EINVAL; > } > - mutex_unlock(&data->lock); > - pm_runtime_mark_last_busy(data->dev); > - pm_runtime_put_autosuspend(data->dev); > - break; > case IIO_CHAN_INFO_SAMP_FREQ: > - pm_runtime_get_sync(data->dev); > - mutex_lock(&data->lock); > - ret = bmp_write_sampling_frequency(data, val, val2); > - mutex_unlock(&data->lock); > - pm_runtime_mark_last_busy(data->dev); > - pm_runtime_put_autosuspend(data->dev); > - break; > + return bmp_write_sampling_frequency(data, val, val2); > case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: > - pm_runtime_get_sync(data->dev); > - mutex_lock(&data->lock); > - ret = bmp_write_iir_filter_coeffs(data, val); > - mutex_unlock(&data->lock); > - pm_runtime_mark_last_busy(data->dev); > - pm_runtime_put_autosuspend(data->dev); > - break; > + return bmp_write_iir_filter_coeffs(data, val); > default: > return -EINVAL; > } > > + return 0; I'm fairly sure you can't get here so this is dead code. Hene drop this final return. > +} >
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c index 50bdf79011bc..51bcdf8cede6 100644 --- a/drivers/iio/pressure/bmp280-core.c +++ b/drivers/iio/pressure/bmp280-core.c @@ -27,6 +27,7 @@ #include <linux/bitops.h> #include <linux/bitfield.h> +#include <linux/cleanup.h> #include <linux/completion.h> #include <linux/delay.h> #include <linux/device.h> @@ -499,77 +500,69 @@ static int bme280_read_humid(struct bmp280_data *data, int *val, int *val2) return IIO_VAL_INT; } -static int bmp_read_raw(struct iio_dev *indio_dev, - struct iio_chan_spec const *chan, - int *val, int *val2, long mask) +static int bmp_read_raw_impl(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) { struct bmp280_data *data = iio_priv(indio_dev); - int ret; - pm_runtime_get_sync(data->dev); - mutex_lock(&data->lock); + guard(mutex)(&data->lock); switch (mask) { case IIO_CHAN_INFO_PROCESSED: switch (chan->type) { case IIO_HUMIDITYRELATIVE: - ret = data->chip_info->read_humid(data, val, val2); - break; + return data->chip_info->read_humid(data, val, val2); case IIO_PRESSURE: - ret = data->chip_info->read_press(data, val, val2); - break; + return data->chip_info->read_press(data, val, val2); case IIO_TEMP: - ret = data->chip_info->read_temp(data, val, val2); - break; + return data->chip_info->read_temp(data, val, val2); default: - ret = -EINVAL; - break; + return -EINVAL; } - break; case IIO_CHAN_INFO_OVERSAMPLING_RATIO: switch (chan->type) { case IIO_HUMIDITYRELATIVE: *val = 1 << data->oversampling_humid; - ret = IIO_VAL_INT; - break; + return IIO_VAL_INT; case IIO_PRESSURE: *val = 1 << data->oversampling_press; - ret = IIO_VAL_INT; - break; + return IIO_VAL_INT; case IIO_TEMP: *val = 1 << data->oversampling_temp; - ret = IIO_VAL_INT; - break; + return IIO_VAL_INT; default: - ret = -EINVAL; - break; + return -EINVAL; } - break; case IIO_CHAN_INFO_SAMP_FREQ: - if (!data->chip_info->sampling_freq_avail) { - ret = -EINVAL; - break; - } + if (!data->chip_info->sampling_freq_avail) + return -EINVAL; *val = data->chip_info->sampling_freq_avail[data->sampling_freq][0]; *val2 = data->chip_info->sampling_freq_avail[data->sampling_freq][1]; - ret = IIO_VAL_INT_PLUS_MICRO; - break; + return IIO_VAL_INT_PLUS_MICRO; case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: - if (!data->chip_info->iir_filter_coeffs_avail) { - ret = -EINVAL; - break; - } + if (!data->chip_info->iir_filter_coeffs_avail) + return -EINVAL; *val = (1 << data->iir_filter_coeff) - 1; - ret = IIO_VAL_INT; - break; + return IIO_VAL_INT; default: - ret = -EINVAL; - break; + return -EINVAL; } - mutex_unlock(&data->lock); + return 0; +} + +static int bmp_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) +{ + struct bmp280_data *data = iio_priv(indio_dev); + int ret; + + pm_runtime_get_sync(data->dev); + ret = bmp_read_raw_impl(indio_dev, chan, val, val2, mask); pm_runtime_mark_last_busy(data->dev); pm_runtime_put_autosuspend(data->dev); @@ -697,12 +690,13 @@ static int bmp_write_iir_filter_coeffs(struct bmp280_data *data, int val) return -EINVAL; } -static int bmp_write_raw(struct iio_dev *indio_dev, - struct iio_chan_spec const *chan, - int val, int val2, long mask) +static int bmp_write_raw_impl(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long mask) { struct bmp280_data *data = iio_priv(indio_dev); - int ret = 0; + + guard(mutex)(&data->lock); /* * Helper functions to update sensor running configuration. @@ -712,46 +706,39 @@ static int bmp_write_raw(struct iio_dev *indio_dev, */ switch (mask) { case IIO_CHAN_INFO_OVERSAMPLING_RATIO: - pm_runtime_get_sync(data->dev); - mutex_lock(&data->lock); switch (chan->type) { case IIO_HUMIDITYRELATIVE: - ret = bmp_write_oversampling_ratio_humid(data, val); - break; + return bmp_write_oversampling_ratio_humid(data, val); case IIO_PRESSURE: - ret = bmp_write_oversampling_ratio_press(data, val); - break; + return bmp_write_oversampling_ratio_press(data, val); case IIO_TEMP: - ret = bmp_write_oversampling_ratio_temp(data, val); - break; + return bmp_write_oversampling_ratio_temp(data, val); default: - ret = -EINVAL; - break; + return -EINVAL; } - mutex_unlock(&data->lock); - pm_runtime_mark_last_busy(data->dev); - pm_runtime_put_autosuspend(data->dev); - break; case IIO_CHAN_INFO_SAMP_FREQ: - pm_runtime_get_sync(data->dev); - mutex_lock(&data->lock); - ret = bmp_write_sampling_frequency(data, val, val2); - mutex_unlock(&data->lock); - pm_runtime_mark_last_busy(data->dev); - pm_runtime_put_autosuspend(data->dev); - break; + return bmp_write_sampling_frequency(data, val, val2); case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: - pm_runtime_get_sync(data->dev); - mutex_lock(&data->lock); - ret = bmp_write_iir_filter_coeffs(data, val); - mutex_unlock(&data->lock); - pm_runtime_mark_last_busy(data->dev); - pm_runtime_put_autosuspend(data->dev); - break; + return bmp_write_iir_filter_coeffs(data, val); default: return -EINVAL; } + return 0; +} + +static int bmp_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long mask) +{ + struct bmp280_data *data = iio_priv(indio_dev); + int ret; + + pm_runtime_get_sync(data->dev); + ret = bmp_write_raw_impl(indio_dev, chan, val, val2, mask); + pm_runtime_mark_last_busy(data->dev); + pm_runtime_put_autosuspend(data->dev); + return ret; }
Introduce 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 | 129 +++++++++++++---------------- 1 file changed, 58 insertions(+), 71 deletions(-)