linux-kernel - Re: [PATCH 4/4] leds: Let GPIO LEDs keep their current state

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: <1226933427.17109.15.camel@ted>
Date:	Mon, 17 Nov 2008 14:50:27 +0000
From:	Richard Purdie <rpurdie@...ys.net>
To:	Trent Piepho <tpiepho@...escale.com>
Cc:	linux-kernel@...r.kernel.org, linuxppc-dev@...abs.org,
	Anton Vorontsov <avorontsov@...mvista.com>,
	Sean MacLennan <smaclennan@...atech.com>,
	Wolfram Sang <w.sang@...gutronix.de>,
	Grant Likely <grant.likely@...retlab.ca>
Subject: Re: [PATCH 4/4] leds: Let GPIO LEDs keep their current state

On Fri, 2008-10-24 at 16:09 -0700, Trent Piepho wrote:
> 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 data for the platform device binding gets a new field
> 'keep_state' which turns this on.  keep_state overrides default_state.
> 
> For the OpenFirmware bindings, the "default-state" property gains a new
> allowable setting "keep".
> 
> Signed-off-by: Trent Piepho <tpiepho@...escale.com>
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index 0dbad87..bb2a234 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -92,10 +92,13 @@ 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 = template->default_state ? LED_FULL : LED_OFF;
> +	if (template->keep_state)
> +		state = !!gpio_get_value(led_dat->gpio) ^ led_dat->active_low;
> +	else
> +		state = template->default_state;
> +	led_dat->cdev.brightness = state ? LED_FULL : LED_OFF;
>  
> -	gpio_direction_output(led_dat->gpio,
> -	                      led_dat->active_low ^ template->default_state);
> +	gpio_direction_output(led_dat->gpio, led_dat->active_low ^ state);
>  
>  	INIT_WORK(&led_dat->work, gpio_led_work);
>  
> @@ -266,6 +269,7 @@ static int __devinit of_gpio_leds_probe(struct of_device *ofdev,
>  			of_get_property(child, "linux,default-trigger", NULL);
>  		state = of_get_property(child, "default-state", NULL);
>  		led.default_state = state && !strcmp(state, "on");
> +		led.keep_state = state && !strcmp(state, "keep");
>  
>  		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 caa3987..c51b625 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -138,7 +138,8 @@ struct gpio_led {
>  	const char *default_trigger;
>  	unsigned 	gpio;
>  	u8 		active_low;
> -	u8		default_state;
> +	u8		default_state;	/* 0 = off, 1 = on */
> +	u8		keep_state; /* overrides default_state */
>  };

How about something simpler here, just make default state have three
different values - "keep", "on" and "off"? I'm not keen on having two
different state variables like this.

Regards,

Richard



--
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/