2008-06-28 15:32:28

by Martin Michlmayr

[permalink] [raw]
Subject: LEDs: how to handle gpios that control brightness for all LEDs

Hi Richard,

I have a device that has various gpios that correspond to the colours
(blue, red) of various LEDs. They only accept 0/1, not the actual
brightness. However, there are two other gpios that can be used to
control the brightness (of all LEDs): gpio V_LED 5V is bright, gpio
V_LED 3.3V is dimmed and if neither of these two gpios are set, all
LEDs stay off.

Can I simply export these brightness gpios to userland or would it be
better to handle the brightness setting in the kernel so that setting
e.g. 100 to a LED would automatically set the V_LED 3.3V gpio and 255
would set V_LED 5V. If so, how should this be done? If I can simply
export them to userland, would mv2120:led:dimmed and mv2120:led:bright
be appropriate names?

Second, the LEDS class sets all gpios to off by default. This is bad
in my case because if neither of the brightness gpios (3.3V or 5V) are
on, the SATA lights won't be shown either. Is there a way to specify
in the platform code that one gpio should be turned on by default?

Thanks.


For the reference, here is my current work-in-progress patch (I use
numbers rather than #defines for the gpio values because there's a
table listing all gpios in the source file).

--- a/arch/arm/mach-orion5x/mv2120-setup.c
+++ b/arch/arm/mach-orion5x/mv2120-setup.c
@@ -137,6 +137,49 @@ static struct i2c_board_info __initdata mv2120_i2c_rtc = {
.irq = 0,
};

+static struct gpio_led mv2120_led_pins[] = {
+ {
+ .name = "mv2120:blue:sys",
+ .gpio = 0,
+ },
+ {
+ .name = "mv2120:red:sys",
+ .gpio = 1,
+ },
+ {
+ .name = "mv2120:led5v",
+ .gpio = 4,
+ },
+ {
+ .name = "mv2120:led3.3v",
+ .gpio = 5,
+ },
+ {
+ .name = "mv2120:red:sata0",
+ .gpio = 8,
+ .active_low = 1,
+ },
+ {
+ .name = "mv2120:red:sata1",
+ .gpio = 9,
+ .active_low = 1,
+ },
+
+};
+
+static struct gpio_led_platform_data mv2120_led_data = {
+ .leds = mv2120_led_pins,
+ .num_leds = ARRAY_SIZE(mv2120_led_pins),
+};
+
+static struct platform_device mv2120_leds = {
+ .name = "leds-gpio",
+ .id = -1,
+ .dev = {
+ .platform_data = &mv2120_led_data,
+ }
+};
+
static void mv2120_power_off(void)
{
pr_info("%s: triggering power-off...\n", __func__);
@@ -172,6 +215,7 @@ static void __init mv2120_init(void)
gpio_free(MV2120_GPIO_RTC_IRQ);
}
i2c_register_board_info(0, &mv2120_i2c_rtc, 1);
+ platform_device_register(&mv2120_leds);

