Hi Rodolfo,
On Tue, 19 Jun 2007 14:10:20 +0200, Rodolfo Giometti wrote:
> Add support for Taos TSL2550 ambient light sensors.
> (http://www.taosinc.com/product_detail.asp?cateid=4&proid=18).
>
> Signed-off-by: Rodolfo Giometti <[email protected]>
Did you ever read my review of your driver?
http://lists.lm-sensors.org/pipermail/i2c/2007-February/000824.html
I'm asking because you never replied and I don't see any of my
suggestions implemented in this new version of your driver.
> diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
> index ea085a0..b59c013 100644
> --- a/drivers/i2c/chips/Kconfig
> +++ b/drivers/i2c/chips/Kconfig
> @@ -124,4 +124,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
You can now omit "I2C", it's handled at menu level.
> + 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
> +static struct i2c_driver tsl2550_driver;
> +static int __devinit tsl2550_probe(struct i2c_client *client)
> +{
> + struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
> + struct tsl2550_data *data;
> + int *opmode, err = 0;
> +
> + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE
> + | I2C_FUNC_SMBUS_WRITE_BYTE_DATA)) {
> + err = -EIO;
> + goto exit;
> + }
> +
> + data = kzalloc(sizeof(struct tsl2550_data), GFP_KERNEL);
> + if (!data) {
> + err = -ENOMEM;
> + goto exit;
> + }
> + data->client = client;
> + i2c_set_clientdata(client, data);
> +
> + /* Check platform data */
> + opmode = client->dev.platform_data;
> + if (opmode) {
> + if (*opmode < 0 || *opmode > 1) {
> + dev_err(&client->dev, "invalid operating_mode (%d)\n",
> + *opmode);
> + err = -EINVAL;
> + goto exit_kfree;
> + }
> + data->operating_mode = *opmode;
> + } else
> + data->operating_mode = 0; /* default mode is standard */
> + dev_info(&client->dev, "%s operating mode\n",
> + data->operating_mode ? "extended" : "standard");
> +
> + /*
> + * Probe the chip. To do so we try to power up the device and then to
> + * read back the 0x03 code
> + */
> + err = i2c_smbus_write_byte(client, TSL2550_POWER_UP);
> + if (err < 0)
> + goto exit_kfree;
> + mdelay(1);
> + err = i2c_smbus_read_byte(client);
> + if (err != TSL2550_POWER_UP) {
> + err = -ENODEV;
> + goto exit_kfree;
> + }
> +
> + mutex_init(&data->update_lock);
> +
> + /* Initialize the TSL2550 chip */
> + tsl2550_init_client(client);
> +
> + /* Register sysfs hooks */
> + err = sysfs_create_group(&client->dev.kobj, &tsl2550_attr_group);
> + if (err)
> + goto exit_kfree;
> +
> + dev_info(&client->dev,
> + "TSL2550 I2C support enabled - ver. %s\n", DRIVER_VERSION);
> + dev_info(&client->dev,
> + "Copyright (C) 2007 Rodolfo Giometti <[email protected]>\n");
> +
> + return 0;
> +
> +exit_kfree:
> + kfree(data);
A call to "i2c_set_clientdata(client, NULL)" at this point would be
welcome.
> +exit:
> + return err;
> +}
> +
> +static int __devexit tsl2550_remove(struct i2c_client *client)
> +{
> + sysfs_remove_group(&client->dev.kobj, &tsl2550_attr_group);
> +
> + /* Power doen the device */
> + tsl2550_set_power_state(client, 0);
> +
> + kfree(i2c_get_clientdata(client));
Same here.
> +
> + return 0;
> +}
Thanks,
--
Jean Delvare
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
