diff mbox

[01/17] leds: create a trigger for CPU activity

Message ID 1312364089-32380-2-git-send-email-bryan.wu@canonical.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bryan Wu Aug. 3, 2011, 9:34 a.m. UTC
Attempting to consolidate the ARM LED code, this removes the
custom RealView LED trigger code to turn LEDs on and off in
response to CPU activity and replace it with a standard trigger.

(bryan.wu@canonical.com:
It moves arch/arm/kernel/leds.c syscore stubs into this trigger.
It also provides ledtrig_cpu trigger event stub in <linux/leds.h>.
Although it was inspired by ARM work, it can be used in other arch.)

Cc: Richard Purdie <rpurdie@rpsys.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bryan Wu <bryan.wu@canonical.com>
---
 drivers/leds/Kconfig       |   10 +++
 drivers/leds/Makefile      |    1 +
 drivers/leds/ledtrig-cpu.c |  144 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/leds.h       |   15 +++++
 4 files changed, 170 insertions(+), 0 deletions(-)
 create mode 100644 drivers/leds/ledtrig-cpu.c

Comments

Michał Mirosław Aug. 3, 2011, 10:23 a.m. UTC | #1
2011/8/3 Bryan Wu <bryan.wu@canonical.com>:
> Attempting to consolidate the ARM LED code, this removes the
> custom RealView LED trigger code to turn LEDs on and off in
> response to CPU activity and replace it with a standard trigger.
[...]
> +void ledtrig_cpu(enum cpu_led_event ledevt)
> +{
> +       struct ledtrig_cpu_data *trigdata = __get_cpu_var(ledtrig_cpu_triggers);
> +
> +       if (!trigdata)
> +               return;
> +
> +       /* Locate the correct CPU LED */
> +
> +       switch (ledevt) {
> +       case CPU_LED_IDLE_START:
> +       case CPU_LED_START:
> +               /* Will turn the LED on, max brightness */
> +               if (trigdata->led)
> +                       led_set_brightness(trigdata->led,
> +                                          trigdata->led->max_brightness);
> +               break;
> +
> +       case CPU_LED_IDLE_END:
> +       case CPU_LED_STOP:
> +       case CPU_LED_HALTED:
> +               /* Will turn the LED off */
> +               if (trigdata->led)
> +                       led_set_brightness(trigdata->led, LED_OFF);
> +               break;

Might be better to swap CPU_LED_IDLE_START with CPU_LED_IDLE_END here
so that when entering idle state the leds turn off.

Best Regards,
Micha? Miros?aw
Jamie Iles Aug. 3, 2011, 10:28 a.m. UTC | #2
Hi Bryan,

On Wed, Aug 03, 2011 at 05:34:33PM +0800, Bryan Wu wrote:
> Attempting to consolidate the ARM LED code, this removes the
> custom RealView LED trigger code to turn LEDs on and off in
> response to CPU activity and replace it with a standard trigger.
> 
> (bryan.wu@canonical.com:
> It moves arch/arm/kernel/leds.c syscore stubs into this trigger.
> It also provides ledtrig_cpu trigger event stub in <linux/leds.h>.
> Although it was inspired by ARM work, it can be used in other arch.)
> 
> Cc: Richard Purdie <rpurdie@rpsys.net>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Bryan Wu <bryan.wu@canonical.com>
> ---
>  drivers/leds/Kconfig       |   10 +++
>  drivers/leds/Makefile      |    1 +
>  drivers/leds/ledtrig-cpu.c |  144 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/leds.h       |   15 +++++
>  4 files changed, 170 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/leds/ledtrig-cpu.c
> 
> diff --git a/drivers/leds/ledtrig-cpu.c b/drivers/leds/ledtrig-cpu.c
> new file mode 100644
> index 0000000..0537c3b
[...]
> +static DEFINE_PER_CPU(struct ledtrig_cpu_data *, ledtrig_cpu_triggers);
[...]
> +static void ledtrig_cpu_activate_cpu(void *data)
> +{
> +	struct ledtrig_cpu_data *cpu_data;
> +	struct led_classdev *led = data;
> +	int my_cpu = smp_processor_id();
> +	unsigned long target_cpu = (unsigned long) led->trigger_data;
> +
> +	if (target_cpu != my_cpu)
> +		return;
> +
> +	cpu_data = kzalloc(sizeof(*cpu_data), GFP_KERNEL);
> +	if (!cpu_data)
> +		return;
> +
> +	dev_info(led->dev, "led %s indicate activity on CPU %d\n",
> +		 led->name, my_cpu);
> +
> +	cpu_data->led = led;
> +	__get_cpu_var(ledtrig_cpu_triggers) = cpu_data;
> +}
> +
> +static void ledtrig_cpu_activate(struct led_classdev *led)
> +{
> +	on_each_cpu(ledtrig_cpu_activate_cpu, led, 1);
> +}
> +
> +static void ledtrig_cpu_deactivate(struct led_classdev *led)
> +{
> +	struct ledtrig_cpu_data *cpu_data = led->trigger_data;
> +
> +	kfree(cpu_data);
> +}

Is this deactivation correct?  My (limited) understanding of the smp api 
is that we'll allocate the ledtrig_cpu_data for each CPU and store it in 
the ledtrig_cpu_triggers pointers.  So shouldn't this be doing a 
__get_cpu_var(ledtrig_cpu_triggers) and a kfree() on that (and setting 
to NULL)?

Also, where does led->trigger_data get assigned with the cpu id?

Jamie
Linus Walleij Aug. 3, 2011, 3:22 p.m. UTC | #3
On Wed, Aug 3, 2011 at 12:28 PM, Jamie Iles <jamie@jamieiles.com> wrote:
> Hi Bryan,
> [...]
>> +static DEFINE_PER_CPU(struct ledtrig_cpu_data *, ledtrig_cpu_triggers);
> [...]
>> +static void ledtrig_cpu_activate_cpu(void *data)
>> +{
>> +     struct ledtrig_cpu_data *cpu_data;
>> +     struct led_classdev *led = data;
>> +     int my_cpu = smp_processor_id();
>> +     unsigned long target_cpu = (unsigned long) led->trigger_data;
>> +
>> +     if (target_cpu != my_cpu)
>> +             return;
>> +
>> +     cpu_data = kzalloc(sizeof(*cpu_data), GFP_KERNEL);
>> +     if (!cpu_data)
>> +             return;
>> +
>> +     dev_info(led->dev, "led %s indicate activity on CPU %d\n",
>> +              led->name, my_cpu);
>> +
>> +     cpu_data->led = led;
>> +     __get_cpu_var(ledtrig_cpu_triggers) = cpu_data;
>> +}
>> +
>> +static void ledtrig_cpu_activate(struct led_classdev *led)
>> +{
>> +     on_each_cpu(ledtrig_cpu_activate_cpu, led, 1);
>> +}
>> +
>> +static void ledtrig_cpu_deactivate(struct led_classdev *led)
>> +{
>> +     struct ledtrig_cpu_data *cpu_data = led->trigger_data;
>> +
>> +     kfree(cpu_data);
>> +}
>
> Is this deactivation correct?  My (limited) understanding of the smp api
> is that we'll allocate the ledtrig_cpu_data for each CPU and store it in
> the ledtrig_cpu_triggers pointers.

At this point you have a handle to a specific LED and that one is tied
to a certain CPU aldready, since it is a member of
struct ledtrig_cpu_data. The LED is CPU-local by nature already since
it is looked up in the code being called from the ARM kernel,
i.e. the activate/deactivate pairs get called once per CPU.

> So shouldn't this be doing a
> __get_cpu_var(ledtrig_cpu_triggers) and a kfree() on that (and setting
> to NULL)?

You could do that but why? There is no way the pointer value
can make any harm. If activate() is called again it will be overwritten
with the new pointer.

> Also, where does led->trigger_data get assigned with the cpu id?

The activate() function is called on every core and if the CPU ID
is not equal to the assigned CPU it just exits, here:

+     int my_cpu = smp_processor_id();
+     unsigned long target_cpu = (unsigned long) led->trigger_data;
+
+     if (target_cpu != my_cpu)
+             return;

Yours,
Linus Walleij
Jamie Iles Aug. 3, 2011, 3:30 p.m. UTC | #4
Hi Linus,

On Wed, Aug 03, 2011 at 05:22:02PM +0200, Linus Walleij wrote:
> On Wed, Aug 3, 2011 at 12:28 PM, Jamie Iles <jamie@jamieiles.com> wrote:
> > Hi Bryan,
> > [...]
> >> +static DEFINE_PER_CPU(struct ledtrig_cpu_data *, ledtrig_cpu_triggers);
> > [...]
> >> +static void ledtrig_cpu_activate_cpu(void *data)
> >> +{
> >> +     struct ledtrig_cpu_data *cpu_data;
> >> +     struct led_classdev *led = data;
> >> +     int my_cpu = smp_processor_id();
> >> +     unsigned long target_cpu = (unsigned long) led->trigger_data;
> >> +
> >> +     if (target_cpu != my_cpu)
> >> +             return;
> >> +
> >> +     cpu_data = kzalloc(sizeof(*cpu_data), GFP_KERNEL);
> >> +     if (!cpu_data)
> >> +             return;
> >> +
> >> +     dev_info(led->dev, "led %s indicate activity on CPU %d\n",
> >> +              led->name, my_cpu);
> >> +
> >> +     cpu_data->led = led;
> >> +     __get_cpu_var(ledtrig_cpu_triggers) = cpu_data;
> >> +}
> >> +
> >> +static void ledtrig_cpu_activate(struct led_classdev *led)
> >> +{
> >> +     on_each_cpu(ledtrig_cpu_activate_cpu, led, 1);
> >> +}
> >> +
> >> +static void ledtrig_cpu_deactivate(struct led_classdev *led)
> >> +{
> >> +     struct ledtrig_cpu_data *cpu_data = led->trigger_data;
> >> +
> >> +     kfree(cpu_data);
> >> +}
> >
> > Is this deactivation correct?  My (limited) understanding of the smp api
> > is that we'll allocate the ledtrig_cpu_data for each CPU and store it in
> > the ledtrig_cpu_triggers pointers.
> 
> At this point you have a handle to a specific LED and that one is tied
> to a certain CPU aldready, since it is a member of
> struct ledtrig_cpu_data. The LED is CPU-local by nature already since
> it is looked up in the code being called from the ARM kernel,
> i.e. the activate/deactivate pairs get called once per CPU.

OK, I'm fine with that.
> 
> > So shouldn't this be doing a
> > __get_cpu_var(ledtrig_cpu_triggers) and a kfree() on that (and setting
> > to NULL)?
> 
> You could do that but why? There is no way the pointer value
> can make any harm. If activate() is called again it will be overwritten
> with the new pointer.

Yes, but can't ledtrig_cpu() potentially still be called after 
deactivation, then the:

	if (!trigdata)
		return;

would be incorrect?  My main point though was that it looks like 
led->trigger_data is the processor id and kfree() is being called on 
that.

> > Also, where does led->trigger_data get assigned with the cpu id?
> 
> The activate() function is called on every core and if the CPU ID
> is not equal to the assigned CPU it just exits, here:
> 
> +     int my_cpu = smp_processor_id();
> +     unsigned long target_cpu = (unsigned long) led->trigger_data;
> +
> +     if (target_cpu != my_cpu)
> +             return;

I understand that bit, but I can't see anything that actually assigns 
led->trigger_data in the first place.

Jamie
Jochen Friedrich Aug. 3, 2011, 4:02 p.m. UTC | #5
Hi Jamie,

> I understand that bit, but I can't see anything that actually assigns
> led->trigger_data in the first place.

I just tested this on simpad and while I can assign the cpu trigger to 
the green LED using the sys fs, the trigger just sits there and does 
nothing, just like the "none" trigger.

Thanks,
Jochen
Linus Walleij Aug. 3, 2011, 4:12 p.m. UTC | #6
On Wed, Aug 3, 2011 at 5:30 PM, Jamie Iles <jamie@jamieiles.com> wrote:
>> > So shouldn't this be doing a
>> > __get_cpu_var(ledtrig_cpu_triggers) and a kfree() on that (and setting
>> > to NULL)?
>>
>> You could do that but why? There is no way the pointer value
>> can make any harm. If activate() is called again it will be overwritten
>> with the new pointer.
>
> Yes, but can't ledtrig_cpu() potentially still be called after
> deactivation, then the:
>
>        if (!trigdata)
>                return;
>
> would be incorrect?  My main point though was that it looks like
> led->trigger_data is the processor id and kfree() is being called on
> that.

You're right, it just works fine because trigdata is always NULL...

>> > Also, where does led->trigger_data get assigned with the cpu id?
>>
>> The activate() function is called on every core and if the CPU ID
>> is not equal to the assigned CPU it just exits, here:
>>
>> +     int my_cpu = smp_processor_id();
>> +     unsigned long target_cpu = (unsigned long) led->trigger_data;
>> +
>> +     if (target_cpu != my_cpu)
>> +             return;
>
> I understand that bit, but I can't see anything that actually assigns
> led->trigger_data in the first place.

You're right, it's because nothing really does. And since
kfree(NULL); is fine this works, and makes the above thing work
fine as well.

Since led->trigger_data is never used we shouldn't touch it in
I believe, Bryan can you just take it out and replace with something
like this (totally untested):

static void ledtrig_cpu_deactivate_cpu(struct led_classdev *led)
{
       struct ledtrig_cpu_data *cpu_data = __get_cpu_var(ledtrig_cpu_triggers);
       int my_cpu = smp_processor_id();
       unsigned long target_cpu = (unsigned long) led->trigger_data;

       if (target_cpu != my_cpu)
               return;

       kfree(cpu_data);
       __get_cpu_var(ledtrig_cpu_triggers) = NULL;
}

static void ledtrig_cpu_activate(struct led_classdev *led)
{
       on_each_cpu(ledtrig_cpu_deactivate_cpu, led, 1);
}

Bryan can you test this change?

Thanks!
Linus Walleij
Bryan Wu Aug. 4, 2011, 9:18 a.m. UTC | #7
2011/8/3 Micha? Miros?aw <mirqus@gmail.com>:
> 2011/8/3 Bryan Wu <bryan.wu@canonical.com>:
>> Attempting to consolidate the ARM LED code, this removes the
>> custom RealView LED trigger code to turn LEDs on and off in
>> response to CPU activity and replace it with a standard trigger.
> [...]
>> +void ledtrig_cpu(enum cpu_led_event ledevt)
>> +{
>> +       struct ledtrig_cpu_data *trigdata = __get_cpu_var(ledtrig_cpu_triggers);
>> +
>> +       if (!trigdata)
>> +               return;
>> +
>> +       /* Locate the correct CPU LED */
>> +
>> +       switch (ledevt) {
>> +       case CPU_LED_IDLE_START:
>> +       case CPU_LED_START:
>> +               /* Will turn the LED on, max brightness */
>> +               if (trigdata->led)
>> +                       led_set_brightness(trigdata->led,
>> +                                          trigdata->led->max_brightness);
>> +               break;
>> +
>> +       case CPU_LED_IDLE_END:
>> +       case CPU_LED_STOP:
>> +       case CPU_LED_HALTED:
>> +               /* Will turn the LED off */
>> +               if (trigdata->led)
>> +                       led_set_brightness(trigdata->led, LED_OFF);
>> +               break;
>
> Might be better to swap CPU_LED_IDLE_START with CPU_LED_IDLE_END here
> so that when entering idle state the leds turn off.
>

Good point, I will change that.

Thanks,
Bryan Wu Aug. 5, 2011, 9:48 a.m. UTC | #8
On Thu, Aug 4, 2011 at 12:12 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Aug 3, 2011 at 5:30 PM, Jamie Iles <jamie@jamieiles.com> wrote:
>>> > So shouldn't this be doing a
>>> > __get_cpu_var(ledtrig_cpu_triggers) and a kfree() on that (and setting
>>> > to NULL)?
>>>
>>> You could do that but why? There is no way the pointer value
>>> can make any harm. If activate() is called again it will be overwritten
>>> with the new pointer.
>>
>> Yes, but can't ledtrig_cpu() potentially still be called after
>> deactivation, then the:
>>
>>        if (!trigdata)
>>                return;
>>
>> would be incorrect?  My main point though was that it looks like
>> led->trigger_data is the processor id and kfree() is being called on
>> that.
>
> You're right, it just works fine because trigdata is always NULL...
>
>>> > Also, where does led->trigger_data get assigned with the cpu id?
>>>
>>> The activate() function is called on every core and if the CPU ID
>>> is not equal to the assigned CPU it just exits, here:
>>>
>>> +     int my_cpu = smp_processor_id();
>>> +     unsigned long target_cpu = (unsigned long) led->trigger_data;
>>> +
>>> +     if (target_cpu != my_cpu)
>>> +             return;
>>
>> I understand that bit, but I can't see anything that actually assigns
>> led->trigger_data in the first place.
>
> You're right, it's because nothing really does. And since
> kfree(NULL); is fine this works, and makes the above thing work
> fine as well.
>
> Since led->trigger_data is never used we shouldn't touch it in
> I believe, Bryan can you just take it out and replace with something
> like this (totally untested):
>
> static void ledtrig_cpu_deactivate_cpu(struct led_classdev *led)
> {
>       struct ledtrig_cpu_data *cpu_data = __get_cpu_var(ledtrig_cpu_triggers);
>       int my_cpu = smp_processor_id();
>       unsigned long target_cpu = (unsigned long) led->trigger_data;
>
>       if (target_cpu != my_cpu)
>               return;
>
>       kfree(cpu_data);
>       __get_cpu_var(ledtrig_cpu_triggers) = NULL;
> }
>
> static void ledtrig_cpu_activate(struct led_classdev *led)
> {
>       on_each_cpu(ledtrig_cpu_deactivate_cpu, led, 1);
> }
>
> Bryan can you test this change?
>

No problem, I've reworked this patchset and included this updates.
Please find patches here:

git://kernel.ubuntu.com/roc/linux-2.6 leds
gitweb: http://kernel.ubuntu.com/git?p=roc/linux-2.6/.git;a=shortlog;h=refs/heads/leds

Please help to test on your hardware.

Thanks,
diff mbox

Patch

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index b591e72..a97c6a3 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -430,6 +430,16 @@  config LEDS_TRIGGER_BACKLIGHT
 
 	  If unsure, say N.
 
+config LEDS_TRIGGER_CPU
+	bool "LED CPU Trigger"
+	depends on LEDS_TRIGGERS
+	help
+	  This allows LEDs to be controlled by active CPUs. This shows
+	  the active CPUs across an array of LEDs so you can see what
+	  CPUs are active on the system at any given moment.
+
+	  If unsure, say N.
+
 config LEDS_TRIGGER_GPIO
 	tristate "LED GPIO Trigger"
 	depends on LEDS_TRIGGERS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index bbfd2e3..5e70d2e 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -53,4 +53,5 @@  obj-$(CONFIG_LEDS_TRIGGER_IDE_DISK)	+= ledtrig-ide-disk.o
 obj-$(CONFIG_LEDS_TRIGGER_HEARTBEAT)	+= ledtrig-heartbeat.o
 obj-$(CONFIG_LEDS_TRIGGER_BACKLIGHT)	+= ledtrig-backlight.o
 obj-$(CONFIG_LEDS_TRIGGER_GPIO)		+= ledtrig-gpio.o
+obj-$(CONFIG_LEDS_TRIGGER_CPU)		+= ledtrig-cpu.o
 obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON)	+= ledtrig-default-on.o
diff --git a/drivers/leds/ledtrig-cpu.c b/drivers/leds/ledtrig-cpu.c
new file mode 100644
index 0000000..0537c3b
--- /dev/null
+++ b/drivers/leds/ledtrig-cpu.c
@@ -0,0 +1,144 @@ 
+/*
+ * ledtrig-cpu.c - LED trigger based on CPU activity
+ *
+ * Copyright 2011 Linus Walleij <linus.walleij@linaro.org>
+ * Copyright 2011 Bryan Wu <bryan.wu@canonical.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/percpu.h>
+#include <linux/syscore_ops.h>
+#include "leds.h"
+
+struct ledtrig_cpu_data {
+	struct led_classdev *led;
+};
+
+static DEFINE_PER_CPU(struct ledtrig_cpu_data *, ledtrig_cpu_triggers);
+
+void ledtrig_cpu(enum cpu_led_event ledevt)
+{
+	struct ledtrig_cpu_data *trigdata = __get_cpu_var(ledtrig_cpu_triggers);
+
+	if (!trigdata)
+		return;
+
+	/* Locate the correct CPU LED */
+
+	switch (ledevt) {
+	case CPU_LED_IDLE_START:
+	case CPU_LED_START:
+		/* Will turn the LED on, max brightness */
+		if (trigdata->led)
+			led_set_brightness(trigdata->led,
+					   trigdata->led->max_brightness);
+		break;
+
+	case CPU_LED_IDLE_END:
+	case CPU_LED_STOP:
+	case CPU_LED_HALTED:
+		/* Will turn the LED off */
+		if (trigdata->led)
+			led_set_brightness(trigdata->led, LED_OFF);
+		break;
+
+	default:
+		/* Will leave the LED as it is */
+		break;
+	}
+}
+EXPORT_SYMBOL(ledtrig_cpu);
+
+static void ledtrig_cpu_activate_cpu(void *data)
+{
+	struct ledtrig_cpu_data *cpu_data;
+	struct led_classdev *led = data;
+	int my_cpu = smp_processor_id();
+	unsigned long target_cpu = (unsigned long) led->trigger_data;
+
+	if (target_cpu != my_cpu)
+		return;
+
+	cpu_data = kzalloc(sizeof(*cpu_data), GFP_KERNEL);
+	if (!cpu_data)
+		return;
+
+	dev_info(led->dev, "led %s indicate activity on CPU %d\n",
+		 led->name, my_cpu);
+
+	cpu_data->led = led;
+	__get_cpu_var(ledtrig_cpu_triggers) = cpu_data;
+}
+
+static void ledtrig_cpu_activate(struct led_classdev *led)
+{
+	on_each_cpu(ledtrig_cpu_activate_cpu, led, 1);
+}
+
+static void ledtrig_cpu_deactivate(struct led_classdev *led)
+{
+	struct ledtrig_cpu_data *cpu_data = led->trigger_data;
+
+	kfree(cpu_data);
+}
+
+static struct led_trigger ledtrig_cpu_led_trigger = {
+	.name		= "cpu",
+	.activate	= ledtrig_cpu_activate,
+	.deactivate	= ledtrig_cpu_deactivate,
+};
+
+
+static int ledtrig_cpu_syscore_suspend(void)
+{
+	ledtrig_cpu(CPU_LED_STOP);
+	return 0;
+}
+
+static void ledtrig_cpu_syscore_resume(void)
+{
+	ledtrig_cpu(CPU_LED_START);
+}
+
+static void ledtrig_cpu_syscore_shutdown(void)
+{
+	ledtrig_cpu(CPU_LED_HALTED);
+}
+
+static struct syscore_ops ledtrig_cpu_syscore_ops = {
+	.shutdown	= ledtrig_cpu_syscore_shutdown,
+	.suspend	= ledtrig_cpu_syscore_suspend,
+	.resume		= ledtrig_cpu_syscore_resume,
+};
+
+static int __init ledtrig_cpu_init(void)
+{
+	int ret;
+
+	ret = led_trigger_register(&ledtrig_cpu_led_trigger);
+	if (!ret)
+		register_syscore_ops(&ledtrig_cpu_syscore_ops);
+
+	return ret;
+}
+module_init(ledtrig_cpu_init);
+
+static void __exit ledtrig_cpu_exit(void)
+{
+	unregister_syscore_ops(&ledtrig_cpu_syscore_ops);
+	led_trigger_unregister(&ledtrig_cpu_led_trigger);
+}
+module_exit(ledtrig_cpu_exit);
+
+MODULE_AUTHOR("Linus Walleij <linus.walleij@linaro.org>");
+MODULE_AUTHOR("Bryan Wu <bryan.wu@canonical.com>");
+MODULE_DESCRIPTION("CPU LED trigger");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 5884def..b782abd 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -210,4 +210,19 @@  struct gpio_led_platform_data {
 struct platform_device *gpio_led_register_device(
 		int id, const struct gpio_led_platform_data *pdata);
 
+#ifdef CONFIG_LEDS_TRIGGER_CPU
+enum cpu_led_event {
+	CPU_LED_IDLE_START,	/* CPU enters idle */
+	CPU_LED_IDLE_END,	/* CPU idle ends */
+	CPU_LED_START,		/* Machine starts, especially resume */
+	CPU_LED_STOP,		/* Machine stops, especially suspend */
+	CPU_LED_HALTED,		/* Machine shutdown */
+};
+
+/* Use this routine to handle LEDs */
+extern void ledtrig_cpu(enum cpu_led_event evt);
+#else
+#define ledtrig_cpu(evt) do {} while (0)
+#endif
+
 #endif		/* __LINUX_LEDS_H_INCLUDED */