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

[Russell King - ARM Linux]

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.