diff mbox

[U-Boot,39/69] x86: Tidy up mp_init to reduce duplication

Message ID 1457317732-18406-40-git-send-email-sjg@chromium.org
State Accepted
Commit 2254e34c3fe5b2545dcad06774a426b2f881a090
Delegated to: Bin Meng
Headers show

Commit Message

Simon Glass March 7, 2016, 2:28 a.m. UTC 
The timeout step is always 50us. By updating apic_wait_timeout() to print
the debug messages we can simplify the code. Also tidy up a few messages and
comments while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/x86/cpu/mp_init.c | 79 +++++++++++++++++---------------------------------
 1 file changed, 26 insertions(+), 53 deletions(-)

Comments

Bin Meng March 11, 2016, 6:43 a.m. UTC | #1 
On Mon, Mar 7, 2016 at 10:28 AM, Simon Glass <sjg@chromium.org> wrote:
> The timeout step is always 50us. By updating apic_wait_timeout() to print
> the debug messages we can simplify the code. Also tidy up a few messages and
> comments while we are here.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  arch/x86/cpu/mp_init.c | 79 +++++++++++++++++---------------------------------
>  1 file changed, 26 insertions(+), 53 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Bin Meng March 11, 2016, 6:49 a.m. UTC | #2 
On Fri, Mar 11, 2016 at 2:43 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Mon, Mar 7, 2016 at 10:28 AM, Simon Glass <sjg@chromium.org> wrote:
>> The timeout step is always 50us. By updating apic_wait_timeout() to print
>> the debug messages we can simplify the code. Also tidy up a few messages and
>> comments while we are here.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  arch/x86/cpu/mp_init.c | 79 +++++++++++++++++---------------------------------
>>  1 file changed, 26 insertions(+), 53 deletions(-)
>>
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86/next, thanks!
diff mbox

Patch

diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
index fc2fb5b..ca47e9e 100644
--- a/arch/x86/cpu/mp_init.c
+++ b/arch/x86/cpu/mp_init.c
@@ -195,7 +195,7 @@  static int save_bsp_msrs(char *start, int size)
 	msr_count = 2 * num_var_mtrrs + NUM_FIXED_MTRRS + 1;
 
 	if ((msr_count * sizeof(struct saved_msr)) > size) {
-		printf("Cannot mirror all %d msrs.\n", msr_count);
+		printf("Cannot mirror all %d msrs\n", msr_count);
 		return -ENOSPC;
 	}
 
@@ -283,21 +283,25 @@  static int check_cpu_devices(int expected_cpus)
 }
 
 /* Returns 1 for timeout. 0 on success */
-static int apic_wait_timeout(int total_delay, int delay_step)
+static int apic_wait_timeout(int total_delay, const char *msg)
 {
 	int total = 0;
-	int timeout = 0;
 
+	if (!(lapic_read(LAPIC_ICR) & LAPIC_ICR_BUSY))
+		return 0;
+
+	debug("Waiting for %s...", msg);
 	while (lapic_read(LAPIC_ICR) & LAPIC_ICR_BUSY) {
-		udelay(delay_step);
-		total += delay_step;
+		udelay(50);
+		total += 50;
 		if (total >= total_delay) {
-			timeout = 1;
-			break;
+			debug("timed out: aborting\n");
+			return -ETIMEDOUT;
 		}
 	}
+	debug("done\n");
 
-	return timeout;
+	return 0;
 }
 
 static int start_aps(int ap_count, atomic_t *num_aps)
@@ -320,73 +324,42 @@  static int start_aps(int ap_count, atomic_t *num_aps)
 
 	debug("Attempting to start %d APs\n", ap_count);
 
