Re: [PATCH 3/3] PowerPC/mpc85xx: Add hotplug support on E6500 cores

[Chenhui Zhao]

On Sat, Aug 1, 2015 at 8:22 AM, Scott Wood <scottwood@xxxxxxxxxxxxx> 
wrote:
> On Fri, 2015-07-31 at 17:20 +0800, b29983@xxxxxxxxxxxxx wrote:
> >  diff --git a/arch/powerpc/platforms/85xx/smp.c
> >  b/arch/powerpc/platforms/85xx/smp.c
> >  index 7f0dadb..8652a49 100644
> >  --- a/arch/powerpc/platforms/85xx/smp.c
> >  +++ b/arch/powerpc/platforms/85xx/smp.c
> >  @@ -189,15 +189,22 @@ static inline u32 read_spin_table_addr_l(void
> >  *spin_table)
> >   static void wake_hw_thread(void *info)
> >   {
> >        void fsl_secondary_thread_init(void);
> >  -     unsigned long imsr1, inia1;
> >  +     unsigned long imsr, inia;
> >        int nr = *(const int *)info;
> >  -
> >  -     imsr1 = MSR_KERNEL;
> >  -     inia1 = *(unsigned long *)fsl_secondary_thread_init;
> >  -
> >  -     mttmr(TMRN_IMSR1, imsr1);
> >  -     mttmr(TMRN_INIA1, inia1);
> >  -     mtspr(SPRN_TENS, TEN_THREAD(1));
> >  +     int hw_cpu = get_hard_smp_processor_id(nr);
> >  +     int thread_idx = cpu_thread_in_core(hw_cpu);
> >  +
> >  +     booting_cpu_hwid = (u32)hw_cpu;
>
> Unnecessary cast.  Please explain why you need booting_cpu_hwid.
>
>
> >  +     imsr = MSR_KERNEL;
> >  +     inia = *(unsigned long *)fsl_secondary_thread_init;
> >  +     if (thread_idx == 0) {
> >  +             mttmr(TMRN_IMSR0, imsr);
> >  +             mttmr(TMRN_INIA0, inia);
> >  +     } else {
> >  +             mttmr(TMRN_IMSR1, imsr);
> >  +             mttmr(TMRN_INIA1, inia);
> >  +     }
> >  +     mtspr(SPRN_TENS, TEN_THREAD(thread_idx));
>
> Please rebase this on top of http://patchwork.ozlabs.org/patch/496952/

OK.

> >  +             /*
> >  +              * If both threads are offline, reset core to start.
> >  +              * When core is up, Thread 0 always gets up first,
> >  +              * so bind the current logical cpu with Thread 0.
> >  +              */
> >  +             if (hw_cpu != cpu_first_thread_sibling(hw_cpu)) {
> >  +                     int hw_cpu1, hw_cpu2;
> >  +
> >  +                     hw_cpu1 = get_hard_smp_processor_id(primary);
> >  +                     hw_cpu2 = get_hard_smp_processor_id(primary + 
> > 1);
> >  +                     set_hard_smp_processor_id(primary, hw_cpu2);
> >  +                     set_hard_smp_processor_id(primary + 1, 
> > hw_cpu1);
> >  +                     /* get new physical cpu id */
> >  +                     hw_cpu = get_hard_smp_processor_id(nr);
>
> NACK as discussed in http://patchwork.ozlabs.org/patch/454944/
>
> -Scott

You said,

   There's no need for this. I have booting from a thread1, and having 
it
   kick its thread0, working locally without messing with the hwid/cpu
   mapping.

I still have questions here. After a core reset, how can you boot 
Thread1
of the core first. As I know, Thread0 boots up first by default.

-Chenhui





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/