Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36180 )
Change subject: mb/ocp/monolake: Configure IPMI BMC FRB2 watchdog timer via VPD variables ......................................................................
Patch Set 12:
(6 comments)
https://review.coreboot.org/c/coreboot/+/36180/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36180/10//COMMIT_MSG@13 PS10, Line 13: Right now the timer is started after FSP-M. Ideally it should be
Please add a blank line above to separate the paragraphs.
Done
https://review.coreboot.org/c/coreboot/+/36180/10//COMMIT_MSG@16 PS10, Line 16: Tested on OCP Mono Lake.
How did you force a failure to get the watchdog to trigger?
coreboot started the watchdog timer, if I don't stop it in payload or OS it will trigger system hard reset in this change.
https://review.coreboot.org/c/coreboot/+/36180/10/src/mainboard/ocp/monolake... File src/mainboard/ocp/monolake/mainboard.c:
https://review.coreboot.org/c/coreboot/+/36180/10/src/mainboard/ocp/monolake... PS10, Line 32: #define DEFAULT_COUNTDOWN 9000
Isn’t that too long?
It's the conventional default value set by UEFI BIOS, here we simply follow. It can be changed via VPD variable.
https://review.coreboot.org/c/coreboot/+/36180/10/src/mainboard/ocp/monolake... PS10, Line 54: printk(BIOS_DEBUG, "FRB2 timer countdown set to:%d\n",
Please add a space after :.
Done
https://review.coreboot.org/c/coreboot/+/36180/10/src/mainboard/ocp/monolake... PS10, Line 57: printk(BIOS_DEBUG, "FRB2 timer use default value:%d\n",
Ditto.
Done
https://review.coreboot.org/c/coreboot/+/36180/10/src/mainboard/ocp/monolake... PS10, Line 63: }
Can this be put into a separate function?
Done