nb/intel/haswell: Tidy up code and comments

- Reformat some lines of code
- Put names to all used MCHBAR registers
- Move MCHBAR registers into a separate file, for future expansion
- Rewrite several comments
- Use C-style comments for consistency
- Rewrite some hex constants
- Use HOST_BRIDGE instead of PCI_DEV(0, 0, 0)

Tested, it does not change the binary of Asrock B85M Pro4.

Change-Id: I926289304acb834f9b13cd7902801798f8ee478a
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38434
Reviewed-by: David Hendricks <david.hendricks@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

13 files changed, 430 insertions, 408 deletions

diff --git a/src/northbridge/intel/haswell/acpi.c b/src/northbridge/intel/haswell/acpi.c
index a9b687b883..02bc1bf2a7 100644
--- a/src/northbridge/intel/haswell/acpi.c
+++ b/src/northbridge/intel/haswell/acpi.c
@@ -37,35 +37,35 @@ unsigned long acpi_fill_mcfg(unsigned long current)
 
 	pciexbar_reg = pci_read_config32(dev, PCIEXBAR);
 
-	// MMCFG not supported or not enabled.
+	/* MMCFG not supported or not enabled. */
 	if (!(pciexbar_reg & (1 << 0)))
 		return current;
 
 	mask = (1UL << 31) | (1 << 30) | (1 << 29) | (1 << 28);
 	switch ((pciexbar_reg >> 1) & 3) {
-	case 0: // 256MB
+	case 0: /* 256MB */
 		pciexbar = pciexbar_reg & mask;
 		max_buses = 256;
 		break;
-	case 1: // 128M
+	case 1: /* 128M */
 		mask |= (1 << 27);
 		pciexbar = pciexbar_reg & mask;
 		max_buses = 128;
 		break;
-	case 2: // 64M
+	case 2: /* 64M */
 		mask |= (1 << 27) | (1 << 26);
 		pciexbar = pciexbar_reg & mask;
 		max_buses = 64;
 		break;
-	default: // RSVD
+	default: /* RSVD */
 		return current;
 	}
 
 	if (!pciexbar)
 		return current;
 
-	current += acpi_create_mcfg_mmconfig((acpi_mcfg_mmconfig_t *) current,
-			pciexbar, 0x0, 0x0, max_buses - 1);
+	current += acpi_create_mcfg_mmconfig((acpi_mcfg_mmconfig_t *) current, pciexbar, 0, 0,
+					     max_buses - 1);
 
 	return current;
 }
@@ -79,8 +79,8 @@ static unsigned long acpi_fill_dmar(unsigned long current)
 	const bool vtvc0en = MCHBAR32(VTVC0BAR) & 0x1;
 
 	/* iGFX has to be enabled; GFXVTBAR set, enabled, in 32-bit space */
