Re: Re[2]: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL

2012-06-01 Thread Benjamin Herrenschmidt
On Thu, 2012-05-31 at 11:05 +0200, Joakim Tjernlund wrote:
> Abatron Support  wrote on 2012/05/30 14:08:26:
> >
> > >> I have tested this briefly with BDI2000 on P2010(e500) and
> > >> it works for me. I don't know if there are any bad side effects,
> > >> therfore
> > >> this RFC.
> >
> > > We used to have MSR_DE surrounded by CONFIG_something
> > > to ensure it wasn't set under normal operation.  IIRC, if MSR_DE
> > > is set, you will have problems with software debuggers that
> > > utilize the the debugging registers in the chip itself.  You only want
> > > to force this to be set when using the BDI, not at other times.
> >
> > This MSR_DE is also of interest and used for software debuggers that
> > make use of the debug registers. Only if MSR_DE is set then debug
> > interrupts are generated. If a debug event leads to a debug interrupt
> > handled by a software debugger or if it leads to a debug halt handled
> > by a JTAG tool is selected with DBCR0_EDM / DBCR0_IDM.
> >
> > The "e500 Core Family Reference Manual" chapter "Chapter 8
> > Debug Support" explains in detail the effect of MSR_DE.
> 
> So what is the verdict on this? I don't buy into Dan argument without some
> hard data.

The kernel normally controls when to set or not set MSR:DE, at least
when using SW breakpoints. Setting it globally should remain some kind
of specific debug option.

In fact on some CPUs, we even leave user set dbcr settings and rely on
DE being off in kernel space to avoid user->kernel attacks via the debug
registers (I think we still do that on 64-bit BookE though it should
eventually change).

Cheers,
Ben.

>  Jocke
> 
> ___
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL

2012-06-01 Thread Benjamin Herrenschmidt
On Wed, 2012-05-30 at 09:26 -0400, Bob Cochran wrote:
> I believe that additional patches are required for CodeWarrior to
> work 
> properly (e.g., assembly start up).  I think the patches should come 
> from Freescale.  For whatever reason, they include them in their SDK, 
> but haven't submitted them for inclusion in the mainline.
> 
> As a developer on Freescale Power products, I would like to see 
> Freescale offer up a CodeWarrior patch set, so I don't have to manage 
> the patches myself when working outside the SDK (i.e., on a more
> recent 
> kernel).

Such patches would have a hard time getting upstream considering that
codewarrior is a commercial product.

Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: kernel panic during kernel module load (powerpc specific part)

2012-06-01 Thread Benjamin Herrenschmidt
On Thu, 2012-05-31 at 13:04 +0200, Gabriel Paubert wrote:

> I believe that the basic premise is that you should provide a directly 
> reachable copy 
> of the save/rstore functions, even if this means that you need several copies 
> of the functions.

I just fixed a very similar problem with grub2 in fact. It was using r0
and trashing the saved LR that way.

The real fix is indeed to statically link those gcc "helpers", we
shouldn't generate things like cross-module calls inside function
prologs and epilogues, when stackframes aren't even guaranteed to be
reliable.

However, in the grub2 case, it was easier to just use r12 :-)

Cheers,
Ben.

> > 
> > Unfortunately the same doc and predecessors show r11 in all basic examples 
> > for PLT/trampoline code AFAICS, which is likely why all trampoline code 
> > uses r11 in any known case.
> > 
> > I would guess that it was never envisioned that compiler generated code 
> > would be in a different section than save/restore functions, i.e., the 
> > Linux module "__init" assumptions for Power break the ABI. Or does the ABI 
> > break the __init concept?!
> > 
> > Using r12 in the trampoline seems to be the obvious solution for module 
> > loading.
> > 
> > But what about other code loading done? If, e.g., a user runs any app from 
> > bash it gets loaded and relocated and trampolines might get set up somehow.
> 
> I don't think so. The linker/whatever should generate a copy of the 
> save/restore functions for every
> executable code area (shared library), and probably more than one copy if the 
> text becomes too large.
> 
> For 64 bit code, these functions are actually inserted by the linker.
> 
> [Ok, I just recompiled my 64 bit kernel with -Os and I see that vmlinux gets 
> one copy
> of the save/restore functions and every module also gets its copy.]
> 
> This makes sense, really these functions are there for a compromise between 
> locality 
> and performance, there should be one per code section, otherwise the cache 
> line 
> used by the trampoline negates a large part of their advantage.
> 
> > 
> > Wouldn't we have to find fix ANY trampoline code generator remotely related 
> > to a basic Power Architecture Linux?
> > Or is it a basic assumption for anything but modules that compiler 
> > generated code may never ever be outside the .text section? I am not sure 
> > that would be a safe assumption.
> > 
> > Isn't this problem going beyond just module loading for Power Architecture 
> > Linux?
> 
> I don't think so. It really seems to be a 32 bit kernel problem.
> 
>   Regards,
>   Gabriel
> ___
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH 18/27] powerpc, smpboot: Use generic SMP booting infrastructure

2012-06-01 Thread Srivatsa S. Bhat
From: Nikunj A. Dadhania 

Convert powerpc to use the generic framework to boot secondary CPUs.

Signed-off-by: Nikunj A. Dadhania 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Thomas Gleixner 
Cc: Yong Zhang 
Cc: Paul Gortmaker 
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Srivatsa S. Bhat 
---

 arch/powerpc/kernel/smp.c |   26 +++---
 1 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 1928058a..96c3718 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -544,16 +544,18 @@ static struct device_node *cpu_to_l2cache(int cpu)
 /* Activate a secondary processor. */
 void __devinit start_secondary(void *unused)
 {
+   smpboot_start_secondary(unused);
+}
+
+void __cpuinit __cpu_pre_starting(void *unused)
+{
unsigned int cpu = smp_processor_id();
-   struct device_node *l2_cache;
-   int i, base;
 
atomic_inc(&init_mm.mm_count);
current->active_mm = &init_mm;
 
smp_store_cpu_info(cpu);
set_dec(tb_ticks_per_jiffy);
-   preempt_disable();
cpu_callin_map[cpu] = 1;
 
if (smp_ops->setup_cpu)
@@ -567,8 +569,16 @@ void __devinit start_secondary(void *unused)
if (system_state == SYSTEM_RUNNING)
vdso_data->processorCount++;
 #endif
-   notify_cpu_starting(cpu);
-   set_cpu_online(cpu, true);
+}
+
+void __cpuinit __cpu_post_online(void *unused)
+{
+   unsigned int cpu;
+   struct device_node *l2_cache;
+   int i, base;
+
+   cpu = smp_processor_id();
+
/* Update sibling maps */
base = cpu_first_thread_sibling(cpu);
for (i = 0; i < threads_per_core; i++) {
@@ -596,12 +606,6 @@ void __devinit start_secondary(void *unused)
of_node_put(np);
}
of_node_put(l2_cache);
-
-   local_irq_enable();
-
-   cpu_idle();
-
-   BUG();
 }
 
 int setup_profiling_timer(unsigned int multiplier)

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH] powerpc/time: Sanity check of decrementer expiration is necessary

