Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35753 )
Change subject: sb/intel/ibexpeak: Implement PCH function disable in chip_ops ......................................................................
Patch Set 7: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/35753/7/src/mainboard/lenovo/x201/d... File src/mainboard/lenovo/x201/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35753/7/src/mainboard/lenovo/x201/d... PS7, Line 108: P2P I've seen this called "PCI bridge". P2P reminds me too much of P2SB
https://review.coreboot.org/c/coreboot/+/35753/7/src/southbridge/intel/ibexp... File src/southbridge/intel/ibexpeak/pch.c:
https://review.coreboot.org/c/coreboot/+/35753/7/src/southbridge/intel/ibexp... PS7, Line 30: #ifdef __SIMPLE_DEVICE__ : pci_devfn_t dev = PCI_DEV(0, 0x1f, 0); : #else : struct device *dev = pcidev_on_root(0x1f, 0); : #endif is this how __SIMPLE_DEVICE__ is meant to be used? IMHO it looks wrong to me
https://review.coreboot.org/c/coreboot/+/35753/7/src/southbridge/intel/ibexp... PS7, Line 36: if (pch_type < 0) : pch_type = pci_read_config8(dev, PCI_DEVICE_ID + 1); : return pch_type; How does this work?
https://review.coreboot.org/c/coreboot/+/35753/7/src/southbridge/intel/ibexp... PS7, Line 41: double empty line