Message ID | 20240405104641.29478-1-subhajit.ghosh@tweaklogic.com (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
Series | iio: light: apds9306: Improve apds9306_write_event_config() | expand |
On Fri, 5 Apr 2024 21:16:41 +1030 Subhajit Ghosh <subhajit.ghosh@tweaklogic.com> wrote: > Simplify event configuration flow. > > Suggested-by: Jonathan Cameron <jic23@kernel.org> > Link: https://lore.kernel.org/all/20240310124237.52fa8a56@jic23-huawei/ > Signed-off-by: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com> LGTM Applied to the togreg branch of iio.git and pushed out initially as testing to let 0-day see if we missed anything. Jonathan > --- > drivers/iio/light/apds9306.c | 48 ++++++++++++++++++++---------------- > 1 file changed, 27 insertions(+), 21 deletions(-) > > diff --git a/drivers/iio/light/apds9306.c b/drivers/iio/light/apds9306.c > index 4d8490602cd7..465b6957682b 100644 > --- a/drivers/iio/light/apds9306.c > +++ b/drivers/iio/light/apds9306.c > @@ -1075,14 +1075,16 @@ static int apds9306_write_event_config(struct iio_dev *indio_dev, > { > struct apds9306_data *data = iio_priv(indio_dev); > struct apds9306_regfields *rf = &data->rf; > - int ret, val; > - > - state = !!state; > + int ret, enabled; > > switch (type) { > case IIO_EV_TYPE_THRESH: { > guard(mutex)(&data->mutex); > > + ret = regmap_field_read(rf->int_en, &enabled); > + if (ret) > + return ret; > + > /* > * If interrupt is enabled, the channel is set before enabling > * the interrupt. In case of disable, no need to switch > @@ -1091,38 +1093,42 @@ static int apds9306_write_event_config(struct iio_dev *indio_dev, > */ > if (state) { > if (chan->type == IIO_LIGHT) > - val = 1; > + ret = regmap_field_write(rf->int_src, 1); > else if (chan->type == IIO_INTENSITY) > - val = 0; > + ret = regmap_field_write(rf->int_src, 0); > else > return -EINVAL; > > - ret = regmap_field_write(rf->int_src, val); > if (ret) > return ret; > - } > > - ret = regmap_field_read(rf->int_en, &val); > - if (ret) > - return ret; > - > - if (val == state) > - return 0; > + if (enabled) > + return 0; > > - ret = regmap_field_write(rf->int_en, state); > - if (ret) > - return ret; > + ret = regmap_field_write(rf->int_en, 1); > + if (ret) > + return ret; > > - if (state) > return pm_runtime_resume_and_get(data->dev); > + } else { > + if (!enabled) > + return 0; > > - pm_runtime_mark_last_busy(data->dev); > - pm_runtime_put_autosuspend(data->dev); > + ret = regmap_field_write(rf->int_en, 0); > + if (ret) > + return ret; > > - return 0; > + pm_runtime_mark_last_busy(data->dev); > + pm_runtime_put_autosuspend(data->dev); > + > + return 0; > + } > } > case IIO_EV_TYPE_THRESH_ADAPTIVE: > - return regmap_field_write(rf->int_thresh_var_en, state); > + if (state) > + return regmap_field_write(rf->int_thresh_var_en, 1); > + else > + return regmap_field_write(rf->int_thresh_var_en, 0); > default: > return -EINVAL; > } > > base-commit: 526f7f17b651c78ead26fea7cea20948c00e47a5
diff --git a/drivers/iio/light/apds9306.c b/drivers/iio/light/apds9306.c index 4d8490602cd7..465b6957682b 100644 --- a/drivers/iio/light/apds9306.c +++ b/drivers/iio/light/apds9306.c @@ -1075,14 +1075,16 @@ static int apds9306_write_event_config(struct iio_dev *indio_dev, { struct apds9306_data *data = iio_priv(indio_dev); struct apds9306_regfields *rf = &data->rf; - int ret, val; - - state = !!state; + int ret, enabled; switch (type) { case IIO_EV_TYPE_THRESH: { guard(mutex)(&data->mutex); + ret = regmap_field_read(rf->int_en, &enabled); + if (ret) + return ret; + /* * If interrupt is enabled, the channel is set before enabling * the interrupt. In case of disable, no need to switch @@ -1091,38 +1093,42 @@ static int apds9306_write_event_config(struct iio_dev *indio_dev, */ if (state) { if (chan->type == IIO_LIGHT) - val = 1; + ret = regmap_field_write(rf->int_src, 1); else if (chan->type == IIO_INTENSITY) - val = 0; + ret = regmap_field_write(rf->int_src, 0); else return -EINVAL; - ret = regmap_field_write(rf->int_src, val); if (ret) return ret; - } - ret = regmap_field_read(rf->int_en, &val); - if (ret) - return ret; - - if (val == state) - return 0; + if (enabled) + return 0; - ret = regmap_field_write(rf->int_en, state); - if (ret) - return ret; + ret = regmap_field_write(rf->int_en, 1); + if (ret) + return ret; - if (state) return pm_runtime_resume_and_get(data->dev); + } else { + if (!enabled) + return 0; - pm_runtime_mark_last_busy(data->dev); - pm_runtime_put_autosuspend(data->dev); + ret = regmap_field_write(rf->int_en, 0); + if (ret) + return ret; - return 0; + pm_runtime_mark_last_busy(data->dev); + pm_runtime_put_autosuspend(data->dev); + + return 0; + } } case IIO_EV_TYPE_THRESH_ADAPTIVE: - return regmap_field_write(rf->int_thresh_var_en, state); + if (state) + return regmap_field_write(rf->int_thresh_var_en, 1); + else + return regmap_field_write(rf->int_thresh_var_en, 0); default: return -EINVAL; }
Simplify event configuration flow. Suggested-by: Jonathan Cameron <jic23@kernel.org> Link: https://lore.kernel.org/all/20240310124237.52fa8a56@jic23-huawei/ Signed-off-by: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com> --- drivers/iio/light/apds9306.c | 48 ++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 21 deletions(-) base-commit: 526f7f17b651c78ead26fea7cea20948c00e47a5