[PATCH] TSL2550 driver bugfix

[PATCH] TSL2550 driver bugfix

[Michele De Candia (VT)]

[-- Attachment #1: Type: text/plain, Size: 380 bytes --]

Hi all,

I've tested TSL2550 driver and I've found a bug: when light is off, 
returned value from tsl2550_calculate_lux function is -1 when it should 
be 0 (sensor correctly read that light was off).

I think the bug is that a zero c0 value (approximated value of ch0) is 
misinterpreted as an error. 

I'm attaching you a patch that fixes the bug.

Regards,
Michele Jr De Candia

[-- Attachment #2: tsl2550_nolight_bugfix.patch --]
[-- Type: text/x-patch, Size: 722 bytes --]

Signed-off-by: Michele Jr De Candia <michele.decandia-EZxuzQJkuwwybS5Ee8rs3A@public.gmane.org>

diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
index 1a9cc13..6bad072 100644
--- a/drivers/i2c/chips/tsl2550.c
+++ b/drivers/i2c/chips/tsl2550.c
@@ -189,10 +189,16 @@ static int tsl2550_calculate_lux(u8 ch0, u8 ch1)
 	u8 r = 128;
 
 	/* Avoid division by 0 and count 1 cannot be greater than count 0 */
-	if (c0 && (c1 <= c0))
-		r = c1 * 128 / c0;
-	else
-		return -1;
+	if (c0)	{
+		if (c1 <= c0)
+			r = c1 * 128 / c0;
+		else
+			return -1;
+			
+		/* Calculate LUX */
+		lux = ((c0 - c1) * ratio_lut[r]) / 256;
+	}
+	else lux = 0;
 
 	/* Calculate LUX */
 	lux = ((c0 - c1) * ratio_lut[r]) / 256;

Re: [PATCH] TSL2550 driver bugfix

[Jonathan Cameron]

Michele De Candia (VT) wrote:
> Hi all,
> 
> I've tested TSL2550 driver and I've found a bug: when light is off,
> returned value from tsl2550_calculate_lux function is -1 when it should
> be 0 (sensor correctly read that light was off).
> 
> I think the bug is that a zero c0 value (approximated value of ch0) is
> misinterpreted as an error.
> I'm attaching you a patch that fixes the bug.
> 
> Regards,
> Michele Jr De Candia
Sounds reasonable, but I think a stray line got away in your patch (see below)
Not to mention, if the c1 <= c0 check is still valid, should you not also
confirm that c1 == 0 as well? Perhaps reverse the ordering?
if (c1 <= c0)
	if(c0) {
		r = c1* 128 / c0;
		lux = ((c0 - c1) * ratio_lut[r]) / 256;
	} else
		lux = 0;
else
	return -1;
	

Signed-off-by: Michele Jr De Candia <michele.decandia-EZxuzQJkuwwybS5Ee8rs3A@public.gmane.org>

diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
index 1a9cc13..6bad072 100644
--- a/drivers/i2c/chips/tsl2550.c
+++ b/drivers/i2c/chips/tsl2550.c
@@ -189,10 +189,16 @@ static int tsl2550_calculate_lux(u8 ch0, u8 ch1)
 	u8 r = 128;
 
 	/* Avoid division by 0 and count 1 cannot be greater than count 0 */
-	if (c0 && (c1 <= c0))
-		r = c1 * 128 / c0;
-	else
-		return -1;
+	if (c0)	{
+		if (c1 <= c0)
+			r = c1 * 128 / c0;
+		else
+			return -1;
+			
+		/* Calculate LUX */
+		lux = ((c0 - c1) * ratio_lut[r]) / 256;
+	}
+	else lux = 0;
 

This last line should have been removed I think?
 	/* Calculate LUX */
	lux = ((c0 - c1) * ratio_lut[r]) / 256;

---
Jonathan Cameron

Re: [PATCH] TSL2550 driver bugfix

[Michele De Candia (VT)]

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

Jonathan Cameron wrote:
> Michele De Candia (VT) wrote:
>   
>> Hi all,
>>
>> I've tested TSL2550 driver and I've found a bug: when light is off,
>> returned value from tsl2550_calculate_lux function is -1 when it should
>> be 0 (sensor correctly read that light was off).
>>
>> I think the bug is that a zero c0 value (approximated value of ch0) is
>> misinterpreted as an error.
>> I'm attaching you a patch that fixes the bug.
>>
>> Regards,
>> Michele Jr De Candia
>>     
> Sounds reasonable, but I think a stray line got away in your patch (see below)
> Not to mention, if the c1 <= c0 check is still valid, should you not also
> confirm that c1 == 0 as well? Perhaps reverse the ordering?
>   
That's right. A new patch can be found in attachment.
> if (c1 <= c0)
> 	if(c0) {
> 		r = c1* 128 / c0;
> 		lux = ((c0 - c1) * ratio_lut[r]) / 256;
> 	} else
> 		lux = 0;
> else
> 	return -1;
> 	
>
> Signed-off-by: Michele Jr De Candia <michele.decandia-EZxuzQJkuwwybS5Ee8rs3A@public.gmane.org>
>
> diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
> index 1a9cc13..6bad072 100644
> --- a/drivers/i2c/chips/tsl2550.c
> +++ b/drivers/i2c/chips/tsl2550.c
> @@ -189,10 +189,16 @@ static int tsl2550_calculate_lux(u8 ch0, u8 ch1)
>  	u8 r = 128;
>  
>  	/* Avoid division by 0 and count 1 cannot be greater than count 0 */
> -	if (c0 && (c1 <= c0))
> -		r = c1 * 128 / c0;
> -	else
> -		return -1;
> +	if (c0)	{
> +		if (c1 <= c0)
> +			r = c1 * 128 / c0;
> +		else
> +			return -1;
> +			
> +		/* Calculate LUX */
> +		lux = ((c0 - c1) * ratio_lut[r]) / 256;
> +	}
> +	else lux = 0;
>  
>
> This last line should have been removed I think?
>  	/* Calculate LUX */
> 	lux = ((c0 - c1) * ratio_lut[r]) / 256;
>
> ---
> Jonathan Cameron
>
>   
Than

[-- Attachment #2: tsl2550_nolight_bugfix.patch --]
[-- Type: text/x-patch, Size: 786 bytes --]

Signed-off-by: Michele Jr De Candia <michele.decandia-EZxuzQJkuwwybS5Ee8rs3A@public.gmane.org>

diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
index 1a9cc13..c4e1104 100644
--- a/drivers/i2c/chips/tsl2550.c
+++ b/drivers/i2c/chips/tsl2550.c
@@ -189,14 +189,17 @@ static int tsl2550_calculate_lux(u8 ch0, u8 ch1)
 	u8 r = 128;
 
 	/* Avoid division by 0 and count 1 cannot be greater than count 0 */
-	if (c0 && (c1 <= c0))
-		r = c1 * 128 / c0;
+	if (c1 <= c0)
+		if (c0) {
+			r = c1 * 128 / c0;
+
+			/* Calculate LUX */
+			lux = ((c0 - c1) * ratio_lut[r]) / 256;
+		} else
+			lux = 0;
 	else
 		return -1;
 
-	/* Calculate LUX */
-	lux = ((c0 - c1) * ratio_lut[r]) / 256;
-
 	/* LUX range check */
 	return lux > TSL2550_MAX_LUX ? TSL2550_MAX_LUX : lux;
 }

Re: [PATCH] TSL2550 driver bugfix

[Jonathan Cameron]

Looks good to me. (perhaps a bit more detail in the commit
message of the patch?)

Acked-by: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org>
> Jonathan Cameron wrote:
>> Michele De Candia (VT) wrote:
>>  
>>> Hi all,
>>>
>>> I've tested TSL2550 driver and I've found a bug: when light is off,
>>> returned value from tsl2550_calculate_lux function is -1 when it should
>>> be 0 (sensor correctly read that light was off).
>>>
>>> I think the bug is that a zero c0 value (approximated value of ch0) is
>>> misinterpreted as an error.
>>> I'm attaching you a patch that fixes the bug.
>>>
>>> Regards,
>>> Michele Jr De Candia
>>>     
>> Sounds reasonable, but I think a stray line got away in your patch
>> (see below)
>> Not to mention, if the c1 <= c0 check is still valid, should you not also
>> confirm that c1 == 0 as well? Perhaps reverse the ordering?
>>   
> That's right. A new patch can be found in attachment.
>> if (c1 <= c0)
>>     if(c0) {
>>         r = c1* 128 / c0;
>>         lux = ((c0 - c1) * ratio_lut[r]) / 256;
>>     } else
>>         lux = 0;
>> else
>>     return -1;
>>     
>>
>> Signed-off-by: Michele Jr De Candia <michele.decandia-EZxuzQJkuwwybS5Ee8rs3A@public.gmane.org>
>>
>> diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
>> index 1a9cc13..6bad072 100644
>> --- a/drivers/i2c/chips/tsl2550.c
>> +++ b/drivers/i2c/chips/tsl2550.c
>> @@ -189,10 +189,16 @@ static int tsl2550_calculate_lux(u8 ch0, u8 ch1)
>>      u8 r = 128;
>>  
>>      /* Avoid division by 0 and count 1 cannot be greater than count 0 */
>> -    if (c0 && (c1 <= c0))
>> -        r = c1 * 128 / c0;
>> -    else
>> -        return -1;
>> +    if (c0)    {
>> +        if (c1 <= c0)
>> +            r = c1 * 128 / c0;
>> +        else
>> +            return -1;
>> +           
>> +        /* Calculate LUX */
>> +        lux = ((c0 - c1) * ratio_lut[r]) / 256;
>> +    }
>> +    else lux = 0;
>>  
>>
>> This last line should have been removed I think?
>>      /* Calculate LUX */
>>     lux = ((c0 - c1) * ratio_lut[r]) / 256;
>>
>> ---
>> Jonathan Cameron
>>
>>   
> Than

Re: [PATCH] TSL2550 driver bugfix

[Jonathan Cameron]

> What do you think about 'Bugfix for missing Lux returning in dark
> environment' ?
That covers the key point, so fine.
 
> Jonathan Cameron wrote:
>> Looks good to me. (perhaps a bit more detail in the commit
>> message of the patch?)
>>
>> Acked-by: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org>
>>  
>>> Jonathan Cameron wrote:
>>>    
>>>> Michele De Candia (VT) wrote:
>>>>  
>>>>      
>>>>> Hi all,
>>>>>
>>>>> I've tested TSL2550 driver and I've found a bug: when light is off,
>>>>> returned value from tsl2550_calculate_lux function is -1 when it
>>>>> should
>>>>> be 0 (sensor correctly read that light was off).
>>>>>
>>>>> I think the bug is that a zero c0 value (approximated value of ch0) is
>>>>> misinterpreted as an error.
>>>>> I'm attaching you a patch that fixes the bug.
>>>>>
>>>>> Regards,
>>>>> Michele Jr De Candia
>>>>>             
>>>> Sounds reasonable, but I think a stray line got away in your patch
>>>> (see below)
>>>> Not to mention, if the c1 <= c0 check is still valid, should you not
>>>> also
>>>> confirm that c1 == 0 as well? Perhaps reverse the ordering?
>>>>         
>>> That's right. A new patch can be found in attachment.
>>>    
>>>> if (c1 <= c0)
>>>>     if(c0) {
>>>>         r = c1* 128 / c0;
>>>>         lux = ((c0 - c1) * ratio_lut[r]) / 256;
>>>>     } else
>>>>         lux = 0;
>>>> else
>>>>     return -1;
>>>>    
>>>> Signed-off-by: Michele Jr De Candia <michele.decandia-EZxuzQJkuwwybS5Ee8rs3A@public.gmane.org>
>>>>
>>>> diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
>>>> index 1a9cc13..6bad072 100644
>>>> --- a/drivers/i2c/chips/tsl2550.c
>>>> +++ b/drivers/i2c/chips/tsl2550.c
>>>> @@ -189,10 +189,16 @@ static int tsl2550_calculate_lux(u8 ch0, u8 ch1)
>>>>      u8 r = 128;
>>>>  
>>>>      /* Avoid division by 0 and count 1 cannot be greater than count
>>>> 0 */
>>>> -    if (c0 && (c1 <= c0))
>>>> -        r = c1 * 128 / c0;
>>>> -    else
>>>> -        return -1;
>>>> +    if (c0)    {
>>>> +        if (c1 <= c0)
>>>> +            r = c1 * 128 / c0;
>>>> +        else
>>>> +            return -1;
>>>> +           +        /* Calculate LUX */
>>>> +        lux = ((c0 - c1) * ratio_lut[r]) / 256;
>>>> +    }
>>>> +    else lux = 0;
>>>>  
>>>>
>>>> This last line should have been removed I think?
>>>>      /* Calculate LUX */
>>>>     lux = ((c0 - c1) * ratio_lut[r]) / 256;
>>>>
>>>> ---
>>>> Jonathan Cameron
>>>>
>>>>         
>>> Than
>>>     
>>
>>   
> 
> 

Re: [PATCH] TSL2550 driver bugfix

[Jean Delvare]

Hi Michele, Jonathan,

Sorry for the late answer, I had a lot of work this week.

On Wed, 01 Jul 2009 10:12:33 +0200, Michele De Candia (VT) wrote:
> Signed-off-by: Michele Jr De Candia <michele.decandia-EZxuzQJkuwwybS5Ee8rs3A@public.gmane.org>
> 
> diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
> index 1a9cc13..c4e1104 100644
> --- a/drivers/i2c/chips/tsl2550.c
> +++ b/drivers/i2c/chips/tsl2550.c
> @@ -189,14 +189,17 @@ static int tsl2550_calculate_lux(u8 ch0, u8 ch1)
>  	u8 r = 128;
>  
>  	/* Avoid division by 0 and count 1 cannot be greater than count 0 */
> -	if (c0 && (c1 <= c0))
> -		r = c1 * 128 / c0;
> +	if (c1 <= c0)
> +		if (c0) {
> +			r = c1 * 128 / c0;
> +
> +			/* Calculate LUX */
> +			lux = ((c0 - c1) * ratio_lut[r]) / 256;
> +		} else
> +			lux = 0;
>  	else
>  		return -1;
>  
> -	/* Calculate LUX */
> -	lux = ((c0 - c1) * ratio_lut[r]) / 256;
> -
>  	/* LUX range check */
>  	return lux > TSL2550_MAX_LUX ? TSL2550_MAX_LUX : lux;
>  }

The driver does indeed not handle the dark room case properly, and your
fix is correct. But I think it is insufficient.

For one thing, returning -1 on error is not a good idea. -1 is -EPERM,
resulting in a confusing error message for the user. We should come up
with a better error code. Maybe -EAGAIN?

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. 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.

-- 
Jean Delvare

Re: [PATCH] TSL2550 driver bugfix

[Jean Delvare]

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.

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.

-- 
Jean Delvare

Re: [PATCH] TSL2550 driver bugfix

[Michele De Candia (VT)]

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.

Re: [PATCH] TSL2550 driver bugfix

[Jean Delvare]

Hi Michele,

On Mon, 13 Jul 2009 09:56:10 +0200, Michele De Candia (VT) wrote:
> 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.

Yes I agree, please send an updated patch and I'll apply it.

In the meantime I've improved the performance of the i2c-taos-evm
driver I'm using for the TSL5220, I'll post it later today, once I am
fully done with testing. I'm not sure if there are other users of this
driver though.

Thanks,
-- 
Jean Delvare

Re: [PATCH] TSL2550 driver bugfix

[Rodolfo Giometti]

On Mon, Jul 13, 2009 at 11:40:47AM +0200, Michele De Candia (VT) wrote:
> Signed-off-by: Michele Jr De Candia <michele.decandia-EZxuzQJkuwwybS5Ee8rs3A@public.gmane.org>
> 
> diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
> index 1a9cc13..561d7bf 100644
> --- a/drivers/i2c/chips/tsl2550.c
> +++ b/drivers/i2c/chips/tsl2550.c
> @@ -27,7 +27,7 @@
>  #include <linux/delay.h>
>  
>  #define TSL2550_DRV_NAME	"tsl2550"
> -#define DRIVER_VERSION		"1.1.1"
> +#define DRIVER_VERSION		"1.1.2"
>  
>  /*
>   * Defines
> @@ -189,13 +189,16 @@ static int tsl2550_calculate_lux(u8 ch0, u8 ch1)
>  	u8 r = 128;
>  
>  	/* Avoid division by 0 and count 1 cannot be greater than count 0 */
> -	if (c0 && (c1 <= c0))
> -		r = c1 * 128 / c0;
> +	if (c1 <= c0)
> +		if (c0) {
> +			r = c1 * 128 / c0;
> +
> +			/* Calculate LUX */
> +			lux = ((c0 - c1) * ratio_lut[r]) / 256;
> +		} else
> +			lux = 0;
>  	else
> -		return -1;
> -
> -	/* Calculate LUX */
> -	lux = ((c0 - c1) * ratio_lut[r]) / 256;
> +		return -EAGAIN;
>  
>  	/* LUX range check */
>  	return lux > TSL2550_MAX_LUX ? TSL2550_MAX_LUX : lux;

Acked-by: Rodolfo Giometti <giometti-k2GhghHVRtY@public.gmane.org>

-- 

GNU/Linux Solutions                  e-mail: giometti-AVVDYK/kqiJWk0Htik3J/w@public.gmane.org
Linux Device Driver                          giometti-k2GhghHVRtY@public.gmane.org
Embedded Systems                     phone:  +39 349 2432127
UNIX programming                     skype:  rodolfo.giometti

Re: [PATCH] TSL2550 driver bugfix

[Jean Delvare]

On Mon, 13 Jul 2009 11:40:47 +0200, Michele De Candia (VT) wrote:
> >> If you are in accord with this, I can pass you a new patch.
> >
> > Yes I agree, please send an updated patch and I'll apply it.
> >
> > In the meantime I've improved the performance of the i2c-taos-evm
> > driver I'm using for the TSL5220, I'll post it later today, once I am
> > fully done with testing. I'm not sure if there are other users of this
> > driver though.
> >
> > Thanks,
> >   
> You can find the patch in attachment. Maybe I'll submit you more patches 
> about this driver but they are not ready yet (extended mode support and 
> spurious values filtering).

Patch applied, thanks.

-- 
Jean Delvare