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 9:
(4 comments)
https://review.coreboot.org/#/c/32533/9/src/southbridge/intel/common/Kconfig File src/southbridge/intel/common/Kconfig:
https://review.coreboot.org/#/c/32533/9/src/southbridge/intel/common/Kconfig... PS9, Line 69: def_bool n Just `bool` should suffice.
https://review.coreboot.org/#/c/32533/9/src/southbridge/intel/common/Kconfig... PS9, Line 70: depends on SOUTHBRIDGE_INTEL_COMMON Why?
https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h File src/southbridge/intel/common/tco.h:
https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h@1... PS8, Line 17: #define TCOBASE 0x60 this is ambiguous. maybe PMBASE_TCO_OFFSET?
https://review.coreboot.org/#/c/32533/8/src/southbridge/intel/common/tco.h@2... PS8, Line 21: define TCO1_TIMEOUT (1 << 3) : #define SECOND_TO_STS (1 << 1) : #define TCO_TMR_HLT (1 << 11) Please indent the names to make them visibly distinctive from the register names. And put them below the registers where they are found, e.g.
#define TCO1_STS 0x04 #define TCO1_TIMEOUT (1 << 3) #define TCO2_STS 0x06 #define SECOND_TO_STS (1 << 1)