2009-09-22 03:39:22

by Zhang Rui

[permalink] [raw]
Subject: [RFC] [PATCH 1/2] introduce ALS sysfs class

Hi, Jonathan,

this is the refresh ALS sysfs class driver.
I just introduced one sysfs attribute "illuminance", because
I didn't catch the exact meaning of the others like "infrared".
So it would be great if you can generate an incremental patch
to introduce the other optional attributes needed, and update
the documentation as well. :)

Subject: Introduce ALS sysfs class

ALS sysfs class device provides a standard sysfs interface
for Ambient Light Sensor devices.

Only one sysfs I/F is introduced currently.
/sys/class/als/xxx/illuminance:
indicates the amount of light incident upon a specified surface area.

Signed-off-by: Zhang Rui <[email protected]>
---
Documentation/ABI/testing/sysfs-class-als | 9 +++
MAINTAINERS | 6 ++
drivers/Kconfig | 2
drivers/Makefile | 1
drivers/als/Kconfig | 10 ++++
drivers/als/Makefile | 5 ++
drivers/als/als_sys.c | 74 ++++++++++++++++++++++++++++++
include/linux/als_sys.h | 35 ++++++++++++++
8 files changed, 142 insertions(+)

Index: linux-2.6/drivers/Kconfig
===================================================================
--- linux-2.6.orig/drivers/Kconfig
+++ linux-2.6/drivers/Kconfig
@@ -62,6 +62,8 @@ source "drivers/power/Kconfig"

source "drivers/hwmon/Kconfig"

+source "drivers/als/Kconfig"
+
source "drivers/thermal/Kconfig"

