Subject: Re: Please pull linux-2.6-mpc52xx.git

From: Bartlomiej Sieka <[email-masked]>
Date: Tue, 25 Mar 2008 16:29:47 +0100

Richard Purdie wrote:
> On Tue, 2008-03-18 at 09:29 +0100, Bartlomiej Sieka wrote:
>> Grant Likely wrote:
>>> The LED code just hasn't been picked up.  IIRC, it was reworked to
>>> make it a proper driver in drivers/leds. 
>> Yes, the Motion-PRO LED driver has been reworked and posted:
>> http://patchwork.ozlabs.org/linuxppc/patch?q=Motion-pro&id=16617
>>
>>  > I need to look at it again,
>>> but it is a lot of code for a very simple thing and I wasn't sure if I
>>> should be the one to pick it up because it is in drivers/leds which
>>> has a different maintainer.
>> I'm copying Richard Purdie who's listed as LED SUBSYSTEM maintainer.
>>
>> Richard -- could pick up the above mentioned Motion-PRO LED driver for
>> upstream inclusion? It started as a MPC5200-specific thing posted to
>> linuxppc-dev and got reviewed there, with the intent to go upstream via
>> Grant (MPC52XX maintainer). However, it seems that it should be merged
>> through your subsystem.
> 
> There are some tweaks this driver needs before it can be merged.
> 
> Firstly, it seems to re implement a timer to make the LED blink and I'm
> not keen on doing that. I notice that you have default_trigger = "timer"
> but that won't make it activate at boot which is probably why the other
> code exists?

That's right. The requirement is to have the LED blink while the system
is booting up, until a custom application takes control over.

> I will accept a patch which allows the default timer state
> to be configurable (either from the defconfig or from the commandline)
> which should solve your problem?

Yes, this should work. Will you accept a patch that allows default timer
configuration based on the information from the device tree blob (the
board in question is under arch/powerpc)?

> 
> Secondly, can you confirm what of_get_property(op->node, "label", NULL);
> returns and whether this conforms to the LED naming guidelines?

No it does not -- thanks for bringing this point up. Will post code that
fixes this.

Thanks for your comments.

Regards,
Bartlomiej