2012-06-01 Thread Paul Mackerras
This reverts 68568add2c ("powerpc/time: Remove unnecessary sanity check
of decrementer expiration").  We do need to check whether we have reached
the expiration time of the next event, because we sometimes get an early
decrementer interrupt, most notably when we set the decrementer to 1 in
arch_irq_work_raise().  The effect of not having the sanity check is that
if timer_interrupt() gets called early, we leave the decrementer set to
its maximum value, which means we then don't get any more decrementer
interrupts for about 4 seconds (or longer, depending on timebase
frequency).  I saw these pauses as a consequence of getting a stray
hypervisor decrementer interrupt left over from exiting a KVM guest.

This isn't quite a straight revert because of changes to the surrounding
code, but it restores the same algorithm as was previously used.

Cc: sta...@kernel.org
Cc: Anton Blanchard 
Acked-by: Benjamin Herrenschmidt 
Signed-off-by: Paul Mackerras 
---
If there are no objections, I'll send this to Linus shortly.  This
regression is present in 3.3 and 3.4 as well as current upstream.

 arch/powerpc/kernel/time.c |   14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 99a995c..be171ee 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -475,6 +475,7 @@ void timer_interrupt(struct pt_regs * regs)
struct pt_regs *old_regs;
u64 *next_tb = &__get_cpu_var(decrementers_next_tb);
struct clock_event_device *evt = &__get_cpu_var(decrementers);
+   u64 now;
 
/* Ensure a positive value is written to the decrementer, or else
 * some CPUs will continue to take decrementer exceptions.
@@ -509,9 +510,16 @@ void timer_interrupt(struct pt_regs * regs)
irq_work_run();
}
 
-   *next_tb = ~(u64)0;
-   if (evt->event_handler)
-   evt->event_handler(evt);
+   now = get_tb_or_rtc();
+   if (now >= *next_tb) {
+   *next_tb = ~(u64)0;
+   if (evt->event_handler)
+   evt->event_handler(evt);
+   } else {
+   now = *next_tb - now;
+   if (now <= DECREMENTER_MAX)
+   set_dec((int)now);
+   }
 
 #ifdef CONFIG_PPC64
/* collect purr register values often, for accurate calculations */
-- 
1.7.10

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


power management patch set for mpc85xx

2012-06-01 Thread Zhao Chenhui-B35336
Hi Ben and Paul,

I am sorry to trouble you. It seems that Kumar is busy recently.

Could you have a review on the following patches? These patches
implement the power management support on MPC85xx platform.

http://patchwork.ozlabs.org/patch/158484/
http://patchwork.ozlabs.org/patch/158485/
http://patchwork.ozlabs.org/patch/158487/
http://patchwork.ozlabs.org/patch/158486/
http://patchwork.ozlabs.org/patch/158488/

Thanks.

Best Regards,
Chenhui


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: Re[2]: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL

2012-06-01 Thread Joakim Tjernlund
Benjamin Herrenschmidt  wrote on 2012/06/01 11:12:51:
>
> On Thu, 2012-05-31 at 11:05 +0200, Joakim Tjernlund wrote:
> > Abatron Support  wrote on 2012/05/30 14:08:26:
> > >
> > > >> I have tested this briefly with BDI2000 on P2010(e500) and
> > > >> it works for me. I don't know if there are any bad side effects,
> > > >> therfore
> > > >> this RFC.
> > >
> > > > We used to have MSR_DE surrounded by CONFIG_something
> > > > to ensure it wasn't set under normal operation.  IIRC, if MSR_DE
> > > > is set, you will have problems with software debuggers that
> > > > utilize the the debugging registers in the chip itself.  You only want
> > > > to force this to be set when using the BDI, not at other times.
> > >
> > > This MSR_DE is also of interest and used for software debuggers that
> > > make use of the debug registers. Only if MSR_DE is set then debug
> > > interrupts are generated. If a debug event leads to a debug interrupt
> > > handled by a software debugger or if it leads to a debug halt handled
> > > by a JTAG tool is selected with DBCR0_EDM / DBCR0_IDM.
> > >
> > > The "e500 Core Family Reference Manual" chapter "Chapter 8
> > > Debug Support" explains in detail the effect of MSR_DE.
> >
> > So what is the verdict on this? I don't buy into Dan argument without some
> > hard data.
>
> The kernel normally controls when to set or not set MSR:DE, at least
> when using SW breakpoints. Setting it globally should remain some kind
> of specific debug option.
>
> In fact on some CPUs, we even leave user set dbcr settings and rely on
> DE being off in kernel space to avoid user->kernel attacks via the debug
> registers (I think we still do that on 64-bit BookE though it should
> eventually change).

hmm, would it not be better to always clear out/control dbcr settings and 
always have MSR:DE
on? It would be much easier to control dbcr, even dynamically, than MSR:DE

 Jocke

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH 03/27] smpboot: Define and use cpu_state per-cpu variable in generic code

2012-06-01 Thread Srivatsa S. Bhat
The per-cpu variable cpu_state is used in x86 and also used in other
architectures, to track the state of the cpu during bringup and hotplug.
Pull it out into generic code.

Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Ralf Baechle 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mundt 
Cc: Chris Metcalf 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: "H. Peter Anvin" 
Cc: x...@kernel.org
Cc: Konrad Rzeszutek Wilk 
Cc: Jeremy Fitzhardinge 
Cc: Peter Zijlstra 
Cc: Andrew Morton 
Cc: Mike Frysinger 
Cc: Yong Zhang 
Cc: Venkatesh Pallipadi 
Cc: Suresh Siddha 
Cc: linux-i...@vger.kernel.org
Cc: linux-m...@linux-mips.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux...@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Cc: virtualizat...@lists.linux-foundation.org
Signed-off-by: Srivatsa S. Bhat 
---

 arch/ia64/include/asm/cpu.h   |2 --
 arch/ia64/kernel/process.c|1 +
 arch/ia64/kernel/smpboot.c|6 +-
 arch/mips/cavium-octeon/smp.c |4 +---
 arch/powerpc/kernel/smp.c |6 +-
 arch/sh/include/asm/smp.h |2 --
 arch/sh/kernel/smp.c  |4 +---
 arch/tile/kernel/smpboot.c|4 +---
 arch/x86/include/asm/cpu.h|2 --
 arch/x86/kernel/smpboot.c |4 +---
 arch/x86/xen/smp.c|1 +
 include/linux/smpboot.h   |1 +
 kernel/smpboot.c  |4 
 13 files changed, 13 insertions(+), 28 deletions(-)