source "drivers/watchdog/Kconfig"
Index: linux-2.6/drivers/Makefile
===================================================================
--- linux-2.6.orig/drivers/Makefile
+++ linux-2.6/drivers/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_PPS) += pps/
obj-$(CONFIG_W1) += w1/
obj-$(CONFIG_POWER_SUPPLY) += power/
obj-$(CONFIG_HWMON) += hwmon/
+obj-$(CONFIG_ALS) += als/
obj-$(CONFIG_THERMAL) += thermal/
obj-$(CONFIG_WATCHDOG) += watchdog/
obj-$(CONFIG_PHONE) += telephony/
Index: linux-2.6/drivers/als/Kconfig
===================================================================
--- /dev/null
+++ linux-2.6/drivers/als/Kconfig
@@ -0,0 +1,10 @@
+#
+# Ambient Light Sensor sysfs device configuration
+#
+
+menuconfig ALS
+ tristate "Ambient Light Sensor sysfs device"
+ help
+ 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.
Index: linux-2.6/drivers/als/Makefile
===================================================================
--- /dev/null
+++ linux-2.6/drivers/als/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for sensor chip drivers.
+#
+
+obj-$(CONFIG_ALS) += als_sys.o
Index: linux-2.6/drivers/als/als_sys.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/als/als_sys.c
@@ -0,0 +1,74 @@
+/*
+ * als_sys.c - Ambient Light Sensor Sysfs support.
+ *
+ * Copyright (C) 2009 Intel Corp
+ * Copyright (C) 2009 Zhang Rui <[email protected]>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kdev_t.h>
+
+MODULE_AUTHOR("Zhang Rui <[email protected]>");
+MODULE_DESCRIPTION("Ambient Light Sensor sysfs/class support");
+MODULE_LICENSE("GPL");
+
+static struct class *als_class;
+
+/**
+ * als_device_register - register a new Ambient Light Sensor class device
+ * @parent: the device to register.
+ *
+ * Returns the pointer to the new device
+ */
+struct device *als_device_register(struct device *dev, char *name)
+{
+ return device_create(als_class, dev, MKDEV(0, 0), NULL, name);
+}
+EXPORT_SYMBOL(als_device_register);
+
+/**
+ * als_device_unregister - removes the registered ALS class device
+ * @dev: the class device to destroy.
+ */
+void als_device_unregister(struct device *dev)
+{
+ device_unregister(dev);
+}
+EXPORT_SYMBOL(als_device_unregister);
+
+static int __init als_init(void)
+{
+ als_class = class_create(THIS_MODULE, "als");
+ if (IS_ERR(als_class)) {
+ printk(KERN_ERR "als_sys.c: couldn't create sysfs class\n");
+ return PTR_ERR(als_class);
+ }
+ return 0;
+}
+
+static void __exit als_exit(void)
+{
+ class_destroy(als_class);
+}
+
+subsys_initcall(als_init);
+module_exit(als_exit);
Index: linux-2.6/include/linux/als_sys.h
===================================================================
--- /dev/null
+++ linux-2.6/include/linux/als_sys.h
@@ -0,0 +1,35 @@
+/*
+ * als_sys.h
+ *
+ * Copyright (C) 2009 Intel Corp
+ * Copyright (C) 2009 Zhang Rui <[email protected]>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * 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
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#ifndef __ALS_SYS_H__
+#define __ALS_SYS_H__
+
+#include <linux/device.h>
+
+#define ALS_ILLUMINANCE_MIN 0
+#define ALS_ILLUMINANCE_MAX -1
+
+struct device *als_device_register(struct device *dev, char *name);
+void als_device_unregister(struct device *dev);
+
+#endif /* __ALS_SYS_H__ */
Index: linux-2.6/Documentation/ABI/testing/sysfs-class-als
===================================================================
--- /dev/null
+++ linux-2.6/Documentation/ABI/testing/sysfs-class-als
@@ -0,0 +1,9 @@
+What: /sys/class/als/.../illuminance
+Date: Sep. 2009
+KernelVersion: 2.6.32
+Contact: Zhang Rui <[email protected]>
+Description: Current Ambient Light Illuminance reported by
+ native ALS driver
+ Unit: lux (lumens per square meter)
+ RO
+
Index: linux-2.6/MAINTAINERS
===================================================================
--- linux-2.6.orig/MAINTAINERS
+++ linux-2.6/MAINTAINERS
@@ -399,6 +399,12 @@ S: Maintained for 2.4; PCI support for 2
L: [email protected]
F: arch/alpha/

+AMBIENT LIGHT SENSOR
+M: Zhang Rui <[email protected]>
+S: Supported
+F: include/linux/als_sys.h
+F: drivers/als/
+
AMD GEODE CS5536 USB DEVICE CONTROLLER DRIVER
M: Thomas Dahlmann <[email protected]>
L: [email protected] (moderated for non-subscribers)


2009-09-22 12:42:04

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [RFC] [PATCH 1/2] introduce ALS sysfs class

Zhang Rui wrote:
> Hi, Jonathan,
>
> this is the refresh ALS sysfs class driver.
> I just introduced one sysfs attribute "illuminance", because
> I didn't catch the exact meaning of the others like "infrared".
> So it would be great if you can generate an incremental patch
> to introduce the other optional attributes needed, and update
> the documentation as well. :)
Will do, though may just leave it out of first pass of drivers
(as it may be controversial and it would be nice to get something
in place before the arguments begin!)

All looks nice and clean. The only real question is whether
we want to standardize naming of devices under sysfs (like hwmon does)
or allow the individual drivers to do the naming?
I'm happy either way.

Acked-by: Jonathan Cameron <[email protected]>

Thanks for all your work on this. I'm out of the lab until the weekend
so I'll get to porting the tsl drivers after that.

