Received-SPF: pass (google.com: domain of linux-kernel+bounces-133991-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Sat, 6 Apr 2024 16:53:14 +0100
From: Jonathan Cameron <jic23@kernel.org>
To: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>, Dan Carpenter
 <dan.carpenter@linaro.org>, linux-iio@vger.kernel.org,
 linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: light: apds9306: Improve
 apds9306_write_event_config()
Message-ID: <20240406165314.1b3403df@jic23-huawei>
In-Reply-To: <20240405104641.29478-1-subhajit.ghosh@tweaklogic.com>
References: <20240405104641.29478-1-subhajit.ghosh@tweaklogic.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

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