Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40234 )
Change subject: drivers/ipmi: Add IPMI KCS support in romstage ......................................................................
Patch Set 35:
(6 comments)
https://review.coreboot.org/c/coreboot/+/40234/33/src/drivers/ipmi/Kconfig File src/drivers/ipmi/Kconfig:
https://review.coreboot.org/c/coreboot/+/40234/33/src/drivers/ipmi/Kconfig@2... PS33, Line 29: BMC_KCS_BASE
not used in code
It's used in later depending changes, for examples: https://review.coreboot.org/c/coreboot/+/42495/10/src/mainboard/ocp/deltalak... https://review.coreboot.org/c/coreboot/+/41279/17/src/mainboard/ocp/deltalak...
https://review.coreboot.org/c/coreboot/+/40234/33/src/drivers/ipmi/ipmi_kcs_... File src/drivers/ipmi/ipmi_kcs_ops_premem.c:
https://review.coreboot.org/c/coreboot/+/40234/33/src/drivers/ipmi/ipmi_kcs_... PS33, Line 73: conf
possible null pointer dereference
Done
https://review.coreboot.org/c/coreboot/+/40234/33/src/drivers/ipmi/ipmi_kcs_... PS33, Line 83: No
message is not useful as it doesn't have a prefix
This is a continuation of the "Get BMC self test result" printk on L72, similar comment was discussed before at https://review.coreboot.org/c/coreboot/+/36638/9/src/drivers/ipmi/ipmi_kcs_o...
https://review.coreboot.org/c/coreboot/+/40234/33/src/drivers/ipmi/ipmi_kcs_... PS33, Line 87: printk(BIOS_DEBUG, "Function Not Implemented\n");
message is not useful as it doesn't have a prefix
This is a continuation of the "Get BMC self test result" printk on L72.
https://review.coreboot.org/c/coreboot/+/40234/33/src/drivers/ipmi/ipmi_kcs_... PS33, Line 91: BMC
why is it prefixed with BMC?
Done
https://review.coreboot.org/c/coreboot/+/40234/33/src/drivers/ipmi/ipmi_kcs_... PS33, Line 97: printk(BIOS_DEBUG, "Reserved\n");
message is not useful as it doesn't have a prefix
This is a continuation of the "Get BMC self test result" printk on L72.