>
> Subject: Introduce ALS sysfs class
>
> ALS sysfs class device provides a standard sysfs interface
> for Ambient Light Sensor devices.
>
> Only one sysfs I/F is introduced currently.
> /sys/class/als/xxx/illuminance:
> indicates the amount of light incident upon a specified surface area.
>
> Signed-off-by: Zhang Rui <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-class-als | 9 +++
> MAINTAINERS | 6 ++
> drivers/Kconfig | 2
> drivers/Makefile | 1
> drivers/als/Kconfig | 10 ++++
> drivers/als/Makefile | 5 ++
> drivers/als/als_sys.c | 74 ++++++++++++++++++++++++++++++
> include/linux/als_sys.h | 35 ++++++++++++++
> 8 files changed, 142 insertions(+)
>
> Index: linux-2.6/drivers/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/Kconfig
> +++ linux-2.6/drivers/Kconfig
> @@ -62,6 +62,8 @@ source "drivers/power/Kconfig"
>
> source "drivers/hwmon/Kconfig"
>
> +source "drivers/als/Kconfig"
> +
> source "drivers/thermal/Kconfig"
>
> source "drivers/watchdog/Kconfig"
> Index: linux-2.6/drivers/Makefile
> ===================================================================
> --- linux-2.6.orig/drivers/Makefile
> +++ linux-2.6/drivers/Makefile
> @@ -76,6 +76,7 @@ obj-$(CONFIG_PPS) += pps/
> obj-$(CONFIG_W1) += w1/
> obj-$(CONFIG_POWER_SUPPLY) += power/
> obj-$(CONFIG_HWMON) += hwmon/
> +obj-$(CONFIG_ALS) += als/
> obj-$(CONFIG_THERMAL) += thermal/
> obj-$(CONFIG_WATCHDOG) += watchdog/
> obj-$(CONFIG_PHONE) += telephony/
> Index: linux-2.6/drivers/als/Kconfig
> ===================================================================
> --- /dev/null
> +++ linux-2.6/drivers/als/Kconfig
> @@ -0,0 +1,10 @@
> +#
> +# Ambient Light Sensor sysfs device configuration
> +#
> +
> +menuconfig ALS
> + tristate "Ambient Light Sensor sysfs device"
> + help
> + 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.
> Index: linux-2.6/drivers/als/Makefile
> ===================================================================
> --- /dev/null
> +++ linux-2.6/drivers/als/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for sensor chip drivers.
> +#
> +
> +obj-$(CONFIG_ALS) += als_sys.o
> Index: linux-2.6/drivers/als/als_sys.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6/drivers/als/als_sys.c
> @@ -0,0 +1,74 @@
> +/*
> + * als_sys.c - Ambient Light Sensor Sysfs support.
> + *
> + * Copyright (C) 2009 Intel Corp
> + * Copyright (C) 2009 Zhang Rui <[email protected]>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * 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
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/kdev_t.h>
> +
> +MODULE_AUTHOR("Zhang Rui <[email protected]>");
> +MODULE_DESCRIPTION("Ambient Light Sensor sysfs/class support");
> +MODULE_LICENSE("GPL");
> +
> +static struct class *als_class;
> +
> +/**
> + * als_device_register - register a new Ambient Light Sensor class device
> + * @parent: the device to register.
> + *
> + * Returns the pointer to the new device
> + */
> +struct device *als_device_register(struct device *dev, char *name)
> +{
> + return device_create(als_class, dev, MKDEV(0, 0), NULL, name);
> +}
> +EXPORT_SYMBOL(als_device_register);
> +
> +/**
> + * als_device_unregister - removes the registered ALS class device
> + * @dev: the class device to destroy.
> + */
> +void als_device_unregister(struct device *dev)
> +{
> + device_unregister(dev);
> +}
> +EXPORT_SYMBOL(als_device_unregister);
> +
> +static int __init als_init(void)
> +{
> + als_class = class_create(THIS_MODULE, "als");
> + if (IS_ERR(als_class)) {
> + printk(KERN_ERR "als_sys.c: couldn't create sysfs class\n");
> + return PTR_ERR(als_class);
> + }
> + return 0;
> +}
> +
> +static void __exit als_exit(void)
> +{
> + class_destroy(als_class);
> +}
> +
> +subsys_initcall(als_init);
> +module_exit(als_exit);
> Index: linux-2.6/include/linux/als_sys.h
> ===================================================================
> --- /dev/null
> +++ linux-2.6/include/linux/als_sys.h
> @@ -0,0 +1,35 @@
> +/*
> + * als_sys.h
> + *
> + * Copyright (C) 2009 Intel Corp
> + * Copyright (C) 2009 Zhang Rui <[email protected]>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + * 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
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +
> +#ifndef __ALS_SYS_H__
> +#define __ALS_SYS_H__
> +
> +#include <linux/device.h>
> +
> +#define ALS_ILLUMINANCE_MIN 0
> +#define ALS_ILLUMINANCE_MAX -1
> +
> +struct device *als_device_register(struct device *dev, char *name);
> +void als_device_unregister(struct device *dev);
I guess these are simple enough that it doesn't really matter,
but some documentation might be nice.
> +
> +#endif /* __ALS_SYS_H__ */
> Index: linux-2.6/Documentation/ABI/testing/sysfs-class-als
> ===================================================================
> --- /dev/null
> +++ linux-2.6/Documentation/ABI/testing/sysfs-class-als
> @@ -0,0 +1,9 @@
> +What: /sys/class/als/.../illuminance
> +Date: Sep. 2009
> +KernelVersion: 2.6.32
> +Contact: Zhang Rui <[email protected]>
> +Description: Current Ambient Light Illuminance reported by
> + native ALS driver
> + Unit: lux (lumens per square meter)
> + RO
> +
> Index: linux-2.6/MAINTAINERS
> ===================================================================
> --- linux-2.6.orig/MAINTAINERS
> +++ linux-2.6/MAINTAINERS
> @@ -399,6 +399,12 @@ S: Maintained for 2.4; PCI support for 2
> L: [email protected]
> F: arch/alpha/
>
> +AMBIENT LIGHT SENSOR
> +M: Zhang Rui <[email protected]>
> +S: Supported
> +F: include/linux/als_sys.h
> +F: drivers/als/
> +
> AMD GEODE CS5536 USB DEVICE CONTROLLER DRIVER
> M: Thomas Dahlmann <[email protected]>
> L: [email protected] (moderated for non-subscribers)
>
>
>

