* [PATCH v2] iio: light: tsl2591: simplify tsl2591_persist functions via lookup table
@ 2026-04-19 0:35 LucasRabaquim
2026-04-19 11:36 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: LucasRabaquim @ 2026-04-19 0:35 UTC (permalink / raw)
To: jic23, dlechner, nuno.sa, andy; +Cc: LucasRabaquim, Matheus Feitosa, linux-iio
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>
---
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,
--
2.47.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: light: tsl2591: simplify tsl2591_persist functions via lookup table
2026-04-19 0:35 [PATCH v2] iio: light: tsl2591: simplify tsl2591_persist functions via lookup table LucasRabaquim
@ 2026-04-19 11:36 ` Jonathan Cameron
2026-04-20 7:44 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2026-04-19 11:36 UTC (permalink / raw)
To: LucasRabaquim; +Cc: dlechner, nuno.sa, andy, Matheus Feitosa, linux-iio
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,
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: light: tsl2591: simplify tsl2591_persist functions via lookup table
2026-04-19 11:36 ` Jonathan Cameron
@ 2026-04-20 7:44 ` Andy Shevchenko
0 siblings, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-04-20 7:44 UTC (permalink / raw)
To: Jonathan Cameron
Cc: LucasRabaquim, dlechner, nuno.sa, andy, Matheus Feitosa,
linux-iio
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-20 7:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-19 0:35 [PATCH v2] iio: light: tsl2591: simplify tsl2591_persist functions via lookup table LucasRabaquim
2026-04-19 11:36 ` Jonathan Cameron
2026-04-20 7:44 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox