On Sun, Feb 9, 2020 at 11:27 AM Gregor Riepl <onitake@xxxxxxxxx> wrote:
> > - for (i = 0; i < ARRAY_SIZE(gp2ap002_illuminance_table) - 1; i++) {
> > - ill1 = &gp2ap002_illuminance_table[i];
> > - ill2 = &gp2ap002_illuminance_table[i + 1];
> > -
> > - if (res > ill2->curr)
> > - continue;
> > - if ((res <= ill1->curr) && (res >= ill2->curr))
> > - break;
>
> That seems like a really, really contrived way to do a table lookup.
(...)
> And since res is linear, interpolation won't even be needed.
Sorry for my quick hack, patches welcome ;)
> > + lux = int_pow(10, (res/10));
> > + if (lux > INT_MAX) {
> > + dev_err(gp2ap002->dev, "lux overflow, capped\n");
> > + lux = INT_MAX;
> > }
>
> This is certainly better, but I wonder if it's worth the computational cost.
We don't do this much so certainly the linecount shrink is
worth it.
> Also: It looks like int_pow doesn't saturate, so even though it uses 64bit
> integer math, it might be better to move the range check before the calculation.
How do you mean I should be doing that without actually
doing the power calculation? (Maybe a dumb question but
math was never my best subject.)
Yours,
Linus Walleij
