| Message ID | 9794969ab8a5a6f62d8ac54cdebcaa3caedfc6d6.1539230481.git.baolin.wang@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v15,1/2] leds: core: Introduce LED pattern trigger | expand |
Hi Baolin, On 10/11/2018 06:07 AM, Baolin Wang wrote: > This patch adds a new led trigger that LED device can employ > software or hardware pattern engine. > > Consumers can write 'pattern' file to enable the software pattern > which alters the brightness for the specified duration with one > software timer. > > Moreover consumers can write 'hw_pattern' file to enable the hardware > pattern for some LED controllers which can autonomously control > brightness over time, according to some preprogrammed hardware > patterns. > > Signed-off-by: Raphael Teysseyre <rteysseyre@gmail.com> > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > --- > Changes from v14: > - Improve the commit message and ABI documentation. > - Fix some coding style issues. > - Do not limit the tuple's duration larger than 50 ms, and treat is > as a a step change of brightness. > > Changes from v13: > - Add duration validation for gradual dimming. > - Coding style optimization. > > Changes from v12: > - Add gradual dimming support for software pattern. > > Changes from v11: > - Change -1 means repeat indefinitely. > > Changes from v10: > - Change 'int' to 'u32' for delta_t field. > > Changes from v9: > - None. > > Changes from v8: > - None. > > Changes from v7: > - Move the SC27XX hardware patterns description into its own ABI file. > > Changes from v6: > - Improve commit message. > - Optimize the description of the hw_pattern file. > - Simplify some logics. > > Changes from v5: > - Add one 'hw_pattern' file for hardware patterns. > > Changes from v4: > - Change the repeat file to return the originally written number. > - Improve comments. > - Fix some build warnings. > > Changes from v3: > - Reset pattern number to 0 if user provides incorrect pattern string. > - Support one pattern. > > Changes from v2: > - Remove hardware_pattern boolen. > - Chnage the pattern string format. > > Changes from v1: > - Use ATTRIBUTE_GROUPS() to define attributes. > - Introduce hardware_pattern flag to determine if software pattern > or hardware pattern. > - Re-implement pattern_trig_store_pattern() function. > - Remove pattern_get() interface. > - Improve comments. > - Other small optimization. > --- > .../ABI/testing/sysfs-class-led-trigger-pattern | 82 ++++ > drivers/leds/trigger/Kconfig | 7 + > drivers/leds/trigger/Makefile | 1 + > drivers/leds/trigger/ledtrig-pattern.c | 411 ++++++++++++++++++++ > include/linux/leds.h | 15 + > 5 files changed, 516 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-pattern > create mode 100644 drivers/leds/trigger/ledtrig-pattern.c > > diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern > new file mode 100644 > index 0000000..fb3d1e0 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern > @@ -0,0 +1,82 @@ > +What: /sys/class/leds/<led>/pattern > +Date: September 2018 > +KernelVersion: 4.20 > +Description: > + Specify a software pattern for the LED, that supports altering > + the brightness for the specified duration with one software > + timer. It can do gradual dimming and step change of brightness. > + > + The pattern is given by a series of tuples, of brightness and > + duration (ms). The LED is expected to traverse the series and > + each brightness value for the specified duration. Duration of > + 0 means brightness should immediately change to new value, and > + writing malformed pattern deactivates any active one. > + > + 1. For gradual dimming, the dimming interval now is set as 50 > + milliseconds. So the tuple with duration less than dimming > + interval (50ms) is treated as a step change of brightness, > + i.e. the subsequent brightness will be applied without adding > + intervening dimming intervals. > + > + The gradual dimming format of the software pattern values should be: > + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 > + duration_3 ...". For example: > + > + echo 0 1000 255 2000 > pattern > + > + It will make the LED go gradually from zero-intensity to max (255) > + intensity in 1000 milliseconds, then back to zero intensity in 2000 > + milliseconds: > + > + LED brightness > + ^ > + 255-| / \ / \ / > + | / \ / \ / > + | / \ / \ / > + | / \ / \ / > + 0-| / \/ \/ > + +---0----1----2----3----4----5----6------------> time (s) > + > + 2. To make the LED go instantly from one brigntess value to another, > + we should use use zero-time lengths (the brightness must be same as > + the previous tuple's). So the format should be: > + "brightness_1 duration_1 brightness_1 0 brightness_2 duration_2 > + brightness_2 0 ...". For example: > + > + echo 0 1000 0 0 255 2000 255 0 > pattern > + > + It will make the LED stay off for one second, then stay at max brightness > + for two seconds: > + > + LED brightness > + ^ > + 255-| +---------+ +---------+ > + | | | | | > + | | | | | > + | | | | | > + 0-| -----+ +----+ +---- > + +---0----1----2----3----4----5----6------------> time (s) > + > +What: /sys/class/leds/<led>/hw_pattern > +Date: September 2018 > +KernelVersion: 4.20 > +Description: > + Specify a hardware pattern for the LED, for LED hardware that > + supports autonomously controlling brightness over time, according > + to some preprogrammed hardware patterns. It deactivates any active > + software pattern. > + > + Since different LED hardware can have different semantics of > + hardware patterns, each driver is expected to provide its own > + description for the hardware patterns in their ABI documentation > + file. > + > +What: /sys/class/leds/<led>/repeat > +Date: September 2018 > +KernelVersion: 4.20 > +Description: > + Specify a pattern repeat number. -1 means repeat indefinitely, > + other negative numbers and number 0 are invalid. > + > + This file will always return the originally written repeat > + number. > diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig > index 4018af7..b76fc3c 100644 > --- a/drivers/leds/trigger/Kconfig > +++ b/drivers/leds/trigger/Kconfig > @@ -129,4 +129,11 @@ config LEDS_TRIGGER_NETDEV > This allows LEDs to be controlled by network device activity. > If unsure, say Y. > > +config LEDS_TRIGGER_PATTERN > + tristate "LED Pattern Trigger" > + help > + This allows LEDs to be controlled by a software or hardware pattern > + which is a series of tuples, of brightness and duration (ms). > + If unsure, say N > + > endif # LEDS_TRIGGERS > diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile > index f3cfe19..9bcb64e 100644 > --- a/drivers/leds/trigger/Makefile > +++ b/drivers/leds/trigger/Makefile > @@ -13,3 +13,4 @@ obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o > obj-$(CONFIG_LEDS_TRIGGER_CAMERA) += ledtrig-camera.o > obj-$(CONFIG_LEDS_TRIGGER_PANIC) += ledtrig-panic.o > obj-$(CONFIG_LEDS_TRIGGER_NETDEV) += ledtrig-netdev.o > +obj-$(CONFIG_LEDS_TRIGGER_PATTERN) += ledtrig-pattern.o > diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c > new file mode 100644 > index 0000000..ce7acd1 > --- /dev/null > +++ b/drivers/leds/trigger/ledtrig-pattern.c > @@ -0,0 +1,411 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* > + * LED pattern trigger > + * > + * Idea discussed with Pavel Machek. Raphael Teysseyre implemented > + * the first version, Baolin Wang simplified and improved the approach. > + */ > + > +#include <linux/kernel.h> > +#include <linux/leds.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/slab.h> > +#include <linux/timer.h> > + > +#define MAX_PATTERNS 1024 > +/* > + * When doing gradual dimming, the led brightness will be updated > + * every 50 milliseconds. > + */ > +#define UPDATE_INTERVAL 50 > + > +struct pattern_trig_data { > + struct led_classdev *led_cdev; > + struct led_pattern patterns[MAX_PATTERNS]; > + struct led_pattern *curr; > + struct led_pattern *next; > + struct mutex lock; > + u32 npatterns; > + int repeat; > + int last_repeat; > + int delta_t; > + bool is_indefinite; > + bool is_hw_pattern; > + struct timer_list timer; > +}; > + > +static void pattern_trig_update_patterns(struct pattern_trig_data *data) > +{ > + data->curr = data->next; > + if (!data->is_indefinite && data->curr == data->patterns) > + data->repeat--; > + > + if (data->next == data->patterns + data->npatterns - 1) > + data->next = data->patterns; > + else > + data->next++; > + > + data->delta_t = 0; > +} > + > +static int pattern_trig_compute_brightness(struct pattern_trig_data *data) > +{ > + int step_brightness; > + > + /* > + * If current tuple's duration is less than the dimming interval, > + * we should treat it as a step change of brightness instead of > + * doing gradual dimming. > + */ > + if (data->delta_t == 0 || data->curr->delta_t < UPDATE_INTERVAL) > + return data->curr->brightness; > + > + step_brightness = abs(data->next->brightness - data->curr->brightness); > + step_brightness = data->delta_t * step_brightness / data->curr->delta_t; > + > + if (data->next->brightness > data->curr->brightness) > + return data->curr->brightness + step_brightness; > + else > + return data->curr->brightness - step_brightness; > +} > + > +static void pattern_trig_timer_function(struct timer_list *t) > +{ > + struct pattern_trig_data *data = from_timer(data, t, timer); > + > + mutex_lock(&data->lock); > + > + for (;;) { > + if (!data->is_indefinite && !data->repeat) > + break; > + > + if (data->curr->brightness == data->next->brightness) { > + /* Step change of brightness */ > + led_set_brightness(data->led_cdev, > + data->curr->brightness); > + mod_timer(&data->timer, > + jiffies + msecs_to_jiffies(data->curr->delta_t)); > + > + /* Skip the tuple with zero duration */ > + pattern_trig_update_patterns(data); > + /* Select next tuple */ > + pattern_trig_update_patterns(data); > + } else { > + /* Gradual dimming */ > + > + /* > + * If the accumulation time is larger than current > + * tuple's duration, we should go next one and re-check > + * if we repeated done. > + */ > + if (data->delta_t > data->curr->delta_t) { > + pattern_trig_update_patterns(data); > + continue; > + } > + > + led_set_brightness(data->led_cdev, > + pattern_trig_compute_brightness(data)); > + mod_timer(&data->timer, > + jiffies + msecs_to_jiffies(UPDATE_INTERVAL)); > + > + /* Accumulate the gradual dimming time */ > + data->delta_t += UPDATE_INTERVAL; > + } > + > + break; > + } > + > + mutex_unlock(&data->lock); > +} > + > +static int pattern_trig_start_pattern(struct led_classdev *led_cdev) > +{ > + struct pattern_trig_data *data = led_cdev->trigger_data; > + > + if (!data->npatterns) > + return 0; > + > + if (data->is_hw_pattern) { > + return led_cdev->pattern_set(led_cdev, data->patterns, > + data->npatterns, data->repeat); > + } > + > + /* At least 2 tuples for software pattern. */ > + if (data->npatterns < 2) > + return -EINVAL; > + > + data->delta_t = 0; > + data->curr = data->patterns; > + data->next = data->patterns + 1; > + data->timer.expires = jiffies; > + add_timer(&data->timer); > + > + return 0; > +} > + > +static ssize_t repeat_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + struct pattern_trig_data *data = led_cdev->trigger_data; > + int repeat; > + > + mutex_lock(&data->lock); > + > + repeat = data->last_repeat; > + > + mutex_unlock(&data->lock); > + > + return scnprintf(buf, PAGE_SIZE, "%d\n", repeat); > +} > + > +static ssize_t repeat_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + struct pattern_trig_data *data = led_cdev->trigger_data; > + int err, res; > + > + err = kstrtos32(buf, 10, &res); > + if (err) > + return err; > + > + /* Number 0 and negative numbers except -1 are invalid. */ > + if (res < -1 || res == 0) > + return -EINVAL; > + > + /* > + * Clear previous patterns' performence firstly, and remove the timer > + * without mutex lock to avoid dead lock. > + */ > + del_timer_sync(&data->timer); > + > + mutex_lock(&data->lock); > + > + if (data->is_hw_pattern) > + led_cdev->pattern_clear(led_cdev); > + > + data->last_repeat = data->repeat = res; > + /* -1 means repeat indefinitely */ > + if (data->repeat == -1) > + data->is_indefinite = true; > + else > + data->is_indefinite = false; > + > + err = pattern_trig_start_pattern(led_cdev); > + > + mutex_unlock(&data->lock); > + return err < 0 ? err : count; > +} > + > +static DEVICE_ATTR_RW(repeat); > + > +static ssize_t pattern_trig_show_patterns(struct pattern_trig_data *data, > + char *buf, bool hw_pattern) > +{ > + ssize_t count = 0; > + int i; > + > + mutex_lock(&data->lock); > + > + if (!data->npatterns || (data->is_hw_pattern ^ hw_pattern)) > + goto out; > + > + for (i = 0; i < data->npatterns; i++) { > + count += scnprintf(buf + count, PAGE_SIZE - count, > + "%d %u ", > + data->patterns[i].brightness, > + data->patterns[i].delta_t); > + } > + > + buf[count - 1] = '\n'; > + > +out: > + mutex_unlock(&data->lock); > + return count; > +} > + > +static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev, > + const char *buf, size_t count, > + bool hw_pattern) > +{ > + struct pattern_trig_data *data = led_cdev->trigger_data; > + int ccount, cr, offset = 0, err = 0; > + > + /* > + * Clear previous patterns' performence firstly, and remove the timer > + * without mutex lock to avoid dead lock. > + */ > + del_timer_sync(&data->timer); > + > + mutex_lock(&data->lock); > + > + if (data->is_hw_pattern) > + led_cdev->pattern_clear(led_cdev); > + > + data->is_hw_pattern = hw_pattern; > + data->npatterns = 0; > + > + while (offset < count - 1 && data->npatterns < MAX_PATTERNS) { > + cr = 0; > + ccount = sscanf(buf + offset, "%d %u %n", > + &data->patterns[data->npatterns].brightness, > + &data->patterns[data->npatterns].delta_t, &cr); > + if (ccount != 2) { > + data->npatterns = 0; > + err = -EINVAL; > + goto out; > + } > + > + offset += cr; > + data->npatterns++; > + } > + > + err = pattern_trig_start_pattern(led_cdev); > + if (err) > + data->npatterns = 0; > + > +out: > + mutex_unlock(&data->lock); > + return err < 0 ? err : count; > +} > + > +static ssize_t pattern_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + struct pattern_trig_data *data = led_cdev->trigger_data; > + > + return pattern_trig_show_patterns(data, buf, false); > +} > + > +static ssize_t pattern_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + > + return pattern_trig_store_patterns(led_cdev, buf, count, false); > +} > + > +static DEVICE_ATTR_RW(pattern); > + > +static ssize_t hw_pattern_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + struct pattern_trig_data *data = led_cdev->trigger_data; > + > + return pattern_trig_show_patterns(data, buf, true); > +} > + > +static ssize_t hw_pattern_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + > + return pattern_trig_store_patterns(led_cdev, buf, count, true); > +} > + > +static DEVICE_ATTR_RW(hw_pattern); > + > +static umode_t pattern_trig_attrs_mode(struct kobject *kobj, > + struct attribute *attr, int index) > +{ > + struct device *dev = container_of(kobj, struct device, kobj); > + struct led_classdev *led_cdev = dev_get_drvdata(dev); > + > + if (attr == &dev_attr_repeat.attr || attr == &dev_attr_pattern.attr) > + return attr->mode; > + else if (attr == &dev_attr_hw_pattern.attr && led_cdev->pattern_set) > + return attr->mode; > + > + return 0; > +} > + > +static struct attribute *pattern_trig_attrs[] = { > + &dev_attr_pattern.attr, > + &dev_attr_hw_pattern.attr, > + &dev_attr_repeat.attr, > + NULL > +}; > + > +static const struct attribute_group pattern_trig_group = { > + .attrs = pattern_trig_attrs, > + .is_visible = pattern_trig_attrs_mode, > +}; > + > +static const struct attribute_group *pattern_trig_groups[] = { > + &pattern_trig_group, > + NULL, > +}; > + > +static int pattern_trig_activate(struct led_classdev *led_cdev) > +{ > + struct pattern_trig_data *data; > + > + data = kzalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + if (!!led_cdev->pattern_set ^ !!led_cdev->pattern_clear) { > + dev_warn(led_cdev->dev, > + "Hardware pattern ops validation failed\n"); > + led_cdev->pattern_set = NULL; > + led_cdev->pattern_clear = NULL; > + } > + > + data->is_indefinite = true; > + data->last_repeat = -1; > + mutex_init(&data->lock); > + data->led_cdev = led_cdev; > + led_set_trigger_data(led_cdev, data); > + timer_setup(&data->timer, pattern_trig_timer_function, 0); > + led_cdev->activated = true; > + > + return 0; > +} > + > +static void pattern_trig_deactivate(struct led_classdev *led_cdev) > +{ > + struct pattern_trig_data *data = led_cdev->trigger_data; > + > + if (!led_cdev->activated) > + return; > + > + if (led_cdev->pattern_clear) > + led_cdev->pattern_clear(led_cdev); > + > + del_timer_sync(&data->timer); > + > + led_set_brightness(led_cdev, LED_OFF); > + kfree(data); > + led_cdev->activated = false; > +} > + > +static struct led_trigger pattern_led_trigger = { > + .name = "pattern", > + .activate = pattern_trig_activate, > + .deactivate = pattern_trig_deactivate, > + .groups = pattern_trig_groups, > +}; > + > +static int __init pattern_trig_init(void) > +{ > + return led_trigger_register(&pattern_led_trigger); > +} > + > +static void __exit pattern_trig_exit(void) > +{ > + led_trigger_unregister(&pattern_led_trigger); > +} > + > +module_init(pattern_trig_init); > +module_exit(pattern_trig_exit); > + > +MODULE_AUTHOR("Raphael Teysseyre <rteysseyre@gmail.com"); > +MODULE_AUTHOR("Baolin Wang <baolin.wang@linaro.org"); > +MODULE_DESCRIPTION("LED Pattern trigger"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/leds.h b/include/linux/leds.h > index 834683d..7393a31 100644 > --- a/include/linux/leds.h > +++ b/include/linux/leds.h > @@ -22,6 +22,7 @@ > #include <linux/workqueue.h> > > struct device; > +struct led_pattern; > /* > * LED Core > */ > @@ -88,6 +89,10 @@ struct led_classdev { > unsigned long *delay_on, > unsigned long *delay_off); > > + int (*pattern_set)(struct led_classdev *led_cdev, > + struct led_pattern *pattern, u32 len, int repeat); > + int (*pattern_clear)(struct led_classdev *led_cdev); > + > struct device *dev; > const struct attribute_group **groups; > > @@ -472,4 +477,14 @@ static inline void led_classdev_notify_brightness_hw_changed( > struct led_classdev *led_cdev, enum led_brightness brightness) { } > #endif > > +/** > + * struct led_pattern - pattern interval settings > + * @delta_t: pattern interval delay, in milliseconds > + * @brightness: pattern interval brightness > + */ > +struct led_pattern { > + u32 delta_t; > + int brightness; > +}; > + > #endif /* __LINUX_LEDS_H_INCLUDED */ > OK, let's abide by constant update interval for now. Thank you for your work on this patch set throughout all these months. We will have -rc8, so one week of testing before sending upstream should be enough. Patch set applied to the for-next branch of linux-leds.git. -- Best regards, Jacek Anaszewski
Hi, On 12 October 2018 at 04:07, Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > Hi Baolin, > > On 10/11/2018 06:07 AM, Baolin Wang wrote: >> This patch adds a new led trigger that LED device can employ >> software or hardware pattern engine. >> >> Consumers can write 'pattern' file to enable the software pattern >> which alters the brightness for the specified duration with one >> software timer. >> >> Moreover consumers can write 'hw_pattern' file to enable the hardware >> pattern for some LED controllers which can autonomously control >> brightness over time, according to some preprogrammed hardware >> patterns. >> >> Signed-off-by: Raphael Teysseyre <rteysseyre@gmail.com> >> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> >> --- >> Changes from v14: >> - Improve the commit message and ABI documentation. >> - Fix some coding style issues. >> - Do not limit the tuple's duration larger than 50 ms, and treat is >> as a a step change of brightness. >> >> Changes from v13: >> - Add duration validation for gradual dimming. >> - Coding style optimization. >> >> Changes from v12: >> - Add gradual dimming support for software pattern. >> >> Changes from v11: >> - Change -1 means repeat indefinitely. >> >> Changes from v10: >> - Change 'int' to 'u32' for delta_t field. >> >> Changes from v9: >> - None. >> >> Changes from v8: >> - None. >> >> Changes from v7: >> - Move the SC27XX hardware patterns description into its own ABI file. >> >> Changes from v6: >> - Improve commit message. >> - Optimize the description of the hw_pattern file. >> - Simplify some logics. >> >> Changes from v5: >> - Add one 'hw_pattern' file for hardware patterns. >> >> Changes from v4: >> - Change the repeat file to return the originally written number. >> - Improve comments. >> - Fix some build warnings. >> >> Changes from v3: >> - Reset pattern number to 0 if user provides incorrect pattern string. >> - Support one pattern. >> >> Changes from v2: >> - Remove hardware_pattern boolen. >> - Chnage the pattern string format. >> >> Changes from v1: >> - Use ATTRIBUTE_GROUPS() to define attributes. >> - Introduce hardware_pattern flag to determine if software pattern >> or hardware pattern. >> - Re-implement pattern_trig_store_pattern() function. >> - Remove pattern_get() interface. >> - Improve comments. >> - Other small optimization. >> --- >> .../ABI/testing/sysfs-class-led-trigger-pattern | 82 ++++ >> drivers/leds/trigger/Kconfig | 7 + >> drivers/leds/trigger/Makefile | 1 + >> drivers/leds/trigger/ledtrig-pattern.c | 411 ++++++++++++++++++++ >> include/linux/leds.h | 15 + >> 5 files changed, 516 insertions(+) >> create mode 100644 Documentation/ABI/testing/sysfs-class-led-trigger-pattern >> create mode 100644 drivers/leds/trigger/ledtrig-pattern.c >> >> diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern >> new file mode 100644 >> index 0000000..fb3d1e0 >> --- /dev/null >> +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern >> @@ -0,0 +1,82 @@ >> +What: /sys/class/leds/<led>/pattern >> +Date: September 2018 >> +KernelVersion: 4.20 >> +Description: >> + Specify a software pattern for the LED, that supports altering >> + the brightness for the specified duration with one software >> + timer. It can do gradual dimming and step change of brightness. >> + >> + The pattern is given by a series of tuples, of brightness and >> + duration (ms). The LED is expected to traverse the series and >> + each brightness value for the specified duration. Duration of >> + 0 means brightness should immediately change to new value, and >> + writing malformed pattern deactivates any active one. >> + >> + 1. For gradual dimming, the dimming interval now is set as 50 >> + milliseconds. So the tuple with duration less than dimming >> + interval (50ms) is treated as a step change of brightness, >> + i.e. the subsequent brightness will be applied without adding >> + intervening dimming intervals. >> + >> + The gradual dimming format of the software pattern values should be: >> + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 >> + duration_3 ...". For example: >> + >> + echo 0 1000 255 2000 > pattern >> + >> + It will make the LED go gradually from zero-intensity to max (255) >> + intensity in 1000 milliseconds, then back to zero intensity in 2000 >> + milliseconds: >> + >> + LED brightness >> + ^ >> + 255-| / \ / \ / >> + | / \ / \ / >> + | / \ / \ / >> + | / \ / \ / >> + 0-| / \/ \/ >> + +---0----1----2----3----4----5----6------------> time (s) >> + >> + 2. To make the LED go instantly from one brigntess value to another, >> + we should use use zero-time lengths (the brightness must be same as >> + the previous tuple's). So the format should be: >> + "brightness_1 duration_1 brightness_1 0 brightness_2 duration_2 >> + brightness_2 0 ...". For example: >> + >> + echo 0 1000 0 0 255 2000 255 0 > pattern >> + >> + It will make the LED stay off for one second, then stay at max brightness >> + for two seconds: >> + >> + LED brightness >> + ^ >> + 255-| +---------+ +---------+ >> + | | | | | >> + | | | | | >> + | | | | | >> + 0-| -----+ +----+ +---- >> + +---0----1----2----3----4----5----6------------> time (s) >> + >> +What: /sys/class/leds/<led>/hw_pattern >> +Date: September 2018 >> +KernelVersion: 4.20 >> +Description: >> + Specify a hardware pattern for the LED, for LED hardware that >> + supports autonomously controlling brightness over time, according >> + to some preprogrammed hardware patterns. It deactivates any active >> + software pattern. >> + >> + Since different LED hardware can have different semantics of >> + hardware patterns, each driver is expected to provide its own >> + description for the hardware patterns in their ABI documentation >> + file. >> + >> +What: /sys/class/leds/<led>/repeat >> +Date: September 2018 >> +KernelVersion: 4.20 >> +Description: >> + Specify a pattern repeat number. -1 means repeat indefinitely, >> + other negative numbers and number 0 are invalid. >> + >> + This file will always return the originally written repeat >> + number. >> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig >> index 4018af7..b76fc3c 100644 >> --- a/drivers/leds/trigger/Kconfig >> +++ b/drivers/leds/trigger/Kconfig >> @@ -129,4 +129,11 @@ config LEDS_TRIGGER_NETDEV >> This allows LEDs to be controlled by network device activity. >> If unsure, say Y. >> >> +config LEDS_TRIGGER_PATTERN >> + tristate "LED Pattern Trigger" >> + help >> + This allows LEDs to be controlled by a software or hardware pattern >> + which is a series of tuples, of brightness and duration (ms). >> + If unsure, say N >> + >> endif # LEDS_TRIGGERS >> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile >> index f3cfe19..9bcb64e 100644 >> --- a/drivers/leds/trigger/Makefile >> +++ b/drivers/leds/trigger/Makefile >> @@ -13,3 +13,4 @@ obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o >> obj-$(CONFIG_LEDS_TRIGGER_CAMERA) += ledtrig-camera.o >> obj-$(CONFIG_LEDS_TRIGGER_PANIC) += ledtrig-panic.o >> obj-$(CONFIG_LEDS_TRIGGER_NETDEV) += ledtrig-netdev.o >> +obj-$(CONFIG_LEDS_TRIGGER_PATTERN) += ledtrig-pattern.o >> diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c >> new file mode 100644 >> index 0000000..ce7acd1 >> --- /dev/null >> +++ b/drivers/leds/trigger/ledtrig-pattern.c >> @@ -0,0 +1,411 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> + >> +/* >> + * LED pattern trigger >> + * >> + * Idea discussed with Pavel Machek. Raphael Teysseyre implemented >> + * the first version, Baolin Wang simplified and improved the approach. >> + */ >> + >> +#include <linux/kernel.h> >> +#include <linux/leds.h> >> +#include <linux/module.h> >> +#include <linux/mutex.h> >> +#include <linux/slab.h> >> +#include <linux/timer.h> >> + >> +#define MAX_PATTERNS 1024 >> +/* >> + * When doing gradual dimming, the led brightness will be updated >> + * every 50 milliseconds. >> + */ >> +#define UPDATE_INTERVAL 50 >> + >> +struct pattern_trig_data { >> + struct led_classdev *led_cdev; >> + struct led_pattern patterns[MAX_PATTERNS]; >> + struct led_pattern *curr; >> + struct led_pattern *next; >> + struct mutex lock; >> + u32 npatterns; >> + int repeat; >> + int last_repeat; >> + int delta_t; >> + bool is_indefinite; >> + bool is_hw_pattern; >> + struct timer_list timer; >> +}; >> + >> +static void pattern_trig_update_patterns(struct pattern_trig_data *data) >> +{ >> + data->curr = data->next; >> + if (!data->is_indefinite && data->curr == data->patterns) >> + data->repeat--; >> + >> + if (data->next == data->patterns + data->npatterns - 1) >> + data->next = data->patterns; >> + else >> + data->next++; >> + >> + data->delta_t = 0; >> +} >> + >> +static int pattern_trig_compute_brightness(struct pattern_trig_data *data) >> +{ >> + int step_brightness; >> + >> + /* >> + * If current tuple's duration is less than the dimming interval, >> + * we should treat it as a step change of brightness instead of >> + * doing gradual dimming. >> + */ >> + if (data->delta_t == 0 || data->curr->delta_t < UPDATE_INTERVAL) >> + return data->curr->brightness; >> + >> + step_brightness = abs(data->next->brightness - data->curr->brightness); >> + step_brightness = data->delta_t * step_brightness / data->curr->delta_t; >> + >> + if (data->next->brightness > data->curr->brightness) >> + return data->curr->brightness + step_brightness; >> + else >> + return data->curr->brightness - step_brightness; >> +} >> + >> +static void pattern_trig_timer_function(struct timer_list *t) >> +{ >> + struct pattern_trig_data *data = from_timer(data, t, timer); >> + >> + mutex_lock(&data->lock); >> + >> + for (;;) { >> + if (!data->is_indefinite && !data->repeat) >> + break; >> + >> + if (data->curr->brightness == data->next->brightness) { >> + /* Step change of brightness */ >> + led_set_brightness(data->led_cdev, >> + data->curr->brightness); >> + mod_timer(&data->timer, >> + jiffies + msecs_to_jiffies(data->curr->delta_t)); >> + >> + /* Skip the tuple with zero duration */ >> + pattern_trig_update_patterns(data); >> + /* Select next tuple */ >> + pattern_trig_update_patterns(data); >> + } else { >> + /* Gradual dimming */ >> + >> + /* >> + * If the accumulation time is larger than current >> + * tuple's duration, we should go next one and re-check >> + * if we repeated done. >> + */ >> + if (data->delta_t > data->curr->delta_t) { >> + pattern_trig_update_patterns(data); >> + continue; >> + } >> + >> + led_set_brightness(data->led_cdev, >> + pattern_trig_compute_brightness(data)); >> + mod_timer(&data->timer, >> + jiffies + msecs_to_jiffies(UPDATE_INTERVAL)); >> + >> + /* Accumulate the gradual dimming time */ >> + data->delta_t += UPDATE_INTERVAL; >> + } >> + >> + break; >> + } >> + >> + mutex_unlock(&data->lock); >> +} >> + >> +static int pattern_trig_start_pattern(struct led_classdev *led_cdev) >> +{ >> + struct pattern_trig_data *data = led_cdev->trigger_data; >> + >> + if (!data->npatterns) >> + return 0; >> + >> + if (data->is_hw_pattern) { >> + return led_cdev->pattern_set(led_cdev, data->patterns, >> + data->npatterns, data->repeat); >> + } >> + >> + /* At least 2 tuples for software pattern. */ >> + if (data->npatterns < 2) >> + return -EINVAL; >> + >> + data->delta_t = 0; >> + data->curr = data->patterns; >> + data->next = data->patterns + 1; >> + data->timer.expires = jiffies; >> + add_timer(&data->timer); >> + >> + return 0; >> +} >> + >> +static ssize_t repeat_show(struct device *dev, struct device_attribute *attr, >> + char *buf) >> +{ >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> + struct pattern_trig_data *data = led_cdev->trigger_data; >> + int repeat; >> + >> + mutex_lock(&data->lock); >> + >> + repeat = data->last_repeat; >> + >> + mutex_unlock(&data->lock); >> + >> + return scnprintf(buf, PAGE_SIZE, "%d\n", repeat); >> +} >> + >> +static ssize_t repeat_store(struct device *dev, struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> + struct pattern_trig_data *data = led_cdev->trigger_data; >> + int err, res; >> + >> + err = kstrtos32(buf, 10, &res); >> + if (err) >> + return err; >> + >> + /* Number 0 and negative numbers except -1 are invalid. */ >> + if (res < -1 || res == 0) >> + return -EINVAL; >> + >> + /* >> + * Clear previous patterns' performence firstly, and remove the timer >> + * without mutex lock to avoid dead lock. >> + */ >> + del_timer_sync(&data->timer); >> + >> + mutex_lock(&data->lock); >> + >> + if (data->is_hw_pattern) >> + led_cdev->pattern_clear(led_cdev); >> + >> + data->last_repeat = data->repeat = res; >> + /* -1 means repeat indefinitely */ >> + if (data->repeat == -1) >> + data->is_indefinite = true; >> + else >> + data->is_indefinite = false; >> + >> + err = pattern_trig_start_pattern(led_cdev); >> + >> + mutex_unlock(&data->lock); >> + return err < 0 ? err : count; >> +} >> + >> +static DEVICE_ATTR_RW(repeat); >> + >> +static ssize_t pattern_trig_show_patterns(struct pattern_trig_data *data, >> + char *buf, bool hw_pattern) >> +{ >> + ssize_t count = 0; >> + int i; >> + >> + mutex_lock(&data->lock); >> + >> + if (!data->npatterns || (data->is_hw_pattern ^ hw_pattern)) >> + goto out; >> + >> + for (i = 0; i < data->npatterns; i++) { >> + count += scnprintf(buf + count, PAGE_SIZE - count, >> + "%d %u ", >> + data->patterns[i].brightness, >> + data->patterns[i].delta_t); >> + } >> + >> + buf[count - 1] = '\n'; >> + >> +out: >> + mutex_unlock(&data->lock); >> + return count; >> +} >> + >> +static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev, >> + const char *buf, size_t count, >> + bool hw_pattern) >> +{ >> + struct pattern_trig_data *data = led_cdev->trigger_data; >> + int ccount, cr, offset = 0, err = 0; >> + >> + /* >> + * Clear previous patterns' performence firstly, and remove the timer >> + * without mutex lock to avoid dead lock. >> + */ >> + del_timer_sync(&data->timer); >> + >> + mutex_lock(&data->lock); >> + >> + if (data->is_hw_pattern) >> + led_cdev->pattern_clear(led_cdev); >> + >> + data->is_hw_pattern = hw_pattern; >> + data->npatterns = 0; >> + >> + while (offset < count - 1 && data->npatterns < MAX_PATTERNS) { >> + cr = 0; >> + ccount = sscanf(buf + offset, "%d %u %n", >> + &data->patterns[data->npatterns].brightness, >> + &data->patterns[data->npatterns].delta_t, &cr); >> + if (ccount != 2) { >> + data->npatterns = 0; >> + err = -EINVAL; >> + goto out; >> + } >> + >> + offset += cr; >> + data->npatterns++; >> + } >> + >> + err = pattern_trig_start_pattern(led_cdev); >> + if (err) >> + data->npatterns = 0; >> + >> +out: >> + mutex_unlock(&data->lock); >> + return err < 0 ? err : count; >> +} >> + >> +static ssize_t pattern_show(struct device *dev, struct device_attribute *attr, >> + char *buf) >> +{ >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> + struct pattern_trig_data *data = led_cdev->trigger_data; >> + >> + return pattern_trig_show_patterns(data, buf, false); >> +} >> + >> +static ssize_t pattern_store(struct device *dev, struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> + >> + return pattern_trig_store_patterns(led_cdev, buf, count, false); >> +} >> + >> +static DEVICE_ATTR_RW(pattern); >> + >> +static ssize_t hw_pattern_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> + struct pattern_trig_data *data = led_cdev->trigger_data; >> + >> + return pattern_trig_show_patterns(data, buf, true); >> +} >> + >> +static ssize_t hw_pattern_store(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, size_t count) >> +{ >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> + >> + return pattern_trig_store_patterns(led_cdev, buf, count, true); >> +} >> + >> +static DEVICE_ATTR_RW(hw_pattern); >> + >> +static umode_t pattern_trig_attrs_mode(struct kobject *kobj, >> + struct attribute *attr, int index) >> +{ >> + struct device *dev = container_of(kobj, struct device, kobj); >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> + >> + if (attr == &dev_attr_repeat.attr || attr == &dev_attr_pattern.attr) >> + return attr->mode; >> + else if (attr == &dev_attr_hw_pattern.attr && led_cdev->pattern_set) >> + return attr->mode; >> + >> + return 0; >> +} >> + >> +static struct attribute *pattern_trig_attrs[] = { >> + &dev_attr_pattern.attr, >> + &dev_attr_hw_pattern.attr, >> + &dev_attr_repeat.attr, >> + NULL >> +}; >> + >> +static const struct attribute_group pattern_trig_group = { >> + .attrs = pattern_trig_attrs, >> + .is_visible = pattern_trig_attrs_mode, >> +}; >> + >> +static const struct attribute_group *pattern_trig_groups[] = { >> + &pattern_trig_group, >> + NULL, >> +}; >> + >> +static int pattern_trig_activate(struct led_classdev *led_cdev) >> +{ >> + struct pattern_trig_data *data; >> + >> + data = kzalloc(sizeof(*data), GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + if (!!led_cdev->pattern_set ^ !!led_cdev->pattern_clear) { >> + dev_warn(led_cdev->dev, >> + "Hardware pattern ops validation failed\n"); >> + led_cdev->pattern_set = NULL; >> + led_cdev->pattern_clear = NULL; >> + } >> + >> + data->is_indefinite = true; >> + data->last_repeat = -1; >> + mutex_init(&data->lock); >> + data->led_cdev = led_cdev; >> + led_set_trigger_data(led_cdev, data); >> + timer_setup(&data->timer, pattern_trig_timer_function, 0); >> + led_cdev->activated = true; >> + >> + return 0; >> +} >> + >> +static void pattern_trig_deactivate(struct led_classdev *led_cdev) >> +{ >> + struct pattern_trig_data *data = led_cdev->trigger_data; >> + >> + if (!led_cdev->activated) >> + return; >> + >> + if (led_cdev->pattern_clear) >> + led_cdev->pattern_clear(led_cdev); >> + >> + del_timer_sync(&data->timer); >> + >> + led_set_brightness(led_cdev, LED_OFF); >> + kfree(data); >> + led_cdev->activated = false; >> +} >> + >> +static struct led_trigger pattern_led_trigger = { >> + .name = "pattern", >> + .activate = pattern_trig_activate, >> + .deactivate = pattern_trig_deactivate, >> + .groups = pattern_trig_groups, >> +}; >> + >> +static int __init pattern_trig_init(void) >> +{ >> + return led_trigger_register(&pattern_led_trigger); >> +} >> + >> +static void __exit pattern_trig_exit(void) >> +{ >> + led_trigger_unregister(&pattern_led_trigger); >> +} >> + >> +module_init(pattern_trig_init); >> +module_exit(pattern_trig_exit); >> + >> +MODULE_AUTHOR("Raphael Teysseyre <rteysseyre@gmail.com"); >> +MODULE_AUTHOR("Baolin Wang <baolin.wang@linaro.org"); >> +MODULE_DESCRIPTION("LED Pattern trigger"); >> +MODULE_LICENSE("GPL v2"); >> diff --git a/include/linux/leds.h b/include/linux/leds.h >> index 834683d..7393a31 100644 >> --- a/include/linux/leds.h >> +++ b/include/linux/leds.h >> @@ -22,6 +22,7 @@ >> #include <linux/workqueue.h> >> >> struct device; >> +struct led_pattern; >> /* >> * LED Core >> */ >> @@ -88,6 +89,10 @@ struct led_classdev { >> unsigned long *delay_on, >> unsigned long *delay_off); >> >> + int (*pattern_set)(struct led_classdev *led_cdev, >> + struct led_pattern *pattern, u32 len, int repeat); >> + int (*pattern_clear)(struct led_classdev *led_cdev); >> + >> struct device *dev; >> const struct attribute_group **groups; >> >> @@ -472,4 +477,14 @@ static inline void led_classdev_notify_brightness_hw_changed( >> struct led_classdev *led_cdev, enum led_brightness brightness) { } >> #endif >> >> +/** >> + * struct led_pattern - pattern interval settings >> + * @delta_t: pattern interval delay, in milliseconds >> + * @brightness: pattern interval brightness >> + */ >> +struct led_pattern { >> + u32 delta_t; >> + int brightness; >> +}; >> + >> #endif /* __LINUX_LEDS_H_INCLUDED */ >> > > OK, let's abide by constant update interval for now. > > Thank you for your work on this patch set throughout > all these months. We will have -rc8, so one week of testing > before sending upstream should be enough. > > Patch set applied to the for-next branch of linux-leds.git. Thanks Jacek and Pavel, you are so patient and always give me lots of help there. Glad to co-work with you, thanks again. -- Baolin Wang Best Regards
Hi! > > + > > + The gradual dimming format of the software pattern values should be: > > + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 > > + duration_3 ...". For example: > > + > > + echo 0 1000 255 2000 > pattern > > + > > + It will make the LED go gradually from zero-intensity to max (255) > > + intensity in 1000 milliseconds, then back to zero intensity in 2000 > > + milliseconds: > > + > > + LED brightness > > + ^ > > + 255-| / \ / \ / > > + | / \ / \ / > > + | / \ / \ / > > + | / \ / \ / > > + 0-| / \/ \/ > > + +---0----1----2----3----4----5----6------------> time (s) > > + Ok, so I got around to testing this. echo "0 1000 10 2550 0 1000 0 100" > pattern makes expected pattern [ .-xXx-. ]. But when I do echo "0 1000 10 2550 0 1000" > pattern I only get expected pattern on the first iteration, then I get [ Xx-. ]. I'm using led-controller:flash on nokia n900 (so everything in software). > OK, let's abide by constant update interval for now. > > Thank you for your work on this patch set throughout > all these months. We will have -rc8, so one week of testing > before sending upstream should be enough. > > Patch set applied to the for-next branch of linux-leds.git. Thanks! Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi! > > OK, let's abide by constant update interval for now. > > > > Thank you for your work on this patch set throughout > > all these months. We will have -rc8, so one week of testing > > before sending upstream should be enough. > > > > Patch set applied to the for-next branch of linux-leds.git. > > Thanks Jacek and Pavel, you are so patient and always give me lots of > help there. Glad to co-work with you, thanks again. Thanks for patience, too. I'm pretty sure pattern trigger will be useful in future. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On 10/24/2018 10:31 AM, Pavel Machek wrote: > Hi! > >>> + >>> + The gradual dimming format of the software pattern values should be: >>> + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 >>> + duration_3 ...". For example: >>> + >>> + echo 0 1000 255 2000 > pattern >>> + >>> + It will make the LED go gradually from zero-intensity to max (255) >>> + intensity in 1000 milliseconds, then back to zero intensity in 2000 >>> + milliseconds: >>> + >>> + LED brightness >>> + ^ >>> + 255-| / \ / \ / >>> + | / \ / \ / >>> + | / \ / \ / >>> + | / \ / \ / >>> + 0-| / \/ \/ >>> + +---0----1----2----3----4----5----6------------> time (s) >>> + > > Ok, so I got around to testing this. > > echo "0 1000 10 2550 0 1000 0 100" > pattern > > makes expected pattern [ .-xXx-. ]. > > But when I do > > echo "0 1000 10 2550 0 1000" > pattern > > I only get expected pattern on the first iteration, then I get > [ Xx-. ]. This is because the tuples are processed in a loop, without discerning between start and end of the sequence. So this sequence ends up being squashed, because of the comparison: if (data->curr->brightness == data->next->brightness) { //step change od brightness ... /* Skip the tuple with zero duration */ Here we actually only assume that this is zero duration since it is not checked. Possibly needs fixing. pattern_trig_update_patterns(data); /* Select next tuple */ pattern_trig_update_patterns(data); In effect we have: 0 -> 10 // dimming 10 -> 0 // dimming 0 skipped // step 10 -> 0 // dimming 0 skipped // step 10 -> 0 // dimming ... In order to get "rise - fall - rise - fall" sequence you need only two tuples: echo "0 1000 10 1000" > pattern which will be processed as follows: 0 -> 10 // dimming 10 -> 0 // dimming 0 -> 10 // dimming 10 -> 0 // dimming 0 -> 10 // dimming 10 -> 0 // dimming > I'm using led-controller:flash on nokia n900 (so everything in > software). > >> OK, let's abide by constant update interval for now. >> >> Thank you for your work on this patch set throughout >> all these months. We will have -rc8, so one week of testing >> before sending upstream should be enough. >> >> Patch set applied to the for-next branch of linux-leds.git. > > Thanks! > Pavel > -- Best regards, Jacek Anaszewski
Hi Pavel. Thank you for the patch. On 10/24/2018 10:44 PM, Pavel Machek wrote: > > Check for zero duration before skipping step. This fixes pattern > > echo "0 1000 10 2550 0 1000" > pattern > > which should do [ .-xXx-.] but does [ Xx-.] > > Signed-off-by: Pavel Machek <pavel@ucw.cz> > Suggested-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> > > > diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c > index ce7acd1..bc5f495 100644 > --- a/drivers/leds/trigger/ledtrig-pattern.c > +++ b/drivers/leds/trigger/ledtrig-pattern.c > @@ -87,9 +87,10 @@ static void pattern_trig_timer_function(struct timer_list *t) > data->curr->brightness); > mod_timer(&data->timer, > jiffies + msecs_to_jiffies(data->curr->delta_t)); > - > - /* Skip the tuple with zero duration */ > - pattern_trig_update_patterns(data); > + if (!data->next->delta_t) { > + /* Skip the tuple with zero duration */ > + pattern_trig_update_patterns(data); > + } > /* Select next tuple */ > pattern_trig_update_patterns(data); > } else { > Yep, it fixes the issue. Applied to the fixes-for-4.20-rc2 branch of linux-leds.git -- Best regards, Jacek Anaszewski
Hi Pavel, On 26 October 2018 at 05:24, Pavel Machek <pavel@ucw.cz> wrote: > > We don't want brightness < 0, but this may not be best way to do > this. We also don't want brightness > max_brightness, but I'm not sure > this check is effective. I think this is reasonable, but we can simplify the patch as: diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c index ce7acd1..3a4ecb9 100644 --- a/drivers/leds/trigger/ledtrig-pattern.c +++ b/drivers/leds/trigger/ledtrig-pattern.c @@ -250,10 +250,11 @@ static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev, while (offset < count - 1 && data->npatterns < MAX_PATTERNS) { cr = 0; - ccount = sscanf(buf + offset, "%d %u %n", + ccount = sscanf(buf + offset, "%u %u %n", &data->patterns[data->npatterns].brightness, &data->patterns[data->npatterns].delta_t, &cr); - if (ccount != 2) { + if (ccount != 2 || + data->patterns[data->npatterns].brightness > data->led_cdev->max_brightness) { data->npatterns = 0; err = -EINVAL; goto out; > > We probably also don't want pattern where all the delta_t s are zero. Yes. Maybe add one function to valid the patterns. > > I came up with this so far.... > Pavel > > > > diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c > index ce7acd1..bc5f495 100644 > --- a/drivers/leds/trigger/ledtrig-pattern.c > +++ b/drivers/leds/trigger/ledtrig-pattern.c > @@ -250,9 +251,16 @@ static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev, > > while (offset < count - 1 && data->npatterns < MAX_PATTERNS) { > cr = 0; > - ccount = sscanf(buf + offset, "%d %u %n", > + ccount = sscanf(buf + offset, "%u %u %n", > &data->patterns[data->npatterns].brightness, > &data->patterns[data->npatterns].delta_t, &cr); > + > + if (data->patterns[data->npatterns].brightness > data->led_cdev->max_brightness) { > + data->npatterns = 0; > + err = -EINVAL; > + goto out; > + } > + > if (ccount != 2) { > data->npatterns = 0; > err = -EINVAL; > > > > > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- Baolin Wang Best Regards
On 10/25/2018 11:24 PM, Pavel Machek wrote: > > We don't want brightness < 0, but this may not be best way to do > this. We also don't want brightness > max_brightness, but I'm not sure > this check is effective. > > We probably also don't want pattern where all the delta_t s are zero. > > I came up with this so far.... > Pavel > > > > diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c > index ce7acd1..bc5f495 100644 > --- a/drivers/leds/trigger/ledtrig-pattern.c > +++ b/drivers/leds/trigger/ledtrig-pattern.c > @@ -250,9 +251,16 @@ static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev, > > while (offset < count - 1 && data->npatterns < MAX_PATTERNS) { > cr = 0; > - ccount = sscanf(buf + offset, "%d %u %n", > + ccount = sscanf(buf + offset, "%u %u %n", > &data->patterns[data->npatterns].brightness, > &data->patterns[data->npatterns].delta_t, &cr); > + > + if (data->patterns[data->npatterns].brightness > data->led_cdev->max_brightness) { > + data->npatterns = 0; > + err = -EINVAL; We don't fail on attempt of setting brightness > max_brightness, but only truncate it. In this case we could do the same for consistency. > + goto out; > + } > + > if (ccount != 2) { > data->npatterns = 0; > err = -EINVAL; > > > > -- Best regards, Jacek Anaszewski
diff --git a/Documentation/ABI/testing/sysfs-class-led-trigger-pattern b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern new file mode 100644 index 0000000..fb3d1e0 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern @@ -0,0 +1,82 @@ +What: /sys/class/leds/<led>/pattern +Date: September 2018 +KernelVersion: 4.20 +Description: + Specify a software pattern for the LED, that supports altering + the brightness for the specified duration with one software + timer. It can do gradual dimming and step change of brightness. + + The pattern is given by a series of tuples, of brightness and + duration (ms). The LED is expected to traverse the series and + each brightness value for the specified duration. Duration of + 0 means brightness should immediately change to new value, and + writing malformed pattern deactivates any active one. + + 1. For gradual dimming, the dimming interval now is set as 50 + milliseconds. So the tuple with duration less than dimming + interval (50ms) is treated as a step change of brightness, + i.e. the subsequent brightness will be applied without adding + intervening dimming intervals. + + The gradual dimming format of the software pattern values should be: + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 + duration_3 ...". For example: + + echo 0 1000 255 2000 > pattern + + It will make the LED go gradually from zero-intensity to max (255) + intensity in 1000 milliseconds, then back to zero intensity in 2000 + milliseconds: + + LED brightness + ^ + 255-| / \ / \ / + | / \ / \ / + | / \ / \ / + | / \ / \ / + 0-| / \/ \/ + +---0----1----2----3----4----5----6------------> time (s) + + 2. To make the LED go instantly from one brigntess value to another, + we should use use zero-time lengths (the brightness must be same as + the previous tuple's). So the format should be: + "brightness_1 duration_1 brightness_1 0 brightness_2 duration_2 + brightness_2 0 ...". For example: + + echo 0 1000 0 0 255 2000 255 0 > pattern + + It will make the LED stay off for one second, then stay at max brightness + for two seconds: + + LED brightness + ^ + 255-| +---------+ +---------+ + | | | | | + | | | | | + | | | | | + 0-| -----+ +----+ +---- + +---0----1----2----3----4----5----6------------> time (s) + +What: /sys/class/leds/<led>/hw_pattern +Date: September 2018 +KernelVersion: 4.20 +Description: + Specify a hardware pattern for the LED, for LED hardware that + supports autonomously controlling brightness over time, according + to some preprogrammed hardware patterns. It deactivates any active + software pattern. + + Since different LED hardware can have different semantics of + hardware patterns, each driver is expected to provide its own + description for the hardware patterns in their ABI documentation + file. + +What: /sys/class/leds/<led>/repeat +Date: September 2018 +KernelVersion: 4.20 +Description: + Specify a pattern repeat number. -1 means repeat indefinitely, + other negative numbers and number 0 are invalid. + + This file will always return the originally written repeat + number. diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig index 4018af7..b76fc3c 100644 --- a/drivers/leds/trigger/Kconfig +++ b/drivers/leds/trigger/Kconfig @@ -129,4 +129,11 @@ config LEDS_TRIGGER_NETDEV This allows LEDs to be controlled by network device activity. If unsure, say Y. +config LEDS_TRIGGER_PATTERN + tristate "LED Pattern Trigger" + help + This allows LEDs to be controlled by a software or hardware pattern + which is a series of tuples, of brightness and duration (ms). + If unsure, say N + endif # LEDS_TRIGGERS diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile index f3cfe19..9bcb64e 100644 --- a/drivers/leds/trigger/Makefile +++ b/drivers/leds/trigger/Makefile @@ -13,3 +13,4 @@ obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT) += ledtrig-transient.o obj-$(CONFIG_LEDS_TRIGGER_CAMERA) += ledtrig-camera.o obj-$(CONFIG_LEDS_TRIGGER_PANIC) += ledtrig-panic.o obj-$(CONFIG_LEDS_TRIGGER_NETDEV) += ledtrig-netdev.o +obj-$(CONFIG_LEDS_TRIGGER_PATTERN) += ledtrig-pattern.o diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c new file mode 100644 index 0000000..ce7acd1 --- /dev/null +++ b/drivers/leds/trigger/ledtrig-pattern.c @@ -0,0 +1,411 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * LED pattern trigger + * + * Idea discussed with Pavel Machek. Raphael Teysseyre implemented + * the first version, Baolin Wang simplified and improved the approach. + */ + +#include <linux/kernel.h> +#include <linux/leds.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/slab.h> +#include <linux/timer.h> + +#define MAX_PATTERNS 1024 +/* + * When doing gradual dimming, the led brightness will be updated + * every 50 milliseconds. + */ +#define UPDATE_INTERVAL 50 + +struct pattern_trig_data { + struct led_classdev *led_cdev; + struct led_pattern patterns[MAX_PATTERNS]; + struct led_pattern *curr; + struct led_pattern *next; + struct mutex lock; + u32 npatterns; + int repeat; + int last_repeat; + int delta_t; + bool is_indefinite; + bool is_hw_pattern; + struct timer_list timer; +}; + +static void pattern_trig_update_patterns(struct pattern_trig_data *data) +{ + data->curr = data->next; + if (!data->is_indefinite && data->curr == data->patterns) + data->repeat--; + + if (data->next == data->patterns + data->npatterns - 1) + data->next = data->patterns; + else + data->next++; + + data->delta_t = 0; +} + +static int pattern_trig_compute_brightness(struct pattern_trig_data *data) +{ + int step_brightness; + + /* + * If current tuple's duration is less than the dimming interval, + * we should treat it as a step change of brightness instead of + * doing gradual dimming. + */ + if (data->delta_t == 0 || data->curr->delta_t < UPDATE_INTERVAL) + return data->curr->brightness; + + step_brightness = abs(data->next->brightness - data->curr->brightness); + step_brightness = data->delta_t * step_brightness / data->curr->delta_t; + + if (data->next->brightness > data->curr->brightness) + return data->curr->brightness + step_brightness; + else + return data->curr->brightness - step_brightness; +} + +static void pattern_trig_timer_function(struct timer_list *t) +{ + struct pattern_trig_data *data = from_timer(data, t, timer); + + mutex_lock(&data->lock); + + for (;;) { + if (!data->is_indefinite && !data->repeat) + break; + + if (data->curr->brightness == data->next->brightness) { + /* Step change of brightness */ + led_set_brightness(data->led_cdev, + data->curr->brightness); + mod_timer(&data->timer, + jiffies + msecs_to_jiffies(data->curr->delta_t)); + + /* Skip the tuple with zero duration */ + pattern_trig_update_patterns(data); + /* Select next tuple */ + pattern_trig_update_patterns(data); + } else { + /* Gradual dimming */ + + /* + * If the accumulation time is larger than current + * tuple's duration, we should go next one and re-check + * if we repeated done. + */ + if (data->delta_t > data->curr->delta_t) { + pattern_trig_update_patterns(data); + continue; + } + + led_set_brightness(data->led_cdev, + pattern_trig_compute_brightness(data)); + mod_timer(&data->timer, + jiffies + msecs_to_jiffies(UPDATE_INTERVAL)); + + /* Accumulate the gradual dimming time */ + data->delta_t += UPDATE_INTERVAL; + } + + break; + } + + mutex_unlock(&data->lock); +} + +static int pattern_trig_start_pattern(struct led_classdev *led_cdev) +{ + struct pattern_trig_data *data = led_cdev->trigger_data; + + if (!data->npatterns) + return 0; + + if (data->is_hw_pattern) { + return led_cdev->pattern_set(led_cdev, data->patterns, + data->npatterns, data->repeat); + } + + /* At least 2 tuples for software pattern. */ + if (data->npatterns < 2) + return -EINVAL; + + data->delta_t = 0; + data->curr = data->patterns; + data->next = data->patterns + 1; + data->timer.expires = jiffies; + add_timer(&data->timer); + + return 0; +} + +static ssize_t repeat_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct pattern_trig_data *data = led_cdev->trigger_data; + int repeat; + + mutex_lock(&data->lock); + + repeat = data->last_repeat; + + mutex_unlock(&data->lock); + + return scnprintf(buf, PAGE_SIZE, "%d\n", repeat); +} + +static ssize_t repeat_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct pattern_trig_data *data = led_cdev->trigger_data; + int err, res; + + err = kstrtos32(buf, 10, &res); + if (err) + return err; + + /* Number 0 and negative numbers except -1 are invalid. */ + if (res < -1 || res == 0) + return -EINVAL; + + /* + * Clear previous patterns' performence firstly, and remove the timer + * without mutex lock to avoid dead lock. + */ + del_timer_sync(&data->timer); + + mutex_lock(&data->lock); + + if (data->is_hw_pattern) + led_cdev->pattern_clear(led_cdev); + + data->last_repeat = data->repeat = res; + /* -1 means repeat indefinitely */ + if (data->repeat == -1) + data->is_indefinite = true; + else + data->is_indefinite = false; + + err = pattern_trig_start_pattern(led_cdev); + + mutex_unlock(&data->lock); + return err < 0 ? err : count; +} + +static DEVICE_ATTR_RW(repeat); + +static ssize_t pattern_trig_show_patterns(struct pattern_trig_data *data, + char *buf, bool hw_pattern) +{ + ssize_t count = 0; + int i; + + mutex_lock(&data->lock); + + if (!data->npatterns || (data->is_hw_pattern ^ hw_pattern)) + goto out; + + for (i = 0; i < data->npatterns; i++) { + count += scnprintf(buf + count, PAGE_SIZE - count, + "%d %u ", + data->patterns[i].brightness, + data->patterns[i].delta_t); + } + + buf[count - 1] = '\n'; + +out: + mutex_unlock(&data->lock); + return count; +} + +static ssize_t pattern_trig_store_patterns(struct led_classdev *led_cdev, + const char *buf, size_t count, + bool hw_pattern) +{ + struct pattern_trig_data *data = led_cdev->trigger_data; + int ccount, cr, offset = 0, err = 0; + + /* + * Clear previous patterns' performence firstly, and remove the timer + * without mutex lock to avoid dead lock. + */ + del_timer_sync(&data->timer); + + mutex_lock(&data->lock); + + if (data->is_hw_pattern) + led_cdev->pattern_clear(led_cdev); + + data->is_hw_pattern = hw_pattern; + data->npatterns = 0; + + while (offset < count - 1 && data->npatterns < MAX_PATTERNS) { + cr = 0; + ccount = sscanf(buf + offset, "%d %u %n", + &data->patterns[data->npatterns].brightness, + &data->patterns[data->npatterns].delta_t, &cr); + if (ccount != 2) { + data->npatterns = 0; + err = -EINVAL; + goto out; + } + + offset += cr; + data->npatterns++; + } + + err = pattern_trig_start_pattern(led_cdev); + if (err) + data->npatterns = 0; + +out: + mutex_unlock(&data->lock); + return err < 0 ? err : count; +} + +static ssize_t pattern_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct pattern_trig_data *data = led_cdev->trigger_data; + + return pattern_trig_show_patterns(data, buf, false); +} + +static ssize_t pattern_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + + return pattern_trig_store_patterns(led_cdev, buf, count, false); +} + +static DEVICE_ATTR_RW(pattern); + +static ssize_t hw_pattern_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct pattern_trig_data *data = led_cdev->trigger_data; + + return pattern_trig_show_patterns(data, buf, true); +} + +static ssize_t hw_pattern_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + + return pattern_trig_store_patterns(led_cdev, buf, count, true); +} + +static DEVICE_ATTR_RW(hw_pattern); + +static umode_t pattern_trig_attrs_mode(struct kobject *kobj, + struct attribute *attr, int index) +{ + struct device *dev = container_of(kobj, struct device, kobj); + struct led_classdev *led_cdev = dev_get_drvdata(dev); + + if (attr == &dev_attr_repeat.attr || attr == &dev_attr_pattern.attr) + return attr->mode; + else if (attr == &dev_attr_hw_pattern.attr && led_cdev->pattern_set) + return attr->mode; + + return 0; +} + +static struct attribute *pattern_trig_attrs[] = { + &dev_attr_pattern.attr, + &dev_attr_hw_pattern.attr, + &dev_attr_repeat.attr, + NULL +}; + +static const struct attribute_group pattern_trig_group = { + .attrs = pattern_trig_attrs, + .is_visible = pattern_trig_attrs_mode, +}; + +static const struct attribute_group *pattern_trig_groups[] = { + &pattern_trig_group, + NULL, +}; + +static int pattern_trig_activate(struct led_classdev *led_cdev) +{ + struct pattern_trig_data *data; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + if (!!led_cdev->pattern_set ^ !!led_cdev->pattern_clear) { + dev_warn(led_cdev->dev, + "Hardware pattern ops validation failed\n"); + led_cdev->pattern_set = NULL; + led_cdev->pattern_clear = NULL; + } + + data->is_indefinite = true; + data->last_repeat = -1; + mutex_init(&data->lock); + data->led_cdev = led_cdev; + led_set_trigger_data(led_cdev, data); + timer_setup(&data->timer, pattern_trig_timer_function, 0); + led_cdev->activated = true; + + return 0; +} + +static void pattern_trig_deactivate(struct led_classdev *led_cdev) +{ + struct pattern_trig_data *data = led_cdev->trigger_data; + + if (!led_cdev->activated) + return; + + if (led_cdev->pattern_clear) + led_cdev->pattern_clear(led_cdev); + + del_timer_sync(&data->timer); + + led_set_brightness(led_cdev, LED_OFF); + kfree(data); + led_cdev->activated = false; +} + +static struct led_trigger pattern_led_trigger = { + .name = "pattern", + .activate = pattern_trig_activate, + .deactivate = pattern_trig_deactivate, + .groups = pattern_trig_groups, +}; + +static int __init pattern_trig_init(void) +{ + return led_trigger_register(&pattern_led_trigger); +} + +static void __exit pattern_trig_exit(void) +{ + led_trigger_unregister(&pattern_led_trigger); +} + +module_init(pattern_trig_init); +module_exit(pattern_trig_exit); + +MODULE_AUTHOR("Raphael Teysseyre <rteysseyre@gmail.com"); +MODULE_AUTHOR("Baolin Wang <baolin.wang@linaro.org"); +MODULE_DESCRIPTION("LED Pattern trigger"); +MODULE_LICENSE("GPL v2"); diff --git a/include/linux/leds.h b/include/linux/leds.h index 834683d..7393a31 100644 --- a/include/linux/leds.h +++ b/include/linux/leds.h @@ -22,6 +22,7 @@ #include <linux/workqueue.h> struct device; +struct led_pattern; /* * LED Core */ @@ -88,6 +89,10 @@ struct led_classdev { unsigned long *delay_on, unsigned long *delay_off); + int (*pattern_set)(struct led_classdev *led_cdev, + struct led_pattern *pattern, u32 len, int repeat); + int (*pattern_clear)(struct led_classdev *led_cdev); + struct device *dev; const struct attribute_group **groups; @@ -472,4 +477,14 @@ static inline void led_classdev_notify_brightness_hw_changed( struct led_classdev *led_cdev, enum led_brightness brightness) { } #endif +/** + * struct led_pattern - pattern interval settings + * @delta_t: pattern interval delay, in milliseconds + * @brightness: pattern interval brightness + */ +struct led_pattern { + u32 delta_t; + int brightness; +}; + #endif /* __LINUX_LEDS_H_INCLUDED */