arch/x86: Fix most of remaining issues detected by checkpatch

Fix the following errors and warnings detected by checkpatch.pl:

ERROR: do not use assignment in if condition
ERROR: trailing statements should be on next line
ERROR: Macros with complex values should be enclosed in parentheses
ERROR: switch and case should be at the same indent
WARNING: char * array declaration might be better as static const
WARNING: else is not generally useful after a break or return
WARNING: storage class should be at the beginning of the declaration
WARNING: void function return statements are not generally useful
WARNING: break is not useful after a goto or return
WARNING: Single statement macros should not use a do {} while (0) loop
WARNING: sizeof *t should be sizeof(*t)
WARNING: Comparisons should place the constant on the right side of the test

TEST=Build and run on Galileo Gen2

Change-Id: I39d49790c5eaeedec5051e1fab0b1279275f6e7f
Signed-off-by: Lee Leahy <Leroy.P.Leahy@intel.com>
Reviewed-on: https://review.coreboot.org/18865
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martinroth@google.com>

18 files changed, 112 insertions, 108 deletions

diff --git a/src/arch/x86/acpi.c b/src/arch/x86/acpi.c
index 5227bd2d35..f8c5b52a3d 100644
--- a/src/arch/x86/acpi.c
+++ b/src/arch/x86/acpi.c
@@ -1075,7 +1075,8 @@ void *acpi_find_wakeup_vector(void)
 
 	/* Find RSDP. */
 	for (p = (char *)0xe0000; p < (char *)0xfffff; p += 16) {
-		if ((rsdp = valid_rsdp((acpi_rsdp_t *)p)))
+		rsdp = valid_rsdp((acpi_rsdp_t *)p);
+		if (rsdp)
 			break;
 	}
 
diff --git a/src/arch/x86/acpi_device.c b/src/arch/x86/acpi_device.c
index 8ea6f5e8aa..f4f1c4c3b5 100644
--- a/src/arch/x86/acpi_device.c
+++ b/src/arch/x86/acpi_device.c
@@ -495,7 +495,7 @@ void acpi_device_add_power_res(
 	struct acpi_gpio *reset, unsigned int reset_delay_ms,
 	struct acpi_gpio *enable, unsigned int enable_delay_ms)
 {
-	const char *power_res_dev_states[] = { "_PR0", "_PR3" };
+	static const char *power_res_dev_states[] = { "_PR0", "_PR3" };
 	unsigned int reset_gpio = reset->pins[0];
 	unsigned int enable_gpio = enable->pins[0];
 
diff --git a/src/arch/x86/acpi_s3.c b/src/arch/x86/acpi_s3.c
index 8549f63d65..61955f5b8e 100644
--- a/src/arch/x86/acpi_s3.c
+++ b/src/arch/x86/acpi_s3.c
@@ -36,10 +36,9 @@ int acpi_get_sleep_type(void)
 	if (romstage_handoff_is_resume()) {
 		printk(BIOS_DEBUG, "S3 Resume.\n");
 		return ACPI_S3;
-	} else {
-		printk(BIOS_DEBUG, "Normal boot.\n");
-		return ACPI_S0;
 	}
+	printk(BIOS_DEBUG, "Normal boot.\n");
+	return ACPI_S0;
 }
 #endif
 
@@ -202,8 +201,8 @@ void acpi_prepare_resume_backup(void)
 
 #define WAKEUP_BASE 0x600
 
-void (*acpi_do_wakeup)(uintptr_t vector, u32 backup_source, u32 backup_target,
-	u32 backup_size) asmlinkage = (void *)WAKEUP_BASE;
+asmlinkage void (*acpi_do_wakeup)(uintptr_t vector, u32 backup_source,
+	u32 backup_target, u32 backup_size) = (void *)WAKEUP_BASE;
 
 extern unsigned char __wakeup;
 extern unsigned int __wakeup_size;