-	if (igfx_dev && igfx_dev->enabled && gfxvtbar
-			&& gfxvten && !MCHBAR32(GFXVTBAR + 4)) {
+	if (igfx_dev && igfx_dev->enabled && gfxvtbar && gfxvten && !MCHBAR32(GFXVTBAR + 4)) {
+
 		const unsigned long tmp = current;
 
 		current += acpi_create_dmar_drhd(current, 0, 0, gfxvtbar);
@@ -91,24 +91,23 @@ static unsigned long acpi_fill_dmar(unsigned long current)
 
 	/* VTVC0BAR has to be set, enabled, and in 32-bit space */
 	if (vtvc0bar && vtvc0en && !MCHBAR32(VTVC0BAR + 4)) {
+
 		const unsigned long tmp = current;
-		current += acpi_create_dmar_drhd(current,
-				DRHD_INCLUDE_PCI_ALL, 0, vtvc0bar);
-		current += acpi_create_dmar_ds_ioapic(current,
-				2, PCH_IOAPIC_PCI_BUS, PCH_IOAPIC_PCI_SLOT, 0);
+		current += acpi_create_dmar_drhd(current, DRHD_INCLUDE_PCI_ALL, 0, vtvc0bar);
+		current += acpi_create_dmar_ds_ioapic(current, 2, PCH_IOAPIC_PCI_BUS,
+						      PCH_IOAPIC_PCI_SLOT, 0);
+
 		size_t i;
 		for (i = 0; i < 8; ++i)
-			current += acpi_create_dmar_ds_msi_hpet(current,
-					0, PCH_HPET_PCI_BUS,
-					PCH_HPET_PCI_SLOT, i);
+			current += acpi_create_dmar_ds_msi_hpet(current, 0, PCH_HPET_PCI_BUS,
+								PCH_HPET_PCI_SLOT, i);
 		acpi_dmar_drhd_fixup(tmp, current);
 	}
 
 	return current;
 }
 
-unsigned long northbridge_write_acpi_tables(struct device *const dev,
-					    unsigned long current,
+unsigned long northbridge_write_acpi_tables(struct device *const dev, unsigned long current,
 					    struct acpi_rsdp *const rsdp)
 {
 	/* Create DMAR table only if we have VT-d capability. */
diff --git a/src/northbridge/intel/haswell/bootblock.c b/src/northbridge/intel/haswell/bootblock.c
index 04fec6fe65..903c770d9d 100644
--- a/src/northbridge/intel/haswell/bootblock.c
+++ b/src/northbridge/intel/haswell/bootblock.c
@@ -20,19 +20,17 @@ void bootblock_early_northbridge_init(void)
 	uint32_t reg;
 
 	/*
-	 * The "io" variant of the config access is explicitly used to
-	 * setup the PCIEXBAR because CONFIG_MMCONF_SUPPORT is set to
-	 * to true. That way all subsequent non-explicit config accesses use
-	 * MCFG. This code also assumes that bootblock_northbridge_init() is
-	 * the first thing called in the non-asm boot block code. The final
-	 * assumption is that no assembly code is using the
-	 * CONFIG_MMCONF_SUPPORT option to do PCI config acceses.
+	 * The "io" variant of the config access is explicitly used to setup the PCIEXBAR
+	 * because CONFIG_MMCONF_SUPPORT is set to true. That way, all subsequent
+	 * non-explicit config accesses use MCFG. This code also assumes that
+	 * bootblock_northbridge_init() is the first thing called in the non-asm
+	 * boot block code. The final assumption is that no assembly code is using
+	 * the CONFIG_MMCONF_SUPPORT option to do PCI config acceses.
 	 *
-	 * The PCIEXBAR is assumed to live in the memory mapped IO space under
-	 * 4GiB.
+	 * The PCIEXBAR is assumed to live in the memory mapped IO space under 4GiB.
 	 */
 	reg = 0;
-	pci_io_write_config32(PCI_DEV(0, 0, 0), PCIEXBAR + 4, reg);
+	pci_io_write_config32(HOST_BRIDGE, PCIEXBAR + 4, reg);
 	reg = CONFIG_MMCONF_BASE_ADDRESS | 4 | 1; /* 64MiB - 0-63 buses. */
-	pci_io_write_config32(PCI_DEV(0, 0, 0), PCIEXBAR, reg);
+	pci_io_write_config32(HOST_BRIDGE, PCIEXBAR, reg);
 }
diff --git a/src/northbridge/intel/haswell/chip.h b/src/northbridge/intel/haswell/chip.h
index 506aaa58e8..27227916f6 100644
--- a/src/northbridge/intel/haswell/chip.h
+++ b/src/northbridge/intel/haswell/chip.h
@@ -20,9 +20,9 @@
 
 /*
  * Digital Port Hotplug Enable:
- *  0x04 = Enabled, 2ms short pulse
+ *  0x04 = Enabled, 2ms   short pulse
  *  0x05 = Enabled, 4.5ms short pulse
- *  0x06 = Enabled, 6ms short pulse
+ *  0x06 = Enabled, 6ms   short pulse
  *  0x07 = Enabled, 100ms short pulse
  */
 struct northbridge_intel_haswell_config {
diff --git a/src/northbridge/intel/haswell/early_init.c b/src/northbridge/intel/haswell/early_init.c
index 6aad4a381f..ea563636f8 100644
--- a/src/northbridge/intel/haswell/early_init.c
+++ b/src/northbridge/intel/haswell/early_init.c
@@ -28,21 +28,21 @@ static void haswell_setup_bars(void)
 {
 	printk(BIOS_DEBUG, "Setting up static northbridge registers...");
 	/* Set up all hardcoded northbridge BARs */
-	pci_write_config32(PCI_DEV(0, 0x00, 0), EPBAR, DEFAULT_EPBAR | 1);
-	pci_write_config32(PCI_DEV(0, 0x00, 0), EPBAR + 4, (0LL+DEFAULT_EPBAR) >> 32);
-	pci_write_config32(PCI_DEV(0, 0x00, 0), MCHBAR, DEFAULT_MCHBAR | 1);
-	pci_write_config32(PCI_DEV(0, 0x00, 0), MCHBAR + 4, (0LL+DEFAULT_MCHBAR) >> 32);
-	pci_write_config32(PCI_DEV(0, 0x00, 0), DMIBAR, (uintptr_t)DEFAULT_DMIBAR | 1);
-	pci_write_config32(PCI_DEV(0, 0x00, 0), DMIBAR + 4, (0LL+(uintptr_t)DEFAULT_DMIBAR) >> 32);
+	pci_write_config32(HOST_BRIDGE, EPBAR,  DEFAULT_EPBAR  | 1);
+	pci_write_config32(HOST_BRIDGE, EPBAR  + 4, (0LL + DEFAULT_EPBAR)  >> 32);
+	pci_write_config32(HOST_BRIDGE, MCHBAR, DEFAULT_MCHBAR | 1);
+	pci_write_config32(HOST_BRIDGE, MCHBAR + 4, (0LL + DEFAULT_MCHBAR) >> 32);
+	pci_write_config32(HOST_BRIDGE, DMIBAR, DEFAULT_DMIBAR | 1);
+	pci_write_config32(HOST_BRIDGE, DMIBAR + 4, (0LL + DEFAULT_DMIBAR) >> 32);
 
 	/* Set C0000-FFFFF to access RAM on both reads and writes */
-	pci_write_config8(PCI_DEV(0, 0x00, 0), PAM0, 0x30);
-	pci_write_config8(PCI_DEV(0, 0x00, 0), PAM1, 0x33);
-	pci_write_config8(PCI_DEV(0, 0x00, 0), PAM2, 0x33);
-	pci_write_config8(PCI_DEV(0, 0x00, 0), PAM3, 0x33);
-	pci_write_config8(PCI_DEV(0, 0x00, 0), PAM4, 0x33);
-	pci_write_config8(PCI_DEV(0, 0x00, 0), PAM5, 0x33);
-	pci_write_config8(PCI_DEV(0, 0x00, 0), PAM6, 0x33);
+	pci_write_config8(HOST_BRIDGE, PAM0, 0x30);
+	pci_write_config8(HOST_BRIDGE, PAM1, 0x33);
+	pci_write_config8(HOST_BRIDGE, PAM2, 0x33);
+	pci_write_config8(HOST_BRIDGE, PAM3, 0x33);
+	pci_write_config8(HOST_BRIDGE, PAM4, 0x33);
+	pci_write_config8(HOST_BRIDGE, PAM5, 0x33);
+	pci_write_config8(HOST_BRIDGE, PAM6, 0x33);
 
 	printk(BIOS_DEBUG, " done.\n");
 }
@@ -55,19 +55,17 @@ static void haswell_setup_igd(void)
 
 	printk(BIOS_DEBUG, "Initializing IGD...\n");
 
-	igd_enabled = !!(pci_read_config32(PCI_DEV(0, 0, 0), DEVEN)
-			 & DEVEN_D2EN);
+	igd_enabled = !!(pci_read_config32(HOST_BRIDGE, DEVEN) & DEVEN_D2EN);
 
-	ggc = pci_read_config16(PCI_DEV(0, 0, 0), GGC);
+	ggc = pci_read_config16(HOST_BRIDGE, GGC);
 	ggc &= ~0x3f8;
 	if (igd_enabled) {
 		ggc |= GGC_GTT_2MB | GGC_IGD_MEM_IN_32MB_UNITS(1);
 		ggc &= ~GGC_DISABLE_VGA_IO_DECODE;
 	} else {
-		ggc |= GGC_GTT_0MB | GGC_IGD_MEM_IN_32MB_UNITS(0) |
-		       GGC_DISABLE_VGA_IO_DECODE;
+		ggc |= GGC_GTT_0MB | GGC_IGD_MEM_IN_32MB_UNITS(0) | GGC_DISABLE_VGA_IO_DECODE;
 	}
-	pci_write_config16(PCI_DEV(0, 0, 0), GGC, ggc);
+	pci_write_config16(HOST_BRIDGE, GGC, ggc);
 
 	if (!igd_enabled) {
 		printk(BIOS_DEBUG, "IGD is disabled.\n");
@@ -104,19 +102,18 @@ static void start_peg2_link_training(const pci_devfn_t dev)
 	printk(BIOS_DEBUG, "Started PEG1%d link training.\n", PCI_FUNC(PCI_DEV2DEVFN(dev)));
 
 	/*
-	 * The PEG device is hidden while the MRC runs. This is because the
-	 * MRC makes configurations that are not ideal if it sees a VGA
-	 * device in a PEG slot, and it locks registers preventing changes
-	 * to these configurations.
+	 * Hide the PEG device while the MRC runs. This is because the MRC makes
+	 * configurations that are not ideal if it sees a VGA device in a PEG slot,
+	 * and it locks registers preventing changes to these configurations.
 	 */
-	pci_update_config32(PCI_DEV(0, 0, 0), DEVEN, ~mask, 0);
+	pci_update_config32(HOST_BRIDGE, DEVEN, ~mask, 0);
 	peg_hidden[PCI_FUNC(PCI_DEV2DEVFN(dev))] = true;
 	printk(BIOS_DEBUG, "Temporarily hiding PEG1%d.\n", PCI_FUNC(PCI_DEV2DEVFN(dev)));
 }
 
 void haswell_unhide_peg(void)
 {
-	u32 deven = pci_read_config32(PCI_DEV(0, 0, 0), DEVEN);
+	u32 deven = pci_read_config32(HOST_BRIDGE, DEVEN);
 
 	for (u8 fn = 0; fn <= 2; fn++) {
 		if (peg_hidden[fn]) {
@@ -126,17 +123,19 @@ void haswell_unhide_peg(void)
 		}
 	}
 
-	pci_write_config32(PCI_DEV(0, 0, 0), DEVEN, deven);
+	pci_write_config32(HOST_BRIDGE, DEVEN, deven);
 }
 
 static void haswell_setup_peg(void)
 {
-	u32 deven = pci_read_config32(PCI_DEV(0, 0, 0), DEVEN);
+	u32 deven = pci_read_config32(HOST_BRIDGE, DEVEN);
 
 	if (deven & DEVEN_D1F2EN)
 		start_peg2_link_training(PCI_DEV(0, 1, 2));
+
 	if (deven & DEVEN_D1F1EN)
 		start_peg2_link_training(PCI_DEV(0, 1, 1));
+
 	if (deven & DEVEN_D1F0EN)
 		start_peg2_link_training(PCI_DEV(0, 1, 0));
 }
@@ -146,50 +145,51 @@ static void haswell_setup_misc(void)
 	u32 reg32;
 
 	/* Erratum workarounds */
-	reg32 = MCHBAR32(0x5f00);
-	reg32 |= (1 << 9)|(1 << 10);
-	MCHBAR32(0x5f00) = reg32;
+	reg32 = MCHBAR32(SAPMCTL);
+	reg32 |= (1 << 9) | (1 << 10);
+	MCHBAR32(SAPMCTL) = reg32;
 
 	/* Enable SA Clock Gating */
-	reg32 = MCHBAR32(0x5f00);
-	MCHBAR32(0x5f00) = reg32 | 1;
+	reg32 = MCHBAR32(SAPMCTL);
+	MCHBAR32(SAPMCTL) = reg32 | 1;
 
 	/* GPU RC6 workaround for sighting 366252 */
-	reg32 = MCHBAR32(0x5d14);
+	reg32 = MCHBAR32(SSKPD + 4);
 	reg32 |= (1UL << 31);
-	MCHBAR32(0x5d14) = reg32;
+	MCHBAR32(SSKPD + 4) = reg32;
 
-	/* VLW */
+	/* VLW (Virtual Legacy Wire?) */
 	reg32 = MCHBAR32(0x6120);
 	reg32 &= ~(1 << 0);
 	MCHBAR32(0x6120) = reg32;
 
-	reg32 = MCHBAR32(0x5418);
+	reg32 = MCHBAR32(INTRDIRCTL);
 	reg32 |= (1 << 4) | (1 << 5);
-	MCHBAR32(0x5418) = reg32;
+	MCHBAR32(INTRDIRCTL) = reg32;
 }
 
 static void haswell_setup_iommu(void)
 {
-	const u32 capid0_a = pci_read_config32(PCI_DEV(0, 0, 0), CAPID0_A);
+	const u32 capid0_a = pci_read_config32(HOST_BRIDGE, CAPID0_A);
 
 	if (capid0_a & VTD_DISABLE)
 		return;
 
-	/* setup BARs: zeroize top 32 bits; set enable bit */
+	/* Setup BARs: zeroize top 32 bits; set enable bit */
 	MCHBAR32(GFXVTBAR + 4) = GFXVT_BASE_ADDRESS >> 32;
-	MCHBAR32(GFXVTBAR) = GFXVT_BASE_ADDRESS | 1;
+	MCHBAR32(GFXVTBAR)     = GFXVT_BASE_ADDRESS | 1;
 	MCHBAR32(VTVC0BAR + 4) = VTVC0_BASE_ADDRESS >> 32;
-	MCHBAR32(VTVC0BAR) = VTVC0_BASE_ADDRESS | 1;
+	MCHBAR32(VTVC0BAR)     = VTVC0_BASE_ADDRESS | 1;
 
-	/* set L3HIT2PEND_DIS, lock GFXVTBAR policy cfg registers */
+	/* Set L3HIT2PEND_DIS, lock GFXVTBAR policy config registers */
 	u32 reg32;
 	reg32 = read32((void *)(GFXVT_BASE_ADDRESS + ARCHDIS));
-	write32((void *)(GFXVT_BASE_ADDRESS + ARCHDIS),
-			reg32 | DMAR_LCKDN | L3HIT2PEND_DIS);
-	/* clear SPCAPCTRL */
+	write32((void *)(GFXVT_BASE_ADDRESS + ARCHDIS), reg32 | DMAR_LCKDN | L3HIT2PEND_DIS);
+
+	/* Clear SPCAPCTRL */
 	reg32 = read32((void *)(VTVC0_BASE_ADDRESS + ARCHDIS)) & ~SPCAPCTRL;
-	/* set GLBIOTLBINV, GLBCTXTINV; lock VTVC0BAR policy cfg registers */
+
+	/* Set GLBIOTLBINV, GLBCTXTINV; lock VTVC0BAR policy config registers */
 	write32((void *)(VTVC0_BASE_ADDRESS + ARCHDIS),
 			reg32 | DMAR_LCKDN | GLBIOTLBINV | GLBCTXTINV);
 }
diff --git a/src/northbridge/intel/haswell/finalize.c b/src/northbridge/intel/haswell/finalize.c
index ca36634f36..024d44e728 100644
--- a/src/northbridge/intel/haswell/finalize.c
+++ b/src/northbridge/intel/haswell/finalize.c
@@ -17,35 +17,33 @@
 #include <device/pci_ops.h>
 #include "haswell.h"
 
-#define PCI_DEV_HSW PCI_DEV(0, 0, 0)
-
 void intel_northbridge_haswell_finalize_smm(void)
 {
-	pci_or_config16(PCI_DEV_HSW, 0x50, 1 << 0);	/* GGC */
-	pci_or_config32(PCI_DEV_HSW, 0x5c, 1 << 0);	/* DPR */
-	pci_or_config32(PCI_DEV_HSW, 0x78, 1 << 10);	/* ME */
-	pci_or_config32(PCI_DEV_HSW, 0x90, 1 << 0);	/* REMAPBASE */
-	pci_or_config32(PCI_DEV_HSW, 0x98, 1 << 0);	/* REMAPLIMIT */
-	pci_or_config32(PCI_DEV_HSW, 0xa0, 1 << 0);	/* TOM */
-	pci_or_config32(PCI_DEV_HSW, 0xa8, 1 << 0);	/* TOUUD */
-	pci_or_config32(PCI_DEV_HSW, 0xb0, 1 << 0);	/* BDSM */
-	pci_or_config32(PCI_DEV_HSW, 0xb4, 1 << 0);	/* BGSM */
-	pci_or_config32(PCI_DEV_HSW, 0xb8, 1 << 0);	/* TSEGMB */
-	pci_or_config32(PCI_DEV_HSW, 0xbc, 1 << 0);	/* TOLUD */
+	pci_or_config16(HOST_BRIDGE, 0x50, 1 << 0);	/* GGC */
+	pci_or_config32(HOST_BRIDGE, 0x5c, 1 << 0);	/* DPR */
+	pci_or_config32(HOST_BRIDGE, 0x78, 1 << 10);	/* ME */
+	pci_or_config32(HOST_BRIDGE, 0x90, 1 << 0);	/* REMAPBASE */
+	pci_or_config32(HOST_BRIDGE, 0x98, 1 << 0);	/* REMAPLIMIT */
+	pci_or_config32(HOST_BRIDGE, 0xa0, 1 << 0);	/* TOM */
+	pci_or_config32(HOST_BRIDGE, 0xa8, 1 << 0);	/* TOUUD */
+	pci_or_config32(HOST_BRIDGE, 0xb0, 1 << 0);	/* BDSM */
+	pci_or_config32(HOST_BRIDGE, 0xb4, 1 << 0);	/* BGSM */
+	pci_or_config32(HOST_BRIDGE, 0xb8, 1 << 0);	/* TSEGMB */
+	pci_or_config32(HOST_BRIDGE, 0xbc, 1 << 0);	/* TOLUD */
 
-	MCHBAR32_OR(0x5500, 1 << 0);	/* PAVP */
-	MCHBAR32_OR(0x5f00, 1UL << 31);	/* SA PM */
-	MCHBAR32_OR(0x6020, 1 << 0);	/* UMA GFX */
-	MCHBAR32_OR(0x63fc, 1 << 0);	/* VTDTRK */
-	MCHBAR32_OR(0x6800, 1UL << 31);
-	MCHBAR32_OR(0x7000, 1UL << 31);
-	MCHBAR32_OR(0x77fc, 1 << 0);
+	MCHBAR32_OR(MMIO_PAVP_MSG, 1 << 0);	/* PAVP */
+	MCHBAR32_OR(SAPMCTL, 1UL << 31);	/* SA PM */
+	MCHBAR32_OR(UMAGFXCTL, 1 << 0);		/* UMA GFX */
+	MCHBAR32_OR(VTDTRKLCK, 1 << 0);		/* VTDTRK */
+	MCHBAR32_OR(REQLIM, 1UL << 31);
+	MCHBAR32_OR(DMIVCLIM, 1UL << 31);
+	MCHBAR32_OR(CRDTLCK, 1 << 0);
 
 	/* Memory Controller Lockdown */
-	MCHBAR8(0x50fc) = 0x8f;
+	MCHBAR8(MC_LOCK) = 0x8f;
 
 	/* Read+write the following */
-	MCHBAR32(0x6030) = MCHBAR32(0x6030);
-	MCHBAR32(0x6034) = MCHBAR32(0x6034);
-	MCHBAR32(0x6008) = MCHBAR32(0x6008);
+	MCHBAR32(VDMBDFBARKVM)  = MCHBAR32(VDMBDFBARKVM);
+	MCHBAR32(VDMBDFBARPAVP) = MCHBAR32(VDMBDFBARPAVP);
+	MCHBAR32(HDAUDRID)      = MCHBAR32(HDAUDRID);
 }
diff --git a/src/northbridge/intel/haswell/gma.c b/src/northbridge/intel/haswell/gma.c
index 3132c20136..cf56c69539 100644
--- a/src/northbridge/intel/haswell/gma.c
+++ b/src/northbridge/intel/haswell/gma.c
@@ -97,9 +97,9 @@ static const struct gt_reg haswell_gt_lock[] = {
 	{ 0 },
 };
 
-/* some vga option roms are used for several chipsets but they only have one
- * PCI ID in their header. If we encounter such an option rom, we need to do
- * the mapping ourselves
+/*
+ * Some VGA option roms are used for several chipsets but they only have one PCI ID in their
+ * header. If we encounter such an option rom, we need to do the mapping ourselves.
  */
 
 u32 map_oprom_vendev(u32 vendev)
@@ -129,39 +129,41 @@ u32 map_oprom_vendev(u32 vendev)
 	return new_vendev;
 }
 
-/* GTT is the Global Translation Table for the graphics pipeline.
- * It is used to translate graphics addresses to physical
- * memory addresses. As in the CPU, GTTs map 4K pages.
- * The setgtt function adds a further bit of flexibility:
- * it allows you to set a range (the first two parameters) to point
- * to a physical address (third parameter);the physical address is
- * incremented by a count (fourth parameter) for each GTT in the
- * range.
- * Why do it this way? For ultrafast startup,
- * we can point all the GTT entries to point to one page,
- * and set that page to 0s:
- * memset(physbase, 0, 4096);
- * setgtt(0, 4250, physbase, 0);
- * this takes about 2 ms, and is a win because zeroing
- * the page takes a up to 200 ms.
- * This call sets the GTT to point to a linear range of pages
- * starting at physbase.
+/** FIXME: Seems to be outdated. */
+/*
+ * GTT is the Global Translation Table for the graphics pipeline. It is used to translate
+ * graphics addresses to physical memory addresses. As in the CPU, GTTs map 4K pages.
+ *
+ * The setgtt function adds a further bit of flexibility: it allows you to set a range (the
+ * first two parameters) to point to a physical address (third parameter); the physical address
+ * is incremented by a count (fourth parameter) for each GTT in the range.
+ *
+ * Why do it this way? For ultrafast startup, we can point all the GTT entries to point to one
+ * page, and set that page to 0s:
+ *
+ *   memset(physbase, 0, 4096);
+ *   setgtt(0, 4250, physbase, 0);
+ *
+ * this takes about 2 ms, and is a win because zeroing the page takes up to 200 ms.
+ *
+ * This call sets the GTT to point to a linear range of pages starting at physbase.
  */
 
 #define GTT_PTE_BASE (2 << 20)
 
-void
-set_translation_table(int start, int end, u64 base, int inc)
+void set_translation_table(int start, int end, u64 base, int inc)
 {
 	int i;
 
 	for (i = start; i < end; i++){
-		u64 physical_address = base + i*inc;
+		u64 physical_address = base + i * inc;
+
 		/* swizzle the 32:39 bits to 4:11 */
 		u32 word = physical_address | ((physical_address >> 28) & 0xff0) | 1;
-		/* note: we've confirmed by checking
-		 * the values that mrc does no
-		 * useful setup before we run this.
+
+		/*
+		 * Note: we've confirmed by checking the values that MRC does no useful
+		 * setup before we run this.
 		 */
 		gtt_write(GTT_PTE_BASE + i * 4, word);
 		gtt_read(GTT_PTE_BASE + i * 4);
@@ -211,6 +213,7 @@ int gtt_poll(u32 reg, u32 mask, u32 value)
 		data = gtt_read(reg);
 		if ((data & mask) == value)
 			return 1;
+
 		udelay(10);
 	}
 
@@ -261,10 +264,13 @@ static void gma_pm_init_pre_vbios(struct device *dev)
 
 	/* Wait for Mailbox Ready */
 	gtt_poll(0x138124, (1UL << 31), (0UL << 31));
+
 	/* Mailbox Data - RC6 VIDS */
 	gtt_write(0x138128, 0x00000000);
+
 	/* Mailbox Command */
 	gtt_write(0x138124, 0x80000004);
+
 	/* Wait for Mailbox Ready */
 	gtt_poll(0x138124, (1UL << 31), (0UL << 31));
 
@@ -291,7 +297,7 @@ static void init_display_planes(void)
 		gtt_write(CURBASE_IVB(pipe), 0x00000000);
 	}
 
-	/* Disable primary plane and set surface base address*/
+	/* Disable primary plane and set surface base address */
 	for (plane = PLANE_A; plane <= PLANE_C; plane++) {
 		gtt_write(DSPCNTR(plane), DISPLAY_PLANE_DISABLE);
 		gtt_write(DSPSURF(plane), 0x00000000);
@@ -357,11 +363,12 @@ static void gma_setup_panel(struct device *dev)
 
 	init_display_planes();
 
-	/* DDI-A params set:
-	   bit 0: Display detected (RO)
-	   bit 4: DDI A supports 4 lanes and DDI E is not used
-	   bit 7: DDI buffer is idle
-	*/
+	/*
+	 * DDI-A params set:
+	 * bit 0: Display detected (RO)
+	 * bit 4: DDI A supports 4 lanes and DDI E is not used
+	 * bit 7: DDI buffer is idle
+	 */
 	reg32 = DDI_BUF_IS_IDLE | DDI_INIT_DISPLAY_DETECTED;
 	if (!conf->gpu_ddi_e_connected)
 		reg32 |= DDI_A_4_LANES;
@@ -374,14 +381,14 @@ static void gma_setup_panel(struct device *dev)
 	/* Enable the handshake with PCH display when processing reset */
 	gtt_write(NDE_RSTWRN_OPT, RST_PCH_HNDSHK_EN);
 
-	/* undocumented */
+	/* Undocumented */
 	gtt_write(0x42090, 0x04000000);
-	gtt_write(0x9840, 0x00000000);
+	gtt_write(0x9840,  0x00000000);
 	gtt_write(0x42090, 0xa4000000);
 
 	gtt_write(SOUTH_DSPCLK_GATE_D, PCH_LP_PARTITION_LEVEL_DISABLE);
 
-	/* undocumented */
+	/* Undocumented */
 	gtt_write(0x42080, 0x00004000);
 
 	/* Prepare DDI buffers for DP and FDI */
@@ -393,9 +400,10 @@ static void gma_setup_panel(struct device *dev)
 	/* Enable HPD buffer for digital port D and B */
 	gtt_write(PCH_PORT_HOTPLUG, PORTD_HOTPLUG_ENABLE | PORTB_HOTPLUG_ENABLE);
 
-	/* Bits 4:0 - Power cycle delay (default 0x6 --> 500ms)
-	   Bits 31:8 - Reference divider (0x0004af ----> 24MHz)
-	*/
+	/*
+	 * Bits 4:0 - Power cycle delay (default 0x6 --> 500ms)
+	 * Bits 31:8 - Reference divider (0x0004af ----> 24MHz)
+	 */
 	gtt_write(PCH_PP_DIVISOR, 0x0004af06);
 }
 
@@ -440,12 +448,12 @@ static void gma_enable_swsci(void)
 {
 	u16 reg16;
 
-	/* clear DMISCI status */
+	/* Clear DMISCI status */
 	reg16 = inw(get_pmbase() + TCO1_STS);
 	reg16 &= DMISCI_STS;
 	outw(get_pmbase() + TCO1_STS, reg16);
 
-	/* clear and enable ACPI TCO SCI */
+	/* Clear and enable ACPI TCO SCI */
 	enable_tco_sci();
 }
 
@@ -491,10 +499,9 @@ static void gma_func0_init(struct device *dev)
 	intel_gma_restore_opregion();
 }
 