diff --git a/arch/ia64/include/asm/cpu.h b/arch/ia64/include/asm/cpu.h
index fcca30b..1c3acac 100644
--- a/arch/ia64/include/asm/cpu.h
+++ b/arch/ia64/include/asm/cpu.h
@@ -12,8 +12,6 @@ struct ia64_cpu {
 
 DECLARE_PER_CPU(struct ia64_cpu, cpu_devices);
 
-DECLARE_PER_CPU(int, cpu_state);
-
 #ifdef CONFIG_HOTPLUG_CPU
 extern int arch_register_cpu(int num);
 extern void arch_unregister_cpu(int);
diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
index 5e0e86d..32566c7 100644
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
index 963d2db..df00a3c 100644
--- a/arch/ia64/kernel/smpboot.c
+++ b/arch/ia64/kernel/smpboot.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -111,11 +112,6 @@ extern unsigned long ia64_iobase;
 
 struct task_struct *task_for_booting_cpu;
 
-/*
- * State for each CPU
- */
-DEFINE_PER_CPU(int, cpu_state);
-
 cpumask_t cpu_core_map[NR_CPUS] __cacheline_aligned;
 EXPORT_SYMBOL(cpu_core_map);
 DEFINE_PER_CPU_SHARED_ALIGNED(cpumask_t, cpu_sibling_map);
diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c
index 97e7ce9..93cd4b0 100644
--- a/arch/mips/cavium-octeon/smp.c
+++ b/arch/mips/cavium-octeon/smp.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -252,9 +253,6 @@ static void octeon_cpus_done(void)
 
 #ifdef CONFIG_HOTPLUG_CPU
 
-/* State of each CPU. */
-DEFINE_PER_CPU(int, cpu_state);
-
 extern void fixup_irqs(void);
 
 static DEFINE_SPINLOCK(smp_reserve_lock);
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index e1417c4..1928058a 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -57,11 +58,6 @@
 #define DBG(fmt...)
 #endif
 
-#ifdef CONFIG_HOTPLUG_CPU
-/* State of each CPU during hotplug phases */
-static DEFINE_PER_CPU(int, cpu_state) = { 0 };
-#endif
-
 struct thread_info *secondary_ti;
 
 DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
diff --git a/arch/sh/include/asm/smp.h b/arch/sh/include/asm/smp.h
index 78b0d0f4..bda041e 100644
--- a/arch/sh/include/asm/smp.h
+++ b/arch/sh/include/asm/smp.h
@@ -31,8 +31,6 @@ enum {
SMP_MSG_NR, /* must be last */
 };
 
-DECLARE_PER_CPU(int, cpu_state);
-
 void smp_message_recv(unsigned int msg);
 void smp_timer_broadcast(const struct cpumask *mask);
 
diff --git a/arch/sh/kernel/smp.c b/arch/sh/kernel/smp.c
index b86e9ca..8e0fde0 100644
--- a/arch/sh/kernel/smp.c
+++ b/arch/sh/kernel/smp.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -34,9 +35,6 @@ int __cpu_logical_map[NR_CPUS];   /* Map logical 
to physical */
 
 struct plat_smp_ops *mp_ops = NULL;
 
-/* State of each CPU */
-DEFINE_PER_CPU(int, cpu_state) = { 0 };
-
 void __cpuinit register_smp_ops(struct plat_smp_ops *ops)
 {
if (mp_ops)
diff --git a/arch/tile/kernel/smpboot.c b/arch/tile/kernel/smpboot.c
index e686c5a..24a9c06 100644
--- a/arch/tile/kernel/smpboot.c
+++ b/arch/tile/kernel/smpboot.c
@@ -25,13 +25,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 
-/* State of each CPU. */
-static DEFINE_PER_CPU(int, cpu_state) = { 0 };
-
 /* The messaging code jumps to this pointer during boot-up */
 unsigned long start_cpu_function_addr;
 
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/c

Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL

2012-06-01 Thread Joakim Tjernlund
Benjamin Herrenschmidt  wrote on 2012/06/01 11:14:49:
>
> On Wed, 2012-05-30 at 09:26 -0400, Bob Cochran wrote:
> > I believe that additional patches are required for CodeWarrior to
> > work
> > properly (e.g., assembly start up).  I think the patches should come
> > from Freescale.  For whatever reason, they include them in their SDK,
> > but haven't submitted them for inclusion in the mainline.
> >
> > As a developer on Freescale Power products, I would like to see
> > Freescale offer up a CodeWarrior patch set, so I don't have to manage
> > the patches myself when working outside the SDK (i.e., on a more
> > recent
> > kernel).
>
> Such patches would have a hard time getting upstream considering that
> codewarrior is a commercial product.

Naa, Abatron BDI is also a commercial product and that is in the kernel. CW
changes pretty much just add the same settings for CONFIG_CW. I think Freescale
should just rename CONFIG_BDI_SWITCH to something generic and just use the same 
code.

 Jocke

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


RE: kernel panic during kernel module load (powerpc specific part)

2012-06-01 Thread Wrobel Heinz-R39252
> > I believe that the basic premise is that you should provide a directly
> > reachable copy of the save/rstore functions, even if this means that
> you need several copies of the functions.
> 
> I just fixed a very similar problem with grub2 in fact. It was using r0
> and trashing the saved LR that way.
> 
> The real fix is indeed to statically link those gcc "helpers", we
> shouldn't generate things like cross-module calls inside function prologs
> and epilogues, when stackframes aren't even guaranteed to be reliable.
> 
> However, in the grub2 case, it was easier to just use r12 :-)

For not just the module loading case, I believe r12 is the only real solution 
now. I checked one debugger capable of doing ELF download. It also uses r12 for 
trampoline code. I am guessing for the reason that prompted this discussion.

Without r12 we'd have to change standard libraries to automagically link in gcc 
helpers for any conceivable non-.text section, which I am not sure is feasible. 
How would you write section independent helper functions which link to any 
section needing them?!
Asking users to create their own section specific copy of helper functions is 
definitely not portable if the module or other code is not architecture 
dependent.
It is a normal gcc feature that you can assign specific code to non-.text 
sections and it is not documented that it may crash depending on the OS arch 
the ELF is built for, so asking for a Power Architecture specific change on 
tool libs to make Power Architecture Linux happy seems a bit much to ask.

Using r12 in any Linux related trampoline code seems a reachable goal, and it 
would eliminate the conflict to the ABI.

Regards,

Heinz
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: power management patch set for mpc85xx

2012-06-01 Thread Kumar Gala

On Jun 1, 2012, at 5:29 AM, Zhao Chenhui-B35336 wrote:

> Hi Ben and Paul,
> 
> I am sorry to trouble you. It seems that Kumar is busy recently.
> 
> Could you have a review on the following patches? These patches
> implement the power management support on MPC85xx platform.
> 
> http://patchwork.ozlabs.org/patch/158484/
> http://patchwork.ozlabs.org/patch/158485/
> http://patchwork.ozlabs.org/patch/158487/
> http://patchwork.ozlabs.org/patch/158486/
> http://patchwork.ozlabs.org/patch/158488/

I have been, but was looking for Scott's Ack on these.

- k
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v5 1/5] powerpc/85xx: implement hardware timebase sync

