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