Am 17.03.2016 um 14:41 schrieb Jacek Anaszewski: > Hi Heiner, > > Thanks for the patch. I have a few questions: > > 1. Why do you want to register another trigger in > a ledtrig-heartbeat.c? The existing trigger uses the heartbeat and can be used with RGB and non-RGB LED's. The new trigger uses color to visualize the load and can be used with RGB LED's only. > 2. heartbeat trigger implements LED blinking timing that > resembles human heart beat rhythm, whereas heartbeat_led_rgb_trigger > seems not to reuse led_heartbeat_function code, but instead > led_heartbeat_rgb_function() sets timer interval to 500ms, after > doing some color magic in led_trigger_range_event(). Where is the > heartbeat rhythm implemented here? It uses a color to display the load instead of a rhythm. I have to admit that this trigger might not perfectly fit here as it doesn't actually implement a heartbeat. However it visualizes the same parameter (system load). If preferrable I can make it a separate trigger. > 3. Why heartbeat_range.end is somehow related to num_online_cpus()? Because e.g. 5 is a high load on a single CPU system but more or less idle on a 64 CPU system. Regards, Heiner > > Best regards, > Jacek Anaszewski > > On 03/13/2016 06:18 PM, Heiner Kallweit wrote: >> Add a RGB version of the heartbeat trigger (heartbeat-rgb). This version of >> the trigger can only be used with LED devices supporting the RGB extension. >> >> It maps the load [0 .. num_online_cpus] to a color between green and red. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx> >> --- >> drivers/leds/trigger/ledtrig-heartbeat.c | 64 +++++++++++++++++++++++++++++++- >> 1 file changed, 63 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/leds/trigger/ledtrig-heartbeat.c b/drivers/leds/trigger/ledtrig-heartbeat.c >> index 410c39c..510a3c9 100644 >> --- a/drivers/leds/trigger/ledtrig-heartbeat.c >> +++ b/drivers/leds/trigger/ledtrig-heartbeat.c >> @@ -22,6 +22,7 @@ >> #include "../leds.h" >> >> static int panic_heartbeats; >> +static struct led_trigger heartbeat_led_rgb_trigger; >> >> struct heartbeat_trig_data { >> unsigned int phase; >> @@ -30,6 +31,14 @@ struct heartbeat_trig_data { >> unsigned int invert; >> }; >> >> +static struct led_trigger_range led_heartbeat_range = { >> + .start = 0, >> + /* default, overridden by module init function */ >> + .end = 4 * FIXED_1, >> + .color_start = 0x54ffff, /* GREEN */ >> + .color_end = 0x00ffff, /* RED */ >> +}; >> + >> static void led_heartbeat_function(unsigned long data) >> { >> struct led_classdev *led_cdev = (struct led_classdev *) data; >> @@ -85,6 +94,15 @@ static void led_heartbeat_function(unsigned long data) >> mod_timer(&heartbeat_data->timer, jiffies + delay); >> } >> >> +static void led_heartbeat_rgb_function(unsigned long data) >> +{ >> + struct led_classdev *led_cdev = (struct led_classdev *) data; >> + struct heartbeat_trig_data *heartbeat_data = led_cdev->trigger_data; >> + >> + led_trigger_range_event(&heartbeat_led_rgb_trigger, avenrun[0]); >> + mod_timer(&heartbeat_data->timer, jiffies + msecs_to_jiffies(500)); >> +} >> + >> static ssize_t led_invert_show(struct device *dev, >> struct device_attribute *attr, char *buf) >> { >> @@ -136,6 +154,22 @@ static void heartbeat_trig_activate(struct led_classdev *led_cdev) >> led_cdev->activated = true; >> } >> >> +static void heartbeat_trig_rgb_activate(struct led_classdev *led_cdev) >> +{ >> + struct heartbeat_trig_data *heartbeat_data; >> + >> + heartbeat_data = kzalloc(sizeof(*heartbeat_data), GFP_KERNEL); >> + if (!heartbeat_data) >> + return; >> + >> + led_cdev->trigger_data = heartbeat_data; >> + >> + setup_timer(&heartbeat_data->timer, >> + led_heartbeat_rgb_function, (unsigned long) led_cdev); >> + led_heartbeat_rgb_function(heartbeat_data->timer.data); >> + led_cdev->activated = true; >> +} >> + >> static void heartbeat_trig_deactivate(struct led_classdev *led_cdev) >> { >> struct heartbeat_trig_data *heartbeat_data = led_cdev->trigger_data; >> @@ -148,12 +182,31 @@ static void heartbeat_trig_deactivate(struct led_classdev *led_cdev) >> } >> } >> >> +static void heartbeat_trig_rgb_deactivate(struct led_classdev *led_cdev) >> +{ >> + struct heartbeat_trig_data *heartbeat_data = led_cdev->trigger_data; >> + >> + if (led_cdev->activated) { >> + del_timer_sync(&heartbeat_data->timer); >> + kfree(heartbeat_data); >> + led_cdev->activated = false; >> + } >> +} >> + >> static struct led_trigger heartbeat_led_trigger = { >> .name = "heartbeat", >> .activate = heartbeat_trig_activate, >> .deactivate = heartbeat_trig_deactivate, >> }; >> >> +static struct led_trigger heartbeat_led_rgb_trigger = { >> + .flags = LED_TRIG_CAP_RGB, >> + .name = "heartbeat-rgb", >> + .range = &led_heartbeat_range, >> + .activate = heartbeat_trig_rgb_activate, >> + .deactivate = heartbeat_trig_rgb_deactivate, >> +}; >> + >> static int heartbeat_reboot_notifier(struct notifier_block *nb, >> unsigned long code, void *unused) >> { >> @@ -178,9 +231,17 @@ static struct notifier_block heartbeat_panic_nb = { >> >> static int __init heartbeat_trig_init(void) >> { >> - int rc = led_trigger_register(&heartbeat_led_trigger); >> + int rc; >> + >> + led_heartbeat_range.end = num_online_cpus() * FIXED_1; >> + >> + rc = led_trigger_register(&heartbeat_led_rgb_trigger); >> + if (rc) >> + return rc; >> >> + rc = led_trigger_register(&heartbeat_led_trigger); >> if (!rc) { >> + led_trigger_unregister(&heartbeat_led_rgb_trigger); >> atomic_notifier_chain_register(&panic_notifier_list, >> &heartbeat_panic_nb); >> register_reboot_notifier(&heartbeat_reboot_nb); >> @@ -194,6 +255,7 @@ static void __exit heartbeat_trig_exit(void) >> atomic_notifier_chain_unregister(&panic_notifier_list, >> &heartbeat_panic_nb); >> led_trigger_unregister(&heartbeat_led_trigger); >> + led_trigger_unregister(&heartbeat_led_rgb_trigger); >> } >> >> module_init(heartbeat_trig_init); >> > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-leds" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html