2012-06-01 Thread Scott Wood
On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
>  #ifdef CONFIG_KEXEC
> +static struct ccsr_guts __iomem *guts;
> +static u64 timebase;
> +static int tb_req;
> +static int tb_valid;
> +
> +static void mpc85xx_timebase_freeze(int freeze)

Why is this under CONFIG_KEXEC?  It'll also be needed for CPU hotplug.

> +{
> + unsigned int mask;
> +
> + if (!guts)
> + return;
> +
> + 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();
> + 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 & 0x);
> + mb();
> + tb_valid = 0;
> +
> + local_irq_restore(flags);
> +}

I know you say this is for dual-core chips only, but it would be nice if
you'd write this in a way that doesn't assume that (even if the
corenet-specific timebase freezing comes later).

Do we need an isync after setting the timebase, to ensure it's happened
before we enable the timebase?  Likewise, do we need a readback after
disabling the timebase to ensure it's disabled before we read the
timebase in give_timebase?

>  atomic_t kexec_down_cpus = ATOMIC_INIT(0);
>  
>  void mpc85xx_smp_kexec_cpu_down(int crash_shutdown, int secondary)
> @@ -228,6 +286,20 @@ smp_85xx_setup_cpu(int cpu_nr)
>   doorbell_setup_this_cpu();
>  }
>  
> +#ifdef CONFIG_KEXEC
> +static const struct of_device_id guts_ids[] = {
> + { .compatible = "fsl,mpc8572-guts", },
> + { .compatible = "fsl,mpc8560-guts", },
> + { .compatible = "fsl,mpc8536-guts", },
> + { .compatible = "fsl,p1020-guts", },
> + { .compatible = "fsl,p1021-guts", },
> + { .compatible = "fsl,p1022-guts", },
> + { .compatible = "fsl,p1023-guts", },
> + { .compatible = "fsl,p2020-guts", },
> + {},
> +};
> +#endif

MPC8560 and MPC8536 are single-core...

Also please use a more specific name, such as e500v2_smp_guts_ids or
mpc85xx_smp_guts_ids -- when corenet support is added it will likely be
in the same file.

>  void __init mpc85xx_smp_init(void)
>  {
>   struct device_node *np;
> @@ -249,6 +321,19 @@ void __init mpc85xx_smp_init(void)
>   smp_85xx_ops.cause_ipi = doorbell_cause_ipi;
>   }
>  
> +#ifdef CONFIG_KEXEC
> + np = of_find_matching_node(NULL, guts_ids);
> + if (np) {
> + guts = of_iomap(np, 0);
> + smp_85xx_ops.give_timebase = mpc85xx_give_timebase;
> + smp_85xx_ops.take_timebase = mpc85xx_take_timebase;
> + of_node_put(np);
> + } else {
> + smp_85xx_ops.give_timebase = smp_generic_give_timebase;
> + smp_85xx_ops.take_timebase = smp_generic_take_timebase;
> + }

Do not use smp_generic_give/take_timebase, ever.  If you don't have the
guts node, then just assume the timebase is already synced.

-Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL

2012-06-01 Thread Scott Wood
On 06/01/2012 04:14 AM, Benjamin Herrenschmidt wrote:
> On Wed, 2012-05-30 at 09:26 -0400, Bob Cochran wrote:
>> I believe that additional patches are required for CodeWarrior to
>> work 
>> properly (e.g., assembly start up).  I think the patches should come 
>> from Freescale.  For whatever reason, they include them in their SDK, 
>> but haven't submitted them for inclusion in the mainline.
>>
>> As a developer on Freescale Power products, I would like to see 
>> Freescale offer up a CodeWarrior patch set, so I don't have to manage 
>> the patches myself when working outside the SDK (i.e., on a more
>> recent 
>> kernel).
> 
> Such patches would have a hard time getting upstream considering that
> codewarrior is a commercial product.

It's not really about CodeWarrior -- it's needed for any external debug
on these chips.

Those chips are commercial products too, BTW. :-)

-Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH 03/27] smpboot: Define and use cpu_state per-cpu variable in generic code

2012-06-01 Thread David Daney

On 06/01/2012 02:10 AM, Srivatsa S. Bhat wrote:

The per-cpu variable cpu_state is used in x86 and also used in other
architectures, to track the state of the cpu during bringup and hotplug.
Pull it out into generic code.

Cc: Tony Luck
Cc: Fenghua Yu
Cc: Ralf Baechle
Cc: Benjamin Herrenschmidt
Cc: Paul Mundt
Cc: Chris Metcalf
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: "H. Peter Anvin"
Cc: x...@kernel.org
Cc: Konrad Rzeszutek Wilk
Cc: Jeremy Fitzhardinge
Cc: Peter Zijlstra
Cc: Andrew Morton
Cc: Mike Frysinger
Cc: Yong Zhang
Cc: Venkatesh Pallipadi
Cc: Suresh Siddha
Cc: linux-i...@vger.kernel.org
Cc: linux-m...@linux-mips.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux...@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Cc: virtualizat...@lists.linux-foundation.org
Signed-off-by: Srivatsa S. Bhat
---

  arch/ia64/include/asm/cpu.h   |2 --
  arch/ia64/kernel/process.c|1 +
  arch/ia64/kernel/smpboot.c|6 +-
  arch/mips/cavium-octeon/smp.c |4 +---
  arch/powerpc/kernel/smp.c |6 +-
  arch/sh/include/asm/smp.h |2 --
  arch/sh/kernel/smp.c  |4 +---
  arch/tile/kernel/smpboot.c|4 +---
  arch/x86/include/asm/cpu.h|2 --
  arch/x86/kernel/smpboot.c |4 +---
  arch/x86/xen/smp.c|1 +
  include/linux/smpboot.h   |1 +
  kernel/smpboot.c  |4 
  13 files changed, 13 insertions(+), 28 deletions(-)


[...]

diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c
index 97e7ce9..93cd4b0 100644
--- a/arch/mips/cavium-octeon/smp.c
+++ b/arch/mips/cavium-octeon/smp.c
@@ -13,6 +13,7 @@
  #include
  #include
  #include
+#include

  #include
  #include
@@ -252,9 +253,6 @@ static void octeon_cpus_done(void)

  #ifdef CONFIG_HOTPLUG_CPU

-/* State of each CPU. */
-DEFINE_PER_CPU(int, cpu_state);
-
  extern void fixup_irqs(void);

  static DEFINE_SPINLOCK(smp_reserve_lock);


The Octeon bit:

Acked-by: David Daney 


FWIW, the rest looks good too.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v5 2/5] powerpc/85xx: add HOTPLUG_CPU support

2012-06-01 Thread Scott Wood
On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
\> +#if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
> +extern void __flush_disable_L1(void);
> +#endif

Prototypes aren't normally guarded by ifdefs.

