Date:	Fri, 25 Jul 2008 13:38:12 -0700 (PDT)
From:	Trent Piepho <tpiepho@...escale.com>
To:	Anton Vorontsov <avorontsov@...mvista.com>
cc:	Grant Likely <grant.likely@...retlab.ca>,
	Richard Purdie <rpurdie@...ys.net>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	Kumar Gala <galak@...nel.crashing.org>,
	linux-kernel@...r.kernel.org, linuxppc-dev@...abs.org
Subject: Re: [PATCH v3] leds: implement OpenFirmare GPIO LED driver

Here are my patches for the OF bindings.  The first is just a tiny change to
the leds code to silence a warning.  The second is the real patch.

The leds-gpio driver gets two sub-options in Kconfig, one for platform
device support and one for openfirmware platform device support.

There is support for setting an LED trigger.  I didn't include support for
active-low or initial brightness, but I think those would be good features
to add.  See below for more on that.

Since each device node can describe multiple leds, I used "gpio-leds"
instead of "gpio-led" for the compatible property.

Examples of the dts syntax:

leds {
         compatible = "gpio-leds";
         hdd {
                 label = "IDE activity";
                 gpios = <&mcu_pio 0 0>;
                 function = "ide-disk";
         };
};

run-control {
         compatible = "gpio-leds";
         red {
                 gpios = <&mpc8572 6 0>;
         };
         green {
                 gpios = <&mpc8572 7 0>;
         };
}


On Fri, 18 Jul 2008, Anton Vorontsov wrote:
> Later I tried to use aliases for default-trigger.
>
> http://ozlabs.org/pipermail/linuxppc-dev/2008-June/057244.html

I doesn't seem to me that the alias method will work very well.  If I want
an LED that starts on, I need to add code to the kernel to support an
"led-on-while-kernel-boots" alias?  And if I want red & green leds to flash
while booting, I need to add aliases "led-flashing-while-kernel-boots-1" and
"led-flashing-while-kernel-boots-2", since you can't assign two leds to the
same alias?

> As for linux,default-brightness... we don't need this since now we
> have default-on trigger.

Except we can't use triggers....

There is an issue with the default-on trigger, besides requiring led
triggers be turned on and the extra code that needs.  It causes the led to
have a glitch in it.  Suppose the LED is already on from reset or the boot
loader.  When the gpio is allocated, it's allocated with an initial value of
off.  Then, later, it's turned back on when the trigger runs.  But say the
trigger doesn't run?

Here's a real example.  I have an LED that comes on the board powers on.  It
was supposed to turn off when the kernel has finished booting.  But suppose
the kernel panics between the gpio getting lowered when the led is added and
the trigger turning it back on?  Now the LED is off and it appears the
problem happened after the kernel finished booting, but really it happened
during the kernel boot.  In an embedded system with no console, that's quite
a bit of misinformation.  It can be entirely avoided by changing just three
lines of code with the leds-gpio driver to add an option to start the led
on.

It could also be the case that the gpio the led is on also triggers some
other piece of hardware.  An alarm that's on the same line as a fault led
for example.  The glitch created by the default-on trigger could be
unacceptable in that case.
--
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/

Powered by blists - more mailing lists