Re: [PATCH v3 2/2] leds: add aw20xx driver

[Andy Shevchenko]

On Tue, Mar 14, 2023 at 6:22 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Tue, Mar 14, 2023 at 2:12 PM Martin Kurbanov
> <mmkurbanov@xxxxxxxxxxxxxx> wrote:
> > On 2023-03-01 00:51, Andy Shevchenko wrote:

...

> > >> +static int aw200xx_set_imax(const struct aw200xx *const chip,
> > >> +                           u32 led_imax_microamp)
> > >> +{
> > >> +       struct imax_global {
> > >> +               u32 regval;
> > >> +               u32 microamp;
> > >> +       } imaxs[] = {
> > >> +               { 8,  3300 },
> > >> +               { 9,  6700 },
> > >> +               { 0,  10000 },
> > >> +               { 11, 13300 },
> > >> +               { 1,  20000 },
> > >> +               { 13, 26700 },
> > >> +               { 2,  30000 },
> > >> +               { 3,  40000 },
> > >> +               { 15, 53300 },
> > >> +               { 4,  60000 },
> > >> +               { 5,  80000 },
> > >> +               { 6,  120000 },
> > >> +               { 7,  160000 },
> > >
> > > This looks a bit random. Is there any pattern on how value is
> > > connected to the register value?
> >
> > There is no ability to create any pattern here, because this table data
> > doesn’t have any regularity.
>
> There is a clear pattern.
>
> You have two tables, i.e. with multiplier 10000 and second one with
> multiplier 3333 (table in the datasheet seems bad from a math
> perspective). And it's even shown correctly in the datasheet.
>
> With this mix you missed 10.
>
> The coefficient table is 1,2,3,4,6,8,12,16 for both tables.
>
> Hence you need one table and two multipliers.
>
> Please, rewrite accordingly.

JFYI: You may see how I killed a table in one driver due to missing
understanding that there is a pattern.
9df461eca18f ("spi: pxa2xx: replace ugly table by approximation")

-- 
With Best Regards,
Andy Shevchenko