linux-kernel - Re: [PATCH 3/4] leds: Add option to have GPIO LEDs start on

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fa686aa40810241659n7aa71870x6da2819b308ee5af@mail.gmail.com>
Date:	Fri, 24 Oct 2008 17:59:40 -0600
From:	"Grant Likely" <grant.likely@...retlab.ca>
To:	"Trent Piepho" <tpiepho@...escale.com>
Cc:	linux-kernel@...r.kernel.org, linuxppc-dev@...abs.org,
	"Anton Vorontsov" <avorontsov@...mvista.com>,
	"Richard Purdie" <rpurdie@...ys.net>,
	"Sean MacLennan" <smaclennan@...atech.com>,
	"Wolfram Sang" <w.sang@...gutronix.de>
Subject: Re: [PATCH 3/4] leds: Add option to have GPIO LEDs start on

On Fri, Oct 24, 2008 at 5:09 PM, Trent Piepho <tpiepho@...escale.com> wrote:
> Yes, there is the "default-on" trigger but there are problems with that.
[...]
> The platform device binding gains a field in the platform data "default_state"
> that controls this.  The OpenFirmware binding uses a property named
> "default-state" that can be set to "on" or "off".  The default the property
> isn't present is off.

This look much preferred to setting the the default-on trigger.  Why
not remove the default-on trigger entirely from the binding
documentation so there is no confusion?

Also, my preference would be an empty "led-default-on" property
instead of "default-state" with a value that needs to be parsed, but
I'm not concerned about it enough to argue.

Otherwise:

Acked-by: Grant Likely <grant.likely@...retlab.ca>

>
> Signed-off-by: Trent Piepho <tpiepho@...escale.com>
> ---
>  Documentation/powerpc/dts-bindings/gpio/led.txt |    7 +++++++
>  drivers/leds/leds-gpio.c                        |    8 ++++++--
>  include/linux/leds.h                            |    1 +
>  3 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt b/Documentation/powerpc/dts-bindings/gpio/led.txt
> index 9f969c2..544ded7 100644
> --- a/Documentation/powerpc/dts-bindings/gpio/led.txt
> +++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
> @@ -20,6 +20,11 @@ LED sub-node properties:
>     "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" and "off".  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 default is off if this property is not present.
>
>  Examples:
>
> @@ -36,8 +41,10 @@ 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 f41b841..0dbad87 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -92,9 +92,10 @@ 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;
> +       led_dat->cdev.brightness = template->default_state ? LED_FULL : LED_OFF;
>
> -       gpio_direction_output(led_dat->gpio, led_dat->active_low);
> +       gpio_direction_output(led_dat->gpio,
> +                             led_dat->active_low ^ template->default_state);
>
>        INIT_WORK(&led_dat->work, gpio_led_work);
>
> @@ -256,12 +257,15 @@ static int __devinit of_gpio_leds_probe(struct of_device *ofdev,
>        memset(&led, 0, sizeof(led));
>        for_each_child_of_node(np, child) {
>                unsigned int flags;
> +               const char *state;
>
>                led.gpio = of_get_gpio(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);
> +               led.default_state = state && !strcmp(state, "on");
>
>                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 d3a73f5..caa3987 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -138,6 +138,7 @@ struct gpio_led {
>        const char *default_trigger;
>        unsigned        gpio;
>        u8              active_low;
> +       u8              default_state;
>  };
>
>  struct gpio_led_platform_data {
> --
> 1.5.4.3
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/