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/
