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

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

[Subhajit Ghosh]

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(-)

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

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

[Jonathan Cameron]

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