Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32533 )
Change subject: sb/{ICH7,NM10,PCH}: Use common watchdog_off function ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/#/c/32533/6/src/southbridge/intel/bd82x6x/Makefi... File src/southbridge/intel/bd82x6x/Makefile.inc:
https://review.coreboot.org/#/c/32533/6/src/southbridge/intel/bd82x6x/Makefi... PS6, Line 34: ramstage-y += ../common/watchdog.c It's generally preferred to have a Kconfig + this line in the common code and select the Kconfig in the chipset code.
https://review.coreboot.org/#/c/32533/6/src/southbridge/intel/common/watchdo... File src/southbridge/intel/common/watchdog.c:
https://review.coreboot.org/#/c/32533/6/src/southbridge/intel/common/watchdo... PS6, Line 28: #define TCO1_STATUS 0x04 : #define TCO2_STATUS 0x06 : #define TCO1_CONTROL 0x08 Please use the names from the datasheets. It would also be nice to have these definitions in a common header file.
https://review.coreboot.org/#/c/32533/6/src/southbridge/intel/common/watchdo... PS6, Line 50: (1 << 11) Please use the defined names like on the lhs.