> +static void __cpuinit smp_85xx_mach_cpu_die(void)
> +{
> + unsigned int cpu = smp_processor_id();
> + u32 tmp;
> +
> + local_irq_disable();
> + idle_task_exit();
> + generic_set_cpu_dead(cpu);
> + mb();
> +
> + mtspr(SPRN_TCR, 0);
> +
> + __flush_disable_L1();
> + tmp = (mfspr(SPRN_HID0) & ~(HID0_DOZE|HID0_SLEEP)) | HID0_NAP;
> + mtspr(SPRN_HID0, tmp);
> +
> + /* Enter NAP mode. */
> + tmp = mfmsr();
> + tmp |= MSR_WE;
> + mb();
> + mtmsr(tmp);
> + isync();

Need isync after writing to HID0.

> + /*
> +  * We don't set the BPTR register here upon it points
> +  * to the boot page properly.
> +  */
> + mpic_reset_core(hw_cpu);

That comment's wording is hard to follow -- maybe s/upon it points/since
it already points/

> + /* wait until core is ready... */
> + if (!spin_event_timeout(in_be32(&spin_table->addr_l) == 1,
> + 1, 100)) {
> + pr_err("%s: timeout waiting for core %d to reset\n",
> + __func__, hw_cpu);
> + ret = -ENOENT;
> + goto out;
> + }

We need to fix U-Boot to write addr_l last (with an msync beforehand).

> -#ifdef CONFIG_KEXEC
> +#if defined(CONFIG_KEXEC) || defined(CONFIG_HOTPLUG_CPU)

Let's not grow lists like this.  Is there any harm in building it
unconditionally?

-Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v5 3/5] powerpc/85xx: add sleep and deep sleep support

2012-06-01 Thread Scott Wood
On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> From: Li Yang 
> 
> In sleep PM mode, the clocks of e500 core and unused IP blocks is
> turned off. IP blocks which are allowed to wake up the processor
> are still running.
> 
> Some Freescale chips like MPC8536 and P1022 has deep sleep PM mode
> in addtion to the sleep PM mode.
> 
> While in deep sleep PM mode, additionally, the power supply is
> removed from e500 core and most IP blocks. Only the blocks needed
> to wake up the chip out of deep sleep are ON.
> 
> This patch supports 32-bit and 36-bit address space.
> 
> The sleep mode is equal to the Standby state in Linux. The deep sleep
> mode is equal to the Suspend-to-RAM state of Linux Power Management.
> 
> Command to enter sleep mode.
>   echo standby > /sys/power/state
> Command to enter deep sleep mode.
>   echo mem > /sys/power/state
> 
> Signed-off-by: Dave Liu 
> Signed-off-by: Li Yang 
> Signed-off-by: Jin Qing 
> Signed-off-by: Jerry Huang 
> Cc: Scott Wood 
> Signed-off-by: Zhao Chenhui 
> ---
> Changes for v5:
>  * Rename flush_disable_L1 to __flush_disable_L1.
> 
>  arch/powerpc/Kconfig  |2 +-
>  arch/powerpc/include/asm/cacheflush.h |5 +
>  arch/powerpc/kernel/Makefile  |3 +
>  arch/powerpc/kernel/l2cache_85xx.S|   53 +++
>  arch/powerpc/platforms/85xx/Makefile  |3 +
>  arch/powerpc/platforms/85xx/sleep.S   |  609 
> +
>  arch/powerpc/sysdev/fsl_pmc.c |   91 -
>  arch/powerpc/sysdev/fsl_soc.h |5 +
>  8 files changed, 752 insertions(+), 19 deletions(-)
>  create mode 100644 arch/powerpc/kernel/l2cache_85xx.S
>  create mode 100644 arch/powerpc/platforms/85xx/sleep.S
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d65ae35..039f0a6 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -673,7 +673,7 @@ config FSL_PCI
>  config FSL_PMC
>   bool
>   default y
> - depends on SUSPEND && (PPC_85xx || PPC_86xx)
> + depends on SUSPEND && (PPC_85xx || PPC_86xx) && !PPC_E500MC
>   help
> Freescale MPC85xx/MPC86xx power management controller support
> (suspend/resume). For MPC83xx see platforms/83xx/suspend.c
> diff --git a/arch/powerpc/include/asm/cacheflush.h 
> b/arch/powerpc/include/asm/cacheflush.h
> index 94ec20a..baa000c 100644
> --- a/arch/powerpc/include/asm/cacheflush.h
> +++ b/arch/powerpc/include/asm/cacheflush.h
> @@ -33,6 +33,11 @@ extern void flush_dcache_page(struct page *page);
>  #if defined(CONFIG_FSL_BOOKE) || defined(CONFIG_6xx)
>  extern void __flush_disable_L1(void);
>  #endif
> +#if defined(CONFIG_FSL_BOOKE)
> +extern void flush_dcache_L1(void);
> +#else
> +#define flush_dcache_L1()do { } while (0)
> +#endif

It doesn't seem right to no-op this on other platforms.

>  extern void __flush_icache_range(unsigned long, unsigned long);
>  static inline void flush_icache_range(unsigned long start, unsigned long 
> stop)
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index f5808a3..cb70dba 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -64,6 +64,9 @@ obj-$(CONFIG_FA_DUMP)   += fadump.o
>  ifeq ($(CONFIG_PPC32),y)
>  obj-$(CONFIG_E500)   += idle_e500.o
>  endif
> +ifneq ($(CONFIG_PPC_E500MC),y)
> +obj-$(CONFIG_PPC_85xx)   += l2cache_85xx.o
> +endif

Can we introduce a symbol that specifically means pre-e500mc e500,
rather than using negative logic?

I think something like CONFIG_PPC_E500_V1_V2 has been proposed before.

> -static int pmc_probe(struct platform_device *ofdev)
> +static int pmc_probe(struct platform_device *pdev)
>  {
> - pmc_regs = of_iomap(ofdev->dev.of_node, 0);
> + struct device_node *np = pdev->dev.of_node;
> +
> + pmc_regs = of_iomap(np, 0);
>   if (!pmc_regs)
>   return -ENOMEM;
>  
> - pmc_dev = &ofdev->dev;
> + pmc_flag = PMC_SLEEP;
> + if (of_device_is_compatible(np, "fsl,mpc8536-pmc"))
> + pmc_flag |= PMC_DEEP_SLEEP;
> +
> + if (of_device_is_compatible(np, "fsl,p1022-pmc"))
> + pmc_flag |= PMC_DEEP_SLEEP;
> +
>   suspend_set_ops(&pmc_suspend_ops);
> +
> + pr_info("Freescale PMC driver\n");

If you're going to be noisy on probe, at least provide some useful info
like whether deep sleep or jog are supported.

>   return 0;
>  }
>  
> diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
> index c6d0073..949377d 100644
> --- a/arch/powerpc/sysdev/fsl_soc.h
> +++ b/arch/powerpc/sysdev/fsl_soc.h
> @@ -48,5 +48,10 @@ extern struct platform_diu_data_ops diu_ops;
>  void fsl_hv_restart(char *cmd);
>  void fsl_hv_halt(void);
>  
> +/*
> + * Cast the ccsrbar to 64-bit parameter so that the assembly
> + * code can be compatible with both 32-bit & 36-bit.
> + */
> +extern void mpc85xx_enter_deep_sleep(u64 ccsrbar, u32 powmgtreq);

s/Cast the ccsrbar to 64-bit parameter/ccsrbar is u64 rather than
phys_addr

Re: [PATCH v5 4/5] fsl_pmc: Add API to enable device as wakeup event source

2012-06-01 Thread Scott Wood
On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> Add APIs for setting wakeup source and lossless Ethernet in low power modes.
> These APIs can be used by wake-on-packet feature.
> 
> Signed-off-by: Dave Liu 
> Signed-off-by: Li Yang 
> Signed-off-by: Jin Qing 
> Signed-off-by: Zhao Chenhui 
> ---
>  arch/powerpc/sysdev/fsl_pmc.c |   71 
> -
>  arch/powerpc/sysdev/fsl_soc.h |9 +
>  2 files changed, 79 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/fsl_pmc.c b/arch/powerpc/sysdev/fsl_pmc.c
> index 1dc6e9e..c1170f7 100644
> --- a/arch/powerpc/sysdev/fsl_pmc.c
> +++ b/arch/powerpc/sysdev/fsl_pmc.c
> @@ -34,6 +34,7 @@ struct pmc_regs {
>   __be32 powmgtcsr;
>  #define POWMGTCSR_SLP0x0002
>  #define POWMGTCSR_DPSLP  0x0010
> +#define POWMGTCSR_LOSSLESS   0x0040
>   __be32 res3[2];
>   __be32 pmcdr;
>  };
> @@ -43,6 +44,74 @@ static unsigned int pmc_flag;
>  
>  #define PMC_SLEEP0x1
>  #define PMC_DEEP_SLEEP   0x2
> +#define PMC_LOSSLESS 0x4
> +
> +/**
> + * mpc85xx_pmc_set_wake - enable devices as wakeup event source
> + * @pdev: platform device affected
> + * @enable: True to enable event generation; false to disable
> + *
> + * This enables the device as a wakeup event source, or disables it.
> + *
> + * RETURN VALUE:
> + * 0 is returned on success
> + * -EINVAL is returned if device is not supposed to wake up the system
> + * Error code depending on the platform is returned if both the platform and
> + * the native mechanism fail to enable the generation of wake-up events
> + */
> +int mpc85xx_pmc_set_wake(struct platform_device *pdev, bool enable)

Why does it have to be a platform_device?  Would a bare device_node work
here?  If it's for stuff like device_may_wakeup() that could be in a
platform_device wrapper function.

Where does this get called from?  I don't see an example user in this
patchset.

> +{
> + int ret = 0;
> + struct device_node *clk_np;
> + u32 *prop;
> + u32 pmcdr_mask;
> +
> + if (!pmc_regs) {
> + pr_err("%s: PMC is unavailable\n", __func__);
> + return -ENODEV;
> + }
> +
> + if (enable && !device_may_wakeup(&pdev->dev))
> + return -EINVAL;

Who is setting can_wakeup for these devices?

> + clk_np = of_parse_phandle(pdev->dev.of_node, "fsl,pmc-handle", 0);
> + if (!clk_np)
> + return -EINVAL;
> +
> + prop = (u32 *)of_get_property(clk_np, "fsl,pmcdr-mask", NULL);

Don't cast the const away.

> + if (!prop) {
> + ret = -EINVAL;
> + goto out;
> + }
> + pmcdr_mask = be32_to_cpup(prop);
> +
> + if (enable)
> + /* clear to enable clock in low power mode */
> + clrbits32(&pmc_regs->pmcdr, pmcdr_mask);
> + else
> + setbits32(&pmc_regs->pmcdr, pmcdr_mask);

What is the default PMCDR if this function is never called?  Should init
to all bits set on PM driver probe (or maybe limit it to defined bits
only, though that's a little harder to do generically).

-Scot

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: power management patch set for mpc85xx

2012-06-01 Thread Benjamin Herrenschmidt
On Fri, 2012-06-01 at 10:29 +, Zhao Chenhui-B35336 wrote:
> Hi Ben and Paul,
> 
> I am sorry to trouble you. It seems that Kumar is busy recently.
> 
> Could you have a review on the following patches? These patches
> implement the power management support on MPC85xx platform.

At this point, I don't have the bandwidth for that and I suspect Paul
doesn't either.

If Kumar isn't able to maintain the FSL stuff anymore then somebody else
needs to step up, maybe Scott Wood ?

I haven't heard from Kumar in a while so I'm not sure what's up.

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: power management patch set for mpc85xx

2012-06-01 Thread Benjamin Herrenschmidt
On Sat, 2012-06-02 at 08:17 +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2012-06-01 at 10:29 +, Zhao Chenhui-B35336 wrote:
> > Hi Ben and Paul,
> > 
> > I am sorry to trouble you. It seems that Kumar is busy recently.
> > 
> > Could you have a review on the following patches? These patches
> > implement the power management support on MPC85xx platform.
> 
> At this point, I don't have the bandwidth for that and I suspect Paul
> doesn't either.
> 
> If Kumar isn't able to maintain the FSL stuff anymore then somebody else
> needs to step up, maybe Scott Wood ?
> 
> I haven't heard from Kumar in a while so I'm not sure what's up.

Ah, I finally did hear from Kumar. So things are still on track, it's
just a missing ack.

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL

2012-06-01 Thread Benjamin Herrenschmidt
On Fri, 2012-06-01 at 11:28 -0500, Scott Wood wrote:
> 
> It's not really about CodeWarrior -- it's needed for any external
> debug
> on these chips.
> 
> Those chips are commercial products too, BTW. :-)

As long as it's not code to specifically interact with the CW software
it's ok.

I don't have a special axe to grind against CW (I use to love it under
ol' MacOS, though the new eclipse based one does seem to suck hard...
but then I never got a "licence" to use it past the demo anyway), it's
just that I don't want to start building SW interfaces to a foreign
tool.

BTW. My point of view is that this whole business about MSR:DE is a HW
design bug. There should be -no- (absolutely 0) interaction between the
SW state and the HW debugger for normal operations unless the user of
the debugger explicitly wants to change some state.

Cheers,
Ben.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL

2012-06-01 Thread Scott Wood
On 06/01/2012 05:30 PM, Benjamin Herrenschmidt wrote:
> BTW. My point of view is that this whole business about MSR:DE is a HW
> design bug. There should be -no- (absolutely 0) interaction between the
> SW state and the HW debugger for normal operations unless the user of
> the debugger explicitly wants to change some state.

I agree entirely, and e500mc at least has less of this than e500v2 (not
sure if it still needs MSR[DE], but supposedly it doesn't have the
requirement for there to be a valid instruction at the debug vector,
which is lots of fun when booting).  But this isn't exactly something
Freescale is going to replace existing chips over.

