soc/intel/broadwell: Align cosmetics with Haswell/Lynx Point

Tested with BUILD_TIMELESS=1, Purism Librem 13v1 does not change.

Change-Id: Icf41d9db20e492ec77a83f8413ac99a654d6c8ed
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45697
Reviewed-by: Matt DeVillier <matt.devillier@gmail.com>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

10 files changed, 60 insertions, 76 deletions

diff --git a/src/soc/intel/broadwell/acpi/lpc.asl b/src/soc/intel/broadwell/acpi/lpc.asl
index 33c8dc92a0..5bdfea24ce 100644
--- a/src/soc/intel/broadwell/acpi/lpc.asl
+++ b/src/soc/intel/broadwell/acpi/lpc.asl
@@ -81,9 +81,7 @@ Device (LPCB)
 		Method (_CRS, 0, Serialized) // Current resources
 		{
 			If (HPTE) {
-				CreateDWordField (BUF0,
-					\_SB.PCI0.LPCB.HPET.FED0._BAS, HPT0)
-
+				CreateDWordField (BUF0, \_SB.PCI0.LPCB.HPET.FED0._BAS, HPT0)
 				If (Lequal(HPAS, 1)) {
 					Add(CONFIG_HPET_ADDRESS, 0x1000, HPT0)
 				}
@@ -153,8 +151,7 @@ Device (LPCB)
 			IO (Decode16, 0x80, 0x80, 0x1, 0x01) // Port 80 Post
 			IO (Decode16, 0x92, 0x92, 0x1, 0x01) // CPU Reserved
 			IO (Decode16, 0xb2, 0xb2, 0x1, 0x02) // SWSMI
-			IO (Decode16, ACPI_BASE_ADDRESS, ACPI_BASE_ADDRESS,
-			    0x1, 0xff)
+			IO (Decode16, ACPI_BASE_ADDRESS, ACPI_BASE_ADDRESS, 0x1, 0xff)
 		})
 
 		Method (_CRS, 0, NotSerialized)
@@ -169,7 +166,6 @@ Device (LPCB)
 		Name (_CRS, ResourceTemplate()
 		{
 			IO (Decode16, 0x70, 0x70, 1, 8)
-			//IRQNoFlags() { 8 }
 		})
 	}
 
diff --git a/src/soc/intel/broadwell/cpu.c b/src/soc/intel/broadwell/cpu.c
index 8a38137b9d..c64af02bba 100644
--- a/src/soc/intel/broadwell/cpu.c
+++ b/src/soc/intel/broadwell/cpu.c
@@ -400,7 +400,7 @@ static void cpu_core_init(struct device *cpu)
 	/* Clear out pending MCEs */
 	configure_mca();
 
-	/* Enable the local CPU apics */
+	/* Enable the local CPU APICs */
 	enable_lapic_tpr();
 	setup_lapic();
 
diff --git a/src/soc/intel/broadwell/fadt.c b/src/soc/intel/broadwell/fadt.c
index f39ad801e0..8fbd0c45ad 100644
--- a/src/soc/intel/broadwell/fadt.c
+++ b/src/soc/intel/broadwell/fadt.c
@@ -38,10 +38,13 @@ void acpi_fill_fadt(acpi_fadt_t *fadt)
 	fadt->century = 0x00;
 	fadt->iapc_boot_arch = ACPI_FADT_LEGACY_DEVICES | ACPI_FADT_8042;
 
-	fadt->flags |= ACPI_FADT_WBINVD | ACPI_FADT_C1_SUPPORTED |
-			ACPI_FADT_C2_MP_SUPPORTED | ACPI_FADT_SLEEP_BUTTON |
-			ACPI_FADT_SEALED_CASE | ACPI_FADT_S4_RTC_WAKE |
-			ACPI_FADT_PLATFORM_CLOCK;
+	fadt->flags |= ACPI_FADT_WBINVD |
+		       ACPI_FADT_C1_SUPPORTED |
+		       ACPI_FADT_C2_MP_SUPPORTED |
+		       ACPI_FADT_SLEEP_BUTTON |
+		       ACPI_FADT_SEALED_CASE |
+		       ACPI_FADT_S4_RTC_WAKE |
+		       ACPI_FADT_PLATFORM_CLOCK;
 
 	fadt->x_pm1a_evt_blk.space_id = ACPI_ADDRESS_SPACE_IO;
 	fadt->x_pm1a_evt_blk.bit_width = fadt->pm1_evt_len * 8;
@@ -82,5 +85,5 @@ void acpi_fill_fadt(acpi_fadt_t *fadt)
 	fadt->x_gpe0_blk.bit_offset = 0;
 	fadt->x_gpe0_blk.access_size = ACPI_ACCESS_SIZE_BYTE_ACCESS;
 	fadt->x_gpe0_blk.addrl = fadt->gpe0_blk;