/* register mv2120 specific power-off method */
if (gpio_request(MV2120_GPIO_POWER_OFF, "POWEROFF") != 0 ||

--
Martin Michlmayr
http://www.cyrius.com/


2008-06-28 18:14:38

by Riku Voipio

[permalink] [raw]
Subject: Re: LEDs: how to handle gpios that control brightness for all LEDs

On Sat, Jun 28, 2008 at 05:31:09PM +0200, Martin Michlmayr wrote:
> I have a device that has various gpios that correspond to the colours
> (blue, red) of various LEDs. They only accept 0/1, not the actual
> brightness. However, there are two other gpios that can be used to
> control the brightness (of all LEDs): gpio V_LED 5V is bright, gpio
> V_LED 3.3V is dimmed and if neither of these two gpios are set, all
> LEDs stay off.

> Can I simply export these brightness gpios to userland or would it be
> better to handle the brightness setting in the kernel so that setting
> e.g. 100 to a LED would automatically set the V_LED 3.3V gpio and 255
> would set V_LED 5V. If so, how should this be done?

There is a enum led_brightness, which defines LED_HALF and LED_FULL. You
would need a custom brightness function. Leds-gpio driver would need to
be extended for it. The main problem with this approach is the
unexpected effect for enduser, that adjusting brighntess of one led
affects all of them.

> Second, the LEDS class sets all gpios to off by default. This is bad
> in my case because if neither of the brightness gpios (3.3V or 5V) are
> on, the SATA lights won't be shown either. Is there a way to specify
> in the platform code that one gpio should be turned on by default?

default-on trigger is probably the easiest way.

--
"rm -rf" only sounds scary if you don't have backups


Attachments:
(No filename) (1.37 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2008-06-29 20:00:11

by Martin Michlmayr

[permalink] [raw]
Subject: Re: LEDs: how to handle gpios that control brightness for all LEDs

* Riku Voipio <[email protected]> [2008-06-28 20:52]:
> > I have a device that has various gpios that correspond to the colours
> > (blue, red) of various LEDs. They only accept 0/1, not the actual
> > brightness. However, there are two other gpios that can be used to
> > control the brightness (of all LEDs): gpio V_LED 5V is bright, gpio
> > V_LED 3.3V is dimmed and if neither of these two gpios are set, all
> > LEDs stay off.
>
> > Can I simply export these brightness gpios to userland or would it be
> > better to handle the brightness setting in the kernel so that setting
> > e.g. 100 to a LED would automatically set the V_LED 3.3V gpio and 255
> > would set V_LED 5V. If so, how should this be done?
>
> There is a enum led_brightness, which defines LED_HALF and LED_FULL. You
> would need a custom brightness function. Leds-gpio driver would need to
> be extended for it. The main problem with this approach is the
> unexpected effect for enduser, that adjusting brighntess of one led
> affects all of them.

OK, this sounds to me like I should simply export the gpios that
influence the brightness.

> default-on trigger is probably the easiest way.

This works for me, thanks.

New patch below. Any comments Richard?


diff --git a/arch/arm/mach-orion5x/mv2120-setup.c b/arch/arm/mach-orion5x/mv2120-setup.c
index d838bdb..0ac5209 100644
--- a/arch/arm/mach-orion5x/mv2120-setup.c
+++ b/arch/arm/mach-orion5x/mv2120-setup.c
@@ -137,6 +137,50 @@ static struct i2c_board_info __initdata mv2120_i2c_rtc = {
.irq = 0,
};

+static struct gpio_led mv2120_led_pins[] = {
+ {
+ .name = "mv2120:blue:health",
+ .gpio = 0,
+ },
+ {
+ .name = "mv2120:red:health",
+ .gpio = 1,
+ },
+ {
+ .name = "mv2120:led:bright",
+ .gpio = 4,
+ .default_trigger = "default-on",
+ },
+ {
+ .name = "mv2120:led:dimmed",
+ .gpio = 5,
+ },
+ {
+ .name = "mv2120:red:sata0",
+ .gpio = 8,
+ .active_low = 1,
+ },
+ {
+ .name = "mv2120:red:sata1",
+ .gpio = 9,
+ .active_low = 1,
+ },
+
+};
+
+static struct gpio_led_platform_data mv2120_led_data = {
+ .leds = mv2120_led_pins,
+ .num_leds = ARRAY_SIZE(mv2120_led_pins),
+};
+
+static struct platform_device mv2120_leds = {
+ .name = "leds-gpio",
+ .id = -1,
+ .dev = {
+ .platform_data = &mv2120_led_data,
+ }
+};
+
static void mv2120_power_off(void)
{
pr_info("%s: triggering power-off...\n", __func__);
@@ -172,6 +216,7 @@ static void __init mv2120_init(void)
gpio_free(MV2120_GPIO_RTC_IRQ);
}
i2c_register_board_info(0, &mv2120_i2c_rtc, 1);
+ platform_device_register(&mv2120_leds);

/* register mv2120 specific power-off method */
if (gpio_request(MV2120_GPIO_POWER_OFF, "POWEROFF") != 0 ||

--
Martin Michlmayr
http://www.cyrius.com/