Getting all the way to zero interaction would require a completely
separate debug facility so software can debug at the same time.  I'd be
all for that (and let's throw in a third, for the hypervisor), but I'm
not the one that needs to be convinced.

-Scott

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL

2012-06-01 Thread Benjamin Herrenschmidt
On Fri, 2012-06-01 at 17:42 -0500, Scott Wood wrote:
> On 06/01/2012 05:30 PM, Benjamin Herrenschmidt wrote:
> > BTW. My point of view is that this whole business about MSR:DE is a HW
> > design bug. There should be -no- (absolutely 0) interaction between the
> > SW state and the HW debugger for normal operations unless the user of
> > the debugger explicitly wants to change some state.
> 
> I agree entirely, and e500mc at least has less of this than e500v2 (not
> sure if it still needs MSR[DE], but supposedly it doesn't have the
> requirement for there to be a valid instruction at the debug vector,
> which is lots of fun when booting).  But this isn't exactly something
> Freescale is going to replace existing chips over.
> 
> Getting all the way to zero interaction would require a completely
> separate debug facility so software can debug at the same time.  I'd be
> all for that (and let's throw in a third, for the hypervisor), but I'm
> not the one that needs to be convinced.

You can find a good compromise. If you have some kind of SPR letting you
know now many DACs and IACs are available, you could essentially
"reserve" some for HW debug with the probe. Not as good as a fully
separate facility but still better than stepping on each other toes.

Things like DBCR should probably still be separated. There's no excuse
for the MSR:DE bullshit tho :-)

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH v5 5/5] powerpc/85xx: add support to JOG feature using cpufreq interface

2012-06-01 Thread Scott Wood
On 05/11/2012 06:53 AM, Zhao Chenhui wrote:
> Some 85xx silicons like MPC8536 and P1022 have a JOG feature, which provides
> a dynamic mechanism to lower or raise the CPU core clock at runtime.

Is there a reason P1023 isn't supported?

> This patch adds the support to change CPU frequency using the standard
> cpufreq interface. The ratio CORE to CCB can be 1:1(except MPC8536), 3:2,
> 2:1, 5:2, 3:1, 7:2 and 4:1.
> 
> Two CPU cores on P1022 must not in the low power state during the frequency
> transition. The driver uses a atomic counter to meet the requirement.
> 
> The jog mode frequency transition process on the MPC8536 is similar to
> the deep sleep process. The driver need save the CPU state and restore
> it after CPU warm reset.
> 
> Note:
>  * The I/O peripherals such as PCIe and eTSEC may lose packets during
>the jog mode frequency transition.

That might be acceptable for eTSEC, but it is not acceptable to lose
anything on PCIe.  Especially not if you're going to make this "default y".


> +static int mpc85xx_job_probe(struct platform_device *ofdev)

Job?

> +{
> + struct device_node *np = ofdev->dev.of_node;
> + unsigned int svr;
> +
> + if (of_device_is_compatible(np, "fsl,mpc8536-guts")) {
> + svr = mfspr(SPRN_SVR);
> + if ((svr & 0x7fff) == 0x10) {
> + pr_err("MPC8536 Rev 1.0 do not support JOG.\n");
> + return -ENODEV;
> + }

s/do not support JOG/does not support cpufreq/

> + mpc85xx_freqs = mpc8536_freqs_table;
> + set_pll = mpc8536_set_pll;
> + } else if (of_device_is_compatible(np, "fsl,p1022-guts")) {
> + mpc85xx_freqs = p1022_freqs_table;
> + set_pll = p1022_set_pll;
> + } else {
> + return -ENODEV;
> + }
> +
> + sysfreq = fsl_get_sys_freq();
> +
> + guts = of_iomap(np, 0);
> + if (!guts)
> + return -ENODEV;
> +
> + max_pll[0] = get_pll(0);
> + if (mpc85xx_freqs == p1022_freqs_table)
> + max_pll[1] = get_pll(1);
> +
> + pr_info("Freescale MPC85xx CPU frequency switching(JOG) driver\n");
> +
> + return cpufreq_register_driver(&mpc85xx_cpufreq_driver);
> +}
> +
> +static int mpc85xx_jog_remove(struct platform_device *ofdev)
> +{
> + iounmap(guts);
> + cpufreq_unregister_driver(&mpc85xx_cpufreq_driver);
> +
> + return 0;
> +}
> +
> +static struct of_device_id mpc85xx_jog_ids[] = {
> + { .compatible = "fsl,mpc8536-guts", },
> + { .compatible = "fsl,p1022-guts", },
> + {}
> +};
> +
> +static struct platform_driver mpc85xx_jog_driver = {
> + .driver = {
> + .name = "mpc85xx_cpufreq_jog",
> + .owner = THIS_MODULE,
> + .of_match_table = mpc85xx_jog_ids,
> + },
> + .probe = mpc85xx_job_probe,
> + .remove = mpc85xx_jog_remove,
> +};

Why is this a separate driver from fsl_pmc.c?

Only one driver can bind to a node through normal mechanisms -- you
don't get to take the entire guts node for this.

