Re: [PATCH] TSL2550 driver bugfix

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jean,

Jean Delvare wrote:
On Sat, 11 Jul 2009 20:20:30 +0200, Jean Delvare wrote:
For another, I do hit the case c1 > c0 from times to times, in
particular when I move the sensor in dark areas. Do you hit this as
well? This might be specific to the serial evaluation module I'm using,
it is very slow and this might cause c0 and c1 to be sampled at
significantly different times (say 200 ms apart.) If the light
conditions change meanwhile, this can explain why c1 > c0.

Scratch this theory. Reading the TSL2550 datasheet again, I understand
that sampling happens continuously and is unrelated to I2C reads.
Sampling of each channel takes 400 ms in standard mode, so c0 and c1
samplings are _always_ 400 ms apart, regardless of how fast the I2C
interface is. So you should be able do see the problem too.

This also means that the code in tsl2550_get_adc_value() is more
complex than it needs to be. The only case where it makes sense to wait
400 ms to get a reading is right after power-up. 400 ms after power-up,
c0 will always have a valid value, and after 800 ms c1 will always have
a valid value as well. So I propose to simplify tsl2550_get_adc_value()
and just return -EAGAIN if there is no valid reading. In practice I
doubt we'll ever hit it.

I am not
sure what to do in this case. We can return -EAGAIN and let user-space
retry. But we could also restart the computation automatically in this
case. Of course if the problem only affects the evaluation module then
we don't really care.

Thinking some more about this: if we want to retry then we will have to
wait at least 400 ms to read c0 again, and if it's not enough 400 more
ms to read c1 again. And even then, there's no guarantee the new
readings will be OK if the light conditions are still changing. This is
an intrinsic weakness of the TSL2550 that both channels are sampled
sequentially. I doubt we want to let the user wait for the lux value
for several seconds, so returning -EAGAIN seems OK to me.
I think that returning -EAGAIN should be fine in this case,
giving to the user application the responsibility to check the returned value.

But I am not using the light sensor a lot myself, so my practical
experience is rather limited, thus I would welcome comments and
opinions on this.

From my experience, the case of c1 > c0 happens when light conditions are still changing and, in this case, returning -EAGAIN it's the correct way. This behaviour is due to the sequentially read of c1 and c0 channel: we can do nothing to improve it through the driver.

I suggest, like you said, just to replace the -1 return value with -EAGAIN.

If you are in accord with this, I can pass you a new patch.


--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux