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    [W:0.030 / U:0.296 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site