diff --git a/src/arch/x86/acpigen.c b/src/arch/x86/acpigen.c
index cbd0b5f131..81a69ce8cb 100644
--- a/src/arch/x86/acpigen.c
+++ b/src/arch/x86/acpigen.c
@@ -246,9 +246,8 @@ static void acpigen_emit_simple_namestring(const char *name)
 		if ((name[i] == '\0') || (name[i] == '.')) {
 			acpigen_emit_stream(ud, 4 - i);
 			break;
-		} else {
-			acpigen_emit_byte(name[i]);
 		}
+		acpigen_emit_byte(name[i]);
 	}
 }
 
diff --git a/src/arch/x86/bootblock_normal.c b/src/arch/x86/bootblock_normal.c
index 43d26c9a25..caffb9117b 100644
--- a/src/arch/x86/bootblock_normal.c
+++ b/src/arch/x86/bootblock_normal.c
@@ -18,7 +18,8 @@
 
 static const char *get_fallback(const char *stagelist)
 {
-	while (*stagelist) stagelist++;
+	while (*stagelist)
+		stagelist++;
 	return ++stagelist;
 }
 
diff --git a/src/arch/x86/bootblock_simple.c b/src/arch/x86/bootblock_simple.c
index 6b41718417..8e44add28e 100644
--- a/src/arch/x86/bootblock_simple.c
+++ b/src/arch/x86/bootblock_simple.c
@@ -36,6 +36,7 @@ static void main(unsigned long bist)
 
 	unsigned long entry;
 	entry = findstage(target1);
-	if (entry) call(entry, bist);
+	if (entry)
+		call(entry, bist);
 	halt();
 }
diff --git a/src/arch/x86/cbfs_and_run.c b/src/arch/x86/cbfs_and_run.c
index 22b5376139..f25052b1ed 100644
--- a/src/arch/x86/cbfs_and_run.c
+++ b/src/arch/x86/cbfs_and_run.c
@@ -16,7 +16,7 @@
 #include <arch/stages.h>
 #include <program_loading.h>
 
-void asmlinkage copy_and_run(void)
+asmlinkage void copy_and_run(void)
 {
 	run_ramstage();
 }
diff --git a/src/arch/x86/cpu.c b/src/arch/x86/cpu.c
index 4b8275939c..ff82661447 100644
--- a/src/arch/x86/cpu.c
+++ b/src/arch/x86/cpu.c
@@ -118,7 +118,7 @@ static struct {
 	{ X86_VENDOR_SIS,       "SiS SiS SiS ", },
 };
 
-static const char *x86_vendor_name[] = {
+static const char * const x86_vendor_name[] = {
 	[X86_VENDOR_INTEL]     = "Intel",
 	[X86_VENDOR_CYRIX]     = "Cyrix",
 	[X86_VENDOR_AMD]       = "AMD",
@@ -209,7 +209,7 @@ struct cpu_driver *find_cpu_driver(struct device *cpu)
 			if ((cpu->vendor == id->vendor) &&
 				(cpu->device == id->device))
 				return driver;
-			if (X86_VENDOR_ANY == id->vendor)
+			if (id->vendor == X86_VENDOR_ANY)
 				return driver;
 		}
 	}
@@ -264,7 +264,8 @@ void cpu_initialize(unsigned int index)
 		cpu->device -= c.x86_mask;
 		set_cpu_ops(cpu);
 		cpu->device += c.x86_mask;
-		if (!cpu->ops) die("Unknown cpu");
+		if (!cpu->ops)
+			die("Unknown cpu");
 		printk(BIOS_DEBUG, "Using generic CPU ops (good)\n");
 	}
 
@@ -278,8 +279,6 @@ void cpu_initialize(unsigned int index)
 	post_log_clear();
 
 	printk(BIOS_INFO, "CPU #%d initialized\n", index);
-
-	return;
 }
 
 void lb_arch_add_records(struct lb_header *header)
diff --git a/src/arch/x86/exception.c b/src/arch/x86/exception.c
index b8b0f3f023..9121738090 100644
--- a/src/arch/x86/exception.c
+++ b/src/arch/x86/exception.c
@@ -286,7 +286,6 @@ static void copy_to_hex(char *buf, void *addr, unsigned long count)
 		*buf++ = hexchars[ch & 0x0f];
 	}
 	*buf = 0;
-	return;
 }
 
 
