leds: new class for led devices
cover leds: new class for led devices
This is an attempt to provide an alternative to the current arm specific led interface. This arm interface does not integrate well with the device model and sysfs.
We create a new class that drivers can register a leds_properties structure with. Each led that is registered is assigned a number, and three attributes are exported to sysfs in /sys/class/leds/1/, /sys/class/leds/2, etc.
color : a read only string attribute that shows the available colors of this led. If it is a multi-color led, then the colors are seperated by a "/" (for example "green/blue").
function : a read/write attribute that sets the current function of this led. The available options are
timer : the led blinks on and off on a timer
idle : the led turns off when the system is idle and on when not idle power : the led represents the current power state
user : the led is controlled by user space
light : a read/write attribute that allows userspace to see the current status of the led. If function="user" then writing to this attribute will change the led on or off. If function != "user" then writing has no effect.
light is an integer, where 0 means off, 1 means on first color, 2
means on second color, etc. (for example, if color="green/blue" then light=1 means turn led on to green and if light=2 means turn led on to blue.
A device and driver symlink point to the actual driver and such, so any extra attributes can also be registered.
This is an example driver for the leds on the Sharp Zaurus using the locomo bus.
#
# Patch managed by http://www.holgerschurig.de/patcher.html
#
drivers/leds/locomo.c new file +112 -0
@@ -0,0 +1,112 @@ +/* +* linux/drivers/leds/locomo.c +* +* Copyright (C) 2004 John Lenz <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#include <linux/config.h> +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/device.h> +#include <linux/leds.h> + +#include <asm/hardware.h> +#include <asm/hardware/locomo.h> + +struct locomoled_data { +unsigned long offset; +int registered; +struct led_properties props; +}; +#define to_locomoled_data (d) container_of(d, struct locomoled_data, props) + +void locomoled_light (struct device *dev,struct led_properties *props) +{ +struct locomo_dev *locomo_dev =LOCOMO_DEV (dev); +struct locomoled_data *data =to_locomoled_data (props); + +unsigned long flags; + +local_irq_save (flags); +if (props->light_state) +locomo_writel (LOCOMO_LPT_TOFH , locomo_dev->mapbase + data->offset); +else +locomo_writel (LOCOMO_LPT_TOFL , locomo_dev->mapbase + data->offset); +local_irq_restore (flags); +} + +static struct locomoled_data leds[] = { + { + .offset =LOCOMO_LPT0 , + .props = { + .owner =THIS_MODULE , + .color ="amber" , + .light_state =0 , + + .light = locomoled_light, + + .function = leds_timer, + } + }, + { + .offset =LOCOMO_LPT1 , + .props = { + .owner =THIS_MODULE , + .color ="green" , + .light_state =0 , + + .light = locomoled_light, + + .function = leds_idle, + } + }, +}; + +static int locomoled_probe (struct locomo_dev *dev) +{ +int i, ret =0 ; + +for (i =0 ; i <ARRAY_SIZE (leds); i++) { + ret =leds_device_register (&dev->dev, &leds[i].props); + leds[i].registered =1 ; +if (unlikely (ret)) { +printk (KERN_WARNING "Unable to register locomo led %s\n" , leds[i].props.color); + leds[i].registered =0 ; + } + } + +return ret; +} + +static int locomoled_remove (struct locomo_dev *dev) { +int i; + +for (i =0 ; i <ARRAY_SIZE (leds); i++) { +if (leds[i].registered) { +leds_device_unregister (&leds[i].props); + } + } +return 0 ; +} + +static struct locomo_driver locomoled_driver = { + .drv = { + .name ="locomoled" + }, + .devid =LOCOMO_DEVID_LED , + .probe = locomoled_probe, + .remove = locomoled_remove, +}; + +static int __init locomoled_init (void ) { +return locomo_driver_register (&locomoled_driver); +} +module_init (locomoled_init); + +MODULE_AUTHOR ("John Lenz <[email protected]>" ); +MODULE_DESCRIPTION ("Locomo LED driver" ); +MODULE_LICENSE ("GPL" );
drivers/leds/Kconfig renamed from drivers/leds/Kconfig~locomo_led +7 -0
@@ -15,4 +15,11 @@ This option enables support for the old leds_event interface used by the arm port. This driver can not be compiled as a module. +config LEDS_LOCOMO + tristate "LED Support for Locomo device" + depends CLASS_LEDS && SHARP_LOCOMO + help + This option enables support for the LEDs on Sharp Locomo. + + endmenu
drivers/leds/Makefile renamed from drivers/leds/Makefile~locomo_led +3 -0
@@ -1,3 +1,6 @@ # Core functionality. obj-$(CONFIG_CLASS_LEDS) += ledscore.o + +# drivers +obj-$(CONFIG_LEDS_LOCOMO) += locomo.o
This provides the base ledscore.c support.
#
# Patch managed by http://www.holgerschurig.de/patcher.html
#
drivers/leds/ledscore.c new file +314 -0
@@ -0,0 +1,314 @@ +/* +* linux/drivers/leds/ledscore.c +* +* Copyright (C) 2004 John Lenz <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#include <linux/config.h> +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/list.h> +#include <linux/spinlock.h> +#include <linux/device.h> +#include <linux/sysdev.h> +#include <linux/timer.h> +#include <linux/leds.h> + +#define LEDS_TIMER_INTERVAL (HZ * 5) + +struct led_device { +/* This protects the props field.*/ +spinlock_t lock; +/* If props is NULL, the driver that registered this device has been unloaded */ +struct led_properties *props; +struct class_device class_dev; +struct list_head list; +}; + +#define to_led_device (d) container_of(d, struct led_device, class_dev) + +static rwlock_t leds_list_lock =RW_LOCK_UNLOCKED ; +static LIST_HEAD (leds_list); +static atomic_t leds_count =ATOMIC_INIT (0 ); + +struct leds_function_name { +const char * name; +enum led_functions function; +}; +static const struct leds_function_name leds_function_names[] = { + {"user" , leds_user }, + {"timer" , leds_timer }, + {"idle" , leds_idle }, + {"power" , leds_power }, +}; + +static void leds_class_release (struct class_device *dev) +{ +struct led_device *d =to_led_device (dev); + +write_lock (&leds_list_lock); +list_del (&d->list); +write_unlock (&leds_list_lock); + +kfree (d); +} + +static struct class leds_class = { + .name ="leds" , + .release = leds_class_release, +}; + +static ssize_t leds_show_color (struct class_device *dev,char *buf) +{ +struct led_device *led_dev =to_led_device (dev); +ssize_t ret =0 ; + +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { +sprintf (buf,"%s\n" , led_dev->props->color); + ret =strlen (buf) +1 ; + } +spin_unlock (&led_dev->lock); + +return ret; +} + +static CLASS_DEVICE_ATTR(color, 0444, leds_show_color, NULL); + +static ssize_t leds_show_light (struct class_device *dev,char *buf) +{ +struct led_device *led_dev =to_led_device (dev); +ssize_t ret =0 ; + +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { +sprintf (buf,"%lu\n" , led_dev->props->light_state); + ret =strlen (buf) +1 ; + } +spin_unlock (&led_dev->lock); + +return ret; +} + +static ssize_t leds_store_light (struct class_device *dev,const char *buf,size_t size) +{ +struct led_device *led_dev =to_led_device (dev); +int ret = -EINVAL ; +char *after; + +unsigned long state =simple_strtoul (buf, &after,10 ); +if (after - buf >0 ) { + ret = after - buf; +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { +if (led_dev->props->function == leds_user) { + led_dev->props->light_state = state; +if (led_dev->props->light) + led_dev->props->light (led_dev->class_dev.dev, led_dev->props); + } + } +spin_unlock (&led_dev->lock); + } + +return ret; +} + +static CLASS_DEVICE_ATTR(light, 0644, leds_show_light, leds_store_light); + +static ssize_t leds_show_function (struct class_device *dev,char *buf) +{ +struct led_device *led_dev =to_led_device (dev); +ssize_t ret =0 ; + +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { +sprintf (buf,"%s\n" , leds_function_names[(int )led_dev->props->function].name); + ret =strlen (buf) +1 ; + } +spin_unlock (&led_dev->lock); + +return ret; +} + +static ssize_t leds_store_function (struct class_device *dev,const char *buf,size_t size) +{ +struct led_device *led_dev =to_led_device (dev); +int ret = -EINVAL ; +int i; + +for (i =0 ; i <ARRAY_SIZE (leds_function_names); i++) { +if (strncmp (buf, leds_function_names[i].name,strlen (leds_function_names[i].name)) ==0 ) { + ret = size; +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { + led_dev->props->function = leds_function_names[i].function; + } +spin_unlock (&led_dev->lock); + } + } + +return ret; +} + +static CLASS_DEVICE_ATTR(function, 0644, leds_show_function, leds_store_function); + +/** +* leds_device_register - register a new object of led_device class. +* @dev: The device to register. +* @prop: the led properties structure for this device. +*/ +int leds_device_register (struct device *dev,struct led_properties *props) +{ +int rc, num; +struct led_device *new_led; + + new_led =kmalloc (sizeof (struct led_device ),GFP_KERNEL ); +if (unlikely (!new_led)) +return -ENOMEM ; + +spin_lock_init (&new_led->lock); + new_led->props = props; + props->led_dev = new_led; + +memset (&new_led->class_dev,0 ,sizeof (new_led->class_dev)); + new_led->class_dev.class = &leds_class; + new_led->class_dev.dev = dev; + +/* assign this led a number */ + num =atomic_add_return (1 , &leds_count); +sprintf (new_led->class_dev.class_id,"%i" , num); + + rc =class_device_register (&new_led->class_dev); +if (unlikely (rc)) { +kfree (new_led); +return rc; + } + +/* register the attributes */ +class_device_create_file (&new_led->class_dev, &class_device_attr_color); +class_device_create_file (&new_led->class_dev, &class_device_attr_light); +class_device_create_file (&new_led->class_dev, &class_device_attr_function); + +/* add to the list of leds */ +write_lock (&leds_list_lock); +list_add_tail (&new_led->list, &leds_list); +write_unlock (&leds_list_lock); + +printk (KERN_INFO "Registered led device: number=%s, color=%s\n" , new_led->class_dev.class_id, props->color); + +return 0 ; +} +EXPORT_SYMBOL (leds_device_register); + +/** +* led_device_unregister - unregisters a object of led_properties class. +* @props: the property to unreigister +* +* Unregisters a previously registered via led_device_register object. +*/ +void leds_device_unregister (struct led_properties *props) +{ +struct led_device *led_dev; +if (!props) +return ; + +pr_debug ("led_device_unregister: color=%s\n" , props->color); + + led_dev = props->led_dev; + +class_device_remove_file (&led_dev->class_dev, &class_device_attr_function); +class_device_remove_file (&led_dev->class_dev, &class_device_attr_light); +class_device_remove_file (&led_dev->class_dev, &class_device_attr_color); + +spin_lock (&led_dev->lock); + led_dev->props =NULL ; + props->led_dev =NULL ; +spin_unlock (&led_dev->lock); + +class_device_unregister (&led_dev->class_dev); +} +EXPORT_SYMBOL (leds_device_unregister); + +/* set up a kernel timer */ +static struct timer_list leds_ktimer; +static atomic_t leds_stop_timer =ATOMIC_INIT (0 ); + +static void leds_timer_function (unsigned long data) +{ +struct led_device *led_dev; + +read_lock (&leds_list_lock); +list_for_each_entry (led_dev, &leds_list, list) { +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { +if (led_dev->props->function == leds_timer) { + led_dev->props->light_state = !led_dev->props->light_state; +if (led_dev->props->light) + led_dev->props->light (led_dev->class_dev.dev, led_dev->props); + } + } +spin_unlock (&led_dev->lock); + } +read_unlock (&leds_list_lock); + +if (!atomic_read (&leds_stop_timer)) +mod_timer (&leds_ktimer, jiffies +LEDS_TIMER_INTERVAL ); +} + +/* arch code should call this function from the idle thread */ +void leds_idle_function (int state) +{ +struct led_device *led_dev; + +read_lock (&leds_list_lock); +list_for_each_entry (led_dev, &leds_list, list) { +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { +if (led_dev->props->function == leds_idle) { + led_dev->props->light_state = state; +if (led_dev->props->light) + led_dev->props->light (led_dev->class_dev.dev, led_dev->props); + } + } +spin_unlock (&led_dev->lock); + } +read_unlock (&leds_list_lock); +} + +static int __init leds_init (void ) +{ +int ret =0 ; + +/* initialize the class device */ +class_register (&leds_class); + +/* register the timer */ +init_timer (&leds_ktimer); + leds_ktimer.function = leds_timer_function; + leds_ktimer.data =0 ; + leds_ktimer.expires = jiffies +LEDS_TIMER_INTERVAL ; +add_timer (&leds_ktimer); + +return ret; +} +subsys_initcall (leds_init); + +static void __exit leds_exit(void) +{ +class_unregister (&leds_class); + +atomic_set (&leds_stop_timer,1 ); + +del_timer_sync (&leds_ktimer); +} +module_exit (leds_exit); + +MODULE_AUTHOR ("John Lenz <[email protected]>" ); +MODULE_LICENSE ("GPL" ); +MODULE_DESCRIPTION ("LED core class interface" ); +
include/linux/leds.h new file +53 -0
53 lines not on this page · raw message
drivers/leds/Kconfig new file +11 -0
@@ -0,0 +1,11 @@ + +menu "LED devices" + +config CLASS_LEDS + tristate "LED support" + help + This option provides the generic support for the leds class. + LEDs can be accessed from /sys/class/leds. It will also allow you + to select individual drivers for LED devices. If unsure, say N. + +endmenu
drivers/leds/Makefile new file +3 -0
@@ -0,0 +1,3 @@ + +# Core functionality. +obj-$(CONFIG_CLASS_LEDS) += ledscore.o
drivers/Kconfig renamed from drivers/Kconfig~leds_sysfs +2 -0
@@ -48,6 +48,8 @@ source "drivers/media/Kconfig" +source "drivers/leds/Kconfig" + source "drivers/video/Kconfig" source "sound/Kconfig"
drivers/Makefile renamed from drivers/Makefile~leds_sysfs +1 -0
@@ -50,4 +50,5 @@ obj-$(CONFIG_MCA) += mca/ obj-$(CONFIG_EISA) += eisa/ obj-$(CONFIG_CPU_FREQ) += cpufreq/ +obj-$(CONFIG_CLASS_LEDS) += leds/ obj-y += firmware/
Quoted 1 line · John Lenz
This is an attempt to provide an alternative to the current arm specific led interface. This arm interface does not integrate well with the device model and sysfs.
I am just curious, but what specific hardware devices can be controlled with this?
[snip]
Quoted 6 lines
function : a read/write attribute that sets the current function of this led. The available options are
timer : the led blinks on and off on a timer
idle : the led turns off when the system is idle and on when not idle power : the led represents the current power state
user : the led is controlled by user space
Please put the power comment in the source too, e.g. :
+enum led_functions {
+ leds_user = 0, /* user has control of this led through sysfs */
+ leds_timer, /* led blinks on a timer */
+ leds_idle, /* led is on when the system is not idle */
+ leds_power, /* led is on when ?????????????????????? */
+};
To be honest, I don't get the meaning of any of the led_functions except leds_user...
Quoted 3 lines
light : a read/write attribute that allows userspace to see the current status of the led. If function="user" then writing to this attribute will change the led on or off. If function != "user" then writing has no effect.
light is an integer, where 0 means off, 1 means on first color, 2
means on second color, etc. (for example, if color="green/blue" then light=1 means turn led on to green and if light=2 means turn led on to blue.
Is something like max_colors or num_colors necessary? Might be handy for changing to the "last" color or something, but it may as well be done by userspace app.
Signature · 3 lines
|| ~~~~~~~~~~~~~~~~~~~~~~ || ( ) http://ThinRope.net/ ( ) || ______________________ ||
Quoted 17 lines · Kalin KOZHUHAROV
Quoted 7 lines
function : a read/write attribute that sets the current function of this led. The available options are
timer : the led blinks on and off on a timer
idle : the led turns off when the system is idle and on when not idle power : the led represents the current power state
user : the led is controlled by user spacePlease put the power comment in the source too, e.g. :
(quoted diff) +6
-0+enum led_functions { + leds_user = 0, /* user has control of this led through sysfs */ + leds_timer, /* led blinks on a timer */ + leds_idle, /* led is on when the system is not idle */ + leds_power, /* led is on when ?????????????????????? */ +};
Quoted 1 line
To be honest, I don't get the meaning of any of the
led_functionsexceptleds_user...
leds_timer means something like a heartbeat (cfr. PPC and m68k)? Or do you need a separate `heartbeat' option for that?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Fri, 2004-09-03 13:54:04 +0900, Kalin KOZHUHAROV <[email protected]> wrote in message <[email protected]>:
Quoted 6 lines
Quoted 3 lines · John Lenz
This is an attempt to provide an alternative to the current arm specific led interface. This arm interface does not integrate well with the device model and sysfs.
I am just curious, but what specific hardware devices can be controlled with this?
For example, the smaller VAX computers offer 8 LEDs which show system status during IPL. After boot finished, the OS may use them...
Quoted 5 lines
Quoted 5 lines
function : a read/write attribute that sets the current function of timer : the led blinks on and off on a timer
idle : the led turns off when the system is idle and on when not idle power : the led represents the current power state
user : the led is controlled by user space
Is idle/timer/power hardware-controlled (eg. by a secondary processor, direct chipset implementation) or is switching on/off controlled by kernel (eg. heartbeat, IO and ethernet for the LEDs you can find on some PA-RISC machines)?
MfG, JBG
Signature · 4 lines
Jan-Benedict Glaw [email protected] . +49-172-7608481 _ O _ "Eine Freie Meinung in einem Freien Kopf | Gegen Zensur | Gegen Krieg _ _ O fuer einen Freien Staat voll Freier Bürger" | im Internet! | im Irak! O O O ret = do_actions((curr | FREE_SPEECH) & ~(NEW_COPYRIGHT_LAW | DRM | TCPA));
Quoted 18 lines · Jan-Benedict Glaw
On Fri, 2004-09-03 13:54:04 +0900, Kalin KOZHUHAROV <[email protected]> wrote in message <[email protected]>:
Quoted 6 lines
Quoted 3 lines · John Lenz
This is an attempt to provide an alternative to the current arm specific led interface. This arm interface does not integrate well with the device model and sysfs.
I am just curious, but what specific hardware devices can be controlled with this?
For example, the smaller VAX computers offer 8 LEDs which show system status during IPL. After boot finished, the OS may use them...
Quoted 5 lines
Quoted 5 lines
function : a read/write attribute that sets the current function of timer : the led blinks on and off on a timer
idle : the led turns off when the system is idle and on when not idle power : the led represents the current power state
user : the led is controlled by user space
Quoted 4 lines
Is idle/timer/power hardware-controlled (eg. by a secondary processor, direct chipset implementation) or is switching on/off controlled by kernel (eg. heartbeat, IO and ethernet for the LEDs you can find on some PA-RISC machines)?
Right now the kernel is in sole control. The device I am testing this on is a Sharp Zaurus SL5500, which has two leds that by default are used to light when new mail arrives and if the power is plugged in.
A read-only interface should probably be added?
The function stuff was intended to be controlled by the kernel, but I think now that I should just remove this and let userspace do it. If we just allow userspace an easy way to turn the leds on and off (and stuff like power state is available elsewhere in sysfs) a simple shell script could turn the led to show the power state.
John
Quoted 9 lines · John Lenz
Quoted 4 lines · Jan-Benedict Glaw
Is idle/timer/power hardware-controlled (eg. by a secondary processor, direct chipset implementation) or is switching on/off controlled by kernel (eg. heartbeat, IO and ethernet for the LEDs you can find on some PA-RISC machines)?
Right now the kernel is in sole control. The device I am testing this on is a Sharp Zaurus SL5500, which has two leds that by default are used to light when new mail arrives and if the power is plugged in.
The kernel is NOT in sole control today on ARM platforms:
echo claim > /sys/devices/system/leds/leds0/event echo red on > /sys/devices/system/leds/leds0/event echo green on > /sys/devices/system/leds/leds0/event echo red off > /sys/devices/system/leds/leds0/event echo release > /sys/devices/system/leds/leds0/event
etc
Sure, we have a weird naming scheme (red, green, amber, blue) but that came around because that's what people were dealing with. There's nothing really stopping us from having any name for a LED in the existing scheme.
I just don't buy the "we must have one sysfs node for every LED" argument.
Signature · 4 lines
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial coreQuoted 24 lines · Russell King
Quoted 2 lines · John Lenz
Quoted 1 line · Jan-Benedict Glaw
Is idle/timer/power hardware-controlled (eg. by a secondary
processor,
Quoted 2 lines
Quoted 2 lines
direct chipset implementation) or is switching on/off controlled by kernel (eg. heartbeat, IO and ethernet for the LEDs you can find on
some
Quoted 3 lines
Quoted 1 line
PA-RISC machines)?
Right now the kernel is in sole control. The device I am testing this
on
Quoted 1 line
is a Sharp Zaurus SL5500, which has two leds that by default are used
to
Quoted 1 line
light when new mail arrives and if the power is plugged in.
The kernel is NOT in sole control today on ARM platforms:
echo claim >
/sys/devices/system/leds/leds0/eventecho red on >/sys/devices/system/leds/leds0/eventecho green on >/sys/devices/system/leds/leds0/eventecho red off >/sys/devices/system/leds/leds0/eventecho release >/sys/devices/system/leds/leds0/event
Quoted 6 lines
etc
Sure, we have a weird naming scheme (red, green, amber, blue) but that came around because that's what people were dealing with. There's nothing really stopping us from having any name for a LED in the existing scheme.
1) This is arm specific. Do any other arches want to provide access to leds? Why should they implement some different api?
2) What happens when you have more than four leds? or 3 dual color leds?
3) no way to read the current status of the led. (could be added to read from /sys/devices/system/leds/leds0 or something)
4) really wierd in-kernel api... The led is associated with a machine, when that sometimes doesn't make sense; code duplication for the same harware drivers. As an example, the led device on the Sharp Zauruses are all the same... exact same code to control the led. Except one model is sa1100 and two are pxa based, so leds-collie.c and leds-poodle.c are exactly the same, except to be registered in the associated leds.c for that mach. This model, instead of associating leds with a specific machine make it a device/driver concept like every other device in the kernel.
We can easily provide backwards compatibility with the arm led code. I have a semi-working driver to register with ledsbase.c and call the leds_event function. As well, the old /sys/devices/system/leds/leds0/event thing can also be emulated.
Quoted 2 lines
I just don't buy the "we must have one sysfs node for every LED" argument.
well, we could just create one attribute per led in /sys/devices/system/leds/leds0/, but that breaks conceptally what sysfs is trying to provide... one device per attribute? 4 devices controlled by ONE attribute?
John
Hi!
Quoted 10 lines
The kernel is NOT in sole control today on ARM platforms:
echo claim >
/sys/devices/system/leds/leds0/eventecho red on >/sys/devices/system/leds/leds0/eventecho green on >/sys/devices/system/leds/leds0/eventecho red off >/sys/devices/system/leds/leds0/eventecho release >/sys/devices/system/leds/leds0/eventetc
Quoted 7 lines
Sure, we have a weird naming scheme (red, green, amber, blue) but that came around because that's what people were dealing with. There's nothing really stopping us from having any name for a LED in the existing scheme.
I just don't buy the "we must have one sysfs node for every LED" argument.
sysfs is one-file-one-value. We do not want to end up with /proc-like mess.
Pavel
Quoted 17 lines · Pavel Machek
Hi!
Quoted 15 lines
The kernel is NOT in sole control today on ARM platforms:
echo claim >
/sys/devices/system/leds/leds0/eventecho red on >/sys/devices/system/leds/leds0/eventecho green on >/sys/devices/system/leds/leds0/eventecho red off >/sys/devices/system/leds/leds0/eventecho release >/sys/devices/system/leds/leds0/eventetc
Sure, we have a weird naming scheme (red, green, amber, blue) but that came around because that's what people were dealing with. There's nothing really stopping us from having any name for a LED in the existing scheme.
Quoted 5 lines
Quoted 2 lines
I just don't buy the "we must have one sysfs node for every LED" argument.
sysfs is one-file-one-value. We do not want to end up with /proc-like mess.
In which case I protest in the strongest terms that having one device node plus attributes _PER_ _LED_ is just fscking stupid. What if you have an embedded machine with 32 LEDs? Do you _really_ need all that extra data just to support sysfs so that maybe you can control them from userspace?
What next? One sysfs node plus attributes per GPIO line? How about we do one sysfs node per virtual memory bit so people can control anything in their system on a bit granularity without needing mmap or any other interfaces? When does this madness stop?
It comes down to this:
- is a single LED in one package one device?
- is a set of two LEDs in one package one device?
- is a set of three LEDs in one package a device?
- what about a bank of 8 LEDs grouped together?
- what about 4 banks of 8 LEDs grouped together?
- what about 7 segment numeric LED displays?
- what about 14 segment alphanumeric LED displays?
All of these are LED devices. Does one sysfs node per individual LED element _really_ make sense for all these cases? I think not.
Signature · 4 lines
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial coreHi!
Quoted 8 lines
Quoted 2 lines
sysfs is one-file-one-value. We do not want to end up with /proc-like mess.
In which case I protest in the strongest terms that having one device node plus attributes _PER_ _LED_ is just fscking stupid. What if you have an embedded machine with 32 LEDs? Do you _really_ need all that extra data just to support sysfs so that maybe you can control them from userspace?
If you do not need that 32 files on that embedded system, do not enable that in config... I do not see what is wrong with two or three files per led.
Quoted 4 lines
What next? One sysfs node plus attributes per GPIO line? How about we do one sysfs node per virtual memory bit so people can control anything in their system on a bit granularity without needing mmap or any other interfaces? When does this madness stop?
GPIO lines are obviously system specific, I guess they could go to /proc or be controlled via ioctl()... But that was attempt at universal LED interface, right?
Pavel
Quoted 8 lines · Pavel Machek
Quoted 4 lines
What next? One sysfs node plus attributes per GPIO line? How about we do one sysfs node per virtual memory bit so people can control anything in their system on a bit granularity without needing mmap or any other interfaces? When does this madness stop?
GPIO lines are obviously system specific, I guess they could go to /proc or be controlled via
ioctl()... But that was attempt at universal LED interface, right?
Yep. As well, the GPIO interface proposed by Robert Schwebel should be kept seperate... I don't see a reason or anything useful from unifying them.
John
John,
Quoted 3 lines · John Lenz
This is an attempt to provide an alternative to the current arm specific led interface. This arm interface does not integrate well with the device model and sysfs.
I have written a GPIO device class driver for the same purpose; before this goes into the kernel I think we should try to merge your attempts with mine.
On embedded systems you normally have several things which are similar to LEDs; embedded processors have quite a lot of general purpose I/O pins. Normally you want some of the pins being used from userspace (like 'echo 1 > /sys/class/gpio/gpio5/level'), others are used from device drivers. My framework offers a request_gpio() function similar to request_gpio(), so the kernel can administrate these ressources.
I suppose it is not too difficult to unify our drivers in a way that the base mechanism is more abstract then LEDs (a GPIO pin can also represent a power switch or whatever) and still can support your LED levels.
I'll pull the gpio patch out of my working tree and post it here for discussion.
Robert
Signature · 5 lines
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Hornemannstraße 12, 31137 Hildesheim, Germany
Phone: +49-5121-28619-0 | Fax: +49-5121-28619-4Quoted 2 lines · Robert Schwebel
I'll pull the gpio patch out of my working tree and post it here for discussion.
Attached. Parts of it have been taken from your LEDs patch anyway, so it should be not too difficult to reunify them.
Robert
Signature · 5 lines
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Hornemannstraße 12, 31137 Hildesheim, Germany
Phone: +49-5121-28619-0 | Fax: +49-5121-28619-4kernel/gpio.c +459 -0
@@ -0,0 +1,459 @@ +/* +* linux/kernel/gpio.c +* +* (C) 2004 Robert Schwebel, Pengutronix +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#include <linux/config.h> +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/list.h> +#include <linux/spinlock.h> +#include <linux/device.h> +#include <linux/sysdev.h> +#include <linux/timer.h> +#include <linux/proc_fs.h> +#include <linux/gpio.h> + +#define DRIVER_NAME "gpio" + +static char mapping[255 ] ="" ; + +MODULE_AUTHOR ("John Lenz, Robert Schwebel" ); +MODULE_LICENSE ("GPL" ); +MODULE_DESCRIPTION ("Generic GPIO Infrastructure" ); +module_param_string (mapping, mapping,sizeof (mapping),0 ); +MODULE_PARM_DESC (mapping, +"period delimited options string to map GPIO pins to userland:\n" +"\n" +" <pin>:[out|in][hi|lo]\n" +"\n" +" example: mapping=5:out:hi.8:in\n" +); + +static ssize_t gpio_show_level (struct class_device *dev,char *buf); +static ssize_t gpio_store_level (struct class_device *dev,const char *buf,size_t size); +static ssize_t gpio_show_policy (struct class_device *dev,char *buf); + +struct gpio_properties { +unsigned int pin_nr; +unsigned char policy;/* GPIO_xxx */ +char pin_level;/* -1=tristate, 0, 1 */ +char owner[20 ]; +struct gpio_device *gpio_dev; +}; + +struct gpio_device { +spinlock_t lock;/* protects the props field */ +struct gpio_properties props; +struct class_device class_dev; +struct list_head list; +}; +#define to_gpio_device (d) container_of(d, struct gpio_device, class_dev) + +static LIST_HEAD (gpio_list); +static rwlock_t gpio_list_lock =RW_LOCK_UNLOCKED ; + +/* gpio_device is static, so we don't have to free it here */ +static void gpio_class_release (struct class_device *dev) +{ +return ; +} + +static struct class gpio_class = { + .name ="gpio" , + .release = gpio_class_release, +}; + + +/* +* Attribute: /sys/class/gpio/gpioX/level +*/ +static struct class_device_attribute attr_gpio_level = { + .attr = { .name ="level" , .mode =0644 , .owner =THIS_MODULE }, + .show = gpio_show_level, + .store = gpio_store_level, +}; + +static ssize_t gpio_show_level (struct class_device *dev,char *buf) +{ +struct gpio_device *gpio_dev =to_gpio_device (dev); +ssize_t ret_size =0 ; + +if (gpio_dev->props.policy &GPIO_INPUT ) + gpio_dev->props.pin_level =gpio_get_pin (gpio_dev->props.pin_nr); + +spin_lock (&gpio_dev->lock); + ret_size +=sprintf (buf,"%i\n" , gpio_dev->props.pin_level); +spin_unlock (&gpio_dev->lock); + +return ret_size; +} + +static ssize_t gpio_store_level (struct class_device *dev,const char *buf,size_t size) +{ +struct gpio_device *gpio_dev =to_gpio_device (dev); +long value; + +if (gpio_dev->props.policy &GPIO_INPUT ) +return -EINVAL ; + + value =simple_strtol (buf,NULL ,10 ); +if ((value !=0 ) && (value !=1 )) +return -EINVAL ; + gpio_dev->props.pin_level = value; + +/* set real hardware */ +switch (value) { +case 0 :gpio_clear_pin (gpio_dev->props.pin_nr); +gpio_dir_output (gpio_dev->props.pin_nr); +break ; +case 1 :gpio_set_pin (gpio_dev->props.pin_nr); +gpio_dir_output (gpio_dev->props.pin_nr); +break ; +default :break ; + } +return size; +} + +/* +* Attribute: /sys/class/gpio/gpioX/policy +*/ +static struct class_device_attribute attr_gpio_policy = { + .attr = { .name ="policy" , .mode =0444 , .owner =THIS_MODULE }, + .show = gpio_show_policy, + .store =NULL , +}; + +static ssize_t gpio_show_policy (struct class_device *dev,char *buf) +{ +struct gpio_device *gpio_dev =to_gpio_device (dev); +ssize_t ret_size =0 ; + +spin_lock (&gpio_dev->lock); +if (gpio_dev->props.policy &GPIO_USER ) + ret_size +=sprintf (buf,"userspace\n" ); +if (gpio_dev->props.policy &GPIO_KERNEL ) + ret_size +=sprintf (buf,"kernel\n" ); +spin_unlock (&gpio_dev->lock); + +return ret_size; +} + +static int gpio_read_proc (char *page,char **start,off_t off, +int count,int *eof,void *data) +{ +char *p = page; +int size =0 ; +struct gpio_device *gpio_dev; +struct list_head *lact, *ltmp; + +if (off !=0 ) + goto end; + + p +=sprintf (p,"GPIO POLICY DIRECTION OWNER\n" ); +list_for_each_safe (lact, ltmp, &gpio_list) { + gpio_dev = list_entry(lact, struct gpio_device, list); + p +=sprintf (p,"%3i: " , gpio_dev->props.pin_nr); +if (gpio_dev->props.policy &GPIO_KERNEL ) + p +=sprintf (p,"kernel " ); +if (gpio_dev->props.policy &GPIO_USER ) + p +=sprintf (p,"user space " ); +if (gpio_dev->props.policy &GPIO_INPUT ) + p +=sprintf (p,"input " ); +if (gpio_dev->props.policy &GPIO_OUTPUT ) + p +=sprintf (p,"output " ); + p +=sprintf (p,"%s\n" , gpio_dev->props.owner); + } +end: + size = (p - page); +if (size <= off + count) + *eof =1 ; + *start = page + off; + size -= off; +if (size > count) + size = count; +if (size <0 ) + size =0 ; + +return size; +} + +/** +* request_gpio - register a new object of gpio_device class. +* +* @pin_nr: GPIO pin which is registered +* @owner: name of the driver that owns this pin +* @policy: set policy for this pin, which is one of these: +* - GPIO_USER or GPIO_KERNEL +* - GPIO_INPUT or GPIO_OUTPUT +* For user space registered pins a sysfs entry is added. +* @init_level: initially configured pin level +*/ +int request_gpio (unsigned int pin_nr,const char *owner, +unsigned char policy,unsigned char init_level) +{ +int rc; +struct gpio_device *gpio_dev; +struct list_head *lact, *ltmp; + +list_for_each_safe (lact, ltmp, &gpio_list) { + gpio_dev = list_entry(lact, struct gpio_device, list); +if (pin_nr == gpio_dev->props.pin_nr) { +printk (KERN_ERR "gpio pin %i is already used by %s\n" , + pin_nr, gpio_dev->props.owner); +return -EBUSY ; + } + } + +/* value checks - FIXME: is there a logical xor in C? */ +if ( ( (policy &GPIO_USER ) && (policy &GPIO_KERNEL )) || + (!(policy &GPIO_USER ) && !(policy &GPIO_KERNEL ))) { +printk (KERN_ERR "%s: policy has to be one of GPIO_KERNEL, GPIO_USER\n" ,DRIVER_NAME ); +return -EINVAL ; + } +if ( ( (policy &GPIO_INPUT ) && (policy &GPIO_OUTPUT )) || + (!(policy &GPIO_INPUT ) && !(policy &GPIO_OUTPUT ))) { +printk (KERN_ERR "%s: policy has to be one of GPIO_INPUT, GPIO_OUTPUT\n" ,DRIVER_NAME ); +return -EINVAL ; + } + + gpio_dev =kmalloc (sizeof (struct gpio_device ),GFP_KERNEL ); + +if (unlikely (!gpio_dev)) { +printk (KERN_ERR "%s: couldn't allocate memory\n" ,DRIVER_NAME ); +return -ENOMEM ; + } + +spin_lock_init (&gpio_dev->lock); + gpio_dev->props.pin_nr = pin_nr; + gpio_dev->props.policy = policy; + gpio_dev->props.pin_level = init_level; + gpio_dev->props.gpio_dev = gpio_dev; +strncpy (gpio_dev->props.owner, owner,20 ); + +memset (&gpio_dev->class_dev,0 ,sizeof (gpio_dev->class_dev)); + gpio_dev->class_dev.class = &gpio_class; +snprintf (gpio_dev->class_dev.class_id,BUS_ID_SIZE ,"gpio%i" , pin_nr); + + rc =class_device_register (&gpio_dev->class_dev); +if (unlikely (rc)) { +printk (KERN_ERR "%s: class registering failed\n" ,DRIVER_NAME ); +kfree (gpio_dev); +return rc; + } + +INIT_LIST_HEAD (&gpio_dev->list); + +/* register the attributes */ +if (policy &GPIO_USER ) +class_device_create_file (&gpio_dev->class_dev, &attr_gpio_level); + +class_device_create_file (&gpio_dev->class_dev, &attr_gpio_policy); + +/* set real hardware */ +if (policy &GPIO_OUTPUT ) { +switch (init_level) { +case 0 :gpio_clear_pin (pin_nr);break ; +case 1 :gpio_set_pin (pin_nr);break ; +default :break ; + } +gpio_dir_output (pin_nr); + } + +write_lock (&gpio_list_lock); +list_add_tail (&gpio_dev->list, &gpio_list); +write_unlock (&gpio_list_lock); + +printk (KERN_INFO "registered gpio%i\n" , pin_nr); + +return 0 ; +} +EXPORT_SYMBOL (request_gpio); + +/** +* free_gpio - unregisters a object of gpio_properties class. +* +* @pin_nr: pin number to free. +* +* Unregisters a previously registered via request_gpio object. +*/ +void free_gpio (unsigned int pin_nr) +{ +struct gpio_device *gpio_dev; +struct list_head *lact, *ltmp; + +list_for_each_safe (lact, ltmp, &gpio_list) { + gpio_dev = list_entry(lact, struct gpio_device, list); +if (pin_nr == gpio_dev->props.pin_nr) { + +printk (KERN_INFO "unregistering gpio pin %i\n" , pin_nr); + +/* unregister attributes */ +if (gpio_dev->props.policy &GPIO_USER ) +class_device_remove_file (&gpio_dev->class_dev, + &attr_gpio_level); + +class_device_remove_file (&gpio_dev->class_dev, &attr_gpio_level); + +class_device_unregister (&gpio_dev->class_dev); +write_lock (&gpio_list_lock); +list_del (&gpio_dev->list); +write_unlock (&gpio_list_lock); +kfree (gpio_dev); +return ; + } + } +} +EXPORT_SYMBOL (free_gpio); + +void free_all_gpios (void ) +{ +struct gpio_device *gpio_dev; +struct list_head *lact, *ltmp; + +list_for_each_safe (lact, ltmp, &gpio_list) { + gpio_dev = list_entry(lact, struct gpio_device, list); + +printk (KERN_INFO "unregistering gpio pin %i\n" , gpio_dev->props.pin_nr); + +/* unregister attributes */ +if (gpio_dev->props.policy &GPIO_USER ) +class_device_remove_file (&gpio_dev->class_dev, + &attr_gpio_level); + +class_device_remove_file (&gpio_dev->class_dev, &attr_gpio_level); + +class_device_unregister (&gpio_dev->class_dev); +write_lock (&gpio_list_lock); +list_del (&gpio_dev->list); +write_unlock (&gpio_list_lock); +kfree (gpio_dev); +return ; + } +} +EXPORT_SYMBOL (free_all_gpios); + +static struct sysdev_class gpio_sysclass = { +set_kset_name ("gpio" ), +}; + +static struct sys_device gpio_sys_device = { + .id =0 , + .cls = &gpio_sysclass, +}; + +static int gpio_setup (char *s) +{ +char *p, *q =NULL ; +int pin_nr; +unsigned char policy; +unsigned char init_level =0 ; + +while ((p =strsep (&s,".," )) !=NULL ) { + +/* end of command reached? -> next command */ +if (*p =='\0' ) +continue ; + +/* process one command */ + pin_nr =-1 ; + policy =0 ; + q =strsep (&p,":" ); +if (*q =='\0' ) { +printk ("%s: invalid token (scanning pin_nr): %s\n" ,DRIVER_NAME , p); +continue ; + } + pin_nr =simple_strtoul (q,NULL ,0 );/* FIXME: this doesn't detect if no number is given! */ + q =strsep (&p,":" ); +if (*q =='\0' ) { +printk ("%s: invalid token (scanning direction): %s\n" ,DRIVER_NAME , p); +continue ; + } + policy |=strncmp (q,"in" ,strlen (q)) ==0 ?GPIO_INPUT :0 ; + policy |=strncmp (q,"out" ,strlen (q)) ==0 ?GPIO_OUTPUT :0 ; +if (policy ==0 ) { +printk ("%s: invalid token (scanning policy): %s\n" ,DRIVER_NAME , p); +continue ; + } +if (policy &GPIO_OUTPUT ) { + q =strsep (&p,":" ); +if (*q =='\0' ) { +printk ("%s: invalid token (scanning init_level): %s\n" ,DRIVER_NAME , p); +continue ; + } + init_level =2 ; +if (strncmp (q,"hi" ,strlen (q)) ==0 ) + init_level =1 ; +if (strncmp (q,"lo" ,strlen (q)) ==0 ) + init_level =0 ; +if (init_level ==2 ) { +printk ("%s: invalid token (scanning level_value): %s\n" ,DRIVER_NAME , p); +continue ; + } + } + policy |=GPIO_USER ; +if (request_gpio (pin_nr,"kernel" , policy, init_level) !=0 ) { +printk (KERN_ERR +"%s: could not register GPIO pins from commandline!\n" , +DRIVER_NAME ); +return -EIO ; + } + } +return 0 ; +} + +static int __init gpio_init (void ) +{ +int ret; + +printk (KERN_INFO "Initialising gpio device class.\n" ); + +class_register (&gpio_class); + + ret =sysdev_class_register (&gpio_sysclass); +if (ret) { +printk (KERN_ERR "%s: couldn't register sysdev class, exiting\n" ,DRIVER_NAME ); + goto out_sysdev_class; + } + + ret =sysdev_register (&gpio_sys_device); +if (ret) { +printk (KERN_ERR "%s: couldn't register sysdev, exiting\n" ,DRIVER_NAME ); + goto out_sysdev_register; + } + +if (!create_proc_read_entry ("gpio" ,0 ,0 , gpio_read_proc,NULL )) { +printk (KERN_ERR "%s: couldn't register proc entry, exiting\n" ,DRIVER_NAME ); + goto out_proc; + } + +/* parse command line parameters */ +printk (KERN_INFO "%s: mapping=" ,DRIVER_NAME ); +if (strcmp (mapping,"" )) { +printk ("%s\n" , mapping); +if (gpio_setup (mapping) !=0 ) { +printk (KERN_ERR "%s: could not register ('mapping=...'), exiting\n" ,DRIVER_NAME ); + } + }else { +printk ("EMPTY\n" ); + }; + +return ret; + +out_proc: +sysdev_unregister (&gpio_sys_device); +out_sysdev_register: +sysdev_class_unregister (&gpio_sysclass); +out_sysdev_class: +class_unregister (&gpio_class); +return ret; +} + +module_init (gpio_init);
kernel/Makefile +1 -0
@@ -23,6 +23,7 @@ obj-$(CONFIG_STOP_MACHINE) += stop_machine.o obj-$(CONFIG_AUDIT) += audit.o obj-$(CONFIG_AUDITSYSCALL) += auditsc.o +obj-$(CONFIG_GPIO) += gpio.o ifneq ($(CONFIG_IA64),y) # According to Alan Modra <[email protected]>, the -fno-omit-frame-pointer is
include/asm-arm/arch-pxa/gpio.h +49 -0
49 lines not on this page · raw message
include/linux/gpio.h +27 -0
@@ -0,0 +1,27 @@ +/* +* include/linux/gpio.h +* +* Copyright (C) 2004 Robert Schwebel, Pengutronix +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#ifndef __GPIO_H +#define __GPIO_H + +#include "asm/arch/gpio.h" + +/* Values for policy */ +#define GPIO_KERNEL (1<<0) +#define GPIO_USER (1<<1) +#define GPIO_INPUT (1<<2) +#define GPIO_OUTPUT (1<<3) + +int request_gpio (unsigned int pin_nr,const char *owner, +unsigned char policy,unsigned char init_level); + +void free_gpio (unsigned int pin_nr); + +#endif
init/Kconfig +12 -0
@@ -294,6 +294,18 @@ If unsure, say N. +config GPIO + bool "GPIO pin support" + default y if ARM || PPC + default n + help + Enabling this option adds support for generic GPIO pins. Most + System-on-Chip processors have this kind of pins. + + FIXME: write more documentation. + + If unsure, say N. + endmenu # General setup
Am Freitag, 3. September 2004 15:31 schrieb Robert Schwebel:
Quoted 8 lines · Robert Schwebel
Quoted 2 lines
I'll pull the gpio patch out of my working tree and post it here for discussion.
Attached. Parts of it have been taken from your LEDs patch anyway, so it should be not too difficult to reunify them.
Robert
+ list_for_each_safe(lact, ltmp, &gpio_list) {
+ gpio_dev = list_entry(lact, struct gpio_device, list);
+ if (pin_nr == gpio_dev->props.pin_nr) {
+ printk(KERN_ERR "gpio pin %i is already used by %s\n",
+ pin_nr, gpio_dev->props.owner);
+ return -EBUSY;
+ }
+ }
[..]
+ spin_lock_init(&gpio_dev->lock);
+ gpio_dev->props.pin_nr = pin_nr;
This looks like a race condition. You need to check and reserve under a lock.
Regards
Oliver
Hi!
Quoted 13 lines
This is an attempt to provide an alternative to the current arm specific led interface. This arm interface does not integrate well with the device model and sysfs.
We create a new class that drivers can register a
leds_propertiesstructure with. Each led that is registered is assigned a number, and three attributes are exported to sysfs in/sys/class/leds/1/,/sys/class/leds/2, etc.color : a read only string attribute that shows the available colors of this led. If it is a multi-color led, then the colors are seperated by a "/" (for example "green/blue").
Quoted 7 lines
function : a read/write attribute that sets the current function of this led. The available options are
timer : the led blinks on and off on a timer
idle : the led turns off when the system is idle and on when not idle power : the led represents the current power state
user : the led is controlled by user space
I'm afraid this is really good idea. It seems quite overengineered to me (and I'd be afraid that idle part slows machine). Perhaps having only "user" mode is better idea?
Quoted 8 lines
light : a read/write attribute that allows userspace to see the current status of the led. If function="user" then writing to this attribute will change the led on or off. If function != "user" then writing has no effect.
light is an integer, where 0 means off, 1 means on first color, 2
means on second color, etc. (for example, if color="green/blue" then light=1 means turn led on to green and if light=2 means turn led on to blue.
Is there ways to turn on both?
Pavel
Signature · 1 line
When do you have heart between your knees?
Quoted 11 lines · Pavel Machek
Quoted 7 lines
function : a read/write attribute that sets the current function of this led. The available options are
timer : the led blinks on and off on a timer
idle : the led turns off when the system is idle and on when not idle power : the led represents the current power state
user : the led is controlled by user spaceI'm afraid this is really good idea. It seems quite overengineered to me (and I'd be afraid that idle part slows machine). Perhaps having only "user" mode is better idea?
I was only mimicing the support currently in the arm led code.
After thinking about it from a broader perspective of including GPIOs,
we should probably get rid of this function thing entirely. Just let user space do everything... userspace can monitor sysfs and hotplug and have the led represent power or idle or whatever.
Quoted 11 lines
Quoted 10 lines
light : a read/write attribute that allows userspace to see the current
status of the led. If function="user" then writing to this attribute will change the led on or off. If function != "user" then writing has no effect.
light is an integer, where 0 means off, 1 means on first color, 2
means on second color, etc. (for example, if color="green/blue" then light=1 means turn led on to green and if light=2 means turn led on to blue.
Quoted 1 line
Is there ways to turn on both?
depends on the hardware...
The led class does not really inforce any policy, it just passes this number along to the actual driver that is registered. So say you had a led that could be red, green, or both red and green at the same time (not sure how that would work hardware wise, but ok). The driver could do something like set color="red/green/red&green" and then use 0,1,2,3. The kernel is just reflecting the possible states the hardware could be in.
John
Quoted 13 lines · John Lenz
Quoted 11 lines · Pavel Machek
Quoted 7 lines
function : a read/write attribute that sets the current function of this led. The available options are
timer : the led blinks on and off on a timer
idle : the led turns off when the system is idle and on when not idle power : the led represents the current power state
user : the led is controlled by user spaceI'm afraid this is really good idea. It seems quite overengineered to me (and I'd be afraid that idle part slows machine). Perhaps having only "user" mode is better idea?
Quoted 5 lines
I was only mimicing the support currently in the arm led code. After thinking about it from a broader perspective of including GPIOs, we should probably get rid of this function thing entirely. Just let user space do everything... userspace can monitor sysfs and hotplug and have the led represent power or idle or whatever.
Bear in mind that my _original_ reason for implementing some of this was to tell what's going on with the kernel on my machines. I'm fairly opposed to shifting it to userspace just because someone wants a bloated sysfs interface to drive some tiddly little LEDs and then having to losing the benefits of the existing implementation.
Quoted 4 lines
The led class does not really inforce any policy, it just passes this number along to the actual driver that is registered. So say you had a led that could be red, green, or both red and green at the same time (not sure how that would work hardware wise, but ok).
See Netwinders. They have a bi-colour LED which has independent red and green LEDs in the same package. When they're both on it's yellow.
It's _VERY_ useful to see the green LED flashing and know that the headless machine is running, or that the red LED being on means that either it hasn't booted a kernel yet or the system has successfully shut down.
It means I don't have to fsck around with monitor cables before pulling the power.
And no, doing that from userspace won't work because userspace is dead _before_ the system has finished shutting down (see drive cache flushing on shutdown.)
It's also _VERY_ useful to know whether the kernel has managed to get far enough through the boot that the heartbeat LED is flashing but maybe not sufficiently to bring up the serial console as well - again another thing that a userspace implementation will never be able to support.
So sorry... I'm definitely opposed.
Signature · 4 lines
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial coreQuoted 20 lines · Russell King
Quoted 6 lines · John Lenz
Quoted 5 lines · Pavel Machek
Quoted 5 lines
function : a read/write attribute that sets the current function of this led. The available options are
timer : the led blinks on and off on a timer
idle : the led turns off when the system is idle and on when notidle
Quoted 10 lines
Quoted 6 lines
Quoted 2 lines
power : the led represents the current power state user : the led is controlled by user space
I'm afraid this is really good idea. It seems quite overengineered to me (and I'd be afraid that idle part slows machine). Perhaps having only "user" mode is better idea?
I was only mimicing the support currently in the arm led code. After thinking about it from a broader perspective of including GPIOs, we should probably get rid of this function thing entirely. Just let
user
Quoted 1 line
space do everything... userspace can monitor sysfs and hotplug and have
Quoted 8 lines
the
Quoted 1 line
led represent power or idle or whatever.
Bear in mind that my _original_ reason for implementing some of this was to tell what's going on with the kernel on my machines. I'm fairly opposed to shifting it to userspace just because someone wants a bloated sysfs interface to drive some tiddly little LEDs and then having to losing the benefits of the existing implementation.
Yea, but there are other uses than developing and debugging. For example, handheld environments use the leds to display when new mail comes, could also turn on the led based on an entry in the calandar to notify the user that some event is coming up, etc.
It's not really intended for debugging, because you can't control the leds until after module_init/device_initcall.
Quoted 21 lines
Quoted 5 lines
The led class does not really inforce any policy, it just passes this number along to the actual driver that is registered. So say you had a led that could be red, green, or both red and green at the same time (not sure how that would work hardware wise, but ok).
See Netwinders. They have a bi-colour LED which has independent red and green LEDs in the same package. When they're both on it's yellow.
It's _VERY_ useful to see the green LED flashing and know that the headless machine is running, or that the red LED being on means that either it hasn't booted a kernel yet or the system has successfully shut down.
It means I don't have to fsck around with monitor cables before pulling the power.
And no, doing that from userspace won't work because userspace is dead _before_ the system has finished shutting down (see drive cache flushing on shutdown.)
Quoted 5 lines
It's also _VERY_ useful to know whether the kernel has managed to get far enough through the boot that the heartbeat LED is flashing but maybe not sufficiently to bring up the serial console as well - again another thing that a userspace implementation will never be able to support.
Hey I agree! I use the led as a debugging tool on the Sharp Zaurus. Before I had the framebuffer or the serial port working, being able to see the led told me something was working :) This patch isn't really intended for debugging support, so perhaps some other interface could be added.
On the other hand, perhaps we can just do a #ifdef LEDS_DEBUGGING that would toggle the led on a timer.
John
Quoted 26 lines · John Lenz
Quoted 20 lines
Quoted 4 lines
The led class does not really inforce any policy, it just passes this number along to the actual driver that is registered. So say you had a led that could be red, green, or both red and green at the same time (not sure how that would work hardware wise, but ok).
See Netwinders. They have a bi-colour LED which has independent red and green LEDs in the same package. When they're both on it's yellow.
It's _VERY_ useful to see the green LED flashing and know that the headless machine is running, or that the red LED being on means that either it hasn't booted a kernel yet or the system has successfully shut down.
It means I don't have to fsck around with monitor cables before pulling the power.
And no, doing that from userspace won't work because userspace is dead _before_ the system has finished shutting down (see drive cache flushing on shutdown.)
Quoted 5 lines
It's also _VERY_ useful to know whether the kernel has managed to get far enough through the boot that the heartbeat LED is flashing but maybe not sufficiently to bring up the serial console as well - again another thing that a userspace implementation will never be able to support.
Quoted 7 lines
Hey I agree! I use the led as a debugging tool on the Sharp Zaurus. Before I had the framebuffer or the serial port working, being able to see the led told me something was working :) This patch isn't really intended for debugging support, so perhaps some other interface could be added.
On the other hand, perhaps we can just do a #ifdef
LEDS_DEBUGGINGthat would toggle the led on a timer.
You missed the point. The above scenarios I was describing are _not_ debugging.
Signature · 4 lines
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial coreQuoted 3 lines · Russell King
You missed the point. The above scenarios I was describing are _not_ debugging.
Ok, so we get rid of the function thing, but still provide a heartbeat_enabled entry in sysfs. Something like this patch
#
# Patch managed by http://www.holgerschurig.de/patcher.html
#
drivers/leds/ledscore.c new file +281 -0
@@ -0,0 +1,281 @@ +/* +* linux/drivers/leds/ledscore.c +* +* Copyright (C) 2004 John Lenz <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +*/ + +#include <linux/config.h> +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/list.h> +#include <linux/spinlock.h> +#include <linux/device.h> +#include <linux/sysdev.h> +#include <linux/timer.h> +#include <linux/leds.h> + +#define LEDS_TIMER_INTERVAL (HZ * 5) + +struct led_device { +/* This protects the props field.*/ +spinlock_t lock; +/* If props is NULL, the driver that registered this device has been unloaded */ +struct led_properties *props; +struct class_device class_dev; +struct list_head list; +}; + +#define to_led_device (d) container_of(d, struct led_device, class_dev) + +static rwlock_t leds_list_lock =RW_LOCK_UNLOCKED ; +static LIST_HEAD (leds_list); +static atomic_t leds_count =ATOMIC_INIT (0 ); + +static void leds_class_release (struct class_device *dev) +{ +struct led_device *d =to_led_device (dev); + +write_lock (&leds_list_lock); +list_del (&d->list); +write_unlock (&leds_list_lock); + +kfree (d); +} + +static struct class leds_class = { + .name ="leds" , + .release = leds_class_release, +}; + +static ssize_t leds_show_color (struct class_device *dev,char *buf) +{ +struct led_device *led_dev =to_led_device (dev); +ssize_t ret =0 ; + +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { +sprintf (buf,"%s\n" , led_dev->props->color); + ret =strlen (buf) +1 ; + } +spin_unlock (&led_dev->lock); + +return ret; +} + +static CLASS_DEVICE_ATTR(color, 0444, leds_show_color, NULL); + +static ssize_t leds_show_light (struct class_device *dev,char *buf) +{ +struct led_device *led_dev =to_led_device (dev); +ssize_t ret =0 ; + +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { +sprintf (buf,"%lu\n" , led_dev->props->light_state); + ret =strlen (buf) +1 ; + } +spin_unlock (&led_dev->lock); + +return ret; +} + +static ssize_t leds_store_light (struct class_device *dev,const char *buf,size_t size) +{ +struct led_device *led_dev =to_led_device (dev); +int ret = -EINVAL ; +char *after; + +unsigned long state =simple_strtoul (buf, &after,10 ); +if (after - buf >0 ) { + ret = after - buf; +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { +if (!led_dev->props->heartbeat_enabled) { + led_dev->props->light_state = state; +if (led_dev->props->light) + led_dev->props->light (led_dev->class_dev.dev, led_dev->props); + } + } +spin_unlock (&led_dev->lock); + } + +return ret; +} + +static CLASS_DEVICE_ATTR(light, 0644, leds_show_light, leds_store_light); + +static ssize_t leds_show_heartbeat (struct class_device *dev,char *buf) +{ +struct led_device *led_dev =to_led_device (dev); +ssize_t ret =0 ; + +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { +sprintf (buf,"%lu\n" , led_dev->props->heartbeat_enabled); + ret =strlen (buf) +1 ; + } +spin_unlock (&led_dev->lock); + +return ret; +} + +static ssize_t leds_store_heartbeat (struct class_device *dev,const char *buf,size_t size) +{ +struct led_device *led_dev =to_led_device (dev); +int ret = -EINVAL ; + +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { +if (buf[0 ] =='1' ) { + led_dev->props->heartbeat_enabled =1 ; + }else { + led_dev->props->heartbeat_enabled =0 ; + } + } +spin_unlock (&led_dev->lock); + +return ret; +} + +static CLASS_DEVICE_ATTR(heartbeat_enabled, 0644, leds_show_heartbeat, leds_store_heartbeat); + +/** +* leds_device_register - register a new object of led_device class. +* @dev: The device to register. +* @prop: the led properties structure for this device. +*/ +int leds_device_register (struct device *dev,struct led_properties *props) +{ +int rc, num; +struct led_device *new_led; + + new_led =kmalloc (sizeof (struct led_device ),GFP_KERNEL ); +if (unlikely (!new_led)) +return -ENOMEM ; + +spin_lock_init (&new_led->lock); + new_led->props = props; + props->led_dev = new_led; + +memset (&new_led->class_dev,0 ,sizeof (new_led->class_dev)); + new_led->class_dev.class = &leds_class; + new_led->class_dev.dev = dev; + +/* assign this led a number */ + num =atomic_add_return (1 , &leds_count); +sprintf (new_led->class_dev.class_id,"%i" , num); + + rc =class_device_register (&new_led->class_dev); +if (unlikely (rc)) { +kfree (new_led); +return rc; + } + +/* register the attributes */ +class_device_create_file (&new_led->class_dev, &class_device_attr_color); +class_device_create_file (&new_led->class_dev, &class_device_attr_light); +class_device_create_file (&new_led->class_dev, &class_device_attr_heartbeat_enabled); + +/* add to the list of leds */ +write_lock (&leds_list_lock); +list_add_tail (&new_led->list, &leds_list); +write_unlock (&leds_list_lock); + +printk (KERN_INFO "Registered led device: number=%s, color=%s\n" , new_led->class_dev.class_id, props->color); + +return 0 ; +} +EXPORT_SYMBOL (leds_device_register); + +/** +* led_device_unregister - unregisters a object of led_properties class. +* @props: the property to unreigister +* +* Unregisters a previously registered via led_device_register object. +*/ +void leds_device_unregister (struct led_properties *props) +{ +struct led_device *led_dev; +if (!props) +return ; + +pr_debug ("led_device_unregister: color=%s\n" , props->color); + + led_dev = props->led_dev; + +class_device_remove_file (&led_dev->class_dev, &class_device_attr_heartbeat_enabled); +class_device_remove_file (&led_dev->class_dev, &class_device_attr_light); +class_device_remove_file (&led_dev->class_dev, &class_device_attr_color); + +spin_lock (&led_dev->lock); + led_dev->props =NULL ; + props->led_dev =NULL ; +spin_unlock (&led_dev->lock); + +class_device_unregister (&led_dev->class_dev); +} +EXPORT_SYMBOL (leds_device_unregister); + +/* set up a kernel timer */ +static struct timer_list leds_ktimer; +static atomic_t leds_stop_timer =ATOMIC_INIT (0 ); + +static void leds_timer_function (unsigned long data) +{ +struct led_device *led_dev; + +read_lock (&leds_list_lock); +list_for_each_entry (led_dev, &leds_list, list) { +spin_lock (&led_dev->lock); +if (likely (led_dev->props)) { +if (led_dev->props->heartbeat_enabled) { + led_dev->props->light_state = !led_dev->props->light_state; +if (led_dev->props->light) + led_dev->props->light (led_dev->class_dev.dev, led_dev->props); + } + } +spin_unlock (&led_dev->lock); + } +read_unlock (&leds_list_lock); + +if (!atomic_read (&leds_stop_timer)) +mod_timer (&leds_ktimer, jiffies +LEDS_TIMER_INTERVAL ); +} + +static int __init leds_init (void ) +{ +int ret =0 ; + +/* initialize the class device */ +class_register (&leds_class); + +/* register the timer */ +init_timer (&leds_ktimer); + leds_ktimer.function = leds_timer_function; + leds_ktimer.data =0 ; + leds_ktimer.expires = jiffies +LEDS_TIMER_INTERVAL ; +add_timer (&leds_ktimer); + +return ret; +} +subsys_initcall (leds_init); + +static void __exit leds_exit(void) +{ +class_unregister (&leds_class); + +atomic_set (&leds_stop_timer,1 ); + +del_timer_sync (&leds_ktimer); +} +module_exit (leds_exit); + +MODULE_AUTHOR ("John Lenz" ); +MODULE_LICENSE ("GPL" ); +MODULE_DESCRIPTION ("LED core class interface" ); +
include/linux/leds.h new file +46 -0
46 lines not on this page · raw message
drivers/leds/Kconfig new file +11 -0
@@ -0,0 +1,11 @@ + +menu "LED devices" + +config CLASS_LEDS + tristate "LED support" + help + This option provides the generic support for the leds class. + LEDs can be accessed from /sys/class/leds. It will also allow you + to select individual drivers for LED devices. If unsure, say N. + +endmenu
drivers/leds/Makefile new file +3 -0
@@ -0,0 +1,3 @@ + +# Core functionality. +obj-$(CONFIG_CLASS_LEDS) += ledscore.o
drivers/Kconfig renamed from drivers/Kconfig~leds_sysfs +2 -0
@@ -48,6 +48,8 @@ source "drivers/media/Kconfig" +source "drivers/leds/Kconfig" + source "drivers/video/Kconfig" source "sound/Kconfig"
drivers/Makefile renamed from drivers/Makefile~leds_sysfs +1 -0
@@ -50,4 +50,5 @@ obj-$(CONFIG_MCA) += mca/ obj-$(CONFIG_EISA) += eisa/ obj-$(CONFIG_CPU_FREQ) += cpufreq/ +obj-$(CONFIG_CLASS_LEDS) += leds/ obj-y += firmware/
Hi!
Quoted 12 lines
Quoted 11 lines
Quoted 7 lines
function : a read/write attribute that sets the current function of this led. The available options are
timer : the led blinks on and off on a timer
idle : the led turns off when the system is idle and on when not idle power : the led represents the current power state
user : the led is controlled by user spaceI'm afraid this is really good idea. It seems quite overengineered to me (and I'd be afraid that idle part slows machine). Perhaps having only "user" mode is better idea?
Quoted 5 lines
I was only mimicing the support currently in the arm led code. After thinking about it from a broader perspective of including GPIOs, we should probably get rid of this function thing entirely. Just let user space do everything... userspace can monitor sysfs and hotplug and have the led represent power or idle or whatever.
Well, it might be okay than.
Another possibility would be to use hard-coded behaviour while system is booting/stopping and /sys-controlled behaviour while userspace is alive.
Pavel