Skip to contentSkip to conversation

TSL2550: extended mode bugfix

PATCH In review Michele De Candia (VT) 3 participants 5 messages Aug 5, 2009 – Nov 9, 2009 4 messages not in the archive I2C subsystem host drivers

lore
Share & tools
lore

Michele De Candia (VT)

author
#0 lore raw

PATCH 1/1 TSL2550: extended mode bugfix

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 <[email protected]>

drivers/i2c/chips/tsl2550.c | 7 +++++--

1 files changed, 5 insertions(+), 2 deletions(-)
drivers/i2c/chips/tsl2550.c +0 -0
index b96f302..04b87e0 100644
@@ -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;

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw

Hi Michele,

Quoted 20 lines · 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 <[email protected]>

drivers/i2c/chips/tsl2550.c | 7 +++++--

1 files changed, 5 insertions(+), 2 deletions(-)
drivers/i2c/chips/tsl2550.c +0 -0
index b96f302..04b87e0 100644

drivers/i2c/chips/tsl2550.c static ssize_t __tsl2550_show_lux(struct i2c_client · line 281

Quoted 3 lines
drivers/i2c/chips/tsl2550.c +0 -0
@@ -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.

Quoted 8 lines
drivers/i2c/chips/tsl2550.c +1 -1
     int ret;
-
+    struct tsl2550_data *data = i2c_get_clientdata(client);
     ret = tsl2550_get_adc_value(client, TSL2550_READ_ADC0);
     if (ret < 0)

         return ret;
drivers/i2c/chips/tsl2550.c +0 -0
@@ -293,7 +293,10 @@ static ssize_t __tsl2550_show_lux(struct i2c_client
 *client, char *buf)

drivers/i2c/chips/tsl2550.c static ssize_t __tsl2550_show_lux(struct i2c_client · line 302

Quoted 10 lines
drivers/i2c/chips/tsl2550.c +4 -1
     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?

Signature · 1 line
Jean Delvare

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw
Quoted 7 lines

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

Signature · 2 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#7 lore raw
Quoted 3 lines

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

Signature · 2 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
#8 lore raw
In reply to a message not in the archive

Hi Michele,

Quoted 2 lines · Michele De Candia (VT)

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.

Signature · 1 line
Jean Delvare