@@ -319,7 +318,8 @@ static int get_packet(char *buffer)
 		/* wait around for the start character, ignore all other
 		 * characters
 		 */
-		while ((ch = (stub_getc() & 0x7f)) != '$');
+		while ((ch = (stub_getc() & 0x7f)) != '$')
+			;
 		checksum = 0;
 		xmitcsum = -1;
 
@@ -470,7 +470,6 @@ void x86_exception(struct eregs *info)
 			if (in_buffer[0] == 's')
 				info->eflags |= (1 << 8);
 			return;
-			break;
 		case 'D':
 			memcpy(out_buffer, "OK", 3);
 			break;
diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h
index e59eb32678..06e4435699 100644
--- a/src/arch/x86/include/arch/cpu.h
+++ b/src/arch/x86/include/arch/cpu.h
@@ -249,7 +249,7 @@ static inline void get_fms(struct cpuinfo_x86 *c, uint32_t tfms)
  * is the symbol jumped to for each stage after bootblock using
  * cache-as-ram.
  */
-void asmlinkage car_stage_entry(void);
+asmlinkage void car_stage_entry(void);
 
 /*
  * Support setting up a stack frame consisting of MTRR information
diff --git a/src/arch/x86/include/arch/early_variables.h b/src/arch/x86/include/arch/early_variables.h
index b48848e4a3..a21781996e 100644
--- a/src/arch/x86/include/arch/early_variables.h
+++ b/src/arch/x86/include/arch/early_variables.h
@@ -54,11 +54,10 @@ void *car_sync_var_ptr(void *var);
 
 /* Get and set a primitive type global variable. */
 #define car_get_var(var) \
-	*(typeof(var) *)car_get_var_ptr(&(var))
+	(*(typeof(var) *)car_get_var_ptr(&(var)))
 #define car_sync_var(var) \