> +static int __init mpc85xx_jog_init(void)
> +{
> + return platform_driver_register(&mpc85xx_jog_driver);
> +}
> +
> +static void __exit mpc85xx_jog_exit(void)
> +{
> + platform_driver_unregister(&mpc85xx_jog_driver);
> +}
> +
> +module_init(mpc85xx_jog_init);
> +module_exit(mpc85xx_jog_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Dave Liu ");
> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
> index a35ca44..445bedd 100644
> --- a/arch/powerpc/platforms/Kconfig
> +++ b/arch/powerpc/platforms/Kconfig
> @@ -204,6 +204,17 @@ config CPU_FREQ_PMAC64
> This adds support for frequency switching on Apple iMac G5,
> and some of the more recent desktop G5 machines as well.
>  
> +config MPC85xx_CPUFREQ
> + bool "Support for Freescale MPC85xx CPU freq"
> + depends on PPC_85xx && PPC32 && !PPC_E500MC

PPC32 is redundant given the !PPC_E500MC.

> index 8976534..401cac0 100644
> --- a/arch/powerpc/sysdev/fsl_soc.h
> +++ b/arch/powerpc/sysdev/fsl_soc.h
> @@ -62,5 +62,10 @@ void fsl_hv_halt(void);
>   * code can be compatible with both 32-bit & 36-bit.
>   */
>  extern void mpc85xx_enter_deep_sleep(u64 ccsrbar, u32 powmgtreq);
> +
> +static inline void mpc85xx_enter_jog(u64 ccsrbar, u32 powmgtreq)
> +{
> + mpc85xx_enter_deep_sleep(ccsrbar, powmgtreq);
> +}

What value is this function adding over mpc85xx_enter_deep_sleep()?

> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 2060c6e..c4cd342 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -381,6 +381,36 @@ out:
>  }
>  EXPORT_SYMBOL_GPL(cpu_up);
>  
> +/*
> + * Prevent regular CPU hotplug from racing with the freezer, by disabling CPU
> + * hotplug when tasks are about to be frozen. Also, don't allow the freezer
> + * to continue until any currently running CPU hotplug operation gets
> + * completed.
> + * To modify the 'cpu_hotplug_disabled' flag, we need to acquire the
> + * 'cpu_add_remove_lock'. And

Serial RAPID IO kernel hang on maintenance read transaction

2012-06-01 Thread Proicou, Mike

I've been struggling with a kernel hang during bootup + enumeration of a Rapid 
IO system.

My current system contains a N.A.T MCH (using the IDT/Tundra Tsi 578 switch) 
and a Vadatech AMC719 card using the Freescale P4080 processor.  There will be 
other cards added to the system, but I'm testing with just this for now.

I'm using a Linux kernel version 2.6.34.6.  I've set riohdid=0 on the kernel 
command line, and I'm expecting Linux to fully enumerate and configure the 
Rapid IO fabric. (This may be a bad assumption on my part.)

After lots of tracing, I've determined that the kernel is hanging on the first 
maintenance transaction to the switch.  The hang will often be followed by a 
"machine check in kernel mode" exception and panic.

 This is very similar to the behavior reported in this mailing list  thread 
from 2010: 
http://lists.ozlabs.org/pipermail/linuxppc-dev/2010-October/086235.html  I've 
read that thread several times and tries most of the suggestions, but they 
don't appear to apply in my hardware configuration.linu



Is it possible that something in the switch isn't completely initialized at the 
time that Linux tries to do the maintenance transaction?  If so, how do I find 
it?

Here's the console  log for a bootup using the supplied kernel:

Freescale XGMAC MDIO Bus: probed
Setting up RapidIO peer-to-peer network /rapidio@ffe0c
fsl-of-rio ffe0c.rapidio: Of-device full name /rapidio@ffe0c
fsl-of-rio ffe0c.rapidio: Regs: [mem 0xffe0c-0xffe0d]
fsl-of-rio ffe0c.rapidio: LAW start 0x000c2000, size 
0x0100.
fsl-of-rio ffe0c.rapidio: errirq: 16, bellirq: 57, txirq: 60, rxirq 61
fsl-of-rio ffe0c.rapidio: RapidIO PHY type: serial
SRIO Port 1 Status: Lane0Sync Lane1Sync Lane2Sync Lane3Sync Aligned
SRIO Port 2 Status: (Note: Freescale driver only supports Port 1)
fsl-of-rio ffe0c.rapidio: Hardware port width: 4
fsl-of-rio ffe0c.rapidio: Training connection status: Four-lane
fsl-of-rio ffe0c.rapidio: RapidIO Common Transport System size: 256
RIO: enumerate master port 0, RIO0 mport
Machine check in kernel mode.
RIO: port1 error
Caused by (from MCSR=a000): Load Error Report
Guarded Load Error Report
Oops: Machine check, sig: 7 [#1]
SMP NR_CPUS=8 amc718_based
last sysfs file:
Modules linked in:
NIP: c001a460 LR: c01ee41c CTR: c001a420
REGS: effc9f10 TRAP: 0204   Not tainted  (2.6.34.6-vt3-svn36835)
MSR: 00021002   CR: 24022024  XER: 
TASK = ebc68000[1] 'swapper' THREAD: ebc62000 CPU: 6
GPR00: f120 ebc63d10 ebc68000   3fc0 3fc0 f1200068
GPR08: 0004 ebc63d18 f1190c20 eb53 24022022 d811c00a  
GPR16:  7ffe2a00   7fff0df0   
GPR24: 0081 00ff  ebd89400 0068 00029002 c05e8914 ebc63d58
NIP [c001a460] fsl_rio_config_read+0x40/0x78
LR [c01ee41c] rio_mport_read_config_32+0x7c/0xac
Call Trace:
[ebc63d50] [c01eed64] rio_get_host_deviceid_lock+0x3c/0x50
[ebc63d70] [c045acd4] rio_enum_peer+0x28/0x3e4
[ebc63dd0] [c045b178] rio_enum_mport+0xe8/0x244
[ebc63e10] [c045a59c] rio_init_mports+0x90/0xe4
[ebc63e30] [c0457a5c] fsl_of_rio_rpn_probe+0x3c/0x50
[ebc63e40] [c034abe4] of_platform_device_probe+0x58/0x98
[ebc63e60] [c02274d8] driver_probe_device+0xa4/0x1b4
[ebc63e80] [c02260cc] bus_for_each_drv+0x6c/0xa8
[ebc63eb0] [c022735c] device_attach+0xa4/0xc8
[ebc63ed0] [c0226afc] bus_probe_device+0x2c/0x44
[ebc63ee0] [c02245f8] device_add+0x460/0x5a8
[ebc63f30] [c034a750] of_device_register+0x34/0x48
[ebc63f40] [c0008d64] of_platform_device_create+0x44/0x74
[ebc63f50] [c0008f90] of_platform_bus_probe+0x130/0x15c
[ebc63f70] [c0565480] declare_of_platform_devices+0x24/0x140
[ebc63f90] [c05651cc] 
__machine_initcall_amc718_based_declare_of_platform_devices+0x2c/0x3c
[ebc63fa0] [c0001cb8] do_one_initcall+0x3c/0x1d0
[ebc63fd0] [c055e9b0] kernel_init+0x190/0x230
[ebc63ff0] [c000f284] kernel_thread+0x4c/0x68
Instruction dump:
814b000c 54e0ba7e 7cc60378 7c0004ac 90ca 2f880001 800b0018 7ce03a14
419e0020 2f880002 419e002c 3860 <80e7> 7c2006ac 90e9 4e800020
---[ end trace 561bb236c800851f ]---
Kernel panic - not syncing: Attempted to kill init!
Call Trace:
Rebooting in 180 seconds..

Here's a partial log with some additional output and a dump of the error 
registers at the time of failure:



fsl-elo-dma ffe101300.dma: request channel 0 IRQ
fsl-elo-dma ffe101300.dma: request channel 1 IRQ
fsl-elo-dma ffe101300.dma: request channel 2 IRQ
fsl-elo-dma ffe101300.dma: request channel 3 IRQ
Freescale PowerQUICC MII Bus: probed
Freescale XGMAC MDIO Bus: probed
fsl-of-rio ffe0c.rapidio: Setting up RapidIO peer-to-peer network 
/rapidio@ffe0c
fsl-of-rio ffe0c.rapidio: Of-device full name /rapidio@ffe0c
fsl-of-rio ffe0c.rapidio: Regs: [mem 0xffe0c-0xffe0d]
fsl-of-rio ffe0c.rapidio: LAW start 0x000c2000, size 
0x0100
fsl-of-rio ffe0c.rapidio: get_immrbase() ffe00
fsl-of-rio ffe

Re: [PATCH 18/27] powerpc, smpboot: Use generic SMP booting infrastructure

2012-06-01 Thread Paul Mackerras
On Fri, Jun 01, 2012 at 02:44:23PM +0530, Srivatsa S. Bhat wrote:
> From: Nikunj A. Dadhania 
> 
> Convert powerpc to use the generic framework to boot secondary CPUs.
> 
> Signed-off-by: Nikunj A. Dadhania 

Acked-by: Paul Mackerras 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev