diff mbox

[16/17] mach-sa1100: retire custom LED code

Message ID 1309955687-19365-17-git-send-email-bryan.wu@canonical.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bryan Wu July 6, 2011, 12:34 p.m. UTC 
Signed-off-by: Bryan Wu <bryan.wu@canonical.com>
---
 arch/arm/mach-sa1100/Kconfig        |   29 +++++++++
 arch/arm/mach-sa1100/Makefile       |   10 ---
 arch/arm/mach-sa1100/assabet.c      |   75 +++++++++++++++++++++++
 arch/arm/mach-sa1100/badge4.c       |   30 +++++++++
 arch/arm/mach-sa1100/cerf.c         |   42 +++++++++++++
 arch/arm/mach-sa1100/hackkit.c      |   32 ++++++++++
 arch/arm/mach-sa1100/lart.c         |   26 ++++++++
 arch/arm/mach-sa1100/leds-assabet.c |  114 -----------------------------------
 arch/arm/mach-sa1100/leds-badge4.c  |  111 ----------------------------------
 arch/arm/mach-sa1100/leds-cerf.c    |  110 ---------------------------------
 arch/arm/mach-sa1100/leds-hackkit.c |  112 ----------------------------------
 arch/arm/mach-sa1100/leds-lart.c    |  101 -------------------------------
 arch/arm/mach-sa1100/leds-simpad.c  |  100 ------------------------------
 arch/arm/mach-sa1100/leds.c         |   52 ----------------
 arch/arm/mach-sa1100/leds.h         |   14 ----
 arch/arm/mach-sa1100/simpad.c       |   48 +++++++++++++++
 16 files changed, 282 insertions(+), 724 deletions(-)
 delete mode 100644 arch/arm/mach-sa1100/leds-assabet.c
 delete mode 100644 arch/arm/mach-sa1100/leds-badge4.c
 delete mode 100644 arch/arm/mach-sa1100/leds-cerf.c
 delete mode 100644 arch/arm/mach-sa1100/leds-hackkit.c
 delete mode 100644 arch/arm/mach-sa1100/leds-lart.c
 delete mode 100644 arch/arm/mach-sa1100/leds-simpad.c
 delete mode 100644 arch/arm/mach-sa1100/leds.c
 delete mode 100644 arch/arm/mach-sa1100/leds.h

Comments

Jochen Friedrich July 6, 2011, 2:04 p.m. UTC | #1 
On 06.07.2011 14:34, Bryan Wu wrote:

> diff --git a/arch/arm/mach-sa1100/simpad.c b/arch/arm/mach-sa1100/simpad.c
> index cfb7607..6f87258 100644
> --- a/arch/arm/mach-sa1100/simpad.c
> +++ b/arch/arm/mach-sa1100/simpad.c
> @@ -13,6 +13,8 @@
>   #include<linux/mtd/mtd.h>
>   #include<linux/mtd/partitions.h>
>   #include<linux/io.h>
> +#include<linux/leds.h>
> +#include<linux/slab.h>
>
>   #include<asm/irq.h>
>   #include<mach/hardware.h>
> @@ -205,7 +207,53 @@ static struct platform_device *devices[] __initdata = {
>   	&simpad_mq200fb
>   };
>
> +/* LEDs */
> +#define LED_GREEN	1
>
> +static void simpad_led_set(struct led_classdev *cdev,
> +			      enum led_brightness b)
> +{
> +	if (b != LED_OFF)
> +		set_cs3_bit(LED_GREEN);
> +	else
> +		clear_cs3_bit(LED_GREEN);
> +}
> +
> +static enum led_brightness simpad_led_get(struct led_classdev *cdev)
> +{
> +	u32 reg = *(CS3BUSTYPE *)(CS3_BASE);
> +
> +	return (reg&  LED_GREEN) ? LED_FULL : LED_OFF;
> +}

NACK, bit 1 on the CS3 register is PCMCIA power control (write) and 
PCMCIA BVD1 (read), but not LED_GREEN. LED2_ON would be correct for 
write, but you would need to read cs3_shadow instead of the register 
itself for the LED status.

BTW: Some time ago I wrote a similar patch for ARM that converts CS3 
functions to GPIOs, wich simplifies the LED stuff even further ;-)

http://www.spinics.net/lists/arm-kernel/msg122479.html

Thanks,
Jochen
Bryan Wu July 7, 2011, 1:40 p.m. UTC | #2 
On Wed, Jul 6, 2011 at 10:04 PM, Jochen Friedrich <jochen@scram.de> wrote:
> On 06.07.2011 14:34, Bryan Wu wrote:
>
>> diff --git a/arch/arm/mach-sa1100/simpad.c b/arch/arm/mach-sa1100/simpad.c
>> index cfb7607..6f87258 100644
>> --- a/arch/arm/mach-sa1100/simpad.c
>> +++ b/arch/arm/mach-sa1100/simpad.c
>> @@ -13,6 +13,8 @@
>>  #include<linux/mtd/mtd.h>
>>  #include<linux/mtd/partitions.h>
>>  #include<linux/io.h>
>> +#include<linux/leds.h>
>> +#include<linux/slab.h>
>>
>>  #include<asm/irq.h>
>>  #include<mach/hardware.h>
>> @@ -205,7 +207,53 @@ static struct platform_device *devices[] __initdata =
>> {
>>        &simpad_mq200fb
>>  };
>>
>> +/* LEDs */
>> +#define LED_GREEN      1
>>
>> +static void simpad_led_set(struct led_classdev *cdev,
>> +                             enum led_brightness b)
>> +{
>> +       if (b != LED_OFF)
>> +               set_cs3_bit(LED_GREEN);
>> +       else
>> +               clear_cs3_bit(LED_GREEN);
>> +}
>> +
>> +static enum led_brightness simpad_led_get(struct led_classdev *cdev)
>> +{
>> +       u32 reg = *(CS3BUSTYPE *)(CS3_BASE);
>> +
>> +       return (reg&  LED_GREEN) ? LED_FULL : LED_OFF;
>> +}
>
> NACK, bit 1 on the CS3 register is PCMCIA power control (write) and PCMCIA
> BVD1 (read), but not LED_GREEN. LED2_ON would be correct for write, but you
> would need to read cs3_shadow instead of the register itself for the LED
> status.
>

Actually, I'm not familiar with this hardware in details. Thanks for
pointing out.

> BTW: Some time ago I wrote a similar patch for ARM that converts CS3
> functions to GPIOs, wich simplifies the LED stuff even further ;-)
>
> http://www.spinics.net/lists/arm-kernel/msg122479.html
>

This looks good and it's the right way to do that. We need gpiolib
supporting firstly. How's status of your patchset, I guess you're
trying to update it.

Thanks,
Bryan Wu July 8, 2011, 3 a.m. UTC | #3 
On Fri, Jul 8, 2011 at 12:23 AM, Jochen Friedrich <jochen@scram.de> wrote:
> Hi Bryan,
>
>> This looks good and it's the right way to do that. We need gpiolib
>> supporting firstly. How's status of your patchset, I guess you're
>> trying to update it.
>
> I've resubmitted it yesterday for another review. In parallel, I'll retest
> this on top of 3.0-rc5 on real hardware and ask Russell if it's OK to submit
> to his patch tracker.
>

Great, I will remove my modification of simpad code in my patchset.

