Messages in this thread | | | Date | Thu, 31 Mar 2016 09:28:57 +0200 | From | Jacek Anaszewski <> | Subject | Re: [PATCH 1/3] leds: trigger: Introduce a kernel panic LED trigger |
| |
On 03/31/2016 09:04 AM, Jacek Anaszewski wrote: > Hi Ezequiel, > > On 03/30/2016 09:11 PM, Ezequiel Garcia wrote: >> +lkml >> >> On 30 Mar 11:29 AM, Jacek Anaszewski wrote: >>> Hi Ezequiel, >>> >>> Thanks for the patch. I've tested it on exynos4412-trats2 board >>> with leds-aat1290 driver, by executing: >>> >>> echo "c" > /proc/sysrq-trigger >>> >>> I was able to notice the blinking then. >>> >>> Applied to the for-next branch of linux-leds.git. >>> >> >> Notice that we currently need LEDs to be dedicated >> to the panic trigger, which is pretty lame as LEDs >> are scarce and are most likely assigned to something else. >> >> So, here's a toy patch to switch all the installed LEDs >> to the panic trigger on kernel panic. Patch is half-baked, >> but it shows the idea. >> >> (Interestingly, it also blinks the LEDs on a USB keyboard!) >> >> Is there any value in polishing this and find a way upstream? > > I like the idea, but I'd rather explicitly mark LEDs as panic > indicators. > > How about adding a "kernel-panic-indicator" Device Tree property to > common LED bindings? LED class device could be registered for the > panic trigger on kernel panic notification only when the property is > present.
Of course we would need similar solution for ACPI based platforms, however I am not sure if this approach is feasible in that case, since device configuration data is enclosed in firmware then. Adding linux-acpi list.
> Adding Rob, Mark and devicetree list. > >> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c >> index aa84e5b37593..caaf6161a7ae 100644 >> --- a/drivers/leds/led-class.c >> +++ b/drivers/leds/led-class.c >> @@ -321,6 +321,20 @@ void devm_led_classdev_unregister(struct device >> *dev, >> } >> EXPORT_SYMBOL_GPL(devm_led_classdev_unregister); >> >> +static int led_panic_notifier(struct notifier_block *nb, >> + unsigned long code, void *unused) >> +{ >> + struct led_classdev *led_cdev; >> + >> + list_for_each_entry(led_cdev, &leds_list, node) >> + led_trigger_set_at_panic(led_cdev, "panic"); >> + return NOTIFY_DONE; >> +} >> + >> +static struct notifier_block led_panic_nb = { >> + .notifier_call = led_panic_notifier, >> +}; >> + >> static int __init leds_init(void) >> { >> leds_class = class_create(THIS_MODULE, "leds"); >> @@ -328,6 +342,8 @@ static int __init leds_init(void) >> return PTR_ERR(leds_class); >> leds_class->pm = &leds_class_dev_pm_ops; >> leds_class->dev_groups = led_groups; >> + atomic_notifier_chain_register(&panic_notifier_list, >> + &led_panic_nb); >> return 0; >> } >> >> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c >> index 2181581795d3..8c1d33acdfa8 100644 >> --- a/drivers/leds/led-triggers.c >> +++ b/drivers/leds/led-triggers.c >> @@ -148,6 +148,21 @@ void led_trigger_remove(struct led_classdev >> *led_cdev) >> } >> EXPORT_SYMBOL_GPL(led_trigger_remove); >> >> +void led_trigger_set_at_panic(struct led_classdev *led_cdev, const >> char *name) >> +{ > > "name" parameter is not required, since setting other trigger than panic > on kernel panic doesn't make sense. > >> + struct led_trigger *trig; >> + >> + list_for_each_entry(trig, &trigger_list, next_trig) { >> + if (strcmp(name, trig->name)) >> + continue; >> + list_add_tail(&led_cdev->trig_list, &trig->led_cdevs); >> + led_cdev->trigger = trig; >> + if (trig->activate) >> + trig->activate(led_cdev); >> + break; >> + } >> +} >> + >> void led_trigger_set_default(struct led_classdev *led_cdev) >> { >> struct led_trigger *trig; >> diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h >> index db3f20da7221..8cfa10f626a6 100644 >> --- a/drivers/leds/leds.h >> +++ b/drivers/leds/leds.h >> @@ -21,6 +21,7 @@ static inline int led_get_brightness(struct >> led_classdev *led_cdev) >> return led_cdev->brightness; >> } >> >> +void led_trigger_set_at_panic(struct led_classdev *led_cdev, const >> char *name); >> void led_init_core(struct led_classdev *led_cdev); >> void led_stop_software_blink(struct led_classdev *led_cdev); >> void led_set_brightness_nopm(struct led_classdev *led_cdev, >> > >
-- Best regards, Jacek Anaszewski
| |