Patrick Rudolph 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 33:
(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
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
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
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
https://review.coreboot.org/c/coreboot/+/40234/33/src/drivers/ipmi/ipmi_kcs_... PS33, Line 91: BMC why is it prefixed with BMC?
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