Tristan Corrick has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29977 )
Change subject: sb/intel/common: Create a common PCH finalise implementation ......................................................................
Patch Set 2:
(2 comments)
Patch Set 1:
While technically correct I don't see why we need to duplicate code and Kconfig options. Can you move the bd82x6x code to sb/intel/common and just use that instead ?
Done.
https://review.coreboot.org/#/c/29977/2/src/southbridge/intel/common/finaliz... File src/southbridge/intel/common/finalize.c:
https://review.coreboot.org/#/c/29977/2/src/southbridge/intel/common/finaliz... PS2, Line 61: pci_update_config32(lpc_dev, D31F0_ETR3, ~ETR3_CF9GR, ETR3_CF9LOCK); Public docs don't mention ETR3 for any platform before Sunrise Point, as far as I can tell. So I'm not sure if it's valid.
https://review.coreboot.org/#/c/29977/2/src/southbridge/intel/common/finaliz... PS2, Line 62: : /* PMSYNC */ : RCBA32_OR(0x33c4, (1UL << 31)); Public docs don't mention PMSYNC, as far as I can tell. So I'm not sure if it's valid for anything other than Lynx Point.