Hi Simon, On Fri, Jun 12, 2015 at 7:39 AM, Simon Glass <s...@chromium.org> wrote: > Hi Bin, > > On 9 June 2015 at 01:45, Bin Meng <bmeng...@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...@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. >> +{ >> + /* 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 Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot