| Message ID | e6d4e7be537a4e4d6fce26c6d446703b1910eab9.1535700092.git.baolin.wang@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v7,1/2] leds: core: Introduce LED pattern trigger | expand |
Hi Baolin, Thank you for the update. Please find my remarks below. On 08/31/2018 09:52 AM, Baolin Wang wrote: > This patch adds one new led trigger that LED device can configure > the software or hardware pattern and trigger it. > > 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 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 | 44 +++ > drivers/leds/trigger/Kconfig | 7 + > drivers/leds/trigger/Makefile | 1 + > drivers/leds/trigger/ledtrig-pattern.c | 337 ++++++++++++++++++++ > include/linux/leds.h | 16 + > 5 files changed, 405 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..f4749d1 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern > @@ -0,0 +1,44 @@ > +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. > + > + 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. > + > + The format of the software pattern values should be: > + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 > + duration_3 ...". > + > +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. > + > + Since different LED hardware can have different semantics of > + hardware patterns, each driver is expected to provide its own > + description for the hardware patterns as below. And I would cut this description here. > + For Spreadtrum SC27XX LED controller, it only supports 4 hardware > + patterns to configure the low time, rise time, high time and fall > + time for the breathing mode, and each stage duration unit is 125ms. > + So the format of the hardware pattern values should be: > + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 > + duration_3 brightness_4 duration_4". This part should go to the separate ABI documentation file, related to the driver for Spreadtrum SC27XX. > + > +What: /sys/class/leds/<led>/repeat > +Date: September 2018 > +KernelVersion: 4.20 > +Description: > + Specify a pattern repeat number. 0 means repeat indefinitely. > + > + 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..0179191 > --- /dev/null > +++ b/drivers/leds/trigger/ledtrig-pattern.c > @@ -0,0 +1,337 @@ > +// 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 > + > +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; > + u32 repeat; > + u32 last_repeat; > + 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++; > +} > + > +static void pattern_trig_timer_function(struct timer_list *t) > +{ > + struct pattern_trig_data *data = from_timer(data, t, timer); > + > + mutex_lock(&data->lock); > + > + if (!data->is_indefinite && !data->repeat) { > + mutex_unlock(&data->lock); > + return; > + } > + > + led_set_brightness(data->led_cdev, data->curr->brightness); > + mod_timer(&data->timer, jiffies + msecs_to_jiffies(data->curr->delta_t)); > + pattern_trig_update_patterns(data); > + > + 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); > + } I have doubts here if it is a good idea to enforce array of tuples as a generic interface for all hw_patterns. It may not fit well for every hw pattern engine. It seems that the only feasible solution will be allowing drivers to come up with their own interfaces, i.e. the approach you proposed at first for your driver. It seems that the ledtrig-pattern with software pattern mechanism will be just a nice side effect of this series :-) Unless someone will propose a better solution. We need a broader consensus here. I'd like to hear Pavel's opinion, since he's been always in favor of common pattern interface, and inspired this work. Pavel? Best regards, Jacek Anaszewski
Hi! > > +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); > > + } > > I have doubts here if it is a good idea to enforce array of tuples > as a generic interface for all hw_patterns. It may not fit well for > every hw pattern engine. It seems that the only feasible solution will > be allowing drivers to come up with their own interfaces, i.e. the > approach you proposed at first for your driver. It seems that the > ledtrig-pattern with software pattern mechanism will be just > a nice side effect of this series :-) > > Unless someone will propose a better solution. I believe array of tuples will work for everyone. It is just a LED, it can change intensity over time. > We need a broader consensus here. I'd like to hear Pavel's opinion, > since he's been always in favor of common pattern interface, and > inspired this work. I believe Baolin did good work here. I believe it will cover most, if not all, hardware engines out there. I think we should merge it, and see what happens -- it should be good enough. (Yes, there's still more work to do, but that will be stuff like RGB LED synchronization.) (And yes, one of the LED chip has pattern engine that can compute prime numbers on its own. I don't expect to support _that_. Fortunately, nobody but me is likely to want that pattern, so we are still okay :-) https://gitlab.com/tui/tui/blob/master/ofone/tests.notcc/primes.nc ) Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Jacek, On 4 September 2018 at 02:58, Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > Hi Baolin, > > Thank you for the update. > > Please find my remarks below. > > On 08/31/2018 09:52 AM, Baolin Wang wrote: >> This patch adds one new led trigger that LED device can configure >> the software or hardware pattern and trigger it. >> >> 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 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 | 44 +++ >> drivers/leds/trigger/Kconfig | 7 + >> drivers/leds/trigger/Makefile | 1 + >> drivers/leds/trigger/ledtrig-pattern.c | 337 ++++++++++++++++++++ >> include/linux/leds.h | 16 + >> 5 files changed, 405 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..f4749d1 >> --- /dev/null >> +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern >> @@ -0,0 +1,44 @@ >> +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. >> + >> + 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. >> + >> + The format of the software pattern values should be: >> + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 >> + duration_3 ...". >> + >> +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. >> + >> + Since different LED hardware can have different semantics of >> + hardware patterns, each driver is expected to provide its own >> + description for the hardware patterns as below. > > And I would cut this description here. > >> + For Spreadtrum SC27XX LED controller, it only supports 4 hardware >> + patterns to configure the low time, rise time, high time and fall >> + time for the breathing mode, and each stage duration unit is 125ms. >> + So the format of the hardware pattern values should be: >> + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 >> + duration_3 brightness_4 duration_4". > > This part should go to the separate ABI documentation file, related to > the driver for Spreadtrum SC27XX. Ah, sure. Will update in v8. Thanks. > >> + >> +What: /sys/class/leds/<led>/repeat >> +Date: September 2018 >> +KernelVersion: 4.20 >> +Description: >> + Specify a pattern repeat number. 0 means repeat indefinitely. >> + >> + 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..0179191 >> --- /dev/null >> +++ b/drivers/leds/trigger/ledtrig-pattern.c >> @@ -0,0 +1,337 @@ >> +// 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 >> + >> +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; >> + u32 repeat; >> + u32 last_repeat; >> + 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++; >> +} >> + >> +static void pattern_trig_timer_function(struct timer_list *t) >> +{ >> + struct pattern_trig_data *data = from_timer(data, t, timer); >> + >> + mutex_lock(&data->lock); >> + >> + if (!data->is_indefinite && !data->repeat) { >> + mutex_unlock(&data->lock); >> + return; >> + } >> + >> + led_set_brightness(data->led_cdev, data->curr->brightness); >> + mod_timer(&data->timer, jiffies + msecs_to_jiffies(data->curr->delta_t)); >> + pattern_trig_update_patterns(data); >> + >> + 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); >> + } > > I have doubts here if it is a good idea to enforce array of tuples > as a generic interface for all hw_patterns. It may not fit well for > every hw pattern engine. It seems that the only feasible solution will > be allowing drivers to come up with their own interfaces, i.e. the > approach you proposed at first for your driver. It seems that the > ledtrig-pattern with software pattern mechanism will be just > a nice side effect of this series :-) > > Unless someone will propose a better solution. > > We need a broader consensus here. I'd like to hear Pavel's opinion, > since he's been always in favor of common pattern interface, and > inspired this work. > > Pavel? > > Best regards, > Jacek Anaszewski -- Baolin Wang Best Regards
Hi Pavel, On 4 September 2018 at 05:53, Pavel Machek <pavel@ucw.cz> wrote: > Hi! > >> > +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); >> > + } >> >> I have doubts here if it is a good idea to enforce array of tuples >> as a generic interface for all hw_patterns. It may not fit well for >> every hw pattern engine. It seems that the only feasible solution will >> be allowing drivers to come up with their own interfaces, i.e. the >> approach you proposed at first for your driver. It seems that the >> ledtrig-pattern with software pattern mechanism will be just >> a nice side effect of this series :-) >> >> Unless someone will propose a better solution. > > I believe array of tuples will work for everyone. It is just a LED, it > can change intensity over time. > >> We need a broader consensus here. I'd like to hear Pavel's opinion, >> since he's been always in favor of common pattern interface, and >> inspired this work. > > I believe Baolin did good work here. I believe it will cover most, if > not all, hardware engines out there. I think we should merge it, and > see what happens -- it should be good enough. > > (Yes, there's still more work to do, but that will be stuff like RGB > LED synchronization.) > > (And yes, one of the LED chip has pattern engine that can compute > prime numbers on its own. I don't expect to support > _that_. Fortunately, nobody but me is likely to want that pattern, so > we are still okay :-) Thanks for your explanation here. So I think I should keep the same logics in next version. -- Baolin Wang Best Regards
On 09/03/2018 11:53 PM, Pavel Machek wrote: > Hi! > >>> +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); >>> + } >> >> I have doubts here if it is a good idea to enforce array of tuples >> as a generic interface for all hw_patterns. It may not fit well for >> every hw pattern engine. It seems that the only feasible solution will >> be allowing drivers to come up with their own interfaces, i.e. the >> approach you proposed at first for your driver. It seems that the >> ledtrig-pattern with software pattern mechanism will be just >> a nice side effect of this series :-) >> >> Unless someone will propose a better solution. > > I believe array of tuples will work for everyone. It is just a LED, it > can change intensity over time. We have an example of different semantics in case of hw pattern for leds-sc27xx-bltc.c, from this patch set. Proposed hw_pattern ABI documentation: +What: /sys/class/leds/<led>/hw_pattern +Date: September 2018 +KernelVersion: 4.20 +Description: + Specify a hardware pattern for the SC27XX LED. For the SC27XX + LED controller, it only supports 4 hardware patterns to configure + the low time, rise time, high time and fall time for the breathing + mode, and each stage duration unit is 125ms. So the format of + the hardware pattern values should be: + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 + duration_3 brightness_4 duration_4". In this case low time and high time can be easily described with use of the proposed [brightness delta_t] tuples. It is not equally obvious in case of rise time and fall time. I can imagine hw pattern that would require defining blink rate over period of time, or blink rate during rise/fall time - in the latter case we would have odd number of pattern components. Probably it wouldn't be a big deal, we'd need one "padding" value, but still there's room for improvement IMHO. >> We need a broader consensus here. I'd like to hear Pavel's opinion, >> since he's been always in favor of common pattern interface, and >> inspired this work. > > I believe Baolin did good work here. I believe it will cover most, if > not all, hardware engines out there. I think we should merge it, and > see what happens -- it should be good enough. > > (Yes, there's still more work to do, but that will be stuff like RGB > LED synchronization.) > > (And yes, one of the LED chip has pattern engine that can compute > prime numbers on its own. I don't expect to support > _that_. Fortunately, nobody but me is likely to want that pattern, so > we are still okay :-) > > https://gitlab.com/tui/tui/blob/master/ofone/tests.notcc/primes.nc > > ) > Pavel > -- Best regards, Jacek Anaszewski
Hi! > +What: /sys/class/leds/<led>/hw_pattern > +Date: September 2018 > +KernelVersion: 4.20 > +Description: > + Specify a hardware pattern for the SC27XX LED. For the SC27XX > + LED controller, it only supports 4 hardware patterns to configure > + the low time, rise time, high time and fall time for the breathing > + mode, and each stage duration unit is 125ms. So the format of > + the hardware pattern values should be: > + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 > + duration_3 brightness_4 duration_4". > > In this case low time and high time can be easily described with > use of the proposed [brightness delta_t] tuples. It is not equally > obvious in case of rise time and fall time. > > I can imagine hw pattern that would require defining blink rate > over period of time, or blink rate during rise/fall time - in the > latter case we would have odd number of pattern components. Probably > it wouldn't be a big deal, we'd need one "padding" value, but still > there's room for improvement IMHO. Well, you can describe blinking while rising, it is just going to be awkward as you'll need to give precise times/brightnesses for each blinking, and pattern will become long. I'm sure some hardware can do that (the led in N900 can compute prime numbers, it can blink while changing brightness, too). OTOH people tend to use pretty simple patterns on their LEDs, so we should be fine. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
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..f4749d1 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-led-trigger-pattern @@ -0,0 +1,44 @@ +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. + + 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. + + The format of the software pattern values should be: + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 + duration_3 ...". + +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. + + Since different LED hardware can have different semantics of + hardware patterns, each driver is expected to provide its own + description for the hardware patterns as below. + + For Spreadtrum SC27XX LED controller, it only supports 4 hardware + patterns to configure the low time, rise time, high time and fall + time for the breathing mode, and each stage duration unit is 125ms. + So the format of the hardware pattern values should be: + "brightness_1 duration_1 brightness_2 duration_2 brightness_3 + duration_3 brightness_4 duration_4". + +What: /sys/class/leds/<led>/repeat +Date: September 2018 +KernelVersion: 4.20 +Description: + Specify a pattern repeat number. 0 means repeat indefinitely. + + 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..0179191 --- /dev/null +++ b/drivers/leds/trigger/ledtrig-pattern.c @@ -0,0 +1,337 @@ +// 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 + +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; + u32 repeat; + u32 last_repeat; + 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++; +} + +static void pattern_trig_timer_function(struct timer_list *t) +{ + struct pattern_trig_data *data = from_timer(data, t, timer); + + mutex_lock(&data->lock); + + if (!data->is_indefinite && !data->repeat) { + mutex_unlock(&data->lock); + return; + } + + led_set_brightness(data->led_cdev, data->curr->brightness); + mod_timer(&data->timer, jiffies + msecs_to_jiffies(data->curr->delta_t)); + pattern_trig_update_patterns(data); + + 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); + } + + data->curr = data->patterns; + data->next = data->npatterns > 1 ? data->patterns + 1 : data->patterns; + 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; + u32 repeat; + + mutex_lock(&data->lock); + + repeat = data->last_repeat; + + mutex_unlock(&data->lock); + + return scnprintf(buf, PAGE_SIZE, "%u\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; + unsigned long res; + int err; + + err = kstrtoul(buf, 10, &res); + if (err) + return err; + + /* + * 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; + /* 0 means repeat indefinitely */ + data->is_indefinite = !data->repeat; + + 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 %d ", + 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 %d %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); + +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; + 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..74fc2c6 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,11 @@ struct led_classdev { unsigned long *delay_on, unsigned long *delay_off); + int (*pattern_set)(struct led_classdev *led_cdev, + struct led_pattern *pattern, int len, + unsigned int repeat); + int (*pattern_clear)(struct led_classdev *led_cdev); + struct device *dev; const struct attribute_group **groups; @@ -472,4 +478,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 { + int delta_t; + int brightness; +}; + #endif /* __LINUX_LEDS_H_INCLUDED */