[PATCH v2 3/3] staging: iio: light: isl29018: Use standard sysfs attributes for scale and integration time

[Roberta Dobrescu]

Thanks for the review, Daniel. I'll fix the issues you noticed.
I just noticed a problem when reading lux.

>>   static int isl29018_read_lux(struct isl29018_chip *chip, int *lux)
>>   {
>> -       int lux_data;
>> -       unsigned int data_x_range, lux_unshifted;
>> +       int lux_data, i;
>> +       unsigned int data_x_range;
>> +       unsigned int adc_bits;
>>
>>          lux_data = isl29018_read_sensor_input(chip, COMMMAND1_OPMODE_ALS_ONCE);
>>
>> @@ -168,10 +207,23 @@ static int isl29018_read_lux(struct isl29018_chip *chip, int *lux)
>>           * ucalibscale ranges from 0-999999, so about 20 bits.  Split
>>           * the /1,000,000 in two to reduce the risk of over/underflow.
>>           */
>> -       data_x_range = lux_data * chip->range;
>> -       lux_unshifted = data_x_range * chip->calibscale;
>> -       lux_unshifted += data_x_range / 1000 * chip->ucalibscale / 1000;
>> -       *lux = lux_unshifted >> chip->adc_bit;
>> +
>> +       for (i = 0; i < ARRAY_SIZE(isl29018_int_times[chip->type]); ++i)
>> +               if (chip->int_time.time == isl29018_int_times[chip->type][i].time &&
>> +                   chip->int_time.utime == isl29018_int_times[chip->type][i].utime) {
>> +                       adc_bits = isl29018_adc_bits[i];
>> +                       break;
>> +               }
>> +
>> +       if (i >= ARRAY_SIZE(isl29018_int_times[chip->type]))
>> +               return -EINVAL;
>> +
>> +       data_x_range = lux_data * chip->scale.scale;
>> +       data_x_range += lux_data / 1000 * chip->scale.uscale / 1000;
>> +       data_x_range = data_x_range << (16 - adc_bits);

I'm not sure about this part above. It seems to be a problem if we have
8 or 4 adc bits, data_x_range becomes 0. And even with 16 or 12 bits
there are accuracy issues.

One of the problems is with this line:
	data_x_range += lux_data / 1000 * chip->scale.uscale / 1000;

If lux_data is let's say 228, data_x_range is automatically 0 after this
operation. I can change this lux_data * scale / 1000000 to solve this,
but I have to see if this won't lead to overflow for 16 or 12 bits.

Even if with this change, accuracy is far from being acceptable.

I did a small test for 8 bits resolution and range 1000. For a measured
lux_data of 228, data_x_range is 890 with the old formula (lux_data * 
range >> adc_bits). When calculating it with the new formula with scale
this is only 768.

I don't think this is ok at all. An option would be to have scale
exposed to userspace, but when calculating illuminace to use the
formula with range.

What do you think about this?

Thanks,
Roberta