Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42277 )
Change subject: mb/ocp/deltalake: Update SMBIOS type 2 Location In Chassis from BMC ......................................................................
Patch Set 9:
(5 comments)
https://review.coreboot.org/c/coreboot/+/42277/6/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/ipmi.h:
https://review.coreboot.org/c/coreboot/+/42277/6/src/mainboard/ocp/deltalake... PS6, Line 8: #define IPMI_NETFN_OEM 0x30
See comment below
Ack
https://review.coreboot.org/c/coreboot/+/42277/6/src/mainboard/ocp/deltalake... PS6, Line 11:
The commands that belong to the same Net function are aligned together, similar style in […]
Ack
https://review.coreboot.org/c/coreboot/+/42277/6/src/mainboard/ocp/deltalake... PS6, Line 11:
Use tabs for alignment of the values?
It's already aligned by tabs in patchset 8.
https://review.coreboot.org/c/coreboot/+/42277/8/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/ramstage.c:
https://review.coreboot.org/c/coreboot/+/42277/8/src/mainboard/ocp/deltalake... PS8, Line 22: return "";
If the result is not CB_SUCCESS, either the BMC communication is not working or there is bug in BMC, […]
Done
https://review.coreboot.org/c/coreboot/+/42277/8/src/mainboard/ocp/deltalake... PS8, Line 24: /* Sanity check, slot_id can only be 1~4 since there are 4 slots in YV3 */
Let's set up an enum in ipmi.h for slot1 to slot4, that would be better than hard coding here.
Done