-	fadt->x_gpe0_blk.addrh = 0;
+	fadt->x_gpe0_blk.addrh = 0x0;
 }
diff --git a/src/soc/intel/broadwell/memmap.c b/src/soc/intel/broadwell/memmap.c
index 4673cceca0..fb9a834f6c 100644
--- a/src/soc/intel/broadwell/memmap.c
+++ b/src/soc/intel/broadwell/memmap.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
+/* Use simple device model for this file even in ramstage */
 #define __SIMPLE_DEVICE__
 
 #include <cbmem.h>
diff --git a/src/soc/intel/broadwell/minihd.c b/src/soc/intel/broadwell/minihd.c
index b91d73d216..a39c496ce5 100644
--- a/src/soc/intel/broadwell/minihd.c
+++ b/src/soc/intel/broadwell/minihd.c
@@ -12,30 +12,30 @@
 
 static const u32 minihd_verb_table[] = {
 	/* coreboot specific header */
-	0x80862808,	// Codec Vendor / Device ID: Intel Broadwell Mini-HD
-	0x80860101,	// Subsystem ID
-	0x00000004,	// Number of jacks
+	0x80862808,	/* Codec Vendor / Device ID: Intel Broadwell 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,
@@ -48,8 +48,8 @@ static const u32 minihd_verb_table[] = {
 static void minihd_init(struct device *dev)
 {
 	struct resource *res;
-	u8 *base;
 	u32 reg32;
+	u8 *base;
 	int codec_mask, i;
 
 	/* Find base address */
@@ -80,8 +80,7 @@ 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);
 		}
 	}
@@ -92,10 +91,10 @@ static void minihd_init(struct device *dev)
 }
 
 static struct device_operations minihd_ops = {
-	.read_resources		= &pci_dev_read_resources,
-	.set_resources		= &pci_dev_set_resources,
-	.enable_resources	= &pci_dev_enable_resources,
-	.init			= &minihd_init,
+	.read_resources		= pci_dev_read_resources,
+	.set_resources		= pci_dev_set_resources,
+	.enable_resources	= pci_dev_enable_resources,
+	.init			= minihd_init,
 	.ops_pci		= &broadwell_pci_ops,
 };
 
diff --git a/src/soc/intel/broadwell/pch.c b/src/soc/intel/broadwell/pch.c
index 479323dd56..2a27d92152 100644
--- a/src/soc/intel/broadwell/pch.c
+++ b/src/soc/intel/broadwell/pch.c
@@ -64,9 +64,7 @@ u32 pch_read_soft_strap(int id)
 /* Put device in D3Hot Power State */
 static void pch_enable_d3hot(struct device *dev)
 {
-	u32 reg32 = pci_read_config32(dev, PCH_PCS);
-	reg32 |= PCH_PCS_PS_D3HOT;
-	pci_write_config32(dev, PCH_PCS, reg32);
+	pci_or_config32(dev, PCH_PCS, PCH_PCS_PS_D3HOT);
 }
 
 /* RCBA function disable and posting read to flush the transaction */
diff --git a/src/soc/intel/broadwell/pcie.c b/src/soc/intel/broadwell/pcie.c
index 00a8595eeb..01ee06895f 100644
--- a/src/soc/intel/broadwell/pcie.c
+++ b/src/soc/intel/broadwell/pcie.c
@@ -18,7 +18,7 @@
 #include <delay.h>
 
 /* Low Power variant has 6 root ports. */
