Skip to contentSkip to conversation

TSL2550 support (I2C device driver)

PATCH In review Rodolfo Giometti 2 participants 6 messages Jan 29, 2007 – Jan 30, 2007

lore
Share & tools
lore

Rodolfo Giometti

author
#0 lore raw

PATCH 1/1 TSL2550 support (I2C device driver)

Hello,

attached a patch to add support for Taos TSL2550 ambient light sensors
(http://www.taosinc.com/product_detail.asp?cateid=4&proid=18).

Ciao,

Rodolfo

Signature · 4 lines
GNU/Linux Solutions                  e-mail:    [email protected]
Linux Device Driver                             [email protected]
Embedded Systems                     		[email protected]
UNIX programming                     phone:     +39 349 2432127
drivers/i2c/chips/Kconfig +10 -0
index 87ee3ce..18e60ec 100644
@@ -125,4 +125,14 @@ config SENSORS_MAX6875
 	  This driver can also be built as a module.  If so, the module
 	  will be called max6875.
 
+config SENSORS_TSL2550
+	tristate "Taos TSL2550 ambient light sensor"
+	depends on I2C && EXPERIMENTAL
+	help
+	  If you say yes here you get support for the Taos TSL2550
+	  ambient light sensor.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called tsl2550.
+
 endmenu
drivers/i2c/chips/Makefile +1 -0
index 779868e..1ef6a04 100644
@@ -12,6 +12,7 @@ obj-$(CONFIG_SENSORS_PCF8574)	+= pcf8574.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_ISP1301_OMAP)	+= isp1301_omap.o
 obj-$(CONFIG_TPS65010)		+= tps65010.o
+obj-$(CONFIG_SENSORS_TSL2550)	+= tsl2550.o
 
 ifeq ($(CONFIG_I2C_DEBUG_CHIP),y)
 EXTRA_CFLAGS += -DDEBUG
drivers/i2c/chips/tsl2550.c new file +445 -0

445 lines not on this page · raw message

include/linux/i2c-id.h +1 -0
index 0a8f750..83a9ab6 100644
@@ -157,6 +157,7 @@
 #define I2C_DRIVERID_FSCHER 1046
 #define I2C_DRIVERID_W83L785TS 1047
 #define I2C_DRIVERID_OV7670 1048	/* Omnivision 7670 camera */
+#define I2C_DRIVERID_TSL2550 1049
 
 /*
  * ---- Adapter types ----------------------------------------------------
#1 lore raw
Quoted 5 lines · On Tue, 30 Jan 2007 00:56:19 +0100 Rodolfo Giometti <[email protected]>…

Hello,

attached a patch to add support for Taos TSL2550 ambient light sensors
(http://www.taosinc.com/product_detail.asp?cateid=4&proid=18).

Some minor things:

Quoted 4 lines
drivers/i2c/chips/tsl2550.c new file +4 -0
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>

drivers/i2c/chips/tsl2550.c line 35

Quoted 10 lines
drivers/i2c/chips/tsl2550.c new file +10 -0
+#include <linux/mutex.h>
+#include <linux/delay.h>
+
+/* Address to scan */
+static unsigned short normal_i2c[] = { 0x39, I2C_CLIENT_END };
+
+/* Insmod parameters */
+I2C_CLIENT_INSMOD_1(tsl2550);
+
+static int operating_mode = 0;

