On Sun, Feb 2, 2020 at 4:08 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On Sun, 26 Jan 2020 20:27:22 +0000 Jonathan Bakker <xc-racer2@xxxxxxx> wrote:
> > - if (ch_type != IIO_CURRENT) {
> > + if (ch_type == IIO_VOLTAGE) {
> > + ret = device_property_read_u8(dev,
> > + "sharp,adc-adjustment-ratio", &val);
> > + if (ret) {
> > + dev_err(dev,
> > + "failed to obtain adc conversion\n");
> > + return -EINVAL;
> > + }
> > + gp2ap002->adc_adj = val;
> > + } else if (ch_type != IIO_CURRENT) {
> > dev_err(dev,
> > "wrong type of IIO channel specified for ALSOUT\n");
> > return -EINVAL;
> >
> > Alternatively, you could collect the resistor value, the ADC precision (this doesn't
> > appear to be queryable via the IIO layer), and the reference voltage level - but I'm
> > not sure how you'd do the inverse calculation in the kernel.
>
> An alternative to doing this is to represent the resistor circuit explicitly.
>
> You end up with a really small ADC driver that is a consumer of a voltage
> and provides a current channel. That has all the properties of the
> circuit via DT.
That is indeed a lot better, more modular and more reusable.
It also becomes its own node in the device tree with very
generic bindings for resistance and ADC bias/offset.
> In general I would prefer we handle this sort of conversion generically
> rather than bolting it into a light sensor driver like you are doing here,
> even if it comes at the cost of a bit more complexity.
Agreed.
There are however two improvements that can be done as separate
patches to the code in this driver, but preferably by someone with access
to the right hardware so they can verify the result.
The Google Android code pointed to by Mr. Bakker:
https://android.googlesource.com/device/samsung/crespo/+/2e0ab7265e3039fee787c2216e0c98d92ea0b49e%5E%21/#F0
+ // Convert adc value to lux assuming:
+ // I = 10 * log(Ev) uA
+ // R = 47kOhm
+ // Max adc value 4095 = 3.3V
+ // 1/4 of light reaches sensor
+ mPendingEvent.light = powf(10, event->value * (330.0f
/ 4095.0f / 47.0f)) * 4;
contains:
- A logarithmic formula based on the datasheet which we
don't have but presumably correct. A patch converting the
crude interpolated look-up table to proper floating point maths
expressing the curve would be much appreciated and cuts
down the size of the driver. This should be one simple
patch with nothing else needing to be changed. According
to the formula it should be lux = 10^(mA/10) which corresponds
to the values in the table. I verified the values with a spreadsheet,
then I sent a patch like this, please test!
- A device tree property to compensate for the attenuation by
the glass in front of the sensor. In the example the
attenuation is 75%, only 1/4 of the light actually hits the
sensor. I am uncertain about the physics of this, should
that really be expressed as fraction or percentage?
Should it rather be in dB? This should be another patch
adding the DT property and maths for the attenuation.
Yours,
Linus Walleij