2009-09-23 07:19:24

by Pavel Machek

[permalink] [raw]
Subject: Re: [RFC] [PATCH 1/2] introduce ALS sysfs class

On Tue 2009-09-22 13:42:23, Jonathan Cameron wrote:
> Zhang Rui wrote:
> > Hi, Jonathan,
> >
> > this is the refresh ALS sysfs class driver.
> > I just introduced one sysfs attribute "illuminance", because
> > I didn't catch the exact meaning of the others like "???infrared".
> > So it would be great if you can generate an incremental patch
> > to introduce the other optional attributes needed, and update
> > the documentation as well. :)
> Will do, though may just leave it out of first pass of drivers
> (as it may be controversial and it would be nice to get something
> in place before the arguments begin!)
>
> All looks nice and clean. The only real question is whether
> we want to standardize naming of devices under sysfs (like hwmon does)
> or allow the individual drivers to do the naming?

Allow the drivers to do the naming. Having useless name like "als0",
with als0/name telling me what the driver is is bad.
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2009-09-23 07:26:07

by Corentin Chary

[permalink] [raw]
Subject: Re: [RFC] [PATCH 1/2] introduce ALS sysfs class

On Wed, Sep 23, 2009 at 9:18 AM, Pavel Machek <[email protected]> wrote:
> On Tue 2009-09-22 13:42:23, Jonathan Cameron wrote:
>> Zhang Rui wrote:
>> > Hi, Jonathan,
>> >
>> > this is the refresh ALS sysfs class driver.
>> > I just introduced one sysfs attribute "illuminance", because
>> > I didn't catch the exact meaning of the others like "???infrared".
>> > So it would be great if you can generate an incremental patch
>> > to introduce the other optional attributes needed, and update
>> > the documentation as well. :)
>> Will do, though may just leave it out of first pass of drivers
>> (as it may be controversial and it would be nice to get something
>> in place before the arguments begin!)
>>
>> All looks nice and clean. The only real question is whether
>> we want to standardize naming of devices under sysfs (like hwmon does)
>> or allow the individual drivers to do the naming?
>
> Allow the drivers to do the naming. Having useless name like "als0",
> with als0/name telling me what the driver is is bad.
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?Pavel
>

