From: Gregor Riepl <onitake@gmail.com>
To: Linus Walleij <linus.walleij@linaro.org>,
Jonathan Cameron <jic23@kernel.org>,
linux-iio@vger.kernel.org
Cc: Hartmut Knaack <knaack.h@gmx.de>,
Lars-Peter Clausen <lars@metafoo.de>,
Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
linux-input@vger.kernel.org, Jonathan Bakker <xc-racer2@live.ca>
Subject: Re: [PATCH] RFT: iio: gp2ap002: Replace LUT with math
Date: Sun, 9 Feb 2020 11:27:00 +0100 [thread overview]
Message-ID: <74ab4b7b-eae2-0c6f-bb4a-eabbd3b4d042@gmail.com> (raw)
In-Reply-To: <20200208123359.396-1-linus.walleij@linaro.org>
> -/*
> - * This array maps current and lux.
> - *
> - * Ambient light sensing range is 3 to 55000 lux.
> - *
> - * This mapping is based on the following formula.
> - * illuminance = 10 ^ (current / 10)
> - */
> -static const struct gp2ap002_illuminance gp2ap002_illuminance_table[] = {
> - { .curr = 5, .lux = 3 },
> - { .curr = 6, .lux = 4 },
> - { .curr = 7, .lux = 5 },
> - { .curr = 8, .lux = 6 },
> - { .curr = 9, .lux = 8 },
> - { .curr = 10, .lux = 10 },
> - { .curr = 11, .lux = 12 },
> - { .curr = 12, .lux = 16 },
> - { .curr = 13, .lux = 20 },
> - { .curr = 14, .lux = 25 },
> - { .curr = 15, .lux = 32 },
> - { .curr = 16, .lux = 40 },
> - { .curr = 17, .lux = 50 },
> - { .curr = 18, .lux = 63 },
> - { .curr = 19, .lux = 79 },
> - { .curr = 20, .lux = 100 },
> - { .curr = 21, .lux = 126 },
> - { .curr = 22, .lux = 158 },
> - { .curr = 23, .lux = 200 },
> - { .curr = 24, .lux = 251 },
> - { .curr = 25, .lux = 316 },
> - { .curr = 26, .lux = 398 },
> - { .curr = 27, .lux = 501 },
> - { .curr = 28, .lux = 631 },
> - { .curr = 29, .lux = 794 },
> - { .curr = 30, .lux = 1000 },
> - { .curr = 31, .lux = 1259 },
> - { .curr = 32, .lux = 1585 },
> - { .curr = 33, .lux = 1995 },
> - { .curr = 34, .lux = 2512 },
> - { .curr = 35, .lux = 3162 },
> - { .curr = 36, .lux = 3981 },
> - { .curr = 37, .lux = 5012 },
> - { .curr = 38, .lux = 6310 },
> - { .curr = 39, .lux = 7943 },
> - { .curr = 40, .lux = 10000 },
> - { .curr = 41, .lux = 12589 },
> - { .curr = 42, .lux = 15849 },
> - { .curr = 43, .lux = 19953 },
> - { .curr = 44, .lux = 25119 },
> - { .curr = 45, .lux = 31623 },
> - { .curr = 46, .lux = 39811 },
> - { .curr = 47, .lux = 50119 },
> -};
...
> - 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.
According to the table above, all successive input values between 5 and 47 are
covered, so shouldn't this be simple array indexing?
Something like:
#define gp2ap002_value_min 5
#define gp2ap002_value_max 47
static const unsigned int gp2ap002_value_to_illuminance_table[] = {
3, 4, 5, 6, 8, ...... 39811, 50119
};
#define gp2ap002_table_size ARRAY_SIZE(gp2ap002_value_to_illuminance_table)
if (res < gp2ap002_value_min)
return gp2ap002_value_to_illuminance_table[0];
if (res > gp2ap002_value_max)
return gp2ap002_value_to_illuminance_table[gp2ap002_table_size - 1];
lux = gp2ap002_value_to_illuminance_table[res - gp2ap002_value_min];
And since res is linear, interpolation won't even be needed.
What am I missing?
> + 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.
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.
next prev parent reply other threads:[~2020-02-09 10:27 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-08 12:33 [PATCH] RFT: iio: gp2ap002: Replace LUT with math Linus Walleij
2020-02-09 10:27 ` Gregor Riepl [this message]
2020-02-10 13:28 ` Linus Walleij
2020-02-10 19:33 ` Gregor Riepl
2020-02-10 23:19 ` Jonathan Bakker
2020-02-11 7:15 ` Gregor Riepl
2020-02-11 7:51 ` Linus Walleij
2020-02-11 8:09 ` Gregor Riepl
2020-02-10 22:53 ` Jonathan Bakker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=74ab4b7b-eae2-0c6f-bb4a-eabbd3b4d042@gmail.com \
--to=onitake@gmail.com \
--cc=jic23@kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=linus.walleij@linaro.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=pmeerw@pmeerw.net \
--cc=xc-racer2@live.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).