Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35521 )
Change subject: intel/pci: Utilise pci_def.h for PCI_BRIDGE_CONTROL ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35521/1/src/northbridge/intel/i945/... File src/northbridge/intel/i945/early_init.c:
https://review.coreboot.org/c/coreboot/+/35521/1/src/northbridge/intel/i945/... PS1, Line 555: pci_write_config16(PCI_DEV(0, 0x01, 0), PCI_BRIDGE_CONTROL, reg16);
Looks like we could use pci_secondary_reset(int assert), the sequence here repeats a lot. […]
Done
https://review.coreboot.org/c/coreboot/+/35521/1/src/northbridge/intel/i945/... PS1, Line 666: reg16 |= PCI_BRIDGE_CTL_VGA;
We can delay setting this to ramstage (with CTL_VGA16)?
Done
https://review.coreboot.org/c/coreboot/+/35521/1/src/soc/intel/common/block/... File src/soc/intel/common/block/pcie/pcie.c:
https://review.coreboot.org/c/coreboot/+/35521/1/src/soc/intel/common/block/... PS1, Line 40: /* disable parity error response, enable ISA */
This "enable ISA" is interesting. […]
Ack
https://review.coreboot.org/c/coreboot/+/35521/1/src/southbridge/intel/i8280... File src/southbridge/intel/i82801gx/pci.c:
https://review.coreboot.org/c/coreboot/+/35521/1/src/southbridge/intel/i8280... PS1, Line 80: ctrl |= (PCI_BRIDGE_CTL_PARITY | PCI_BRIDGE_CTL_SERR); /* error check */
Not sure why we set these for a short period of time, init() above disables them again.
Ack