Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42152 )
Change subject: sb/intel/bd82x6x: Use PCI bitwise ops ......................................................................
sb/intel/bd82x6x: Use PCI bitwise ops
Some cases could not be factored out while keeping reproducibility.
Tested with BUILD_TIMELESS=1, Asus P8Z77-V LX2 does not change.
Change-Id: Iafe62d952a146bf53a28a1a83b87a3ae31f46720 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/bd82x6x/azalia.c M src/southbridge/intel/bd82x6x/bootblock.c M src/southbridge/intel/bd82x6x/early_thermal.c M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/bd82x6x/me.c M src/southbridge/intel/bd82x6x/me_8.x.c M src/southbridge/intel/bd82x6x/pch.c M src/southbridge/intel/bd82x6x/pci.c M src/southbridge/intel/bd82x6x/pcie.c M src/southbridge/intel/bd82x6x/sata.c M src/southbridge/intel/bd82x6x/smbus.c M src/southbridge/intel/bd82x6x/smihandler.c M src/southbridge/intel/bd82x6x/usb_xhci.c 13 files changed, 88 insertions(+), 162 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/42152/1
diff --git a/src/southbridge/intel/bd82x6x/azalia.c b/src/southbridge/intel/bd82x6x/azalia.c index 470c67c..4f5d8ca 100644 --- a/src/southbridge/intel/bd82x6x/azalia.c +++ b/src/southbridge/intel/bd82x6x/azalia.c @@ -215,8 +215,6 @@ u8 *base; struct resource *res; u32 codec_mask; - u8 reg8; - u16 reg16; u32 reg32;
/* Find base address */ @@ -236,48 +234,30 @@ reg32 |= RCBA32(CIR31) & 0xfe; pci_write_config32(dev, 0x120, reg32);
- reg16 = pci_read_config16(dev, 0x78); - reg16 |= (1 << 11); - pci_write_config16(dev, 0x78, reg16); + pci_or_config16(dev, 0x78, 1 << 11); } else printk(BIOS_DEBUG, "Azalia: V1CTL disabled.\n");
- reg32 = pci_read_config32(dev, 0x114); - reg32 &= ~0xfe; - pci_write_config32(dev, 0x114, reg32); + pci_and_config32(dev, 0x114, ~0xfe);
// Set VCi enable bit - reg32 = pci_read_config32(dev, 0x120); - reg32 |= (1 << 31); - pci_write_config32(dev, 0x120, reg32); + pci_or_config32(dev, 0x120, 1 << 31);
// Enable HDMI codec: - reg32 = pci_read_config32(dev, 0xc4); - reg32 |= (1 << 1); - pci_write_config32(dev, 0xc4, reg32); + pci_or_config32(dev, 0xc4, 1 << 1);
- reg8 = pci_read_config8(dev, 0x43); - reg8 |= (1 << 6); - pci_write_config8(dev, 0x43, reg8); + pci_or_config8(dev, 0x43, 1 << 6);
/* Additional programming steps */ - reg32 = pci_read_config32(dev, 0xc4); - reg32 |= (1 << 13); - pci_write_config32(dev, 0xc4, reg32); + pci_or_config32(dev, 0xc4, 1 << 13);
- reg32 = pci_read_config32(dev, 0xc4); - reg32 |= (1 << 10); - pci_write_config32(dev, 0xc4, reg32); + pci_or_config32(dev, 0xc4, 1 << 10);
- reg32 = pci_read_config32(dev, 0xd0); - reg32 &= ~(1 << 31); - pci_write_config32(dev, 0xd0, reg32); + pci_and_config32(dev, 0xd0, ~(1 << 31));
if (dev->device == 0x1e20) { /* Additional step on Panther Point */ - reg32 = pci_read_config32(dev, 0xc4); - reg32 |= (1 << 17); - pci_write_config32(dev, 0xc4, reg32); + pci_or_config32(dev, 0xc4, 1 << 17); }
/* Set Bus Master */ @@ -294,14 +274,11 @@ /* Wait 1ms */ udelay(1000);
- // - reg8 = pci_read_config8(dev, 0x40); // Audio Control - reg8 |= 1; // Select Azalia mode. This needs to be controlled via devicetree.cb - pci_write_config8(dev, 0x40, reg8); + // Select Azalia mode. This needs to be controlled via devicetree.cb + pci_or_config8(dev, 0x40, 1); // Audio Control
- reg8 = pci_read_config8(dev, 0x4d); // Docking Status - reg8 &= ~(1 << 7); // Docking not supported - pci_write_config8(dev, 0x4d, reg8); + // Docking not supported + pci_and_config8(dev, 0x4d, (u8)~(1 << 7)); // Docking Status
codec_mask = codec_detect(base);
@@ -311,10 +288,7 @@ }
/* Enable dynamic clock gating */ - reg8 = pci_read_config8(dev, 0x43); - reg8 &= ~0x7; - reg8 |= (1 << 2) | (1 << 0); - pci_write_config8(dev, 0x43, reg8); + pci_update_config8(dev, 0x43, ~0x07, (1 << 2) | (1 << 0)); }
static const char *azalia_acpi_name(const struct device *dev) diff --git a/src/southbridge/intel/bd82x6x/bootblock.c b/src/southbridge/intel/bd82x6x/bootblock.c index ab5cbf0..3a99f51 100644 --- a/src/southbridge/intel/bd82x6x/bootblock.c +++ b/src/southbridge/intel/bd82x6x/bootblock.c @@ -9,13 +9,7 @@ */ static void enable_spi_prefetch(void) { - u8 reg8; - pci_devfn_t dev = PCH_LPC_DEV; - - reg8 = pci_read_config8(dev, BIOS_CNTL); - reg8 &= ~(3 << 2); - reg8 |= (2 << 2); /* Prefetching and Caching Enabled */ - pci_write_config8(dev, BIOS_CNTL, reg8); + pci_update_config8(PCH_LPC_DEV, BIOS_CNTL, ~(3 << 2), 2 << 2); }
static void enable_port80_on_lpc(void) diff --git a/src/southbridge/intel/bd82x6x/early_thermal.c b/src/southbridge/intel/bd82x6x/early_thermal.c index 0abd85cb..b3510ac 100644 --- a/src/southbridge/intel/bd82x6x/early_thermal.c +++ b/src/southbridge/intel/bd82x6x/early_thermal.c @@ -38,7 +38,7 @@ pci_write_config32(dev, 0x44, 0x0);
/* Activate temporary BAR. */ - pci_write_config32(dev, 0x40, pci_read_config32(dev, 0x40) | 5); + pci_or_config32(dev, 0x40, 5);
write16p(TBARB_TEMP + 0x04, 0x3a2b);
@@ -61,7 +61,8 @@ write16p(TBARB_TEMP + 0x1a, (read16p(TBARB_TEMP + 0x1a) & ~0xf) | 0x10f0);
/* Disable temporary BAR */ - pci_write_config32(dev, 0x40, pci_read_config32(dev, 0x40) & ~1); + pci_and_config32(dev, 0x40, ~1); + pci_write_config32(dev, 0x40, 0);
write32(DEFAULT_RCBA + 0x38b0, (read32(DEFAULT_RCBA + 0x38b0) & 0xffff8003) | 0x403c); diff --git a/src/southbridge/intel/bd82x6x/lpc.c b/src/southbridge/intel/bd82x6x/lpc.c index 691db62..0c0d902 100644 --- a/src/southbridge/intel/bd82x6x/lpc.c +++ b/src/southbridge/intel/bd82x6x/lpc.c @@ -417,22 +417,15 @@
static void pch_disable_smm_only_flashing(struct device *dev) { - u8 reg8; - printk(BIOS_SPEW, "Enabling BIOS updates outside of SMM... "); - reg8 = pci_read_config8(dev, BIOS_CNTL); - reg8 &= ~(1 << 5); - pci_write_config8(dev, BIOS_CNTL, reg8); + + pci_and_config8(dev, BIOS_CNTL, ~(1 << 5)); }
static void pch_fixups(struct device *dev) { - u8 gen_pmcon_2; - /* Indicate DRAM init done for MRC S3 to know it can resume */ - gen_pmcon_2 = pci_read_config8(dev, GEN_PMCON_2); - gen_pmcon_2 |= (1 << 7); - pci_write_config8(dev, GEN_PMCON_2, gen_pmcon_2); + pci_or_config8(dev, GEN_PMCON_2, 1 << 7);
/* * Enable DMI ASPM in the PCH diff --git a/src/southbridge/intel/bd82x6x/me.c b/src/southbridge/intel/bd82x6x/me.c index bc0a71e..cce3708 100644 --- a/src/southbridge/intel/bd82x6x/me.c +++ b/src/southbridge/intel/bd82x6x/me.c @@ -438,7 +438,6 @@ { struct me_hfs hfs; u32 reg32; - u16 reg16;
mei_base_address = (u32 *) (pci_read_config32(PCH_ME_DEV, PCI_BASE_ADDRESS_0) & ~0xf); @@ -461,10 +460,8 @@ mkhi_end_of_post();
/* Make sure IO is disabled */ - reg16 = pci_read_config16(PCH_ME_DEV, PCI_COMMAND); - reg16 &= ~(PCI_COMMAND_MASTER | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config16(PCH_ME_DEV, PCI_COMMAND, reg16); + pci_and_config16(PCH_ME_DEV, PCI_COMMAND, + ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO));
/* Hide the PCI device */ RCBA32_OR(FD2, PCH_DISABLE_MEI1); diff --git a/src/southbridge/intel/bd82x6x/me_8.x.c b/src/southbridge/intel/bd82x6x/me_8.x.c index f64e29c..cbb7463 100644 --- a/src/southbridge/intel/bd82x6x/me_8.x.c +++ b/src/southbridge/intel/bd82x6x/me_8.x.c @@ -435,7 +435,6 @@ { struct me_hfs hfs; u32 reg32; - u16 reg16;
mei_base_address = (void *) (pci_read_config32(PCH_ME_DEV, PCI_BASE_ADDRESS_0) & ~0xf); @@ -458,10 +457,8 @@ mkhi_end_of_post();
/* Make sure IO is disabled */ - reg16 = pci_read_config16(PCH_ME_DEV, PCI_COMMAND); - reg16 &= ~(PCI_COMMAND_MASTER | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config16(PCH_ME_DEV, PCI_COMMAND, reg16); + pci_and_config16(PCH_ME_DEV, PCI_COMMAND, + ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO));
/* Hide the PCI device */ RCBA32_OR(FD2, PCH_DISABLE_MEI1); diff --git a/src/southbridge/intel/bd82x6x/pch.c b/src/southbridge/intel/bd82x6x/pch.c index 00d5e12..7b0662b 100644 --- a/src/southbridge/intel/bd82x6x/pch.c +++ b/src/southbridge/intel/bd82x6x/pch.c @@ -301,7 +301,6 @@ static void pch_pcie_enable(struct device *dev) { struct southbridge_intel_bd82x6x_config *config = dev->chip_info; - u16 reg16;
if (!config) return; @@ -345,9 +344,7 @@ /* Handle workaround for PPT and CPT/B1+ */ if (pch_silicon_supported(PCH_TYPE_CPT, PCH_STEP_B1) && !pch_pcie_check_set_enabled(dev)) { - u8 reg8 = pci_read_config8(dev, 0xe2); - reg8 |= 1; - pci_write_config8(dev, 0xe2, reg8); + pci_or_config8(dev, 0xe2, 1); }
/* @@ -358,10 +355,8 @@ }
/* Ensure memory, io, and bus master are all disabled */ - reg16 = pci_read_config16(dev, PCI_COMMAND); - reg16 &= ~(PCI_COMMAND_MASTER | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config16(dev, PCI_COMMAND, reg16); + pci_and_config16(dev, PCI_COMMAND, + ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO));
/* Do not claim downstream transactions for PCIe ports */ new_rpfn |= RPFN_HIDE(PCI_FUNC(dev->path.pci.devfn)); @@ -408,8 +403,6 @@
void pch_enable(struct device *dev) { - u16 reg16; - /* PCH PCIe Root Ports get special handling */ if (PCI_SLOT(dev->path.pci.devfn) == PCH_PCIE_DEV_SLOT) return pch_pcie_enable(dev); @@ -418,10 +411,8 @@ printk(BIOS_DEBUG, "%s: Disabling device\n", dev_path(dev));
/* Ensure memory, io, and bus master are all disabled */ - reg16 = pci_read_config16(dev, PCI_COMMAND); - reg16 &= ~(PCI_COMMAND_MASTER | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config16(dev, PCI_COMMAND, reg16); + pci_and_config16(dev, PCI_COMMAND, + ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO));
/* Hide this device if possible */ pch_hide_devfn(dev->path.pci.devfn); diff --git a/src/southbridge/intel/bd82x6x/pci.c b/src/southbridge/intel/bd82x6x/pci.c index e61c60c..895135b 100644 --- a/src/southbridge/intel/bd82x6x/pci.c +++ b/src/southbridge/intel/bd82x6x/pci.c @@ -11,30 +11,22 @@ static void pci_init(struct device *dev) { u16 reg16; - u8 reg8;
printk(BIOS_DEBUG, "PCI init.\n"); /* Enable Bus Master */ - reg16 = pci_read_config16(dev, PCI_COMMAND); - reg16 |= PCI_COMMAND_MASTER; - pci_write_config16(dev, PCI_COMMAND, reg16); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
/* This device has no interrupt */ pci_write_config8(dev, INTR, 0xff);
/* disable parity error response and SERR */ - reg16 = pci_read_config16(dev, PCI_BRIDGE_CONTROL); - reg16 &= ~PCI_BRIDGE_CTL_PARITY; - reg16 &= ~PCI_BRIDGE_CTL_SERR; - pci_write_config16(dev, PCI_BRIDGE_CONTROL, reg16); + pci_and_config16(dev, PCI_BRIDGE_CONTROL, + ~(PCI_BRIDGE_CTL_PARITY | PCI_BRIDGE_CTL_SERR));
/* Master Latency Count must be set to 0x04! */ - reg8 = pci_read_config8(dev, SMLT); - reg8 &= 0x07; - reg8 |= (0x04 << 3); - pci_write_config8(dev, SMLT, reg8); + pci_update_config8(dev, SMLT, 0x07, (0x04 << 3));
- /* Clear errors in status registers */ + /* Clear errors in status registers. FIXME: do we need to do something? */ reg16 = pci_read_config16(dev, PSTS); //reg16 |= 0xf900; pci_write_config16(dev, PSTS, reg16); diff --git a/src/southbridge/intel/bd82x6x/pcie.c b/src/southbridge/intel/bd82x6x/pcie.c index 5bc3412..48e5037 100644 --- a/src/southbridge/intel/bd82x6x/pcie.c +++ b/src/southbridge/intel/bd82x6x/pcie.c @@ -89,9 +89,7 @@ pci_write_config8(dev, 0xe1, reg8);
/* Set 0xE8[0] = 1 */ - reg32 = pci_read_config32(dev, 0xe8); - reg32 |= 1; - pci_write_config32(dev, 0xe8, reg32); + pci_or_config32(dev, 0xe8, 1);
/* Adjust Common Clock exit latency */ reg32 = pci_read_config32(dev, 0xd8); @@ -124,38 +122,24 @@ { struct southbridge_intel_bd82x6x_config *config = dev->chip_info; enum aspm_type apmc = 0; - u32 reg32;
/* Set 0x314 = 0x743a361b */ pci_write_config32(dev, 0x314, 0x743a361b);
/* Set 0x318[31:16] = 0x1414 */ - reg32 = pci_read_config32(dev, 0x318); - reg32 &= 0x0000ffff; - reg32 |= 0x14140000; - pci_write_config32(dev, 0x318, reg32); + pci_update_config32(dev, 0x318, 0x0000ffff, 0x14140000);
/* Set 0x324[5] = 1 */ - reg32 = pci_read_config32(dev, 0x324); - reg32 |= (1 << 5); - pci_write_config32(dev, 0x324, reg32); + pci_or_config32(dev, 0x324, 1 << 5);
/* Set 0x330[7:0] = 0x40 */ - reg32 = pci_read_config32(dev, 0x330); - reg32 &= ~(0xff); - reg32 |= 0x40; - pci_write_config32(dev, 0x330, reg32); + pci_update_config32(dev, 0x330, ~0xff, 0x40);
/* Set 0x33C[24:0] = 0x854c74 */ - reg32 = pci_read_config32(dev, 0x33c); - reg32 &= 0xff000000; - reg32 |= 0x00854c74; - pci_write_config32(dev, 0x33c, reg32); + pci_update_config32(dev, 0x33c, 0xff000000, 0x00854c74);
/* No IO-APIC, Disable EOI forwarding */ - reg32 = pci_read_config32(dev, 0xd4); - reg32 |= (1 << 1); - pci_write_config32(dev, 0xd4, reg32); + pci_or_config32(dev, 0xd4, 1 << 1);
/* Check for a rootport ASPM override */ switch (PCI_FUNC(dev->path.pci.devfn)) { @@ -187,19 +171,15 @@
/* Setup the override or get the real ASPM setting */ if (apmc) { - reg32 = pci_read_config32(dev, 0xd4); - reg32 |= (apmc << 2) | (1 << 4); - pci_write_config32(dev, 0xd4, reg32); + pci_or_config32(dev, 0xd4, (apmc << 2) | (1 << 4)); + } else { apmc = pci_read_config32(dev, 0x50) & 3; }
/* If both L0s and L1 enabled then set root port 0xE8[1]=1 */ - if (apmc == PCIE_ASPM_BOTH) { - reg32 = pci_read_config32(dev, 0xe8); - reg32 |= (1 << 1); - pci_write_config32(dev, 0xe8, reg32); - } + if (apmc == PCIE_ASPM_BOTH) + pci_or_config32(dev, 0xe8, 1 << 1); }
static void pci_init(struct device *dev) @@ -216,10 +196,8 @@ // This has no effect but the OS might expect it pci_write_config8(dev, 0x0c, 0x10);
- 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); + pci_update_config16(dev, PCI_BRIDGE_CONTROL, + ~PCI_BRIDGE_CTL_PARITY, PCI_BRIDGE_CTL_NO_ISA);
#ifdef EVEN_MORE_DEBUG u32 reg32; @@ -233,7 +211,7 @@ printk(BIOS_SPEW, " PMLU32 = 0x%08x\n", reg32); #endif
- /* Clear errors in status registers */ + /* Clear errors in status registers. FIXME: Do something? */ reg16 = pci_read_config16(dev, 0x06); //reg16 |= 0xf900; pci_write_config16(dev, 0x06, reg16); @@ -244,9 +222,7 @@
/* Enable expresscard hotplug events. */ if (config->pcie_hotplug_map[PCI_FUNC(dev->path.pci.devfn)]) { - pci_write_config32(dev, 0xd8, - pci_read_config32(dev, 0xd8) - | (1 << 30)); + pci_or_config32(dev, 0xd8, 1 << 30); pci_write_config16(dev, 0x42, 0x142); } } @@ -272,6 +248,30 @@ pch_pcie_pm_late(dev); }
+/* FIXME: That ASSERT() macro contains the line number... Add padding so that it's the same */ + + + + + + + + + + + + + + + + + + + + + + + static const char *pch_pcie_acpi_name(const struct device *dev) { ASSERT(dev); diff --git a/src/southbridge/intel/bd82x6x/sata.c b/src/southbridge/intel/bd82x6x/sata.c index 1974e2b..63801a2 100644 --- a/src/southbridge/intel/bd82x6x/sata.c +++ b/src/southbridge/intel/bd82x6x/sata.c @@ -138,8 +138,7 @@ pci_write_config16(dev, 0x92, reg16);
/* SATA Initialization register */ - pci_write_config32(dev, 0x94, - ((config->sata_port_map ^ 0x3f) << 24) | 0x183); + pci_write_config32(dev, 0x94, ((config->sata_port_map ^ 0x3f) << 24) | 0x183);
/* Initialize AHCI memory-mapped space */ abar = (u8 *)pci_read_config32(dev, PCI_BASE_ADDRESS_5); @@ -172,9 +171,7 @@ /* IDE */
/* Without AHCI BAR no memory decoding */ - reg16 = pci_read_config16(dev, PCI_COMMAND); - reg16 &= ~PCI_COMMAND_MEMORY; - pci_write_config16(dev, PCI_COMMAND, reg16); + pci_and_config16(dev, PCI_COMMAND, ~PCI_COMMAND_MEMORY);
if (sata_mode == 1) { /* Native mode on both primary and secondary. */ @@ -182,7 +179,7 @@ printk(BIOS_DEBUG, "SATA: Controller in IDE compat mode.\n"); } else { /* Legacy mode on both primary and secondary. */ - pci_update_config8(dev, 0x09, ~0x05, 0x00); + pci_and_config8(dev, 0x09, ~0x05); printk(BIOS_DEBUG, "SATA: Controller in IDE legacy mode.\n"); }
@@ -191,14 +188,10 @@ pci_write_config16(dev, IDE_TIM_SEC, IDE_DECODE_ENABLE);
/* Port enable + OOB retry mode */ - reg16 = pci_read_config16(dev, 0x92); - reg16 &= ~0x3f; - reg16 |= config->sata_port_map | 0x8000; - pci_write_config16(dev, 0x92, reg16); + pci_update_config16(dev, 0x92, ~0x3f, config->sata_port_map | 0x8000);
/* SATA Initialization register */ - pci_write_config32(dev, 0x94, - ((config->sata_port_map ^ 0x3f) << 24) | 0x183); + pci_write_config32(dev, 0x94, ((config->sata_port_map ^ 0x3f) << 24) | 0x183); }
/* Set Gen3 Transmitter settings if needed */ diff --git a/src/southbridge/intel/bd82x6x/smbus.c b/src/southbridge/intel/bd82x6x/smbus.c index 1b72ea5..dcd2724 100644 --- a/src/southbridge/intel/bd82x6x/smbus.c +++ b/src/southbridge/intel/bd82x6x/smbus.c @@ -15,6 +15,7 @@ u16 reg16;
/* Enable clock gating */ + /* FIXME: Using 32-bit ops with a 16-bit variable is a bug! These should be 16-bit! */ reg16 = pci_read_config32(dev, 0x80); reg16 &= ~((1 << 8)|(1 << 10)|(1 << 12)|(1 << 14)); pci_write_config32(dev, 0x80, reg16); diff --git a/src/southbridge/intel/bd82x6x/smihandler.c b/src/southbridge/intel/bd82x6x/smihandler.c index 57b5959..83799bee 100644 --- a/src/southbridge/intel/bd82x6x/smihandler.c +++ b/src/southbridge/intel/bd82x6x/smihandler.c @@ -99,15 +99,15 @@ switch (slp_typ) { case ACPI_S3: case ACPI_S4: + /* FIXME: Unbalanced width in read/write ops (16-bit read then 32-bit write) */ reg16 = pci_read_config16(PCH_XHCI_DEV, 0x74); reg16 &= ~0x03UL; pci_write_config32(PCH_XHCI_DEV, 0x74, reg16);
- pci_or_config16(PCH_XHCI_DEV, PCI_COMMAND, PCI_COMMAND_MASTER | - PCI_COMMAND_MEMORY); + pci_or_config16(PCH_XHCI_DEV, PCI_COMMAND, + PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY);
- xhci_bar = pci_read_config32(PCH_XHCI_DEV, - PCI_BASE_ADDRESS_0) & ~0xFUL; + xhci_bar = pci_read_config32(PCH_XHCI_DEV, PCI_BASE_ADDRESS_0) & ~0xFUL;
if ((xhci_bar + 0x4C0) & 1) pch_iobp_update(0xEC000082, ~0UL, (3 << 2)); @@ -118,19 +118,14 @@ if ((xhci_bar + 0x4F0) & 1) pch_iobp_update(0xEC000382, ~0UL, (3 << 2));
- reg16 = pci_read_config16(PCH_XHCI_DEV, PCI_COMMAND); - reg16 &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY); - pci_write_config16(PCH_XHCI_DEV, PCI_COMMAND, reg16); + pci_and_config16(PCH_XHCI_DEV, PCI_COMMAND, + ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY));
- reg16 = pci_read_config16(PCH_XHCI_DEV, 0x74); - reg16 |= 0x03; - pci_write_config16(PCH_XHCI_DEV, 0x74, reg16); + pci_or_config16(PCH_XHCI_DEV, 0x74, 0x03); break;
case ACPI_S5: - reg16 = pci_read_config16(PCH_XHCI_DEV, 0x74); - reg16 |= ((1 << 8) | 0x03); - pci_write_config16(PCH_XHCI_DEV, 0x74, reg16); + pci_or_config16(PCH_XHCI_DEV, 0x74, (1 << 8) | 0x03); break; } } diff --git a/src/southbridge/intel/bd82x6x/usb_xhci.c b/src/southbridge/intel/bd82x6x/usb_xhci.c index 5a5b418..8bd0641 100644 --- a/src/southbridge/intel/bd82x6x/usb_xhci.c +++ b/src/southbridge/intel/bd82x6x/usb_xhci.c @@ -20,9 +20,7 @@ pci_write_config32(dev, XOCM, config->xhci_overcurrent_mapping);
/* lock overcurrent map */ - reg32 = pci_read_config32(dev, 0x44); - reg32 |= 1; - pci_write_config32(dev, 0x44, reg32); + pci_or_config32(dev, 0x44, 1);
pci_write_config32(dev, XUSB2PRM, config->xhci_switchable_ports); pci_write_config32(dev, USB3PRM, config->superspeed_capable_ports);
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42152 )
Change subject: sb/intel/bd82x6x: Use PCI bitwise ops ......................................................................
Patch Set 1:
Missed one case in early_me.c:
printk(BIOS_NOTICE, "CPU was replaced & warm reset required...\n"); reg16 = pci_read_config16(PCI_DEV(0, 31, 0), 0xa2) & ~0x80; pci_write_config16(PCI_DEV(0, 31, 0), 0xa2, reg16);
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42152
to look at the new patch set (#2).
Change subject: sb/intel/bd82x6x: Use PCI bitwise ops ......................................................................
sb/intel/bd82x6x: Use PCI bitwise ops
Some cases could not be factored out while keeping reproducibility. Also mark some potential bugs with a FIXME comment, since fixing them while also keeping the binary unchanged is pretty much impossible.
Tested with BUILD_TIMELESS=1, Asus P8Z77-V LX2 does not change.
Change-Id: Iafe62d952a146bf53a28a1a83b87a3ae31f46720 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/bd82x6x/azalia.c M src/southbridge/intel/bd82x6x/bootblock.c M src/southbridge/intel/bd82x6x/early_me.c M src/southbridge/intel/bd82x6x/early_thermal.c M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/bd82x6x/me.c M src/southbridge/intel/bd82x6x/me_8.x.c M src/southbridge/intel/bd82x6x/pch.c M src/southbridge/intel/bd82x6x/pci.c M src/southbridge/intel/bd82x6x/pcie.c M src/southbridge/intel/bd82x6x/sata.c M src/southbridge/intel/bd82x6x/smbus.c M src/southbridge/intel/bd82x6x/smihandler.c M src/southbridge/intel/bd82x6x/usb_xhci.c 14 files changed, 89 insertions(+), 165 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/42152/2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42152 )
Change subject: sb/intel/bd82x6x: Use PCI bitwise ops ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42152/4/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/pcie.c:
https://review.coreboot.org/c/coreboot/+/42152/4/src/southbridge/intel/bd82x... PS4, Line 251: /* FIXME: That ASSERT() macro contains the line number... Add padding so that it's the same */ I'm fine with this, just put immediate followup to remove.
https://review.coreboot.org/c/coreboot/+/42152/4/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/smihandler.c:
https://review.coreboot.org/c/coreboot/+/42152/4/src/southbridge/intel/bd82x... PS4, Line 112: if ((xhci_bar + 0x4C0) & 1) I dont get this.. CB:2759
(*((u8 *)xhci_bar + 0x4c0) & 1)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42152 )
Change subject: sb/intel/bd82x6x: Use PCI bitwise ops ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42152/4/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/pcie.c:
https://review.coreboot.org/c/coreboot/+/42152/4/src/southbridge/intel/bd82x... PS4, Line 251: /* FIXME: That ASSERT() macro contains the line number... Add padding so that it's the same */
I'm fine with this, just put immediate followup to remove.
I forgot... I think I can do something less horrible: move `pch_pcie_acpi_name` at the beginning of the file
https://review.coreboot.org/c/coreboot/+/42152/4/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/smihandler.c:
https://review.coreboot.org/c/coreboot/+/42152/4/src/southbridge/intel/bd82x... PS4, Line 112: if ((xhci_bar + 0x4C0) & 1)
I dont get this.. CB:2759 […]
c.f. CB:41852
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42152
to look at the new patch set (#5).
Change subject: sb/intel/bd82x6x: Use PCI bitwise ops ......................................................................
sb/intel/bd82x6x: Use PCI bitwise ops
Some cases could not be factored out while keeping reproducibility. Also mark some potential bugs with a FIXME comment, since fixing them while also keeping the binary unchanged is pretty much impossible.
Tested with BUILD_TIMELESS=1, Asus P8Z77-V LX2 does not change.
Change-Id: Iafe62d952a146bf53a28a1a83b87a3ae31f46720 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/bd82x6x/azalia.c M src/southbridge/intel/bd82x6x/bootblock.c M src/southbridge/intel/bd82x6x/early_me.c M src/southbridge/intel/bd82x6x/early_thermal.c M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/bd82x6x/me.c M src/southbridge/intel/bd82x6x/me_8.x.c M src/southbridge/intel/bd82x6x/pch.c M src/southbridge/intel/bd82x6x/pci.c M src/southbridge/intel/bd82x6x/pcie.c M src/southbridge/intel/bd82x6x/sata.c M src/southbridge/intel/bd82x6x/smbus.c M src/southbridge/intel/bd82x6x/smihandler.c M src/southbridge/intel/bd82x6x/usb_xhci.c 14 files changed, 65 insertions(+), 165 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/42152/5
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42152 )
Change subject: sb/intel/bd82x6x: Use PCI bitwise ops ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42152/4/src/southbridge/intel/bd82x... File src/southbridge/intel/bd82x6x/pcie.c:
https://review.coreboot.org/c/coreboot/+/42152/4/src/southbridge/intel/bd82x... PS4, Line 251: /* FIXME: That ASSERT() macro contains the line number... Add padding so that it's the same */
I forgot... […]
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42152 )
Change subject: sb/intel/bd82x6x: Use PCI bitwise ops ......................................................................
Patch Set 5: Code-Review+2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42152 )
Change subject: sb/intel/bd82x6x: Use PCI bitwise ops ......................................................................
Patch Set 5:
I'd just submit this one and drop 42195
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42152 )
Change subject: sb/intel/bd82x6x: Use PCI bitwise ops ......................................................................
sb/intel/bd82x6x: Use PCI bitwise ops
Some cases could not be factored out while keeping reproducibility. Also mark some potential bugs with a FIXME comment, since fixing them while also keeping the binary unchanged is pretty much impossible.
Tested with BUILD_TIMELESS=1, Asus P8Z77-V LX2 does not change.
Change-Id: Iafe62d952a146bf53a28a1a83b87a3ae31f46720 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42152 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M src/southbridge/intel/bd82x6x/azalia.c M src/southbridge/intel/bd82x6x/bootblock.c M src/southbridge/intel/bd82x6x/early_me.c M src/southbridge/intel/bd82x6x/early_thermal.c M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/bd82x6x/me.c M src/southbridge/intel/bd82x6x/me_8.x.c M src/southbridge/intel/bd82x6x/pch.c M src/southbridge/intel/bd82x6x/pci.c M src/southbridge/intel/bd82x6x/pcie.c M src/southbridge/intel/bd82x6x/sata.c M src/southbridge/intel/bd82x6x/smbus.c M src/southbridge/intel/bd82x6x/smihandler.c M src/southbridge/intel/bd82x6x/usb_xhci.c 14 files changed, 65 insertions(+), 165 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved
diff --git a/src/southbridge/intel/bd82x6x/azalia.c b/src/southbridge/intel/bd82x6x/azalia.c index 470c67c..4f5d8ca 100644 --- a/src/southbridge/intel/bd82x6x/azalia.c +++ b/src/southbridge/intel/bd82x6x/azalia.c @@ -215,8 +215,6 @@ u8 *base; struct resource *res; u32 codec_mask; - u8 reg8; - u16 reg16; u32 reg32;
/* Find base address */ @@ -236,48 +234,30 @@ reg32 |= RCBA32(CIR31) & 0xfe; pci_write_config32(dev, 0x120, reg32);
- reg16 = pci_read_config16(dev, 0x78); - reg16 |= (1 << 11); - pci_write_config16(dev, 0x78, reg16); + pci_or_config16(dev, 0x78, 1 << 11); } else printk(BIOS_DEBUG, "Azalia: V1CTL disabled.\n");
- reg32 = pci_read_config32(dev, 0x114); - reg32 &= ~0xfe; - pci_write_config32(dev, 0x114, reg32); + pci_and_config32(dev, 0x114, ~0xfe);
// Set VCi enable bit - reg32 = pci_read_config32(dev, 0x120); - reg32 |= (1 << 31); - pci_write_config32(dev, 0x120, reg32); + pci_or_config32(dev, 0x120, 1 << 31);
// Enable HDMI codec: - reg32 = pci_read_config32(dev, 0xc4); - reg32 |= (1 << 1); - pci_write_config32(dev, 0xc4, reg32); + pci_or_config32(dev, 0xc4, 1 << 1);
- reg8 = pci_read_config8(dev, 0x43); - reg8 |= (1 << 6); - pci_write_config8(dev, 0x43, reg8); + pci_or_config8(dev, 0x43, 1 << 6);
/* Additional programming steps */ - reg32 = pci_read_config32(dev, 0xc4); - reg32 |= (1 << 13); - pci_write_config32(dev, 0xc4, reg32); + pci_or_config32(dev, 0xc4, 1 << 13);
- reg32 = pci_read_config32(dev, 0xc4); - reg32 |= (1 << 10); - pci_write_config32(dev, 0xc4, reg32); + pci_or_config32(dev, 0xc4, 1 << 10);
- reg32 = pci_read_config32(dev, 0xd0); - reg32 &= ~(1 << 31); - pci_write_config32(dev, 0xd0, reg32); + pci_and_config32(dev, 0xd0, ~(1 << 31));
if (dev->device == 0x1e20) { /* Additional step on Panther Point */ - reg32 = pci_read_config32(dev, 0xc4); - reg32 |= (1 << 17); - pci_write_config32(dev, 0xc4, reg32); + pci_or_config32(dev, 0xc4, 1 << 17); }
/* Set Bus Master */ @@ -294,14 +274,11 @@ /* Wait 1ms */ udelay(1000);
- // - reg8 = pci_read_config8(dev, 0x40); // Audio Control - reg8 |= 1; // Select Azalia mode. This needs to be controlled via devicetree.cb - pci_write_config8(dev, 0x40, reg8); + // Select Azalia mode. This needs to be controlled via devicetree.cb + pci_or_config8(dev, 0x40, 1); // Audio Control
- reg8 = pci_read_config8(dev, 0x4d); // Docking Status - reg8 &= ~(1 << 7); // Docking not supported - pci_write_config8(dev, 0x4d, reg8); + // Docking not supported + pci_and_config8(dev, 0x4d, (u8)~(1 << 7)); // Docking Status
codec_mask = codec_detect(base);
@@ -311,10 +288,7 @@ }
/* Enable dynamic clock gating */ - reg8 = pci_read_config8(dev, 0x43); - reg8 &= ~0x7; - reg8 |= (1 << 2) | (1 << 0); - pci_write_config8(dev, 0x43, reg8); + pci_update_config8(dev, 0x43, ~0x07, (1 << 2) | (1 << 0)); }
static const char *azalia_acpi_name(const struct device *dev) diff --git a/src/southbridge/intel/bd82x6x/bootblock.c b/src/southbridge/intel/bd82x6x/bootblock.c index ab5cbf0..3a99f51 100644 --- a/src/southbridge/intel/bd82x6x/bootblock.c +++ b/src/southbridge/intel/bd82x6x/bootblock.c @@ -9,13 +9,7 @@ */ static void enable_spi_prefetch(void) { - u8 reg8; - pci_devfn_t dev = PCH_LPC_DEV; - - reg8 = pci_read_config8(dev, BIOS_CNTL); - reg8 &= ~(3 << 2); - reg8 |= (2 << 2); /* Prefetching and Caching Enabled */ - pci_write_config8(dev, BIOS_CNTL, reg8); + pci_update_config8(PCH_LPC_DEV, BIOS_CNTL, ~(3 << 2), 2 << 2); }
static void enable_port80_on_lpc(void) diff --git a/src/southbridge/intel/bd82x6x/early_me.c b/src/southbridge/intel/bd82x6x/early_me.c index 83639a4..1d132ee 100644 --- a/src/southbridge/intel/bd82x6x/early_me.c +++ b/src/southbridge/intel/bd82x6x/early_me.c @@ -110,7 +110,6 @@ int intel_early_me_init_done(u8 status) { u8 reset, errorcode, opmode; - u16 reg16; u32 mebase_l, mebase_h; u32 millisec; u32 hfs, me_fws2; @@ -163,8 +162,7 @@ } else if ((me_fws2 & 0x100) == 0x100) { if ((me_fws2 & 0x80) == 0x80) { printk(BIOS_NOTICE, "CPU was replaced & warm reset required...\n"); - reg16 = pci_read_config16(PCI_DEV(0, 31, 0), 0xa2) & ~0x80; - pci_write_config16(PCI_DEV(0, 31, 0), 0xa2, reg16); + pci_and_config16(PCI_DEV(0, 31, 0), 0xa2, ~0x80); set_global_reset(0); system_reset(); } diff --git a/src/southbridge/intel/bd82x6x/early_thermal.c b/src/southbridge/intel/bd82x6x/early_thermal.c index 0abd85cb..b3510ac 100644 --- a/src/southbridge/intel/bd82x6x/early_thermal.c +++ b/src/southbridge/intel/bd82x6x/early_thermal.c @@ -38,7 +38,7 @@ pci_write_config32(dev, 0x44, 0x0);
/* Activate temporary BAR. */ - pci_write_config32(dev, 0x40, pci_read_config32(dev, 0x40) | 5); + pci_or_config32(dev, 0x40, 5);
write16p(TBARB_TEMP + 0x04, 0x3a2b);
@@ -61,7 +61,8 @@ write16p(TBARB_TEMP + 0x1a, (read16p(TBARB_TEMP + 0x1a) & ~0xf) | 0x10f0);
/* Disable temporary BAR */ - pci_write_config32(dev, 0x40, pci_read_config32(dev, 0x40) & ~1); + pci_and_config32(dev, 0x40, ~1); + pci_write_config32(dev, 0x40, 0);
write32(DEFAULT_RCBA + 0x38b0, (read32(DEFAULT_RCBA + 0x38b0) & 0xffff8003) | 0x403c); diff --git a/src/southbridge/intel/bd82x6x/lpc.c b/src/southbridge/intel/bd82x6x/lpc.c index bdaca57..c0f62bd 100644 --- a/src/southbridge/intel/bd82x6x/lpc.c +++ b/src/southbridge/intel/bd82x6x/lpc.c @@ -417,22 +417,15 @@
static void pch_disable_smm_only_flashing(struct device *dev) { - u8 reg8; - printk(BIOS_SPEW, "Enabling BIOS updates outside of SMM... "); - reg8 = pci_read_config8(dev, BIOS_CNTL); - reg8 &= ~(1 << 5); - pci_write_config8(dev, BIOS_CNTL, reg8); + + pci_and_config8(dev, BIOS_CNTL, ~(1 << 5)); }
static void pch_fixups(struct device *dev) { - u8 gen_pmcon_2; - /* Indicate DRAM init done for MRC S3 to know it can resume */ - gen_pmcon_2 = pci_read_config8(dev, GEN_PMCON_2); - gen_pmcon_2 |= (1 << 7); - pci_write_config8(dev, GEN_PMCON_2, gen_pmcon_2); + pci_or_config8(dev, GEN_PMCON_2, 1 << 7);
/* * Enable DMI ASPM in the PCH diff --git a/src/southbridge/intel/bd82x6x/me.c b/src/southbridge/intel/bd82x6x/me.c index ebb9db9..40b0cc2b 100644 --- a/src/southbridge/intel/bd82x6x/me.c +++ b/src/southbridge/intel/bd82x6x/me.c @@ -438,7 +438,6 @@ { struct me_hfs hfs; u32 reg32; - u16 reg16;
mei_base_address = (u32 *) (pci_read_config32(PCH_ME_DEV, PCI_BASE_ADDRESS_0) & ~0xf); @@ -461,10 +460,8 @@ mkhi_end_of_post();
/* Make sure IO is disabled */ - reg16 = pci_read_config16(PCH_ME_DEV, PCI_COMMAND); - reg16 &= ~(PCI_COMMAND_MASTER | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config16(PCH_ME_DEV, PCI_COMMAND, reg16); + pci_and_config16(PCH_ME_DEV, PCI_COMMAND, + ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO));
/* Hide the PCI device */ RCBA32_OR(FD2, PCH_DISABLE_MEI1); diff --git a/src/southbridge/intel/bd82x6x/me_8.x.c b/src/southbridge/intel/bd82x6x/me_8.x.c index dd972d2..e65b8e5 100644 --- a/src/southbridge/intel/bd82x6x/me_8.x.c +++ b/src/southbridge/intel/bd82x6x/me_8.x.c @@ -432,7 +432,6 @@ { struct me_hfs hfs; u32 reg32; - u16 reg16;
mei_base_address = (void *) (pci_read_config32(PCH_ME_DEV, PCI_BASE_ADDRESS_0) & ~0xf); @@ -455,10 +454,8 @@ mkhi_end_of_post();
/* Make sure IO is disabled */ - reg16 = pci_read_config16(PCH_ME_DEV, PCI_COMMAND); - reg16 &= ~(PCI_COMMAND_MASTER | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config16(PCH_ME_DEV, PCI_COMMAND, reg16); + pci_and_config16(PCH_ME_DEV, PCI_COMMAND, + ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO));
/* Hide the PCI device */ RCBA32_OR(FD2, PCH_DISABLE_MEI1); diff --git a/src/southbridge/intel/bd82x6x/pch.c b/src/southbridge/intel/bd82x6x/pch.c index 00d5e12..7b0662b 100644 --- a/src/southbridge/intel/bd82x6x/pch.c +++ b/src/southbridge/intel/bd82x6x/pch.c @@ -301,7 +301,6 @@ static void pch_pcie_enable(struct device *dev) { struct southbridge_intel_bd82x6x_config *config = dev->chip_info; - u16 reg16;
if (!config) return; @@ -345,9 +344,7 @@ /* Handle workaround for PPT and CPT/B1+ */ if (pch_silicon_supported(PCH_TYPE_CPT, PCH_STEP_B1) && !pch_pcie_check_set_enabled(dev)) { - u8 reg8 = pci_read_config8(dev, 0xe2); - reg8 |= 1; - pci_write_config8(dev, 0xe2, reg8); + pci_or_config8(dev, 0xe2, 1); }
/* @@ -358,10 +355,8 @@ }
/* Ensure memory, io, and bus master are all disabled */ - reg16 = pci_read_config16(dev, PCI_COMMAND); - reg16 &= ~(PCI_COMMAND_MASTER | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config16(dev, PCI_COMMAND, reg16); + pci_and_config16(dev, PCI_COMMAND, + ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO));
/* Do not claim downstream transactions for PCIe ports */ new_rpfn |= RPFN_HIDE(PCI_FUNC(dev->path.pci.devfn)); @@ -408,8 +403,6 @@
void pch_enable(struct device *dev) { - u16 reg16; - /* PCH PCIe Root Ports get special handling */ if (PCI_SLOT(dev->path.pci.devfn) == PCH_PCIE_DEV_SLOT) return pch_pcie_enable(dev); @@ -418,10 +411,8 @@ printk(BIOS_DEBUG, "%s: Disabling device\n", dev_path(dev));
/* Ensure memory, io, and bus master are all disabled */ - reg16 = pci_read_config16(dev, PCI_COMMAND); - reg16 &= ~(PCI_COMMAND_MASTER | - PCI_COMMAND_MEMORY | PCI_COMMAND_IO); - pci_write_config16(dev, PCI_COMMAND, reg16); + pci_and_config16(dev, PCI_COMMAND, + ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY | PCI_COMMAND_IO));
/* Hide this device if possible */ pch_hide_devfn(dev->path.pci.devfn); diff --git a/src/southbridge/intel/bd82x6x/pci.c b/src/southbridge/intel/bd82x6x/pci.c index e61c60c..895135b 100644 --- a/src/southbridge/intel/bd82x6x/pci.c +++ b/src/southbridge/intel/bd82x6x/pci.c @@ -11,30 +11,22 @@ static void pci_init(struct device *dev) { u16 reg16; - u8 reg8;
printk(BIOS_DEBUG, "PCI init.\n"); /* Enable Bus Master */ - reg16 = pci_read_config16(dev, PCI_COMMAND); - reg16 |= PCI_COMMAND_MASTER; - pci_write_config16(dev, PCI_COMMAND, reg16); + pci_or_config16(dev, PCI_COMMAND, PCI_COMMAND_MASTER);
/* This device has no interrupt */ pci_write_config8(dev, INTR, 0xff);
/* disable parity error response and SERR */ - reg16 = pci_read_config16(dev, PCI_BRIDGE_CONTROL); - reg16 &= ~PCI_BRIDGE_CTL_PARITY; - reg16 &= ~PCI_BRIDGE_CTL_SERR; - pci_write_config16(dev, PCI_BRIDGE_CONTROL, reg16); + pci_and_config16(dev, PCI_BRIDGE_CONTROL, + ~(PCI_BRIDGE_CTL_PARITY | PCI_BRIDGE_CTL_SERR));
/* Master Latency Count must be set to 0x04! */ - reg8 = pci_read_config8(dev, SMLT); - reg8 &= 0x07; - reg8 |= (0x04 << 3); - pci_write_config8(dev, SMLT, reg8); + pci_update_config8(dev, SMLT, 0x07, (0x04 << 3));
- /* Clear errors in status registers */ + /* Clear errors in status registers. FIXME: do we need to do something? */ reg16 = pci_read_config16(dev, PSTS); //reg16 |= 0xf900; pci_write_config16(dev, PSTS, reg16); diff --git a/src/southbridge/intel/bd82x6x/pcie.c b/src/southbridge/intel/bd82x6x/pcie.c index ff881ac..c381d33 100644 --- a/src/southbridge/intel/bd82x6x/pcie.c +++ b/src/southbridge/intel/bd82x6x/pcie.c @@ -109,9 +109,7 @@ pci_write_config8(dev, 0xe1, reg8);
/* Set 0xE8[0] = 1 */ - reg32 = pci_read_config32(dev, 0xe8); - reg32 |= 1; - pci_write_config32(dev, 0xe8, reg32); + pci_or_config32(dev, 0xe8, 1);
/* Adjust Common Clock exit latency */ reg32 = pci_read_config32(dev, 0xd8); @@ -144,38 +142,24 @@ { struct southbridge_intel_bd82x6x_config *config = dev->chip_info; enum aspm_type apmc = 0; - u32 reg32;
/* Set 0x314 = 0x743a361b */ pci_write_config32(dev, 0x314, 0x743a361b);
/* Set 0x318[31:16] = 0x1414 */ - reg32 = pci_read_config32(dev, 0x318); - reg32 &= 0x0000ffff; - reg32 |= 0x14140000; - pci_write_config32(dev, 0x318, reg32); + pci_update_config32(dev, 0x318, 0x0000ffff, 0x14140000);
/* Set 0x324[5] = 1 */ - reg32 = pci_read_config32(dev, 0x324); - reg32 |= (1 << 5); - pci_write_config32(dev, 0x324, reg32); + pci_or_config32(dev, 0x324, 1 << 5);
/* Set 0x330[7:0] = 0x40 */ - reg32 = pci_read_config32(dev, 0x330); - reg32 &= ~(0xff); - reg32 |= 0x40; - pci_write_config32(dev, 0x330, reg32); + pci_update_config32(dev, 0x330, ~0xff, 0x40);
/* Set 0x33C[24:0] = 0x854c74 */ - reg32 = pci_read_config32(dev, 0x33c); - reg32 &= 0xff000000; - reg32 |= 0x00854c74; - pci_write_config32(dev, 0x33c, reg32); + pci_update_config32(dev, 0x33c, 0xff000000, 0x00854c74);
/* No IO-APIC, Disable EOI forwarding */ - reg32 = pci_read_config32(dev, 0xd4); - reg32 |= (1 << 1); - pci_write_config32(dev, 0xd4, reg32); + pci_or_config32(dev, 0xd4, 1 << 1);
/* Check for a rootport ASPM override */ switch (PCI_FUNC(dev->path.pci.devfn)) { @@ -207,19 +191,15 @@
/* Setup the override or get the real ASPM setting */ if (apmc) { - reg32 = pci_read_config32(dev, 0xd4); - reg32 |= (apmc << 2) | (1 << 4); - pci_write_config32(dev, 0xd4, reg32); + pci_or_config32(dev, 0xd4, (apmc << 2) | (1 << 4)); + } else { apmc = pci_read_config32(dev, 0x50) & 3; }
/* If both L0s and L1 enabled then set root port 0xE8[1]=1 */ - if (apmc == PCIE_ASPM_BOTH) { - reg32 = pci_read_config32(dev, 0xe8); - reg32 |= (1 << 1); - pci_write_config32(dev, 0xe8, reg32); - } + if (apmc == PCIE_ASPM_BOTH) + pci_or_config32(dev, 0xe8, 1 << 1); }
static void pci_init(struct device *dev) @@ -236,10 +216,8 @@ // This has no effect but the OS might expect it pci_write_config8(dev, 0x0c, 0x10);
- 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); + pci_update_config16(dev, PCI_BRIDGE_CONTROL, + ~PCI_BRIDGE_CTL_PARITY, PCI_BRIDGE_CTL_NO_ISA);
#ifdef EVEN_MORE_DEBUG u32 reg32; @@ -253,7 +231,7 @@ printk(BIOS_SPEW, " PMLU32 = 0x%08x\n", reg32); #endif
- /* Clear errors in status registers */ + /* Clear errors in status registers. FIXME: Do something? */ reg16 = pci_read_config16(dev, 0x06); //reg16 |= 0xf900; pci_write_config16(dev, 0x06, reg16); @@ -264,9 +242,7 @@
/* Enable expresscard hotplug events. */ if (config->pcie_hotplug_map[PCI_FUNC(dev->path.pci.devfn)]) { - pci_write_config32(dev, 0xd8, - pci_read_config32(dev, 0xd8) - | (1 << 30)); + pci_or_config32(dev, 0xd8, 1 << 30); pci_write_config16(dev, 0x42, 0x142); } } diff --git a/src/southbridge/intel/bd82x6x/sata.c b/src/southbridge/intel/bd82x6x/sata.c index 1974e2b..63801a2 100644 --- a/src/southbridge/intel/bd82x6x/sata.c +++ b/src/southbridge/intel/bd82x6x/sata.c @@ -138,8 +138,7 @@ pci_write_config16(dev, 0x92, reg16);
/* SATA Initialization register */ - pci_write_config32(dev, 0x94, - ((config->sata_port_map ^ 0x3f) << 24) | 0x183); + pci_write_config32(dev, 0x94, ((config->sata_port_map ^ 0x3f) << 24) | 0x183);
/* Initialize AHCI memory-mapped space */ abar = (u8 *)pci_read_config32(dev, PCI_BASE_ADDRESS_5); @@ -172,9 +171,7 @@ /* IDE */
/* Without AHCI BAR no memory decoding */ - reg16 = pci_read_config16(dev, PCI_COMMAND); - reg16 &= ~PCI_COMMAND_MEMORY; - pci_write_config16(dev, PCI_COMMAND, reg16); + pci_and_config16(dev, PCI_COMMAND, ~PCI_COMMAND_MEMORY);
if (sata_mode == 1) { /* Native mode on both primary and secondary. */ @@ -182,7 +179,7 @@ printk(BIOS_DEBUG, "SATA: Controller in IDE compat mode.\n"); } else { /* Legacy mode on both primary and secondary. */ - pci_update_config8(dev, 0x09, ~0x05, 0x00); + pci_and_config8(dev, 0x09, ~0x05); printk(BIOS_DEBUG, "SATA: Controller in IDE legacy mode.\n"); }
@@ -191,14 +188,10 @@ pci_write_config16(dev, IDE_TIM_SEC, IDE_DECODE_ENABLE);
/* Port enable + OOB retry mode */ - reg16 = pci_read_config16(dev, 0x92); - reg16 &= ~0x3f; - reg16 |= config->sata_port_map | 0x8000; - pci_write_config16(dev, 0x92, reg16); + pci_update_config16(dev, 0x92, ~0x3f, config->sata_port_map | 0x8000);
/* SATA Initialization register */ - pci_write_config32(dev, 0x94, - ((config->sata_port_map ^ 0x3f) << 24) | 0x183); + pci_write_config32(dev, 0x94, ((config->sata_port_map ^ 0x3f) << 24) | 0x183); }
/* Set Gen3 Transmitter settings if needed */ diff --git a/src/southbridge/intel/bd82x6x/smbus.c b/src/southbridge/intel/bd82x6x/smbus.c index 1b72ea5..dcd2724 100644 --- a/src/southbridge/intel/bd82x6x/smbus.c +++ b/src/southbridge/intel/bd82x6x/smbus.c @@ -15,6 +15,7 @@ u16 reg16;
/* Enable clock gating */ + /* FIXME: Using 32-bit ops with a 16-bit variable is a bug! These should be 16-bit! */ reg16 = pci_read_config32(dev, 0x80); reg16 &= ~((1 << 8)|(1 << 10)|(1 << 12)|(1 << 14)); pci_write_config32(dev, 0x80, reg16); diff --git a/src/southbridge/intel/bd82x6x/smihandler.c b/src/southbridge/intel/bd82x6x/smihandler.c index 57b5959..83799bee 100644 --- a/src/southbridge/intel/bd82x6x/smihandler.c +++ b/src/southbridge/intel/bd82x6x/smihandler.c @@ -99,15 +99,15 @@ switch (slp_typ) { case ACPI_S3: case ACPI_S4: + /* FIXME: Unbalanced width in read/write ops (16-bit read then 32-bit write) */ reg16 = pci_read_config16(PCH_XHCI_DEV, 0x74); reg16 &= ~0x03UL; pci_write_config32(PCH_XHCI_DEV, 0x74, reg16);
- pci_or_config16(PCH_XHCI_DEV, PCI_COMMAND, PCI_COMMAND_MASTER | - PCI_COMMAND_MEMORY); + pci_or_config16(PCH_XHCI_DEV, PCI_COMMAND, + PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY);
- xhci_bar = pci_read_config32(PCH_XHCI_DEV, - PCI_BASE_ADDRESS_0) & ~0xFUL; + xhci_bar = pci_read_config32(PCH_XHCI_DEV, PCI_BASE_ADDRESS_0) & ~0xFUL;
if ((xhci_bar + 0x4C0) & 1) pch_iobp_update(0xEC000082, ~0UL, (3 << 2)); @@ -118,19 +118,14 @@ if ((xhci_bar + 0x4F0) & 1) pch_iobp_update(0xEC000382, ~0UL, (3 << 2));
- reg16 = pci_read_config16(PCH_XHCI_DEV, PCI_COMMAND); - reg16 &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY); - pci_write_config16(PCH_XHCI_DEV, PCI_COMMAND, reg16); + pci_and_config16(PCH_XHCI_DEV, PCI_COMMAND, + ~(PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY));
- reg16 = pci_read_config16(PCH_XHCI_DEV, 0x74); - reg16 |= 0x03; - pci_write_config16(PCH_XHCI_DEV, 0x74, reg16); + pci_or_config16(PCH_XHCI_DEV, 0x74, 0x03); break;
case ACPI_S5: - reg16 = pci_read_config16(PCH_XHCI_DEV, 0x74); - reg16 |= ((1 << 8) | 0x03); - pci_write_config16(PCH_XHCI_DEV, 0x74, reg16); + pci_or_config16(PCH_XHCI_DEV, 0x74, (1 << 8) | 0x03); break; } } diff --git a/src/southbridge/intel/bd82x6x/usb_xhci.c b/src/southbridge/intel/bd82x6x/usb_xhci.c index 5a5b418..8bd0641 100644 --- a/src/southbridge/intel/bd82x6x/usb_xhci.c +++ b/src/southbridge/intel/bd82x6x/usb_xhci.c @@ -20,9 +20,7 @@ pci_write_config32(dev, XOCM, config->xhci_overcurrent_mapping);
/* lock overcurrent map */ - reg32 = pci_read_config32(dev, 0x44); - reg32 |= 1; - pci_write_config32(dev, 0x44, reg32); + pci_or_config32(dev, 0x44, 1);
pci_write_config32(dev, XUSB2PRM, config->xhci_switchable_ports); pci_write_config32(dev, USB3PRM, config->superspeed_capable_ports);
-
Angel Pons (Code Review) -
Felix Held (Code Review)
-
Kyösti Mälkki (Code Review)
-
Patrick Georgi (Code Review)