| Message ID | 20210906135320.23134-1-chaochao2021666@163.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] leds:triggers:Extend the kernel panic LED trigger | expand |
On Mon, 6 Sep 2021 21:53:18 +0800 chaochao2021666@163.com wrote: > From: chao zeng <chao.zeng@siemens.com> > > This commit extend panic trigger, add two new panic trigger > "panic_on" and "panic_off" and keep the "panic" compatible with > "panic_blink". > > All the led on the "panic_on" would light and on > the "panic_off" would turn off We don't wont gazillion triggers, each for every possible setting. Instead extend the existing panic trigger to have another sysfs setting where you can set this behavior. echo panic >trigger echo blink >on_panic So the on_panic file can accept "on", "off" or "blink". Alternatively a pattern could be set as in the ledtrig-pattern trigger. Also your patches do not use correct spacing in commit titles: leds:triggers:Extend the kernel panic LED trigger should instead be leds: triggers: Extend the kernel panic LED trigger Marek
On 08.09.21 03:45, chaochao2021666 wrote: > Dear Marek > > > For other types of led could be set at the userspace level. But for the > panic, > maybe it would trigger at kernel space during the kernel boot up. > > And currently only blink to indicate the error. we need more kinds of > type to indicate the error. > > we have two leds in the panic trigger group, all in the panic only one > behavior-- blink. > we need different panic led behavior, so extend the led behavior. I > think add more types of > LED behavior could be helpful. > To make it even clearer, there are three issues to solve for us: One is that we have two LEDs mixing a color, red and green, and the obviously desired panic color it red, not orange. The other is that the desired state in an error case is non-blinking, just on (in line with what our U-Boot will do in case the boot fails). And as we need that behavior prior to userspace, it should be configurable via DT. But that does not exclude extending the sysfs interface as well with the new options. Jan > BRs > Chao > > > At 2021-09-07 20:20:18, "Marek Behún" <kabel@kernel.org> wrote: >>On Mon, 6 Sep 2021 21:53:18 +0800 >>chaochao2021666@163.com wrote: >> >>> From: chao zeng <chao.zeng@siemens.com> >>> >>> This commit extend panic trigger, add two new panic trigger >>> "panic_on" and "panic_off" and keep the "panic" compatible with >>> "panic_blink". >>> >>> All the led on the "panic_on" would light and on >>> the "panic_off" would turn off >> >>We don't wont gazillion triggers, each for every possible setting. >> >>Instead extend the existing panic trigger to have another sysfs setting >>where you can set this behavior. >> echo panic >trigger >> echo blink >on_panic >>So the on_panic file can accept "on", "off" or "blink". >> >>Alternatively a pattern could be set as in the ledtrig-pattern trigger. >> >>Also your patches do not use correct spacing in commit titles: >> leds:triggers:Extend the kernel panic LED trigger >>should instead be >> leds: triggers: Extend the kernel panic LED trigger >> >>Marek >
diff --git a/drivers/leds/trigger/ledtrig-panic.c b/drivers/leds/trigger/ledtrig-panic.c index 64abf2e91608..1274bc94b5dd 100644 --- a/drivers/leds/trigger/ledtrig-panic.c +++ b/drivers/leds/trigger/ledtrig-panic.c @@ -12,19 +12,26 @@ #include <linux/leds.h> #include "../leds.h" -static struct led_trigger *trigger; +enum led_display_type { + ON, + OFF, + BLINK, + DISPLAY_TYPE_COUNT, +}; + +static struct led_trigger *panic_trigger[DISPLAY_TYPE_COUNT]; /* * This is called in a special context by the atomic panic * notifier. This means the trigger can be changed without * worrying about locking. */ -static void led_trigger_set_panic(struct led_classdev *led_cdev) +static void led_trigger_set_panic(struct led_classdev *led_cdev, const char *type) { struct led_trigger *trig; list_for_each_entry(trig, &trigger_list, next_trig) { - if (strcmp("panic", trig->name)) + if (strcmp(type, trig->name)) continue; if (led_cdev->trigger) list_del(&led_cdev->trig_list); @@ -37,6 +44,10 @@ static void led_trigger_set_panic(struct led_classdev *led_cdev) led_cdev->trigger = trig; if (trig->activate) trig->activate(led_cdev); + + /*Clear current brightness work*/ + led_cdev->work_flags = 0; + break; } } @@ -48,7 +59,12 @@ static int led_trigger_panic_notifier(struct notifier_block *nb, list_for_each_entry(led_cdev, &leds_list, node) if (led_cdev->flags & LED_PANIC_INDICATOR) - led_trigger_set_panic(led_cdev); + led_trigger_set_panic(led_cdev, "panic"); + else if (led_cdev->flags & LED_PANIC_INDICATOR_ON) + led_trigger_set_panic(led_cdev, "panic_on"); + else if (led_cdev->flags & LED_PANIC_INDICATOR_OFF) + led_trigger_set_panic(led_cdev, "panic_off"); + return NOTIFY_DONE; } @@ -56,9 +72,12 @@ static struct notifier_block led_trigger_panic_nb = { .notifier_call = led_trigger_panic_notifier, }; -static long led_panic_blink(int state) +static long led_panic_activity(int state) { - led_trigger_event(trigger, state ? LED_FULL : LED_OFF); + led_trigger_event(panic_trigger[BLINK], state ? LED_FULL : LED_OFF); + led_trigger_event(panic_trigger[ON], LED_FULL); + led_trigger_event(panic_trigger[OFF], LED_OFF); + return 0; } @@ -67,8 +86,12 @@ static int __init ledtrig_panic_init(void) atomic_notifier_chain_register(&panic_notifier_list, &led_trigger_panic_nb); - led_trigger_register_simple("panic", &trigger); - panic_blink = led_panic_blink; + led_trigger_register_simple("panic", &panic_trigger[BLINK]); + led_trigger_register_simple("panic_on", &panic_trigger[ON]); + led_trigger_register_simple("panic_off", &panic_trigger[OFF]); + + panic_blink = led_panic_activity; + return 0; } device_initcall(ledtrig_panic_init);