Re: [PATCH v2] iio: light: tsl2591: simplify tsl2591_persist functions via lookup table

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 18 Apr 2026 21:35:43 -0300
LucasRabaquim <lucas.rabaquim@usp.br> wrote:

> Replace the switch-case logic in tsl2591_persist_cycle_to_lit()
> and tsl2591_persist_lit_to_cycle() with a bidirectional
> lookup table tsl2591_persist_table.
> 
> These functions have redundant switch-cases. Now they use a for-loop,
> like in tsl2591_als_time_to_fval(). The change simplifies
> addition or modification of TSL2591_PRST_ALS_INT_CYCLE_* values by
> associating cycle and lit values in a single entry defined by
> tsl2591_persist_entry type, reducing code duplication and verbosity.
> 
> Signed-off-by: LucasRabaquim <lucas.rabaquim@usp.br>
> Co-developed-by: Matheus Feitosa <matheus.feitosa@usp.br>
> Signed-off-by: Matheus Feitosa <matheus.feitosa@usp.br>
Hi.

First a general process thing.  Slow down!  You might well have
gotten more feedback on v1 if it had been on list without being
replaced for more than a day.  I'd advise generally not sending new
versions for at least a week so multiple reviewers have time to look
at the changes.

In general the change looks good, but I'll leave it on list for
a while before considering picking it up.

Thanks,

Jonathan


> ---
>  drivers/iio/light/tsl2591.c | 104 +++++++++++++-----------------------
>  1 file changed, 38 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/iio/light/tsl2591.c b/drivers/iio/light/tsl2591.c
> index c5557867ea43..3fe34a48ec2b 100644
> --- a/drivers/iio/light/tsl2591.c
> +++ b/drivers/iio/light/tsl2591.c
> @@ -170,6 +170,12 @@ struct tsl2591_chip {
>  	bool events_enabled;
>  };
>  
> +/* Persist cycle conversion table mapping register cycles to lit values */
> +struct tsl2591_persist_entry {
> +	u8 cycle;
> +	u8 lit;
> +};
> +
>  /*
>   * Period table is ALS persist cycle x integration time setting
>   * Integration times: 100ms, 200ms, 300ms, 400ms, 500ms, 600ms
> @@ -229,80 +235,46 @@ static int tsl2591_multiplier_to_gain(const u32 multiplier)
>  	}
>  }
>  
> +static const struct tsl2591_persist_entry tsl2591_persist_table[] = {
> +	{ TSL2591_PRST_ALS_INT_CYCLE_ANY,  1  },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_2,    2  },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_3,    3  },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_5,    5  },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_10,   10 },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_15,   15 },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_20,   20 },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_25,   25 },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_30,   30 },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_35,   35 },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_40,   40 },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_45,   45 },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_50,   50 },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_55,   55 },
> +	{ TSL2591_PRST_ALS_INT_CYCLE_60,   60 },
> +};
> +
>  static int tsl2591_persist_cycle_to_lit(const u8 als_persist)
>  {
> -	switch (als_persist) {
> -	case TSL2591_PRST_ALS_INT_CYCLE_ANY:
> -		return 1;
> -	case TSL2591_PRST_ALS_INT_CYCLE_2:
> -		return 2;
> -	case TSL2591_PRST_ALS_INT_CYCLE_3:
> -		return 3;
> -	case TSL2591_PRST_ALS_INT_CYCLE_5:
> -		return 5;
> -	case TSL2591_PRST_ALS_INT_CYCLE_10:
> -		return 10;
> -	case TSL2591_PRST_ALS_INT_CYCLE_15:
> -		return 15;
> -	case TSL2591_PRST_ALS_INT_CYCLE_20:
> -		return 20;
> -	case TSL2591_PRST_ALS_INT_CYCLE_25:
> -		return 25;
> -	case TSL2591_PRST_ALS_INT_CYCLE_30:
> -		return 30;
> -	case TSL2591_PRST_ALS_INT_CYCLE_35:
> -		return 35;
> -	case TSL2591_PRST_ALS_INT_CYCLE_40:
> -		return 40;
> -	case TSL2591_PRST_ALS_INT_CYCLE_45:
> -		return 45;
> -	case TSL2591_PRST_ALS_INT_CYCLE_50:
> -		return 50;
> -	case TSL2591_PRST_ALS_INT_CYCLE_55:
> -		return 55;
> -	case TSL2591_PRST_ALS_INT_CYCLE_60:
> -		return 60;
> -	default:
> -		return -EINVAL;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tsl2591_persist_table); i++) {
> +		if (als_persist == tsl2591_persist_table[i].cycle)
> +			return tsl2591_persist_table[i].lit;
>  	}
> +
> +	return -EINVAL;
>  }
>  
>  static int tsl2591_persist_lit_to_cycle(const u8 als_persist)
>  {
> -	switch (als_persist) {
> -	case 1:
> -		return TSL2591_PRST_ALS_INT_CYCLE_ANY;
> -	case 2:
> -		return TSL2591_PRST_ALS_INT_CYCLE_2;
> -	case 3:
> -		return TSL2591_PRST_ALS_INT_CYCLE_3;
> -	case 5:
> -		return TSL2591_PRST_ALS_INT_CYCLE_5;
> -	case 10:
> -		return TSL2591_PRST_ALS_INT_CYCLE_10;
> -	case 15:
> -		return TSL2591_PRST_ALS_INT_CYCLE_15;
> -	case 20:
> -		return TSL2591_PRST_ALS_INT_CYCLE_20;
> -	case 25:
> -		return TSL2591_PRST_ALS_INT_CYCLE_25;
> -	case 30:
> -		return TSL2591_PRST_ALS_INT_CYCLE_30;
> -	case 35:
> -		return TSL2591_PRST_ALS_INT_CYCLE_35;
> -	case 40:
> -		return TSL2591_PRST_ALS_INT_CYCLE_40;
> -	case 45:
> -		return TSL2591_PRST_ALS_INT_CYCLE_45;
> -	case 50:
> -		return TSL2591_PRST_ALS_INT_CYCLE_50;
> -	case 55:
> -		return TSL2591_PRST_ALS_INT_CYCLE_55;
> -	case 60:
> -		return TSL2591_PRST_ALS_INT_CYCLE_60;
> -	default:
> -		return -EINVAL;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tsl2591_persist_table); i++) {
> +		if (als_persist == tsl2591_persist_table[i].lit)
> +			return tsl2591_persist_table[i].cycle;
>  	}
> +
> +	return -EINVAL;
>  }
>  
>  static int tsl2591_compatible_int_time(struct tsl2591_chip *chip,