-const struct i915_gpu_controller_info *
-intel_gma_get_controller_info(void)
+const struct i915_gpu_controller_info *intel_gma_get_controller_info(void)
 {
-	struct device *dev = pcidev_on_root(0x2, 0);
+	struct device *dev = pcidev_on_root(2, 0);
 	if (!dev) {
 		return NULL;
 	}
@@ -512,9 +519,8 @@ static void gma_ssdt(struct device *device)
 	drivers_intel_gma_displays_ssdt_generate(gfx);
 }
 
-static unsigned long
-gma_write_acpi_tables(struct device *const dev, unsigned long current,
-		      struct acpi_rsdp *const rsdp)
+static unsigned long gma_write_acpi_tables(struct device *const dev, unsigned long current,
+					   struct acpi_rsdp *const rsdp)
 {
 	igd_opregion_t *opregion = (igd_opregion_t *)current;
 	global_nvs_t *gnvs;
@@ -538,19 +544,19 @@ gma_write_acpi_tables(struct device *const dev, unsigned long current,
 }
 
 static struct pci_operations gma_pci_ops = {
-	.set_subsystem    = pci_dev_set_subsystem,
+	.set_subsystem = pci_dev_set_subsystem,
 };
 
 static struct device_operations gma_func0_ops = {
-	.read_resources		= pci_dev_read_resources,
-	.set_resources		= pci_dev_set_resources,
-	.enable_resources	= pci_dev_enable_resources,
-	.init			= gma_func0_init,
+	.read_resources           = pci_dev_read_resources,
+	.set_resources            = pci_dev_set_resources,
+	.enable_resources         = pci_dev_enable_resources,
+	.init                     = gma_func0_init,
 	.acpi_fill_ssdt_generator = gma_ssdt,
-	.scan_bus		= 0,
-	.enable			= 0,
-	.ops_pci		= &gma_pci_ops,
-	.write_acpi_tables	= gma_write_acpi_tables,
+	.scan_bus                 = NULL,
+	.enable                   = NULL,
+	.ops_pci                  = &gma_pci_ops,
+	.write_acpi_tables        = gma_write_acpi_tables,
 };
 
 static const unsigned short pci_device_ids[] = {
@@ -570,7 +576,7 @@ static const unsigned short pci_device_ids[] = {
 };
 
 static const struct pci_driver pch_lpc __pci_driver = {
-	.ops	 = &gma_func0_ops,
-	.vendor	 = PCI_VENDOR_ID_INTEL,
+	.ops     = &gma_func0_ops,
+	.vendor  = PCI_VENDOR_ID_INTEL,
 	.devices = pci_device_ids,
 };
diff --git a/src/northbridge/intel/haswell/haswell.h b/src/northbridge/intel/haswell/haswell.h
index fce94166a7..b45036ee33 100644
--- a/src/northbridge/intel/haswell/haswell.h
+++ b/src/northbridge/intel/haswell/haswell.h
@@ -26,13 +26,9 @@
 #define IED_SIZE	CONFIG_IED_REGION_SIZE
 
 /* Northbridge BARs */
-#define DEFAULT_MCHBAR		0xfed10000	/* 16 KB */
-#ifndef __ACPI__
-#define DEFAULT_DMIBAR		((u8 *)0xfed18000)	/* 4 KB */
-#else
-#define DEFAULT_DMIBAR		0xfed18000	/* 4 KB */
-#endif
-#define DEFAULT_EPBAR		0xfed19000	/* 4 KB */
+#define DEFAULT_MCHBAR		0xfed10000		/* 16 KB */
+#define DEFAULT_DMIBAR		0xfed18000		/* 4 KB */
+#define DEFAULT_EPBAR		0xfed19000		/* 4 KB */
 
 #define GFXVT_BASE_ADDRESS	0xfed90000ULL
 #define GFXVT_BASE_SIZE		0x1000
@@ -46,6 +42,7 @@
 #ifndef __ACPI__
 
 /* Device 0:0.0 PCI configuration space (Host Bridge) */
+#define HOST_BRIDGE	PCI_DEV(0, 0, 0)
 
 #define EPBAR		0x40
 #define MCHBAR		0x48
@@ -55,9 +52,9 @@
 #define GGC		0x50			/* GMCH Graphics Control */
 #define  GGC_DISABLE_VGA_IO_DECODE	(1 << 1)
 #define  GGC_IGD_MEM_IN_32MB_UNITS(x)	(((x) & 0x1f) << 3)
-#define  GGC_GTT_0MB		(0 << 8)
-#define  GGC_GTT_1MB		(1 << 8)
-#define  GGC_GTT_2MB		(2 << 8)
+#define  GGC_GTT_0MB			(0 << 8)
+#define  GGC_GTT_1MB			(1 << 8)
+#define  GGC_GTT_2MB			(2 << 8)
 
 #define DEVEN		0x54			/* Device Enable */
 #define  DEVEN_D7EN	(1 << 14)
@@ -85,11 +82,11 @@
 #define  G_SMRAME	(1 << 3)
 #define  C_BASE_SEG	((0 << 2) | (1 << 1) | (0 << 0))
 
-#define MESEG_BASE	0x70	/* Management Engine Base. */
-#define MESEG_LIMIT	0x78	/* Management Engine Limit. */
-#define REMAPBASE	0x90	/* Remap base. */
-#define REMAPLIMIT	0x98	/* Remap limit. */
-#define TOM		0xa0	/* Top of DRAM in memory controller space. */
+#define MESEG_BASE	0x70	/* Management Engine Base */
+#define MESEG_LIMIT	0x78	/* Management Engine Limit */
+#define REMAPBASE	0x90	/* Remap base */
+#define REMAPLIMIT	0x98	/* Remap limit */
+#define TOM		0xa0	/* Top of DRAM in memory controller space */
 #define TOUUD		0xa8	/* Top of Upper Usable DRAM */
 #define BDSM		0xb0	/* Base Data Stolen Memory */
 #define BGSM		0xb4	/* Base GTT Stolen Memory */
@@ -117,26 +114,27 @@
  * MCHBAR
  */
 
-#define MCHBAR8(x) *((volatile u8 *)(DEFAULT_MCHBAR + x))
-#define MCHBAR16(x) *((volatile u16 *)(DEFAULT_MCHBAR + x))
-#define MCHBAR32(x) *((volatile u32 *)(DEFAULT_MCHBAR + x))
-#define MCHBAR32_OR(x, or) MCHBAR32(x) = (MCHBAR32(x) | (or))
-
-#define BIOS_RESET_CPL	0x5da8	/* 8bit */
-#define GFXVTBAR	0x5400
-#define VTVC0BAR	0x5410
-
-/* Some power MSRs are also represented in MCHBAR */
-#define MCH_PKG_POWER_LIMIT_LO	0x59a0
-#define MCH_PKG_POWER_LIMIT_HI	0x59a4
-#define MCH_DDR_POWER_LIMIT_LO	0x58e0
-#define MCH_DDR_POWER_LIMIT_HI	0x58e4
+#define MCHBAR8(x)  (*((volatile u8  *)(DEFAULT_MCHBAR + (x))))
+#define MCHBAR16(x) (*((volatile u16 *)(DEFAULT_MCHBAR + (x))))
+#define MCHBAR32(x) (*((volatile u32 *)(DEFAULT_MCHBAR + (x))))
+#define MCHBAR8_AND(x,  and) (MCHBAR8(x)  = MCHBAR8(x)  & (and))
+#define MCHBAR16_AND(x, and) (MCHBAR16(x) = MCHBAR16(x) & (and))
+#define MCHBAR32_AND(x, and) (MCHBAR32(x) = MCHBAR32(x) & (and))
+#define MCHBAR8_OR(x,  or) (MCHBAR8(x)  = MCHBAR8(x)  | (or))
+#define MCHBAR16_OR(x, or) (MCHBAR16(x) = MCHBAR16(x) | (or))
+#define MCHBAR32_OR(x, or) (MCHBAR32(x) = MCHBAR32(x) | (or))
+#define MCHBAR8_AND_OR(x,  and, or) (MCHBAR8(x)  = (MCHBAR8(x)  & (and)) | (or))
+#define MCHBAR16_AND_OR(x, and, or) (MCHBAR16(x) = (MCHBAR16(x) & (and)) | (or))
+#define MCHBAR32_AND_OR(x, and, or) (MCHBAR32(x) = (MCHBAR32(x) & (and)) | (or))
+
+/* As there are many registers, define them on a separate file */
+#include "mchbar_regs.h"
 
 /*
  * EPBAR - Egress Port Root Complex Register Block
  */
 
-#define EPBAR8(x) *((volatile u8 *)(DEFAULT_EPBAR + x))
+#define EPBAR8(x)  *((volatile u8  *)(DEFAULT_EPBAR + x))
 #define EPBAR16(x) *((volatile u16 *)(DEFAULT_EPBAR + x))
 #define EPBAR32(x) *((volatile u32 *)(DEFAULT_EPBAR + x))
 
@@ -167,7 +165,7 @@
  * DMIBAR
  */
 
-#define DMIBAR8(x) *((volatile u8 *)(DEFAULT_DMIBAR + x))
+#define DMIBAR8(x)  *((volatile u8  *)(DEFAULT_DMIBAR + x))
 #define DMIBAR16(x) *((volatile u16 *)(DEFAULT_DMIBAR + x))
 #define DMIBAR32(x) *((volatile u32 *)(DEFAULT_DMIBAR + x))
 
@@ -215,9 +213,9 @@ void report_platform_info(void);
 #include <device/device.h>
 
 struct acpi_rsdp;
-unsigned long northbridge_write_acpi_tables(struct device *device,
-		unsigned long start, struct acpi_rsdp *rsdp);
+unsigned long northbridge_write_acpi_tables(struct device *device, unsigned long start,
+					    struct acpi_rsdp *rsdp);
 
-#endif
-#endif
+#endif /* __ASSEMBLER__ */
+#endif /* __ACPI__ */
 #endif /* __NORTHBRIDGE_INTEL_HASWELL_HASWELL_H__ */
diff --git a/src/northbridge/intel/haswell/mchbar_regs.h b/src/northbridge/intel/haswell/mchbar_regs.h
new file mode 100644
index 0000000000..dfc0f7becf
--- /dev/null
+++ b/src/northbridge/intel/haswell/mchbar_regs.h
@@ -0,0 +1,61 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2007-2008 coresystems GmbH
+ * Copyright (C) 2011 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __HASWELL_MCHBAR_REGS_H__
+#define __HASWELL_MCHBAR_REGS_H__
+
+/* Register definitions */
+#define MAD_CHNL		0x5000 /* Address Decoder Channel Configuration */
+#define MAD_DIMM_CH0		0x5004 /* Address Decode Channel 0 */
+#define MAD_DIMM_CH1		0x5008 /* Address Decode Channel 1 */
+#define MAD_DIMM_CH2		0x500c /* Address Decode Channel 2 (unused on HSW) */
+#define MC_INIT_STATE_G		0x5030
+#define MRC_REVISION		0x5034 /* MRC Revision */
+
+#define MC_LOCK			0x50fc /* Memory Controlller Lock register */
+
+#define GFXVTBAR		0x5400 /* Base address for IGD */
+#define EDRAMBAR		0x5408 /* Base address for eDRAM */
+#define VTVC0BAR		0x5410 /* Base address for PEG, USB, SATA, etc. */
+#define INTRDIRCTL		0x5418 /* Interrupt Redirection Control (PAIR) */
+#define GDXCBAR			0x5420 /* Generic Debug eXternal Connection */
+
+/* PAVP message register. Bit 0 locks PAVP settings, and bits [31..20] are an offset. */
+#define MMIO_PAVP_MSG		0x5500
+
+/* Some power MSRs are also represented in MCHBAR */
+#define MCH_PKG_POWER_LIMIT_LO	0x59a0
+#define MCH_PKG_POWER_LIMIT_HI	0x59a4
+
+#define MCH_DDR_POWER_LIMIT_LO	0x58e0
+#define MCH_DDR_POWER_LIMIT_HI	0x58e4
+
+#define SSKPD			0x5d10 /* 64-bit scratchpad register */
+#define BIOS_RESET_CPL		0x5da8 /* 8-bit */
+
+#define MC_BIOS_DATA		0x5e04 /* Miscellaneous information for BIOS */
+#define SAPMCTL			0x5f00
+
+#define HDAUDRID		0x6008
+#define UMAGFXCTL		0x6020
+#define VDMBDFBARKVM		0x6030
+#define VDMBDFBARPAVP		0x6034
+#define VTDTRKLCK		0x63fc
+#define REQLIM			0x6800
+#define DMIVCLIM		0x7000
+#define CRDTLCK			0x77fc
+
+#endif /* __HASWELL_MCHBAR_REGS_H__ */
diff --git a/src/northbridge/intel/haswell/memmap.c b/src/northbridge/intel/haswell/memmap.c
index 74d9292c14..2e8addef97 100644
--- a/src/northbridge/intel/haswell/memmap.c
+++ b/src/northbridge/intel/haswell/memmap.c
@@ -13,7 +13,7 @@
  * GNU General Public License for more details.
  */
 
-// Use simple device model for this file even in ramstage
+/* Use simple device model for this file even in ramstage */
 #define __SIMPLE_DEVICE__
 
 #include <arch/romstage.h>
@@ -30,7 +30,7 @@ static uintptr_t smm_region_start(void)
 	 * Base of TSEG is top of usable DRAM below 4GiB. The register has
 	 * 1 MiB alignment.
 	 */
-	uintptr_t tom = pci_read_config32(PCI_DEV(0,0,0), TSEG);
+	uintptr_t tom = pci_read_config32(HOST_BRIDGE, TSEG);
 	return tom & ~((1 << 20) - 1);
 }
 
@@ -53,7 +53,6 @@ void fill_postcar_frame(struct postcar_frame *pcf)
 	 * above top of the ram. This satisfies MTRR alignment requirement
 	 * with different TSEG size configurations.
 	 */
-	top_of_ram = ALIGN_DOWN((uintptr_t)cbmem_top(), 8*MiB);
-	postcar_frame_add_mtrr(pcf, top_of_ram - 8*MiB, 16*MiB,
-			MTRR_TYPE_WRBACK);
+	top_of_ram = ALIGN_DOWN((uintptr_t)cbmem_top(), 8 * MiB);
+	postcar_frame_add_mtrr(pcf, top_of_ram - 8 * MiB, 16 * MiB, MTRR_TYPE_WRBACK);
 }
