Hi Bin,
On 11 June 2015 at 20:07, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Fri, Jun 12, 2015 at 7:39 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 9 June 2015 at 01:45, Bin Meng bmeng.cn@gmail.com wrote:
Most of the MP initialization codes in arch/x86/cpu/baytrail/cpu.c is common to all x86 processors, except detect_num_cpus() which varies from cpu to cpu. Move these to arch/x86/cpu/cpu.c and declare a weak detect_num_cpus() which just returns 2 which is minimally required.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/baytrail/cpu.c | 44 +----------------------------------------- arch/x86/cpu/cpu.c | 47 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 43 deletions(-)
diff --git a/arch/x86/cpu/baytrail/cpu.c b/arch/x86/cpu/baytrail/cpu.c index 05156a5..7805056 100644 --- a/arch/x86/cpu/baytrail/cpu.c +++ b/arch/x86/cpu/baytrail/cpu.c @@ -12,23 +12,11 @@ #include <asm/cpu.h> #include <asm/cpu_x86.h> #include <asm/lapic.h> -#include <asm/mp.h> #include <asm/msr.h> #include <asm/turbo.h>
#ifdef CONFIG_SMP -static int enable_smis(struct udevice *cpu, void *unused) -{
return 0;-}
-static struct mp_flight_record mp_steps[] = {
MP_FR_BLOCK_APS(mp_init_cpu, NULL, mp_init_cpu, NULL),/* Wait for APs to finish initialization before proceeding. */MP_FR_BLOCK_APS(NULL, NULL, enable_smis, NULL),-};
-static int detect_num_cpus(void) +int detect_num_cpus(void) { int ecx = 0;
@@ -52,38 +40,8 @@ static int detect_num_cpus(void) ecx++; } }
-static int baytrail_init_cpus(void) -{
struct mp_params mp_params;lapic_setup();mp_params.num_cpus = detect_num_cpus();mp_params.parallel_microcode_load = 0,mp_params.flight_plan = &mp_steps[0];mp_params.num_records = ARRAY_SIZE(mp_steps);mp_params.microcode_pointer = 0;if (mp_init(&mp_params)) {printf("Warning: MP init failure\n");return -EIO;}return 0;-} #endif
-int x86_init_cpus(void) -{ -#ifdef CONFIG_SMP
debug("Init additional CPUs\n");baytrail_init_cpus();-#endif
return 0;-}
static void set_max_freq(void) { msr_t perf_ctl; diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c index ffb6e43..ddc7dc3 100644 --- a/arch/x86/cpu/cpu.c +++ b/arch/x86/cpu/cpu.c @@ -21,10 +21,13 @@
#include <common.h> #include <command.h> +#include <dm.h> #include <errno.h> #include <malloc.h> #include <asm/control_regs.h> #include <asm/cpu.h> +#include <asm/lapic.h> +#include <asm/mp.h> #include <asm/post.h> #include <asm/processor.h> #include <asm/processor-flags.h> @@ -601,8 +604,52 @@ int last_stage_init(void) } #endif
+#ifdef CONFIG_SMP +static int enable_smis(struct udevice *cpu, void *unused) +{
return 0;+}
+static struct mp_flight_record mp_steps[] = {
MP_FR_BLOCK_APS(mp_init_cpu, NULL, mp_init_cpu, NULL),/* Wait for APs to finish initialization before proceeding */MP_FR_BLOCK_APS(NULL, NULL, enable_smis, NULL),+};
+__weak int detect_num_cpus(void)
Does this need to be weak? We could perhaps require that the function exists?
Yes, since I don't see there is a common way to detect number of cpu cores. Or maybe I am not aware of one. If we want to remove the weak, maybe we can just switch to count the number of cpu nodes in /cpus from device tree, which should be generic for all cases. I believe we discussed this during the review when you added MP stuff.
I worry what might happen if the device tree says 4 but there are only 2 CPUs. There should be a way to detect the number of CPUs gracefully in each arch.
Can you add get_cpu_count() as a method to the UCLASS_CPU ops perhaps? Then baytrail can use it.
mp_init() can make the get_cpu_count() call after finding the first CPU, and we can remove num_cpus from struct mp_params.
To me that seams cleaner than creating a weak function.
+{
/* We need at least 2 cores to perform mp_init() */return 2;+}
+static int x86_mp_init(void) +{
struct mp_params mp_params;lapic_setup();mp_params.num_cpus = detect_num_cpus();mp_params.parallel_microcode_load = 0,mp_params.flight_plan = &mp_steps[0];mp_params.num_records = ARRAY_SIZE(mp_steps);mp_params.microcode_pointer = 0;if (mp_init(&mp_params)) {printf("Warning: MP init failure\n");return -EIO;}return 0;+} +#endif
__weak int x86_init_cpus(void) { +#ifdef CONFIG_SMP
debug("Init additional CPUs\n");x86_mp_init();+#endif
return 0;}
-- 1.8.2.1
Regards, Simon