This is how rfkill works, is there a good reason for that ? Should we
change that ?

--
Corentin Chary
http://xf.iksaif.net

2009-10-09 14:40:04

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [RFC] [PATCH 1/2] introduce ALS sysfs class

Hi Zhang,

Do you have any thoughts on merge route for this?
I've just posted the tsl2550 port to als and it is bound to come
up in that thread.

Thanks,

Jonathan

2009-10-10 01:45:07

by Zhang Rui

[permalink] [raw]
Subject: Re: [RFC] [PATCH 1/2] introduce ALS sysfs class

cc Greg

On Fri, 2009-10-09 at 22:39 +0800, Jonathan Cameron wrote:
> Hi Zhang,
>
> Do you have any thoughts on merge route for this?
> I've just posted the tsl2550 port to als and it is bound to come
> up in that thread.
>
no.
I think we can queue this patch set for 2.6.33.

thanks,
rui

2009-10-15 15:54:56

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [RFC] [PATCH 1/2] introduce ALS sysfs class

Pavel Machek wrote:
> On Tue 2009-09-22 13:42:23, Jonathan Cameron wrote:
>
>> Zhang Rui wrote:
>>
>>> Hi, Jonathan,
>>>
>>> this is the refresh ALS sysfs class driver.
>>> I just introduced one sysfs attribute "illuminance", because
>>> I didn't catch the exact meaning of the others like "???infrared".
>>> So it would be great if you can generate an incremental patch
>>> to introduce the other optional attributes needed, and update
>>> the documentation as well. :)
>>>
>> Will do, though may just leave it out of first pass of drivers
>> (as it may be controversial and it would be nice to get something
>> in place before the arguments begin!)
>>
>> All looks nice and clean. The only real question is whether
>> we want to standardize naming of devices under sysfs (like hwmon does)
>> or allow the individual drivers to do the naming?
>>
>
> Allow the drivers to do the naming. Having useless name like "als0",
> with als0/name telling me what the driver is is bad.
>
This topic came up again in the discussion of the tsl2550 driver port.

Jean Delvare raised a point that I'm inclined to agree with (with several
more ports from elsewhere in the kernel underway).

.... (quoted from [PATCH] ALS: TSL2550 driver move from i2c/chips)

> I'd imaging that als-class devices are simply named als%u. Just like
> > > hwmon devices are named hwmon%u, input devices are names input%u and
> > > event%u, etc. I don't know of any class pushing the naming down to its
> > > drivers, do you? The only example I can remember are i2c drivers back
> > > in year 1999, when they were part of lm-sensors.I have personally put
> > > an end to this years ago.
> >
> > This debate started in the als thread. Personally I couldn't care less
> > either way but it does need to be put to bed before that subsystem merges.
> > To my mind either approach is trivially handled in a userspace library
> > so it doesn't matter. I don't suppose you can remember what the original
> > reasons for squashing this naming approach were?
>
> Code duplication. Having the same unique-id handling code repeated in
> 50 drivers was no fun, as it did not add any value compared to a
> central handling.
>

So does anyone have a strong objection to moving over the more conventional
als0/ naming and move the id handling into the als core?

Note that unless there is a clear reason for doing it any other way it will
probably meet resistance beyond Jean and myself.

Jonathan