Hi!
Handheld machines have limited number of software-controlled status
LEDs. Collie, for example has two of them; one is labeled "charge" and
second is labeled "mail".
At least the "mail" led should be handled from userspace, and it would
be nice if (at least) different speeds of blinking could be used --
original Sharp ROM uses at least:
yellow off: not charging
yellow on: charging
yellow fast blink: charge error
I think even slow blinking was used somewhere. I have some code from
John Lenz (attached); it uses sysfs interface, exports led collor, and
allows setting different frequencies.
Is that acceptable, or should some other interface be used?
Pavel
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -244,6 +244,8 @@ source "arch/arm/mach-versatile/Kconfig"
source "arch/arm/mach-aaec2000/Kconfig"
+source "drivers/leds/Kconfig"
+
# Definitions to make life easier
config ARCH_ACORN
bool
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -54,6 +54,8 @@ source "drivers/mfd/Kconfig"
source "drivers/media/Kconfig"
+source "drivers/leds/Kconfig"
+
source "drivers/video/Kconfig"
source "sound/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -67,3 +67,4 @@ obj-$(CONFIG_INFINIBAND) += infiniband/
obj-$(CONFIG_SGI_IOC4) += sn/
obj-y += firmware/
obj-$(CONFIG_CRYPTO) += crypto/
+obj-$(CONFIG_CLASS_LEDS) += leds/
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
new file mode 100644
--- /dev/null
+++ b/drivers/leds/Kconfig
@@ -0,0 +1,19 @@
+
+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.
+
+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.
+ Zaurus models SL-5500 and SL-5600.
+
+endmenu
+
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
new file mode 100644
--- /dev/null
+++ b/drivers/leds/Makefile
@@ -0,0 +1,4 @@
+
+# Core functionality.
+obj-$(CONFIG_CLASS_LEDS) += ledscore.o
+obj-$(CONFIG_LEDS_LOCOMO) += locomo.o
diff --git a/drivers/leds/ledscore.c b/drivers/leds/ledscore.c
new file mode 100644
--- /dev/null
+++ b/drivers/leds/ledscore.c
@@ -0,0 +1,460 @@
+/*
+ * linux/drivers/leds/ledscore.c
+ *
+ * Copyright (C) 2005 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>
+
+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;
+
+ unsigned long frequency; /* frequency of blinking, in milliseconds */
+ int in_use; /* 1 if this device is in use by the kernel somewhere */
+
+ struct class_device class_dev;
+ struct timer_list *ktimer;
+ struct list_head node;
+};
+
+#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 rwlock_t leds_interface_list_lock = RW_LOCK_UNLOCKED;
+static LIST_HEAD(leds_interface_list);
+
+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->node);
+ write_unlock(&leds_list_lock);
+
+ kfree(d);
+}
+
+static struct class leds_class = {
+ .name = "leds",
+ .release = leds_class_release,
+};
+
+static void leds_timer_function(unsigned long data)
+{
+ struct led_device *led_dev = (struct led_device *) data;
+ unsigned long delay = 0;
+
+ spin_lock(&led_dev->lock);
+ if (led_dev->frequency) {
+ delay = led_dev->frequency;
+ if (likely(led_dev->props->brightness_get)) {
+ unsigned long value;
+ if (led_dev->props->brightness_get(led_dev->class_dev.dev, led_dev->props))
+ value = 0;
+ else
+ value = 100;
+ if (likely(led_dev->props->brightness_set))
+ led_dev->props->brightness_set(led_dev->class_dev.dev, led_dev->props, value);
+ }
+ }
+ spin_unlock(&led_dev->lock);
+
+ if (delay)
+ mod_timer(led_dev->ktimer, jiffies + msecs_to_jiffies(delay));
+}
+
+/* This function MUST be called with led_dev->lock held */
+static int leds_enable_timer(struct led_device *led_dev)
+{
+ if (led_dev->frequency && led_dev->ktimer) {
+ /* timer already created, just enable it */
+ mod_timer(led_dev->ktimer, jiffies + msecs_to_jiffies(led_dev->frequency));
+ } else if (led_dev->frequency && led_dev->ktimer == NULL) {
+ /* create a new timer */
+ led_dev->ktimer = kmalloc(sizeof(struct timer_list), GFP_KERNEL);
+ if (led_dev->ktimer) {
+ init_timer(led_dev->ktimer);
+ led_dev->ktimer->function = leds_timer_function;
+ led_dev->ktimer->data = (unsigned long) led_dev;
+ led_dev->ktimer->expires = jiffies + msecs_to_jiffies(led_dev->frequency);
+ add_timer(led_dev->ktimer);
+ } else {
+ led_dev->frequency = 0;
+ return -ENOMEM;
+ }
+ }
+
+ return 0;
+}
+
+
+static ssize_t leds_show_in_use(struct class_device *dev, char *buf)
+{
+ struct led_device *led_dev = to_led_device(dev);
+ ssize_t ret = 0;
+
+ spin_lock(&led_dev->lock);
+ sprintf(buf, "%i\n", led_dev->in_use);
+ ret = strlen(buf) + 1;
+ spin_unlock(&led_dev->lock);
+
+ return ret;
+}
+
+static CLASS_DEVICE_ATTR(in_use, 0444, leds_show_in_use, NULL);
+
+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_current_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)) {
+ if (led_dev->props->color_get) {
+ sprintf(buf, "%u\n", led_dev->props->color_get(led_dev->class_dev.dev, led_dev->props));
+ ret = strlen(buf) + 1;
+ }
+ }
+ spin_unlock(&led_dev->lock);
+
+ return ret;
+}
+
+static ssize_t leds_store_current_color(struct class_device *dev, const char *buf, size_t size)
+{
+ struct led_device *led_dev = to_led_device(dev);
+ ssize_t 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 (led_dev->props && !led_dev->in_use) {
+ if (led_dev->props->color_set)
+ led_dev->props->color_set(led_dev->class_dev.dev, led_dev->props, state);
+ }
+ spin_unlock(&led_dev->lock);
+ }
+
+ return ret;
+}
+
+static CLASS_DEVICE_ATTR(current_color, 0444, leds_show_current_color, leds_store_current_color);
+
+static ssize_t leds_show_brightness(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)) {
+ if (likely(led_dev->props->brightness_get)) {
+ sprintf(buf, "%u\n",
+ led_dev->props->brightness_get(led_dev->class_dev.dev, led_dev->props));
+ ret = strlen(buf) + 1;
+ }
+ }
+ spin_unlock(&led_dev->lock);
+
+ return ret;
+}
+
+static ssize_t leds_store_brightness(struct class_device *dev, const char *buf, size_t size)
+{
+ struct led_device *led_dev = to_led_device(dev);
+ ssize_t 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 (led_dev->props && !led_dev->in_use) {
+ if (state > 100) state = 100;
+ if (led_dev->props->brightness_set)
+ led_dev->props->brightness_set(led_dev->class_dev.dev, led_dev->props, state);
+ }
+ spin_unlock(&led_dev->lock);
+ }
+
+ return ret;
+}
+
+static CLASS_DEVICE_ATTR(brightness, 0644, leds_show_brightness, leds_store_brightness);
+
+static ssize_t leds_show_frequency(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->frequency);
+ ret = strlen(buf) + 1;
+ }
+ spin_unlock(&led_dev->lock);
+
+ return ret;
+}
+
+static ssize_t leds_store_frequency(struct class_device *dev, const char *buf, size_t size)
+{
+ struct led_device *led_dev = to_led_device(dev);
+ int ret = -EINVAL, ret2;
+ char *after;
+
+ unsigned long state = simple_strtoul(buf, &after, 10);
+ if (after - buf > 0) {
+ ret = after - buf;
+ spin_lock(&led_dev->lock);
+ if (led_dev->props && !led_dev->in_use) {
+ led_dev->frequency = state;
+ ret2 = leds_enable_timer(led_dev);
+ if (ret2) ret = ret2;
+ }
+ spin_unlock(&led_dev->lock);
+ }
+
+ return ret;
+}
+
+static CLASS_DEVICE_ATTR(frequency, 0644, leds_show_frequency, leds_store_frequency);
+
+/**
+ * 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;
+ struct led_device *new_led;
+ struct led_interface *interface;
+
+ new_led = kmalloc (sizeof (struct led_device), GFP_KERNEL);
+ if (unlikely (!new_led))
+ return -ENOMEM;
+
+ memset(new_led, 0, sizeof(struct led_device));
+
+ spin_lock_init(&new_led->lock);
+ new_led->props = props;
+ props->led_dev = new_led;
+
+ new_led->class_dev.class = &leds_class;
+ new_led->class_dev.dev = dev;
+
+ new_led->frequency = 0;
+ new_led->in_use = 0;
+
+ /* assign this led its name */
+ strncpy(new_led->class_dev.class_id, props->name, sizeof(new_led->class_dev.class_id));
+
+ 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_in_use);
+ class_device_create_file(&new_led->class_dev, &class_device_attr_color);
+ class_device_create_file(&new_led->class_dev, &class_device_attr_current_color);
+ class_device_create_file(&new_led->class_dev, &class_device_attr_brightness);
+ class_device_create_file(&new_led->class_dev, &class_device_attr_frequency);
+
+ /* add to the list of leds */
+ write_lock(&leds_list_lock);
+ list_add_tail(&new_led->node, &leds_list);
+ write_unlock(&leds_list_lock);
+
+ /* notify any interfaces */
+ read_lock(&leds_interface_list_lock);
+ list_for_each_entry(interface, &leds_interface_list, node) {
+ if (interface->add)
+ interface->add(dev, props);
+ }
+ read_unlock(&leds_interface_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);
+
+/**
+ * leds_device_unregister - unregisters a object of led_properties class.
+ * @props: the property to unreigister
+ *
+ * Unregisters a previously registered via leds_device_register object.
+ */
+void leds_device_unregister(struct led_properties *props)
+{
+ struct led_device *led_dev;
+ struct led_interface *interface;
+
+ if (!props || !props->led_dev)
+ return;
+
+ led_dev = props->led_dev;
+
+ /* notify interfaces device is going away */
+ read_lock(&leds_interface_list_lock);
+ list_for_each_entry(interface, &leds_interface_list, node) {
+ if (interface->remove)
+ interface->remove(led_dev->class_dev.dev, props);
+ }
+ read_unlock(&leds_interface_list_lock);
+
+ class_device_remove_file (&led_dev->class_dev, &class_device_attr_frequency);
+ class_device_remove_file (&led_dev->class_dev, &class_device_attr_brightness);
+ class_device_remove_file (&led_dev->class_dev, &class_device_attr_current_color);
+ class_device_remove_file (&led_dev->class_dev, &class_device_attr_color);
+ class_device_remove_file (&led_dev->class_dev, &class_device_attr_in_use);
+
+ spin_lock(&led_dev->lock);
+ led_dev->props = NULL;
+ props->led_dev = NULL;
+ spin_unlock(&led_dev->lock);
+
+ if (led_dev->ktimer) {
+ del_timer_sync(led_dev->ktimer);
+ kfree(led_dev->ktimer);
+ led_dev->ktimer = NULL;
+ }
+
+ class_device_unregister(&led_dev->class_dev);
+}
+EXPORT_SYMBOL(leds_device_unregister);
+
+int leds_acquire(struct led_properties *led)
+{
+ int ret = -EBUSY;
+
+ spin_lock(&led->led_dev->lock);
+ if (!led->led_dev->in_use) {
+ led->led_dev->in_use = 1;
+ /* Disable the userspace blinking, if any */
+ led->led_dev->frequency = 0;
+ ret = 0;
+ }
+ spin_unlock(&led->led_dev->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(leds_acquire);
+
+void leds_release(struct led_properties *led)
+{
+ spin_lock(&led->led_dev->lock);
+ led->led_dev->in_use = 0;
+ /* Disable the kernel blinking, if any */
+ led->led_dev->frequency = 0;
+ spin_unlock(&led->led_dev->lock);
+}
+EXPORT_SYMBOL(leds_release);
+
+/* Sets the frequency of the led in milliseconds.
+ * Only call this function after leds_acquire returns true
+ */
+int leds_set_frequency(struct led_properties *led, unsigned long frequency)
+{
+ int ret = 0;
+
+ spin_lock(&led->led_dev->lock);
+
+ if (!led->led_dev->in_use)
+ return -EINVAL;
+
+ led->led_dev->frequency = frequency;
+ ret = leds_enable_timer(led->led_dev);
+
+ spin_unlock(&led->led_dev->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(leds_set_frequency);
+
+int leds_interface_register(struct led_interface *interface)
+{
+ struct led_device *led_dev;
+
+ write_lock(&leds_interface_list_lock);
+ list_add_tail(&interface->node, &leds_interface_list);
+
+ read_lock(&leds_list);
+ list_for_each_entry(led_dev, &leds_list, node) {
+ spin_lock(&led_dev->lock);
+ if (led_dev->props) {
+ interface->add(led_dev->class_dev.dev, led_dev->props);
+ }
+ spin_unlock(&led_dev->lock);
+ }
+ read_unlock(&leds_list);
+
+ write_unlock(&leds_interface_list_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(leds_interface_register);
+
+void leds_interface_unregister(struct led_interface *interface)
+{
+ write_lock(&leds_interface_list_lock);
+ list_del(&interface->node);
+ write_unlock(&leds_interface_list_lock);
+}
+EXPORT_SYMBOL(leds_interface_unregister);
+
+static int __init leds_init(void)
+{
+ /* initialize the class device */
+ return class_register(&leds_class);
+}
+subsys_initcall(leds_init);
+
+static void __exit leds_exit(void)
+{
+ class_unregister(&leds_class);
+}
+module_exit(leds_exit);
+
+MODULE_AUTHOR("John Lenz");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("LED core class interface");
+
diff --git a/drivers/leds/locomo.c b/drivers/leds/locomo.c
new file mode 100644
--- /dev/null
+++ b/drivers/leds/locomo.c
@@ -0,0 +1,124 @@
+/*
+ * linux/drivers/leds/locomo.c
+ *
+ * Copyright (C) 2005 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;
+ int brightness;
+ struct led_properties props;
+};
+#define to_locomoled_data(d) container_of(d, struct locomoled_data, props)
+
+int locomoled_brightness_get(struct device *dev, struct led_properties *props)
+{
+ struct locomoled_data *data = to_locomoled_data(props);
+
+ return data->brightness;
+}
+
+void locomoled_brightness_set(struct device *dev, struct led_properties *props, int value)
+{
+ struct locomo_dev *locomo_dev = LOCOMO_DEV(dev);
+ struct locomoled_data *data = to_locomoled_data(props);
+
+ unsigned long flags;
+
+ if (value < 0) value = 0;
+ data->brightness = value;
+ local_irq_save(flags);
+ if (data->brightness) {
+ data->brightness = 100;
+ 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,
+ .name = "power",
+ .color = "amber",
+ .brightness_get = locomoled_brightness_get,
+ .brightness_set = locomoled_brightness_set,
+ .color_get = NULL,
+ .color_set = NULL,
+ }
+ },
+ {
+ .offset = LOCOMO_LPT1,
+ .props = {
+ .owner = THIS_MODULE,
+ .name = "mail",
+ .color = "green",
+ .brightness_get = locomoled_brightness_get,
+ .brightness_set = locomoled_brightness_set,
+ .color_get = NULL,
+ .color_set = NULL,
+ }
+ },
+};
+
+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");
--- /dev/null
+++ b/include/linux/leds.h
@@ -0,0 +1,63 @@
+/*
+ * linux/include/leds.h
+ *
+ * Copyright (C) 2005 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.
+ *
+ * Driver model for leds
+ */
+#ifndef ASM_ARM_LEDS_H
+#define ASM_ARM_LEDS_H
+
+#include <linux/device.h>
+
+struct led_device;
+
+struct led_properties {
+ struct module *owner;
+
+ /* Read-only name for this led */
+ char *name;
+
+ /* Color of the led. For multiple color leds, the color names should
+ * be seperated by a "/". For example, "amber/green".
+ * This is read-only.
+ */
+ char *color;
+
+ /* For multi-colored leds, these function are called to manipulate the
+ * current color. The integer value should be the position in the above
+ * list of colors. For a single color led, set equal to NULL.
+ */
+ int (*color_get)(struct device *, struct led_properties *props);
+ void (*color_set)(struct device *, struct led_properties *props, int value);
+
+ /* These functions manipulate the brightness of the led.
+ * Values are between 0-100 */
+ int (*brightness_get)(struct device *, struct led_properties *props);
+ void (*brightness_set)(struct device *, struct led_properties *props, int value);
+
+ /* private structure */
+ struct led_device *led_dev;
+};
+
+int leds_device_register(struct device *dev, struct led_properties *props);
+void leds_device_unregister(struct led_properties *props);
+
+int leds_acquire(struct led_properties *led);
+void leds_release(struct led_properties *led);
+int leds_set_frequency(struct led_properties *led, unsigned long frequency);
+
+struct led_interface {
+ int (*add)(struct device *dev, struct led_properties *led);
+ void (*remove)(struct device *dev, struct led_properties *led);
+
+ struct list_head node;
+};
+int leds_interface_register(struct led_interface *interface);
+void leds_interface_unregister(struct led_interface *interface);
+
+#endif
--
Thanks, Sharp!
On Wed, 2005-11-02 at 00:44 +0100, Pavel Machek wrote:
> Handheld machines have limited number of software-controlled status
> LEDs. Collie, for example has two of them; one is labeled "charge" and
> second is labeled "mail".
> I think even slow blinking was used somewhere. I have some code from
> John Lenz (attached); it uses sysfs interface, exports led collor, and
> allows setting different frequencies.
>
> Is that acceptable, or should some other interface be used?
This has been discussed before and I know there are several differing
opinions.
Based upon previous discussion both here, on linux-arm-kernel and in the
handhelds community in general I came up with some ideas which I've yet
to have time to code. I'll try and describe it though:
The system would be in two sections (classes?), leds themselves and led
triggers. The leds would be driven by something similar to John's driver
Pavel attached. I think colour and other unchanging properties of the
device should be something exported in the device name which could have
some format like: device_name-colour-otherprops.
Led triggers would be kernel sources of led on/off events. Some
examples:
2Hz Heartbeat - useful for debugging (and/or Generic Timer)
CPU Load indicator
Charging indicator
HDD activity (useful for microdrive on handheld)
Network activity
no doubt many more
led triggers would be connected to leds via sysfs. Each trigger would
probably have a number you could echo into an led's trigger attribute.
Sensible default mappings could be had by assigning a default trigger to
a device by name in the platform code that declares the led.
A trigger of "0" would mean the led becomes under userspace control via
sysfs for whatever userspace wishes to do with it.
The underlying principle would be to keep this class as simple as
possible whilst maximising the options open for triggering the leds from
both the kernel and userspace.
Does this sound like a sensible way forward?
Richard
On Tue, November 1, 2005 6:39 pm, Richard Purdie said:
> On Wed, 2005-11-02 at 00:44 +0100, Pavel Machek wrote:
>> Handheld machines have limited number of software-controlled status
>> LEDs. Collie, for example has two of them; one is labeled "charge" and
>> second is labeled "mail".
>
>> I think even slow blinking was used somewhere. I have some code from
>> John Lenz (attached); it uses sysfs interface, exports led collor, and
>> allows setting different frequencies.
>>
>> Is that acceptable, or should some other interface be used?
>
> This has been discussed before and I know there are several differing
> opinions.
>
> Based upon previous discussion both here, on linux-arm-kernel and in the
> handhelds community in general I came up with some ideas which I've yet
> to have time to code. I'll try and describe it though:
>
> The system would be in two sections (classes?), leds themselves and led
> triggers. The leds would be driven by something similar to John's driver
> Pavel attached. I think colour and other unchanging properties of the
> device should be something exported in the device name which could have
> some format like: device_name-colour-otherprops.
I believe that this can be built on top of my patch. If you take a look
at the led patch Pavel posted, it allows for in kernel code to acquire the
led by calling leds_acquire. Once a led is acquired through leds_acquire
function, any input from userspace is ignored.
Any interested kernel code can also register an interface to watch for led
additions and removals.
>
> Led triggers would be kernel sources of led on/off events. Some
> examples:
>
> 2Hz Heartbeat - useful for debugging (and/or Generic Timer)
This is included already in the leds driver part, although it could be
removed I guess...
> CPU Load indicator
> Charging indicator
> HDD activity (useful for microdrive on handheld)
> Network activity
> no doubt many more
All these are great ideas for triggers.
>
> led triggers would be connected to leds via sysfs. Each trigger would
> probably have a number you could echo into an led's trigger attribute.
> Sensible default mappings could be had by assigning a default trigger to
> a device by name in the platform code that declares the led.
This would be part of the triggers module... passing the right stuff into
those sysfs options would cause the triggers module to call leds_acquire
on the right led_properties structure.
>
> A trigger of "0" would mean the led becomes under userspace control via
> sysfs for whatever userspace wishes to do with it.
A trigger of "0" would call leds_release, which would cause the input from
userspace in the leds driver to be accepted again.
>
> The underlying principle would be to keep this class as simple as
> possible whilst maximising the options open for triggering the leds from
> both the kernel and userspace.
>
> Does this sound like a sensible way forward?
>
I think it can already be done from my patch... all the functions are
already there for in kernel manipulation.
John
In article <[email protected]> you wrote:
> +static ssize_t leds_store_frequency(struct class_device *dev, const char *buf, size_t size)
how about using a on and a off timer, so you can set up 50,50 or 10,90 or
stuff like that to make different pulse.
I know the TI avalanch platform has a quite complicated led driver, which I
think is much overworked since it allows led settings based on logical
states. This should all be in userspace.
Gruss
Bernd
On Wed, Nov 02, 2005 at 12:44:59AM +0100, Pavel Machek wrote:
> Hi!
>
> Handheld machines have limited number of software-controlled status
> LEDs. Collie, for example has two of them; one is labeled "charge" and
> second is labeled "mail".
>
> At least the "mail" led should be handled from userspace, and it would
> be nice if (at least) different speeds of blinking could be used --
> original Sharp ROM uses at least:
>
> yellow off: not charging
> yellow on: charging
> yellow fast blink: charge error
>
> I think even slow blinking was used somewhere. I have some code from
> John Lenz (attached); it uses sysfs interface, exports led collor, and
> allows setting different frequencies.
>
> Is that acceptable, or should some other interface be used?
there is already an LED interface for linux-arm, which is
used by a number of the extant machines in the sa11x0 and
pxa range.
--
Ben ([email protected], http://www.fluff.org/)
'a smiley only costs 4 bytes'
On Tuesday 01 November 2005 06:38 pm, Richard Purdie wrote:
> led triggers would be connected to leds via sysfs. Each trigger would
> probably have a number you could echo into an led's trigger attribute.
> Sensible default mappings could be had by assigning a default trigger to
> a device by name in the platform code that declares the led.
>
> A trigger of "0" would mean the led becomes under userspace control via
> sysfs for whatever userspace wishes to do with it.
Any reason to represent the triggers as numbers in sysfs? I'm a fan of how the
IO scheduler is chosen on a block device. The file looks like:
noop [anticipatory] deadline cfq
You get a list of all available triggers, which one is active (and it's easy
enough for user space code to parse). Obviously, to select a trigger, you
just write its name to the file. Instead of 0, you say user, etc...
Cheers,
Chase Venters
On Wed, 2 Nov 2005 02:47:55 +0000
Ben Dooks <[email protected]> wrote:
> > I think even slow blinking was used somewhere. I have some code from
> > John Lenz (attached); it uses sysfs interface, exports led collor, and
> > allows setting different frequencies.
> >
> > Is that acceptable, or should some other interface be used?
>
> there is already an LED interface for linux-arm, which is
> used by a number of the extant machines in the sa11x0 and
> pxa range.
Hello,
the current interface is very low-level, while the proposal
from Pavel/John has a much different scope, imho.
I'm working on a led driver for the ixp4xx/NSLU2 which
has 2 leds plus a multi-coloured one.
We would benefit a lot from such an implementation so,
FWIW, I express my personal support and the one
of the whole nslu2-linux community.
Once this patch is upstream, the linux-arm interface could easily
be adapted to use it or cooperate nicely.
--
Best regards,
Alessandro Zummo,
Tower Technologies - Turin, Italy
http://www.towertech.it
Hi!
> > I think even slow blinking was used somewhere. I have some code from
> > John Lenz (attached); it uses sysfs interface, exports led collor, and
> > allows setting different frequencies.
> >
> > Is that acceptable, or should some other interface be used?
>
> there is already an LED interface for linux-arm, which is
> used by a number of the extant machines in the sa11x0 and
> pxa range.
Where is that interface? I think that making collie use it is obvious
first step...
Pavel
--
Thanks, Sharp!
On Wed, Nov 02, 2005 at 12:44:59AM +0100, Pavel Machek wrote:
> Handheld machines have limited number of software-controlled status
> LEDs. Collie, for example has two of them; one is labeled "charge" and
> second is labeled "mail".
>
> At least the "mail" led should be handled from userspace, and it would
> be nice if (at least) different speeds of blinking could be used --
> original Sharp ROM uses at least:
>
> yellow off: not charging
> yellow on: charging
> yellow fast blink: charge error
>
> I think even slow blinking was used somewhere. I have some code from
> John Lenz (attached); it uses sysfs interface, exports led collor, and
> allows setting different frequencies.
>
> Is that acceptable, or should some other interface be used?
>
I would also be in favour of having a more generic interface for
something like this (as opposed to each architecture rolling their own).
For example, sh currently has a very simplistic framework in place where
LED manipulation is done via a heartbeat callback in the machvec for each
timer tick, so it would be nice to have things somewhat more flexible in
this regard (especially for boards with multi-coloured LEDs, and for
userspace input).
On Wed, 2 Nov 2005, root wrote:
> In article <[email protected]> you wrote:
> > +static ssize_t leds_store_frequency(struct class_device *dev, const char *buf, size_t size)
>
> how about using a on and a off timer, so you can set up 50,50 or 10,90 or
> stuff like that to make different pulse.
Or the heartbeat that goes faster if load average increases, as is already
implemented on some archs?
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
Hi!
> Led triggers would be kernel sources of led on/off events. Some
> examples:
>
> 2Hz Heartbeat - useful for debugging (and/or Generic Timer)
> CPU Load indicator
> Charging indicator
> HDD activity (useful for microdrive on handheld)
> Network activity
> no doubt many more
>
> led triggers would be connected to leds via sysfs. Each trigger would
> probably have a number you could echo into an led's trigger attribute.
> Sensible default mappings could be had by assigning a default trigger to
> a device by name in the platform code that declares the led.
Perhaps I'd keep it simple and leave it at
* do hardcoded kernel action for this led
or
* do whatever userspace tells you.
That way you will not be able to remap charger LED onto hard disk
indicator, but we can support that on ibm-acpi too. (Where hw controls
LEDs like "sleep", but lets you control them. You can't remap,
though).
Pavel
--
Thanks, Sharp!
On Wed, 2005-11-02 at 14:56 +0100, Pavel Machek wrote:
> > Led triggers would be kernel sources of led on/off events. Some
> > examples:
> >
> > 2Hz Heartbeat - useful for debugging (and/or Generic Timer)
> > CPU Load indicator
> > Charging indicator
> > HDD activity (useful for microdrive on handheld)
> > Network activity
> > no doubt many more
> >
> > led triggers would be connected to leds via sysfs. Each trigger would
> > probably have a number you could echo into an led's trigger attribute.
> > Sensible default mappings could be had by assigning a default trigger to
> > a device by name in the platform code that declares the led.
>
> Perhaps I'd keep it simple and leave it at
>
> * do hardcoded kernel action for this led
>
> or
>
> * do whatever userspace tells you.
>
> That way you will not be able to remap charger LED onto hard disk
> indicator, but we can support that on ibm-acpi too. (Where hw controls
> LEDs like "sleep", but lets you control them. You can't remap,
> though).
Then the arguments start about which function should be hardcoded to
which leds and why can't userspace access these triggers?
I'd prefer a totally flexible system and it doesn't really add much
complexity once you have a trigger framework which we're going to need
to handle mutiple led trigger sources sanely anyway.
Richard
On Wed, Nov 02, 2005 at 12:44:59AM +0100, Pavel Machek wrote:
> Handheld machines have limited number of software-controlled status
> LEDs. Collie, for example has two of them; one is labeled "charge" and
> second is labeled "mail".
>
> At least the "mail" led should be handled from userspace, and it would
> be nice if (at least) different speeds of blinking could be used --
> original Sharp ROM uses at least:
>
> yellow off: not charging
> yellow on: charging
> yellow fast blink: charge error
>
> I think even slow blinking was used somewhere. I have some code from
> John Lenz (attached); it uses sysfs interface, exports led collor, and
> allows setting different frequencies.
>
> Is that acceptable, or should some other interface be used?
IMHO reducing digital outputs to LEDs goes not far enough. All System-
on-Chip CPUs have General Purpose I/O pins today, which can act as
inputs or outputs and may be used for LEDs, matrix keyboard lines, we
even use them as shutdown lines for DC/DC converters in Linux based
power controllers for autonomous measurement systems. I think a
framework for LEDs should be based upon a low level infrastrucutre for
GPIOs. I've written such a thing some time ago, it has been discussed in
http://marc.theaimsgroup.com/?l=linux-kernel&m=109419621428925&w=2
Unfortunately, I didn't have time to take care of that stuff yet and
port them to recent kernels :-(
Robert
--
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Hannoversche Str. 2, 31134 Hildesheim, Germany
Phone: +49-5121-206917-0 | Fax: +49-5121-206917-9
Hi!
> > Perhaps I'd keep it simple and leave it at
> >
> > * do hardcoded kernel action for this led
> >
> > or
> >
> > * do whatever userspace tells you.
> >
> > That way you will not be able to remap charger LED onto hard disk
> > indicator, but we can support that on ibm-acpi too. (Where hw controls
> > LEDs like "sleep", but lets you control them. You can't remap,
> > though).
>
> Then the arguments start about which function should be hardcoded to
> which leds and why can't userspace access these triggers?
Because there are some machines (IBM thinkpad) where LEDs are either
driven by userspace, or driven by hardware. I'd like to export that
functionality using same interface.
> I'd prefer a totally flexible system and it doesn't really add much
> complexity once you have a trigger framework which we're going to need
> to handle mutiple led trigger sources sanely anyway.
Unfortunately hardware can not do that, at least for IBM
thinkpad. Plus, remapping harddisk indicator on battery led is not
something I'd like to support :-).
Pavel
--
Thanks, Sharp!
Hi!
> > yellow off: not charging
> > yellow on: charging
> > yellow fast blink: charge error
> >
> > I think even slow blinking was used somewhere. I have some code from
> > John Lenz (attached); it uses sysfs interface, exports led collor, and
> > allows setting different frequencies.
> >
> > Is that acceptable, or should some other interface be used?
>
> IMHO reducing digital outputs to LEDs goes not far enough. All System-
> on-Chip CPUs have General Purpose I/O pins today, which can act as
> inputs or outputs and may be used for LEDs, matrix keyboard lines,
We have some leds that are *not* on GPIO pins (like driven by
ACPI). We'd like to support those, too.
Pavel
--
Thanks, Sharp!
On Wed, Nov 02, 2005 at 10:13:34PM +0100, Pavel Machek wrote:
> We have some leds that are *not* on GPIO pins (like driven by
> ACPI). We'd like to support those, too.
One more argument to have a LED framework which sits ontop of a lowlevel
one.
Robert
--
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Hannoversche Str. 2, 31134 Hildesheim, Germany
Phone: +49-5121-206917-0 | Fax: +49-5121-206917-9
On Wed, 2005-11-02 at 22:11 +0100, Pavel Machek wrote:
> > > Perhaps I'd keep it simple and leave it at
> > >
> > > * do hardcoded kernel action for this led
> > >
> > > or
> > >
> > > * do whatever userspace tells you.
> > >
> > > That way you will not be able to remap charger LED onto hard disk
> > > indicator, but we can support that on ibm-acpi too. (Where hw controls
> > > LEDs like "sleep", but lets you control them. You can't remap,
> > > though).
> >
> > Then the arguments start about which function should be hardcoded to
> > which leds and why can't userspace access these triggers?
>
> Because there are some machines (IBM thinkpad) where LEDs are either
> driven by userspace, or driven by hardware. I'd like to export that
> functionality using same interface.
A specific limitation of ACPI LEDs shouldn't constrain the interface and
spoil things for everything that can support generic triggers.
> > I'd prefer a totally flexible system and it doesn't really add much
> > complexity once you have a trigger framework which we're going to need
> > to handle mutiple led trigger sources sanely anyway.
>
> Unfortunately hardware can not do that, at least for IBM
> thinkpad.
So we need to find a way of providing this functionality, maybe by
allowing leds to provide their own specific triggers. Thinking about
this limitation, I think it can be handled by the trigger
implementation.
> Plus, remapping harddisk indicator on battery led is not
> something I'd like to support :-).
It wouldn't do that be default but I see no reason to constrain any
interface to stop this. The kernel isn't supposed to set policy (or in
my view put unnecessary constraints on interfaces).
Richard
On Wed, November 2, 2005 3:51 am, Pavel Machek said:
> Hi!
>
>> > I think even slow blinking was used somewhere. I have some code from
>> > John Lenz (attached); it uses sysfs interface, exports led collor, and
>> > allows setting different frequencies.
>> >
>> > Is that acceptable, or should some other interface be used?
>>
>> there is already an LED interface for linux-arm, which is
>> used by a number of the extant machines in the sa11x0 and
>> pxa range.
>
> Where is that interface? I think that making collie use it is obvious
> first step...
> Pavel
>
I originally wrote the collie led code to use that interface, and you
might look at some of the old versions of the patch on my web site. The
actual code is in arch/arm/kernel/time.c, but this code calls out to an
individual machine function through say arch/arm/mach-sa1100/leds.c...
The problem for collie was that the device model for locomo did not allow
an easy way to do it... as you can see, in my patch it implements a driver
for those leds and the driver model takes care of it.
I just looked, and
http://www.cs.wisc.edu/~lenz/zaurus/files/patch-2.6.7-jl2.diff.gz contins
the implementation of the arm led interface for collie.... not sure if it
will still work anymore, but...
I then grew the led code to support more than four leds, multi color leds,
more than just the arm architecture, etc... Russell has been very critical
of my patch on the "waste" of memory (a struct device for each led, along
with a bunch of struct attributes for each led), where by contrast the arm
one has a single attribute.
But this led interface and the one arm provides can almost not even be
compared. My patch correctly implements the sysfs model of one attribute,
one file (instead of all leds controlled through a single file). My patch
also is more flexible and provides needed options for many leds. The
downside is, it makes the leds hard to use for debugging, which is the
primary purpose of the original arm led code.
For debugging, the arm specific led code is a great tool, and I am not
meaning to replace it. For debugging, you can just specifiy
CONFIG_CLASS_LEDS=n.
John
On Wed, November 2, 2005 3:33 pm, Robert Schwebel said:
> On Wed, Nov 02, 2005 at 10:13:34PM +0100, Pavel Machek wrote:
>> We have some leds that are *not* on GPIO pins (like driven by
>> ACPI). We'd like to support those, too.
>
> One more argument to have a LED framework which sits ontop of a lowlevel
> one.
>
Except the led code that is being proposed CAN sit on top of a generic
GPIO layer. If a generic GPIO layer is created, you can create a led
driver that calls out to that GPIO layer.
You just need to fill in the following functions with some that raise and
lower the GPIO on the correct line....
int (*color_get)(struct device *, struct led_properties *props);
void (*color_set)(struct device *, struct led_properties *props, int value);
int (*brightness_get)(struct device *, struct led_properties *props);
void (*brightness_set)(struct device *, struct led_properties *props, int
value);
John
On Wednesday 02 November 2005 15:33, Robert Schwebel wrote:
> On Wed, Nov 02, 2005 at 10:13:34PM +0100, Pavel Machek wrote:
> > We have some leds that are *not* on GPIO pins (like driven by
> > ACPI). We'd like to support those, too.
>
> One more argument to have a LED framework which sits ontop of a lowlevel
> one.
In which case, why not submit the standalone LED framework now and shoehorn
the low-level one underneath later. From a userspace API perspective, it
really shouldn't matter...
Rob
On Wed, Nov 02, 2005 at 08:52:41PM -0600, John Lenz wrote:
> Except the led code that is being proposed CAN sit on top of a generic
> GPIO layer. If a generic GPIO layer is created, you can create a led
> driver that calls out to that GPIO layer.
>
> You just need to fill in the following functions with some that raise and
> lower the GPIO on the correct line....
>
> int (*color_get)(struct device *, struct led_properties *props);
> void (*color_set)(struct device *, struct led_properties *props, int value);
>
> int (*brightness_get)(struct device *, struct led_properties *props);
> void (*brightness_set)(struct device *, struct led_properties *props, int
> value);
Ok, great! I'll see that I update my patch for 2.6.14.
Robert
--
Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Handelsregister: Amtsgericht Hildesheim, HRA 2686
Hannoversche Str. 2, 31134 Hildesheim, Germany
Phone: +49-5121-206917-0 | Fax: +49-5121-206917-9
On Wed, Nov 02, 2005 at 08:52:41PM -0600, John Lenz wrote:
> On Wed, November 2, 2005 3:33 pm, Robert Schwebel said:
> > On Wed, Nov 02, 2005 at 10:13:34PM +0100, Pavel Machek wrote:
> >> We have some leds that are *not* on GPIO pins (like driven by
> >> ACPI). We'd like to support those, too.
> >
> > One more argument to have a LED framework which sits ontop of a lowlevel
> > one.
> >
>
> Except the led code that is being proposed CAN sit on top of a generic
> GPIO layer.
I also have issues with a generic GPIO layer. As I mentioned in the
past, there's serious locking issues with any generic abstraction of
GPIOs.
1. You want to be able to change GPIO state from interrupts. This
implies you can not sleep in GPIO state changing functions.
2. Some GPIOs are implemented on I2C devices. This means that to
change state, you must sleep.
(1) and (2) are incompatible requirements, so you can not offer a
generic interface for these GPIOs which has a consistent behaviour -
where users of the interface know whether the function may sleep or
may be used from interrupt context.
If you also consider that LEDs may be connected to these GPIOs, and
you may wish to change their state from interrupt context, you also
run into these same issues - you have an interface which has ill-
defined behaviour and ill-defined calling requirements.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Wed, 2 Nov 2005, Pavel Machek wrote:
> > > yellow off: not charging
> > > yellow on: charging
> > > yellow fast blink: charge error
> > >
> > > I think even slow blinking was used somewhere. I have some code from
> > > John Lenz (attached); it uses sysfs interface, exports led collor, and
> > > allows setting different frequencies.
> > >
> > > Is that acceptable, or should some other interface be used?
> >
> > IMHO reducing digital outputs to LEDs goes not far enough. All System-
> > on-Chip CPUs have General Purpose I/O pins today, which can act as
> > inputs or outputs and may be used for LEDs, matrix keyboard lines,
>
> We have some leds that are *not* on GPIO pins (like driven by
> ACPI). We'd like to support those, too.
I though ACPI was software? These LEDs have to be driven by some hardware,
right?
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
Hi!
> > > > yellow off: not charging
> > > > yellow on: charging
> > > > yellow fast blink: charge error
> > > >
> > > > I think even slow blinking was used somewhere. I have some code from
> > > > John Lenz (attached); it uses sysfs interface, exports led collor, and
> > > > allows setting different frequencies.
> > > >
> > > > Is that acceptable, or should some other interface be used?
> > >
> > > IMHO reducing digital outputs to LEDs goes not far enough. All System-
> > > on-Chip CPUs have General Purpose I/O pins today, which can act as
> > > inputs or outputs and may be used for LEDs, matrix keyboard lines,
> >
> > We have some leds that are *not* on GPIO pins (like driven by
> > ACPI). We'd like to support those, too.
>
> I though ACPI was software? These LEDs have to be driven by some hardware,
> right?
Right, but you don't know by *which* hardware. Only ACPI AML code knows.
So from linux POW, they are handled by ACPI.
It may even trap to SMM BIOS.
--
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms
Hi!
> > Except the led code that is being proposed CAN sit on top of a generic
> > GPIO layer.
>
> I also have issues with a generic GPIO layer. As I mentioned in the
> past, there's serious locking issues with any generic abstraction of
> GPIOs.
>
> 1. You want to be able to change GPIO state from interrupts. This
> implies you can not sleep in GPIO state changing functions.
>
> 2. Some GPIOs are implemented on I2C devices. This means that to
> change state, you must sleep.
Can't you just busywait? Yes, it is ugly in general, but perhaps it
is better than alternatives...
--
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms
On Thu, Nov 03, 2005 at 10:57:26AM +0100, Pavel Machek wrote:
> Hi!
>
> > > Except the led code that is being proposed CAN sit on top of a generic
> > > GPIO layer.
> >
> > I also have issues with a generic GPIO layer. As I mentioned in the
> > past, there's serious locking issues with any generic abstraction of
> > GPIOs.
> >
> > 1. You want to be able to change GPIO state from interrupts. This
> > implies you can not sleep in GPIO state changing functions.
> >
> > 2. Some GPIOs are implemented on I2C devices. This means that to
> > change state, you must sleep.
>
> Can't you just busywait? Yes, it is ugly in general, but perhaps it
> is better than alternatives...
Does the i2c layer support busy waiting or are you suggesting something
else?
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
Russell King <[email protected]> writes:
>
> I also have issues with a generic GPIO layer. As I mentioned in the
> past, there's serious locking issues with any generic abstraction of
> GPIOs.
>
> 1. You want to be able to change GPIO state from interrupts. This
> implies you can not sleep in GPIO state changing functions.
>
> 2. Some GPIOs are implemented on I2C devices. This means that to
> change state, you must sleep.
>
> (1) and (2) are incompatible requirements, so you can not offer a
> generic interface for these GPIOs which has a consistent behaviour -
> where users of the interface know whether the function may sleep or
> may be used from interrupt context.
>
So you have a request_gpio API, where you specify usage characteristics
and get back a handle?
cheers, Rich.
--
rich walker | Shadow Robot Company | [email protected]
technical director 251 Liverpool Road |
need a Hand? London N1 1LX | +UK 20 7700 2487
http://www.shadow.org.uk/products/newhand.shtml
On Thu, Nov 03, 2005 at 02:49:04PM +0000, Russell King wrote:
> On Thu, Nov 03, 2005 at 10:57:26AM +0100, Pavel Machek wrote:
> > Hi!
> >
> > > > Except the led code that is being proposed CAN sit on top of a generic
> > > > GPIO layer.
> > >
> > > I also have issues with a generic GPIO layer. As I mentioned in the
> > > past, there's serious locking issues with any generic abstraction of
> > > GPIOs.
> > >
> > > 1. You want to be able to change GPIO state from interrupts. This
> > > implies you can not sleep in GPIO state changing functions.
> > >
> > > 2. Some GPIOs are implemented on I2C devices. This means that to
> > > change state, you must sleep.
> >
> > Can't you just busywait? Yes, it is ugly in general, but perhaps it
> > is better than alternatives...
>
> Does the i2c layer support busy waiting or are you suggesting something
> else?
It usually doesn't support anything else. At least on the controllers
I've seen the drivers for.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
On Thu, Nov 03, 2005 at 04:34:45PM +0100, Vojtech Pavlik wrote:
> On Thu, Nov 03, 2005 at 02:49:04PM +0000, Russell King wrote:
> > On Thu, Nov 03, 2005 at 10:57:26AM +0100, Pavel Machek wrote:
> > > Hi!
> > >
> > > > > Except the led code that is being proposed CAN sit on top of a generic
> > > > > GPIO layer.
> > > >
> > > > I also have issues with a generic GPIO layer. As I mentioned in the
> > > > past, there's serious locking issues with any generic abstraction of
> > > > GPIOs.
> > > >
> > > > 1. You want to be able to change GPIO state from interrupts. This
> > > > implies you can not sleep in GPIO state changing functions.
> > > >
> > > > 2. Some GPIOs are implemented on I2C devices. This means that to
> > > > change state, you must sleep.
> > >
> > > Can't you just busywait? Yes, it is ugly in general, but perhaps it
> > > is better than alternatives...
> >
> > Does the i2c layer support busy waiting or are you suggesting something
> > else?
>
> It usually doesn't support anything else. At least on the controllers
> I've seen the drivers for.
I think you misunderstand me. Please read:
http://dictionary.reference.com/search?q=busywait
Since the i2c transfer functions allow drivers to sleep (and some do)
you can't "just busywait" without modifying the i2c layer to tell
drivers to busy wait instead of sleeping.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Thu, Nov 03, 2005 at 04:01:23PM +0000, Russell King wrote:
> I think you misunderstand me. Please read:
>
> http://dictionary.reference.com/search?q=busywait
>
> Since the i2c transfer functions allow drivers to sleep (and some do)
> you can't "just busywait" without modifying the i2c layer to tell
> drivers to busy wait instead of sleeping.
Agreed.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Hi!
> > > > Except the led code that is being proposed CAN sit on top of a generic
> > > > GPIO layer.
> > >
> > > I also have issues with a generic GPIO layer. As I mentioned in the
> > > past, there's serious locking issues with any generic abstraction of
> > > GPIOs.
> > >
> > > 1. You want to be able to change GPIO state from interrupts. This
> > > implies you can not sleep in GPIO state changing functions.
> > >
> > > 2. Some GPIOs are implemented on I2C devices. This means that to
> > > change state, you must sleep.
> >
> > Can't you just busywait? Yes, it is ugly in general, but perhaps it
> > is better than alternatives...
>
> Does the i2c layer support busy waiting or are you suggesting something
> else?
I'm suggesting that adding busy wait support to i2c is probably good
idea. GPIOs are on many small machines, and there are machine
(spitz/akita?) that differ mainly in "where GPIO lines are
connected". That cries for GPIO layer.
Pavel
--
Thanks, Sharp!
On Thu, 3 Nov 2005, Russell King wrote:
> On Thu, Nov 03, 2005 at 04:34:45PM +0100, Vojtech Pavlik wrote:
>> On Thu, Nov 03, 2005 at 02:49:04PM +0000, Russell King wrote:
>>> On Thu, Nov 03, 2005 at 10:57:26AM +0100, Pavel Machek wrote:
>>>> Hi!
>>>>
>>>>>> Except the led code that is being proposed CAN sit on top of a generic
>>>>>> GPIO layer.
>>>>>
>>>>> I also have issues with a generic GPIO layer. As I mentioned in the
>>>>> past, there's serious locking issues with any generic abstraction of
>>>>> GPIOs.
>>>>>
>>>>> 1. You want to be able to change GPIO state from interrupts. This
>>>>> implies you can not sleep in GPIO state changing functions.
>>>>>
>>>>> 2. Some GPIOs are implemented on I2C devices. This means that to
>>>>> change state, you must sleep.
>>>>
>>>> Can't you just busywait? Yes, it is ugly in general, but perhaps it
>>>> is better than alternatives...
>>>
>>> Does the i2c layer support busy waiting or are you suggesting something
>>> else?
>>
>> It usually doesn't support anything else. At least on the controllers
>> I've seen the drivers for.
>
> I think you misunderstand me. Please read:
>
> http://dictionary.reference.com/search?q=busywait
>
> Since the i2c transfer functions allow drivers to sleep (and some do)
> you can't "just busywait" without modifying the i2c layer to tell
> drivers to busy wait instead of sleeping.
>
> --
> Russell King
> Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
> maintainer of: 2.6 Serial core
The whole thing is pretty easy. The interrupt code just signals
a task to do whatever...
For example:
Interrupt service routine or anything else, sets the
value in 'what' and 'which'. It then executes
(in demo code) wake_up_interruptible(info->twait);
... and coutinues whatever it was doing. The daemon wakes
up (whenever) and, in process-context does whatever was
you wanted it to do.
extern void turn_led_on(int led); // Can sleep
extern void turn_len_off(int len); // Can sleep
enum WHAT {
TURN_LED_ON,
TURN_LED_OFF,
FLASH_LED,
STOP_FLASH };
static WHAT what;
static int which;
static int local_kernel_thread(void *unused) {
daemonize(name);
for(;;) {
set_current_state(TASK_INTERRUPTIBLE);
if(signal_pending(current))
complete_and_exit(&info->quit, 0);
interruptible_sleep_on(&info->twait);
set_current_state(TASK_RUNNING);
preempt_disable(); // So we get this done soon
switch(what) {
case TURN_LED_ON:
turn_led_on(which);
break;
case TURN_LEN_OFF:
turn_led_off(which);
break;
case FLASH_LED:
start_timer_queue_for_flashing_led(which); // Can sleep
break;
case STOP_FLASH:
stop_timer_queue_for_flashing_led(which); // Can sleep
break;
default:
printk("Brain fart\n");
}
preempt_enable();
}
return 0;
}
You never want to busy-wait for some "bells and whistles". You
just make a bells-and-whistles kernel daemon that does whatever
you want.
Cheers,
Dick Johnson
Penguin : Linux version 2.6.13.4 on an i686 machine (5589.55 BogoMips).
Warning : 98.36% of all statistics are fiction.
.
****************************************************************
The information transmitted in this message is confidential and may be privileged. Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited. If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to [email protected] - and destroy all copies of this information, including any attachments, without reading or disclosing them.
Thank you.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Pavel Machek wrote:
> Hi!
>
>
>>>>>Except the led code that is being proposed CAN sit on top of a generic
>>>>>GPIO layer.
>>>>
>>>>I also have issues with a generic GPIO layer. As I mentioned in the
>>>>past, there's serious locking issues with any generic abstraction of
>>>>GPIOs.
>>>>
>>>>1. You want to be able to change GPIO state from interrupts. This
>>>> implies you can not sleep in GPIO state changing functions.
>>>>
>>>>2. Some GPIOs are implemented on I2C devices. This means that to
>>>> change state, you must sleep.
>>>
>>>Can't you just busywait? Yes, it is ugly in general, but perhaps it
>>>is better than alternatives...
>>
>>Does the i2c layer support busy waiting or are you suggesting something
>>else?
>
>
> I'm suggesting that adding busy wait support to i2c is probably good
> idea. GPIOs are on many small machines, and there are machine
> (spitz/akita?) that differ mainly in "where GPIO lines are
> connected". That cries for GPIO layer.
> Pavel
Wouldn't it make sense to make either two register functions or one
that takes an extra argument?
register_led(.., .., ..)
and
register_led_irq(.., .., ..)
?
Then register_led() will call register_gpio() and
register_led_irq() will call register_gpio_irq().
If the low level driver (and everything else) can't
be put into busywaiting mode the register_gpio_irq()
will fail which will make register_led_irq() fail.
Naturally it can be
register_led(.., .., ..., use_irq)
and
register_gpio(.., .., .., use_irq)
I haven't looked at the API's so this is just theoretical
on my behalf, but it should solve some issues.
If a driver really really can't be busywait for some reason
then it will fail, otherwise it will switch to busywaiting
instead of sleeping.
Or am I just talking out of my ass?
// Stefan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
iD8DBQFDaq4yBrn2kJu9P78RAuh7AJ4wK/+xoMYyfdOXEtY0+LqMNxYuuACgtuDH
GhjBeXxbnW2SaQ4x7hlZg6w=
=hl/m
-----END PGP SIGNATURE-----
Hi!
> >> > I think even slow blinking was used somewhere. I have some code from
> >> > John Lenz (attached); it uses sysfs interface, exports led collor, and
> >> > allows setting different frequencies.
> >> >
> >> > Is that acceptable, or should some other interface be used?
> >>
> >> there is already an LED interface for linux-arm, which is
> >> used by a number of the extant machines in the sa11x0 and
> >> pxa range.
> >
> > Where is that interface? I think that making collie use it is obvious
> > first step...
>
> I originally wrote the collie led code to use that interface, and you
> might look at some of the old versions of the patch on my web site. The
> actual code is in arch/arm/kernel/time.c, but this code calls out to an
> individual machine function through say arch/arm/mach-sa1100/leds.c...
> The problem for collie was that the device model for locomo did not allow
> an easy way to do it... as you can see, in my patch it implements a driver
> for those leds and the driver model takes care of it.
>
> I just looked, and
> http://www.cs.wisc.edu/~lenz/zaurus/files/patch-2.6.7-jl2.diff.gz contins
> the implementation of the arm led interface for collie.... not sure if it
> will still work anymore, but...
It does, after kconfig fixups. Do you think we could get that merged?
Some led driver is better than none at all.
---
Simple collie LED driver, by John Lenz.
Signed-off-by: Pavel Machek <[email protected]>
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -408,7 +408,8 @@ config LEDS
ARCH_EBSA285 || ARCH_IMX || ARCH_INTEGRATOR || \
ARCH_LUBBOCK || MACH_MAINSTONE || ARCH_NETWINDER || \
ARCH_OMAP || ARCH_P720T || ARCH_PXA_IDP || \
- ARCH_SA1100 || ARCH_SHARK || ARCH_VERSATILE
+ ARCH_SA1100 || ARCH_SHARK || ARCH_VERSATILE || \
+ SA1100_COLLIE
help
If you say Y here, the LEDs on your machine will be used
to provide useful information about your current system status.
@@ -422,7 +423,8 @@ config LEDS
config LEDS_TIMER
bool "Timer LED" if (!ARCH_CDB89712 && !ARCH_OMAP) || \
- MACH_OMAP_H2 || MACH_OMAP_PERSEUS2
+ MACH_OMAP_H2 || MACH_OMAP_PERSEUS2 || \\
+ SA1100_COLLIE
depends on LEDS
default y if ARCH_EBSA110
help
@@ -438,7 +440,8 @@ config LEDS_TIMER
config LEDS_CPU
bool "CPU usage LED" if (!ARCH_CDB89712 && !ARCH_EBSA110 && \
- !ARCH_OMAP) || MACH_OMAP_H2 || MACH_OMAP_PERSEUS2
+ !ARCH_OMAP) || MACH_OMAP_H2 || MACH_OMAP_PERSEUS2 || \
+ SA1100_COLLIE
depends on LEDS
help
If you say Y here, the red LED will be used to give a good real
diff --git a/arch/arm/mach-sa1100/Makefile b/arch/arm/mach-sa1100/Makefile
--- a/arch/arm/mach-sa1100/Makefile
+++ b/arch/arm/mach-sa1100/Makefile
@@ -26,6 +26,7 @@ led-$(CONFIG_SA1100_CERF) += leds-cerf.
obj-$(CONFIG_SA1100_COLLIE) += collie.o
#obj-$(CONFIG_SA1100_COLLIE) += battery-collie.o
obj-$(CONFIG_SA1100_COLLIE) += collie_batswitch.o
+obj-$(CONFIG_SA1100_COLLIE) += leds-collie.o
obj-$(CONFIG_SA1100_H3600) += h3600.o
diff --git a/arch/arm/mach-sa1100/leds-collie.c b/arch/arm/mach-sa1100/leds-collie.c
new file mode 100644
--- /dev/null
+++ b/arch/arm/mach-sa1100/leds-collie.c
@@ -0,0 +1,126 @@
+/*
+ * linux/arch/arm/mach-sa1100/leds-collie.c
+ *
+ * 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.
+ *
+ * ChangeLog:
+ * - John Lenz <4/27/04> - added support for new locomo device model
+ */
+
+#include <linux/config.h>
+#include <linux/init.h>
+#include <linux/device.h>
+
+#include <asm/hardware.h>
+#include <asm/hardware/locomo.h>
+#include <asm/leds.h>
+#include <asm/system.h>
+
+#include "leds.h"
+
+#define LED_STATE_ENABLED 1
+#define LED_STATE_CLAIMED 2
+
+static struct locomo_dev *locomo_dev;
+static unsigned int led_state;
+static unsigned int hw_led0_state;
+static unsigned int hw_led1_state;
+
+#define LED_ONOFF_MASK (LOCOMO_LPT_TOFL|LOCOMO_LPT_TOFH)
+#define LED_OFF(REG) ((REG)|=LOCOMO_LPT_TOFL)
+#define LED_ON(REG) ((REG)=((REG)&~LED_ONOFF_MASK)|LOCOMO_LPT_TOFH)
+#define LED_FLIP(REG) ((REG)^=LED_ONOFF_MASK)
+
+void collie_leds_event(led_event_t evt)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+
+ switch (evt) {
+ case led_start:
+ led_state = LED_STATE_ENABLED;
+ LED_ON(hw_led0_state);
+ LED_ON(hw_led1_state);
+ break;
+
+ case led_stop:
+ led_state &= ~LED_STATE_ENABLED;
+ break;
+
+ case led_claim:
+ led_state |= LED_STATE_CLAIMED;
+ LED_ON(hw_led0_state);
+ LED_ON(hw_led1_state);
+ break;
+
+ case led_release:
+ led_state &= ~LED_STATE_CLAIMED;
+ LED_ON(hw_led0_state);
+ LED_ON(hw_led1_state);
+ break;
+
+#ifdef CONFIG_LEDS_TIMER
+ case led_timer:
+ if (!(led_state & LED_STATE_CLAIMED)) {
+ LED_FLIP(hw_led0_state);
+ }
+ break;
+#endif
+
+#ifdef CONFIG_LEDS_CPU
+ case led_idle_start:
+ /* LED off when system is idle */
+ if (!(led_state & LED_STATE_CLAIMED))
+ LED_OFF(hw_led1_state);
+ break;
+
+ case led_idle_end:
+ if (!(led_state & LED_STATE_CLAIMED))
+ LED_ON(hw_led1_state);
+ break;
+#endif
+
+ default:
+ break;
+ }
+
+ if (locomo_dev && led_state & LED_STATE_ENABLED) {
+ locomo_writel(hw_led0_state, locomo_dev->mapbase + LOCOMO_LPT0);
+ locomo_writel(hw_led1_state, locomo_dev->mapbase + LOCOMO_LPT1);
+ }
+
+ local_irq_restore(flags);
+}
+
+static int collieled_probe(struct locomo_dev *dev)
+{
+ locomo_dev = dev;
+ return 0;
+}
+
+static int collieled_remove(struct locomo_dev *dev) {
+ locomo_dev = NULL;
+ return 0;
+}
+
+static struct locomo_driver collieled_driver = {
+ .drv = {
+ .name = "locomoled"
+ },
+ .devid = LOCOMO_DEVID_LED,
+ .probe = collieled_probe,
+ .remove = collieled_remove,
+};
+
+static int __init collieled_init(void) {
+ return locomo_driver_register(&collieled_driver);
+}
+
+device_initcall(collieled_init);
+
+MODULE_AUTHOR("John Lenz <[email protected]>, Chris Larson <[email protected]>");
+MODULE_DESCRIPTION("LoCoMo Collie LED driver");
+MODULE_LICENSE("GPL");
diff --git a/arch/arm/mach-sa1100/leds.c b/arch/arm/mach-sa1100/leds.c
--- a/arch/arm/mach-sa1100/leds.c
+++ b/arch/arm/mach-sa1100/leds.c
@@ -26,6 +26,8 @@ sa1100_leds_init(void)
leds_event = brutus_leds_event;
if (machine_is_cerf())
leds_event = cerf_leds_event;
+ if (machine_is_collie())
+ leds_event = collie_leds_event;
if (machine_is_flexanet())
leds_event = flexanet_leds_event;
if (machine_is_graphicsclient())
diff --git a/arch/arm/mach-sa1100/leds.h b/arch/arm/mach-sa1100/leds.h
--- a/arch/arm/mach-sa1100/leds.h
+++ b/arch/arm/mach-sa1100/leds.h
@@ -3,6 +3,7 @@ extern void badge4_leds_event(led_event_
extern void consus_leds_event(led_event_t evt);
extern void brutus_leds_event(led_event_t evt);
extern void cerf_leds_event(led_event_t evt);
+extern void collie_leds_event(led_event_t evt);
extern void flexanet_leds_event(led_event_t evt);
extern void graphicsclient_leds_event(led_event_t evt);
extern void hackkit_leds_event(led_event_t evt);
--
Thanks, Sharp!
On Mon, November 7, 2005 5:30 pm, Pavel Machek said:
> Hi!
>
>> >> > I think even slow blinking was used somewhere. I have some code
>> from
>> >> > John Lenz (attached); it uses sysfs interface, exports led collor,
>> and
>> >> > allows setting different frequencies.
>> >> >
>> >> > Is that acceptable, or should some other interface be used?
>> >>
>> >> there is already an LED interface for linux-arm, which is
>> >> used by a number of the extant machines in the sa11x0 and
>> >> pxa range.
>> >
>> > Where is that interface? I think that making collie use it is obvious
>> > first step...
>>
>> I originally wrote the collie led code to use that interface, and you
>> might look at some of the old versions of the patch on my web site. The
>> actual code is in arch/arm/kernel/time.c, but this code calls out to an
>> individual machine function through say arch/arm/mach-sa1100/leds.c...
>> The problem for collie was that the device model for locomo did not
>> allow
>> an easy way to do it... as you can see, in my patch it implements a
>> driver
>> for those leds and the driver model takes care of it.
>>
>> I just looked, and
>> http://www.cs.wisc.edu/~lenz/zaurus/files/patch-2.6.7-jl2.diff.gz
>> contins
>> the implementation of the arm led interface for collie.... not sure if
>> it
>> will still work anymore, but...
>
> It does, after kconfig fixups. Do you think we could get that merged?
> Some led driver is better than none at all.
I wonder, because we are exporting an API to userspace. I don't think the
openembedded people want to use this API, and will hold off doing anything
with the leds till we get something else straigtend out. If we have this
API now, we will have issues breaking it later (or we will have to do some
wierd locking scheme to allow both interfaces control, and crap like
that).
Secondly, leds aren't that importent unless they are supported by the
userspace programs (to do things like blink when email shows up). And
before the userspace starts using leds, I think they might want to clear
up the interface API issue first.
John
Hi!
> >> I just looked, and
> >> http://www.cs.wisc.edu/~lenz/zaurus/files/patch-2.6.7-jl2.diff.gz
> >> contins
> >> the implementation of the arm led interface for collie.... not sure if
> >> it
> >> will still work anymore, but...
> >
> > It does, after kconfig fixups. Do you think we could get that merged?
> > Some led driver is better than none at all.
>
> I wonder, because we are exporting an API to userspace. I don't think the
> openembedded people want to use this API, and will hold off doing anything
> with the leds till we get something else straigtend out. If we have this
> API now, we will have issues breaking it later (or we will have to do some
> wierd locking scheme to allow both interfaces control, and crap like
> that).
* they are 9 users of "old" interface already, one more does not seem
like a big deal.
* arm maintainer does not want anything more complex than "old"
interface. And I can see his point. It is not clear if "new" interface
will get into mainline.
* there are very little users of collie, currently. Changing LED API
on myself does not seem like a big deal. [I'm trying hard to get _two
more_ users :-)]
* if openembedded people do not like current interface, they should a)
convince rmk API needs to change and b) convert all the drivers.
> Secondly, leds aren't that importent unless they are supported by the
> userspace programs (to do things like blink when email shows up). And
> before the userspace starts using leds, I think they might want to clear
> up the interface API issue first.
I'd say charger LED is somehow important, and I liked CPU usage LED a lot.
Pavel
--
Thanks, Sharp!
On Tue, 2005-11-08 at 10:28 +0100, Pavel Machek wrote:
> * they are 9 users of "old" interface already, one more does not seem
> like a big deal.
>
> * arm maintainer does not want anything more complex than "old"
> interface. And I can see his point. It is not clear if "new" interface
> will get into mainline.
>
> * there are very little users of collie, currently. Changing LED API
> on myself does not seem like a big deal. [I'm trying hard to get _two
> more_ users :-)]
>
> * if openembedded people do not like current interface, they should a)
> convince rmk API needs to change and b) convert all the drivers.
>
> > Secondly, leds aren't that importent unless they are supported by the
> > userspace programs (to do things like blink when email shows up). And
> > before the userspace starts using leds, I think they might want to clear
> > up the interface API issue first.
>
> I'd say charger LED is somehow important, and I liked CPU usage LED a lot.
You can implement the charger LED without needing to implement any led
interface, as the sharpsl_pm charger code does. For now the charge led
is hard coded (but easily changed later to become a trigger).
My rough plan is to implement the led trigger code and present this
along with a modified version of John's sysfs class code to LKML. If
accepted, great (and there was a demand for a standard interface from
various quarters). If not, I'll offer it as a patch series outside of
the kernel as openembedded, opie, gpe and handhelds.org are likely to
use it regardless of what LKML thinks. We can then see how its usage
becomes. The only change I will ask for is that the CONFIG_LEDS
namespace as used by ARM is reconsidered.
Both John and myself have resisted putting any LED code into mainline as
we don't feel the existing interfaces match Zaurus developer's and
user's needs. To submit such patches would suggest we planned to support
the existing interfaces which we do not.
I agree with John's point about changing API's as if LED code gets
added, developers and users may start using it which we don't want to
happen until we have an API we're happy with.
Richard
Hi!
> > * they are 9 users of "old" interface already, one more does not seem
> > like a big deal.
> >
> > * arm maintainer does not want anything more complex than "old"
> > interface. And I can see his point. It is not clear if "new" interface
> > will get into mainline.
> >
> > * there are very little users of collie, currently. Changing LED API
> > on myself does not seem like a big deal. [I'm trying hard to get _two
> > more_ users :-)]
> >
> > * if openembedded people do not like current interface, they should a)
> > convince rmk API needs to change and b) convert all the drivers.
> >
> > > Secondly, leds aren't that importent unless they are supported by the
> > > userspace programs (to do things like blink when email shows up). And
> > > before the userspace starts using leds, I think they might want to clear
> > > up the interface API issue first.
> >
> > I'd say charger LED is somehow important, and I liked CPU usage LED a lot.
>
> You can implement the charger LED without needing to implement any led
> interface, as the sharpsl_pm charger code does. For now the charge led
> is hard coded (but easily changed later to become a trigger).
Yes, it can be done.
> My rough plan is to implement the led trigger code and present this
> along with a modified version of John's sysfs class code to LKML. If
> accepted, great (and there was a demand for a standard interface from
> various quarters). If not, I'll offer it as a patch series outside of
> the kernel as openembedded, opie, gpe and handhelds.org are likely to
> use it regardless of what LKML thinks. We can then see how its usage
> becomes. The only change I will ask for is that the CONFIG_LEDS
> namespace as used by ARM is reconsidered.
Well, if it is accepted by LKML, fine. [I did not realize someone is
actually working on it].
If it is rejected, tough, I believe openembedded etc. should just use
official interface.
> Both John and myself have resisted putting any LED code into mainline as
> we don't feel the existing interfaces match Zaurus developer's and
> user's needs. To submit such patches would suggest we planned to support
> the existing interfaces which we do not.
Ok, I guess it all depends on LMKL and rmk's reaction. If LED layer is
rejected, I'd rather have something that can handle the simple
case. Hopefully enough cases (handhelds, IBM special leds, ...) are
there and LED layer gets accepted.
Pavel
--
Thanks, Sharp!