linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/11] LED Class, Triggers and Drivers
@ 2006-01-31 13:41 Richard Purdie
  2006-01-31 13:41 ` [PATCH 2/11] LED: Add LED Class Richard Purdie
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Richard Purdie @ 2006-01-31 13:41 UTC (permalink / raw)
  To: LKML
  Cc: Russell King, John Lenz, Pavel Machek, Andrew Morton, tglx, dirk,
	jbowler

This is an updated version of the LED class/subsystem I proposed a while
ago. It takes John Lenz's work and extends and alters it to give what I
think should be a fairly universal LED implementation.

Hopefully a decision on whether this is going to head into mainline or
not can be made soon. I've not had any feedback from Russell on this
issue (but have asked). 

I'm unsure what the kconfig name for the class should be (something
better than NEW_LEDS is needed but arm is using LEDS).

The series consists of several logical units:

* LED Core + Class implementation
* LED Trigger Core implementation
* LED timer trigger (example of a complex trigger)
* LED device drivers for corgi, spitz and tosa Zaurus models
* LED device driver for locomo LEDs
* LED device driver for ARM ixp4xx LEDs
* Zaurus charging LED trigger
* IDE disk activity LED trigger
* NAND MTD activity LED trigger


Changes
=======

The patches have been updated after the feedback from GregKH and others
on LKML and other feedback elsewhere. These include:

* using class_create()/class_destroy()/class_device_create()
* updates to the timer trigger so it accepts a duty parameter
* addition of led drivers for tosa and ixp4xx
* using an enum for led brightness
* correct the ide trigger when ide_disk isn't compiled
* added some brief documentation


Why?
====

LEDs are really simple devices usually amounting to a GPIO that can be
turned on and off so why do we need all this code? On handheld or
embedded devices they're an important part of an often limited user
interface. Both users and developers want to be able to control and
configure what the LED does and the number of different things they'd
potentially want the LED to show is large. 

A subsystem is needed to try and provide all this different
functionality in an architecture independent, simple but complete,
generic and scalable manner.

The alternative is for everyone to implement just what they need hidden
away in different corners of the kernel source tree and to provide an
inconsistent interface to userspace.


Other Implementations
=====================

I'm aware of the existing arm led implementation. Currently the new
subsystem and the arm code can coexist quite happily. Its up to the arm
community to decide whether this new interface is acceptable to them. As
far as I can see, the new interface can do everything the existing arm
implementation can with the advantage that the new code is architecture
independent and much more generic configurable and scalable.

I'm prepared to make the conversion to the LED subsystem (or assist with
it) if appropriate.


Implementation Details
======================

I've stripped a lot of code out of John's original LED class. Colours
were removed as LED colour is now part of the device name. Multiple
colours are to be handled as multiple led devices. This means you get
full control over each colour. I also removed the LED hardware timer
code as the generic timer isn't going to add much overhead and is just
as useful. I also decided to have the LED core track the current LED
status (to ease suspend/resume handling) removing the need for
brightness_get implementations in the LED drivers.

An underlying design philosophy is simplicity. The aim is to keep a
small amount of code giving as much functionality as possible.

The major new idea is the led "trigger". A trigger is a source of led
events. Triggers can either be simple or complex. A simple trigger isn't
configurable and is designed to slot into existing subsystems with
minimal additional code. Examples are the ide-disk, nand-disk and
zaurus-charging triggers. With leds disabled, the code optimises away.
Examples are nand-disk and ide-disk.

Complex triggers whilst available to all LEDs have LED specific
parameters and work on a per LED basis. The timer trigger is an example.

You can change triggers in a similar manner to the way an IO scheduler
is chosen (via /sys/class/leds/somedevice/trigger).

So far there are only a handful of examples but it should easy to add
further LED triggers without too much interference into other
subsystems.


Known Issues
============

The LED Trigger core cannot be a module as the simple trigger functions
would cause nightmare dependency issues. I see this as a minor issue
compared to the benefits the simple trigger functionality brings. The
rest of the LED subsystem can be modular.

Some leds can be programmed to flash in hardware. As this isn't a
generic LED device property, I think this should be exported as a device
specific sysfs attribute rather than part of the class if this
functionality is required (eg. to keep the led flashing whilst the
device is suspended).


Future Development
==================

At the moment, a trigger can't be created specifically for a single LED.
There are a number of cases where a trigger might only be mappable to a
particular LED. The addition of triggers provided by the LED driver
should cover this option and be possible to add without breaking the
current interface.

A cpu activity trigger similar to that found in the arm led
implementation should be trivial to add.


Richard



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/11] LED: Add LED Class
@ 2006-01-31 13:41 ` Richard Purdie
  2006-01-31 14:55   ` Jan-Benedict Glaw
  2006-01-31 20:59   ` Greg KH
  0 siblings, 2 replies; 12+ messages in thread
From: Richard Purdie @ 2006-01-31 13:41 UTC (permalink / raw)
  To: LKML

Add the foundations of a new LEDs subsystem. This patch adds a class
which presents LED devices within sysfs and allows their brightness to
be controlled.

Signed-off-by: Richard Purdie <rpurdie@rpsys.net>

Index: linux-2.6.15/drivers/leds/Kconfig
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.15/drivers/leds/Kconfig	2006-01-29 16:02:38.000000000 +0000
@@ -0,0 +1,18 @@
+
+menu "LED devices"
+
+config NEW_LEDS
+	bool "LED Support"
+	help
+	  Say Y to enable Linux LED support.  This is not related to standard
+	  keyboard LEDs which are controlled via the input system.
+
+config LEDS_CLASS
+	tristate "LED Class Support"
+	depends NEW_LEDS
+	help
+	  This option enables the led sysfs class in /sys/class/leds.  You'll
+	  need this to do anything useful with LEDs.  If unsure, say N.
+
+endmenu
+
Index: linux-2.6.15/drivers/leds/Makefile
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.15/drivers/leds/Makefile	2006-01-29 16:02:38.000000000 +0000
@@ -0,0 +1,4 @@
+
+# LED Core
+obj-$(CONFIG_NEW_LEDS)			+= led-core.o
+obj-$(CONFIG_LEDS_CLASS)		+= led-class.o
Index: linux-2.6.15/include/linux/leds.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.15/include/linux/leds.h	2006-01-29 16:03:21.000000000 +0000
@@ -0,0 +1,48 @@
+/*
+ * Driver model for leds and led triggers
+ *
+ * Copyright (C) 2005 John Lenz <lenz@cs.wisc.edu>
+ * Copyright (C) 2005 Richard Purdie <rpurdie@openedhand.com>
+ *
+ * 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.
+ *
+ */
+
+struct device;
+struct class_device;
+/*
+ * LED Core
+ */
+
+enum led_brightness {
+	LED_OFF = 0,
+	LED_HALF = 127,
+	LED_FULL = 255,
+};
+
+struct led_device {
+	const char *name;
+	int brightness;
+	int flags;
+#define LED_SUSPENDED       (1 << 0)
+
+	/* A function to set the brightness of the led */
+	void (*brightness_set)(struct led_device *led_dev, enum led_brightness brightness);
+
+	struct class_device *class_dev;
+	/* LED Device linked list */
+	struct list_head node;
+
+	/* Trigger data */
+	char *default_trigger;
+
+	/* This protects the data in this structure */
+	rwlock_t lock;
+};
+
+extern int led_device_register(struct device *dev, struct led_device *led_dev);
+extern void led_device_unregister(struct led_device *led_dev);
+extern void led_device_suspend(struct led_device *led_dev);
+extern void led_device_resume(struct led_device *led_dev);
Index: linux-2.6.15/arch/arm/Kconfig
===================================================================
--- linux-2.6.15.orig/arch/arm/Kconfig	2006-01-29 14:37:31.000000000 +0000
+++ linux-2.6.15/arch/arm/Kconfig	2006-01-29 16:02:15.000000000 +0000
@@ -774,6 +774,8 @@
 
 source "drivers/mfd/Kconfig"
 
+source "drivers/leds/Kconfig"
+
 source "drivers/media/Kconfig"
 
 source "drivers/video/Kconfig"
Index: linux-2.6.15/drivers/Makefile
===================================================================
--- linux-2.6.15.orig/drivers/Makefile	2006-01-29 14:42:52.000000000 +0000
+++ linux-2.6.15/drivers/Makefile	2006-01-29 14:43:11.000000000 +0000
@@ -68,6 +68,7 @@
 obj-$(CONFIG_EISA)		+= eisa/
 obj-$(CONFIG_CPU_FREQ)		+= cpufreq/
 obj-$(CONFIG_MMC)		+= mmc/
+obj-$(CONFIG_NEW_LEDS)		+= leds/
 obj-$(CONFIG_INFINIBAND)	+= infiniband/
 obj-$(CONFIG_SGI_IOC4)		+= sn/
 obj-y				+= firmware/
Index: linux-2.6.15/drivers/Kconfig
===================================================================
--- linux-2.6.15.orig/drivers/Kconfig	2006-01-29 14:42:51.000000000 +0000
+++ linux-2.6.15/drivers/Kconfig	2006-01-29 14:43:11.000000000 +0000
@@ -64,6 +64,8 @@
 
 source "drivers/mmc/Kconfig"
 
+source "drivers/leds/Kconfig"
+
 source "drivers/infiniband/Kconfig"
 
 source "drivers/sn/Kconfig"
Index: linux-2.6.15/drivers/leds/leds.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.15/drivers/leds/leds.h	2006-01-29 16:02:38.000000000 +0000
@@ -0,0 +1,26 @@
+/*
+ * LED Core
+ *
+ * Copyright 2005 Openedhand Ltd.
+ *
+ * Author: Richard Purdie <rpurdie@openedhand.com>
+ *
+ * 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.
+ *
+ */
+
+/* led_dev->lock must be held as write */
+static inline void led_set_brightness(struct led_device *led_dev, enum led_brightness value)
+{
+	if (value > LED_FULL)
+		value = LED_FULL;
+	led_dev->brightness = value;
+	if (!(led_dev->flags & LED_SUSPENDED))
+		led_dev->brightness_set(led_dev, value);
+}
+
+extern rwlock_t leds_list_lock;
+extern struct list_head leds_list;
+
Index: linux-2.6.15/drivers/leds/led-class.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.15/drivers/leds/led-class.c	2006-01-29 16:02:38.000000000 +0000
@@ -0,0 +1,150 @@
+/*
+ * LED Class Core
+ *
+ * Copyright (C) 2005 John Lenz <lenz@cs.wisc.edu>
+ * Copyright (C) 2005-2006 Richard Purdie <rpurdie@openedhand.com>
+ *
+ * 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/err.h>
+#include <linux/leds.h>
+#include "leds.h"
+
+static struct class *leds_class;
+
+static ssize_t led_brightness_show(struct class_device *dev, char *buf)
+{
+	struct led_device *led_dev = dev->class_data;
+	ssize_t ret = 0;
+
+	/* no lock needed for this */
+	sprintf(buf, "%u\n", led_dev->brightness);
+	ret = strlen(buf) + 1;
+
+	return ret;
+}
+
+static ssize_t led_brightness_store(struct class_device *dev, const char *buf, size_t size)
+{
+	struct led_device *led_dev = dev->class_data;
+	ssize_t ret = -EINVAL;
+	char *after;
+
+	unsigned long state = simple_strtoul(buf, &after, 10);
+	if (after - buf > 0) {
+		ret = after - buf;
+		write_lock(&led_dev->lock);
+		led_set_brightness(led_dev, state);
+		write_unlock(&led_dev->lock);
+	}
+
+	return ret;
+}
+
+static CLASS_DEVICE_ATTR(brightness, 0644, led_brightness_show, led_brightness_store);
+
+
+/**
+ * led_device_suspend - suspend an led_device.
+ * @led_dev: the led_device to suspend.
+ */
+void led_device_suspend(struct led_device *led_dev)
+{
+	write_lock(&led_dev->lock);
+	led_dev->flags |= LED_SUSPENDED;
+	led_dev->brightness_set(led_dev, 0);
+	write_unlock(&led_dev->lock);
+}
+
+/**
+ * led_device_resume - resume an led_device.
+ * @led_dev: the led_device to resume.
+ */
+void led_device_resume(struct led_device *led_dev)
+{
+	write_lock(&led_dev->lock);
+	led_dev->flags &= ~LED_SUSPENDED;
+	led_dev->brightness_set(led_dev, led_dev->brightness);
+	write_unlock(&led_dev->lock);
+}
+
+/**
+ * led_device_register - register a new object of led_device class.
+ * @dev: The device to register.
+ * @led_dev: the led_device structure for this device.
+ */
+int led_device_register(struct device *dev, struct led_device *led_dev)
+{
+	led_dev->class_dev = class_device_create(leds_class, NULL, 0, dev, "%s", led_dev->name);
+	if (unlikely(IS_ERR(led_dev->class_dev)))
+		return PTR_ERR(led_dev->class_dev);
+
+	rwlock_init(&led_dev->lock);
+	led_dev->class_dev->class_data = led_dev;
+
+	/* register the attributes */
+	class_device_create_file(led_dev->class_dev, &class_device_attr_brightness);
+
+	/* add to the list of leds */
+	write_lock(&leds_list_lock);
+	list_add_tail(&led_dev->node, &leds_list);
+	write_unlock(&leds_list_lock);
+
+	printk(KERN_INFO "Registered led device: %s\n", led_dev->class_dev->class_id);
+
+	return 0;
+}
+
+/**
+ * led_device_unregister - unregisters a object of led_properties class.
+ * @led_dev: the led device to unreigister
+ *
+ * Unregisters a previously registered via led_device_register object.
+ */
+void led_device_unregister(struct led_device *led_dev)
+{
+	class_device_remove_file(led_dev->class_dev, &class_device_attr_brightness);
+
+	class_device_unregister(led_dev->class_dev);
+
+	write_lock(&leds_list_lock);
+	list_del(&led_dev->node);
+	write_unlock(&leds_list_lock);
+}
+
+EXPORT_SYMBOL_GPL(led_device_suspend);
+EXPORT_SYMBOL_GPL(led_device_resume);
+EXPORT_SYMBOL_GPL(led_device_register);
+EXPORT_SYMBOL_GPL(led_device_unregister);
+
+static int __init leds_init(void)
+{
+	leds_class = class_create(THIS_MODULE, "leds");
+	if (IS_ERR(leds_class))
+		return PTR_ERR(leds_class);
+	return 0;
+}
+
+static void __exit leds_exit(void)
+{
+	class_destroy(leds_class);
+}
+
+subsys_initcall(leds_init);
+module_exit(leds_exit);
+
+MODULE_AUTHOR("John Lenz, Richard Purdie");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("LED Class Interface");
Index: linux-2.6.15/drivers/leds/led-core.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.15/drivers/leds/led-core.c	2006-01-29 14:43:11.000000000 +0000
@@ -0,0 +1,24 @@
+/*
+ * LED Class Core
+ *
+ * Copyright 2005-2006 Openedhand Ltd.
+ *
+ * Author: Richard Purdie <rpurdie@openedhand.com>
+ *
+ * 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/kernel.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/leds.h>
+#include "leds.h"
+
+rwlock_t leds_list_lock = RW_LOCK_UNLOCKED;
+LIST_HEAD(leds_list);
+
+EXPORT_SYMBOL_GPL(leds_list);



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 4/11] LED: Add LED Timer Trigger
@ 2006-01-31 13:41 ` Richard Purdie
  2006-01-31 15:01   ` Jan-Benedict Glaw
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Purdie @ 2006-01-31 13:41 UTC (permalink / raw)
  To: LKML

Add an example of a complex LED trigger in the form of a generic timer 
which triggers the LED its attached to at a user specified frequency
and duty cycle.

Signed-off-by: Richard Purdie <rpurdie@rpsys.net>

Index: linux-2.6.15/drivers/leds/Kconfig
===================================================================
--- linux-2.6.15.orig/drivers/leds/Kconfig	2006-01-29 16:13:48.000000000 +0000
+++ linux-2.6.15/drivers/leds/Kconfig	2006-01-29 20:32:16.000000000 +0000
@@ -22,5 +22,12 @@
 	  These triggers allow kernel events to drive the LEDs and can
 	  be configured via sysfs. If unsure, say Y.
 
+config LEDS_TRIGGER_TIMER
+	tristate "LED Timer Trigger"
+	depends LEDS_TRIGGERS
+	help
+	  This allows LEDs to be controlled by a programmable timer
+	  via sysfs. If unsure, say Y.
+
 endmenu
 
Index: linux-2.6.15/drivers/leds/Makefile
===================================================================
--- linux-2.6.15.orig/drivers/leds/Makefile	2006-01-29 16:13:48.000000000 +0000
+++ linux-2.6.15/drivers/leds/Makefile	2006-01-29 20:32:16.000000000 +0000
@@ -3,3 +3,6 @@
 obj-$(CONFIG_NEW_LEDS)			+= led-core.o
 obj-$(CONFIG_LEDS_CLASS)		+= led-class.o
 obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
+
+# LED Triggers
+obj-$(CONFIG_LEDS_TRIGGER_TIMER)	+= ledtrig-timer.o
Index: linux-2.6.15/drivers/leds/ledtrig-timer.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.15/drivers/leds/ledtrig-timer.c	2006-01-29 17:40:11.000000000 +0000
@@ -0,0 +1,204 @@
+/*
+ * LED Kernel Timer Trigger
+ *
+ * Copyright 2005-2006 Openedhand Ltd.
+ *
+ * Author: Richard Purdie <rpurdie@openedhand.com>
+ *
+ * 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>
+#include "leds.h"
+
+struct timer_trig_data {
+	unsigned long duty; /* duty cycle, as a percentage */
+	unsigned long frequency; /* frequency of blinking, in Hz */
+	unsigned long delay_on; /* milliseconds on */
+	unsigned long delay_off; /* milliseconds off */
+	struct timer_list timer;
+};
+
+static void led_timer_function(unsigned long data)
+{
+	struct led_device *led_dev = (struct led_device *) data;
+	struct timer_trig_data *timer_data = led_dev->trigger_data;
+	unsigned long brightness = LED_OFF;
+	unsigned long delay = timer_data->delay_off;
+
+	write_lock(&led_dev->lock);
+
+	if (!timer_data->frequency) {
+		led_set_brightness(led_dev, LED_OFF);
+		write_unlock(&led_dev->lock);
+		return;
+	}
+
+	if (!led_dev->brightness) {
+		brightness = LED_FULL;
+		delay = timer_data->delay_on;
+	}
+
+	led_set_brightness(led_dev, brightness);
+
+	mod_timer(&timer_data->timer, jiffies + msecs_to_jiffies(delay));
+	write_unlock(&led_dev->lock);
+}
+
+/* led_dev write lock needs to be held */
+static void led_timer_setdata(struct led_device *led_dev, unsigned long duty, unsigned long frequency)
+{
+	struct timer_trig_data *timer_data = led_dev->trigger_data;
+	signed long duty1;
+
+	if (frequency > 500)
+		frequency = 500;
+
+	if (duty > 100)
+		duty = 100;
+
+	duty1 = duty - 50;
+
+	timer_data->duty = duty;
+	timer_data->frequency = frequency;
+	if (frequency != 0) {
+		timer_data->delay_on = (50 - duty1) * 1000 / 50 / frequency;
+		timer_data->delay_off = (50 + duty1) * 1000 / 50 / frequency;
+	}
+
+	mod_timer(&timer_data->timer, jiffies);
+}
+
+static ssize_t led_duty_show(struct class_device *dev, char *buf)
+{
+	struct led_device *led_dev = dev->class_data;
+	struct timer_trig_data *timer_data;
+
+	read_lock(&led_dev->lock);
+	timer_data = led_dev->trigger_data;
+	sprintf(buf, "%lu\n", timer_data->duty);
+	read_unlock(&led_dev->lock);
+
+	return strlen(buf) + 1;
+}
+
+static ssize_t led_duty_store(struct class_device *dev, const char *buf, size_t size)
+{
+	struct led_device *led_dev = dev->class_data;
+	struct timer_trig_data *timer_data;
+	int ret = -EINVAL;
+	char *after;
+
+	unsigned long state = simple_strtoul(buf, &after, 10);
+	if (after - buf > 0) {
+		ret = after - buf;
+		write_lock(&led_dev->lock);
+		timer_data = led_dev->trigger_data;
+		led_timer_setdata(led_dev, state, timer_data->frequency);
+		write_unlock(&led_dev->lock);
+	}
+
+	return ret;
+}
+
+
+static ssize_t led_frequency_show(struct class_device *dev, char *buf)
+{
+	struct led_device *led_dev = dev->class_data;
+	struct timer_trig_data *timer_data;
+
+	read_lock(&led_dev->lock);
+	timer_data = led_dev->trigger_data;
+	sprintf(buf, "%lu\n", timer_data->frequency);
+	read_unlock(&led_dev->lock);
+
+	return strlen(buf) + 1;
+}
+
+static ssize_t led_frequency_store(struct class_device *dev, const char *buf, size_t size)
+{
+	struct led_device *led_dev = dev->class_data;
+	struct timer_trig_data *timer_data;
+	int ret = -EINVAL;
+	char *after;
+
+	unsigned long state = simple_strtoul(buf, &after, 10);
+	if (after - buf > 0) {
+		ret = after - buf;
+		write_lock(&led_dev->lock);
+		timer_data = led_dev->trigger_data;
+		led_timer_setdata(led_dev, timer_data->duty, state);
+		write_unlock(&led_dev->lock);
+	}
+
+	return ret;
+}
+
+static CLASS_DEVICE_ATTR(duty, 0644, led_duty_show, led_duty_store);
+static CLASS_DEVICE_ATTR(frequency, 0644, led_frequency_show, led_frequency_store);
+
+void timer_trig_activate(struct led_device *led_dev)
+{
+	struct timer_trig_data *timer_data;
+
+	timer_data = kzalloc(sizeof(struct timer_trig_data), GFP_KERNEL);
+	if (!timer_data)
+		return;
+
+	led_dev->trigger_data = timer_data;
+
+	init_timer(&timer_data->timer);
+	timer_data->timer.function = led_timer_function;
+	timer_data->timer.data = (unsigned long) led_dev;
+
+	timer_data->duty = 50;
+
+	class_device_create_file(led_dev->class_dev, &class_device_attr_duty);
+	class_device_create_file(led_dev->class_dev, &class_device_attr_frequency);
+}
+
+void timer_trig_deactivate(struct led_device *led_dev)
+{
+	struct timer_trig_data *timer_data = led_dev->trigger_data;
+	if (timer_data) {
+		class_device_remove_file(led_dev->class_dev, &class_device_attr_duty);
+		class_device_remove_file(led_dev->class_dev, &class_device_attr_frequency);
+		del_timer_sync(&timer_data->timer);
+		kfree(timer_data);
+	}
+}
+
+static struct led_trigger timer_led_trigger = {
+	.name     = "timer",
+	.activate = timer_trig_activate,
+	.deactivate = timer_trig_deactivate,
+};
+
+static int __init timer_trig_init(void)
+{
+	return led_trigger_register(&timer_led_trigger);
+}
+
+static void __exit timer_trig_exit (void)
+{
+	led_trigger_unregister(&timer_led_trigger);
+}
+
+module_init(timer_trig_init);
+module_exit(timer_trig_exit);
+
+MODULE_AUTHOR("Richard Purdie <rpurdie@openedhand.com>");
+MODULE_DESCRIPTION("Timer LED trigger");
+MODULE_LICENSE("GPL");



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/11] LED: Add LED Class
  2006-01-31 13:41 ` [PATCH 2/11] LED: Add LED Class Richard Purdie
@ 2006-01-31 14:55   ` Jan-Benedict Glaw
  2006-01-31 20:59   ` Greg KH
  1 sibling, 0 replies; 12+ messages in thread
From: Jan-Benedict Glaw @ 2006-01-31 14:55 UTC (permalink / raw)
  To: Richard Purdie; +Cc: LKML

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

On Tue, 2006-01-31 13:41:28 +0000, Richard Purdie <rpurdie@rpsys.net> wrote:
> Index: linux-2.6.15/include/linux/leds.h
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6.15/include/linux/leds.h	2006-01-29 16:03:21.000000000 +0000
> +enum led_brightness {
> +	LED_OFF = 0,
> +	LED_HALF = 127,
> +	LED_FULL = 255,
> +};
> +
> +struct led_device {
> +	/* A function to set the brightness of the led */
> +	void (*brightness_set)(struct led_device *led_dev, enum led_brightness brightness);

I somehow dislike using an enum being abused for a dimmed LED value...

MfG, JBG

-- 
Jan-Benedict Glaw       jbglaw@lug-owl.de    . +49-172-7608481             _ O _
"Eine Freie Meinung in  einem Freien Kopf    | Gegen Zensur | Gegen Krieg  _ _ O
 für 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));

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/11] LED: Add LED Timer Trigger
  2006-01-31 13:41 ` [PATCH 4/11] LED: Add LED Timer Trigger Richard Purdie
@ 2006-01-31 15:01   ` Jan-Benedict Glaw
  2006-01-31 17:36     ` Richard Purdie
  0 siblings, 1 reply; 12+ messages in thread
From: Jan-Benedict Glaw @ 2006-01-31 15:01 UTC (permalink / raw)
  To: Richard Purdie; +Cc: LKML

[-- Attachment #1: Type: text/plain, Size: 1454 bytes --]

On Tue, 2006-01-31 13:41:37 +0000, Richard Purdie <rpurdie@rpsys.net> wrote:
> Index: linux-2.6.15/drivers/leds/ledtrig-timer.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6.15/drivers/leds/ledtrig-timer.c	2006-01-29 17:40:11.000000000 +0000
> @@ -0,0 +1,204 @@
> +/* led_dev write lock needs to be held */
> +static void led_timer_setdata(struct led_device *led_dev, unsigned long duty, unsigned long frequency)
> +{
> +	struct timer_trig_data *timer_data = led_dev->trigger_data;
> +	signed long duty1;
> +
> +	if (frequency > 500)
> +		frequency = 500;

Why? ...and especially: why, without complaining?

> +	if (duty > 100)
> +		duty = 100;

Dito.

> +	duty1 = duty - 50;
> +
> +	timer_data->duty = duty;
> +	timer_data->frequency = frequency;
> +	if (frequency != 0) {
> +		timer_data->delay_on = (50 - duty1) * 1000 / 50 / frequency;
> +		timer_data->delay_off = (50 + duty1) * 1000 / 50 / frequency;
> +	}

Nice math :-)

> +	mod_timer(&timer_data->timer, jiffies);
> +}
> +

MfG, JBG

-- 
Jan-Benedict Glaw       jbglaw@lug-owl.de    . +49-172-7608481             _ O _
"Eine Freie Meinung in  einem Freien Kopf    | Gegen Zensur | Gegen Krieg  _ _ O
 für 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));

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/11] LED Class, Triggers and Drivers
  2006-01-31 13:41 [PATCH 0/11] LED Class, Triggers and Drivers Richard Purdie
  2006-01-31 13:41 ` [PATCH 2/11] LED: Add LED Class Richard Purdie
  2006-01-31 13:41 ` [PATCH 4/11] LED: Add LED Timer Trigger Richard Purdie
@ 2006-01-31 15:04 ` Jan-Benedict Glaw
  2006-01-31 15:33 ` Pierre Ossman
  3 siblings, 0 replies; 12+ messages in thread
From: Jan-Benedict Glaw @ 2006-01-31 15:04 UTC (permalink / raw)
  To: Richard Purdie
  Cc: LKML, Russell King, John Lenz, Pavel Machek, Andrew Morton, tglx,
	dirk, jbowler

[-- Attachment #1: Type: text/plain, Size: 914 bytes --]

On Tue, 2006-01-31 13:41:22 +0000, Richard Purdie <rpurdie@rpsys.net> wrote:
> This is an updated version of the LED class/subsystem I proposed a while
> ago. It takes John Lenz's work and extends and alters it to give what I
> think should be a fairly universal LED implementation.

I'd abandon my own LED implementation I wrote for the status LEDs of
some VAXen, though it cannot substitute it fully (eg. there are some
7-segment displays using bitmasks _or_ 0x00..0x0f for the bars) once
some small tweaks are done (which I just sent with the other emails.)

MfG, JBG

-- 
Jan-Benedict Glaw       jbglaw@lug-owl.de    . +49-172-7608481             _ O _
"Eine Freie Meinung in  einem Freien Kopf    | Gegen Zensur | Gegen Krieg  _ _ O
 für 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));

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/11] LED Class, Triggers and Drivers
  2006-01-31 13:41 [PATCH 0/11] LED Class, Triggers and Drivers Richard Purdie
                   ` (2 preceding siblings ...)
  2006-01-31 15:04 ` [PATCH 0/11] LED Class, Triggers and Drivers Jan-Benedict Glaw
@ 2006-01-31 15:33 ` Pierre Ossman
  2006-01-31 16:48   ` Richard Purdie
  3 siblings, 1 reply; 12+ messages in thread
From: Pierre Ossman @ 2006-01-31 15:33 UTC (permalink / raw)
  To: Richard Purdie
  Cc: LKML, Russell King, John Lenz, Pavel Machek, Andrew Morton, tglx,
	dirk, jbowler

Richard Purdie wrote:
> 
> Future Development
> ==================

A MMC trigger in the pipeline? My new SDHCI MMC driver can be built as a
LED device, but it should probably be mapped to MMC activity by default.

Rgds
Pierre


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/11] LED Class, Triggers and Drivers
  2006-01-31 15:33 ` Pierre Ossman
@ 2006-01-31 16:48   ` Richard Purdie
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Purdie @ 2006-01-31 16:48 UTC (permalink / raw)
  To: Pierre Ossman
  Cc: LKML, Russell King, John Lenz, Pavel Machek, Andrew Morton, dirk

On Tue, 2006-01-31 at 16:33 +0100, Pierre Ossman wrote:
>
> A MMC trigger in the pipeline? My new SDHCI MMC driver can be built as a
> LED device, but it should probably be mapped to MMC activity by default.

Not specifically. I suspect about 5 lines of code in mmc_block.c would
be enough though. Patches welcome :)

Richard


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/11] LED: Add LED Timer Trigger
  2006-01-31 15:01   ` Jan-Benedict Glaw
@ 2006-01-31 17:36     ` Richard Purdie
  0 siblings, 0 replies; 12+ messages in thread
From: Richard Purdie @ 2006-01-31 17:36 UTC (permalink / raw)
  To: Jan-Benedict Glaw; +Cc: LKML

On Tue, 2006-01-31 at 16:01 +0100, Jan-Benedict Glaw wrote:
> On Tue, 2006-01-31 13:41:37 +0000, Richard Purdie <rpurdie@rpsys.net> wrote:
> > +static void led_timer_setdata(struct led_device *led_dev, unsigned long duty, unsigned long frequency)
> > +{
> > +	struct timer_trig_data *timer_data = led_dev->trigger_data;
> > +	signed long duty1;
> > +
> > +	if (frequency > 500)
> > +		frequency = 500;
> 
> Why? 

We're dealing with msec delays. Any frequency > 1000 will just cause
problems. There was a reason for using half that but it escapes me and
might be unneeded now. 500Hz/1000Hz is above the frequency the human eye
can see so is unlikely to present a problem.

> ...and especially: why, without complaining?

This is the important bit. It should return an -EINVAL back to
userspace.

> > +	if (duty > 100)
> > +		duty = 100;
> 
> Dito.

Duty cycles > 100 make no sense and would break the subsequent
calculation. Same problem/solution as above.

Thanks,

Richard



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/11] LED: Add LED Class
  2006-01-31 13:41 ` [PATCH 2/11] LED: Add LED Class Richard Purdie
  2006-01-31 14:55   ` Jan-Benedict Glaw
@ 2006-01-31 20:59   ` Greg KH
  2006-01-31 21:41     ` Richard Purdie
  1 sibling, 1 reply; 12+ messages in thread
From: Greg KH @ 2006-01-31 20:59 UTC (permalink / raw)
  To: Richard Purdie; +Cc: LKML

On Tue, Jan 31, 2006 at 01:41:28PM +0000, Richard Purdie wrote:
> +/**
> + * led_device_register - register a new object of led_device class.
> + * @dev: The device to register.
> + * @led_dev: the led_device structure for this device.
> + */
> +int led_device_register(struct device *dev, struct led_device *led_dev)

Shouldn't struct led_device contain a struct device within it, like the
rest of the driver model?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/11] LED: Add LED Class
  2006-01-31 20:59   ` Greg KH
@ 2006-01-31 21:41     ` Richard Purdie
  2006-02-01  1:41       ` Greg KH
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Purdie @ 2006-01-31 21:41 UTC (permalink / raw)
  To: Greg KH; +Cc: LKML

On Tue, 2006-01-31 at 12:59 -0800, Greg KH wrote:
> On Tue, Jan 31, 2006 at 01:41:28PM +0000, Richard Purdie wrote:
> > +/**
> > + * led_device_register - register a new object of led_device class.
> > + * @dev: The device to register.
> > + * @led_dev: the led_device structure for this device.
> > + */
> > +int led_device_register(struct device *dev, struct led_device *led_dev)
> 
> Shouldn't struct led_device contain a struct device within it, like the
> rest of the driver model?

The code supports more than one led per struct device. 

Perhaps the name is misleading and should be led_class_register? The
code has changed a lot through its various development stages (it did
start out as a device IIRC).

led_device should also probably be led_class by that argument...

Richard


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/11] LED: Add LED Class
  2006-01-31 21:41     ` Richard Purdie
@ 2006-02-01  1:41       ` Greg KH
  0 siblings, 0 replies; 12+ messages in thread
From: Greg KH @ 2006-02-01  1:41 UTC (permalink / raw)
  To: Richard Purdie; +Cc: LKML

On Tue, Jan 31, 2006 at 09:41:37PM +0000, Richard Purdie wrote:
> On Tue, 2006-01-31 at 12:59 -0800, Greg KH wrote:
> > On Tue, Jan 31, 2006 at 01:41:28PM +0000, Richard Purdie wrote:
> > > +/**
> > > + * led_device_register - register a new object of led_device class.
> > > + * @dev: The device to register.
> > > + * @led_dev: the led_device structure for this device.
> > > + */
> > > +int led_device_register(struct device *dev, struct led_device *led_dev)
> > 
> > Shouldn't struct led_device contain a struct device within it, like the
> > rest of the driver model?
> 
> The code supports more than one led per struct device. 
> 
> Perhaps the name is misleading and should be led_class_register?

Yes, that would make more sense, and have the struct device * be called
"parent".

> The code has changed a lot through its various development stages (it
> did start out as a device IIRC).
> 
> led_device should also probably be led_class by that argument...

Yes.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2006-02-01  1:41 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-01-31 13:41 [PATCH 0/11] LED Class, Triggers and Drivers Richard Purdie
2006-01-31 13:41 ` [PATCH 2/11] LED: Add LED Class Richard Purdie
2006-01-31 14:55   ` Jan-Benedict Glaw
2006-01-31 20:59   ` Greg KH
2006-01-31 21:41     ` Richard Purdie
2006-02-01  1:41       ` Greg KH
2006-01-31 13:41 ` [PATCH 4/11] LED: Add LED Timer Trigger Richard Purdie
2006-01-31 15:01   ` Jan-Benedict Glaw
2006-01-31 17:36     ` Richard Purdie
2006-01-31 15:04 ` [PATCH 0/11] LED Class, Triggers and Drivers Jan-Benedict Glaw
2006-01-31 15:33 ` Pierre Ossman
2006-01-31 16:48   ` Richard Purdie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).