[PATCH v2 2/2] leds/powernv: Add driver for PowerNV platform
Jacek Anaszewski
j.anaszewski81 at gmail.com
Thu Apr 23 07:45:09 AEST 2015
Hi Vasant,
On 20.04.2015 17:53, Vasant Hegde wrote:
> On 04/20/2015 08:50 PM, Jacek Anaszewski wrote:
>> On 04/20/2015 02:34 PM, Vasant Hegde wrote:
>>> On 04/20/2015 05:15 PM, Jacek Anaszewski wrote:
>>>> Hi Vasant,
>>>>
>>>> I'd like to clarify some details regarding your explanation.
>>>>
>>>> On 04/15/2015 12:15 PM, Vasant Hegde wrote:
>>>> [...]
>>>>>>>
>>>>>>> In Power Systems LEDs are overloaded (meaning same LED is used
>>>>>>> for identify and
>>>>>>> fault depending on their state --- blinking = identify and
>>>>>>> solid = fault). Hence here append LED type info.
>>>>>>
>>>>>> The label could be composed of segments and an ordinal number as
>>>>>> labels have to be unique, e.g. attn_ident_0, attn_ident_1.
>>>>>> The segments would have to be parsed by the driver to discover
>>>>>> all the LED's available modes.
>>>>>>
>>>>>> nitpicking: identify is a verb and is not a proper name for the
>>>>>> LED. Could you describe the purpose of this mode, so that we
>>>>>> could come up with a better name?
>>>>>
>>>>> Each component (Field Replacement Unit) will have service
>>>>> indicator (LEDS) which
>>>>> can have below states :
>>>>> - OFF : no action
>>>>> - Identify: blinking state (user can use this state to
>>>>> identify particular component).
>>>>> In Power Systems world we call it as "identify"
>>>>> indicator.. Hence I retained same name here.
>>>>> How about just "ident" ?
>>>>> - fault : solid state (when component goes bad, LED goes to
>>>>> solid state) Note that our FW is capable of isolating some of the
>>>>> issues and it can turn
>>>>> on LEDs without OS
>>>>> interference.
>>>>>
>>>>> We have one more System level LED (System Attention Indicator)..
>>>>> This LED has two states:
>>>>> - OFF : Everything is fine
>>>>> - ON : Some component has issues and needs attention.
>>>>
>>>> We have three modes:
>>>> - identify - blinking
>>>> - fault - solid
>>>> - attention indicator - solid
>>>>
>>>> How does LED operation differ for fault and attention modes?
>>>> Does a LED have different intensity?
>>>
>>> Jacek,
>>>
>>> System Attention LED is special LED and its single LED
>>> available/system. where as identify and fault is applicable to all
>>> field replaceable units in the system..
>>>
>>> So Typical server will have
>>> 1 System Attention LED
>>> N Identify/fault LED (N = Field Replaceable Unit).
>>>
>>> Apart from above two, we do have two more LEDs/Enclosure (external
>>> visible LEDs)
>>> - Enclosure Identify
>>> - Enclosure fault
>>> These LEDs reflects state of all Field Replaceable Units
>>> (FRU) inside this enclosure
>>> If any of FRU state is ON, this will become ON
>>> Also we can independently enable this LED!!
>>>
>>> But from kernel side implementation point of view, I just
>>> treat this as another LED.. as our platform code (OPAL firmware)
>>> takes care of roll up etc.
>>>
>>>
>>> Now our LED can operate in two mode (Depending on our service
>>> model, typically one/two socket servers are Light Path mode,
>>> whereas high end servers are Guiding Light Mode).
>>>
>>> 1. Guiding Light
>>> Only Identify indicator is support.. Fault is not supported
>>> System attention indicator is used to point there is some
>>> problem in system and need attention
>>> 2. Light Path mode
>>> Both identify and fault indicator is supported ..
>>> Fault is ON whenever some component is faulty
>>> System attention indicator is used to point that FW/OS is not
>>> able to isolate the problem and needs user to look into serviceable
>>> event (like syslog/ our agents like ESA which analyzes and reports
>>> events)
>>>
>>>
>>> Handling LED states :
>>> - Though physically single LED is overloaded for identify and
>>> fault, logically (FW/OS level) we treat them as separate LED.
>>> - We can enable both fault and identify simultaneously.
>>> - Hardware decides physical LED state (rule : identify has
>>> priority over fault).
>>> ex: Say location code 'X',
>>> Identify = ON, fault = ON , state of 'X' = identify
>>> (blinking) Identify = OFF, fault = ON, state of 'X' = fault
>>> (solid) Identify = OFF, fault = ON, state of 'X' = identify
>>> (blinking) Identify = OFF, fault = OFF , state of 'X' = OFF
>>>
>>> Since we have various above combinations, I thought its best to
>>> have separate class dev for each individual LEDs. That way we keep
>>> everything simple and let firmware handle all complexities.
>>>
>>> Hope this clarifies.
>>>
>>> I just posted v3 where I addressed your comments.
>>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2015-April/127702.html
>>>
>>> Please let me know if you have any comments/suggestions.
>
> Jacek,
>
>>
>> From what I can see from the driver code the LEDs are set with:
>>
>> opal_leds_set_ind(token, loc_code, led_mask, led_value,
>> &max_led_type);
>>
>> and their state can be read with:
>>
>> opal_leds_get_ind(loc_code, &led_mask, &led_value, &max_led_type)
>>
>> From the kernel point of view these are very simple operations.
>>
>> All the logic you described should be handled by user space.
>> If you need to be able to specify the LED mode you want to set/read,
>> then additional 'mode' sysfs attribute should be added by the driver.
>> There would have to be also additional sysfs attribute
>> 'available_modes" provided. The ABI documentation should inform how
>> the mode identifiers map to the modes. I already explained how to
>> add it, when we were discussing about retaining led state on remove.
>
> Sorry..My fault.. I should have elaborated mode operation...
What I was thinking about here was actually LED type, not mode in terms
of Guiding Light/Light Path. However, please look at the newest
approach in the end of this message.
> I forgot to mention that LED mode is static... meaning platform
> provides this information, but we cannot change during runtime..
>
> Presently we have this information in Device Tree. Since this is
> static one (and also LED Mode is system wide.. nothing to do
> individual LED), I didn't add it in LED driver code.. .Do you think
> we should add that property ?
The property shouldn't be documented at all if it isn't to be used.
>>
>>
>> I'd see following use cases.
>>
>> (let's assume that modes are defined as follows:
>> 0: ident, 1: attn, 2: fault)
>
> Modes are : Guiding Light / Light Path ... which is static and
> platform provides this information.
>
> LED types : IDENT, FAULT and ATTN .... which can be get/set/reset by
> OS (kernel/userspace)
>
> Also only 1 LED can be ATTN ...
>
>> #cat available_modes
>> #0 1 2
>> #echo 0 > mode //set ident mode
>> #echo 1 > brightness //set ident state
>> #echo 2 > mode //set fault mode
>> #cat brightness //read fault state
>> #0
>> #echo 1 > attn //set attn mode
>> #echo 1 > brightness
>>
>> This would set the LED in blinking mode, so I am wondering if we
>> shouldn't employ timer trigger for this to keep the LED API
>> consistent.
>>
>> Can a single LED support other mode than 'attention'? I'd like to
>> know if a LED in attention mode (blinking), can be set to some solid
>> mode?
>
> No.. Its always single attention LED/system ... which can be Set
> (Solid) / reset state.
I confused it with ident.
>>
>> Please let me know if such an approach would still not fit for your
>> requirements.
>>
>
> Given above conditions, I think current approach (my v3 patchset) is
> simple and works better. What you say?
Yes, but we still have naming and blinking issues to solve.
Please look at this draft design of device tree node:
opal-leds {
compatible = "ibm,opal-v3-led";
U78C9.001.RST0027-P1-C1:attn {
};
U78C9.001.RST0027-P2-C1:identify:fault
};
U78C9.001.RST0027-P3-C2:identify:fault {
};
};
};
The LED nodes could be empty as the name would convey all the
required information. The implications would be as follows:
1. Each LED would have one corresponding LED class device.
2. Operations on attn and fault LED types:
turn on:
echo 255 > brightness
turn off:
echo 0 > brightness
get status
cat brightness
3. Operations on identify LED:
turn on:
echo "timer" > trigger
(blink_set op would have to be implemented in the
driver)
turn off:
echo 0 > brightness
get status:
support for this would have to be added to the LED
subsystem core
4. Since 'identify' is the platform specific name it could be preserved
Does it work for you?
--
Best Regards,
Jacek Anaszewski
More information about the Linuxppc-dev
mailing list