The `= 0' is unneeded and undesirable.

Quoted 12 lines

...

drivers/i2c/chips/tsl2550.c new file +10 -0
+static int tsl2550_get_adc_value(struct i2c_client *client, int channel)
+{
+	u8 cmd = channel == 0 ? TSL2550_READ_ADC0 : TSL2550_READ_ADC1;
+	int timeout, ret;
+
+	/* Read ADC channel waiting at most 400ms (see data sheet for further
+         * info) */
+	for (timeout = 400; timeout > 0; timeout--) {
+		i2c_smbus_write_byte(client, cmd);
+		mdelay(1);

drivers/i2c/chips/tsl2550.c line 125

Quoted 10 lines
drivers/i2c/chips/tsl2550.c new file +10 -0
+		ret = i2c_smbus_read_byte(client);
+		if (ret < 0)
+			return ret;
+		else if (ret & 0x0080)
+			break;
+	}
+	if (timeout == 0)
+		return -EIO;
+	return ((u8) ret) & 0x7f;	/* remove the "valid" bit */
+}

eek. Is there no way to avoid the busy-wait? We cannot sleep here?

Quoted 32 lines
drivers/i2c/chips/tsl2550.c new file +32 -0
+/* --- LUX calculation ----------------------------------------------------- */
+
+#define	TSL2550_MAX_LUX		1846
+
+static u8 ratio_lut[] = {
+	100, 100, 100, 100, 100, 100, 100, 100, 
+	100, 100, 100, 100, 100, 100, 99, 99, 
+	99, 99, 99, 99, 99, 99, 99, 99, 
+	99, 99, 99, 98, 98, 98, 98, 98, 
+	98, 98, 97, 97, 97, 97, 97, 96, 
+	96, 96, 96, 95, 95, 95, 94, 94, 
+	93, 93, 93, 92, 92, 91, 91, 90, 
+	89, 89, 88, 87, 87, 86, 85, 84, 
+	83, 82, 81, 80, 79, 78, 77, 75, 
+	74, 73, 71, 69, 68, 66, 64, 62, 
+	60, 58, 56, 54, 52, 49, 47, 44, 
+	42, 41, 40, 40, 39, 39, 38, 38, 
+	37, 37, 37, 36, 36, 36, 35, 35, 
+	35, 35, 34, 34, 34, 34, 33, 33, 
+	33, 33, 32, 32, 32, 32, 32, 31, 
+	31, 31, 31, 31, 30, 30, 30, 30, 
+	30, 
+};
+
+static u16 count_lut[] = {
+	0, 1, 2, 3, 4, 5, 6, 7, 
+	8, 9, 10, 11, 12, 13, 14, 15, 
+	16, 18, 20, 22, 24, 26, 28, 30, 
+	32, 34, 36, 38, 40, 42, 44, 46, 
+	49, 53, 57, 61, 65, 69, 73, 77, 
+	81, 85, 89, 93, 97, 101, 105, 109, 
+	115, 123, 131, 139, 147, 155, 163, 171,

drivers/i2c/chips/tsl2550.c line 168

Quoted 10 lines
drivers/i2c/chips/tsl2550.c new file +10 -0
+	179, 187, 195, 203, 211, 219, 227, 235, 
+	247, 263, 279, 295, 311, 327, 343, 359, 
+	375,  391, 407, 423, 439, 455, 471, 487, 
+	511, 543, 575, 607, 639, 671, 703, 735, 
+	767, 799, 831, 863, 895, 927, 959, 991, 
+	1039, 1103, 1167, 1231, 1295, 1359, 1423, 1487, 
+	1551, 1615, 1679, 1743, 1807, 1871, 1935, 1999, 
+	2095, 2223, 2351, 2479, 2607, 2735, 2863, 2991, 
+	3119, 3247, 3375, 3503, 3631, 3759, 3887, 4015
+};

These tables could perhaps be const?

Quoted 1 line

+static ssize_t tsl2550_show_power_state(struct device *dev, struct device_attribute *attr, char *buf)

It's preferred to try to fit the code into an 80-col window. (But some people disagree with this specifically for function definitions such as this).

Quoted 1 line

+static ssize_t tsl2550_store_power_state(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)

Ditto (and other places).

drivers/i2c/chips/tsl2550.c line 405

Quoted 8 lines
drivers/i2c/chips/tsl2550.c new file +8 -0
+static int tsl2550_detach_client(struct i2c_client *client)
+{
+	int err;
+
+	sysfs_remove_group(&client->dev.kobj, &tsl2550_attr_group);
+
+	if ((err = i2c_detach_client(client)))
+		return err;

Preferred coding style is

	err = i2c_detach_client(client);
	if (err)
		return err;

(multiple places)

author
#3 lore raw
In reply to Andrew Morton
Quoted 6 lines · Andrew Morton

drivers/i2c/chips/tsl2550.c line 35

Quoted 4 lines
drivers/i2c/chips/tsl2550.c new file +4 -0
+/* Insmod parameters */
+I2C_CLIENT_INSMOD_1(tsl2550);
+
+static int operating_mode = 0;

The `= 0' is unneeded and undesirable.

Fixed.

Quoted 16 lines
Quoted 16 lines

...

drivers/i2c/chips/tsl2550.c new file +14 -0
+static int tsl2550_get_adc_value(struct i2c_client *client, int channel)
+{
+	u8 cmd = channel == 0 ? TSL2550_READ_ADC0 : TSL2550_READ_ADC1;
+	int timeout, ret;
+
+	/* Read ADC channel waiting at most 400ms (see data sheet for further
+         * info) */
+	for (timeout = 400; timeout > 0; timeout--) {
+		i2c_smbus_write_byte(client, cmd);
+		mdelay(1);
+		ret = i2c_smbus_read_byte(client);
+		if (ret < 0)
+			return ret;
+		else if (ret & 0x0080)
Quoted 8 lines

drivers/i2c/chips/tsl2550.c line 125

Quoted 6 lines
drivers/i2c/chips/tsl2550.c new file +6 -0
+			break;
+	}
+	if (timeout == 0)
+		return -EIO;
+	return ((u8) ret) & 0x7f;	/* remove the "valid" bit */
+}

eek. Is there no way to avoid the busy-wait? We cannot sleep here?

That's why I have to retry reading data for at most 400ms otherwise the chip will start a new ADC conversion.

I can replace mdelay(1) with schedule_timeout(HZ/1000) but doing this I'm not sure that just 1ms has elapesed. Also the chip has no irq lines to use for that.