Thanks,
Russell King - ARM Linux July 10, 2011, 9:24 a.m. UTC | #4 
On Wed, Jul 06, 2011 at 08:34:46PM +0800, Bryan Wu wrote:
> diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
> index 5778274..13e4e5d 100644
> --- a/arch/arm/mach-sa1100/assabet.c
> +++ b/arch/arm/mach-sa1100/assabet.c
> @@ -19,6 +19,8 @@
>  #include <linux/mtd/partitions.h>
>  #include <linux/delay.h>
>  #include <linux/mm.h>
> +#include <linux/leds.h>
> +#include <linux/slab.h>
>  
>  #include <mach/hardware.h>
>  #include <asm/mach-types.h>
> @@ -445,6 +447,79 @@ static void __init assabet_map_io(void)
>  	sa1100_register_uart(2, 3);
>  }
>  
> +/* LEDs */
> +struct assabet_led {
> +	struct led_classdev cdev;
> +	u32 mask;
> +};
> +
> +/*
> + * The triggers lines up below will only be used if the
> + * LED triggers are compiled in.
> + */
> +static const struct {
> +	const char *name;
> +	const char *trigger;
> +} assabet_leds[] = {
> +	{ "assabet:red", "cpu",},
> +	{ "assabet:green", "heartbeat", },
> +};
> +
> +static void assabet_led_set(struct led_classdev *cdev,
> +		enum led_brightness b)
> +{
> +	struct assabet_led *led = container_of(cdev,
> +			struct assabet_led, cdev);
> +
> +	if (b != LED_OFF)
> +		ASSABET_BCR_clear(led->mask);
> +	else
> +		ASSABET_BCR_set(led->mask);
> +}
> +
> +static enum led_brightness assabet_led_get(struct led_classdev *cdev)
> +{
> +	struct assabet_led *led = container_of(cdev,
> +			struct assabet_led, cdev);
> +
> +	return (ASSABET_BCR & led->mask) ? LED_OFF : LED_FULL;
> +}
> +
> +static int __init assabet_leds_init(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(assabet_leds); i++) {
> +		struct assabet_led *led;
> +
> +		led = kzalloc(sizeof(*led), GFP_KERNEL);
> +		if (!led)
> +			break;
> +
> +		led->cdev.name = assabet_leds[i].name;
> +		led->cdev.brightness_set = assabet_led_set;
> +		led->cdev.brightness_get = assabet_led_get;
> +		led->cdev.default_trigger = assabet_leds[i].trigger;
> +
> +		if (!i)
> +			led->mask = ASSABET_BCR_LED_RED;
> +		else
> +			led->mask = ASSABET_BCR_LED_GREEN;
> +
> +		if (led_classdev_register(NULL, &led->cdev) < 0) {
> +			kfree(led);
> +			break;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Since we may have triggers on any subsystem, defer registration
> + * until after subsystem_init.
> + */
> +fs_initcall(assabet_leds_init);

This is buggy.  init functions are called irrespective of what platform
we're running on, so this means if assabet is built in to the kernel,
then we'll register its LEDs and try to access the BCR register
irrespective of whether we are on assabet or not.

> diff --git a/arch/arm/mach-sa1100/simpad.c b/arch/arm/mach-sa1100/simpad.c
> index cfb7607..6f87258 100644
> --- a/arch/arm/mach-sa1100/simpad.c
> +++ b/arch/arm/mach-sa1100/simpad.c
> @@ -13,6 +13,8 @@
>  #include <linux/mtd/mtd.h>
>  #include <linux/mtd/partitions.h>
>  #include <linux/io.h>
> +#include <linux/leds.h>
> +#include <linux/slab.h>
>  
>  #include <asm/irq.h>
>  #include <mach/hardware.h>
> @@ -205,7 +207,53 @@ static struct platform_device *devices[] __initdata = {
>  	&simpad_mq200fb
>  };
>  
> +/* LEDs */
> +#define LED_GREEN	1
>  
> +static void simpad_led_set(struct led_classdev *cdev,
> +			      enum led_brightness b)
> +{
> +	if (b != LED_OFF)
> +		set_cs3_bit(LED_GREEN);
> +	else
> +		clear_cs3_bit(LED_GREEN);
> +}
> +
> +static enum led_brightness simpad_led_get(struct led_classdev *cdev)
> +{
> +	u32 reg = *(CS3BUSTYPE *)(CS3_BASE);
> +
> +	return (reg & LED_GREEN) ? LED_FULL : LED_OFF;
> +}
> +
> +static int __init simpad_leds_init(void)
> +{
> +	struct led_classdev *simpad_led;
> +	int ret;
> +
> +	simpad_led = kzalloc(sizeof(*simpad_led), GFP_KERNEL);
> +	if (!simpad_led)
> +		return -ENOMEM;
> +
> +	simpad_led->name = "simpad:green";
> +	simpad_led->brightness_set = simpad_led_set;
> +	simpad_led->brightness_get = simpad_led_get;
> +	simpad_led->default_trigger = "heartbeat";
> +
> +	ret = led_classdev_register(NULL, simpad_led);
> +	if (ret < 0) {
> +		kfree(simpad_led);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Since we may have triggers on any subsystem, defer registration
> + * until after subsystem_init.
> + */
> +fs_initcall(simpad_leds_init);

Looks like simpad is similarly afflicted.  I haven't gone through the
rest to check for similar issues.
Jochen Friedrich July 11, 2011, 8:28 a.m. UTC | #5 
Hi Russell,

>> +fs_initcall(simpad_leds_init);
>
> Looks like simpad is similarly afflicted.  I haven't gone through the
> rest to check for similar issues.

simpad is buggy in other means, as well (wrong bit, read of write-only 
register, missing locking). That's why I want to push my CS3 cleanup 
patch first, converting a custom IO to standard GPIO ;-)

Thanks,
Jochen
Bryan Wu July 11, 2011, 2:36 p.m. UTC | #6 
On Sun, Jul 10, 2011 at 5:24 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jul 06, 2011 at 08:34:46PM +0800, Bryan Wu wrote:
>> diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
>> index 5778274..13e4e5d 100644
>> --- a/arch/arm/mach-sa1100/assabet.c
>> +++ b/arch/arm/mach-sa1100/assabet.c
>> @@ -19,6 +19,8 @@
>>  #include <linux/mtd/partitions.h>
>>  #include <linux/delay.h>
>>  #include <linux/mm.h>
>> +#include <linux/leds.h>
>> +#include <linux/slab.h>
>>
>>  #include <mach/hardware.h>
>>  #include <asm/mach-types.h>
>> @@ -445,6 +447,79 @@ static void __init assabet_map_io(void)
>>       sa1100_register_uart(2, 3);
>>  }
>>
>> +/* LEDs */
>> +struct assabet_led {
>> +     struct led_classdev cdev;
>> +     u32 mask;
>> +};
>> +
>> +/*
>> + * The triggers lines up below will only be used if the
>> + * LED triggers are compiled in.
>> + */
>> +static const struct {
>> +     const char *name;
>> +     const char *trigger;
>> +} assabet_leds[] = {
>> +     { "assabet:red", "cpu",},
>> +     { "assabet:green", "heartbeat", },
>> +};
>> +
>> +static void assabet_led_set(struct led_classdev *cdev,
>> +             enum led_brightness b)
>> +{
>> +     struct assabet_led *led = container_of(cdev,
>> +                     struct assabet_led, cdev);
>> +
>> +     if (b != LED_OFF)
>> +             ASSABET_BCR_clear(led->mask);
>> +     else
>> +             ASSABET_BCR_set(led->mask);
>> +}
>> +
>> +static enum led_brightness assabet_led_get(struct led_classdev *cdev)
>> +{
>> +     struct assabet_led *led = container_of(cdev,
>> +                     struct assabet_led, cdev);
>> +
>> +     return (ASSABET_BCR & led->mask) ? LED_OFF : LED_FULL;
>> +}
>> +
>> +static int __init assabet_leds_init(void)
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < ARRAY_SIZE(assabet_leds); i++) {
>> +             struct assabet_led *led;
>> +
>> +             led = kzalloc(sizeof(*led), GFP_KERNEL);
>> +             if (!led)
>> +                     break;
>> +
>> +             led->cdev.name = assabet_leds[i].name;
>> +             led->cdev.brightness_set = assabet_led_set;
>> +             led->cdev.brightness_get = assabet_led_get;
>> +             led->cdev.default_trigger = assabet_leds[i].trigger;
>> +
>> +             if (!i)
>> +                     led->mask = ASSABET_BCR_LED_RED;
>> +             else
>> +                     led->mask = ASSABET_BCR_LED_GREEN;
>> +
>> +             if (led_classdev_register(NULL, &led->cdev) < 0) {
>> +                     kfree(led);
>> +                     break;
>> +             }
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +/*
>> + * Since we may have triggers on any subsystem, defer registration
>> + * until after subsystem_init.
>> + */
>> +fs_initcall(assabet_leds_init);
>
> This is buggy.  init functions are called irrespective of what platform
> we're running on, so this means if assabet is built in to the kernel,
> then we'll register its LEDs and try to access the BCR register
> irrespective of whether we are on assabet or not.
>

Right, I got it. So if we are going to run a single binary for all the
sa1100 machines, how about we check the machine in each fs_initcall

static int __init assabet_leds_init(void)
{
     int i;

     if (!machine_is_assabet())
          return -ENODEV,

     for (i = 0; i < ARRAY_SIZE(assabet_leds); i++) {
             struct assabet_led *led;

Thanks,

>> diff --git a/arch/arm/mach-sa1100/simpad.c b/arch/arm/mach-sa1100/simpad.c
>> index cfb7607..6f87258 100644
>> --- a/arch/arm/mach-sa1100/simpad.c
>> +++ b/arch/arm/mach-sa1100/simpad.c
>> @@ -13,6 +13,8 @@
>>  #include <linux/mtd/mtd.h>
>>  #include <linux/mtd/partitions.h>
>>  #include <linux/io.h>
>> +#include <linux/leds.h>
>> +#include <linux/slab.h>
>>
>>  #include <asm/irq.h>
>>  #include <mach/hardware.h>
>> @@ -205,7 +207,53 @@ static struct platform_device *devices[] __initdata = {
>>       &simpad_mq200fb
>>  };
>>
>> +/* LEDs */
>> +#define LED_GREEN    1
>>
>> +static void simpad_led_set(struct led_classdev *cdev,
>> +                           enum led_brightness b)
>> +{
>> +     if (b != LED_OFF)
>> +             set_cs3_bit(LED_GREEN);
>> +     else
>> +             clear_cs3_bit(LED_GREEN);
>> +}
>> +
>> +static enum led_brightness simpad_led_get(struct led_classdev *cdev)
>> +{
>> +     u32 reg = *(CS3BUSTYPE *)(CS3_BASE);
>> +
>> +     return (reg & LED_GREEN) ? LED_FULL : LED_OFF;
>> +}
>> +
>> +static int __init simpad_leds_init(void)
>> +{
>> +     struct led_classdev *simpad_led;
>> +     int ret;
>> +
>> +     simpad_led = kzalloc(sizeof(*simpad_led), GFP_KERNEL);
>> +     if (!simpad_led)
>> +             return -ENOMEM;
>> +
>> +     simpad_led->name = "simpad:green";
>> +     simpad_led->brightness_set = simpad_led_set;
>> +     simpad_led->brightness_get = simpad_led_get;
>> +     simpad_led->default_trigger = "heartbeat";
>> +
>> +     ret = led_classdev_register(NULL, simpad_led);
>> +     if (ret < 0) {
>> +             kfree(simpad_led);
>> +             return ret;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +/*
>> + * Since we may have triggers on any subsystem, defer registration
>> + * until after subsystem_init.
>> + */
>> +fs_initcall(simpad_leds_init);
>
> Looks like simpad is similarly afflicted.  I haven't gone through the
> rest to check for similar issues.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Bryan Wu July 11, 2011, 2:38 p.m. UTC | #7 
On Mon, Jul 11, 2011 at 4:28 PM, Jochen Friedrich <jochen@scram.de> wrote:
> Hi Russell,
>
>>> +fs_initcall(simpad_leds_init);
>>
>> Looks like simpad is similarly afflicted.  I haven't gone through the
>> rest to check for similar issues.
>
> simpad is buggy in other means, as well (wrong bit, read of write-only
> register, missing locking). That's why I want to push my CS3 cleanup patch
> first, converting a custom IO to standard GPIO ;-)
>

if you can use this GPIO based gpio_led driver, there is not such
fs_initcall issue.

Thanks,
Russell King - ARM Linux July 11, 2011, 9:09 p.m. UTC | #8 
On Mon, Jul 11, 2011 at 10:36:49PM +0800, Bryan Wu wrote:
> Right, I got it. So if we are going to run a single binary for all the
> sa1100 machines, how about we check the machine in each fs_initcall
> 
> static int __init assabet_leds_init(void)
> {
>      int i;
> 
>      if (!machine_is_assabet())
>           return -ENODEV,
> 
>      for (i = 0; i < ARRAY_SIZE(assabet_leds); i++) {
>              struct assabet_led *led;

Yes, that'll do, thanks.
Arnd Bergmann July 11, 2011, 9:52 p.m. UTC | #9 
On Monday 11 July 2011, Russell King - ARM Linux wrote:
> On Mon, Jul 11, 2011 at 10:36:49PM +0800, Bryan Wu wrote:
> > Right, I got it. So if we are going to run a single binary for all the
> > sa1100 machines, how about we check the machine in each fs_initcall
> > 
> > static int __init assabet_leds_init(void)
> > {
> >      int i;
> > 
> >      if (!machine_is_assabet())
> >           return -ENODEV,
> > 
> >      for (i = 0; i < ARRAY_SIZE(assabet_leds); i++) {
> >              struct assabet_led *led;
> 
> Yes, that'll do, thanks.
> 

Do you think it's worth copying the logic from powerpc for this?


arch/powerpc/include/asm/machdep.h:
#define __define_machine_initcall(mach,level,fn,id) \
        static int __init __machine_initcall_##mach##_##fn(void) { \
                if (machine_is(mach)) return fn(); \
                return 0; \
        } \
        __define_initcall(level,__machine_initcall_##mach##_##fn,id);
#define machine_core_initcall(mach,fn)          __define_machine_initcall(mach,"1",fn,1)
#define machine_core_initcall_sync(mach,fn)     __define_machine_initcall(mach,"1s",fn,1s)
#define machine_postcore_initcall(mach,fn)      __define_machine_initcall(mach,"2",fn,2)
#define machine_postcore_initcall_sync(mach,fn) __define_machine_initcall(mach,"2s",fn,2s)
#define machine_arch_initcall(mach,fn)          __define_machine_initcall(mach,"3",fn,3)
#define machine_arch_initcall_sync(mach,fn)     __define_machine_initcall(mach,"3s",fn,3s)
#define machine_subsys_initcall(mach,fn)        __define_machine_initcall(mach,"4",fn,4)
#define machine_subsys_initcall_sync(mach,fn)   __define_machine_initcall(mach,"4s",fn,4s)
#define machine_fs_initcall(mach,fn)            __define_machine_initcall(mach,"5",fn,5)
#define machine_fs_initcall_sync(mach,fn)       __define_machine_initcall(mach,"5s",fn,5s)
#define machine_rootfs_initcall(mach,fn)        __define_machine_initcall(mach,"rootfs",fn,rootfs)
#define machine_device_initcall(mach,fn)        __define_machine_initcall(mach,"6",fn,6)
#define machine_device_initcall_sync(mach,fn)   __define_machine_initcall(mach,"6s",fn,6s)
#define machine_late_initcall(mach,fn)          __define_machine_initcall(mach,"7",fn,7)
#define machine_late_initcall_sync(mach,fn)     __define_machine_initcall(mach,"7s",fn,7s)

I don't really like the magic macros, but my feeling is that it's easier
to do the conversion without introducing subtle bugs this way.

	Arnd
Bryan Wu July 12, 2011, 12:31 a.m. UTC | #10 
On Tue, Jul 12, 2011 at 5:52 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 11 July 2011, Russell King - ARM Linux wrote:
>> On Mon, Jul 11, 2011 at 10:36:49PM +0800, Bryan Wu wrote:
>> > Right, I got it. So if we are going to run a single binary for all the
>> > sa1100 machines, how about we check the machine in each fs_initcall
>> >
>> > static int __init assabet_leds_init(void)
>> > {
>> >      int i;
>> >
>> >      if (!machine_is_assabet())
>> >           return -ENODEV,
>> >
>> >      for (i = 0; i < ARRAY_SIZE(assabet_leds); i++) {
>> >              struct assabet_led *led;
>>
>> Yes, that'll do, thanks.
>>
>
> Do you think it's worth copying the logic from powerpc for this?
>
>
> arch/powerpc/include/asm/machdep.h:
> #define __define_machine_initcall(mach,level,fn,id) \
>        static int __init __machine_initcall_##mach##_##fn(void) { \
>                if (machine_is(mach)) return fn(); \
>                return 0; \
>        } \
>        __define_initcall(level,__machine_initcall_##mach##_##fn,id);
> #define machine_core_initcall(mach,fn)          __define_machine_initcall(mach,"1",fn,1)
> #define machine_core_initcall_sync(mach,fn)     __define_machine_initcall(mach,"1s",fn,1s)
> #define machine_postcore_initcall(mach,fn)      __define_machine_initcall(mach,"2",fn,2)
> #define machine_postcore_initcall_sync(mach,fn) __define_machine_initcall(mach,"2s",fn,2s)
> #define machine_arch_initcall(mach,fn)          __define_machine_initcall(mach,"3",fn,3)
> #define machine_arch_initcall_sync(mach,fn)     __define_machine_initcall(mach,"3s",fn,3s)
> #define machine_subsys_initcall(mach,fn)        __define_machine_initcall(mach,"4",fn,4)
> #define machine_subsys_initcall_sync(mach,fn)   __define_machine_initcall(mach,"4s",fn,4s)
> #define machine_fs_initcall(mach,fn)            __define_machine_initcall(mach,"5",fn,5)
> #define machine_fs_initcall_sync(mach,fn)       __define_machine_initcall(mach,"5s",fn,5s)
> #define machine_rootfs_initcall(mach,fn)        __define_machine_initcall(mach,"rootfs",fn,rootfs)
> #define machine_device_initcall(mach,fn)        __define_machine_initcall(mach,"6",fn,6)
> #define machine_device_initcall_sync(mach,fn)   __define_machine_initcall(mach,"6s",fn,6s)
> #define machine_late_initcall(mach,fn)          __define_machine_initcall(mach,"7",fn,7)
> #define machine_late_initcall_sync(mach,fn)     __define_machine_initcall(mach,"7s",fn,7s)
>
> I don't really like the magic macros, but my feeling is that it's easier
> to do the conversion without introducing subtle bugs this way.
>

I think this is a great idea. what about moving it to include/asm-generic/

Thanks,
Russell King - ARM Linux July 12, 2011, 7:30 a.m. UTC | #11 
On Tue, Jul 12, 2011 at 08:31:28AM +0800, Bryan Wu wrote:
> I think this is a great idea. what about moving it to include/asm-generic/

Except, instead of:

		if (machine_is(mach)) return fn(); \

we'd need:

		if (machine_is_##mach()) return fn(); \
Arnd Bergmann July 12, 2011, 12:52 p.m. UTC | #12 
On Tuesday 12 July 2011, Russell King - ARM Linux wrote:
> On Tue, Jul 12, 2011 at 08:31:28AM +0800, Bryan Wu wrote:
> > I think this is a great idea. what about moving it to include/asm-generic/

I wouldn't want to encourage new architectures to actually use this method
until they grow to need it. For most simple architectures, it's probably
better to have a different method of doing machine specific work.

> Except, instead of:
> 
> 		if (machine_is(mach)) return fn(); \
> 
> we'd need:
> 
> 		if (machine_is_##mach()) return fn(); \

Right. Maybe we can start simple and just do a single definition at fs_initcall level:

#define machine_initcall(mach,level,fn) \
        static int __init __machine_initcall_##mach##_##fn(void) { \
                if (machine_is ## mach()) return fn(); \
                return 0; \
        } \
        fs_initcall(,__machine_initcall_##mach##_##fn);


	Arnd
diff mbox

Patch

diff --git a/arch/arm/mach-sa1100/Kconfig b/arch/arm/mach-sa1100/Kconfig
index 42625e4..266fce9 100644
--- a/arch/arm/mach-sa1100/Kconfig
+++ b/arch/arm/mach-sa1100/Kconfig
@@ -5,6 +5,11 @@  menu "SA11x0 Implementations"
 config SA1100_ASSABET
 	bool "Assabet"
 	select CPU_FREQ_SA1110
+	select NEW_LEDS
+	select LEDS_CLASS
+	select LEDS_TRIGGERS
+	select LEDS_TRIGGER_CPU
+	select LEDS_TRIGGER_HEARTBEAT
 	help
 	  Say Y here if you are using the Intel(R) StrongARM(R) SA-1110
 	  Microprocessor Development Board (also known as the Assabet).
@@ -21,6 +26,12 @@  config ASSABET_NEPONSET
 config SA1100_CERF
 	bool "CerfBoard"
 	select CPU_FREQ_SA1110
+	select NEW_LEDS
+	select LEDS_CLASS
+	select LEDS_TRIGGERS
+	select LEDS_TRIGGER_CPU
+	select LEDS_TRIGGER_HEARTBEAT
+	select LEDS_TRIGGER_DEFAULT_ON
 	help
 	  The Intrinsyc CerfBoard is based on the StrongARM 1110 (Discontinued).
 	  More information is available at:
@@ -80,6 +91,11 @@  config SA1100_BADGE4
 	bool "HP Labs BadgePAD 4"
 	select SA1111
 	select CPU_FREQ_SA1100
+	select NEW_LEDS
+	select LEDS_CLASS
+	select LEDS_TRIGGERS
+	select LEDS_TRIGGER_CPU
+	select LEDS_TRIGGER_HEARTBEAT
 	help
 	  Say Y here if you want to build a kernel for the HP Laboratories
 	  BadgePAD 4.
@@ -106,6 +122,11 @@  config SA1100_JORNADA720_SSP
 config SA1100_HACKKIT
 	bool "HackKit Core CPU Board"
 	select CPU_FREQ_SA1100
+	select NEW_LEDS
+	select LEDS_CLASS
+	select LEDS_TRIGGERS
+	select LEDS_TRIGGER_CPU
+	select LEDS_TRIGGER_HEARTBEAT
 	help
 	  Say Y here to support the HackKit Core CPU Board
 	  <http://hackkit.eletztrick.de>;
@@ -113,6 +134,10 @@  config SA1100_HACKKIT
 config SA1100_LART
 	bool "LART"
 	select CPU_FREQ_SA1100
+	select NEW_LEDS
+	select LEDS_CLASS
+	select LEDS_TRIGGERS
+	select LEDS_TRIGGER_CPU
 	help
 	  Say Y here if you are using the Linux Advanced Radio Terminal
 	  (also known as the LART).  See <http://www.lartmaker.nl/> for
@@ -149,6 +174,10 @@  config SA1100_SHANNON
 config SA1100_SIMPAD
 	bool "Simpad"
 	select CPU_FREQ_SA1110
+	select NEW_LEDS
+	select LEDS_CLASS
+	select LEDS_TRIGGERS
+	select LEDS_TRIGGER_HEARTBEAT
 	help
 	  The SIEMENS webpad SIMpad is based on the StrongARM 1110. There
 	  are two different versions CL4 and SL4. CL4 has 32MB RAM and 16MB
diff --git a/arch/arm/mach-sa1100/Makefile b/arch/arm/mach-sa1100/Makefile
index 41252d2..5f5f491 100644
--- a/arch/arm/mach-sa1100/Makefile
+++ b/arch/arm/mach-sa1100/Makefile
@@ -7,21 +7,17 @@  obj-y := clock.o generic.o gpio.o irq.o dma.o time.o #nmi-oopser.o
 obj-m :=
 obj-n :=
 obj-  :=
-led-y := leds.o
 
 obj-$(CONFIG_CPU_FREQ_SA1100)		+= cpu-sa1100.o
 obj-$(CONFIG_CPU_FREQ_SA1110)		+= cpu-sa1110.o
 
 # Specific board support
 obj-$(CONFIG_SA1100_ASSABET)		+= assabet.o
-led-$(CONFIG_SA1100_ASSABET)		+= leds-assabet.o
 obj-$(CONFIG_ASSABET_NEPONSET)		+= neponset.o
 
 obj-$(CONFIG_SA1100_BADGE4)		+= badge4.o
-led-$(CONFIG_SA1100_BADGE4)		+= leds-badge4.o
 
 obj-$(CONFIG_SA1100_CERF)		+= cerf.o
-led-$(CONFIG_SA1100_CERF)		+= leds-cerf.o
 
 obj-$(CONFIG_SA1100_COLLIE)		+= collie.o
 
@@ -29,13 +25,11 @@  obj-$(CONFIG_SA1100_H3100)		+= h3100.o h3xxx.o
 obj-$(CONFIG_SA1100_H3600)		+= h3600.o h3xxx.o
 
 obj-$(CONFIG_SA1100_HACKKIT)		+= hackkit.o
-led-$(CONFIG_SA1100_HACKKIT)		+= leds-hackkit.o
 
 obj-$(CONFIG_SA1100_JORNADA720)		+= jornada720.o
 obj-$(CONFIG_SA1100_JORNADA720_SSP)	+= jornada720_ssp.o
 
 obj-$(CONFIG_SA1100_LART)		+= lart.o
-led-$(CONFIG_SA1100_LART)		+= leds-lart.o
 
 obj-$(CONFIG_SA1100_NANOENGINE)		+= nanoengine.o
 obj-$(CONFIG_PCI_NANOENGINE)		+= pci-nanoengine.o
@@ -45,10 +39,6 @@  obj-$(CONFIG_SA1100_PLEB)		+= pleb.o
 obj-$(CONFIG_SA1100_SHANNON)		+= shannon.o
 
 obj-$(CONFIG_SA1100_SIMPAD)		+= simpad.o
-led-$(CONFIG_SA1100_SIMPAD)		+= leds-simpad.o
-
-# LEDs support
-obj-$(CONFIG_LEDS) += $(led-y)
 
 # Miscellaneous functions
 obj-$(CONFIG_PM)			+= pm.o sleep.o
diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
index 5778274..13e4e5d 100644
--- a/arch/arm/mach-sa1100/assabet.c
+++ b/arch/arm/mach-sa1100/assabet.c
@@ -19,6 +19,8 @@ 
 #include <linux/mtd/partitions.h>
 #include <linux/delay.h>
 #include <linux/mm.h>
+#include <linux/leds.h>
+#include <linux/slab.h>
 
 #include <mach/hardware.h>
 #include <asm/mach-types.h>
@@ -445,6 +447,79 @@  static void __init assabet_map_io(void)
 	sa1100_register_uart(2, 3);
 }
 
+/* LEDs */
+struct assabet_led {
+	struct led_classdev cdev;
+	u32 mask;
+};
+
+/*
+ * The triggers lines up below will only be used if the
+ * LED triggers are compiled in.
+ */
+static const struct {
+	const char *name;
+	const char *trigger;
+} assabet_leds[] = {
+	{ "assabet:red", "cpu",},
+	{ "assabet:green", "heartbeat", },
+};
+
+static void assabet_led_set(struct led_classdev *cdev,
+		enum led_brightness b)
+{
+	struct assabet_led *led = container_of(cdev,
+			struct assabet_led, cdev);
+
+	if (b != LED_OFF)
+		ASSABET_BCR_clear(led->mask);
+	else
+		ASSABET_BCR_set(led->mask);
+}
+
+static enum led_brightness assabet_led_get(struct led_classdev *cdev)
+{
+	struct assabet_led *led = container_of(cdev,
+			struct assabet_led, cdev);
+
+	return (ASSABET_BCR & led->mask) ? LED_OFF : LED_FULL;
+}
+
+static int __init assabet_leds_init(void)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(assabet_leds); i++) {
+		struct assabet_led *led;
+
+		led = kzalloc(sizeof(*led), GFP_KERNEL);
+		if (!led)
+			break;
+
+		led->cdev.name = assabet_leds[i].name;
+		led->cdev.brightness_set = assabet_led_set;
+		led->cdev.brightness_get = assabet_led_get;
+		led->cdev.default_trigger = assabet_leds[i].trigger;
+
+		if (!i)
+			led->mask = ASSABET_BCR_LED_RED;
+		else
+			led->mask = ASSABET_BCR_LED_GREEN;
+
+		if (led_classdev_register(NULL, &led->cdev) < 0) {
+			kfree(led);
+			break;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * Since we may have triggers on any subsystem, defer registration
+ * until after subsystem_init.
+ */
+fs_initcall(assabet_leds_init);
 
 MACHINE_START(ASSABET, "Intel-Assabet")
 	.boot_params	= 0xc0000100,
diff --git a/arch/arm/mach-sa1100/badge4.c b/arch/arm/mach-sa1100/badge4.c
index 4f19ff8..0711c30 100644
--- a/arch/arm/mach-sa1100/badge4.c
+++ b/arch/arm/mach-sa1100/badge4.c
@@ -22,6 +22,8 @@ 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 #include <linux/errno.h>
+#include <linux/gpio.h>
+#include <linux/leds.h>
 
 #include <mach/hardware.h>
 #include <asm/mach-types.h>
@@ -69,8 +71,36 @@  static struct platform_device sa1111_device = {
 	.resource	= sa1111_resources,
 };
 
+/* LEDs */
+struct gpio_led badge4_gpio_leds[] = {
+	{
+		.name			= "badge4:red",
+		.default_trigger	= "heartbeat",
+		.gpio			= 7,
+	},
+	{
+		.name			= "badge4:green",
+		.default_trigger	= "cpu",
+		.gpio			= 9,
+	},
+};
+
+static struct gpio_led_platform_data badge4_gpio_led_info = {
+	.leds		= badge4_gpio_leds,
+	.num_leds	= ARRAY_SIZE(badge4_gpio_leds),
+};
+
+static struct platform_device badge4_leds = {
+	.name	= "leds-gpio",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &badge4_gpio_led_info,
+	}
+};
+
 static struct platform_device *devices[] __initdata = {
 	&sa1111_device,
+	&badge4_leds;
 };
 
 static int __init badge4_sa1111_init(void)
diff --git a/arch/arm/mach-sa1100/cerf.c b/arch/arm/mach-sa1100/cerf.c
index 7f3da4b..29c099d 100644
--- a/arch/arm/mach-sa1100/cerf.c
+++ b/arch/arm/mach-sa1100/cerf.c
@@ -17,6 +17,8 @@ 
 #include <linux/irq.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
+#include <linux/gpio.h>
+#include <linux/leds.h>
 
 #include <asm/irq.h>
 #include <mach/hardware.h>
@@ -47,8 +49,48 @@  static struct platform_device cerfuart2_device = {
 	.resource	= cerfuart2_resources,
 };
 
+/* LEDs */
+struct gpio_led cerf_gpio_leds[] = {
+	{
+		.name			= "cerf:d0",
+		.default_trigger	= "heartbeat",
+		.gpio			= 0,
+	},
+	{
+		.name			= "cerf:d1",
+		.default_trigger	= "cpu",
+		.gpio			= 1,
+	},
+	{
+		.name			= "cerf:d2",
+		.default_trigger	= "default-on",
+		.gpio			= 2,
+	},
+	{
+		.name			= "cerf:d3",
+		.default_trigger	= "default-on",
+		.gpio			= 3,
+	},
+
+};
+
+static struct gpio_led_platform_data cerf_gpio_led_info = {
+	.leds		= cerf_gpio_leds,
+	.num_leds	= ARRAY_SIZE(cerf_gpio_leds),
+};
+
+static struct platform_device cerf_leds = {
+	.name	= "leds-gpio",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &cerf_gpio_led_info,
+	}
+};
+
+
 static struct platform_device *cerf_devices[] __initdata = {
 	&cerfuart2_device,
+	&cerf_leds,
 };
 
 #ifdef CONFIG_SA1100_CERF_FLASH_32MB
diff --git a/arch/arm/mach-sa1100/hackkit.c b/arch/arm/mach-sa1100/hackkit.c
index db5e434..1661f2e 100644
--- a/arch/arm/mach-sa1100/hackkit.c
+++ b/arch/arm/mach-sa1100/hackkit.c
@@ -21,6 +21,10 @@ 
 #include <linux/serial_core.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
+#include <linux/tty.h>
+#include <linux/gpio.h>
+#include <linux/leds.h>
+#include <linux/platform_device.h>
 
 #include <mach/hardware.h>
 #include <asm/mach-types.h>
@@ -185,9 +189,37 @@  static struct resource hackkit_flash_resource = {
 	.flags		= IORESOURCE_MEM,
 };
 
+/* LEDs */
+struct gpio_led hackkit_gpio_leds[] = {
+	{
+		.name			= "hackkit:red",
+		.default_trigger	= "cpu",
+		.gpio			= 22,
+	},
+	{
+		.name			= "hackkit:green",
+		.default_trigger	= "heartbeat",
+		.gpio			= 23,
+	},
+};
+
+static struct gpio_led_platform_data hackkit_gpio_led_info = {
+	.leds		= hackkit_gpio_leds,
+	.num_leds	= ARRAY_SIZE(hackkit_gpio_leds),
+};
+
+static struct platform_device hackkit_leds = {
+	.name	= "leds-gpio",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &hackkit_gpio_led_info,
+	}
+};
+
 static void __init hackkit_init(void)
 {
 	sa11x0_register_mtd(&hackkit_flash_data, &hackkit_flash_resource, 1);
+	platform_device_register(&hackkit_leds);
 }
 
 /**********************************************************************
diff --git a/arch/arm/mach-sa1100/lart.c b/arch/arm/mach-sa1100/lart.c
index 7b9556b..1678fa5 100644
--- a/arch/arm/mach-sa1100/lart.c
+++ b/arch/arm/mach-sa1100/lart.c
@@ -5,6 +5,9 @@ 
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/tty.h>
+#include <linux/gpio.h>
+#include <linux/leds.h>
+#include <linux/platform_device.h>
 
 #include <mach/hardware.h>
 #include <asm/setup.h>
@@ -45,6 +48,27 @@  static struct map_desc lart_io_desc[] __initdata = {
 	}
 };
 
+/* LEDs */
+struct gpio_led lart_gpio_leds[] = {
+	{
+		.name			= "lart:red",
+		.default_trigger	= "cpu",
+		.gpio			= 23,
+	},
+};
+
+static struct gpio_led_platform_data lart_gpio_led_info = {
+	.leds		= lart_gpio_leds,
+	.num_leds	= ARRAY_SIZE(lart_gpio_leds),
+};
+
+static struct platform_device lart_leds = {
+	.name	= "leds-gpio",
+	.id	= -1,
+	.dev	= {
+		.platform_data	= &lart_gpio_led_info,
+	}
+};
 static void __init lart_map_io(void)
 {
 	sa1100_map_io();
@@ -58,6 +82,8 @@  static void __init lart_map_io(void)
 	GPDR |= GPIO_UART_TXD;
 	GPDR &= ~GPIO_UART_RXD;
 	PPAR |= PPAR_UPR;
+
+	platform_device_register(&lart_leds);
 }
 
 MACHINE_START(LART, "LART")
diff --git a/arch/arm/mach-sa1100/leds-assabet.c b/arch/arm/mach-sa1100/leds-assabet.c
deleted file mode 100644
index 64e9b4b..0000000
--- a/arch/arm/mach-sa1100/leds-assabet.c
+++ /dev/null
@@ -1,114 +0,0 @@ 
-/*
- * linux/arch/arm/mach-sa1100/leds-assabet.c
- *
- * Copyright (C) 2000 John Dorsey <john+@cs.cmu.edu>
- *
- * Original (leds-footbridge.c) by Russell King
- *
- * Assabet uses the LEDs as follows:
- *   - Green - toggles state every 50 timer interrupts
- *   - Red   - on if system is not idle
- */
-#include <linux/init.h>
-
-#include <mach/hardware.h>
-#include <asm/leds.h>
-#include <asm/system.h>
-#include <mach/assabet.h>
-
-#include "leds.h"
-
-
-#define LED_STATE_ENABLED	1
-#define LED_STATE_CLAIMED	2
-
-static unsigned int led_state;
-static unsigned int hw_led_state;
-
-#define ASSABET_BCR_LED_MASK	(ASSABET_BCR_LED_GREEN | ASSABET_BCR_LED_RED)
-
-void assabet_leds_event(led_event_t evt)
-{
-	unsigned long flags;
-
-	local_irq_save(flags);
-
-	switch (evt) {
-	case led_start:
-		hw_led_state = ASSABET_BCR_LED_RED | ASSABET_BCR_LED_GREEN;
-		led_state = LED_STATE_ENABLED;
-		break;
-
-	case led_stop:
-		led_state &= ~LED_STATE_ENABLED;
-		hw_led_state = ASSABET_BCR_LED_RED | ASSABET_BCR_LED_GREEN;
-		ASSABET_BCR_frob(ASSABET_BCR_LED_MASK, hw_led_state);
-		break;
-
-	case led_claim:
-		led_state |= LED_STATE_CLAIMED;
-		hw_led_state = ASSABET_BCR_LED_RED | ASSABET_BCR_LED_GREEN;
-		break;
-
-	case led_release:
-		led_state &= ~LED_STATE_CLAIMED;
-		hw_led_state = ASSABET_BCR_LED_RED | ASSABET_BCR_LED_GREEN;
-		break;
-
-#ifdef CONFIG_LEDS_TIMER
-	case led_timer:
-		if (!(led_state & LED_STATE_CLAIMED))
-			hw_led_state ^= ASSABET_BCR_LED_GREEN;
-		break;
-#endif
-
-#ifdef CONFIG_LEDS_CPU
-	case led_idle_start:
-		if (!(led_state & LED_STATE_CLAIMED))
-			hw_led_state |= ASSABET_BCR_LED_RED;
-		break;
-
-	case led_idle_end:
-		if (!(led_state & LED_STATE_CLAIMED))
-			hw_led_state &= ~ASSABET_BCR_LED_RED;
-		break;
-#endif
-
-	case led_halted:
-		break;
-
-	case led_green_on:
-		if (led_state & LED_STATE_CLAIMED)
-			hw_led_state &= ~ASSABET_BCR_LED_GREEN;
-		break;
-
-	case led_green_off:
-		if (led_state & LED_STATE_CLAIMED)
-			hw_led_state |= ASSABET_BCR_LED_GREEN;
-		break;
-
-	case led_amber_on:
-		break;
-
-	case led_amber_off:
-		break;
-
-	case led_red_on:
-		if (led_state & LED_STATE_CLAIMED)
-			hw_led_state &= ~ASSABET_BCR_LED_RED;
-		break;
-
-	case led_red_off:
-		if (led_state & LED_STATE_CLAIMED)
-			hw_led_state |= ASSABET_BCR_LED_RED;
-		break;
-
-	default:
-		break;
-	}
-
-	if  (led_state & LED_STATE_ENABLED)
-		ASSABET_BCR_frob(ASSABET_BCR_LED_MASK, hw_led_state);
-
-	local_irq_restore(flags);
-}
diff --git a/arch/arm/mach-sa1100/leds-badge4.c b/arch/arm/mach-sa1100/leds-badge4.c
deleted file mode 100644
index cf1e384..0000000
--- a/arch/arm/mach-sa1100/leds-badge4.c
+++ /dev/null
@@ -1,111 +0,0 @@ 
-/*
- * linux/arch/arm/mach-sa1100/leds-badge4.c
- *
- * Author: Christopher Hoover <ch@hpl.hp.com>
- * Copyright (C) 2002 Hewlett-Packard Company
- *
- * 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/init.h>
-
-#include <mach/hardware.h>
-#include <asm/leds.h>
-#include <asm/system.h>
-
-#include "leds.h"
-
-#define LED_STATE_ENABLED	1
-#define LED_STATE_CLAIMED	2
-
-static unsigned int led_state;
-static unsigned int hw_led_state;
-
-#define LED_RED		GPIO_GPIO(7)
-#define LED_GREEN       GPIO_GPIO(9)
-#define LED_MASK	(LED_RED|LED_GREEN)
-
-#define LED_IDLE	LED_GREEN
-#define LED_TIMER	LED_RED
-
-void badge4_leds_event(led_event_t evt)
-{
-        unsigned long flags;
-
-	local_irq_save(flags);
-
-        switch (evt) {
-        case led_start:
-		GPDR |= LED_MASK;
-                hw_led_state = LED_MASK;
-                led_state = LED_STATE_ENABLED;
-                break;
-
-        case led_stop:
-                led_state &= ~LED_STATE_ENABLED;
-                break;
-
-        case led_claim:
-                led_state |= LED_STATE_CLAIMED;
-                hw_led_state = LED_MASK;
-                break;
-
-        case led_release:
-                led_state &= ~LED_STATE_CLAIMED;
-                hw_led_state = LED_MASK;
-                break;
-
-#ifdef CONFIG_LEDS_TIMER
-        case led_timer:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state ^= LED_TIMER;
-                break;
-#endif
-
-#ifdef CONFIG_LEDS_CPU
-        case led_idle_start:
-		/* LED off when system is idle */
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state &= ~LED_IDLE;
-                break;
-
-        case led_idle_end:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state |= LED_IDLE;
-                break;
-#endif
-
-        case led_red_on:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state &= ~LED_RED;
-                break;
-
-        case led_red_off:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state |= LED_RED;
-                break;
-
-        case led_green_on:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state &= ~LED_GREEN;
-                break;
-
-        case led_green_off:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state |= LED_GREEN;
-                break;
-
-	default:
-		break;
-        }
-
-        if  (led_state & LED_STATE_ENABLED) {
-                GPSR = hw_led_state;
-                GPCR = hw_led_state ^ LED_MASK;
-        }
-
-	local_irq_restore(flags);
-}
diff --git a/arch/arm/mach-sa1100/leds-cerf.c b/arch/arm/mach-sa1100/leds-cerf.c
deleted file mode 100644
index 259b48e..0000000
--- a/arch/arm/mach-sa1100/leds-cerf.c
+++ /dev/null
@@ -1,110 +0,0 @@ 
-/*
- * linux/arch/arm/mach-sa1100/leds-cerf.c
- *
- * Author: ???
- */
-#include <linux/init.h>
-
-#include <mach/hardware.h>
-#include <asm/leds.h>
-#include <asm/system.h>
-
-#include "leds.h"
-
-
-#define LED_STATE_ENABLED	1
-#define LED_STATE_CLAIMED	2
-
-static unsigned int led_state;
-static unsigned int hw_led_state;
-
-#define LED_D0          GPIO_GPIO(0)
-#define LED_D1          GPIO_GPIO(1)
-#define LED_D2          GPIO_GPIO(2)
-#define LED_D3          GPIO_GPIO(3)
-#define LED_MASK        (LED_D0|LED_D1|LED_D2|LED_D3)
-
-void cerf_leds_event(led_event_t evt)
-{
-        unsigned long flags;
-
-	local_irq_save(flags);
-
-        switch (evt) {
-        case led_start:
-                hw_led_state = LED_MASK;
-                led_state = LED_STATE_ENABLED;
-                break;
-
-        case led_stop:
-                led_state &= ~LED_STATE_ENABLED;
-                break;
-
-        case led_claim:
-                led_state |= LED_STATE_CLAIMED;
-                hw_led_state = LED_MASK;
-                break;
-        case led_release:
-                led_state &= ~LED_STATE_CLAIMED;
-                hw_led_state = LED_MASK;
-                break;
-
-#ifdef CONFIG_LEDS_TIMER
-        case led_timer:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state ^= LED_D0;
-                break;
-#endif
-
-#ifdef CONFIG_LEDS_CPU
-        case led_idle_start:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state &= ~LED_D1;
-                break;
-
-        case led_idle_end:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state |= LED_D1;
-                break;
-#endif
-        case led_green_on:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state &= ~LED_D2;
-                break;
-
-        case led_green_off:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state |= LED_D2;
-                break;
-
-        case led_amber_on:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state &= ~LED_D3;
-                break;
-
-        case led_amber_off:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state |= LED_D3;
-                break;
-
-        case led_red_on:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state &= ~LED_D1;
-                break;
-
-        case led_red_off:
-                if (!(led_state & LED_STATE_CLAIMED))
-                        hw_led_state |= LED_D1;
-                break;
-
-        default:
-                break;
-        }
-
-        if  (led_state & LED_STATE_ENABLED) {
-                GPSR = hw_led_state;
-                GPCR = hw_led_state ^ LED_MASK;
-        }
-
-	local_irq_restore(flags);
-}
diff --git a/arch/arm/mach-sa1100/leds-hackkit.c b/arch/arm/mach-sa1100/leds-hackkit.c
deleted file mode 100644
index 2bce137..0000000
--- a/arch/arm/mach-sa1100/leds-hackkit.c
+++ /dev/null
@@ -1,112 +0,0 @@ 
-/*
- * linux/arch/arm/mach-sa1100/leds-hackkit.c
- *
- * based on leds-lart.c
- *
- * (C) Erik Mouw (J.A.K.Mouw@its.tudelft.nl), April 21, 2000
- * (C) Stefan Eletzhofer <stefan.eletzhofer@eletztrick.de>, 2002
- *
- * The HackKit has two leds (GPIO 22/23). The red led (gpio 22) is used
- * as cpu led, the green one is used as timer led.
- */
-#include <linux/init.h>
-
-#include <mach/hardware.h>
-#include <asm/leds.h>
-#include <asm/system.h>
-
-#include "leds.h"
-
-
-#define LED_STATE_ENABLED	1
-#define LED_STATE_CLAIMED	2
-
-static unsigned int led_state;
-static unsigned int hw_led_state;
-
-#define LED_GREEN    GPIO_GPIO23
-#define LED_RED    GPIO_GPIO22
-#define LED_MASK  (LED_RED | LED_GREEN)
-
-void hackkit_leds_event(led_event_t evt)
-{
-	unsigned long flags;
-
-	local_irq_save(flags);
-
-	switch(evt) {
-		case led_start:
-			/* pin 22/23 are outputs */
-			GPDR |= LED_MASK;
-			hw_led_state = LED_MASK;
-			led_state = LED_STATE_ENABLED;
-			break;
-
-		case led_stop:
-			led_state &= ~LED_STATE_ENABLED;
-			break;
-
-		case led_claim:
-			led_state |= LED_STATE_CLAIMED;
-			hw_led_state = LED_MASK;
-			break;
-
-		case led_release:
-			led_state &= ~LED_STATE_CLAIMED;
-			hw_led_state = LED_MASK;
-			break;
-
-#ifdef CONFIG_LEDS_TIMER
-		case led_timer:
-			if (!(led_state & LED_STATE_CLAIMED))
-				hw_led_state ^= LED_GREEN;
-			break;
-#endif
-
-#ifdef CONFIG_LEDS_CPU
-		case led_idle_start:
-			/* The LART people like the LED to be off when the
-			   system is idle... */
-			if (!(led_state & LED_STATE_CLAIMED))
-				hw_led_state &= ~LED_RED;
-			break;
-
-		case led_idle_end:
-			/* ... and on if the system is not idle */
-			if (!(led_state & LED_STATE_CLAIMED))
-				hw_led_state |= LED_RED;
-			break;
-#endif
-
-		case led_red_on:
-			if (led_state & LED_STATE_CLAIMED)
-				hw_led_state &= ~LED_RED;
-			break;
-
-		case led_red_off:
-			if (led_state & LED_STATE_CLAIMED)
-				hw_led_state |= LED_RED;
-			break;
-
-		case led_green_on:
-			if (led_state & LED_STATE_CLAIMED)
-				hw_led_state &= ~LED_GREEN;
-			break;
-
-		case led_green_off:
-			if (led_state & LED_STATE_CLAIMED)
-				hw_led_state |= LED_GREEN;
-			break;
-
-		default:
-			break;
-	}
-
-	/* Now set the GPIO state, or nothing will happen at all */
-	if (led_state & LED_STATE_ENABLED) {
-		GPSR = hw_led_state;
-		GPCR = hw_led_state ^ LED_MASK;
-	}
-
-	local_irq_restore(flags);
-}
diff --git a/arch/arm/mach-sa1100/leds-lart.c b/arch/arm/mach-sa1100/leds-lart.c
deleted file mode 100644
index 0505a1f..0000000
--- a/arch/arm/mach-sa1100/leds-lart.c
+++ /dev/null
@@ -1,101 +0,0 @@ 
-/*
- * linux/arch/arm/mach-sa1100/leds-lart.c
- *
- * (C) Erik Mouw (J.A.K.Mouw@its.tudelft.nl), April 21, 2000
- *
- * LART uses the LED as follows:
- *   - GPIO23 is the LED, on if system is not idle
- *  You can use both CONFIG_LEDS_CPU and CONFIG_LEDS_TIMER at the same
- *  time, but in that case the timer events will still dictate the
- *  pace of the LED.
- */
-#include <linux/init.h>
-
-#include <mach/hardware.h>
-#include <asm/leds.h>
-#include <asm/system.h>
-
-#include "leds.h"
-
-
-#define LED_STATE_ENABLED	1
-#define LED_STATE_CLAIMED	2
-
-static unsigned int led_state;
-static unsigned int hw_led_state;
-
-#define LED_23    GPIO_GPIO23
-#define LED_MASK  (LED_23)
-
-void lart_leds_event(led_event_t evt)
-{
-	unsigned long flags;
-
-	local_irq_save(flags);
-
-	switch(evt) {
-	case led_start:
-		/* pin 23 is output pin */
-		GPDR |= LED_23;
-		hw_led_state = LED_MASK;
-		led_state = LED_STATE_ENABLED;
-		break;
-
-	case led_stop:
-		led_state &= ~LED_STATE_ENABLED;
-		break;
-
-	case led_claim:
-		led_state |= LED_STATE_CLAIMED;
-		hw_led_state = LED_MASK;
-		break;
-
-	case led_release:
-		led_state &= ~LED_STATE_CLAIMED;
-		hw_led_state = LED_MASK;
-		break;
-
-#ifdef CONFIG_LEDS_TIMER
-	case led_timer:
-		if (!(led_state & LED_STATE_CLAIMED))
-			hw_led_state ^= LED_23;
-		break;
-#endif
-
-#ifdef CONFIG_LEDS_CPU
-	case led_idle_start:
-		/* The LART people like the LED to be off when the
-                   system is idle... */
-		if (!(led_state & LED_STATE_CLAIMED))
-			hw_led_state &= ~LED_23;
-		break;
-
-	case led_idle_end:
-		/* ... and on if the system is not idle */
-		if (!(led_state & LED_STATE_CLAIMED))
-			hw_led_state |= LED_23;
-		break;
-#endif
-
-	case led_red_on:
-		if (led_state & LED_STATE_CLAIMED)
-			hw_led_state &= ~LED_23;
-		break;
-
-	case led_red_off:
-		if (led_state & LED_STATE_CLAIMED)
-			hw_led_state |= LED_23;
-		break;
-
-	default:
-		break;
-	}
-
-	/* Now set the GPIO state, or nothing will happen at all */
-	if (led_state & LED_STATE_ENABLED) {
-		GPSR = hw_led_state;
-		GPCR = hw_led_state ^ LED_MASK;
-	}
-
-	local_irq_restore(flags);
-}
diff --git a/arch/arm/mach-sa1100/leds-simpad.c b/arch/arm/mach-sa1100/leds-simpad.c
deleted file mode 100644
index d50f4ee..0000000
--- a/arch/arm/mach-sa1100/leds-simpad.c
+++ /dev/null
@@ -1,100 +0,0 @@ 
-/*
- * linux/arch/arm/mach-sa1100/leds-simpad.c
- *
- * Author: Juergen Messerer <juergen.messerer@siemens.ch>
- */
-#include <linux/init.h>
-
-#include <mach/hardware.h>
-#include <asm/leds.h>
-#include <asm/system.h>
-#include <mach/simpad.h>
-
-#include "leds.h"
-
-
-#define LED_STATE_ENABLED	1
-#define LED_STATE_CLAIMED	2
-
-static unsigned int led_state;
-static unsigned int hw_led_state;
-
-#define	LED_GREEN	(1)
-#define	LED_MASK	(1)
-
-extern void set_cs3_bit(int value);
-extern void clear_cs3_bit(int value);     
-
-void simpad_leds_event(led_event_t evt)
-{
-	switch (evt)
-	{
-	case led_start:
-	        hw_led_state = LED_GREEN;
-		led_state = LED_STATE_ENABLED;
-		break;
-
-	case led_stop:
-		led_state &= ~LED_STATE_ENABLED;
-		break;
-
-	case led_claim:
-		led_state |= LED_STATE_CLAIMED;
-		hw_led_state = LED_GREEN;
-		break;
-
-	case led_release:
-		led_state &= ~LED_STATE_CLAIMED;
-		hw_led_state = LED_GREEN;
-		break;
-
-#ifdef CONFIG_LEDS_TIMER
-	case led_timer:
-		if (!(led_state & LED_STATE_CLAIMED))
-			hw_led_state ^= LED_GREEN;
-		break;
-#endif
-
-#ifdef CONFIG_LEDS_CPU
-	case led_idle_start:
-		break;
-
-	case led_idle_end:
-		break;
-#endif
-
-	case led_halted:
-		break;
-
-	case led_green_on:
-		if (led_state & LED_STATE_CLAIMED)
-			hw_led_state |= LED_GREEN;
-		break;
-
-	case led_green_off:
-		if (led_state & LED_STATE_CLAIMED)
-			hw_led_state &= ~LED_GREEN;
-		break;
-
-	case led_amber_on:
-		break;
-
-	case led_amber_off:
-		break;
-
-	case led_red_on:
-		break;
-
-	case led_red_off:
-		break;
-
-	default:
-		break;
-	}
-
-	if  (led_state & LED_STATE_ENABLED)
-		set_cs3_bit(LED2_ON);
-	else 
-	        clear_cs3_bit(LED2_ON);
-}
-
diff --git a/arch/arm/mach-sa1100/leds.c b/arch/arm/mach-sa1100/leds.c
deleted file mode 100644
index bbfe197..0000000
--- a/arch/arm/mach-sa1100/leds.c
+++ /dev/null
@@ -1,52 +0,0 @@ 
-/*
- * linux/arch/arm/mach-sa1100/leds.c
- *
- * SA1100 LEDs dispatcher
- *
- * Copyright (C) 2001 Nicolas Pitre
- */
-#include <linux/compiler.h>
-#include <linux/init.h>
-
-#include <asm/leds.h>
-#include <asm/mach-types.h>
-
-#include "leds.h"
-
-static int __init
-sa1100_leds_init(void)
-{
-	if (machine_is_assabet())
-		leds_event = assabet_leds_event;
-	if (machine_is_consus())
-		leds_event = consus_leds_event;
-	if (machine_is_badge4())
-		leds_event = badge4_leds_event;
-	if (machine_is_brutus())
-		leds_event = brutus_leds_event;
-	if (machine_is_cerf())
-		leds_event = cerf_leds_event;
-	if (machine_is_flexanet())
-		leds_event = flexanet_leds_event;
-	if (machine_is_graphicsclient())
-		leds_event = graphicsclient_leds_event;
-	if (machine_is_hackkit())
-		leds_event = hackkit_leds_event;
-	if (machine_is_lart())
-		leds_event = lart_leds_event;
-	if (machine_is_pfs168())
-		leds_event = pfs168_leds_event;
-	if (machine_is_graphicsmaster())
-		leds_event = graphicsmaster_leds_event;
-	if (machine_is_adsbitsy())
-		leds_event = adsbitsy_leds_event;
-	if (machine_is_pt_system3())
-		leds_event = system3_leds_event;
-	if (machine_is_simpad())
-		leds_event = simpad_leds_event; /* what about machine registry? including led, apm... -zecke */
-
-	leds_event(led_start);
-	return 0;
-}
-
-core_initcall(sa1100_leds_init);
diff --git a/arch/arm/mach-sa1100/leds.h b/arch/arm/mach-sa1100/leds.h
deleted file mode 100644
index 68cc9f7..0000000
--- a/arch/arm/mach-sa1100/leds.h
+++ /dev/null
@@ -1,14 +0,0 @@ 
-extern void assabet_leds_event(led_event_t evt);
-extern void badge4_leds_event(led_event_t evt);
-extern void consus_leds_event(led_event_t evt);
-extern void brutus_leds_event(led_event_t evt);
-extern void cerf_leds_event(led_event_t evt);
-extern void flexanet_leds_event(led_event_t evt);
-extern void graphicsclient_leds_event(led_event_t evt);
-extern void hackkit_leds_event(led_event_t evt);
-extern void lart_leds_event(led_event_t evt);
-extern void pfs168_leds_event(led_event_t evt);
-extern void graphicsmaster_leds_event(led_event_t evt);
-extern void adsbitsy_leds_event(led_event_t evt);
-extern void system3_leds_event(led_event_t evt);
-extern void simpad_leds_event(led_event_t evt);
diff --git a/arch/arm/mach-sa1100/simpad.c b/arch/arm/mach-sa1100/simpad.c
index cfb7607..6f87258 100644
--- a/arch/arm/mach-sa1100/simpad.c
+++ b/arch/arm/mach-sa1100/simpad.c
@@ -13,6 +13,8 @@ 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 #include <linux/io.h>
+#include <linux/leds.h>
+#include <linux/slab.h>
 
 #include <asm/irq.h>
 #include <mach/hardware.h>
@@ -205,7 +207,53 @@  static struct platform_device *devices[] __initdata = {
 	&simpad_mq200fb
 };
 
+/* LEDs */
+#define LED_GREEN	1
 
+static void simpad_led_set(struct led_classdev *cdev,
+			      enum led_brightness b)
+{
+	if (b != LED_OFF)
+		set_cs3_bit(LED_GREEN);
+	else
+		clear_cs3_bit(LED_GREEN);
+}
+
+static enum led_brightness simpad_led_get(struct led_classdev *cdev)
+{
+	u32 reg = *(CS3BUSTYPE *)(CS3_BASE);
+
+	return (reg & LED_GREEN) ? LED_FULL : LED_OFF;
+}
+
+static int __init simpad_leds_init(void)
+{
+	struct led_classdev *simpad_led;
+	int ret;
+
+	simpad_led = kzalloc(sizeof(*simpad_led), GFP_KERNEL);
+	if (!simpad_led)
+		return -ENOMEM;
+
+	simpad_led->name = "simpad:green";
+	simpad_led->brightness_set = simpad_led_set;
+	simpad_led->brightness_get = simpad_led_get;
+	simpad_led->default_trigger = "heartbeat";
+
+	ret = led_classdev_register(NULL, simpad_led);
+	if (ret < 0) {
+		kfree(simpad_led);
+		return ret;
+	}
+
+	return 0;
+}
+
+/*
+ * Since we may have triggers on any subsystem, defer registration
+ * until after subsystem_init.
+ */
+fs_initcall(simpad_leds_init);
 
 static int __init simpad_init(void)
 {