-#define NUM_ROOT_PORTS 6
+#define MAX_NUM_ROOT_PORTS 6
 
 struct root_port_config {
 	/* RPFN is a write-once register so keep a copy until it is written */
@@ -34,7 +34,7 @@ struct root_port_config {
 	int coalesce;
 	int gbe_port;
 	int num_ports;
-	struct device *ports[NUM_ROOT_PORTS];
+	struct device *ports[MAX_NUM_ROOT_PORTS];
 };
 
 static struct root_port_config rpc;
@@ -110,7 +110,7 @@ static void root_port_init_config(struct device *dev)
 	if (root_port_is_first(dev)) {
 		rpc.orig_rpfn = RCBA32(RPFN);
 		rpc.new_rpfn = rpc.orig_rpfn;
-		rpc.num_ports = NUM_ROOT_PORTS;
+		rpc.num_ports = MAX_NUM_ROOT_PORTS;
 		rpc.gbe_port = -1;
 		/* RP0 f5[3:0] = 0101b*/
 		pci_update_config8(dev, 0xf5, ~0xa, 0x5);
@@ -473,8 +473,7 @@ static void pch_pcie_early(struct device *dev)
 
 	if (do_aspm) {
 		/* Set ASPM bits in MPC2 register. */
-		pci_update_config32(dev, 0xd4, ~(0x3 << 2),
-			(1 << 4) | (0x2 << 2));
+		pci_update_config32(dev, 0xd4, ~(0x3 << 2), (1 << 4) | (0x2 << 2));
 
 		/* Set unique clock exit latency in MPC register. */
 		pci_update_config32(dev, 0xd8, ~(0x7 << 18), (0x7 << 18));
@@ -526,7 +525,7 @@ static void pch_pcie_early(struct device *dev)
 	else
 		pci_update_config32(dev, 0x4c, ~(0x7 << 15), (0x2 << 15));
 
-	pci_update_config32(dev, 0x314, 0x0, 0x743a361b);
+	pci_update_config32(dev, 0x314, 0, 0x743a361b);
 
 	/* Set Common Clock Exit Latency in MPC register. */
 	pci_update_config32(dev, 0xd8, ~(0x7 << 15), (0x3 << 15));
@@ -572,8 +571,6 @@ static void pch_pcie_early(struct device *dev)
 
 static void pch_pcie_init(struct device *dev)
 {
-	u16 reg16;
-
 	printk(BIOS_DEBUG, "Initializing PCH PCIe bridge.\n");
 
 	/* Enable SERR */
@@ -585,15 +582,11 @@ static void pch_pcie_init(struct device *dev)
 	/* Set Cache Line Size to 0x10 */
 	pci_write_config8(dev, 0x0c, 0x10);
 
-	reg16 = pci_read_config16(dev, PCI_BRIDGE_CONTROL);
-	reg16 &= ~PCI_BRIDGE_CTL_PARITY;
-	pci_write_config16(dev, PCI_BRIDGE_CONTROL, reg16);
+	pci_and_config16(dev, PCI_BRIDGE_CONTROL, ~PCI_BRIDGE_CTL_PARITY);
 
 	/* Clear errors in status registers */
-	reg16 = pci_read_config16(dev, 0x06);
-	pci_write_config16(dev, 0x06, reg16);
-	reg16 = pci_read_config16(dev, 0x1e);
-	pci_write_config16(dev, 0x1e, reg16);
+	pci_update_config16(dev, 0x06, ~0, 0);
+	pci_update_config16(dev, 0x1e, ~0, 0);
 }
 
 static void pch_pcie_enable(struct device *dev)
diff --git a/src/soc/intel/broadwell/pmutil.c b/src/soc/intel/broadwell/pmutil.c
index 92cf3637f9..c96ed18b8a 100644
--- a/src/soc/intel/broadwell/pmutil.c
+++ b/src/soc/intel/broadwell/pmutil.c
@@ -312,7 +312,7 @@ void enable_tco_sci(void)
  */
 
 /* Clear a GPE0 status and return events that are enabled and active */
-static u32 reset_gpe(u16 sts_reg, u16 en_reg)
+static u32 reset_gpe_status(u16 sts_reg, u16 en_reg)
 {
 	u32 gpe0_sts = inl(ACPI_BASE_ADDRESS + sts_reg);
 	u32 gpe0_en = inl(ACPI_BASE_ADDRESS + en_reg);
@@ -366,10 +366,10 @@ u32 clear_gpe_status(void)
 		[18] = "WADT"
 	};
 
-	print_gpe_gpio(reset_gpe(GPE0_STS(GPE_31_0), GPE0_EN(GPE_31_0)), 0);
-	print_gpe_gpio(reset_gpe(GPE0_STS(GPE_63_32), GPE0_EN(GPE_63_32)), 32);
-	print_gpe_gpio(reset_gpe(GPE0_STS(GPE_94_64), GPE0_EN(GPE_94_64)), 64);
-	return print_gpe_status(reset_gpe(GPE0_STS(GPE_STD), GPE0_EN(GPE_STD)),
+	print_gpe_gpio(reset_gpe_status(GPE0_STS(GPE_31_0), GPE0_EN(GPE_31_0)), 0);
+	print_gpe_gpio(reset_gpe_status(GPE0_STS(GPE_63_32), GPE0_EN(GPE_63_32)), 32);
+	print_gpe_gpio(reset_gpe_status(GPE0_STS(GPE_94_64), GPE0_EN(GPE_94_64)), 64);
+	return print_gpe_status(reset_gpe_status(GPE0_STS(GPE_STD), GPE0_EN(GPE_STD)),
 				gpe0_sts_3_bits);
 }
 
diff --git a/src/soc/intel/broadwell/smbus.c b/src/soc/intel/broadwell/smbus.c
index 562db4e4cc..31715c7182 100644
--- a/src/soc/intel/broadwell/smbus.c
+++ b/src/soc/intel/broadwell/smbus.c
@@ -18,6 +18,7 @@ static void pch_smbus_init(struct device *dev)
 	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/soc/intel/broadwell/smihandler.c b/src/soc/intel/broadwell/smihandler.c
index 1732ef7803..4a12d786dd 100644
--- a/src/soc/intel/broadwell/smihandler.c
+++ b/src/soc/intel/broadwell/smihandler.c
@@ -58,25 +58,22 @@ static void busmaster_disable_on_bus(int bus)
 
 	for (slot = 0; slot < 0x20; slot++) {
 		for (func = 0; func < 8; func++) {
-			u16 reg16;
-
 			pci_devfn_t dev = PCI_DEV(bus, slot, func);
+
 			val = pci_read_config32(dev, PCI_VENDOR_ID);
 
 			if (val == 0xffffffff || val == 0x00000000 ||
-				val == 0x0000ffff || val == 0xffff0000)
+			    val == 0x0000ffff || val == 0xffff0000)
 				continue;
 
 			/* Disable Bus Mastering for this one device */
-			reg16 = pci_read_config16(dev, PCI_COMMAND);
-			reg16 &= ~PCI_COMMAND_MASTER;
-			pci_write_config16(dev, PCI_COMMAND, reg16);
+			pci_and_config16(dev, PCI_COMMAND, ~PCI_COMMAND_MASTER);
 
 			/* If this is a bridge, then follow it. */
 			hdr = pci_read_config8(dev, PCI_HEADER_TYPE);
 			hdr &= 0x7f;
 			if (hdr == PCI_HEADER_TYPE_BRIDGE ||
-				hdr == PCI_HEADER_TYPE_CARDBUS) {
+			    hdr == PCI_HEADER_TYPE_CARDBUS) {
 				unsigned int buses;
 				buses = pci_read_config32(dev, PCI_PRIMARY_BUS);
 				busmaster_disable_on_bus((buses >> 8) & 0xff);
@@ -249,11 +246,11 @@ static em64t101_smm_state_save_area_t *smi_apmc_find_state_save(u8 cmd)
 	for (node = 0; node < CONFIG_MAX_CPUS; node++) {
 		state = smm_get_save_state(node);
 
-		/* Check for Synchronous IO (bit0==1) */
+		/* Check for Synchronous IO (bit0 == 1) */
 		if (!(state->io_misc_info & (1 << 0)))
 			continue;
 
-		/* Make sure it was a write (bit4==0) */
+		/* Make sure it was a write (bit4 == 0) */
 		if (state->io_misc_info & (1 << 4))
 			continue;
 
@@ -429,8 +426,7 @@ static void southbridge_smi_tco(void)
 			 * box.
 			 */
 			printk(BIOS_DEBUG, "Switching back to RO\n");
-			pci_write_config32(PCH_DEV_LPC, BIOS_CNTL,
-					   (bios_cntl & ~1));
+			pci_write_config32(PCH_DEV_LPC, BIOS_CNTL,  (bios_cntl & ~1));
 		} /* No else for now? */
 	} else if (tco_sts & (1 << 3)) { /* TIMEOUT */
 		/* Handle TCO timeout */
@@ -453,7 +449,7 @@ static void southbridge_smi_monitor(void)
 {
 #define IOTRAP(x) (trap_sts & (1 << x))
 	u32 trap_sts, trap_cycle;
-	u32 data, mask = 0;
+	u32 mask = 0;
 	int i;
 
 	trap_sts = RCBA32(0x1e00); // TRSR - Trap Status Register
@@ -480,8 +476,9 @@ static void southbridge_smi_monitor(void)
 		// It's a write
 		if (!(trap_cycle & (1 << 24))) {
 			printk(BIOS_DEBUG, "SMI1 command\n");
-			data = RCBA32(0x1e18);
-			data &= mask;
+			(void)RCBA32(0x1e18);
+			// data = RCBA32(0x1e18);
+			// data &= mask;
 			// if (smi1)
 			//	southbridge_smi_command(data);
 			// return;
@@ -501,8 +498,7 @@ static void southbridge_smi_monitor(void)
 
 	if (!(trap_cycle & (1 << 24))) {
 		/* Write Cycle */
-		data = RCBA32(0x1e18);
-		printk(BIOS_DEBUG, "  iotrap written data = 0x%08x\n", data);
+		printk(BIOS_DEBUG, "  iotrap written data = 0x%08x\n", RCBA32(0x1e18));
 	}
 #undef IOTRAP
 }
@@ -546,10 +542,7 @@ static smi_handler_t southbridge_smi[32] = {
 
 /**
  * @brief Interrupt handler for SMI#
- *
- * @param smm_revision revision of the smm state save map
  */
-
 void southbridge_smi_handler(void)
 {
 	int i;