Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34569 )
Change subject: drivers/ipmi: Add option to not query BMC ......................................................................
Patch Set 1:
(17 comments)
https://review.coreboot.org/c/coreboot/+/34569/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34569/1//COMMIT_MSG@9 PS1, Line 9: 34seconds Please add a space before the unit.
https://review.coreboot.org/c/coreboot/+/34569/1//COMMIT_MSG@9 PS1, Line 9: SuperMicro Supermicro
https://review.coreboot.org/c/coreboot/+/34569/1//COMMIT_MSG@9 PS1, Line 9: bu but
https://review.coreboot.org/c/coreboot/+/34569/1//COMMIT_MSG@10 PS1, Line 10: timeout time-out
https://review.coreboot.org/c/coreboot/+/34569/1//COMMIT_MSG@10 PS1, Line 10: 100msec Space: 100 msec
https://review.coreboot.org/c/coreboot/+/34569/1//COMMIT_MSG@12 PS1, Line 12: 34seconds 34 s
https://review.coreboot.org/c/coreboot/+/34569/1//COMMIT_MSG@15 PS1, Line 15: SuperMicro Supermicro
https://review.coreboot.org/c/coreboot/+/34569/1//COMMIT_MSG@16 PS1, Line 16: timeout time-out
https://review.coreboot.org/c/coreboot/+/34569/1//COMMIT_MSG@16 PS1, Line 16: an a
https://review.coreboot.org/c/coreboot/+/34569/1/src/drivers/ipmi/chip.h File src/drivers/ipmi/chip.h:
https://review.coreboot.org/c/coreboot/+/34569/1/src/drivers/ipmi/chip.h@27 PS1, Line 27: used use
https://review.coreboot.org/c/coreboot/+/34569/1/src/drivers/ipmi/chip.h@27 PS1, Line 27: /* Do not send IPMI_BMC_GET_DEVICE_ID to BMC, but used hardcoded Please use:
``` /* * … */ ```
https://review.coreboot.org/c/coreboot/+/34569/1/src/drivers/ipmi/chip.h@30 PS1, Line 30: SuperMicro Supermicro
https://review.coreboot.org/c/coreboot/+/34569/1/src/drivers/ipmi/chip.h@30 PS1, Line 30: 34seconds 34 s
https://review.coreboot.org/c/coreboot/+/34569/1/src/drivers/ipmi/chip.h@32 PS1, Line 32: u8 bool?
https://review.coreboot.org/c/coreboot/+/34569/1/src/drivers/ipmi/chip.h@33 PS1, Line 33: of if
https://review.coreboot.org/c/coreboot/+/34569/1/src/drivers/ipmi/ipmi_kcs_o... File src/drivers/ipmi/ipmi_kcs_ops.c:
https://review.coreboot.org/c/coreboot/+/34569/1/src/drivers/ipmi/ipmi_kcs_o... PS1, Line 77: printk(BIOS_INFO, "IPMI: Assuming BMC is installed\n"); This could be added to the print line below.
https://review.coreboot.org/c/coreboot/+/34569/1/src/drivers/ipmi/ipmi_kcs_o... PS1, Line 83: ipmi_revision_major, ipmi_revision_minor); Too bad the line is duplicated in the next if-else branch. You could add a `return` in the else branch, and then move the printk statement out of the if else statement.