Quoted 5 lines
Quoted 5 lines
drivers/i2c/chips/tsl2550.c new file +5 -0
+/* --- LUX calculation ----------------------------------------------------- */
+
+#define	TSL2550_MAX_LUX		1846
+
+static u8 ratio_lut[] = {
Quoted 8 lines

drivers/i2c/chips/tsl2550.c line 152

Quoted 6 lines
drivers/i2c/chips/tsl2550.c new file +4 -0
+	100, 100, 100, 100, 100, 100, 100, 100, 
[snip]
+
+static u16 count_lut[] = {
+	0, 1, 2, 3, 4, 5, 6, 7, 
[snip]

These tables could perhaps be const?

Fixed.

Quoted 5 lines
Quoted 1 line

+static ssize_t tsl2550_show_power_state(struct device *dev, struct device_attribute *attr, char *buf)

It's preferred to try to fit the code into an 80-col window. (But some people disagree with this specifically for function definitions such as this).

I agree with them. :)

Quoted 7 lines

Preferred coding style is

	err = i2c_detach_client(client);
	if (err)
		return err;

(multiple places)

Fixed.

I'm going to repost the patch after some tests.

Thanks,

Rodolfo

Signature · 4 lines
GNU/Linux Solutions                  e-mail:    [email protected]
Linux Device Driver                             [email protected]
Embedded Systems                     		[email protected]
UNIX programming                     phone:     +39 349 2432127
#4 lore raw
Quoted 23 lines · On Tue, 30 Jan 2007 11:26:42 +0100 Rodolfo Giometti <[email protected]>…
Quoted 14 lines
Quoted 14 lines
drivers/i2c/chips/tsl2550.c new file +14 -0
+static int tsl2550_get_adc_value(struct i2c_client *client, int channel)
+{
+	u8 cmd = channel == 0 ? TSL2550_READ_ADC0 : TSL2550_READ_ADC1;
+	int timeout, ret;
+
+	/* Read ADC channel waiting at most 400ms (see data sheet for further
+         * info) */
+	for (timeout = 400; timeout > 0; timeout--) {
+		i2c_smbus_write_byte(client, cmd);
+		mdelay(1);
+		ret = i2c_smbus_read_byte(client);
+		if (ret < 0)
+			return ret;
+		else if (ret & 0x0080)
Quoted 8 lines

drivers/i2c/chips/tsl2550.c line 125

Quoted 6 lines
drivers/i2c/chips/tsl2550.c new file +6 -0
+			break;
+	}
+	if (timeout == 0)
+		return -EIO;
+	return ((u8) ret) & 0x7f;	/* remove the "valid" bit */
+}

eek. Is there no way to avoid the busy-wait? We cannot sleep here?

Quoted 6 lines

That's why I have to retry reading data for at most 400ms otherwise the chip will start a new ADC conversion.

I can replace mdelay(1) with schedule_timeout(HZ/1000) but doing this I'm not sure that just 1ms has elapesed. Also the chip has no irq lines to use for that.

I expect an msleep(1) will be OK, if you can indeed sleep on this codepath. Yes, that can delay for longer: ten milliseconds for sure if HZ=100.

If that's a problem I guess you could spin for a millisecond or two, then start sleeping:

	end = jiffies + msecs_to_jiffies(400);
	while (time_before(jiffies, end)) {
		loop++;
		...
		if (loop < 5)
			mdelay(1);
		else
			msleep(1);
	}

or something like that.

author
#5 lore raw

PATCH 1/1 I2C: TSL2550 support

In reply to Andrew Morton

From: Rodolfo Giometti <[email protected]>

Patch to add support for Taos TSL2550 ambient light sensors.

Signed-off-by Rodolfo Giometti <[email protected]>

---

drivers/i2c/chips/Kconfig +10 -0
index 87ee3ce..18e60ec 100644
@@ -125,4 +125,14 @@ config SENSORS_MAX6875
 	  This driver can also be built as a module.  If so, the module
 	  will be called max6875.
 
+config SENSORS_TSL2550
+	tristate "Taos TSL2550 ambient light sensor"
+	depends on I2C && EXPERIMENTAL
+	help
+	  If you say yes here you get support for the Taos TSL2550
+	  ambient light sensor.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called tsl2550.
+
 endmenu
drivers/i2c/chips/Makefile +1 -0
index 779868e..1ef6a04 100644
@@ -12,6 +12,7 @@ obj-$(CONFIG_SENSORS_PCF8574)	+= pcf8574.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_ISP1301_OMAP)	+= isp1301_omap.o
 obj-$(CONFIG_TPS65010)		+= tps65010.o
+obj-$(CONFIG_SENSORS_TSL2550)	+= tsl2550.o
 
 ifeq ($(CONFIG_I2C_DEBUG_CHIP),y)
 EXTRA_CFLAGS += -DDEBUG
drivers/i2c/chips/tsl2550.c new file +457 -0

457 lines not on this page · raw message

include/linux/i2c-id.h +1 -0
index b2e83df..344c42c 100644
@@ -160,6 +160,7 @@
 #define I2C_DRIVERID_FSCHER 1046
 #define I2C_DRIVERID_W83L785TS 1047
 #define I2C_DRIVERID_OV7670 1048	/* Omnivision 7670 camera */
+#define I2C_DRIVERID_TSL2550 1049
 
 /*
  * ---- Adapter types ----------------------------------------------------