Re: [PATCH v4 3/3] leds/powernv: Add driver for PowerNV platform

[Vasant Hegde <hegdevasant@linux.vnet.ibm.com>]

On 06/25/2015 06:39 AM, Benjamin Herrenschmidt wrote:
> On Tue, 2015-04-28 at 15:40 +0530, Vasant Hegde wrote:
> 
>> +Device Tree binding for LEDs on IBM Power Systems
>> +-------------------------------------------------
>> +
>> +The 'led' node under '/ibm,opal' lists service indicators available in the
>> +system and their capabilities.
>> +
>> +led {
>> +	compatible = "ibm,opal-v3-led";
>> +	phandle = <0x1000006b>;
>> +	linux,phandle = <0x1000006b>;
>> +	led-mode = "lightpath";
>> +
>> +	U78C9.001.RST0027-P1-C1 {
>> +		led-types = "identify", "fault";
>> +		led-loc = "descendent";
>> +		phandle = <0x1000006f>;
>> +		linux,phandle = <0x1000006f>;
>> +	};
>> +	...
>> +	...
>> +};

Ben,

> 
> My only issue is that /led should probably have been /leds but afaik
> this is already committed in the FW tree. Vasant, have we done a release
> what that code yet or can we still change this ?

I think we can change OPAL side as we haven't released kernel code... So no
consumer yet.
Will send a patch to skiboot mailing list. lets see

> 
> Also what does led-mode = "lightpath" means ? Can you describe it ?
> Are there alternative values ? I don't see the point myself ...

Yes.. Our system can work in two modes...
   - light path --> Both identify and faults are supported
     typically all low end servers are in this mode
  - guiding light -> Only identify LEDs are supports . no fault indicator
support for individual FRU
     typically high end servers are in this mode.

These modes are static in nature.. meaning we cannot change that during run time...
AFAIK all the PowerNV boxes shipped today are in Light Path mode.. I have added
this, so that in future if they decide to ship system with guiding light mode,
then we don't need to make any changes.


> 
> Don't leave the "linux,phandle" in the description of the binding (nor
> the phandle actually). They are implicit for all nodes, no need to
> clutter the documentation with them.

Sure. Will fix in next version.


> 
>> +Each node under 'led' node describes location code of FRU/Enclosure.
>> +
>> +The properties under each node:
>> +
>> +  led-types : Supported LED types (attention/identify/fault).
>> +
>> +  led-loc   : enclosure/descendent(FRU) location code.
> 
> I don't understand what that means. Please provide a more detailed
> explanation.
> 

This describes the LED location (FRU or enclosure level).. This was added to
identify the component (as FRU leds are overloaded and enclosure level we have
separate LEDs for each component)...


> Is the name of the node the loc code of the FRU ? In that case, how do
> you deal with multiple LEDs on the same FRU without a unit address ?

We use location code + LED type for node. So that we can identify multiple LEDs.

Looking back again, probably we can take out above property as we are not using
that today.

-Vasant