From b82b4314ad32dcdb02cb5db1efcda3417aa8fe5d Mon Sep 17 00:00:00 2001 From: Angel Pons Date: Thu, 23 Jul 2020 23:32:46 +0200 Subject: src: Never set ISA Enable on PCI bridges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Looks like no one really knows what this bit would be useful for, nor when it would need to be set. Especially if coreboot is setting it even on PCI *Express* bridges. Digging through git history, nearly all instances of setting it on PCIe bridges comes from i82801gx, for which no reason was given as to why this would be needed. The other instances in Intel code seem to have been, unsurprisingly, copy-pasted. Drop all uses of this definition and rename it to avoid confusion. The negation in the name could trick people into setting this bit again. Tested on Asrock B85M Pro4, no visible difference. Change-Id: Ifaff29561769c111fb7897e95dbea842faec5df4 Signed-off-by: Angel Pons Reviewed-on: https://review.coreboot.org/c/coreboot/+/43775 Tested-by: build bot (Jenkins) Reviewed-by: Patrick Rudolph Reviewed-by: Kyösti Mälkki --- src/device/cardbus_device.c | 1 - src/include/device/pci_def.h | 2 +- src/soc/intel/broadwell/pcie.c | 1 - src/soc/intel/common/block/pcie/pcie.c | 5 ++--- src/southbridge/intel/bd82x6x/pcie.c | 3 +-- src/southbridge/intel/i82801gx/pcie.c | 1 - src/southbridge/intel/i82801ix/pcie.c | 3 +-- src/southbridge/intel/i82801jx/pcie.c | 3 +-- src/southbridge/intel/lynxpoint/pcie.c | 1 - 9 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/device/cardbus_device.c b/src/device/cardbus_device.c index 266e194f9b..b014946144 100644 --- a/src/device/cardbus_device.c +++ b/src/device/cardbus_device.c @@ -119,7 +119,6 @@ void cardbus_enable_resources(struct device *dev) ctrl = pci_read_config16(dev, PCI_CB_BRIDGE_CONTROL); ctrl |= (dev->link_list->bridge_ctrl & ( - PCI_BRIDGE_CTL_NO_ISA | PCI_BRIDGE_CTL_VGA | PCI_BRIDGE_CTL_MASTER_ABORT | PCI_BRIDGE_CTL_BUS_RESET)); diff --git a/src/include/device/pci_def.h b/src/include/device/pci_def.h index 07ba4a2b30..25372bf51f 100644 --- a/src/include/device/pci_def.h +++ b/src/include/device/pci_def.h @@ -136,7 +136,7 @@ /* Enable parity detection on secondary interface */ #define PCI_BRIDGE_CTL_PARITY 0x01 #define PCI_BRIDGE_CTL_SERR 0x02 /* The same for SERR forwarding */ -#define PCI_BRIDGE_CTL_NO_ISA 0x04 /* Disable bridging of ISA ports */ +#define PCI_BRIDGE_CTL_ISA 0x04 /* Disable bridging of ISA ports */ #define PCI_BRIDGE_CTL_VGA 0x08 /* Forward VGA addresses */ #define PCI_BRIDGE_CTL_VGA16 0x10 /* Enable 16-bit i/o port decoding */ #define PCI_BRIDGE_CTL_MASTER_ABORT 0x20 /* Report master aborts */ diff --git a/src/soc/intel/broadwell/pcie.c b/src/soc/intel/broadwell/pcie.c index 14dcd3f556..00a8595eeb 100644 --- a/src/soc/intel/broadwell/pcie.c +++ b/src/soc/intel/broadwell/pcie.c @@ -587,7 +587,6 @@ static void pch_pcie_init(struct device *dev) reg16 = pci_read_config16(dev, PCI_BRIDGE_CONTROL); reg16 &= ~PCI_BRIDGE_CTL_PARITY; - reg16 |= PCI_BRIDGE_CTL_NO_ISA; pci_write_config16(dev, PCI_BRIDGE_CONTROL, reg16); /* Clear errors in status registers */ diff --git a/src/soc/intel/common/block/pcie/pcie.c b/src/soc/intel/common/block/pcie/pcie.c index f36366a754..ada380e493 100644 --- a/src/soc/intel/common/block/pcie/pcie.c +++ b/src/soc/intel/common/block/pcie/pcie.c @@ -24,9 +24,8 @@ static void pch_pcie_init(struct device *dev) /* Set Cache Line Size to 0x10 */ pci_write_config8(dev, PCI_CACHE_LINE_SIZE, CACHE_LINE_SIZE); - /* disable parity error response, enable ISA */ - pci_update_config16(dev, PCI_BRIDGE_CONTROL, - ~PCI_BRIDGE_CTL_PARITY, PCI_BRIDGE_CTL_NO_ISA); + /* disable parity error response */ + pci_and_config16(dev, PCI_BRIDGE_CONTROL, ~PCI_BRIDGE_CTL_PARITY); if (CONFIG(PCIE_DEBUG_INFO)) { printk(BIOS_SPEW, " MBL = 0x%08x\n", diff --git a/src/southbridge/intel/bd82x6x/pcie.c b/src/southbridge/intel/bd82x6x/pcie.c index 2eff162bd5..f6bffbb3b9 100644 --- a/src/southbridge/intel/bd82x6x/pcie.c +++ b/src/southbridge/intel/bd82x6x/pcie.c @@ -216,8 +216,7 @@ static void pci_init(struct device *dev) // This has no effect but the OS might expect it pci_write_config8(dev, 0x0c, 0x10); - pci_update_config16(dev, PCI_BRIDGE_CONTROL, - ~PCI_BRIDGE_CTL_PARITY, PCI_BRIDGE_CTL_NO_ISA); + pci_and_config16(dev, PCI_BRIDGE_CONTROL, ~PCI_BRIDGE_CTL_PARITY); /* Clear errors in status registers. FIXME: Do something? */ reg16 = pci_read_config16(dev, 0x06); diff --git a/src/southbridge/intel/i82801gx/pcie.c b/src/southbridge/intel/i82801gx/pcie.c index 6c0ca5d8e3..ca0ae2eee5 100644 --- a/src/southbridge/intel/i82801gx/pcie.c +++ b/src/southbridge/intel/i82801gx/pcie.c @@ -54,7 +54,6 @@ static void pci_init(struct device *dev) reg16 = pci_read_config16(dev, PCI_BRIDGE_CONTROL); reg16 &= ~PCI_BRIDGE_CTL_PARITY; - reg16 |= PCI_BRIDGE_CTL_NO_ISA; pci_write_config16(dev, PCI_BRIDGE_CONTROL, reg16); /* Enable IO xAPIC on this PCIe port */ diff --git a/src/southbridge/intel/i82801ix/pcie.c b/src/southbridge/intel/i82801ix/pcie.c index a8e7b11179..3900e92bf5 100644 --- a/src/southbridge/intel/i82801ix/pcie.c +++ b/src/southbridge/intel/i82801ix/pcie.c @@ -23,8 +23,7 @@ static void pci_init(struct device *dev) // This has no effect but the OS might expect it pci_write_config8(dev, 0x0c, 0x10); - pci_update_config16(dev, PCI_BRIDGE_CONTROL, ~PCI_BRIDGE_CTL_PARITY, - PCI_BRIDGE_CTL_NO_ISA); + pci_and_config16(dev, PCI_BRIDGE_CONTROL, ~PCI_BRIDGE_CTL_PARITY); /* Enable IO xAPIC on this PCIe port */ pci_or_config32(dev, 0xd8, 1 << 7); diff --git a/src/southbridge/intel/i82801jx/pcie.c b/src/southbridge/intel/i82801jx/pcie.c index 5195522217..18d2c72321 100644 --- a/src/southbridge/intel/i82801jx/pcie.c +++ b/src/southbridge/intel/i82801jx/pcie.c @@ -23,8 +23,7 @@ static void pci_init(struct device *dev) // This has no effect but the OS might expect it pci_write_config8(dev, 0x0c, 0x10); - pci_update_config16(dev, PCI_BRIDGE_CONTROL, ~PCI_BRIDGE_CTL_PARITY, - PCI_BRIDGE_CTL_NO_ISA); + pci_and_config16(dev, PCI_BRIDGE_CONTROL, ~PCI_BRIDGE_CTL_PARITY); /* Enable IO xAPIC on this PCIe port */ pci_or_config32(dev, 0xd8, 1 << 7); diff --git a/src/southbridge/intel/lynxpoint/pcie.c b/src/southbridge/intel/lynxpoint/pcie.c index 883dfc781b..d2950e7916 100644 --- a/src/southbridge/intel/lynxpoint/pcie.c +++ b/src/southbridge/intel/lynxpoint/pcie.c @@ -667,7 +667,6 @@ static void pci_init(struct device *dev) reg16 = pci_read_config16(dev, PCI_BRIDGE_CONTROL); reg16 &= ~PCI_BRIDGE_CTL_PARITY; - reg16 |= PCI_BRIDGE_CTL_NO_ISA; pci_write_config16(dev, PCI_BRIDGE_CONTROL, reg16); /* Clear errors in status registers */ -- cgit v1.2.3