-	*(typeof(var) *)car_sync_var_ptr(&(var))
-#define car_set_var(var, val) \
-	do { car_get_var(var) = (val); } while (0)
+	(*(typeof(var) *)car_sync_var_ptr(&(var)))
+#define car_set_var(var, val)	car_get_var(var) = (val)
 
 static inline size_t car_data_size(void)
 {
@@ -76,7 +75,7 @@ static inline size_t car_object_offset(void *ptr)
 static inline void *car_get_var_ptr(void *var) { return var; }
 #define car_get_var(var) (var)
 #define car_sync_var(var) (var)
-#define car_set_var(var, val) do { (var) = (val); } while (0)
+#define car_set_var(var, val)	(var) = (val)
 #endif
 
 #endif
diff --git a/src/arch/x86/include/arch/ebda.h b/src/arch/x86/include/arch/ebda.h
index 116603437c..cd25ccaba1 100644
--- a/src/arch/x86/include/arch/ebda.h
+++ b/src/arch/x86/include/arch/ebda.h
@@ -18,9 +18,9 @@
 #define __ARCH_EBDA_H
 
 #define X86_BDA_SIZE		0x200
-#define X86_BDA_BASE		(void *)0x400
-#define X86_EBDA_SEGMENT	(void *)0x40e
-#define X86_EBDA_LOWMEM		(void *)0x413
+#define X86_BDA_BASE		((void *)0x400)
+#define X86_EBDA_SEGMENT	((void *)0x40e)
+#define X86_EBDA_LOWMEM		((void *)0x413)
 
 #define DEFAULT_EBDA_LOWMEM	(1024 << 10)
 #define DEFAULT_EBDA_SEGMENT	0xF600
diff --git a/src/arch/x86/include/arch/smp/spinlock.h b/src/arch/x86/include/arch/smp/spinlock.h
index 716de15d15..1c1def1252 100644
--- a/src/arch/x86/include/arch/smp/spinlock.h
+++ b/src/arch/x86/include/arch/smp/spinlock.h
@@ -39,7 +39,8 @@ void initialize_romstage_microcode_cbfs_lock(void);
 #define SPIN_LOCK_UNLOCKED { 1 }
 
 #ifndef __PRE_RAM__
-#define DECLARE_SPIN_LOCK(x) static spinlock_t x = SPIN_LOCK_UNLOCKED;
+#define DECLARE_SPIN_LOCK(x)	\
+	static spinlock_t x = SPIN_LOCK_UNLOCKED;
 #else
 #define DECLARE_SPIN_LOCK(x)
 #endif
diff --git a/src/arch/x86/include/arch/stages.h b/src/arch/x86/include/arch/stages.h
index 49921ae0e1..5716030274 100644
--- a/src/arch/x86/include/arch/stages.h
+++ b/src/arch/x86/include/arch/stages.h
@@ -18,6 +18,6 @@
 
 #include <arch/cpu.h>
 
-void asmlinkage copy_and_run(void);
+asmlinkage void copy_and_run(void);
 
 #endif
diff --git a/src/arch/x86/mmap_boot.c b/src/arch/x86/mmap_boot.c
index b03a17e5da..168b17d71b 100644
--- a/src/arch/x86/mmap_boot.c
+++ b/src/arch/x86/mmap_boot.c
@@ -54,7 +54,7 @@ static int cbfs_master_header_props(struct cbfs_props *props)
 		return -1;
 
 	props->offset = header.offset;
-	if (CONFIG_ROM_SIZE != header.romsize)
+	if (header.romsize != CONFIG_ROM_SIZE)
 		props->size = CONFIG_ROM_SIZE;
 	else
 		props->size = header.romsize;
diff --git a/src/arch/x86/mpspec.c b/src/arch/x86/mpspec.c
index ab0514b347..d41abaff5a 100644
--- a/src/arch/x86/mpspec.c
+++ b/src/arch/x86/mpspec.c
@@ -481,8 +481,10 @@ void mptable_write_buses(struct mp_config_table *mc, int *max_pci_bus,
 	char buses[256];
 	struct device *dev;
 
-	if (!max_pci_bus) max_pci_bus = &dummy;
-	if (!isa_bus) isa_bus = &dummy;
+	if (!max_pci_bus)
+		max_pci_bus = &dummy;
+	if (!isa_bus)
+		isa_bus = &dummy;
 
 	*max_pci_bus = 0;
 	highest = 0;
@@ -497,7 +499,8 @@ void mptable_write_buses(struct mp_config_table *mc, int *max_pci_bus,
 				return;
 			}
 			buses[bus->secondary] = 1;
-			if (highest < bus->secondary) highest = bus->secondary;
+			if (highest < bus->secondary)
+				highest = bus->secondary;
 		}
 	}
 	for (i = 0; i <= highest; i++) {
@@ -545,7 +548,8 @@ unsigned long __attribute__((weak)) write_smp_table(unsigned long addr)
 		if (dev->path.type != DEVICE_PATH_IOAPIC)
 			continue;
 
-		if (!(ioapic_config = dev->chip_info)) {
+		ioapic_config = dev->chip_info;
+		if (!ioapic_config) {
 			printk(BIOS_ERR, "%s has no config, ignoring\n",
 				dev_path(dev));
 			continue;
diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c
index 62443274dc..0909900ca7 100644
--- a/src/arch/x86/smbios.c
+++ b/src/arch/x86/smbios.c
@@ -129,60 +129,61 @@ void smbios_fill_dimm_manufacturer_from_id(uint16_t mod_id,
 	struct smbios_type17 *t)
 {
 	switch (mod_id) {
-		case 0x2c80:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "Crucial");
-			break;
-		case 0x4304:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "Ramaxel");
-			break;
-		case 0x4f01:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "Transcend");
-			break;
-		case 0x9801:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "Kingston");
-			break;
-		case 0x987f:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "Hynix");
-			break;
-		case 0x9e02:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "Corsair");
-			break;
-		case 0xb004:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "OCZ");
-			break;
-		case 0xad80:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "Hynix/Hyundai");
-			break;
-		case 0xb502:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "SuperTalent");
-			break;
-		case 0xcd04:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "GSkill");
-			break;
-		case 0xce80:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "Samsung");
-			break;
-		case 0xfe02:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "Elpida");
-			break;
-		case 0xff2c:
-			t->manufacturer = smbios_add_string(t->eos,
-							    "Micron");
-			break;
-		default: {
+	case 0x2c80:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "Crucial");
+		break;
+	case 0x4304:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "Ramaxel");
+		break;
+	case 0x4f01:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "Transcend");
+		break;
+	case 0x9801:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "Kingston");
+		break;
+	case 0x987f:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "Hynix");
+		break;
+	case 0x9e02:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "Corsair");
+		break;
+	case 0xb004:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "OCZ");
+		break;
+	case 0xad80:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "Hynix/Hyundai");
+		break;
+	case 0xb502:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "SuperTalent");
+		break;
+	case 0xcd04:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "GSkill");
+		break;
+	case 0xce80:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "Samsung");
+		break;
+	case 0xfe02:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "Elpida");
+		break;
+	case 0xff2c:
+		t->manufacturer = smbios_add_string(t->eos,
+						    "Micron");
+		break;
+	default: {
 			char string_buffer[256];
+
 			snprintf(string_buffer, sizeof(string_buffer),
 						"Unknown (%x)", mod_id);
 			t->manufacturer = smbios_add_string(t->eos,
@@ -209,21 +210,21 @@ static int create_smbios_type17_for_dimm(struct dimm_info *dimm,
 	t->total_width = t->data_width + 8 * ((dimm->bus_width & 0x18) >> 3);
 
 	switch (dimm->mod_type) {
-		case SPD_RDIMM:
-		case SPD_MINI_RDIMM:
-			t->form_factor = MEMORY_FORMFACTOR_RIMM;
-			break;
-		case SPD_UDIMM:
-		case SPD_MICRO_DIMM:
-		case SPD_MINI_UDIMM:
-			t->form_factor = MEMORY_FORMFACTOR_DIMM;
-			break;
-		case SPD_SODIMM:
-			t->form_factor = MEMORY_FORMFACTOR_SODIMM;
-			break;
-		default:
-			t->form_factor = MEMORY_FORMFACTOR_UNKNOWN;
-			break;
+	case SPD_RDIMM:
+	case SPD_MINI_RDIMM:
+		t->form_factor = MEMORY_FORMFACTOR_RIMM;
+		break;
+	case SPD_UDIMM:
+	case SPD_MICRO_DIMM:
+	case SPD_MINI_UDIMM:
+		t->form_factor = MEMORY_FORMFACTOR_DIMM;
+		break;
+	case SPD_SODIMM:
+		t->form_factor = MEMORY_FORMFACTOR_SODIMM;
+		break;
+	default:
+		t->form_factor = MEMORY_FORMFACTOR_UNKNOWN;
+		break;
 	}
 
 	smbios_fill_dimm_manufacturer_from_id(dimm->mod_id, t);
@@ -476,10 +477,10 @@ static int smbios_write_type11(unsigned long *current, int *handle)
 	int len;
 	struct device *dev;
 
-	memset(t, 0, sizeof *t);
+	memset(t, 0, sizeof(*t));
 	t->type = SMBIOS_OEM_STRINGS;
 	t->handle = *handle;
-	t->length = len = sizeof *t - 2;
+	t->length = len = sizeof(*t) - 2;
 
 	for (dev = all_devices; dev; dev = dev->next) {
 		if (dev->ops && dev->ops->get_smbios_strings)
@@ -487,7 +488,7 @@ static int smbios_write_type11(unsigned long *current, int *handle)
 	}
 
 	if (t->count == 0) {
-		memset(t, 0, sizeof *t);
+		memset(t, 0, sizeof(*t));
 		return 0;
 	}
 
diff --git a/src/arch/x86/verstage.c b/src/arch/x86/verstage.c
index a44bf0fa01..7987e1ca6e 100644
--- a/src/arch/x86/verstage.c
+++ b/src/arch/x86/verstage.c
@@ -17,7 +17,7 @@
 #include <vendorcode/google/chromeos/chromeos.h>
 
 /* Provide an entry point for verstage when it's a separate stage. */
-void asmlinkage car_stage_entry(void)
+asmlinkage void car_stage_entry(void)
 {
 	verstage();
 }