[v6,1/5] powerpc/85xx: implement hardware timebase sync

Do hardware timebase sync. Firstly, stop all timebases, and transfer
the timebase value of the boot core to the other core. Finally,
start all timebases.

Only apply to dual-core chips, such as MPC8572, P2020, etc.

Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
Changes for v6:
 * added 85xx_TB_SYNC
 * added isync() after set_tb()
 * removed extra entries from mpc85xx_smp_guts_ids

 arch/powerpc/include/asm/fsl_guts.h |    2 +
 arch/powerpc/platforms/85xx/Kconfig |    5 ++
 arch/powerpc/platforms/85xx/smp.c   |   84 +++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 0 deletions(-)

On Jun 26, 2012, at 5:25 AM, Zhao Chenhui wrote:

> Do hardware timebase sync. Firstly, stop all timebases, and transfer
> the timebase value of the boot core to the other core. Finally,
> start all timebases.
> 
> Only apply to dual-core chips, such as MPC8572, P2020, etc.
> 
> Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> Changes for v6:
> * added 85xx_TB_SYNC
> * added isync() after set_tb()
> * removed extra entries from mpc85xx_smp_guts_ids

Why only on dual-core chips?  Is this because of something related to 2 cores, or related to corenet vs non-corenet SoCs and how turning on/off the timebase works in the SOC?

- k

On 06/26/2012 09:03 AM, Kumar Gala wrote:
> 
> On Jun 26, 2012, at 5:25 AM, Zhao Chenhui wrote:
> 
>> Do hardware timebase sync. Firstly, stop all timebases, and transfer
>> the timebase value of the boot core to the other core. Finally,
>> start all timebases.
>>
>> Only apply to dual-core chips, such as MPC8572, P2020, etc.
>>
>> Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
>> Signed-off-by: Li Yang <leoli@freescale.com>
>> ---
>> Changes for v6:
>> * added 85xx_TB_SYNC
>> * added isync() after set_tb()
>> * removed extra entries from mpc85xx_smp_guts_ids
> 
> Why only on dual-core chips?  Is this because of something related to
> 2 cores, or related to corenet vs non-corenet SoCs and how turning
> on/off the timebase works in the SOC?

Some parts are due to corenet versus non-corenet, such as the actual
register you write to to disable/enable the timebase.

There's also a two-core assumption in the synchronization code which
I've complained about multiple times -- although on closer inspection it
looks like this is done under cpu_add_remove_lock, and we can assume
that there's only one core at a time in take_timebase(), regardless of
how many cores are in the system.

-Scott

On Tue, 2012-06-26 at 16:45 -0500, Scott Wood wrote:

> Some parts are due to corenet versus non-corenet, such as the actual
> register you write to to disable/enable the timebase.
> 
> There's also a two-core assumption in the synchronization code which
> I've complained about multiple times -- although on closer inspection it
> looks like this is done under cpu_add_remove_lock, and we can assume
> that there's only one core at a time in take_timebase(), regardless of
> how many cores are in the system.

Right, it should work fine with any number of cores or am I missing
something ? (btw, since when complaining about something helps ? :-)

Cheers,
Ben.

On Tue, 2012-06-26 at 18:25 +0800, Zhao Chenhui wrote:
> Do hardware timebase sync. Firstly, stop all timebases, and transfer
> the timebase value of the boot core to the other core. Finally,
> start all timebases.
> 
> Only apply to dual-core chips, such as MPC8572, P2020, etc.
> 
> Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> Changes for v6:
>  * added 85xx_TB_SYNC
>  * added isync() after set_tb()
>  * removed extra entries from mpc85xx_smp_guts_ids

What's that CONFIG option for ?

Cheers,
Ben.

