Hi Linus,
After implementing a small shim from the voltage ADC on my device, I was able to test this.
Please see my inline comments.
On 2020-02-08 4:33 a.m., Linus Walleij wrote:
> This replaces the current-to-lux look up table with some
> integer math using int_pow() from <linux/kernel.h>.
> Drop the unused <linux/math64.h> in the process.
>
> Cc: Jonathan Bakker <xc-racer2@xxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> I would appreciate testing that this give the right
> result. Using math does look a lot better.
> ---
> drivers/iio/light/gp2ap002.c | 94 +++---------------------------------
> 1 file changed, 7 insertions(+), 87 deletions(-)
>
> diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
> index 433907619268..153ae0a163ab 100644
> --- a/drivers/iio/light/gp2ap002.c
> +++ b/drivers/iio/light/gp2ap002.c
> @@ -33,7 +33,6 @@
> #include <linux/pm_runtime.h>
> #include <linux/interrupt.h>
> #include <linux/bits.h>
> -#include <linux/math64.h>
> #include <linux/pm.h>
>
> #define GP2AP002_PROX_CHANNEL 0
> @@ -205,71 +204,10 @@ static irqreturn_t gp2ap002_prox_irq(int irq, void *d)
> return IRQ_HANDLED;
> }
>
> -struct gp2ap002_illuminance {
> - unsigned int curr;
> - unsigned int lux;
> -};
> -
> -/*
> - * 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 },
> -};
> -
> static int gp2ap002_get_lux(struct gp2ap002 *gp2ap002)
> {
> - const struct gp2ap002_illuminance *ill1;
> - const struct gp2ap002_illuminance *ill2;
> int ret, res;
> - int i;
> + u64 lux;
>
> ret = iio_read_channel_processed(gp2ap002->alsout, &res);
> if (ret < 0)
> @@ -277,31 +215,13 @@ static int gp2ap002_get_lux(struct gp2ap002 *gp2ap002)
>
> dev_dbg(gp2ap002->dev, "read %d mA from ADC\n", res);
>
> - ill1 = &gp2ap002_illuminance_table[0];
> - if (res < ill1->curr) {
> - dev_dbg(gp2ap002->dev, "total darkness\n");
> - return 0;
> - }
> - 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;
> + lux = int_pow(10, (res/10));
Unfortunately, this doesn't seem to work. Since it's integer math, it means we
only have an output of 100, 1000, or 10000 depending on the reading. A LUT is
probably a much better solution.
> + if (lux > INT_MAX) {
> + dev_err(gp2ap002->dev, "lux overflow, capped\n");
> + lux = INT_MAX;
> }
> - if (res > ill2->curr) {
> - dev_info_once(gp2ap002->dev, "max current overflow\n");
> - return ill2->curr;
> - }
> - /* Interpolate and return */
> - dev_dbg(gp2ap002->dev, "interpolate index %d and %d\n", i, i + 1);
> - /* How many steps along the curve */
> - i = res - ill1->curr; /* x - x0 */
> - /* Linear interpolation */
> - return ill1->lux + i *
> - ((ill2->lux - ill1->lux) / (ill2->curr - ill1->curr));
However, this also apparently didn't really work either as I was having what appeared to be
overflow errors (ie when a raw ADC value of 37, this calculation pulls out -52961).
Here's a few entries after add a debug print right after the gp2ap002_get_lux() call in
gp2ap002_read_raw():
gp2ap002 11-0044: read 33 mA from ADC
iio iio:device4: read value of -94193
gp2ap002 11-0044: read 37 mA from ADC
iio iio:device4: read value of -52961
gp2ap002 11-0044: read 39 mA from ADC
iio iio:device4: read value of -32345
gp2ap002 11-0044: read 42 mA from ADC
iio iio:device4: read value of -1421
gp2ap002 11-0044: read 39 mA from ADC
iio iio:device4: read value of -32345
gp2ap002 11-0044: read 29 mA from ADC
iio iio:device4: read value of -135425
gp2ap002 11-0044: read 48 mA from ADC
gp2ap002 11-0044: max current overflow
iio iio:device4: read value of 47
gp2ap002 11-0044: read 33 mA from ADC
My apologies for not catching this first time around.
> +
> + return (int)lux;
> }
>
> static int gp2ap002_read_raw(struct iio_dev *indio_dev,
>
Thanks,
Jonathan Bakker