diff --git a/src/northbridge/intel/haswell/minihd.c b/src/northbridge/intel/haswell/minihd.c
index 52b158e0c1..c59d02ed95 100644
--- a/src/northbridge/intel/haswell/minihd.c
+++ b/src/northbridge/intel/haswell/minihd.c
@@ -25,30 +25,30 @@
 
 static const u32 minihd_verb_table[] = {
 	/* coreboot specific header */
-	0x80862807,	// Codec Vendor / Device ID: Intel Haswell Mini-HD
-	0x80860101,	// Subsystem ID
-	0x00000004,	// Number of jacks
+	0x80862807,	/* Codec Vendor / Device ID: Intel Haswell Mini-HD */
+	0x80860101,	/* Subsystem ID */
+	4,		/* Number of jacks */
 
 	/* Enable 3rd Pin and Converter Widget */
 	0x00878101,
 
 	/* Pin Widget 5 - PORT B */
-	0x00571C10,
-	0x00571D00,
-	0x00571E56,
-	0x00571F18,
+	0x00571c10,
+	0x00571d00,
+	0x00571e56,
+	0x00571f18,
 
 	/* Pin Widget 6 - PORT C */
-	0x00671C20,
-	0x00671D00,
-	0x00671E56,
-	0x00671F18,
+	0x00671c20,
+	0x00671d00,
+	0x00671e56,
+	0x00671f18,
 
 	/* Pin Widget 7 - PORT D */
-	0x00771C30,
-	0x00771D00,
-	0x00771E56,
-	0x00771F18,
+	0x00771c30,
+	0x00771d00,
+	0x00771e56,
+	0x00771f18,
 
 	/* Disable 3rd Pin and Converter Widget */
 	0x00878100,
@@ -94,15 +94,14 @@ static void minihd_init(struct device *dev)
 	if (codec_mask) {
 		for (i = 3; i >= 0; i--) {
 			if (codec_mask & (1 << i))
-				hda_codec_init(base, i,
-					       sizeof(minihd_verb_table),
+				hda_codec_init(base, i, sizeof(minihd_verb_table),
 					       minihd_verb_table);
 		}
 	}
 }
 
 static struct pci_operations minihd_pci_ops = {
-	.set_subsystem    = pci_dev_set_subsystem,
+	.set_subsystem = pci_dev_set_subsystem,
 };
 
 static struct device_operations minihd_ops = {
@@ -110,7 +109,7 @@ static struct device_operations minihd_ops = {
 	.set_resources		= pci_dev_set_resources,
 	.enable_resources	= pci_dev_enable_resources,
 	.init			= minihd_init,
-	.scan_bus		= 0,
+	.scan_bus		= NULL,
 	.ops_pci		= &minihd_pci_ops,
 };
 
diff --git a/src/northbridge/intel/haswell/northbridge.c b/src/northbridge/intel/haswell/northbridge.c
index 79ab747f6d..d1b6ca730f 100644
--- a/src/northbridge/intel/haswell/northbridge.c
+++ b/src/northbridge/intel/haswell/northbridge.c
@@ -31,11 +31,9 @@
 #include "chip.h"
 #include "haswell.h"
 
-static int get_pcie_bar(struct device *dev, unsigned int index, u32 *base,
-			u32 *len)
+static int get_pcie_bar(struct device *dev, unsigned int index, u32 *base, u32 *len)
 {
-	u32 pciexbar_reg;
-	u32 mask;
+	u32 pciexbar_reg, mask;
 
 	*base = 0;
 	*len = 0;
@@ -46,18 +44,18 @@ static int get_pcie_bar(struct device *dev, unsigned int index, u32 *base,
 		return 0;
 
 	switch ((pciexbar_reg >> 1) & 3) {
-	case 0: // 256MB
+	case 0: /* 256MB */
 		mask = (1UL << 31) | (1 << 30) | (1 << 29) | (1 << 28);
 		*base = pciexbar_reg & mask;
 		*len = 256 * 1024 * 1024;
 		return 1;
-	case 1: // 128M
+	case 1: /* 128M */
 		mask = (1UL << 31) | (1 << 30) | (1 << 29) | (1 << 28);
 		mask |= (1 << 27);
 		*base = pciexbar_reg & mask;
 		*len = 128 * 1024 * 1024;
 		return 1;
-	case 2: // 64M
+	case 2: /* 64M */
 		mask = (1UL << 31) | (1 << 30) | (1 << 29) | (1 << 28);
 		mask |= (1 << 27) | (1 << 26);
 		*base = pciexbar_reg & mask;
@@ -89,50 +87,47 @@ static const char *northbridge_acpi_name(const struct device *dev)
 	return NULL;
 }
 
-	/* TODO We could determine how many PCIe busses we need in
-	 * the bar. For now that number is hardcoded to a max of 64.
-	 */
+/*
+ * TODO: We could determine how many PCIe busses we need in the bar.
+ *       For now, that number is hardcoded to a max of 64.
+ */
 static struct device_operations pci_domain_ops = {
-	.read_resources   = pci_domain_read_resources,
-	.set_resources    = pci_domain_set_resources,
-	.enable_resources = NULL,
-	.init             = NULL,
-	.scan_bus         = pci_domain_scan_bus,
-	.acpi_name	  = northbridge_acpi_name,
+	.read_resources    = pci_domain_read_resources,
+	.set_resources     = pci_domain_set_resources,
+	.enable_resources  = NULL,
+	.init              = NULL,
+	.scan_bus          = pci_domain_scan_bus,
+	.acpi_name         = northbridge_acpi_name,
 	.write_acpi_tables = northbridge_write_acpi_tables,
 };
 
 static int get_bar(struct device *dev, unsigned int index, u32 *base, u32 *len)
 {
-	u32 bar;
+	u32 bar = pci_read_config32(dev, index);
 
-	bar = pci_read_config32(dev, index);
-
-	/* If not enabled don't report it. */
+	/* If not enabled don't report it */
 	if (!(bar & 0x1))
 		return 0;
 
-	/* Knock down the enable bit. */
+	/* Knock down the enable bit */
 	*base = bar & ~1;
 
 	return 1;
 }
 
-/* There are special BARs that actually are programmed in the MCHBAR. These
- * Intel special features, but they do consume resources that need to be
- * accounted for. */
-static int get_bar_in_mchbar(struct device *dev, unsigned int index,
-			     u32 *base, u32 *len)
+/*
+ * There are special BARs that actually are programmed in the MCHBAR. These Intel special
+ * features, but they do consume resources that need to be accounted for.
+ */
+static int get_bar_in_mchbar(struct device *dev, unsigned int index, u32 *base, u32 *len)
 {
-	u32 bar;
+	u32 bar = MCHBAR32(index);
 
-	bar = MCHBAR32(index);
-
-	/* If not enabled don't report it. */
+	/* If not enabled don't report it */
 	if (!(bar & 0x1))
 		return 0;
 
-	/* Knock down the enable bit. */
+	/* Knock down the enable bit */
 	*base = bar & ~1;
 
 	return 1;
@@ -141,26 +136,22 @@ static int get_bar_in_mchbar(struct device *dev, unsigned int index,
 struct fixed_mmio_descriptor {
 	unsigned int index;
 	u32 size;
-	int (*get_resource)(struct device *dev, unsigned int index,
-			    u32 *base, u32 *size);
+	int (*get_resource)(struct device *dev, unsigned int index, u32 *base, u32 *size);
 	const char *description;
 };
 
-#define SIZE_KB(x) ((x)*1024)
+#define SIZE_KB(x) ((x) * 1024)
 struct fixed_mmio_descriptor mc_fixed_resources[] = {
 	{ PCIEXBAR, SIZE_KB(0),  get_pcie_bar,      "PCIEXBAR" },
 	{ MCHBAR,   SIZE_KB(32), get_bar,           "MCHBAR"   },
 	{ DMIBAR,   SIZE_KB(4),  get_bar,           "DMIBAR"   },
 	{ EPBAR,    SIZE_KB(4),  get_bar,           "EPBAR"    },
-	{ 0x5420,   SIZE_KB(4),  get_bar_in_mchbar, "GDXCBAR"  },
-	{ 0x5408,   SIZE_KB(16), get_bar_in_mchbar, "EDRAMBAR" },
+	{ GDXCBAR,  SIZE_KB(4),  get_bar_in_mchbar, "GDXCBAR"  },
+	{ EDRAMBAR, SIZE_KB(16), get_bar_in_mchbar, "EDRAMBAR" },
 };
 #undef SIZE_KB
 
-/*
- * Add all known fixed MMIO ranges that hang off the host bridge/memory
- * controller device.
- */
+/* Add all known fixed MMIO ranges that hang off the host bridge/memory controller device. */
 static void mc_add_fixed_mmio_resources(struct device *dev)
 {
 	int i;
@@ -173,14 +164,13 @@ static void mc_add_fixed_mmio_resources(struct device *dev)
 
 		size = mc_fixed_resources[i].size;
 		index = mc_fixed_resources[i].index;
-		if (!mc_fixed_resources[i].get_resource(dev, index,
-							&base, &size))
+		if (!mc_fixed_resources[i].get_resource(dev, index, &base, &size))
 			continue;
 
 		resource = new_resource(dev, mc_fixed_resources[i].index);
-		resource->flags = IORESOURCE_MEM | IORESOURCE_FIXED |
-				  IORESOURCE_STORED | IORESOURCE_RESERVE |
-				  IORESOURCE_ASSIGNED;
+		resource->flags = IORESOURCE_MEM | IORESOURCE_FIXED | IORESOURCE_STORED |
+				  IORESOURCE_RESERVE | IORESOURCE_ASSIGNED;
+
 		resource->base = base;
 		resource->size = size;
 		printk(BIOS_DEBUG, "%s: Adding %s @ %x 0x%08lx-0x%08lx.\n",
@@ -205,10 +195,10 @@ static void mc_add_fixed_mmio_resources(struct device *dev)
  * |     Usage DRAM           |
  * +--------------------------+ 0
  *
- * Some of the base registers above can be equal making the size of those
- * regions 0. The reason is because the memory controller internally subtracts
- * the base registers from each other to determine sizes of the regions. In
- * other words, the memory map is in a fixed order no matter what.
+ * Some of the base registers above can be equal, making the size of the regions within 0.
+ * This is because the memory controller internally subtracts the base registers from each
+ * other to determine sizes of the regions. In other words, the memory map regions are always
+ * in a fixed order, no matter what sizes they have.
  */
 
 struct map_entry {
@@ -218,14 +208,13 @@ struct map_entry {
 	const char *description;
 };
 
-static void read_map_entry(struct device *dev, struct map_entry *entry,
-			   uint64_t *result)
+static void read_map_entry(struct device *dev, struct map_entry *entry, uint64_t *result)
 {
 	uint64_t value;
 	uint64_t mask;
 
-	/* All registers are on a 1MiB granularity. */
-	mask = ((1ULL<<20)-1);
+	/* All registers have a 1MiB granularity */
+	mask = ((1ULL << 20) - 1);
 	mask = ~mask;
 
 	value = 0;
@@ -252,12 +241,9 @@ static void read_map_entry(struct device *dev, struct map_entry *entry,
 		.description = desc_,  \
 	}
 
-#define MAP_ENTRY_BASE_64(reg_, desc_) \
-	MAP_ENTRY(reg_, 1, 0, desc_)
-#define MAP_ENTRY_LIMIT_64(reg_, desc_) \
-	MAP_ENTRY(reg_, 1, 1, desc_)
-#define MAP_ENTRY_BASE_32(reg_, desc_) \
-	MAP_ENTRY(reg_, 0, 0, desc_)
+#define MAP_ENTRY_BASE_32(reg_, desc_)	MAP_ENTRY(reg_, 0, 0, desc_)
+#define MAP_ENTRY_BASE_64(reg_, desc_)	MAP_ENTRY(reg_, 1, 0, desc_)
+#define MAP_ENTRY_LIMIT_64(reg_, desc_)	MAP_ENTRY(reg_, 1, 1, desc_)
 
 enum {
 	TOM_REG,
@@ -270,21 +256,21 @@ enum {
 	BGSM_REG,
 	BDSM_REG,
 	TSEG_REG,
-	// Must be last.
-	NUM_MAP_ENTRIES
+	/* Must be last */
+	NUM_MAP_ENTRIES,
 };
 
 static struct map_entry memory_map[NUM_MAP_ENTRIES] = {
-	[TOM_REG] = MAP_ENTRY_BASE_64(TOM, "TOM"),
-	[TOUUD_REG] = MAP_ENTRY_BASE_64(TOUUD, "TOUUD"),
-	[MESEG_BASE_REG] = MAP_ENTRY_BASE_64(MESEG_BASE, "MESEG_BASE"),
+	[TOM_REG]         = MAP_ENTRY_BASE_64(TOM, "TOM"),
+	[TOUUD_REG]       = MAP_ENTRY_BASE_64(TOUUD, "TOUUD"),
+	[MESEG_BASE_REG]  = MAP_ENTRY_BASE_64(MESEG_BASE, "MESEG_BASE"),
 	[MESEG_LIMIT_REG] = MAP_ENTRY_LIMIT_64(MESEG_LIMIT, "MESEG_LIMIT"),
-	[REMAP_BASE_REG] = MAP_ENTRY_BASE_64(REMAPBASE, "REMAP_BASE"),
+	[REMAP_BASE_REG]  = MAP_ENTRY_BASE_64(REMAPBASE, "REMAP_BASE"),
 	[REMAP_LIMIT_REG] = MAP_ENTRY_LIMIT_64(REMAPLIMIT, "REMAP_LIMIT"),
-	[TOLUD_REG] = MAP_ENTRY_BASE_32(TOLUD, "TOLUD"),
-	[BDSM_REG] = MAP_ENTRY_BASE_32(BDSM, "BDSM"),
-	[BGSM_REG] = MAP_ENTRY_BASE_32(BGSM, "BGSM"),
-	[TSEG_REG] = MAP_ENTRY_BASE_32(TSEG, "TESGMB"),
+	[TOLUD_REG]       = MAP_ENTRY_BASE_32(TOLUD, "TOLUD"),
+	[BDSM_REG]        = MAP_ENTRY_BASE_32(BDSM, "BDSM"),
+	[BGSM_REG]        = MAP_ENTRY_BASE_32(BGSM, "BGSM"),
+	[TSEG_REG]        = MAP_ENTRY_BASE_32(TSEG, "TESGMB"),
 };
 
 static void mc_read_map_entries(struct device *dev, uint64_t *values)
@@ -302,51 +288,45 @@ static void mc_report_map_entries(struct device *dev, uint64_t *values)
 		printk(BIOS_DEBUG, "MC MAP: %s: 0x%llx\n",
 		       memory_map[i].description, values[i]);
 	}
-	/* One can validate the BDSM and BGSM against the GGC. */
+	/* One can validate the BDSM and BGSM against the GGC */
 	printk(BIOS_DEBUG, "MC MAP: GGC: 0x%x\n", pci_read_config16(dev, GGC));
 }
 
 static void mc_add_dram_resources(struct device *dev, int *resource_cnt)
 {
-	unsigned long base_k, size_k;
-	unsigned long touud_k;
-	unsigned long index;
+	unsigned long base_k, size_k, touud_k, index;
 	struct resource *resource;
 	uint64_t mc_values[NUM_MAP_ENTRIES];
 
-	/* Read in the MAP registers and report their values. */
+	/* Read in the MAP registers and report their values */
 	mc_read_map_entries(dev, &mc_values[0]);
 	mc_report_map_entries(dev, &mc_values[0]);
 
 	/*
 	 * These are the host memory ranges that should be added:
-	 * - 0 -> 0xa0000: cacheable
-	 * - 0xc0000 -> TSEG : cacheable
-	 * - TESG -> BGSM: cacheable with standard MTRRs and reserved
-	 * - BGSM -> TOLUD: not cacheable with standard MTRRs and reserved
-	 * - 4GiB -> TOUUD: cacheable
+	 * - 0 -> 0xa0000:    cacheable
+	 * - 0xc0000 -> TSEG: cacheable
+	 * - TSEG -> BGSM:    cacheable with standard MTRRs and reserved
+	 * - BGSM -> TOLUD:   not cacheable with standard MTRRs and reserved
+	 * - 4GiB -> TOUUD:   cacheable
 	 *
-	 * The default SMRAM space is reserved so that the range doesn't
-	 * have to be saved during S3 Resume. Once marked reserved the OS
-	 * cannot use the memory. This is a bit of an odd place to reserve
-	 * the region, but the CPU devices don't have dev_ops->read_resources()
-	 * called on them.
+	 * The default SMRAM space is reserved so that the range doesn't have to be saved
+	 * during S3 Resume. Once marked reserved the OS cannot use the memory. This is a
+	 * bit of an odd place to reserve the region, but the CPU devices don't have
+	 * dev_ops->read_resources() called on them.
 	 *
-	 * The range 0xa0000 -> 0xc0000 does not have any resources
-	 * associated with it to handle legacy VGA memory. If this range
-	 * is not omitted the mtrr code will setup the area as cacheable
-	 * causing VGA access to not work.
+	 * The range 0xa0000 -> 0xc0000 does not have any resources associated with it to
+	 * handle legacy VGA memory. If this range is not omitted the mtrr code will setup
+	 * the area as cacheable, causing VGA access to not work.
 	 *
-	 * The TSEG region is mapped as cacheable so that one can perform
-	 * SMRAM relocation faster. Once the SMRR is enabled the SMRR takes
-	 * precedence over the existing MTRRs covering this region.
+	 * The TSEG region is mapped as cacheable so that one can perform SMRAM relocation
+	 * faster. Once the SMRR is enabled, the SMRR takes precedence over the existing
+	 * MTRRs covering this region.
 	 *
-	 * It should be noted that cacheable entry types need to be added in
-	 * order. The reason is that the current MTRR code assumes this and
-	 * falls over itself if it isn't.
+	 * It should be noted that cacheable entry types need to be added in order. The reason
+	 * is that the current MTRR code assumes this and falls over itself if it isn't.
 	 *
-	 * The resource index starts low and should not meet or exceed
-	 * PCI_BASE_ADDRESS_0.
+	 * The resource index starts low and should not meet or exceed PCI_BASE_ADDRESS_0.
 	 */
 	index = *resource_cnt;
 
@@ -364,18 +344,16 @@ static void mc_add_dram_resources(struct device *dev, int *resource_cnt)
 	resource = new_resource(dev, index++);
 	resource->base = mc_values[TSEG_REG];
 	resource->size = mc_values[BGSM_REG] - resource->base;
-	resource->flags = IORESOURCE_MEM | IORESOURCE_FIXED |
-			  IORESOURCE_STORED | IORESOURCE_RESERVE |
-			  IORESOURCE_ASSIGNED | IORESOURCE_CACHEABLE;
+	resource->flags = IORESOURCE_MEM | IORESOURCE_FIXED | IORESOURCE_STORED |
+			  IORESOURCE_RESERVE | IORESOURCE_ASSIGNED | IORESOURCE_CACHEABLE;
 
-	/* BGSM -> TOLUD. If the IGD is disabled, BGSM can equal TOLUD */
+	/* BGSM -> TOLUD. If the IGD is disabled, BGSM can equal TOLUD. */
 	if (mc_values[BGSM_REG] != mc_values[TOLUD_REG]) {
 		resource = new_resource(dev, index++);
 		resource->base = mc_values[BGSM_REG];
 		resource->size = mc_values[TOLUD_REG] - resource->base;
-		resource->flags = IORESOURCE_MEM | IORESOURCE_FIXED |
-				  IORESOURCE_STORED | IORESOURCE_RESERVE |
-				  IORESOURCE_ASSIGNED;
+		resource->flags = IORESOURCE_MEM | IORESOURCE_FIXED | IORESOURCE_STORED |
+				  IORESOURCE_RESERVE | IORESOURCE_ASSIGNED;
 	}
 
 	/* 4GiB -> TOUUD */
@@ -387,16 +365,16 @@ static void mc_add_dram_resources(struct device *dev, int *resource_cnt)
 
 	/* Reserve everything between A segment and 1MB:
 	 *
-	 * 0xa0000 - 0xbffff: legacy VGA
+	 * 0xa0000 - 0xbffff: Legacy VGA
 	 * 0xc0000 - 0xfffff: RAM
 	 */
 	mmio_resource(dev, index++, (0xa0000 >> 10), (0xc0000 - 0xa0000) >> 10);
-	reserved_ram_resource(dev, index++, (0xc0000 >> 10),
-			      (0x100000 - 0xc0000) >> 10);
+	reserved_ram_resource(dev, index++, (0xc0000 >> 10), (0x100000 - 0xc0000) >> 10);
+
 #if CONFIG(CHROMEOS_RAMOOPS)
 	reserved_ram_resource(dev, index++,
 			CONFIG_CHROMEOS_RAMOOPS_RAM_START >> 10,
-			CONFIG_CHROMEOS_RAMOOPS_RAM_SIZE >> 10);
+			CONFIG_CHROMEOS_RAMOOPS_RAM_SIZE  >> 10);
 #endif
 	*resource_cnt = index;
 }
@@ -404,31 +382,27 @@ static void mc_add_dram_resources(struct device *dev, int *resource_cnt)
 static void mc_read_resources(struct device *dev)
 {
 	int index = 0;
-	const bool vtd_capable =
-		!(pci_read_config32(dev, CAPID0_A) & VTD_DISABLE);
+	const bool vtd_capable = !(pci_read_config32(dev, CAPID0_A) & VTD_DISABLE);
 
-	/* Read standard PCI resources. */
+	/* Read standard PCI resources */
 	pci_dev_read_resources(dev);
 
-	/* Add all fixed MMIO resources. */
+	/* Add all fixed MMIO resources */
 	mc_add_fixed_mmio_resources(dev);
 
-	/* Add VT-d MMIO resources if capable */
+	/* Add VT-d MMIO resources, if capable */
 	if (vtd_capable) {
-		mmio_resource(dev, index++, GFXVT_BASE_ADDRESS / KiB,
-			GFXVT_BASE_SIZE / KiB);
-		mmio_resource(dev, index++, VTVC0_BASE_ADDRESS / KiB,
-			VTVC0_BASE_SIZE / KiB);
+		mmio_resource(dev, index++, GFXVT_BASE_ADDRESS / KiB, GFXVT_BASE_SIZE / KiB);
+		mmio_resource(dev, index++, VTVC0_BASE_ADDRESS / KiB, VTVC0_BASE_SIZE / KiB);
 	}
 
-	/* Calculate and add DRAM resources. */
+	/* Calculate and add DRAM resources */
 	mc_add_dram_resources(dev, &index);
 }
 
 /*
- * The Mini-HD audio device is disabled whenever the IGD is. This is
- * because it provides audio over the integrated graphics port(s), which
- * requires the IGD to be functional.
+ * The Mini-HD audio device is disabled whenever the IGD is. This is because it provides
+ * audio over the integrated graphics port(s), which requires the IGD to be functional.
  */
 static void disable_devices(void)
 {
@@ -446,7 +420,7 @@ static void disable_devices(void)
 		{ PCI_DEVFN(7, 0), DEVEN_D7EN, "\"device 7\"" },
 	};
 
-	struct device *host_dev = pcidev_on_root(0x0, 0);
+	struct device *host_dev = pcidev_on_root(0, 0);
 	u32 deven;
 	size_t i;
 
@@ -470,29 +444,29 @@ static void northbridge_init(struct device *dev)
 {
 	u8 bios_reset_cpl, pair;
 
-	/* Enable Power Aware Interrupt Routing */
-	pair = MCHBAR8(0x5418);
+	/* Enable Power Aware Interrupt Routing. */
+	pair = MCHBAR8(INTRDIRCTL);
 	pair &= ~0x7;	/* Clear 2:0 */
 	pair |= 0x4;	/* Fixed Priority */
-	MCHBAR8(0x5418) = pair;
+	MCHBAR8(INTRDIRCTL) = pair;
 
 	disable_devices();
 
 	/*
-	 * Set bits 0+1 of BIOS_RESET_CPL to indicate to the CPU
-	 * that BIOS has initialized memory and power management
+	 * Set bits 0 + 1 of BIOS_RESET_CPL to indicate to the CPU
+	 * that BIOS has initialized memory and power management.
 	 */
 	bios_reset_cpl = MCHBAR8(BIOS_RESET_CPL);
 	bios_reset_cpl |= 3;
 	MCHBAR8(BIOS_RESET_CPL) = bios_reset_cpl;
 	printk(BIOS_DEBUG, "Set BIOS_RESET_CPL\n");
 
-	/* Configure turbo power limits 1ms after reset complete bit */
+	/* Configure turbo power limits 1ms after reset complete bit. */
 	mdelay(1);
 	set_power_limits(28);
 
-	/* Set here before graphics PM init */
-	MCHBAR32(0x5500) = 0x00100001;
+	/* Set here before graphics PM init. */
+	MCHBAR32(MMIO_PAVP_MSG) = 0x00100001;
 }
 
 static struct pci_operations intel_pci_ops = {
@@ -500,13 +474,13 @@ static struct pci_operations intel_pci_ops = {
 };
 
 static struct device_operations mc_ops = {
-	.read_resources   = mc_read_resources,
-	.set_resources    = pci_dev_set_resources,
-	.enable_resources = pci_dev_enable_resources,
-	.init             = northbridge_init,
+	.read_resources           = mc_read_resources,
+	.set_resources            = pci_dev_set_resources,
+	.enable_resources         = pci_dev_enable_resources,
+	.init                     = northbridge_init,
 	.acpi_fill_ssdt_generator = generate_cpu_entries,
-	.scan_bus         = 0,
-	.ops_pci          = &intel_pci_ops,
+	.scan_bus                 = NULL,
+	.ops_pci                  = &intel_pci_ops,
 };
 
 static const unsigned short mc_pci_device_ids[] = {
@@ -528,12 +502,12 @@ static struct device_operations cpu_bus_ops = {
 	.set_resources    = DEVICE_NOOP,
 	.enable_resources = DEVICE_NOOP,
 	.init             = mp_cpu_bus_init,
-	.scan_bus         = 0,
+	.scan_bus         = NULL,
 };
 
 static void enable_dev(struct device *dev)
 {
-	/* Set the operations if it is a special bus type */
+	/* Set the operations if it is a special bus type. */
 	if (dev->path.type == DEVICE_PATH_DOMAIN) {
 		dev->ops = &pci_domain_ops;
 	} else if (dev->path.type == DEVICE_PATH_CPU_CLUSTER) {
diff --git a/src/northbridge/intel/haswell/pei_data.h b/src/northbridge/intel/haswell/pei_data.h
index dfc34d8ce9..6e537403b3 100644
--- a/src/northbridge/intel/haswell/pei_data.h
+++ b/src/northbridge/intel/haswell/pei_data.h
@@ -101,10 +101,7 @@ struct pei_data
 	/* Data from MRC that should be saved to flash */
 	unsigned char *mrc_output;
 	unsigned int mrc_output_len;
-	/*
-	 * Max frequency DDR3 could be ran at. Could be one of four values: 800,
-	 * 1067, 1333, 1600
-	 */
+	/* Max frequency to run DDR3 at. Can be one of four values: 800, 1067, 1333, 1600 */
 	uint32_t max_ddr3_freq;
 	/* Route all USB ports to XHCI controller in resume path */
 	int usb_xhci_on_resume;
diff --git a/src/northbridge/intel/haswell/raminit.c b/src/northbridge/intel/haswell/raminit.c
index 9615cb08f6..1eb4f4361b 100644
--- a/src/northbridge/intel/haswell/raminit.c
+++ b/src/northbridge/intel/haswell/raminit.c
@@ -37,71 +37,71 @@
 void save_mrc_data(struct pei_data *pei_data)
 {
 	/* Save the MRC S3 restore data to cbmem */
-	mrc_cache_stash_data(MRC_TRAINING_DATA, MRC_CACHE_VERSION,
-			pei_data->mrc_output, pei_data->mrc_output_len);
+	mrc_cache_stash_data(MRC_TRAINING_DATA, MRC_CACHE_VERSION, pei_data->mrc_output,
+			     pei_data->mrc_output_len);
 }
 
 static void prepare_mrc_cache(struct pei_data *pei_data)
 {
 	struct region_device rdev;
 
-	// preset just in case there is an error
+	/* Preset just in case there is an error */
 	pei_data->mrc_input = NULL;
 	pei_data->mrc_input_len = 0;
 
 	if (mrc_cache_get_current(MRC_TRAINING_DATA, MRC_CACHE_VERSION, &rdev))
-		/* error message printed in find_current_mrc_cache */
+		/* Error message printed in find_current_mrc_cache */
 		return;
 
 	pei_data->mrc_input = rdev_mmap_full(&rdev);
 	pei_data->mrc_input_len = region_device_sz(&rdev);
 
-	printk(BIOS_DEBUG, "%s: at %p, size %x\n",
-	       __func__, pei_data->mrc_input, pei_data->mrc_input_len);
+	printk(BIOS_DEBUG, "%s: at %p, size %x\n", __func__, pei_data->mrc_input,
+	       pei_data->mrc_input_len);
 }
 
 static const char *ecc_decoder[] = {
 	"inactive",
 	"active on IO",
 	"disabled on IO",
-	"active"
+	"active",
 };
 
-/*
- * Dump in the log memory controller configuration as read from the memory
- * controller registers.
- */
+/* Print out the memory controller configuration, as per the values in its registers. */
 static void report_memory_config(void)
 {
-	u32 addr_decoder_common, addr_decode_ch[2];
+	u32 addr_decoder_common, addr_decode_chan[2];
 	int i;
 
-	addr_decoder_common = MCHBAR32(0x5000);
-	addr_decode_ch[0] = MCHBAR32(0x5004);
-	addr_decode_ch[1] = MCHBAR32(0x5008);
+	addr_decoder_common = MCHBAR32(MAD_CHNL);
+	addr_decode_chan[0] = MCHBAR32(MAD_DIMM_CH0);
+	addr_decode_chan[1] = MCHBAR32(MAD_DIMM_CH1);
 
 	printk(BIOS_DEBUG, "memcfg DDR3 clock %d MHz\n",
-	       (MCHBAR32(0x5e04) * 13333 * 2 + 50)/100);
+	       (MCHBAR32(MC_BIOS_DATA) * 13333 * 2 + 50) / 100);
+
 	printk(BIOS_DEBUG, "memcfg channel assignment: A: %d, B % d, C % d\n",
-	       addr_decoder_common & 3,
+	       (addr_decoder_common >> 0) & 3,
 	       (addr_decoder_common >> 2) & 3,
 	       (addr_decoder_common >> 4) & 3);
 
-	for (i = 0; i < ARRAY_SIZE(addr_decode_ch); i++) {
-		u32 ch_conf = addr_decode_ch[i];
-		printk(BIOS_DEBUG, "memcfg channel[%d] config (%8.8x):\n",
-		       i, ch_conf);
-		printk(BIOS_DEBUG, "   ECC %s\n",
-		       ecc_decoder[(ch_conf >> 24) & 3]);
+	for (i = 0; i < ARRAY_SIZE(addr_decode_chan); i++) {
+		u32 ch_conf = addr_decode_chan[i];
+
+		printk(BIOS_DEBUG, "memcfg channel[%d] config (%8.8x):\n", i, ch_conf);
+		printk(BIOS_DEBUG, "   ECC %s\n", ecc_decoder[(ch_conf >> 24) & 3]);
 		printk(BIOS_DEBUG, "   enhanced interleave mode %s\n",
 		       ((ch_conf >> 22) & 1) ? "on" : "off");
+
 		printk(BIOS_DEBUG, "   rank interleave %s\n",
 		       ((ch_conf >> 21) & 1) ? "on" : "off");
+
 		printk(BIOS_DEBUG, "   DIMMA %d MB width %s %s rank%s\n",
 		       ((ch_conf >> 0) & 0xff) * 256,
 		       ((ch_conf >> 19) & 1) ? "x16" : "x8 or x32",
 		       ((ch_conf >> 17) & 1) ? "dual" : "single",
 		       ((ch_conf >> 16) & 1) ? "" : ", selected");
+
 		printk(BIOS_DEBUG, "   DIMMB %d MB width %s %s rank%s\n",
 		       ((ch_conf >> 8) & 0xff) * 256,
 		       ((ch_conf >> 20) & 1) ? "x16" : "x8 or x32",
@@ -123,14 +123,11 @@ void sdram_initialize(struct pei_data *pei_data)
 
 	printk(BIOS_DEBUG, "Starting UEFI PEI System Agent\n");
 
-	/*
-	 * Do not pass MRC data in for recovery mode boot,
-	 * Always pass it in for S3 resume.
-	 */
+	/* Do not pass MRC data in for recovery mode boot, always pass it in for S3 resume */
 	if (!vboot_recovery_mode_enabled() || pei_data->boot_mode == 2)
 		prepare_mrc_cache(pei_data);
 
-	/* If MRC data is not found we cannot continue S3 resume. */
+	/* If MRC data is not found, we cannot continue S3 resume */
 	if (pei_data->boot_mode == 2 && !pei_data->mrc_input) {
 		post_code(POST_RESUME_FAILURE);
 		printk(BIOS_DEBUG, "Giving up in %s: No MRC data\n", __func__);
@@ -141,21 +138,20 @@ void sdram_initialize(struct pei_data *pei_data)
 	pei_data->tx_byte = do_putchar;
 
 	/*
-	 * Locate and call UEFI System Agent binary. The binary needs to be at
-	 * a fixed offset in the flash and can therefore only reside in the
-	 * COREBOOT fmap region
+	 * Locate and call UEFI System Agent binary. The binary needs to be at a fixed offset
+	 * in the flash and can therefore only reside in the COREBOOT fmap region.
 	 */
 	if (cbfs_locate_file_in_region(&f, "COREBOOT", "mrc.bin", &type) < 0)
 		die("mrc.bin not found!");
+
 	/* We don't care about leaking the mapping */
 	entry = (unsigned long)rdev_mmap_full(&f.data);
 	if (entry) {
 		int rv;
-		asm volatile (
-			      "call *%%ecx\n\t"
+		asm volatile ("call *%%ecx\n\t"
 			      :"=a" (rv) : "c" (entry), "a" (pei_data));
 
-		/* mrc.bin reconfigures USB, so reinit it to have debug */
+		/* The mrc.bin reconfigures USB, so usbdebug needs to be reinitialized */
 		if (CONFIG(USBDEBUG_IN_PRE_RAM))
 			usbdebug_hw_init(true);
 
@@ -177,13 +173,11 @@ void sdram_initialize(struct pei_data *pei_data)
 		die("UEFI PEI System Agent not found.\n");
 	}
 
-	/* For reference print the System Agent version
-	 * after executing the UEFI PEI stage.
-	 */
-	u32 version = MCHBAR32(0x5034);
+	/* For reference, print the System Agent version after executing the UEFI PEI stage */
+	u32 version = MCHBAR32(MRC_REVISION);
 	printk(BIOS_DEBUG, "System Agent Version %d.%d.%d Build %d\n",
-		version >> 24, (version >> 16) & 0xff,
-		(version >> 8) & 0xff, version & 0xff);
+	       (version >> 24) & 0xff, (version >> 16) & 0xff,
+	       (version >>  8) & 0xff, (version >>  0) & 0xff);
 
 	report_memory_config();
 }
@@ -191,24 +185,23 @@ void sdram_initialize(struct pei_data *pei_data)
 void setup_sdram_meminfo(struct pei_data *pei_data)
 {
 	u32 addr_decode_ch[2];
-	struct memory_info* mem_info;
+	struct memory_info *mem_info;
 	struct dimm_info *dimm;
-	int ddr_frequency;
-	int dimm_size;
-	int ch, d_num;
+	int ddr_frequency, dimm_size, ch, d_num;
 	int dimm_cnt = 0;
 
 	mem_info = cbmem_add(CBMEM_ID_MEMINFO, sizeof(struct memory_info));
 	if (!mem_info)
 		die("Failed to add memory info to CBMEM.\n");
+
 	memset(mem_info, 0, sizeof(struct memory_info));
 
-	/* FIXME: Do we need to read MCHBAR32(0x5000) ? */
-	MCHBAR32(0x5000);
-	addr_decode_ch[0] = MCHBAR32(0x5004);
-	addr_decode_ch[1] = MCHBAR32(0x5008);
+	/* FIXME: Do we need to read MCHBAR32(MAD_CHNL) ? (Answer: Nope) */
+	MCHBAR32(MAD_CHNL);
+	addr_decode_ch[0] = MCHBAR32(MAD_DIMM_CH0);
+	addr_decode_ch[1] = MCHBAR32(MAD_DIMM_CH1);
 
-	ddr_frequency = (MCHBAR32(0x5e04) * 13333 * 2 + 50) / 100;
+	ddr_frequency = (MCHBAR32(MC_BIOS_DATA) * 13333 * 2 + 50) / 100;
 
 	for (ch = 0; ch < ARRAY_SIZE(addr_decode_ch); ch++) {
 		u32 ch_conf = addr_decode_ch[ch];
@@ -232,7 +225,7 @@ void setup_sdram_meminfo(struct pei_data *pei_data)
 					SPD_DIMM_PART_LEN);
 				dimm->mod_id =
 					(pei_data->spd_data[dimm_cnt][SPD_DIMM_MOD_ID2] << 8) |
-					(pei_data->spd_data[dimm_cnt][SPD_DIMM_MOD_ID1] & 0xFF);
+					(pei_data->spd_data[dimm_cnt][SPD_DIMM_MOD_ID1] & 0xff);
 				dimm->mod_type = SPD_SODIMM;
 				dimm->bus_width = 0x3;	/* 64-bit */
 				dimm_cnt++;