2024-04-05 10:47:11

by Subhajit Ghosh

Subject: [PATCH] iio: light: apds9306: Improve apds9306_write_event_config()

Simplify event configuration flow.

Suggested-by: Jonathan Cameron <[email protected]>
Link: https://lore.kernel.org/all/20240310124237.52fa8a56@jic23-huawei/
Signed-off-by: Subhajit Ghosh <[email protected]>
---
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
--
2.34.1

2024-04-06 15:53:34

by Jonathan Cameron

Subject: Re: [PATCH] iio: light: apds9306: Improve apds9306_write_event_config()

On Fri, 5 Apr 2024 21:16:41 +1030
Subhajit Ghosh <[email protected]> wrote:

> Simplify event configuration flow.
>
> Suggested-by: Jonathan Cameron <[email protected]>
> Link: https://lore.kernel.org/all/20240310124237.52fa8a56@jic23-huawei/
> Signed-off-by: Subhajit Ghosh <[email protected]>
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