[PATCH] TSL2550: extended mode bugfix

[PATCH] TSL2550: extended mode bugfix

[Michele De Candia (VT)]

Hi all,
according to the TAOS Application Note 'Controlling a Backlight with the
TSL2550 Ambient Light Sensor' (page 14), the actual lux value in
extended mode should be obtained multiplying the calculated lux value by 5.
The following patch should fix the lux calculation in this case.

Regards,
Michele De Candia
--

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

drivers/i2c/chips/tsl2550.c |    7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
index b96f302..04b87e0 100644
--- a/drivers/i2c/chips/tsl2550.c
+++ b/drivers/i2c/chips/tsl2550.c
@@ -279,7 +279,7 @@ static ssize_t __tsl2550_show_lux(struct i2c_client
*client, char *buf)
{
     u8 ch0, ch1;
     int ret;
-
+    struct tsl2550_data *data = i2c_get_clientdata(client);
     ret = tsl2550_get_adc_value(client, TSL2550_READ_ADC0);
     if (ret < 0)
         return ret;
@@ -293,7 +293,10 @@ static ssize_t __tsl2550_show_lux(struct i2c_client
*client, char *buf)
     ch1 = ret;

     /* Do the job */
-    ret = tsl2550_calculate_lux(ch0, ch1);
+    if (!data->operating_mode)
+        ret = tsl2550_calculate_lux(ch0, ch1);
+    else
+        ret = tsl2550_calculate_lux(ch0, ch1) * 5;
     if (ret < 0)
         return ret;

Re: [PATCH] TSL2550: extended mode bugfix

[Jean Delvare]

Hi Michele,

On Wed, 05 Aug 2009 17:08:05 +0200, Michele De Candia (VT) wrote:
> Hi all,
> according to the TAOS Application Note 'Controlling a Backlight with the
> TSL2550 Ambient Light Sensor' (page 14), the actual lux value in
> extended mode should be obtained multiplying the calculated lux value by 5.
> The following patch should fix the lux calculation in this case.
> 
> Regards,
> Michele De Candia
> --
> 
> Signed-off-by: Michele Jr De Candia <michele.decandia-EZxuzQJkuwwybS5Ee8rs3A@public.gmane.org>
> 
> drivers/i2c/chips/tsl2550.c |    7 +++++--
> 1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/i2c/chips/tsl2550.c
> index b96f302..04b87e0 100644
> --- a/drivers/i2c/chips/tsl2550.c
> +++ b/drivers/i2c/chips/tsl2550.c
> @@ -279,7 +279,7 @@ static ssize_t __tsl2550_show_lux(struct i2c_client
> *client, char *buf)
> {
>      u8 ch0, ch1;

I don't know what your e-mail client is doing to the source code, but
it's bad. It replaces tabs with spaces and folds long lines at least -
I won't be able to apply your patch. Please either fix your client, or
attach the patch instead of inlining it.

>      int ret;
> -
> +    struct tsl2550_data *data = i2c_get_clientdata(client);
>      ret = tsl2550_get_adc_value(client, TSL2550_READ_ADC0);
>      if (ret < 0)
>          return ret;
> @@ -293,7 +293,10 @@ static ssize_t __tsl2550_show_lux(struct i2c_client
> *client, char *buf)
>      ch1 = ret;
> 
>      /* Do the job */
> -    ret = tsl2550_calculate_lux(ch0, ch1);
> +    if (!data->operating_mode)
> +        ret = tsl2550_calculate_lux(ch0, ch1);
> +    else
> +        ret = tsl2550_calculate_lux(ch0, ch1) * 5;
>      if (ret < 0)
>          return ret;

Operating on values before you check for errors is wrong. I'd rather do:

    ret = tsl2550_calculate_lux(ch0, ch1);
    if (ret < 0)
        return ret;
    if (data->operating_mode == 1) /* extended mode */
        ret *= 5;

OK?

-- 
Jean Delvare

Re: [PATCH] TSL2550: extended mode bugfix

[Wolfram Sang]

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

> Operating on values before you check for errors is wrong. I'd rather do:
> 
>     ret = tsl2550_calculate_lux(ch0, ch1);
>     if (ret < 0)
>         return ret;
>     if (data->operating_mode == 1) /* extended mode */
>         ret *= 5;

I just wanted to suggest the same :) Pity that "operating_mode" is exported to
sysfs, "extended_mode" would have been a better name, me thinks...

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] TSL2550: extended mode bugfix

[Wolfram Sang]

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

> About the name of 'operating_mode' field, I think it can be changed in  
> 'extended_mode' as Wolfram Sang has suggested.
> What do you think about it?

Nope, because it is exported to userspace via sysfs, this is not possible.
There may be already software depending on this name. And to have internally
one name and externally another won't help reducing the confusion :) Having an
extra comment as Jean suggested is the way to go.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] TSL2550: extended mode bugfix

[Jean Delvare]

Hi Michele,

On Wed, 05 Aug 2009 17:53:25 +0200, Michele De Candia (VT) wrote:
> Hi Jean,
> in attachment you can find the patch with your suggestions applied.

It seems this fix somehow felt through the cracks. Sorry about that,
I'm applying it now.

-- 
Jean Delvare