Change in coreboot[master]: soc/intel/broadwell: Fix 16-bit read/write PCI_COMMAND register
HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40850 )
Change subject: soc/intel/broadwell: Fix 16-bit read/write PCI_COMMAND register ......................................................................
soc/intel/broadwell: Fix 16-bit read/write PCI_COMMAND register
Change-Id: I0fd1a758d8838b3eea5640b41eee6a6893360aa3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/soc/intel/broadwell/adsp.c M src/soc/intel/broadwell/hda.c M src/soc/intel/broadwell/me.c M src/soc/intel/broadwell/minihd.c M src/soc/intel/broadwell/pch.c M src/soc/intel/broadwell/pcie.c M src/soc/intel/broadwell/serialio.c M src/soc/intel/broadwell/smihandler.c 8 files changed, 34 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/40850/1
diff --git a/src/soc/intel/broadwell/adsp.c b/src/soc/intel/broadwell/adsp.c index 82904de..cde7496 100644 --- a/src/soc/intel/broadwell/adsp.c +++ b/src/soc/intel/broadwell/adsp.c @@ -22,11 +22,12 @@ config_t *config = config_of(dev); struct resource *bar0, *bar1; u32 tmp32; + uint16_t reg16;
/* Ensure memory and bus master are enabled */ - tmp32 = pci_read_config32(dev, PCI_COMMAND); - tmp32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY; - pci_write_config32(dev, PCI_COMMAND, tmp32); + reg16 = pci_read_config16(dev, PCI_COMMAND); + reg16 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY; + pci_write_config16(dev, PCI_COMMAND, reg16);
/* Find BAR0 and BAR1 */ bar0 = find_resource(dev, PCI_BASE_ADDRESS_0); diff --git a/src/soc/intel/broadwell/hda.c b/src/soc/intel/broadwell/hda.c index 476d092..c5ce2e4 100644 --- a/src/soc/intel/broadwell/hda.c +++ b/src/soc/intel/broadwell/hda.c @@ -84,7 +84,6 @@ u8 *base; struct resource *res; u32 codec_mask; - u32 reg32;
/* Find base address */ res = find_resource(dev, PCI_BASE_ADDRESS_0); @@ -95,8 +94,7 @@ printk(BIOS_DEBUG, "HDA: base = %p\n", base);
/* Set Bus Master */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - pci_write_config32(dev, PCI_COMMAND, reg32 | PCI_COMMAND_MASTER); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
hda_pch_init(dev, base);
@@ -110,7 +108,7 @@
static void hda_enable(struct device *dev) { - u32 reg32; + u16 reg16; u8 reg8;
reg8 = pci_read_config8(dev, 0x43); @@ -126,10 +124,10 @@ printk(BIOS_INFO, "HDA disabled, I/O buffers routed to ADSP\n");
/* Ensure memory, io, and bus master are all disabled */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 &= ~(PCI_COMMAND_MASTER | + reg16 = pci_read_config16(dev, PCI_COMMAND); + reg16 &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_write_config16(dev, PCI_COMMAND, reg16);
/* Disable this device */ pch_disable_devfn(dev); diff --git a/src/soc/intel/broadwell/me.c b/src/soc/intel/broadwell/me.c index 730d77e..0edc1d3 100644 --- a/src/soc/intel/broadwell/me.c +++ b/src/soc/intel/broadwell/me.c @@ -600,17 +600,17 @@
static void intel_me_finalize(struct device *dev) { - u32 reg32; + u16 reg16;
/* S3 path will have hidden this device already */ if (!mei_base_address || mei_base_address == (u8 *) 0xfffffff0) return;
/* Make sure IO is disabled */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 &= ~(PCI_COMMAND_MASTER | + reg16 = pci_read_config16(dev, PCI_COMMAND); + reg16 &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_write_config16(dev, PCI_COMMAND, reg16);
/* Hide the PCI device */ RCBA32_OR(FD2, PCH_DISABLE_MEI1); @@ -712,7 +712,7 @@ { struct resource *res; struct mei_csr host; - u32 reg32; + u16 reg16;
/* Find the MMIO base for the ME interface */ res = find_resource(dev, PCI_BASE_ADDRESS_0); @@ -723,9 +723,9 @@ mei_base_address = res2mmio(res, 0, 0);
/* Ensure Memory and Bus Master bits are set */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY; - pci_write_config32(dev, PCI_COMMAND, reg32); + reg16 = pci_read_config16(dev, PCI_COMMAND); + reg16 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY; + pci_write_config16(dev, PCI_COMMAND, reg16);
/* Clean up status for next message */ read_host_csr(&host); diff --git a/src/soc/intel/broadwell/minihd.c b/src/soc/intel/broadwell/minihd.c index 2dcab97..da0b42a 100644 --- a/src/soc/intel/broadwell/minihd.c +++ b/src/soc/intel/broadwell/minihd.c @@ -62,8 +62,7 @@ printk(BIOS_DEBUG, "Mini-HD: base = %p\n", base);
/* Set Bus Master */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - pci_write_config32(dev, PCI_COMMAND, reg32 | PCI_COMMAND_MASTER); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
/* Mini-HD configuration */ reg32 = read32(base + 0x100c); diff --git a/src/soc/intel/broadwell/pch.c b/src/soc/intel/broadwell/pch.c index f6f3746..ac6ac2a 100644 --- a/src/soc/intel/broadwell/pch.c +++ b/src/soc/intel/broadwell/pch.c @@ -171,7 +171,7 @@
void broadwell_pch_enable_dev(struct device *dev) { - u32 reg32; + u16 reg16;
/* These devices need special enable/disable handling */ switch (PCI_SLOT(dev->path.pci.devfn)) { @@ -185,18 +185,16 @@ printk(BIOS_DEBUG, "%s: Disabling device\n", dev_path(dev));
/* Ensure memory, io, and bus master are all disabled */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 &= ~(PCI_COMMAND_MASTER | + reg16 = pci_read_config16(dev, PCI_COMMAND); + reg16 &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_write_config16(dev, PCI_COMMAND, reg16);
/* Disable this device if possible */ pch_disable_devfn(dev); } else { /* Enable SERR */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_SERR; - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_SERR); } }
diff --git a/src/soc/intel/broadwell/pcie.c b/src/soc/intel/broadwell/pcie.c index f81f042..d506057 100644 --- a/src/soc/intel/broadwell/pcie.c +++ b/src/soc/intel/broadwell/pcie.c @@ -574,19 +574,14 @@ static void pch_pcie_init(struct device *dev) { u16 reg16; - u32 reg32;
printk(BIOS_DEBUG, "Initializing PCH PCIe bridge.\n");
/* Enable SERR */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_SERR; - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_SERR);
/* Enable Bus Master */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_MASTER; - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
/* Set Cache Line Size to 0x10 */ pci_write_config8(dev, 0x0c, 0x10); @@ -597,6 +592,7 @@ pci_write_config16(dev, PCI_BRIDGE_CONTROL, reg16);
#ifdef EVEN_MORE_DEBUG + u32 reg32; reg32 = pci_read_config32(dev, 0x20); printk(BIOS_SPEW, " MBL = 0x%08x\n", reg32); reg32 = pci_read_config32(dev, 0x24); diff --git a/src/soc/intel/broadwell/serialio.c b/src/soc/intel/broadwell/serialio.c index a4922f7..8ad9b54 100644 --- a/src/soc/intel/broadwell/serialio.c +++ b/src/soc/intel/broadwell/serialio.c @@ -160,14 +160,14 @@ config_t *config = config_of(dev); struct resource *bar0, *bar1; int sio_index = -1; - u32 reg32; + u16 reg16;
printk(BIOS_DEBUG, "Initializing Serial IO device\n");
/* Ensure memory and bus master are enabled */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY; - pci_write_config32(dev, PCI_COMMAND, reg32); + reg16 = pci_read_config16(dev, PCI_COMMAND); + reg16 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY; + pci_write_config16(dev, PCI_COMMAND, reg16);
/* Find BAR0 and BAR1 */ bar0 = find_resource(dev, PCI_BASE_ADDRESS_0); diff --git a/src/soc/intel/broadwell/smihandler.c b/src/soc/intel/broadwell/smihandler.c index bce157d..1d92c12 100644 --- a/src/soc/intel/broadwell/smihandler.c +++ b/src/soc/intel/broadwell/smihandler.c @@ -69,7 +69,7 @@
for (slot = 0; slot < 0x20; slot++) { for (func = 0; func < 8; func++) { - u32 reg32; + u16 reg16;
pci_devfn_t dev = PCI_DEV(bus, slot, func); val = pci_read_config32(dev, PCI_VENDOR_ID); @@ -79,9 +79,9 @@ continue;
/* Disable Bus Mastering for this one device */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 &= ~PCI_COMMAND_MASTER; - pci_write_config32(dev, PCI_COMMAND, reg32); + reg16 = pci_read_config16(dev, PCI_COMMAND); + reg16 &= ~PCI_COMMAND_MASTER; + pci_write_config16(dev, PCI_COMMAND, reg16);
/* If this is a bridge, then follow it. */ hdr = pci_read_config8(dev, PCI_HEADER_TYPE);
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40850
to look at the new patch set (#3).
Change subject: soc/intel/broadwell: Fix 16-bit read/write PCI_COMMAND register ......................................................................
soc/intel/broadwell: Fix 16-bit read/write PCI_COMMAND register
Change-Id: I0fd1a758d8838b3eea5640b41eee6a6893360aa3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/soc/intel/broadwell/adsp.c M src/soc/intel/broadwell/hda.c M src/soc/intel/broadwell/me.c M src/soc/intel/broadwell/minihd.c M src/soc/intel/broadwell/pch.c M src/soc/intel/broadwell/pcie.c M src/soc/intel/broadwell/serialio.c M src/soc/intel/broadwell/smihandler.c 8 files changed, 25 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/40850/3
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40850 )
Change subject: soc/intel/broadwell: Fix 16-bit read/write PCI_COMMAND register ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40850 )
Change subject: soc/intel/broadwell: Fix 16-bit read/write PCI_COMMAND register ......................................................................
soc/intel/broadwell: Fix 16-bit read/write PCI_COMMAND register
Change-Id: I0fd1a758d8838b3eea5640b41eee6a6893360aa3 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr Reviewed-on: https://review.coreboot.org/c/coreboot/+/40850 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Georgi pgeorgi@google.com --- M src/soc/intel/broadwell/adsp.c M src/soc/intel/broadwell/hda.c M src/soc/intel/broadwell/me.c M src/soc/intel/broadwell/minihd.c M src/soc/intel/broadwell/pch.c M src/soc/intel/broadwell/pcie.c M src/soc/intel/broadwell/serialio.c M src/soc/intel/broadwell/smihandler.c 8 files changed, 25 insertions(+), 42 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
diff --git a/src/soc/intel/broadwell/adsp.c b/src/soc/intel/broadwell/adsp.c index 82904de..64b7d5e 100644 --- a/src/soc/intel/broadwell/adsp.c +++ b/src/soc/intel/broadwell/adsp.c @@ -24,9 +24,7 @@ u32 tmp32;
/* Ensure memory and bus master are enabled */ - tmp32 = pci_read_config32(dev, PCI_COMMAND); - tmp32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY; - pci_write_config32(dev, PCI_COMMAND, tmp32); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY);
/* Find BAR0 and BAR1 */ bar0 = find_resource(dev, PCI_BASE_ADDRESS_0); diff --git a/src/soc/intel/broadwell/hda.c b/src/soc/intel/broadwell/hda.c index 476d092..c5ce2e4 100644 --- a/src/soc/intel/broadwell/hda.c +++ b/src/soc/intel/broadwell/hda.c @@ -84,7 +84,6 @@ u8 *base; struct resource *res; u32 codec_mask; - u32 reg32;
/* Find base address */ res = find_resource(dev, PCI_BASE_ADDRESS_0); @@ -95,8 +94,7 @@ printk(BIOS_DEBUG, "HDA: base = %p\n", base);
/* Set Bus Master */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - pci_write_config32(dev, PCI_COMMAND, reg32 | PCI_COMMAND_MASTER); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
hda_pch_init(dev, base);
@@ -110,7 +108,7 @@
static void hda_enable(struct device *dev) { - u32 reg32; + u16 reg16; u8 reg8;
reg8 = pci_read_config8(dev, 0x43); @@ -126,10 +124,10 @@ printk(BIOS_INFO, "HDA disabled, I/O buffers routed to ADSP\n");
/* Ensure memory, io, and bus master are all disabled */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 &= ~(PCI_COMMAND_MASTER | + reg16 = pci_read_config16(dev, PCI_COMMAND); + reg16 &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_write_config16(dev, PCI_COMMAND, reg16);
/* Disable this device */ pch_disable_devfn(dev); diff --git a/src/soc/intel/broadwell/me.c b/src/soc/intel/broadwell/me.c index 730d77e..afe9c82 100644 --- a/src/soc/intel/broadwell/me.c +++ b/src/soc/intel/broadwell/me.c @@ -600,17 +600,17 @@
static void intel_me_finalize(struct device *dev) { - u32 reg32; + u16 reg16;
/* S3 path will have hidden this device already */ if (!mei_base_address || mei_base_address == (u8 *) 0xfffffff0) return;
/* Make sure IO is disabled */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 &= ~(PCI_COMMAND_MASTER | + reg16 = pci_read_config16(dev, PCI_COMMAND); + reg16 &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_write_config16(dev, PCI_COMMAND, reg16);
/* Hide the PCI device */ RCBA32_OR(FD2, PCH_DISABLE_MEI1); @@ -712,7 +712,6 @@ { struct resource *res; struct mei_csr host; - u32 reg32;
/* Find the MMIO base for the ME interface */ res = find_resource(dev, PCI_BASE_ADDRESS_0); @@ -723,9 +722,7 @@ mei_base_address = res2mmio(res, 0, 0);
/* Ensure Memory and Bus Master bits are set */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY; - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY);
/* Clean up status for next message */ read_host_csr(&host); diff --git a/src/soc/intel/broadwell/minihd.c b/src/soc/intel/broadwell/minihd.c index 2dcab97..da0b42a 100644 --- a/src/soc/intel/broadwell/minihd.c +++ b/src/soc/intel/broadwell/minihd.c @@ -62,8 +62,7 @@ printk(BIOS_DEBUG, "Mini-HD: base = %p\n", base);
/* Set Bus Master */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - pci_write_config32(dev, PCI_COMMAND, reg32 | PCI_COMMAND_MASTER); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
/* Mini-HD configuration */ reg32 = read32(base + 0x100c); diff --git a/src/soc/intel/broadwell/pch.c b/src/soc/intel/broadwell/pch.c index f6f3746..ac6ac2a 100644 --- a/src/soc/intel/broadwell/pch.c +++ b/src/soc/intel/broadwell/pch.c @@ -171,7 +171,7 @@
void broadwell_pch_enable_dev(struct device *dev) { - u32 reg32; + u16 reg16;
/* These devices need special enable/disable handling */ switch (PCI_SLOT(dev->path.pci.devfn)) { @@ -185,18 +185,16 @@ printk(BIOS_DEBUG, "%s: Disabling device\n", dev_path(dev));
/* Ensure memory, io, and bus master are all disabled */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 &= ~(PCI_COMMAND_MASTER | + reg16 = pci_read_config16(dev, PCI_COMMAND); + reg16 &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_write_config16(dev, PCI_COMMAND, reg16);
/* Disable this device if possible */ pch_disable_devfn(dev); } else { /* Enable SERR */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_SERR; - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_SERR); } }
diff --git a/src/soc/intel/broadwell/pcie.c b/src/soc/intel/broadwell/pcie.c index f81f042..d506057 100644 --- a/src/soc/intel/broadwell/pcie.c +++ b/src/soc/intel/broadwell/pcie.c @@ -574,19 +574,14 @@ static void pch_pcie_init(struct device *dev) { u16 reg16; - u32 reg32;
printk(BIOS_DEBUG, "Initializing PCH PCIe bridge.\n");
/* Enable SERR */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_SERR; - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_SERR);
/* Enable Bus Master */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_MASTER; - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
/* Set Cache Line Size to 0x10 */ pci_write_config8(dev, 0x0c, 0x10); @@ -597,6 +592,7 @@ pci_write_config16(dev, PCI_BRIDGE_CONTROL, reg16);
#ifdef EVEN_MORE_DEBUG + u32 reg32; reg32 = pci_read_config32(dev, 0x20); printk(BIOS_SPEW, " MBL = 0x%08x\n", reg32); reg32 = pci_read_config32(dev, 0x24); diff --git a/src/soc/intel/broadwell/serialio.c b/src/soc/intel/broadwell/serialio.c index a4922f7..9e6cf32 100644 --- a/src/soc/intel/broadwell/serialio.c +++ b/src/soc/intel/broadwell/serialio.c @@ -160,14 +160,11 @@ config_t *config = config_of(dev); struct resource *bar0, *bar1; int sio_index = -1; - u32 reg32;
printk(BIOS_DEBUG, "Initializing Serial IO device\n");
/* Ensure memory and bus master are enabled */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 |= PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY; - pci_write_config32(dev, PCI_COMMAND, reg32); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY);
/* Find BAR0 and BAR1 */ bar0 = find_resource(dev, PCI_BASE_ADDRESS_0); diff --git a/src/soc/intel/broadwell/smihandler.c b/src/soc/intel/broadwell/smihandler.c index bce157d..1d92c12 100644 --- a/src/soc/intel/broadwell/smihandler.c +++ b/src/soc/intel/broadwell/smihandler.c @@ -69,7 +69,7 @@
for (slot = 0; slot < 0x20; slot++) { for (func = 0; func < 8; func++) { - u32 reg32; + u16 reg16;
pci_devfn_t dev = PCI_DEV(bus, slot, func); val = pci_read_config32(dev, PCI_VENDOR_ID); @@ -79,9 +79,9 @@ continue;
/* Disable Bus Mastering for this one device */ - reg32 = pci_read_config32(dev, PCI_COMMAND); - reg32 &= ~PCI_COMMAND_MASTER; - pci_write_config32(dev, PCI_COMMAND, reg32); + reg16 = pci_read_config16(dev, PCI_COMMAND); + reg16 &= ~PCI_COMMAND_MASTER; + pci_write_config16(dev, PCI_COMMAND, reg16);
/* If this is a bridge, then follow it. */ hdr = pci_read_config8(dev, PCI_HEADER_TYPE);
-
HAOUAS Elyes (Code Review) -
Patrick Georgi (Code Review)