Re: [PATCH v4] iio: light: apds9306: Refactor threshold get/set functions to use helper

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 11 Jun 2025 14:42:53 -0300
nattan <nattanferreira58@gmail.com> wrote:

> From: Nattan Ferreira <nattanferreira58@gmail.com>
> 
> Refactor the apds9306_event_thresh_get and apds9306_event_thresh_set
Applied with () added after function names in this description.

Thanks,

J
> functions to use a helper function (apds9306_get_thresh_reg) for
> obtaining the correct register based on the direction of the event. This
> improves code readability and maintains consistency in accessing
> threshold registers.
> 
> Signed-off-by: Nattan Ferreira <nattanferreira58@gmail.com>
> Co-developed-by: Lucas Antonio <lucasantonio.santos@usp.br>
> Signed-off-by: Lucas Antonio <lucasantonio.santos@usp.br>
> 
> ---
> 
> Changes in v4:
> - Noted that the change actually adds four more lines to the driver file,
>   so rephrased description to avoid claiming code line reduction.
> ---
>  drivers/iio/light/apds9306.c | 36 ++++++++++++++++++++----------------
>  1 file changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/light/apds9306.c b/drivers/iio/light/apds9306.c
> index 69a0d609c..9216d4974 100644
> --- a/drivers/iio/light/apds9306.c
> +++ b/drivers/iio/light/apds9306.c
> @@ -744,20 +744,27 @@ static int apds9306_event_period_set(struct apds9306_data *data, int val)
>  	return regmap_field_write(rf->int_persist_val, val);
>  }
>  
> -static int apds9306_event_thresh_get(struct apds9306_data *data, int dir,
> -				     int *val)
> +static int apds9306_get_thresh_reg(int dir)
>  {
> -	int var, ret;
> -	u8 buff[3];
> -
>  	if (dir == IIO_EV_DIR_RISING)
> -		var = APDS9306_ALS_THRES_UP_0_REG;
> +		return APDS9306_ALS_THRES_UP_0_REG;
>  	else if (dir == IIO_EV_DIR_FALLING)
> -		var = APDS9306_ALS_THRES_LOW_0_REG;
> +		return APDS9306_ALS_THRES_LOW_0_REG;
>  	else
>  		return -EINVAL;
> +}
> +
> +static int apds9306_event_thresh_get(struct apds9306_data *data, int dir,
> +				     int *val)
> +{
> +	int reg, ret;
> +	u8 buff[3];
>  
> -	ret = regmap_bulk_read(data->regmap, var, buff, sizeof(buff));
> +	reg = apds9306_get_thresh_reg(dir);
> +	if (reg < 0)
> +		return reg;
> +
> +	ret = regmap_bulk_read(data->regmap, reg, buff, sizeof(buff));
>  	if (ret)
>  		return ret;
>  
> @@ -769,22 +776,19 @@ static int apds9306_event_thresh_get(struct apds9306_data *data, int dir,
>  static int apds9306_event_thresh_set(struct apds9306_data *data, int dir,
>  				     int val)
>  {
> -	int var;
> +	int reg;
>  	u8 buff[3];
>  
> -	if (dir == IIO_EV_DIR_RISING)
> -		var = APDS9306_ALS_THRES_UP_0_REG;
> -	else if (dir == IIO_EV_DIR_FALLING)
> -		var = APDS9306_ALS_THRES_LOW_0_REG;
> -	else
> -		return -EINVAL;
> +	reg = apds9306_get_thresh_reg(dir);
> +	if (reg < 0)
> +		return reg;
>  
>  	if (!in_range(val, 0, APDS9306_ALS_THRES_VAL_MAX))
>  		return -EINVAL;
>  
>  	put_unaligned_le24(val, buff);
>  
> -	return regmap_bulk_write(data->regmap, var, buff, sizeof(buff));
> +	return regmap_bulk_write(data->regmap, reg, buff, sizeof(buff));
>  }
>  
>  static int apds9306_event_thresh_adaptive_get(struct apds9306_data *data, int *val)