Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35525 )
Change subject: soc/intel/skylake: lock TCO base address on pch finalize ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35525/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35525/2//COMMIT_MSG@7 PS2, Line 7: soc/intel/skylake This is applying the logic for not just skylake but all Intel SoCs using this common code.
https://review.coreboot.org/c/coreboot/+/35525/2//COMMIT_MSG@7 PS2, Line 7: soc/intel/skylake: lock TCO base address on pch finalize Just curious: What is the motivation behind this locking? Is this following any particular recommendation?
https://review.coreboot.org/c/coreboot/+/35525/2/src/soc/intel/common/block/... File src/soc/intel/common/block/smbus/tco.c:
https://review.coreboot.org/c/coreboot/+/35525/2/src/soc/intel/common/block/... PS2, Line 67: #if defined(__SIMPLE_DEVICE__) : int devfn = PCH_DEVFN_SMBUS; : pci_devfn_t dev = PCI_DEV(0, PCI_SLOT(devfn), PCI_FUNC(devfn)); : #else : struct device *dev; : dev = PCH_DEV_SMBUS; : #endif : : /* TCO base address lockdown */ : reg32 = pci_read_config32(dev, TCOCTL); : pci_write_config32(dev, TCOCTL, reg32 | TCO_BASE_LOCK); I think at minimum this should be guarded by SOC_INTEL_COMMON_BLOCK_TCO_ENABLE_THROUGH_SMBUS. It might be better to just create a separate function to do this: static void tco_lockdown_bar()