On Tue, Jun 26, 2012 at 09:03:42AM -0500, Kumar Gala wrote:
> 
> On Jun 26, 2012, at 5:25 AM, Zhao Chenhui wrote:
> 
> > Do hardware timebase sync. Firstly, stop all timebases, and transfer
> > the timebase value of the boot core to the other core. Finally,
> > start all timebases.
> > 
> > Only apply to dual-core chips, such as MPC8572, P2020, etc.
> > 
> > Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> > Changes for v6:
> > * added 85xx_TB_SYNC
> > * added isync() after set_tb()
> > * removed extra entries from mpc85xx_smp_guts_ids
> 
> Why only on dual-core chips?  Is this because of something related to 2 cores, or related to corenet vs non-corenet SoCs and how turning on/off the timebase works in the SOC?
> 
> - k

I am working on a timebase sync patch for corenet SoCs which have more than 2 cores.
It is based on this patch.

-Chenhui

On Wed, Jun 27, 2012 at 08:10:34AM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2012-06-26 at 18:25 +0800, Zhao Chenhui wrote:
> > Do hardware timebase sync. Firstly, stop all timebases, and transfer
> > the timebase value of the boot core to the other core. Finally,
> > start all timebases.
> > 
> > Only apply to dual-core chips, such as MPC8572, P2020, etc.
> > 
> > Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> > Changes for v6:
> >  * added 85xx_TB_SYNC
> >  * added isync() after set_tb()
> >  * removed extra entries from mpc85xx_smp_guts_ids
> 
> What's that CONFIG option for ?
> 
> Cheers,
> Ben.

This option is to guard the timebase sync routines. It is selected
when KEXEC or HOTPLUG_CPU is enabled on Freescale Book-E platforms.

-Chenhui

On Wed, 2012-06-27 at 18:21 +0800, Zhao Chenhui wrote:
> > What's that CONFIG option for ?
> > 
> > Cheers,
> > Ben.
> 
> This option is to guard the timebase sync routines. It is selected
> when KEXEC or HOTPLUG_CPU is enabled on Freescale Book-E platforms.

Any reason not to just make it unconditional ? That sort of config
option tend to just confuse things and make bug reports harder to sort
out.... Also you decrease your test coverage.

Cheers,
Ben.

On Wed, Jun 27, 2012 at 09:48:52PM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2012-06-27 at 18:21 +0800, Zhao Chenhui wrote:
> > > What's that CONFIG option for ?
> > > 
> > > Cheers,
> > > Ben.
> > 
> > This option is to guard the timebase sync routines. It is selected
> > when KEXEC or HOTPLUG_CPU is enabled on Freescale Book-E platforms.
> 
> Any reason not to just make it unconditional ? That sort of config
> option tend to just confuse things and make bug reports harder to sort
> out.... Also you decrease your test coverage.
> 
> Cheers,
> Ben.

The bootloader have done a timebase sync. If we do not need KEXEC or HOTPLUG_CPU feature,
it is unnecessary to do it again at boot time of kernel. I only compile the timebase sync routines
when users enable KEXEC or HOTPLUG_CPU.

-Chenhui

On Thu, 2012-06-28 at 11:38 +0800, Zhao Chenhui wrote:
> 
> 
> The bootloader have done a timebase sync. If we do not need KEXEC or
> HOTPLUG_CPU feature, it is unnecessary to do it again at boot time of
> kernel. I only compile the timebase sync routines
> when users enable KEXEC or HOTPLUG_CPU. 

Still, how much are you really saving ? Is it worth the added mess and
loss of test coverage ?

We have too many conditional stuff like that already.

Cheers,
Ben.

On Jun 28, 2012, at 5:50 AM, Benjamin Herrenschmidt wrote:

> On Thu, 2012-06-28 at 11:38 +0800, Zhao Chenhui wrote:
>> 
>> 
>> The bootloader have done a timebase sync. If we do not need KEXEC or
>> HOTPLUG_CPU feature, it is unnecessary to do it again at boot time of
>> kernel. I only compile the timebase sync routines
>> when users enable KEXEC or HOTPLUG_CPU. 
> 
> Still, how much are you really saving ? Is it worth the added mess and
> loss of test coverage ?
> 
> We have too many conditional stuff like that already.
> 
> Cheers,
> Ben.
> 

