lkml.org 
[lkml]   [2006]   [Feb]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRE: [PATCH 4/12] LED: Add LED Timer Trigger
From
Date
As mentioned on LKML by John Bowler, using frequency and duty as
attributes is a bad idea as it restricts the potential usage of this
trigger. Therefore, instead export the two delay parameters directly.
Also remove unneeded locking and use class_get_devdata().

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

Index: linux-2.6.15/drivers/leds/ledtrig-timer.c
===================================================================
--- linux-2.6.15.orig/drivers/leds/ledtrig-timer.c 2006-02-08 01:42:26.000000000 +0000
+++ linux-2.6.15/drivers/leds/ledtrig-timer.c 2006-02-08 02:03:20.000000000 +0000
@@ -24,8 +24,6 @@
#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;
@@ -38,9 +36,8 @@
unsigned long brightness = LED_OFF;
unsigned long delay = timer_data->delay_off;

- write_lock(&led_cdev->lock);
-
- if (!timer_data->frequency) {
+ if (!timer_data->delay_on || !timer_data->delay_off) {
+ write_lock(&led_cdev->lock);
led_set_brightness(led_cdev, LED_OFF);
write_unlock(&led_cdev->lock);
return;
@@ -51,107 +48,73 @@
delay = timer_data->delay_on;
}

+ write_lock(&led_cdev->lock);
led_set_brightness(led_cdev, brightness);
+ write_unlock(&led_cdev->lock);

mod_timer(&timer_data->timer, jiffies + msecs_to_jiffies(delay));
- write_unlock(&led_cdev->lock);
}

-/* led_cdev write lock needs to be held */
-static int led_timer_setdata(struct led_classdev *led_cdev, unsigned long duty,
- unsigned long frequency)
+static ssize_t led_delay_on_show(struct class_device *dev, char *buf)
{
+ struct led_classdev *led_cdev = class_get_devdata(dev);
struct timer_trig_data *timer_data = led_cdev->trigger_data;

- if (frequency > 500)
- return -EINVAL;
-
- if (duty > 100)
- return -EINVAL;
-
- timer_data->duty = duty;
- timer_data->frequency = frequency;
- if (frequency != 0) {
- timer_data->delay_on = duty * 1000 / 50 / frequency / 2;
- timer_data->delay_off = (100 - duty)*1000 / 50 / frequency / 2;
- }
-
- mod_timer(&timer_data->timer, jiffies + 1);
-
- return 0;
-}
-
-static ssize_t led_duty_show(struct class_device *dev, char *buf)
-{
- struct led_classdev *led_cdev = dev->class_data;
- struct timer_trig_data *timer_data;
-
- read_lock(&led_cdev->lock);
- timer_data = led_cdev->trigger_data;
- sprintf(buf, "%lu\n", timer_data->duty);
- read_unlock(&led_cdev->lock);
+ sprintf(buf, "%lu\n", timer_data->delay_on);

return strlen(buf) + 1;
}

-static ssize_t led_duty_store(struct class_device *dev, const char *buf,
+static ssize_t led_delay_on_store(struct class_device *dev, const char *buf,
size_t size)
{
- struct led_classdev *led_cdev = dev->class_data;
- struct timer_trig_data *timer_data;
+ struct led_classdev *led_cdev = class_get_devdata(dev);
+ struct timer_trig_data *timer_data = led_cdev->trigger_data;
int ret = -EINVAL;
char *after;
-
unsigned long state = simple_strtoul(buf, &after, 10);
+
if (after - buf > 0) {
- write_lock(&led_cdev->lock);
- timer_data = led_cdev->trigger_data;
- ret = led_timer_setdata(led_cdev, state, timer_data->frequency);
- if (!ret)
- ret = after - buf;
- write_unlock(&led_cdev->lock);
+ timer_data->delay_on = state;
+ mod_timer(&timer_data->timer, jiffies + 1);
+ ret = after - buf;
}

return ret;
}

-static ssize_t led_frequency_show(struct class_device *dev, char *buf)
+static ssize_t led_delay_off_show(struct class_device *dev, char *buf)
{
- struct led_classdev *led_cdev = dev->class_data;
- struct timer_trig_data *timer_data;
+ struct led_classdev *led_cdev = class_get_devdata(dev);
+ struct timer_trig_data *timer_data = led_cdev->trigger_data;

- read_lock(&led_cdev->lock);
- timer_data = led_cdev->trigger_data;
- sprintf(buf, "%lu\n", timer_data->frequency);
- read_unlock(&led_cdev->lock);
+ sprintf(buf, "%lu\n", timer_data->delay_off);

return strlen(buf) + 1;
}

-static ssize_t led_frequency_store(struct class_device *dev, const char *buf,
+static ssize_t led_delay_off_store(struct class_device *dev, const char *buf,
size_t size)
{
- struct led_classdev *led_cdev = dev->class_data;
- struct timer_trig_data *timer_data;
+ struct led_classdev *led_cdev = class_get_devdata(dev);
+ struct timer_trig_data *timer_data = led_cdev->trigger_data;
int ret = -EINVAL;
char *after;
unsigned long state = simple_strtoul(buf, &after, 10);

if (after - buf > 0) {
- write_lock(&led_cdev->lock);
- timer_data = led_cdev->trigger_data;
- ret = led_timer_setdata(led_cdev, timer_data->duty, state);
- if (!ret)
- ret = after - buf;
- write_unlock(&led_cdev->lock);
+ timer_data->delay_off = state;
+ mod_timer(&timer_data->timer, jiffies + 1);
+ ret = after - buf;
}

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);
+static CLASS_DEVICE_ATTR(delay_on, 0644, led_delay_on_show,
+ led_delay_on_store);
+static CLASS_DEVICE_ATTR(delay_off, 0644, led_delay_off_show,
+ led_delay_off_store);

static void timer_trig_activate(struct led_classdev *led_cdev)
{
@@ -167,11 +130,10 @@
timer_data->timer.function = led_timer_function;
timer_data->timer.data = (unsigned long) led_cdev;

- timer_data->duty = 50;
-
- class_device_create_file(led_cdev->class_dev, &class_device_attr_duty);
+ class_device_create_file(led_cdev->class_dev,
+ &class_device_attr_delay_on);
class_device_create_file(led_cdev->class_dev,
- &class_device_attr_frequency);
+ &class_device_attr_delay_off);
}

static void timer_trig_deactivate(struct led_classdev *led_cdev)
@@ -180,9 +142,9 @@

if (timer_data) {
class_device_remove_file(led_cdev->class_dev,
- &class_device_attr_duty);
+ &class_device_attr_delay_on);
class_device_remove_file(led_cdev->class_dev,
- &class_device_attr_frequency);
+ &class_device_attr_delay_off);
del_timer_sync(&timer_data->timer);
kfree(timer_data);
}
@@ -199,7 +161,7 @@
return led_trigger_register(&timer_led_trigger);
}

-static void __exit timer_trig_exit (void)
+static void __exit timer_trig_exit(void)
{
led_trigger_unregister(&timer_led_trigger);
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-02-08 03:48    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site