leds: Add options to have GPIO LEDs start on or keep their state

There already is a "default-on" trigger but there are problems with it.

For one, it's a inefficient way to do it and requires led trigger support
to be compiled in.

But the real reason is that is produces a glitch on the LED.  The GPIO is
allocate with the LED *off*, then *later* when the trigger runs it is
turned back on.  If the LED was already on via the GPIO's reset default or
action of the firmware, this produces a glitch where the LED goes from on
to off to on.  While normally this is fast enough that it wouldn't be
noticeable to a human observer, there are still serious problems.

One is that there may be something else on the GPIO line, like a hardware
alarm or watchdog, that is fast enough to notice the glitch.

Another is that the kernel may panic before the LED is turned back on, thus
hanging with the LED in the wrong state.  This is not just speculation, but
actually happened to me with an embedded system that has an LED which
should turn off when the kernel finishes booting, which was left in the
incorrect state due to a bug in the OF LED binding code.

We also let GPIO LEDs get their initial value from whatever the current
state of the GPIO line is.  On some systems the LEDs are put into some
state by the firmware or hardware before Linux boots, and it is desired to
have them keep this state which is otherwise unknown to Linux.

This requires that the underlying GPIO driver support reading the value of
output GPIOs.  Some drivers support this and some do not.

The platform device binding gains a field in the platform data
"default_state" that controls this.  There are three constants defined to
select from on, off, or keeping the current state.  The OpenFirmware
binding uses a property named "default-state" that can be set to "on",
"off", or "keep".  The default if the property isn't present is off.

Signed-off-by: Trent Piepho <xyzzy@speakeasy.org>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
Acked-by: Wolfram Sang <w.sang@pengutronix.de>
Acked-by: Sean MacLennan <smaclennan@pikatech.com>
Signed-off-by: Richard Purdie <rpurdie@linux.intel.com>

diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt b/Documentation/powerpc/dts-bindings/gpio/led.txt
index 4fe14deedc0a3e5aa3988288873c5e4f9a2ee01c..064db928c3c12cc3d6141664e57d25e7050520ed 100644 (file)
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -16,10 +16,17 @@ LED sub-node properties:
   string defining the trigger assigned to the LED.  Current triggers are:
     "backlight" - LED will act as a back-light, controlled by the framebuffer
                  system
-    "default-on" - LED will turn on
+    "default-on" - LED will turn on, but see "default-state" below
     "heartbeat" - LED "double" flashes at a load average based rate
     "ide-disk" - LED indicates disk activity
     "timer" - LED flashes at a fixed, configurable rate
+- default-state:  (optional) The initial state of the LED.  Valid
+  values are "on", "off", and "keep".  If the LED is already on or off
+  and the default-state property is set the to same value, then no
+  glitch should be produced where the LED momentarily turns off (or
+  on).  The "keep" setting will keep the LED at whatever its current
+  state is, without producing a glitch.  The default is off if this
+  property is not present.
 
 Examples:
 
@@ -30,14 +37,22 @@ leds {
                gpios = <&mcu_pio 0 1>; /* Active low */
                linux,default-trigger = "ide-disk";
        };
+
+       fault {
+               gpios = <&mcu_pio 1 0>;
+               /* Keep LED on if BIOS detected hardware fault */
+               default-state = "keep";
+       };
 };
 
 run-control {
        compatible = "gpio-leds";
        red {
                gpios = <&mpc8572 6 0>;
+               default-state = "off";
        };
        green {
                gpios = <&mpc8572 7 0>;
+               default-state = "on";
        };
 }

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 76895e691042d212770fb77755c81b9bb87a7303..6b06638eb5b42f40ca9c858a4e5d1626c07a0688 100644 (file)
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -76,7 +76,7 @@ static int __devinit create_gpio_led(const struct gpio_led *template,
        struct gpio_led_data *led_dat, struct device *parent,
        int (*blink_set)(unsigned, unsigned long *, unsigned long *))
 {
-       int ret;
+       int ret, state;
 
        /* skip leds that aren't available */
        if (!gpio_is_valid(template->gpio)) {
@@ -99,11 +99,15 @@ static int __devinit create_gpio_led(const struct gpio_led *template,
                led_dat->cdev.blink_set = gpio_blink_set;
        }
        led_dat->cdev.brightness_set = gpio_led_set;
-       led_dat->cdev.brightness = LED_OFF;
+       if (template->default_state == LEDS_GPIO_DEFSTATE_KEEP)
+               state = !!gpio_get_value(led_dat->gpio) ^ led_dat->active_low;
+       else
+               state = (template->default_state == LEDS_GPIO_DEFSTATE_ON);
+       led_dat->cdev.brightness = state ? LED_FULL : LED_OFF;
        if (!template->retain_state_suspended)
                led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
 
-       ret = gpio_direction_output(led_dat->gpio, led_dat->active_low);
+       ret = gpio_direction_output(led_dat->gpio, led_dat->active_low ^ state);
        if (ret < 0)
                goto err;
 
@@ -223,12 +227,22 @@ static int __devinit of_gpio_leds_probe(struct of_device *ofdev,
        memset(&led, 0, sizeof(led));
        for_each_child_of_node(np, child) {
                enum of_gpio_flags flags;
+               const char *state;
 
                led.gpio = of_get_gpio_flags(child, 0, &flags);
                led.active_low = flags & OF_GPIO_ACTIVE_LOW;
                led.name = of_get_property(child, "label", NULL) ? : child->name;
                led.default_trigger =
                        of_get_property(child, "linux,default-trigger", NULL);
+               state = of_get_property(child, "default-state", NULL);
+               if (state) {
+                       if (!strcmp(state, "keep"))
+                               led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
+                       else if(!strcmp(state, "on"))
+                               led.default_state = LEDS_GPIO_DEFSTATE_ON;
+                       else
+                               led.default_state = LEDS_GPIO_DEFSTATE_OFF;
+               }
 
                ret = create_gpio_led(&led, &pdata->led_data[pdata->num_leds++],
                                      &ofdev->dev, NULL);

diff --git a/include/linux/leds.h b/include/linux/leds.h
index c7f0b148df06053b8f089ddd3c5c0bcb71f9f6bb..62af62915cf750a561e5231b0cb04b9a25f72e5d 100644 (file)
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -143,9 +143,14 @@ struct gpio_led {
        const char *name;
        const char *default_trigger;
        unsigned        gpio;
-       u8              active_low : 1;
-       u8              retain_state_suspended : 1;
+       unsigned        active_low : 1;
+       unsigned        retain_state_suspended : 1;
+       unsigned        default_state : 2;
+       /* default_state should be one of LEDS_GPIO_DEFSTATE_(ON|OFF|KEEP) */
 };
+#define LEDS_GPIO_DEFSTATE_OFF 0
+#define LEDS_GPIO_DEFSTATE_ON  1
+#define LEDS_GPIO_DEFSTATE_KEEP        2
 
 struct gpio_led_platform_data {
        int             num_leds;