-	if ((lapic_read(LAPIC_ICR) & LAPIC_ICR_BUSY)) {
-		debug("Waiting for ICR not to be busy...");
-		if (apic_wait_timeout(1000, 50)) {
-			debug("timed out. Aborting.\n");
-			return -1;
-		} else {
-			debug("done.\n");
-		}
-	}
+	if (apic_wait_timeout(1000, "ICR not to be busy"))
+		return -ETIMEDOUT;
 
 	/* Send INIT IPI to all but self */
 	lapic_write(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(0));
 	lapic_write(LAPIC_ICR, LAPIC_DEST_ALLBUT | LAPIC_INT_ASSERT |
 		    LAPIC_DM_INIT);
-	debug("Waiting for 10ms after sending INIT.\n");
+	debug("Waiting for 10ms after sending INIT\n");
 	mdelay(10);
 
 	/* Send 1st SIPI */
-	if ((lapic_read(LAPIC_ICR) & LAPIC_ICR_BUSY)) {
-		debug("Waiting for ICR not to be busy...");
-		if (apic_wait_timeout(1000, 50)) {
-			debug("timed out. Aborting.\n");
-			return -1;
-		} else {
-			debug("done.\n");
-		}
-	}
+	if (apic_wait_timeout(1000, "ICR not to be busy"))
+		return -ETIMEDOUT;
 
 	lapic_write(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(0));
 	lapic_write(LAPIC_ICR, LAPIC_DEST_ALLBUT | LAPIC_INT_ASSERT |
 		    LAPIC_DM_STARTUP | sipi_vector);
-	debug("Waiting for 1st SIPI to complete...");
-	if (apic_wait_timeout(10000, 50)) {
-		debug("timed out.\n");
-		return -1;
-	} else {
-		debug("done.\n");
-	}
+	if (apic_wait_timeout(10000, "first SIPI to complete"))
+		return -ETIMEDOUT;
 
 	/* Wait for CPUs to check in up to 200 us */
 	wait_for_aps(num_aps, ap_count, 200, 15);
 
 	/* Send 2nd SIPI */
-	if ((lapic_read(LAPIC_ICR) & LAPIC_ICR_BUSY)) {
-		debug("Waiting for ICR not to be busy...");
-		if (apic_wait_timeout(1000, 50)) {
-			debug("timed out. Aborting.\n");
-			return -1;
-		} else {
-			debug("done.\n");
-		}
-	}
+	if (apic_wait_timeout(1000, "ICR not to be busy"))
+		return -ETIMEDOUT;
 
 	lapic_write(LAPIC_ICR2, SET_LAPIC_DEST_FIELD(0));
 	lapic_write(LAPIC_ICR, LAPIC_DEST_ALLBUT | LAPIC_INT_ASSERT |
 		    LAPIC_DM_STARTUP | sipi_vector);
-	debug("Waiting for 2nd SIPI to complete...");
-	if (apic_wait_timeout(10000, 50)) {
-		debug("timed out.\n");
-		return -1;
-	} else {
-		debug("done.\n");
-	}
+	if (apic_wait_timeout(10000, "second SIPI to complete"))
+		return -ETIMEDOUT;
 
 	/* Wait for CPUs to check in */
 	if (wait_for_aps(num_aps, ap_count, 10000, 50)) {
-		debug("Not all APs checked in: %d/%d.\n",
+		debug("Not all APs checked in: %d/%d\n",
 		      atomic_read(num_aps), ap_count);
 		return -1;
 	}
@@ -410,7 +383,7 @@  static int bsp_do_flight_plan(struct udevice *cpu, struct mp_params *mp_params)
 			/* Wait for the APs to check in */
 			if (wait_for_aps(&rec->cpus_entered, num_aps,
 					 timeout_us, step_us)) {
-				debug("MP record %d timeout.\n", i);
+				debug("MP record %d timeout\n", i);
 				ret = -1;
 			}
 		}
@@ -430,7 +403,7 @@  static int init_bsp(struct udevice **devp)
 	int ret;
 
 	cpu_get_name(processor_name);
-	debug("CPU: %s.\n", processor_name);
+	debug("CPU: %s\n", processor_name);
 
 	lapic_setup();