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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Sun, Apr 19, 2026 at 12:36:04PM +0100, Jonathan Cameron wrote:
> On Sat, 18 Apr 2026 21:35:43 -0300
> LucasRabaquim <lucas.rabaquim@usp.br> wrote:

> 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.

Indeed.

...

> > +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 },
> > +};

Instead, make this to be an index of the just an array of u8:s.

	...
	[TSL2591_PRST_ALS_INT_CYCLE_30] = 30,
	[TSL2591_PRST_ALS_INT_CYCLE_35] = 35,
	...

...

> >  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++) {

Besides the style of

	for (unsigned int i = 0; i < ARRAY_SIZE(tsl2591_persist_table); i++) {

> > +		if (als_persist == tsl2591_persist_table[i].cycle)
> > +			return tsl2591_persist_table[i].lit;
> >  	}

...this loop now not needed at all.

	if (als_persist < 1 || als_persist >= ARRAY_SIZE(tsl2591_persist_table))
		return -EINVAL;

	return tsl2591_persist_table[als_persist];

> > +	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;
> >  }

Ditto.

-- 
With Best Regards,
Andy Shevchenko