LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "chenhui.zhao@freescale.com" <chenhui.zhao@freescale.com>
To: Scott Wood <scottwood@freescale.com>
Cc: "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Jason.Jin@freescale.com" <Jason.Jin@freescale.com>
Subject: Re: [3/4] powerpc: support CPU hotplug for e500mc, e5500 and e6500
Date: Fri, 3 Apr 2015 02:54:08 +0000	[thread overview]
Message-ID: <1428029650867.38904@freescale.com> (raw)
In-Reply-To: <1427990598.22867.271.camel@freescale.com>



________________________________________
From: Wood Scott-B07421
Sent: Friday, April 3, 2015 0:03
To: Zhao Chenhui-B35336
Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Jin Zhengxiong-R64188
Subject: Re: [3/4] powerpc: support CPU hotplug for e500mc, e5500 and e6500

On Thu, 2015-04-02 at 06:16 -0500, Zhao Chenhui-B35336 wrote:
>
> ________________________________________
> From: Wood Scott-B07421
> Sent: Tuesday, March 31, 2015 10:07
> To: Zhao Chenhui-B35336
> Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Jin Zhengxiong-R64188
> Subject: Re: [3/4] powerpc: support CPU hotplug for e500mc, e5500 and e6500
>
> On Thu, Mar 26, 2015 at 06:18:14PM +0800, chenhui zhao wrote:
> > @@ -189,16 +193,14 @@ _GLOBAL(fsl_secondary_thread_init)
> >       isync
> >
> >       /*
> > -      * Fix PIR to match the linear numbering in the device tree.
> > -      *
> > -      * On e6500, the reset value of PIR uses the low three bits for
> > -      * the thread within a core, and the upper bits for the core
> > -      * number.  There are two threads per core, so shift everything
> > -      * but the low bit right by two bits so that the cpu numbering is
> > -      * continuous.
>
> Why are you getting rid of this?  If it's to avoid doing it twice on the
> same thread, in my work-in-progress kexec patches I instead check to see
> whether BUCSR has already been set up -- if it has, I assume we've
> already been here.
>
> [chenhui] I didn't delete the branch prediction related code.

I didn't say you did.  I'm saying that you can check whether BUCSR has
been set up, to determine whether PIR has already been adjusted, if your
concern is avoiding running this twice on a thread between core resets.
If that's not your concern, then please explain.

[chenhui] If no need to change PIR in CPU hotplug, I will change the code as you mentioned.

> > +             /*
> > +              * 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.
> > +              */
>
> What if the core is not in a PM state that requires a reset?
> Where does this reset occur?
>
> [chenhui] Reset occurs in the function mpic_reset_core().
>
> > +             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);
>
> Why are you swapping the hard smp ids?
>
> [chenhui] For example, Core1 has two threads, Thread0 and Thread1. In normal boot, Thread0 is CPU2, and Thread1 is CPU3.
> But, if CPU2 and CPU3 are all off, user wants CPU3 up first. we need to call Thread0 as CPU3 and Thead1 as CPU2, considering
> the limitation, after core is reset, only Thread0 is up, then Thread0 kicks up Thread1.

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.

[chenhui] Great. If you have completed your patches, can we merge our code?

> > @@ -252,11 +340,7 @@ static int smp_85xx_kick_cpu(int nr)
> >               spin_table = phys_to_virt(*cpu_rel_addr);
> >
> >       local_irq_save(flags);
> > -#ifdef CONFIG_PPC32
> >  #ifdef CONFIG_HOTPLUG_CPU
> > -     /* Corresponding to generic_set_cpu_dead() */
> > -     generic_set_cpu_up(nr);
> > -
>
> Why did you move this?
>
> [chenhui] It would be better to set this after CPU is really up.

Please make it a separate patch with an explanation.

-Scott

[chenhui] OK.



  reply	other threads:[~2015-04-03  2:54 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-26 10:18 [PATCH 1/4] powerpc/cache: add cache flush operation for various e500 Chenhui Zhao
2015-03-26 10:18 ` [PATCH 2/4] powerpc/rcpm: add RCPM driver Chenhui Zhao
2015-03-31  1:30   ` [2/4] " Scott Wood
2015-04-02 10:33     ` chenhui.zhao
2015-04-02 15:50       ` Scott Wood
2015-03-26 10:18 ` [PATCH 3/4] powerpc: support CPU hotplug for e500mc, e5500 and e6500 Chenhui Zhao
2015-03-31  2:07   ` [3/4] " Scott Wood
2015-04-02 11:16     ` chenhui.zhao
2015-04-02 16:03       ` Scott Wood
2015-04-03  2:54         ` chenhui.zhao [this message]
2015-03-26 10:18 ` [PATCH 4/4] powerpc/85xx: support sleep feature on QorIQ SoCs with RCPM Chenhui Zhao
2015-03-31  2:35   ` [4/4] " Scott Wood
2015-04-02 11:18     ` chenhui.zhao
2015-03-31  1:10 ` [1/4] powerpc/cache: add cache flush operation for various e500 Scott Wood
2015-04-02 10:14   ` chenhui.zhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1428029650867.38904@freescale.com \
    --to=chenhui.zhao@freescale.com \
    --cc=Jason.Jin@freescale.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=scottwood@freescale.com \
    --subject='Re: [3/4] powerpc: support CPU hotplug for e500mc, e5500 and e6500' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).