lkml.org 
[lkml]   [2013]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 12/15] powerpc/85xx: add time base sync support for e6500
On 04/24/2013 06:29:29 AM, Zhao Chenhui wrote:
> On Tue, Apr 23, 2013 at 07:04:06PM -0500, Scott Wood wrote:
> > On 04/19/2013 05:47:45 AM, Zhao Chenhui wrote:
> > >From: Chen-Hui Zhao <chenhui.zhao@freescale.com>
> > >
> > >For e6500, two threads in one core share one time base. Just need
> > >to do time base sync on first thread of one core, and skip it on
> > >the other thread.
> > >
> > >Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> > >Signed-off-by: Li Yang <leoli@freescale.com>
> > >Signed-off-by: Andy Fleming <afleming@freescale.com>
> > >---
> > > arch/powerpc/platforms/85xx/smp.c | 52
> > >+++++++++++++++++++++++++++++++-----
> > > 1 files changed, 44 insertions(+), 8 deletions(-)
> > >
> > >diff --git a/arch/powerpc/platforms/85xx/smp.c
> > >b/arch/powerpc/platforms/85xx/smp.c
> > >index 74d8cde..5f3eee3 100644
> > >--- a/arch/powerpc/platforms/85xx/smp.c
> > >+++ b/arch/powerpc/platforms/85xx/smp.c
> > >@@ -26,6 +26,7 @@
> > > #include <asm/cacheflush.h>
> > > #include <asm/dbell.h>
> > > #include <asm/fsl_guts.h>
> > >+#include <asm/cputhreads.h>
> > >
> > > #include <sysdev/fsl_soc.h>
> > > #include <sysdev/mpic.h>
> > >@@ -45,6 +46,7 @@ static u64 timebase;
> > > static int tb_req;
> > > static int tb_valid;
> > > static u32 cur_booting_core;
> > >+static bool rcpmv2;
> > >
> > > #ifdef CONFIG_PPC_E500MC
> > > /* get a physical mask of online cores and booting core */
> > >@@ -53,26 +55,40 @@ static inline u32 get_phy_cpu_mask(void)
> > > u32 mask;
> > > int cpu;
> > >
> > >- mask = 1 << cur_booting_core;
> > >- for_each_online_cpu(cpu)
> > >- mask |= 1 << get_hard_smp_processor_id(cpu);
> > >+ if (smt_capable()) {
> > >+ /* two threads in one core share one time base */
> > >+ mask = 1 << cpu_core_index_of_thread(cur_booting_core);
> > >+ for_each_online_cpu(cpu)
> > >+ mask |= 1 << cpu_core_index_of_thread(
> > >+ get_hard_smp_processor_id(cpu));
> > >+ } else {
> > >+ mask = 1 << cur_booting_core;
> > >+ for_each_online_cpu(cpu)
> > >+ mask |= 1 << get_hard_smp_processor_id(cpu);
> > >+ }
> >
> > Where is smt_capable defined()? I assume somewhere in the patchset
> > but it's a pain to search 12 patches...
> >
>
> It is defined in arch/powerpc/include/asm/topology.h.
> #define smt_capable() (cpu_has_feature(CPU_FTR_SMT))
>
> Thanks for your review again.

We shouldn't base it on CPU_FTR_SMT. For example, e6500 doesn't claim
that feature yet, except in our SDK kernel. That doesn't change the
topology of CPU numbering.

> > Is this really about whether we're SMT-capable or whether we have
> > rcpm v2?
> >
> > -Scott
>
> I think this "if" statement can be removed. The
> cpu_core_index_of_thread()
> can return the correct cpu number with thread or without thread.
>
> Like this:
> static inline u32 get_phy_cpu_mask(void)
> {
> u32 mask;
> int cpu;
>
> mask = 1 << cpu_core_index_of_thread(cur_booting_core);
> for_each_online_cpu(cpu)
> mask |= 1 << cpu_core_index_of_thread(
> get_hard_smp_processor_id(cpu));
>
> return mask;
> }

Likewise, this will get it wrong if SMT is disabled or not yet
implemented on a core.

-Scott


\
 
 \ /
  Last update: 2013-04-25 01:01    [W:0.068 / U:0.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site