Prev: check_usage_backwards() && forwards? (Was: [2.6.33-rc5] starting emacs makes lockdep warning)
Next: [PATCH V2] tsl2550: Move form i2c/chips to als and update interfaces
From: Jonathan Cameron on 26 Jan 2010 13:50 Gah! Sorry all, I forgot to move the i2c/Makefile etc changes to the directory removal patch! New version in a few mins. > > Signed-off-by: Jonathan Cameron <jic23(a)cam.ac.uk> > --- > > Simple changes to meet all of Jean's comments. > > Made formatting changes, removed the defined maximum lux value > and separated out the crushing of drivers/i2c/chips. > > If Jean is happy with fixes I would propose taking this through the ALS tree. > > Attached is a full patch without git move detection. > > Documentation/ABI/testing/sysfs-class-als | 9 +++ > drivers/als/Kconfig | 14 ++++ > drivers/als/Makefile | 2 + > drivers/{i2c/chips => als}/tsl2550.c | 101 +++++++++++++++++----------- > drivers/i2c/Kconfig | 1 - > drivers/i2c/Makefile | 2 +- > drivers/i2c/chips/Kconfig | 10 --- > drivers/i2c/chips/Makefile | 2 - > 8 files changed, 87 insertions(+), 54 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-class-als b/Documentation/ABI/testing/sysfs-class-als > index d3b33f3..732f449 100644 > --- a/Documentation/ABI/testing/sysfs-class-als > +++ b/Documentation/ABI/testing/sysfs-class-als > @@ -7,3 +7,12 @@ Description: Current Ambient Light Illuminance reported by > Unit: lux (lumens per square meter) > RO > > +What: /sys/class/als/.../exposure_time[n] > +Date: Dec. 2009 > +KernelVersion: 2.6.32 > +Contact: Jonathan Cameron <jic23(a)cam.ac.uk> > +Description: Sensor exposure time. In some devices this > + corresponds to the combined time needed to > + to internally read several different sensors. > + Unit: microseconds > + RW > diff --git a/drivers/als/Kconfig b/drivers/als/Kconfig > index 200c52b..1564ffc 100644 > --- a/drivers/als/Kconfig > +++ b/drivers/als/Kconfig > @@ -8,3 +8,17 @@ menuconfig ALS > This framework provides a generic sysfs I/F for Ambient Light > Sensor devices. > If you want this support, you should say Y or M here. > + > +if ALS > + > +config ALS_TSL2550 > + tristate "Taos TSL2550 ambient light sensor" > + depends on EXPERIMENTAL && I2C > + 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. > + > +endif #ALS > diff --git a/drivers/als/Makefile b/drivers/als/Makefile > index a527197..7be5631 100644 > --- a/drivers/als/Makefile > +++ b/drivers/als/Makefile > @@ -3,3 +3,5 @@ > # > > obj-$(CONFIG_ALS) += als_sys.o > + > +obj-$(CONFIG_ALS_TSL2550) += tsl2550.o > \ No newline at end of file > diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/als/tsl2550.c > similarity index 81% > rename from drivers/i2c/chips/tsl2550.c > rename to drivers/als/tsl2550.c > index a0702f3..8d5f2a1 100644 > --- a/drivers/i2c/chips/tsl2550.c > +++ b/drivers/als/tsl2550.c > @@ -3,6 +3,7 @@ > * > * Copyright (C) 2007 Rodolfo Giometti <giometti(a)linux.it> > * Copyright (C) 2007 Eurotech S.p.A. <info(a)eurotech.it> > + * Copyright (C) 2009 Jonathan Cameron <jic23(a)cam.ac.uk> > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License as published by > @@ -24,9 +25,11 @@ > #include <linux/slab.h> > #include <linux/i2c.h> > #include <linux/mutex.h> > +#include <linux/err.h> > +#include <linux/als_sys.h> > > #define TSL2550_DRV_NAME "tsl2550" > -#define DRIVER_VERSION "1.2" > +#define DRIVER_VERSION "2.0" > > /* > * Defines > @@ -44,11 +47,12 @@ > */ > > struct tsl2550_data { > + struct device *classdev; > struct i2c_client *client; > struct mutex update_lock; > > - unsigned int power_state : 1; > - unsigned int operating_mode : 1; > + unsigned int power_state:1; > + unsigned int operating_mode:1; > }; > > /* > @@ -102,15 +106,16 @@ static int tsl2550_get_adc_value(struct i2c_client *client, u8 cmd) > return ret; > if (!(ret & 0x80)) > return -EAGAIN; > + if (ret == 0x7f) > + return -ERANGE; > return ret & 0x7f; /* remove the "valid" bit */ > } > > /* > - * LUX calculation > + * LUX calculation - note the range is dependent on combination > + * of infrared level and visible light levels. > */ > > -#define TSL2550_MAX_LUX 1846 > - > static const u8 ratio_lut[] = { > 100, 100, 100, 100, 100, 100, 100, 100, > 100, 100, 100, 100, 100, 100, 99, 99, > @@ -180,8 +185,7 @@ static int tsl2550_calculate_lux(u8 ch0, u8 ch1) > else > return -EAGAIN; > > - /* LUX range check */ > - return lux > TSL2550_MAX_LUX ? TSL2550_MAX_LUX : lux; > + return lux; > } > > /* > @@ -191,7 +195,8 @@ static int tsl2550_calculate_lux(u8 ch0, u8 ch1) > static ssize_t tsl2550_show_power_state(struct device *dev, > struct device_attribute *attr, char *buf) > { > - struct tsl2550_data *data = i2c_get_clientdata(to_i2c_client(dev)); > + struct i2c_client *client = to_i2c_client(dev->parent); > + struct tsl2550_data *data = i2c_get_clientdata(client); > > return sprintf(buf, "%u\n", data->power_state); > } > @@ -199,12 +204,12 @@ static ssize_t tsl2550_show_power_state(struct device *dev, > static ssize_t tsl2550_store_power_state(struct device *dev, > struct device_attribute *attr, const char *buf, size_t count) > { > - struct i2c_client *client = to_i2c_client(dev); > + struct i2c_client *client = to_i2c_client(dev->parent); > struct tsl2550_data *data = i2c_get_clientdata(client); > - unsigned long val = simple_strtoul(buf, NULL, 10); > - int ret; > + unsigned long val; > + int ret = strict_strtoul(buf, 10, &val); > > - if (val < 0 || val > 1) > + if (val < 0 || val > 1 || ret) > return -EINVAL; > > mutex_lock(&data->update_lock); > @@ -220,40 +225,45 @@ static ssize_t tsl2550_store_power_state(struct device *dev, > static DEVICE_ATTR(power_state, S_IWUSR | S_IRUGO, > tsl2550_show_power_state, tsl2550_store_power_state); > > -static ssize_t tsl2550_show_operating_mode(struct device *dev, > - struct device_attribute *attr, char *buf) > +static ssize_t tsl2550_show_exposure(struct device *dev, > + struct device_attribute *attr, > + char *buf) > { > - struct tsl2550_data *data = i2c_get_clientdata(to_i2c_client(dev)); > - > - return sprintf(buf, "%u\n", data->operating_mode); > + struct i2c_client *client = to_i2c_client(dev->parent); > + struct tsl2550_data *data = i2c_get_clientdata(client); > + if (data->operating_mode) > + return sprintf(buf, "160000\n"); > + else > + return sprintf(buf, "800000\n"); > } > > -static ssize_t tsl2550_store_operating_mode(struct device *dev, > - struct device_attribute *attr, const char *buf, size_t count) > +static ssize_t tsl2550_store_exposure(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t count) > { > - struct i2c_client *client = to_i2c_client(dev); > + struct i2c_client *client = to_i2c_client(dev->parent); > struct tsl2550_data *data = i2c_get_clientdata(client); > - unsigned long val = simple_strtoul(buf, NULL, 10); > - int ret; > - > - if (val < 0 || val > 1) > - return -EINVAL; > + unsigned long val; > > - if (data->power_state == 0) > - return -EBUSY; > + int ret = strict_strtoul(buf, 10, &val); > > + if (ret) > + return -EINVAL; > mutex_lock(&data->update_lock); > - ret = tsl2550_set_operating_mode(client, val); > + if (val >= 800000) > + ret = tsl2550_set_operating_mode(client, 0); > + else > + ret = tsl2550_set_operating_mode(client, 1); > mutex_unlock(&data->update_lock); > - > if (ret < 0) > return ret; > > return count; > } > > -static DEVICE_ATTR(operating_mode, S_IWUSR | S_IRUGO, > - tsl2550_show_operating_mode, tsl2550_store_operating_mode); > +static DEVICE_ATTR(exposure_time0, S_IWUSR | S_IRUGO, > + tsl2550_show_exposure, tsl2550_store_exposure); > > static ssize_t __tsl2550_show_lux(struct i2c_client *client, char *buf) > { > @@ -284,7 +294,7 @@ static ssize_t __tsl2550_show_lux(struct i2c_client *client, char *buf) > static ssize_t tsl2550_show_lux1_input(struct device *dev, > struct device_attribute *attr, char *buf) > { > - struct i2c_client *client = to_i2c_client(dev); > + struct i2c_client *client = to_i2c_client(dev->parent); > struct tsl2550_data *data = i2c_get_clientdata(client); > int ret; > > @@ -299,13 +309,13 @@ static ssize_t tsl2550_show_lux1_input(struct device *dev, > return ret; > } > > -static DEVICE_ATTR(lux1_input, S_IRUGO, > +static DEVICE_ATTR(illuminance0, S_IRUGO, > tsl2550_show_lux1_input, NULL); > > static struct attribute *tsl2550_attributes[] = { > &dev_attr_power_state.attr, > - &dev_attr_operating_mode.attr, > - &dev_attr_lux1_input.attr, > + &dev_attr_exposure_time0.attr, > + &dev_attr_illuminance0.attr, > NULL > }; > > @@ -391,14 +401,22 @@ static int __devinit tsl2550_probe(struct i2c_client *client, > goto exit_kfree; > > /* Register sysfs hooks */ > - err = sysfs_create_group(&client->dev.kobj, &tsl2550_attr_group); > - if (err) > + data->classdev = als_device_register(&client->dev); > + if (IS_ERR(data->classdev)) { > + err = PTR_ERR(data->classdev); > goto exit_kfree; > + } > + > + err = sysfs_create_group(&data->classdev->kobj, &tsl2550_attr_group); > + if (err) > + goto exit_unreg; > > dev_info(&client->dev, "support ver. %s enabled\n", DRIVER_VERSION); > > return 0; > > +exit_unreg: > + als_device_unregister(data->classdev); > exit_kfree: > kfree(data); > exit: > @@ -407,12 +425,15 @@ exit: > > static int __devexit tsl2550_remove(struct i2c_client *client) > { > - sysfs_remove_group(&client->dev.kobj, &tsl2550_attr_group); > + struct tsl2550_data *data = i2c_get_clientdata(client); > + > + sysfs_remove_group(&data->classdev->kobj, &tsl2550_attr_group); > + als_device_unregister(data->classdev); > > /* Power down the device */ > tsl2550_set_power_state(client, 0); > > - kfree(i2c_get_clientdata(client)); > + kfree(data); > > return 0; > } > diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig > index 8d8a00e..5c33a71 100644 > --- a/drivers/i2c/Kconfig > +++ b/drivers/i2c/Kconfig > @@ -63,7 +63,6 @@ config I2C_HELPER_AUTO > > source drivers/i2c/algos/Kconfig > source drivers/i2c/busses/Kconfig > -source drivers/i2c/chips/Kconfig > > config I2C_DEBUG_CORE > bool "I2C Core debugging messages" > diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile > index ba26e6c..ce5fd62 100644 > --- a/drivers/i2c/Makefile > +++ b/drivers/i2c/Makefile > @@ -5,7 +5,7 @@ > obj-$(CONFIG_I2C_BOARDINFO) += i2c-boardinfo.o > obj-$(CONFIG_I2C) += i2c-core.o > obj-$(CONFIG_I2C_CHARDEV) += i2c-dev.o > -obj-y += busses/ chips/ algos/ > +obj-y += busses/ algos/ > > ifeq ($(CONFIG_I2C_DEBUG_CORE),y) > EXTRA_CFLAGS += -DDEBUG > diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig > index ae4539d..c11f8d6 100644 > --- a/drivers/i2c/chips/Kconfig > +++ b/drivers/i2c/chips/Kconfig > @@ -6,14 +6,4 @@ > > menu "Miscellaneous I2C Chip support" > > -config SENSORS_TSL2550 > - tristate "Taos TSL2550 ambient light sensor" > - depends on 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 > diff --git a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile > index fe0af0f..ffde18d 100644 > --- a/drivers/i2c/chips/Makefile > +++ b/drivers/i2c/chips/Makefile > @@ -10,8 +10,6 @@ > # * I/O expander drivers go to drivers/gpio > # > > -obj-$(CONFIG_SENSORS_TSL2550) += tsl2550.o > - > ifeq ($(CONFIG_I2C_DEBUG_CHIP),y) > EXTRA_CFLAGS += -DDEBUG > endif -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jonathan Cameron on 27 Jan 2010 09:40
On 01/27/10 13:36, Jean Delvare wrote: > Hi Jonathan, > > On Tue, 26 Jan 2010 18:57:02 +0000, Jonathan Cameron wrote: >> Signed-off-by: Jonathan Cameron <jic23(a)cam.ac.uk> >> --- >> Reverted incorrect removal of i2c/chips references in i2c/Kconfig etc. >> >> Documentation/ABI/testing/sysfs-class-als | 9 +++ >> drivers/als/Kconfig | 14 ++++ >> drivers/als/Makefile | 2 + >> drivers/{i2c/chips => als}/tsl2550.c | 101 +++++++++++++++++----------- >> drivers/i2c/chips/Kconfig | 10 --- >> drivers/i2c/chips/Makefile | 2 - >> 6 files changed, 86 insertions(+), 52 deletions(-) >> > > Some more comments... sorry for not seeing this before, the git patch > format for moving files makes things easier to review for sure. Indeed, very handy feature when making these sorts of changes. > >> diff --git a/Documentation/ABI/testing/sysfs-class-als b/Documentation/ABI/testing/sysfs-class-als >> index d3b33f3..732f449 100644 >> --- a/Documentation/ABI/testing/sysfs-class-als >> +++ b/Documentation/ABI/testing/sysfs-class-als >> @@ -7,3 +7,12 @@ Description: Current Ambient Light Illuminance reported by >> Unit: lux (lumens per square meter) >> RO >> >> +What: /sys/class/als/.../exposure_time[n] >> +Date: Dec. 2009 >> +KernelVersion: 2.6.32 >> +Contact: Jonathan Cameron <jic23(a)cam.ac.uk> >> +Description: Sensor exposure time. In some devices this >> + corresponds to the combined time needed to >> + to internally read several different sensors. >> + Unit: microseconds >> + RW >> diff --git a/drivers/als/Kconfig b/drivers/als/Kconfig >> index 200c52b..1564ffc 100644 >> --- a/drivers/als/Kconfig >> +++ b/drivers/als/Kconfig >> @@ -8,3 +8,17 @@ menuconfig ALS >> This framework provides a generic sysfs I/F for Ambient Light >> Sensor devices. >> If you want this support, you should say Y or M here. >> + >> +if ALS >> + >> +config ALS_TSL2550 >> + tristate "Taos TSL2550 ambient light sensor" >> + depends on EXPERIMENTAL && I2C >> + 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. >> + >> +endif #ALS >> diff --git a/drivers/als/Makefile b/drivers/als/Makefile >> index a527197..7be5631 100644 >> --- a/drivers/als/Makefile >> +++ b/drivers/als/Makefile >> @@ -3,3 +3,5 @@ >> # >> >> obj-$(CONFIG_ALS) += als_sys.o >> + >> +obj-$(CONFIG_ALS_TSL2550) += tsl2550.o >> \ No newline at end of file > > Not sure if the missing new-line is in the old file or the new. If the > new, please fix it. Good point. > >> diff --git a/drivers/i2c/chips/tsl2550.c b/drivers/als/tsl2550.c >> similarity index 81% >> rename from drivers/i2c/chips/tsl2550.c >> rename to drivers/als/tsl2550.c >> index a0702f3..8d5f2a1 100644 >> --- a/drivers/i2c/chips/tsl2550.c >> +++ b/drivers/als/tsl2550.c >> @@ -3,6 +3,7 @@ >> * >> * Copyright (C) 2007 Rodolfo Giometti <giometti(a)linux.it> >> * Copyright (C) 2007 Eurotech S.p.A. <info(a)eurotech.it> >> + * Copyright (C) 2009 Jonathan Cameron <jic23(a)cam.ac.uk> >> * >> * This program is free software; you can redistribute it and/or modify >> * it under the terms of the GNU General Public License as published by >> @@ -24,9 +25,11 @@ >> #include <linux/slab.h> >> #include <linux/i2c.h> >> #include <linux/mutex.h> >> +#include <linux/err.h> >> +#include <linux/als_sys.h> >> >> #define TSL2550_DRV_NAME "tsl2550" >> -#define DRIVER_VERSION "1.2" >> +#define DRIVER_VERSION "2.0" >> >> /* >> * Defines >> @@ -44,11 +47,12 @@ >> */ >> >> struct tsl2550_data { >> + struct device *classdev; >> struct i2c_client *client; >> struct mutex update_lock; >> >> - unsigned int power_state : 1; >> - unsigned int operating_mode : 1; >> + unsigned int power_state:1; >> + unsigned int operating_mode:1; > > These style changes don't seem needed. iirc checkpatch.pl fussiness, but arguably shouldn't be in this patch which claims just to be a move. Personally I don't care about this one, so for simplicity I'll just drop the change. Afterall, checkpatch won't see it if I do move detection :) > >> }; >> >> /* >> @@ -102,15 +106,16 @@ static int tsl2550_get_adc_value(struct i2c_client *client, u8 cmd) >> return ret; >> if (!(ret & 0x80)) >> return -EAGAIN; >> + if (ret == 0x7f) >> + return -ERANGE; >> return ret & 0x7f; /* remove the "valid" bit */ >> } >> >> /* >> - * LUX calculation >> + * LUX calculation - note the range is dependent on combination >> + * of infrared level and visible light levels. >> */ >> >> -#define TSL2550_MAX_LUX 1846 >> - >> static const u8 ratio_lut[] = { >> 100, 100, 100, 100, 100, 100, 100, 100, >> 100, 100, 100, 100, 100, 100, 99, 99, >> @@ -180,8 +185,7 @@ static int tsl2550_calculate_lux(u8 ch0, u8 ch1) >> else >> return -EAGAIN; >> >> - /* LUX range check */ >> - return lux > TSL2550_MAX_LUX ? TSL2550_MAX_LUX : lux; >> + return lux; >> } > > On second thought, this clean-up is unrelated to the driver move to > ALS... so it might be better left for a later, separate patch? That is true. I'll break it out so we have a pair of patches. Just for clarity (and because you spotted the issue in the first place!) I'd like your ack on that new patch. > >> >> /* >> @@ -191,7 +195,8 @@ static int tsl2550_calculate_lux(u8 ch0, u8 ch1) >> static ssize_t tsl2550_show_power_state(struct device *dev, >> struct device_attribute *attr, char *buf) >> { >> - struct tsl2550_data *data = i2c_get_clientdata(to_i2c_client(dev)); >> + struct i2c_client *client = to_i2c_client(dev->parent); >> + struct tsl2550_data *data = i2c_get_clientdata(client); >> >> return sprintf(buf, "%u\n", data->power_state); >> } >> @@ -199,12 +204,12 @@ static ssize_t tsl2550_show_power_state(struct device *dev, >> static ssize_t tsl2550_store_power_state(struct device *dev, >> struct device_attribute *attr, const char *buf, size_t count) >> { >> - struct i2c_client *client = to_i2c_client(dev); >> + struct i2c_client *client = to_i2c_client(dev->parent); >> struct tsl2550_data *data = i2c_get_clientdata(client); >> - unsigned long val = simple_strtoul(buf, NULL, 10); >> - int ret; >> + unsigned long val; >> + int ret = strict_strtoul(buf, 10, &val); >> >> - if (val < 0 || val > 1) >> + if (val < 0 || val > 1 || ret) > > It would be much more logical to test ret first and val next, rather > than the other way around. Good point. > > I also have a personal preference for not including code that can fail > in the variable declaration section. But up to you of course. I don't care so I'll make the change... > >> return -EINVAL; >> >> mutex_lock(&data->update_lock); >> @@ -220,40 +225,45 @@ static ssize_t tsl2550_store_power_state(struct device *dev, >> static DEVICE_ATTR(power_state, S_IWUSR | S_IRUGO, >> tsl2550_show_power_state, tsl2550_store_power_state); >> >> -static ssize_t tsl2550_show_operating_mode(struct device *dev, >> - struct device_attribute *attr, char *buf) >> +static ssize_t tsl2550_show_exposure(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> { >> - struct tsl2550_data *data = i2c_get_clientdata(to_i2c_client(dev)); >> - >> - return sprintf(buf, "%u\n", data->operating_mode); >> + struct i2c_client *client = to_i2c_client(dev->parent); >> + struct tsl2550_data *data = i2c_get_clientdata(client); >> + if (data->operating_mode) >> + return sprintf(buf, "160000\n"); >> + else >> + return sprintf(buf, "800000\n"); >> } >> >> -static ssize_t tsl2550_store_operating_mode(struct device *dev, >> - struct device_attribute *attr, const char *buf, size_t count) >> +static ssize_t tsl2550_store_exposure(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, >> + size_t count) >> { >> - struct i2c_client *client = to_i2c_client(dev); >> + struct i2c_client *client = to_i2c_client(dev->parent); >> struct tsl2550_data *data = i2c_get_clientdata(client); >> - unsigned long val = simple_strtoul(buf, NULL, 10); >> - int ret; >> - >> - if (val < 0 || val > 1) >> - return -EINVAL; >> + unsigned long val; >> >> - if (data->power_state == 0) >> - return -EBUSY; >> + int ret = strict_strtoul(buf, 10, &val); >> >> + if (ret) >> + return -EINVAL; >> mutex_lock(&data->update_lock); >> - ret = tsl2550_set_operating_mode(client, val); >> + if (val >= 800000) >> + ret = tsl2550_set_operating_mode(client, 0); >> + else >> + ret = tsl2550_set_operating_mode(client, 1); >> mutex_unlock(&data->update_lock); >> - >> if (ret < 0) >> return ret; >> >> return count; >> } >> >> -static DEVICE_ATTR(operating_mode, S_IWUSR | S_IRUGO, >> - tsl2550_show_operating_mode, tsl2550_store_operating_mode); >> +static DEVICE_ATTR(exposure_time0, S_IWUSR | S_IRUGO, >> + tsl2550_show_exposure, tsl2550_store_exposure); >> >> static ssize_t __tsl2550_show_lux(struct i2c_client *client, char *buf) >> { >> @@ -284,7 +294,7 @@ static ssize_t __tsl2550_show_lux(struct i2c_client *client, char *buf) >> static ssize_t tsl2550_show_lux1_input(struct device *dev, >> struct device_attribute *attr, char *buf) >> { >> - struct i2c_client *client = to_i2c_client(dev); >> + struct i2c_client *client = to_i2c_client(dev->parent); >> struct tsl2550_data *data = i2c_get_clientdata(client); >> int ret; >> >> @@ -299,13 +309,13 @@ static ssize_t tsl2550_show_lux1_input(struct device *dev, >> return ret; >> } >> >> -static DEVICE_ATTR(lux1_input, S_IRUGO, >> +static DEVICE_ATTR(illuminance0, S_IRUGO, >> tsl2550_show_lux1_input, NULL); >> >> static struct attribute *tsl2550_attributes[] = { >> &dev_attr_power_state.attr, >> - &dev_attr_operating_mode.attr, >> - &dev_attr_lux1_input.attr, >> + &dev_attr_exposure_time0.attr, >> + &dev_attr_illuminance0.attr, >> NULL >> }; >> >> @@ -391,14 +401,22 @@ static int __devinit tsl2550_probe(struct i2c_client *client, >> goto exit_kfree; >> >> /* Register sysfs hooks */ >> - err = sysfs_create_group(&client->dev.kobj, &tsl2550_attr_group); >> - if (err) >> + data->classdev = als_device_register(&client->dev); >> + if (IS_ERR(data->classdev)) { >> + err = PTR_ERR(data->classdev); >> goto exit_kfree; >> + } >> + >> + err = sysfs_create_group(&data->classdev->kobj, &tsl2550_attr_group); >> + if (err) >> + goto exit_unreg; >> >> dev_info(&client->dev, "support ver. %s enabled\n", DRIVER_VERSION); >> >> return 0; >> >> +exit_unreg: >> + als_device_unregister(data->classdev); >> exit_kfree: >> kfree(data); >> exit: >> @@ -407,12 +425,15 @@ exit: >> >> static int __devexit tsl2550_remove(struct i2c_client *client) >> { >> - sysfs_remove_group(&client->dev.kobj, &tsl2550_attr_group); >> + struct tsl2550_data *data = i2c_get_clientdata(client); >> + >> + sysfs_remove_group(&data->classdev->kobj, &tsl2550_attr_group); >> + als_device_unregister(data->classdev); >> >> /* Power down the device */ >> tsl2550_set_power_state(client, 0); >> >> - kfree(i2c_get_clientdata(client)); >> + kfree(data); >> >> return 0; >> } >> diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig >> index ae4539d..c11f8d6 100644 >> --- a/drivers/i2c/chips/Kconfig >> +++ b/drivers/i2c/chips/Kconfig >> @@ -6,14 +6,4 @@ >> >> menu "Miscellaneous I2C Chip support" >> >> -config SENSORS_TSL2550 >> - tristate "Taos TSL2550 ambient light sensor" >> - depends on 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 >> diff --git a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile >> index fe0af0f..ffde18d 100644 >> --- a/drivers/i2c/chips/Makefile >> +++ b/drivers/i2c/chips/Makefile >> @@ -10,8 +10,6 @@ >> # * I/O expander drivers go to drivers/gpio >> # >> >> -obj-$(CONFIG_SENSORS_TSL2550) += tsl2550.o >> - >> ifeq ($(CONFIG_I2C_DEBUG_CHIP),y) >> EXTRA_CFLAGS += -DDEBUG >> endif > > All the rest looks OK to me. Feel free to add: > > Acked-by: Jean Delvare <khali(a)linux-fr.org> Cool, for reference I'll post the final version as a reply to this and take it into the ALS for-next tree. Jonathan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |