1*696c1d87SAlexander Hansen# OCP LED Policy Support
2*696c1d87SAlexander Hansen
3*696c1d87SAlexander HansenAuthor: Alexander Hansen
4*696c1d87SAlexander Hansen[alexander.hansen@9elements.com](mailto:alexander.hansen@9elements.com)
5*696c1d87SAlexander Hansen
6*696c1d87SAlexander HansenOther contributors: None
7*696c1d87SAlexander Hansen
8*696c1d87SAlexander HansenCreated: July 22, 2024
9*696c1d87SAlexander Hansen
10*696c1d87SAlexander Hansen## Problem Description
11*696c1d87SAlexander Hansen
12*696c1d87SAlexander HansenOpenBMC must support the [OCP LED Policy](#background-and-references). Currently
13*696c1d87SAlexander Hansenthere are some problematic cases in which the Policy is not supported by
14*696c1d87SAlexander Hansenexisting services like phosphor-led-manager.
15*696c1d87SAlexander Hansen
16*696c1d87SAlexander Hansen### Example ([Spec](#background-and-references) 6.2. System General Status)
17*696c1d87SAlexander Hansen
18*696c1d87SAlexander Hansen#### Defined states
19*696c1d87SAlexander Hansen
20*696c1d87SAlexander Hansen| STS (blue) | FAULT (amber) | NAME                                            |
21*696c1d87SAlexander Hansen| ---------- | ------------- | ----------------------------------------------- |
22*696c1d87SAlexander Hansen| 1          | 0             | "All OK"                                        |
23*696c1d87SAlexander Hansen| 0          | 1             | "Module missing / fault"                        |
24*696c1d87SAlexander Hansen| 1          | 1             | LED Priority (cannot represent the fault state) |
25*696c1d87SAlexander Hansen
26*696c1d87SAlexander Hansen#### Example sequence
27*696c1d87SAlexander Hansen
28*696c1d87SAlexander Hansen| STS | FAULT | action        |
29*696c1d87SAlexander Hansen| --- | ----- | ------------- |
30*696c1d87SAlexander Hansen| 0   | 0     | initial state |
31*696c1d87SAlexander Hansen| 1   | 0     | assert ok     |
32*696c1d87SAlexander Hansen| 1   | 1     | assert fault  |
33*696c1d87SAlexander Hansen| 1   | 1     | final state   |
34*696c1d87SAlexander Hansen
35*696c1d87SAlexander HansenWhen for example both of these groups are asserted, the change in LED state will
36*696c1d87SAlexander Hansenbe determined by the LED priority. The result is not allowed in the state
37*696c1d87SAlexander Hansendiagram in [Spec](#background-and-references).
38*696c1d87SAlexander Hansen
39*696c1d87SAlexander HansenIn this trivial example we can fix it by just assigning Priority "Off" to the
40*696c1d87SAlexander Hansenled-blue. But that's not possible at the time of writing (commit
41*696c1d87SAlexander Hansen[bdbfcde](https://github.com/openbmc/phosphor-led-manager/commit/bdbfcde10520fc841b44d1e777c353a698b774ff)).
42*696c1d87SAlexander Hansen
43*696c1d87SAlexander HansenThere are other examples with more LEDs where that alone will not fix the issue.
44*696c1d87SAlexander Hansen
45*696c1d87SAlexander Hansen### Example ([Spec](#background-and-references) 6.4 PSU Status)
46*696c1d87SAlexander Hansen
47*696c1d87SAlexander HansenHere we pretend that LED Priority "Off" is allowed and discover that it will not
48*696c1d87SAlexander Hansenbe sufficient to fix the issue.
49*696c1d87SAlexander Hansen
50*696c1d87SAlexander Hansen#### Defined states
51*696c1d87SAlexander Hansen
52*696c1d87SAlexander Hansen| AC OK (blue) | FAULT (amber) | LOW V (amber) | BACK UP (amber) | State                     |
53*696c1d87SAlexander Hansen| ------------ | ------------- | ------------- | --------------- | ------------------------- |
54*696c1d87SAlexander Hansen| 1            | 0             | 0             | 0               | "AC OK"                   |
55*696c1d87SAlexander Hansen| 0            | 1             | 0             | 0               | "AC Fault"                |
56*696c1d87SAlexander Hansen| 0            | 1             | 1             | 0               | "AC Under Voltage"        |
57*696c1d87SAlexander Hansen| 0            | 0             | 0             | 1               | "Backup due to AC Outage" |
58*696c1d87SAlexander Hansen
59*696c1d87SAlexander Hansen#### LED Priority
60*696c1d87SAlexander Hansen
61*696c1d87SAlexander Hansen| AC OK (blue) | FAULT (amber) | LOW V (amber) | BACK UP (amber) | State                     |
62*696c1d87SAlexander Hansen| ------------ | ------------- | ------------- | --------------- | ------------------------- |
63*696c1d87SAlexander Hansen| 0            | 0             | 0             | 1               | "Backup due to AC Outage" |
64*696c1d87SAlexander Hansen
65*696c1d87SAlexander Hansen#### Example Sequence
66*696c1d87SAlexander Hansen
67*696c1d87SAlexander Hansen| AC OK (blue) | FAULT (amber) | LOW V (amber) | BACK UP (amber) | State                        |
68*696c1d87SAlexander Hansen| ------------ | ------------- | ------------- | --------------- | ---------------------------- |
69*696c1d87SAlexander Hansen| 0            | 0             | 0             | 1               | LED Priority (for reference) |
70*696c1d87SAlexander Hansen| 0            | 0             | 0             | 0               | initial state                |
71*696c1d87SAlexander Hansen| 1            | 0             | 0             | 0               | assert "AC OK"               |
72*696c1d87SAlexander Hansen| 0            | 0             | 0             | 0               | assert "AC Fault"            |
73*696c1d87SAlexander Hansen| 0            | 0             | 0             | 0               | final state                  |
74*696c1d87SAlexander Hansen
75*696c1d87SAlexander HansenThe final state is undefined according to [Spec](#background-and-references)
76*696c1d87SAlexander Hansen6.4.
77*696c1d87SAlexander Hansen
78*696c1d87SAlexander HansenThe different possible LED priorities do not matter here, since "Backup due to
79*696c1d87SAlexander HansenAC Outage" could very well be the highest priority state. And choosing any other
80*696c1d87SAlexander HansenLED Priority will prevent it from being applied completely when any other state
81*696c1d87SAlexander Hansenis asserted simultaneously.
82*696c1d87SAlexander Hansen
83*696c1d87SAlexander HansenBy now it becomes clear that what's needed is a way to define priorities in
84*696c1d87SAlexander Hansenterms of groups and not single LEDs.
85*696c1d87SAlexander Hansen
86*696c1d87SAlexander Hansen## Background and References
87*696c1d87SAlexander Hansen
88*696c1d87SAlexander Hansen- [OCP Panel Indicator Specification rev1.0 (pdf)](http://files.opencompute.org/oc/public.php?service=files&t=65c02b1c6d59188351357cfb232cbfaa&download)
89*696c1d87SAlexander Hansen- [OCP Panel Indicator Specification rev1.0 (presentation)](https://www.opencompute.org/files/OCP18-EngWorkshop-Indicator-Specification-Proposal.pdf)
90*696c1d87SAlexander Hansen
91*696c1d87SAlexander HansenQuick summary of what's important for us here:
92*696c1d87SAlexander Hansen
93*696c1d87SAlexander Hansen### Permitted Indicator Colors
94*696c1d87SAlexander Hansen
95*696c1d87SAlexander Hansen- Blue
96*696c1d87SAlexander Hansen- Amber
97*696c1d87SAlexander Hansen- Combined Blue/Amber in tight spaces
98*696c1d87SAlexander Hansen
99*696c1d87SAlexander Hansen### Permitted Indicator States
100*696c1d87SAlexander Hansen
101*696c1d87SAlexander Hansen- OFF
102*696c1d87SAlexander Hansen- ON
103*696c1d87SAlexander Hansen- BLINK (500ms on, 500ms off)
104*696c1d87SAlexander Hansen
105*696c1d87SAlexander Hansen### Permitted Indicator Count on Component
106*696c1d87SAlexander Hansen
107*696c1d87SAlexander Hansen- 1 blue LED
108*696c1d87SAlexander Hansen- 1 to n amber LED to communicate multiple fault conditions OR
109*696c1d87SAlexander Hansen- 1 blue/amber LED
110*696c1d87SAlexander Hansen
111*696c1d87SAlexander Hansen## Requirements
112*696c1d87SAlexander Hansen
113*696c1d87SAlexander Hansen- Ability to prioritize the different LED Groups and thus possible indicator
114*696c1d87SAlexander Hansen  states.
115*696c1d87SAlexander Hansen- An Indicator should not be in an inconsistent state. It can display the states
116*696c1d87SAlexander Hansen  as per the [Spec](#background-and-references)
117*696c1d87SAlexander Hansen- Other services cannot be trusted to keep the assertion of the LED groups in a
118*696c1d87SAlexander Hansen  consistent state. The indicator state must be consistent even when conflicting
119*696c1d87SAlexander Hansen  LED groups are asserted.
120*696c1d87SAlexander Hansen
121*696c1d87SAlexander Hansen## Proposed Design
122*696c1d87SAlexander Hansen
123*696c1d87SAlexander HansenExtend the concept of "Priority" to groups in phosphor-led-manager. The group
124*696c1d87SAlexander Hansenpriority will be an integer value. A larger number means higher priority for
125*696c1d87SAlexander Hansenthat group. This allows users to have configurations like:
126*696c1d87SAlexander Hansen
127*696c1d87SAlexander Hansen1. OK
128*696c1d87SAlexander Hansen2. Fault 1
129*696c1d87SAlexander Hansen3. Fault 2
130*696c1d87SAlexander Hansen4. Locate
131*696c1d87SAlexander Hansen
132*696c1d87SAlexander Hansenwith locating and fault states having higher priorities. The LEDs would always
133*696c1d87SAlexander Hansenbe in a consistent state. The change can be done in a backwards-compatible way.
134*696c1d87SAlexander Hansen
135*696c1d87SAlexander Hansen### Configuration example
136*696c1d87SAlexander Hansen
137*696c1d87SAlexander Hansen```json
138*696c1d87SAlexander Hansen{
139*696c1d87SAlexander Hansen  "leds": [
140*696c1d87SAlexander Hansen    {
141*696c1d87SAlexander Hansen      "group": "enclosure_identify",
142*696c1d87SAlexander Hansen      "Priority": 2,
143*696c1d87SAlexander Hansen      "members": [
144*696c1d87SAlexander Hansen        {
145*696c1d87SAlexander Hansen          "Name": "sys_id",
146*696c1d87SAlexander Hansen          "Action": "Blink"
147*696c1d87SAlexander Hansen        },
148*696c1d87SAlexander Hansen        {
149*696c1d87SAlexander Hansen          "Name": "rear_id",
150*696c1d87SAlexander Hansen          "Action": "Blink"
151*696c1d87SAlexander Hansen        }
152*696c1d87SAlexander Hansen      ]
153*696c1d87SAlexander Hansen    },
154*696c1d87SAlexander Hansen    {
155*696c1d87SAlexander Hansen      "group": "fault",
156*696c1d87SAlexander Hansen      "Priority": 1,
157*696c1d87SAlexander Hansen      "members": [
158*696c1d87SAlexander Hansen        {
159*696c1d87SAlexander Hansen          "Name": "sys_id",
160*696c1d87SAlexander Hansen          "Action": "On"
161*696c1d87SAlexander Hansen        },
162*696c1d87SAlexander Hansen        {
163*696c1d87SAlexander Hansen          "Name": "fault",
164*696c1d87SAlexander Hansen          "Action": "On"
165*696c1d87SAlexander Hansen        }
166*696c1d87SAlexander Hansen      ]
167*696c1d87SAlexander Hansen    },
168*696c1d87SAlexander Hansen    {
169*696c1d87SAlexander Hansen      "group": "unrelated",
170*696c1d87SAlexander Hansen      "members": [
171*696c1d87SAlexander Hansen        {
172*696c1d87SAlexander Hansen          "Name": "rear_id",
173*696c1d87SAlexander Hansen          "Action": "On"
174*696c1d87SAlexander Hansen        }
175*696c1d87SAlexander Hansen      ]
176*696c1d87SAlexander Hansen    }
177*696c1d87SAlexander Hansen  ]
178*696c1d87SAlexander Hansen}
179*696c1d87SAlexander Hansen```
180*696c1d87SAlexander Hansen
181*696c1d87SAlexander Hansen### Mixed Use
182*696c1d87SAlexander Hansen
183*696c1d87SAlexander HansenThe group priority and led priority can be mutually exclusive in configuration
184*696c1d87SAlexander Hansento prevent any issues arising from mixed use. So the existing configs continue
185*696c1d87SAlexander Hansento work with individual LED Priority and when a group priority is assigned, we
186*696c1d87SAlexander Hansenassume the user wants to use that for configuration.
187*696c1d87SAlexander Hansen
188*696c1d87SAlexander Hansen## Alternatives Considered
189*696c1d87SAlexander Hansen
190*696c1d87SAlexander Hansen- Extend phosphor-led-sysfs to expose 2 LEDs (blue/amber) as a single LED.
191*696c1d87SAlexander Hansen
192*696c1d87SAlexander Hansen  - This does not work since there may be n different fault LEDs as per
193*696c1d87SAlexander Hansen    [Spec](#background-and-references)
194*696c1d87SAlexander Hansen  - Also this is basically lying to ourselves and making it difficult for other
195*696c1d87SAlexander Hansen    sw to get any meaningful info about LEDs from dbus anymore.
196*696c1d87SAlexander Hansen
197*696c1d87SAlexander Hansen- Allow Priority "Off" for LED config.
198*696c1d87SAlexander Hansen
199*696c1d87SAlexander Hansen  - This only solves the issue for very simple configurations.
200*696c1d87SAlexander Hansen  - Individual LED priorities are hard to think about when multiple LEDs form an
201*696c1d87SAlexander Hansen    indicator
202*696c1d87SAlexander Hansen
203*696c1d87SAlexander Hansen- Allow the mixed use of group and individual led priority
204*696c1d87SAlexander Hansen
205*696c1d87SAlexander Hansen  - This will require considering more edge cases arising from the mixed use.
206*696c1d87SAlexander Hansen  - Not aware of a use-case which would benefit from mixed use.
207*696c1d87SAlexander Hansen
208*696c1d87SAlexander Hansen- Allow each LED to configure the priority of groups it represents, instead of
209*696c1d87SAlexander Hansen  just one state.
210*696c1d87SAlexander Hansen
211*696c1d87SAlexander Hansen  - e.g. "Priority": ["enclosure_identify", "fault", "power"]
212*696c1d87SAlexander Hansen  - This config would have to be repeated on each instance of an LED
213*696c1d87SAlexander Hansen  - Or assumed that the first instance defines it?
214*696c1d87SAlexander Hansen  - Would need to check for equality of all these priority lists for an LED
215*696c1d87SAlexander Hansen  - This does not solve the problem of a group being represented incompletely
216*696c1d87SAlexander Hansen    - For example it is possible for 2 LEDs belonging to the same group to
217*696c1d87SAlexander Hansen      prioritize these groups differently in their priority list.
218*696c1d87SAlexander Hansen
219*696c1d87SAlexander Hansen- Allow configuring an "indicator" that's comprised of multiple LEDs and then
220*696c1d87SAlexander Hansen  just define states for that indicator.
221*696c1d87SAlexander Hansen
222*696c1d87SAlexander Hansen  - Need to translate these states to groups anyways to be compatible with the
223*696c1d87SAlexander Hansen    existing internal data structures
224*696c1d87SAlexander Hansen  - Handle the case of member LEDs of that indicator also being members of other
225*696c1d87SAlexander Hansen    groups
226*696c1d87SAlexander Hansen  - This is basically the same as group priorities but with additional overhead
227*696c1d87SAlexander Hansen    in config parsing
228*696c1d87SAlexander Hansen
229*696c1d87SAlexander Hansen- Only display the group that was last asserted, in case of conflicting groups
230*696c1d87SAlexander Hansen  - This is undesirable since there are groups which are more important to
231*696c1d87SAlexander Hansen    display
232*696c1d87SAlexander Hansen
233*696c1d87SAlexander Hansen## Impacts
234*696c1d87SAlexander Hansen
235*696c1d87SAlexander HansenNeed to
236*696c1d87SAlexander Hansen
237*696c1d87SAlexander Hansen- extend docs on how to configure LED group priority
238*696c1d87SAlexander Hansen- implement the LED group priority
239*696c1d87SAlexander Hansen- write unit tests for LED group priority
240*696c1d87SAlexander Hansen- perhaps change some configs to use this new feature
241*696c1d87SAlexander Hansen  - this is optional as the change is backwards-compatible
242*696c1d87SAlexander Hansen
243*696c1d87SAlexander HansenThere should be no impact on d-bus except if we choose to expose the led group
244*696c1d87SAlexander Hansenpriority.
245*696c1d87SAlexander Hansen
246*696c1d87SAlexander Hansen### Organizational
247*696c1d87SAlexander Hansen
248*696c1d87SAlexander Hansen- Does this repository require a new repository?
249*696c1d87SAlexander Hansen  - No
250*696c1d87SAlexander Hansen- Who will be the initial maintainer(s) of this repository?
251*696c1d87SAlexander Hansen- Which repositories are expected to be modified to execute this design?
252*696c1d87SAlexander Hansen  - phosphor-led-manager
253*696c1d87SAlexander Hansen  - optionally openbmc to upgrade existing configs
254*696c1d87SAlexander Hansen- Make a list, and add listed repository maintainers to the gerrit review.
255*696c1d87SAlexander Hansen
256*696c1d87SAlexander Hansen## Testing
257*696c1d87SAlexander Hansen
258*696c1d87SAlexander HansenHow will this be tested? How will this feature impact CI testing?
259*696c1d87SAlexander Hansen
260*696c1d87SAlexander HansenThe feature can easily be unit-tested.
261