Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42242 )
Change subject: drivers/ocp/ipmi: Add ipmi set processor information ......................................................................
Patch Set 3:
(4 comments)
Thanks!
https://review.coreboot.org/c/coreboot/+/42242/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42242/3//COMMIT_MSG@9 PS3, Line 9: Implement setting processor information to BMC. If this IPMI OEM command is defined in a public doc (such as OCP doc), please include the link.
https://review.coreboot.org/c/coreboot/+/42242/3/src/drivers/ocp/ipmi/ipmi_o... File src/drivers/ocp/ipmi/ipmi_ocp.h:
https://review.coreboot.org/c/coreboot/+/42242/3/src/drivers/ocp/ipmi/ipmi_o... PS3, Line 7: #include <cpu/x86/msr.h> Please only keep header files needed for this header file.
https://review.coreboot.org/c/coreboot/+/42242/3/src/drivers/ocp/ipmi/ipmi_o... File src/drivers/ocp/ipmi/ipmi_ocp.c:
https://review.coreboot.org/c/coreboot/+/42242/3/src/drivers/ocp/ipmi/ipmi_o... PS3, Line 65: memcpy(&req1.data.manufacturer_id, &mfid, 3); what if CONFIG_MANU_ID is not defined?
https://review.coreboot.org/c/coreboot/+/42242/3/src/drivers/ocp/ipmi/ipmi_o... PS3, Line 125: /* TBD */ What does the spec say about this? Why TBD? Should we just put stepping_id in req2.revision[0]?