Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eBMC LED Management :: LampTest :: PLDM message to PHYP to start LampTest #2732

Open
vishwabmc opened this issue Oct 30, 2020 · 13 comments
Open
Assignees
Labels
Migrate-No Do not migrate to Jira Milestone Tgt Overrides Epic Milestone prio_high_2 ReadyForDev Stories ready for development work
Milestone

Comments

@vishwabmc
Copy link

As part of Lamp Test, we need to send a command to PHYP and thus need to send a PLDM command. The details would be pretty much like how a soft off did SetHostStateEffector. I am working with PHYP team so that they define an effector that BMC can use.

The change needs to go into the phosphor-led-manager and needs to be part of a lamp-test process. That is, in addition to sending commands to physical LEDs, the led-manager needs to make this PLDM call.

@vishwabmc
Copy link
Author

vishwabmc commented Nov 26, 2020

There is a change in the design on this.. The change is that we will use xyz/openbmc_project/led/groups/lamp_test group and its Asserted property. As a by product of this, a PropertyChangedSignal gets emitted and that will be caught by PLDM daemon and hence would send a command to PHYP.
Keeping it open until we are closed on that.

@vishwabmc
Copy link
Author

vishwabmc commented Jan 18, 2021

We are still committed to using Asserted property. However, instead of leaning onto PLDM daemon, the best is to have this code send the SetHostStateEffector since it's entirely driven by LED Manager Lamp-Test

At this moment, we don't have the HostEffector defined . but we can get going with rest of BMC implementation.
This code needs to be called whenever Asserted property is set to true.. Nothing needs to be done as part of Asserted being set to false

@vishwabmc vishwabmc added the ReadyForDev Stories ready for development work label Jan 18, 2021
@vishwabmc
Copy link
Author

vishwabmc commented Jan 20, 2021

From discussion with PHYP, here is the proposal:


So, BMC will only set this HostEffector with "4 minutes" or whatever that is configured. There is nothing to be done as part of stopping lamp test.

@lxwinspur
Copy link

@vishwabmc
We get the effecter ID by the FindStateEffecterPDR interface,
https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/PLDM/PDR.interface.yaml#L18
Now, we only know the EntityID value and not the StateSetId value.
Since both are the values are hard-code, Could you ask PHYP team and what the StateSetID value is?

@vishwabmc
Copy link
Author

https://gerrit.openbmc-project.xyz/c/openbmc/meta-phosphor/+/39963 : it was discussed in this and we have a new direction.

  • PLDM daemon would host a new interface that LED manager will call into
  • the new interface abstracts everything. The API will be like LampTest(time-out-in-milli-secs)
  • LED manager just makes that call.

@sampmisr FYA ^

@vishwabmc
Copy link
Author

vishwabmc commented Feb 5, 2021

https://gerrit.openbmc-project.xyz/c/openbmc/meta-phosphor/+/39963 : it was discussed in this and we have a new direction.

* PLDM daemon would host a new interface that LED manager will call into

* the new interface abstracts everything. The API will be like `LampTest(time-out-in-milli-secs)`

* LED manager just makes that call.

@sampmisr FYA ^

Okay.. finally, we have full clarity :)

  • PLDM daemon will implement xyz.openbmc_project.Led.Group interface
  • PLDM will host /xyz/openbmc_project/led/groups/host_lamp_test D-Bus object
  • LED manager will set asserted property to true on the above object
  • PLDM daemon will do SetHostStateEffector when the property is true
  • When the LampTest times out, LED manager sets the property to false
  • PLDM will not do anything when it is false as PHYP will auto stop the timer

@sampmisr @lxwinspur ^

@lxwinspur
Copy link

lxwinspur commented Feb 7, 2021

@vishwabmc @sampmisr
Okay, Pls let me do all of the above (LED manager and PLDM daemon). :)

@lxwinspur
Copy link

@mzipse mzipse modified the milestones: A.1.211, A.1.213.2 Apr 8, 2021
@mzipse mzipse added the Milestone Tgt Overrides Epic Milestone label Apr 8, 2021
@nmuruli nmuruli modified the milestones: A.1.213.2, A.1.212.5 Apr 30, 2021
@vishwabmc
Copy link
Author

@mzipse mzipse added the Migrate-No Do not migrate to Jira label Oct 26, 2022
@gtmills
Copy link

gtmills commented Jun 21, 2023

@lxwinspur
Copy link

Sure, will update

@lxwinspur
Copy link

@gtmills @manojkiraneda

Updated, Please review this patch.
https://gerrit.openbmc-project.xyz/c/openbmc/pldm/+/40373

@lxwinspur
Copy link

@mzipse @manojkiraneda
https://gerrit.openbmc.org/c/openbmc/pldm/+/40373
This patch Merge Conflict again. Does IBM have any plans to review this patch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Migrate-No Do not migrate to Jira Milestone Tgt Overrides Epic Milestone prio_high_2 ReadyForDev Stories ready for development work
Projects
None yet
Development

No branches or pull requests

7 participants