I'd also be interested to know how long it actually takes to do time base sync this way.  Since you are freezing the timers for some period how long does it really take between the freeze/unfreeze in mpc85xx_give_timebase()

+	mpc85xx_timebase_freeze(1);
...
+	mpc85xx_timebase_freeze(0);

You can use ATBL/U as a way to see # of cycles taken.

- k

> -----Original Message-----
> From: Linuxppc-dev [mailto:linuxppc-dev-bounces+chenhui.zhao=freescale.com@lists.ozlabs.org] On Behalf
> Of Kumar Gala
> Sent: Friday, June 29, 2012 2:30 AM
> To: Zhao Chenhui-B35336
> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org list; linux-kernel@vger.kernel.org list
> Subject: Re: [PATCH v6 1/5] powerpc/85xx: implement hardware timebase sync
> 
> 
> On Jun 28, 2012, at 5:50 AM, Benjamin Herrenschmidt wrote:
> 
> > On Thu, 2012-06-28 at 11:38 +0800, Zhao Chenhui wrote:
> >>
> >>
> >> The bootloader have done a timebase sync. If we do not need KEXEC or
> >> HOTPLUG_CPU feature, it is unnecessary to do it again at boot time of
> >> kernel. I only compile the timebase sync routines
> >> when users enable KEXEC or HOTPLUG_CPU.
> >
> > Still, how much are you really saving ? Is it worth the added mess and
> > loss of test coverage ?
> >
> > We have too many conditional stuff like that already.
> >
> > Cheers,
> > Ben.
> >
> 
> I'd also be interested to know how long it actually takes to do time base sync this way.  Since you
> are freezing the timers for some period how long does it really take between the freeze/unfreeze in
> mpc85xx_give_timebase()
> 
> +	mpc85xx_timebase_freeze(1);
> ...
> +	mpc85xx_timebase_freeze(0);
> 
> You can use ATBL/U as a way to see # of cycles taken.
> 
> - k

I measured it using ATBL on MPC8572DS with 1.5GHz core frequency and 600MHz CCB frequency.
The average of 10 times is 1019 clock. It seems that most of the time spent by isync and msync.

-Chenhui

On Tue, Jun 26, 2012 at 5:25 AM, Zhao Chenhui
<chenhui.zhao@freescale.com> wrote:
> Do hardware timebase sync. Firstly, stop all timebases, and transfer
> the timebase value of the boot core to the other core. Finally,
> start all timebases.
>
> Only apply to dual-core chips, such as MPC8572, P2020, etc.
>
> Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> Changes for v6:
>  * added 85xx_TB_SYNC
>  * added isync() after set_tb()
>  * removed extra entries from mpc85xx_smp_guts_ids
>
>  arch/powerpc/include/asm/fsl_guts.h |    2 +
>  arch/powerpc/platforms/85xx/Kconfig |    5 ++
>  arch/powerpc/platforms/85xx/smp.c   |   84 +++++++++++++++++++++++++++++++++++
>  3 files changed, 91 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/fsl_guts.h b/arch/powerpc/include/asm/fsl_guts.h
> index aa4c488..dd5ba2c 100644
> --- a/arch/powerpc/include/asm/fsl_guts.h
> +++ b/arch/powerpc/include/asm/fsl_guts.h
> @@ -48,6 +48,8 @@ struct ccsr_guts {
>         __be32  dmuxcr;                /* 0x.0068 - DMA Mux Control Register */
>         u8     res06c[0x70 - 0x6c];
>        __be32  devdisr;        /* 0x.0070 - Device Disable Control */
> +#define CCSR_GUTS_DEVDISR_TB1  0x00001000
> +#define CCSR_GUTS_DEVDISR_TB0  0x00004000
>        __be32  devdisr2;       /* 0x.0074 - Device Disable Control 2 */
>        u8      res078[0x7c - 0x78];
>        __be32  pmjcr;          /* 0x.007c - 4 Power Management Jog Control Register */
> diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> index f000d81..8dd7147 100644
> --- a/arch/powerpc/platforms/85xx/Kconfig
> +++ b/arch/powerpc/platforms/85xx/Kconfig
> @@ -8,6 +8,7 @@ menuconfig FSL_SOC_BOOKE
>        select FSL_PCI if PCI
>        select SERIAL_8250_EXTENDED if SERIAL_8250
>        select SERIAL_8250_SHARE_IRQ if SERIAL_8250
> +       select 85xx_TB_SYNC if KEXEC
>        default y
>
>  if FSL_SOC_BOOKE
> @@ -267,3 +268,7 @@ endif # FSL_SOC_BOOKE
>
>  config TQM85xx
>        bool
> +
> +config 85xx_TB_SYNC
> +       bool
> +       default n
> diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
> index ff42490..edb0cad 100644
> --- a/arch/powerpc/platforms/85xx/smp.c
> +++ b/arch/powerpc/platforms/85xx/smp.c
> @@ -24,6 +24,7 @@
>  #include <asm/mpic.h>
>  #include <asm/cacheflush.h>
>  #include <asm/dbell.h>
> +#include <asm/fsl_guts.h>
>
>  #include <sysdev/fsl_soc.h>
>  #include <sysdev/mpic.h>
> @@ -42,6 +43,69 @@ extern void __early_start(void);
>  #define NUM_BOOT_ENTRY         8
>  #define SIZE_BOOT_ENTRY                (NUM_BOOT_ENTRY * sizeof(u32))
>
> +#ifdef CONFIG_85xx_TB_SYNC
> +static struct ccsr_guts __iomem *guts;
> +static u64 timebase;
> +static int tb_req;
> +static int tb_valid;
> +
> +static void mpc85xx_timebase_freeze(int freeze)
> +{
> +       unsigned int mask;

'mask' should be uint32_t

> +
> +       if (!guts)
> +               return;

This function should never be called if guts is NULL, so this check
should be unnecessary.

> +
> +       mask = CCSR_GUTS_DEVDISR_TB0 | CCSR_GUTS_DEVDISR_TB1;
> +       if (freeze)
> +               setbits32(&guts->devdisr, mask);
> +       else
> +               clrbits32(&guts->devdisr, mask);
> +
> +       in_be32(&guts->devdisr);
> +}
> +
> +static void mpc85xx_give_timebase(void)
> +{
> +       unsigned long flags;
> +
> +       local_irq_save(flags);
> +
> +       while (!tb_req)
> +               barrier();

I think tb_req and tb_valid need to be 'volatile'.

> +       tb_req = 0;
> +
> +       mpc85xx_timebase_freeze(1);
> +       timebase = get_tb();
> +       mb();
> +       tb_valid = 1;
> +
> +       while (tb_valid)
> +               barrier();
> +
> +       mpc85xx_timebase_freeze(0);
> +
> +       local_irq_restore(flags);
> +}
> +
> +static void mpc85xx_take_timebase(void)
> +{
> +       unsigned long flags;
> +
> +       local_irq_save(flags);
> +
> +       tb_req = 1;
> +       while (!tb_valid)
> +               barrier();
> +
> +       set_tb(timebase >> 32, timebase & 0xffffffff);
> +       isync();
> +       tb_valid = 0;
> +
> +       local_irq_restore(flags);
> +}
> +#endif
> +
>  static int __init
>  smp_85xx_kick_cpu(int nr)
>  {
> @@ -228,6 +292,16 @@ smp_85xx_setup_cpu(int cpu_nr)
>                doorbell_setup_this_cpu();
>  }
>
> +static const struct of_device_id mpc85xx_smp_guts_ids[] = {
> +       { .compatible = "fsl,mpc8572-guts", },
> +       { .compatible = "fsl,p1020-guts", },
> +       { .compatible = "fsl,p1021-guts", },
> +       { .compatible = "fsl,p1022-guts", },
> +       { .compatible = "fsl,p1023-guts", },
> +       { .compatible = "fsl,p2020-guts", },
> +       {},
> +};

I wonder if it's possible to dynamically generate the compatible
string by using the SOC name?

> +
>  void __init mpc85xx_smp_init(void)
>  {
>        struct device_node *np;
> @@ -249,6 +323,16 @@ void __init mpc85xx_smp_init(void)
>                smp_85xx_ops.cause_ipi = doorbell_cause_ipi;
>        }
>
> +       np = of_find_matching_node(NULL, mpc85xx_smp_guts_ids);
> +       if (np) {
> +#ifdef CONFIG_85xx_TB_SYNC
> +               guts = of_iomap(np, 0);

You need to test the return value of of_iomap().  smp_85xx_ops should
be set only if guts is not NULL.

> +               smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
> +               smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
> +#endif
> +               of_node_put(np);
> +       }
> +
>        smp_ops = &smp_85xx_ops;
>
>  #ifdef CONFIG_KEXEC
> --
> 1.6.4.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

On 06/29/2012 10:39 AM, Tabi Timur-B04825 wrote:
> On Tue, Jun 26, 2012 at 5:25 AM, Zhao Chenhui
> <chenhui.zhao@freescale.com> wrote:
>> +static void mpc85xx_give_timebase(void)
>> +{
>> +       unsigned long flags;
>> +
>> +       local_irq_save(flags);
>> +
>> +       while (!tb_req)
>> +               barrier();
> 
> I think tb_req and tb_valid need to be 'volatile'.

No, barrier() and mb() take care of that.

>> +static const struct of_device_id mpc85xx_smp_guts_ids[] = {
>> +       { .compatible = "fsl,mpc8572-guts", },
>> +       { .compatible = "fsl,p1020-guts", },
>> +       { .compatible = "fsl,p1021-guts", },
>> +       { .compatible = "fsl,p1022-guts", },
>> +       { .compatible = "fsl,p1023-guts", },
>> +       { .compatible = "fsl,p2020-guts", },
>> +       {},
>> +};
> 
> I wonder if it's possible to dynamically generate the compatible
> string by using the SOC name?

Where are you going to get the SoC name from?

-Scott

Scott Wood wrote:

>> I wonder if it's possible to dynamically generate the compatible
>> string by using the SOC name?
> 
> Where are you going to get the SoC name from?

Well, that is why I said "I wonder".   I'm disappointed that the "cpus"
node doesn't help much.  You'd think the name of the CPU would be in the
CPU node somewhere.

On 06/29/2012 11:04 AM, Timur Tabi wrote:
> Scott Wood wrote:
> 
>>> I wonder if it's possible to dynamically generate the compatible
>>> string by using the SOC name?
>>
>> Where are you going to get the SoC name from?
> 
> Well, that is why I said "I wonder".   I'm disappointed that the "cpus"
> node doesn't help much.  You'd think the name of the CPU would be in the
> CPU node somewhere.

The SoC is not the CPU.  The CPU is e500v2.

Why is this different from anywhere else where we have a list of
compatibles to match, often based on various SoCs?  Note that we
explicitly want to match only certain SoCs here.

-Scott

Scott Wood wrote:
> Why is this different from anywhere else where we have a list of
> compatibles to match, often based on various SoCs?  Note that we
> explicitly want to match only certain SoCs here.

I was just hoping to find a way to avoid an ever increasing list of
compatible strings.  Other posts on this thread imply that this code could
work for all multi-core e500 parts.

On 06/29/2012 11:12 AM, Timur Tabi wrote:
> Scott Wood wrote:
>> Why is this different from anywhere else where we have a list of
>> compatibles to match, often based on various SoCs?  Note that we
>> explicitly want to match only certain SoCs here.
> 
> I was just hoping to find a way to avoid an ever increasing list of
> compatible strings. 

PCI drivers have to put up with it, why should we be different? :-)

> Other posts on this thread imply that this code could
> work for all multi-core e500 parts.

That list covers all multi-core e500v2 parts that I know of.  Corenet
based chips will need a slightly different implementation, since the
registers are different.

-Scott

On Fri, Jun 29, 2012 at 10:39:24AM -0500, Tabi Timur-B04825 wrote:
> On Tue, Jun 26, 2012 at 5:25 AM, Zhao Chenhui
> <chenhui.zhao@freescale.com> wrote:
> > Do hardware timebase sync. Firstly, stop all timebases, and transfer
> > the timebase value of the boot core to the other core. Finally,
> > start all timebases.
> >
> > Only apply to dual-core chips, such as MPC8572, P2020, etc.
> >
> > Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> > Changes for v6:
> >  * added 85xx_TB_SYNC
> >  * added isync() after set_tb()
> >  * removed extra entries from mpc85xx_smp_guts_ids
> >
> >  arch/powerpc/include/asm/fsl_guts.h |    2 +
> >  arch/powerpc/platforms/85xx/Kconfig |    5 ++
> >  arch/powerpc/platforms/85xx/smp.c   |   84 +++++++++++++++++++++++++++++++++++
> >  3 files changed, 91 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
> > index ff42490..edb0cad 100644
> > --- a/arch/powerpc/platforms/85xx/smp.c
> > +++ b/arch/powerpc/platforms/85xx/smp.c
> > @@ -24,6 +24,7 @@
> >  #include <asm/mpic.h>
> >  #include <asm/cacheflush.h>
> >  #include <asm/dbell.h>
> > +#include <asm/fsl_guts.h>
> >
> >  #include <sysdev/fsl_soc.h>
> >  #include <sysdev/mpic.h>
> > @@ -42,6 +43,69 @@ extern void __early_start(void);
> >  #define NUM_BOOT_ENTRY         8
> >  #define SIZE_BOOT_ENTRY                (NUM_BOOT_ENTRY * sizeof(u32))
> >
> > +#ifdef CONFIG_85xx_TB_SYNC
> > +static struct ccsr_guts __iomem *guts;
> > +static u64 timebase;
> > +static int tb_req;
> > +static int tb_valid;
> > +
> > +static void mpc85xx_timebase_freeze(int freeze)
> > +{
> > +       unsigned int mask;
> 
> 'mask' should be uint32_t

OK.

> 
> > +
> > +       if (!guts)
> > +               return;
> 
> This function should never be called if guts is NULL, so this check
> should be unnecessary.

OK.

> 
> > +
> > +       mask = CCSR_GUTS_DEVDISR_TB0 | CCSR_GUTS_DEVDISR_TB1;
> > +       if (freeze)
> > +               setbits32(&guts->devdisr, mask);
> > +       else
> > +               clrbits32(&guts->devdisr, mask);
> > +
> > +       in_be32(&guts->devdisr);
> > +}
> > +
> > @@ -249,6 +323,16 @@ void __init mpc85xx_smp_init(void)
> >                smp_85xx_ops.cause_ipi = doorbell_cause_ipi;
> >        }
> >
> > +       np = of_find_matching_node(NULL, mpc85xx_smp_guts_ids);
> > +       if (np) {
> > +#ifdef CONFIG_85xx_TB_SYNC
> > +               guts = of_iomap(np, 0);
> 
> You need to test the return value of of_iomap().  smp_85xx_ops should
> be set only if guts is not NULL.

Yes. Thanks.

> 
> > +               smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
> > +               smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
> > +#endif
> > +               of_node_put(np);
> > +       }
> > +
> >        smp_ops = &smp_85xx_ops;
> >
> >  #ifdef CONFIG_KEXEC
> > --
> > 1.6.4.1

On Thu, Jun 28, 2012 at 08:50:51PM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2012-06-28 at 11:38 +0800, Zhao Chenhui wrote:
> > 
> > 
> > The bootloader have done a timebase sync. If we do not need KEXEC or
> > HOTPLUG_CPU feature, it is unnecessary to do it again at boot time of
> > kernel. I only compile the timebase sync routines
> > when users enable KEXEC or HOTPLUG_CPU. 
> 
> Still, how much are you really saving ? Is it worth the added mess and
> loss of test coverage ?
> 
> We have too many conditional stuff like that already.
> 
> Cheers,
> Ben.

OK. I will remove this config option.

-Chenhui