Patchwork powerpc/86xx: clean up smp init code

login
register
mail settings
Submitter Kumar Gala
Date April 22, 2009, 5:34 p.m.
Message ID <1240421691-10193-1-git-send-email-galak@kernel.crashing.org>
Download mbox | patch
Permalink /patch/26327/
State Accepted
Commit 1e76dff22ce45bc8b869a9956b24a77877915364
Delegated to: Kumar Gala
Headers show

Comments

Kumar Gala - April 22, 2009, 5:34 p.m.
Removed the need for asm/mpc86xx.h as it was only used in mpc86xx_smp.c
and just moved the defines it cared about into there.  Also fixed up
the ioremap to only map the one 4k page we need access to and to iounmap
when we are done.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/include/asm/mpc86xx.h         |   33 ----------------------------
 arch/powerpc/platforms/86xx/gef_ppc9a.c    |    1 -
 arch/powerpc/platforms/86xx/gef_sbc310.c   |    1 -
 arch/powerpc/platforms/86xx/gef_sbc610.c   |    1 -
 arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    1 -
 arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |    1 -
 arch/powerpc/platforms/86xx/mpc86xx_smp.c  |    8 ++++++-
 arch/powerpc/platforms/86xx/sbc8641d.c     |    1 -
 8 files changed, 7 insertions(+), 40 deletions(-)
 delete mode 100644 arch/powerpc/include/asm/mpc86xx.h
Martyn Welch - April 23, 2009, 12:54 p.m.
Kumar Gala wrote:
> Removed the need for asm/mpc86xx.h as it was only used in mpc86xx_smp.c
> and just moved the defines it cared about into there.  Also fixed up
> the ioremap to only map the one 4k page we need access to and to iounmap
> when we are done.
>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
>  arch/powerpc/include/asm/mpc86xx.h         |   33 ----------------------------
>  arch/powerpc/platforms/86xx/gef_ppc9a.c    |    1 -
>  arch/powerpc/platforms/86xx/gef_sbc310.c   |    1 -
>  arch/powerpc/platforms/86xx/gef_sbc610.c   |    1 -
>  arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    1 -
>  arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |    1 -
>  arch/powerpc/platforms/86xx/mpc86xx_smp.c  |    8 ++++++-
>  arch/powerpc/platforms/86xx/sbc8641d.c     |    1 -
>  8 files changed, 7 insertions(+), 40 deletions(-)
>  delete mode 100644 arch/powerpc/include/asm/mpc86xx.h
>
>   
I assume this patch relies on one of the other patches posted?

Just applying this patch to my development tree (based on your main 
branch) resulted in the following on a PPC9A:

mpic: requesting IPIs ...
__ioremap(): phys addr 0x0 is RAM lr c041e5c8
Unable to handle kernel paging request for data at address 0x00000010
Faulting instruction address: 0xc041e5cc
Oops: Kernel access of bad area, sig: 11 [#1]
PREEMPT SMP NR_CPUS=2 GE Fanuc PPC9A
Modules linked in:
NIP: c041e5cc LR: c041e5c8 CTR: c0013d90
REGS: ef841ea0 TRAP: 0300   Not tainted  (2.6.30-rc3-00016-gabae74f)
MSR: 00001032 <ME,IR,DR>  CR: 24000022  XER: 00000000
DAR: 00000010, DSISR: 40000000
TASK = ef83f980[1] 'swapper' THREAD: ef840000 CPU: 0
GPR00: c041e5c8 ef841f50 ef83f980 00000000 00001032 ffffffff c0480000 
00004000
GPR08: c0441a4c 00000000 ef840000 c0440000 22000042 ffffdfff 0ff50d00 
00000001
GPR16: ffffffff 00000000 c0440000 c0480000 c0480000 c0468000 c0440000 
c0442838
GPR24: 00000002 c0480000 c0480000 7d5043a6 00009032 00000004 00000001 
0000c350
NIP [c041e5cc] smp_86xx_kick_cpu+0x70/0x11c
LR [c041e5c8] smp_86xx_kick_cpu+0x6c/0x11c
Call Trace:
[ef841f50] [c041e5c8] smp_86xx_kick_cpu+0x6c/0x11c (unreliable)

[ef841f70] [c0435010] __cpu_up+0xa4/0x1b0
[ef841f90] [c04355ec] cpu_up+0x104/0x1cc
[ef841fd0] [c0412368] kernel_init+0x1d8/0x1f0
[ef841ff0] [c0012cb8] kernel_thread+0x4c/0x68
Instruction dump:
3c80c000 61290100 38a00001 7d234b78 38843464 83690000 4bbfa7f9 4bbfcb21
38801000 38631000 4bbf91ad 7c0004ac <81230010> 0c090000 4c00012c 38000001
---[ end trace 31fd0ba7d8756001 ]---
Kernel panic - not syncing: Attempted to kill init!
Rebooting in 180 seconds..


Martyn
Kumar Gala - April 23, 2009, 6:42 p.m.
On Apr 23, 2009, at 7:54 AM, Martyn Welch wrote:

> Kumar Gala wrote:
>> Removed the need for asm/mpc86xx.h as it was only used in  
>> mpc86xx_smp.c
>> and just moved the defines it cared about into there.  Also fixed up
>> the ioremap to only map the one 4k page we need access to and to  
>> iounmap
>> when we are done.
>>
>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>> ---
>> arch/powerpc/include/asm/mpc86xx.h         |   33  
>> ----------------------------
>> arch/powerpc/platforms/86xx/gef_ppc9a.c    |    1 -
>> arch/powerpc/platforms/86xx/gef_sbc310.c   |    1 -
>> arch/powerpc/platforms/86xx/gef_sbc610.c   |    1 -
>> arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    1 -
>> arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |    1 -
>> arch/powerpc/platforms/86xx/mpc86xx_smp.c  |    8 ++++++-
>> arch/powerpc/platforms/86xx/sbc8641d.c     |    1 -
>> 8 files changed, 7 insertions(+), 40 deletions(-)
>> delete mode 100644 arch/powerpc/include/asm/mpc86xx.h
>>
>>
> I assume this patch relies on one of the other patches posted?
>
> Just applying this patch to my development tree (based on your main  
> branch) resulted in the following on a PPC9A:

Darn, this should have been a simple change.  I'll try it out on  
mpc8641 to see if its happening there.

- k
Kumar Gala - April 27, 2009, 1:59 p.m.
On Apr 23, 2009, at 7:54 AM, Martyn Welch wrote:

> Kumar Gala wrote:
>> Removed the need for asm/mpc86xx.h as it was only used in  
>> mpc86xx_smp.c
>> and just moved the defines it cared about into there.  Also fixed up
>> the ioremap to only map the one 4k page we need access to and to  
>> iounmap
>> when we are done.
>>
>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>> ---
>> arch/powerpc/include/asm/mpc86xx.h         |   33  
>> ----------------------------
>> arch/powerpc/platforms/86xx/gef_ppc9a.c    |    1 -
>> arch/powerpc/platforms/86xx/gef_sbc310.c   |    1 -
>> arch/powerpc/platforms/86xx/gef_sbc610.c   |    1 -
>> arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    1 -
>> arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |    1 -
>> arch/powerpc/platforms/86xx/mpc86xx_smp.c  |    8 ++++++-
>> arch/powerpc/platforms/86xx/sbc8641d.c     |    1 -
>> 8 files changed, 7 insertions(+), 40 deletions(-)
>> delete mode 100644 arch/powerpc/include/asm/mpc86xx.h
>>
>>
> I assume this patch relies on one of the other patches posted?
>
> Just applying this patch to my development tree (based on your main  
> branch) resulted in the following on a PPC9A:
>
> mpic: requesting IPIs ...
> __ioremap(): phys addr 0x0 is RAM lr c041e5c8
> Unable to handle kernel paging request for data at address 0x00000010
> Faulting instruction address: 0xc041e5cc
> Oops: Kernel access of bad area, sig: 11 [#1]
> PREEMPT SMP NR_CPUS=2 GE Fanuc PPC9A
> Modules linked in:
> NIP: c041e5cc LR: c041e5c8 CTR: c0013d90
> REGS: ef841ea0 TRAP: 0300   Not tainted  (2.6.30-rc3-00016-gabae74f)
> MSR: 00001032 <ME,IR,DR>  CR: 24000022  XER: 00000000
> DAR: 00000010, DSISR: 40000000
> TASK = ef83f980[1] 'swapper' THREAD: ef840000 CPU: 0
> GPR00: c041e5c8 ef841f50 ef83f980 00000000 00001032 ffffffff  
> c0480000 00004000
> GPR08: c0441a4c 00000000 ef840000 c0440000 22000042 ffffdfff  
> 0ff50d00 00000001
> GPR16: ffffffff 00000000 c0440000 c0480000 c0480000 c0468000  
> c0440000 c0442838
> GPR24: 00000002 c0480000 c0480000 7d5043a6 00009032 00000004  
> 00000001 0000c350
> NIP [c041e5cc] smp_86xx_kick_cpu+0x70/0x11c
> LR [c041e5c8] smp_86xx_kick_cpu+0x6c/0x11c
> Call Trace:
> [ef841f50] [c041e5c8] smp_86xx_kick_cpu+0x6c/0x11c (unreliable)
>
> [ef841f70] [c0435010] __cpu_up+0xa4/0x1b0
> [ef841f90] [c04355ec] cpu_up+0x104/0x1cc
> [ef841fd0] [c0412368] kernel_init+0x1d8/0x1f0
> [ef841ff0] [c0012cb8] kernel_thread+0x4c/0x68
> Instruction dump:
> 3c80c000 61290100 38a00001 7d234b78 38843464 83690000 4bbfa7f9  
> 4bbfcb21
> 38801000 38631000 4bbf91ad 7c0004ac <81230010> 0c090000 4c00012c  
> 38000001
> ---[ end trace 31fd0ba7d8756001 ]---
> Kernel panic - not syncing: Attempted to kill init!
> Rebooting in 180 seconds..

I'm not able to reproduce this failure.  It seems like either ioremap  
is returning 0 or you are getting 0 from get_immrbase().. either way I  
don't see how my change would cause what you are seeing on your board.

Are you running w/CONFIG_PHYS_64BIT=y?

- k

- k
Martyn Welch - April 27, 2009, 3:06 p.m.
Kumar Gala wrote:
>
> On Apr 23, 2009, at 7:54 AM, Martyn Welch wrote:
>
>> Kumar Gala wrote:
>>> Removed the need for asm/mpc86xx.h as it was only used in mpc86xx_smp.c
>>> and just moved the defines it cared about into there.  Also fixed up
>>> the ioremap to only map the one 4k page we need access to and to 
>>> iounmap
>>> when we are done.
>>>
>>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>>> ---
>>> arch/powerpc/include/asm/mpc86xx.h         |   33 
>>> ----------------------------
>>> arch/powerpc/platforms/86xx/gef_ppc9a.c    |    1 -
>>> arch/powerpc/platforms/86xx/gef_sbc310.c   |    1 -
>>> arch/powerpc/platforms/86xx/gef_sbc610.c   |    1 -
>>> arch/powerpc/platforms/86xx/mpc8610_hpcd.c |    1 -
>>> arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |    1 -
>>> arch/powerpc/platforms/86xx/mpc86xx_smp.c  |    8 ++++++-
>>> arch/powerpc/platforms/86xx/sbc8641d.c     |    1 -
>>> 8 files changed, 7 insertions(+), 40 deletions(-)
>>> delete mode 100644 arch/powerpc/include/asm/mpc86xx.h
>>>
>>>
>> I assume this patch relies on one of the other patches posted?
>>
>> Just applying this patch to my development tree (based on your main 
>> branch) resulted in the following on a PPC9A:
>>
>> mpic: requesting IPIs ...
>> __ioremap(): phys addr 0x0 is RAM lr c041e5c8
>> Unable to handle kernel paging request for data at address 0x00000010
>> Faulting instruction address: 0xc041e5cc
>> Oops: Kernel access of bad area, sig: 11 [#1]
>> PREEMPT SMP NR_CPUS=2 GE Fanuc PPC9A
>> Modules linked in:
>> NIP: c041e5cc LR: c041e5c8 CTR: c0013d90
>> REGS: ef841ea0 TRAP: 0300   Not tainted  (2.6.30-rc3-00016-gabae74f)
>> MSR: 00001032 <ME,IR,DR>  CR: 24000022  XER: 00000000
>> DAR: 00000010, DSISR: 40000000
>> TASK = ef83f980[1] 'swapper' THREAD: ef840000 CPU: 0
>> GPR00: c041e5c8 ef841f50 ef83f980 00000000 00001032 ffffffff c0480000 
>> 00004000
>> GPR08: c0441a4c 00000000 ef840000 c0440000 22000042 ffffdfff 0ff50d00 
>> 00000001
>> GPR16: ffffffff 00000000 c0440000 c0480000 c0480000 c0468000 c0440000 
>> c0442838
>> GPR24: 00000002 c0480000 c0480000 7d5043a6 00009032 00000004 00000001 
>> 0000c350
>> NIP [c041e5cc] smp_86xx_kick_cpu+0x70/0x11c
>> LR [c041e5c8] smp_86xx_kick_cpu+0x6c/0x11c
>> Call Trace:
>> [ef841f50] [c041e5c8] smp_86xx_kick_cpu+0x6c/0x11c (unreliable)
>>
>> [ef841f70] [c0435010] __cpu_up+0xa4/0x1b0
>> [ef841f90] [c04355ec] cpu_up+0x104/0x1cc
>> [ef841fd0] [c0412368] kernel_init+0x1d8/0x1f0
>> [ef841ff0] [c0012cb8] kernel_thread+0x4c/0x68
>> Instruction dump:
>> 3c80c000 61290100 38a00001 7d234b78 38843464 83690000 4bbfa7f9 4bbfcb21
>> 38801000 38631000 4bbf91ad 7c0004ac <81230010> 0c090000 4c00012c 
>> 38000001
>> ---[ end trace 31fd0ba7d8756001 ]---
>> Kernel panic - not syncing: Attempted to kill init!
>> Rebooting in 180 seconds..
>
> I'm not able to reproduce this failure.  It seems like either ioremap 
> is returning 0 or you are getting 0 from get_immrbase().. either way I 
> don't see how my change would cause what you are seeing on your board.
I've just built the kernel with no local patches in case they were 
causing the problem and adding this patch causes the above problem.
> Are you running w/CONFIG_PHYS_64BIT=y?
I am using the config in the kernel 
("arch/powerpc/configs/68xx/gef_ppc9a_defconfig") as is, ditto for the DTS.

CONFIG_PHYS_64BIT is not set.

However, looking into it a bit further 'device_type = "soc";' is missing 
from the DTS file, so I assume get_immrbase() is returning "-1".

That might explain some other weird errors I recently noticed that I 
haven't managed to find the time to track down yet...

Martyn
Kumar Gala - April 27, 2009, 3:51 p.m.
On Apr 27, 2009, at 10:06 AM, Martyn Welch wrote:

> I am using the config in the kernel ("arch/powerpc/configs/68xx/ 
> gef_ppc9a_defconfig") as is, ditto for the DTS.
>
> CONFIG_PHYS_64BIT is not set.
>
> However, looking into it a bit further 'device_type = "soc";' is  
> missing from the DTS file, so I assume get_immrbase() is returning  
> "-1".
>
> That might explain some other weird errors I recently noticed that I  
> haven't managed to find the time to track down yet...

I'm amazed this ever worked.  If you want to send me a patch for  
the .dts to add device_type = "soc" for your various .dts I'll get  
that into 2.6.30

- k

Patch

diff --git a/arch/powerpc/include/asm/mpc86xx.h b/arch/powerpc/include/asm/mpc86xx.h
deleted file mode 100644
index 15f650f..0000000
--- a/arch/powerpc/include/asm/mpc86xx.h
+++ /dev/null
@@ -1,33 +0,0 @@ 
-/*
- * MPC86xx definitions
- *
- * Author: Jeff Brown
- *
- * Copyright 2004 Freescale Semiconductor, Inc
- *
- * This program is free software; you can redistribute  it and/or modify it
- * under  the terms of  the GNU General  Public License as published by the
- * Free Software Foundation;  either version 2 of the  License, or (at your
- * option) any later version.
- */
-
-#ifdef __KERNEL__
-#ifndef __ASM_POWERPC_MPC86xx_H__
-#define __ASM_POWERPC_MPC86xx_H__
-
-#include <asm/mmu.h>
-
-#ifdef CONFIG_PPC_86xx
-
-#define CPU0_BOOT_RELEASE 0x01000000
-#define CPU1_BOOT_RELEASE 0x02000000
-#define CPU_ALL_RELEASED (CPU0_BOOT_RELEASE | CPU1_BOOT_RELEASE)
-#define MCM_PORT_CONFIG_OFFSET 0x1010
-
-/* Offset from CCSRBAR */
-#define MPC86xx_MCM_OFFSET      (0x00000)
-#define MPC86xx_MCM_SIZE        (0x02000)
-
-#endif /* CONFIG_PPC_86xx */
-#endif /* __ASM_POWERPC_MPC86xx_H__ */
-#endif /* __KERNEL__ */
diff --git a/arch/powerpc/platforms/86xx/gef_ppc9a.c b/arch/powerpc/platforms/86xx/gef_ppc9a.c
index d791046..2efa052 100644
--- a/arch/powerpc/platforms/86xx/gef_ppc9a.c
+++ b/arch/powerpc/platforms/86xx/gef_ppc9a.c
@@ -28,7 +28,6 @@ 
 #include <asm/time.h>
 #include <asm/machdep.h>
 #include <asm/pci-bridge.h>
-#include <asm/mpc86xx.h>
 #include <asm/prom.h>
 #include <mm/mmu_decl.h>
 #include <asm/udbg.h>
diff --git a/arch/powerpc/platforms/86xx/gef_sbc310.c b/arch/powerpc/platforms/86xx/gef_sbc310.c
index af14f85..90754e7 100644
--- a/arch/powerpc/platforms/86xx/gef_sbc310.c
+++ b/arch/powerpc/platforms/86xx/gef_sbc310.c
@@ -28,7 +28,6 @@ 
 #include <asm/time.h>
 #include <asm/machdep.h>
 #include <asm/pci-bridge.h>
-#include <asm/mpc86xx.h>
 #include <asm/prom.h>
 #include <mm/mmu_decl.h>
 #include <asm/udbg.h>
diff --git a/arch/powerpc/platforms/86xx/gef_sbc610.c b/arch/powerpc/platforms/86xx/gef_sbc610.c
index ea23606..72b31a6 100644
--- a/arch/powerpc/platforms/86xx/gef_sbc610.c
+++ b/arch/powerpc/platforms/86xx/gef_sbc610.c
@@ -28,7 +28,6 @@ 
 #include <asm/time.h>
 #include <asm/machdep.h>
 #include <asm/pci-bridge.h>
-#include <asm/mpc86xx.h>
 #include <asm/prom.h>
 #include <mm/mmu_decl.h>
 #include <asm/udbg.h>
diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
index 3f49a6f..51eec0c 100644
--- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
+++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c
@@ -28,7 +28,6 @@ 
 #include <asm/time.h>
 #include <asm/machdep.h>
 #include <asm/pci-bridge.h>
-#include <asm/mpc86xx.h>
 #include <asm/prom.h>
 #include <mm/mmu_decl.h>
 #include <asm/udbg.h>
diff --git a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
index c4ec49b..7e9e83c 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_hpcn.c
@@ -24,7 +24,6 @@ 
 #include <asm/time.h>
 #include <asm/machdep.h>
 #include <asm/pci-bridge.h>
-#include <asm/mpc86xx.h>
 #include <asm/prom.h>
 #include <mm/mmu_decl.h>
 #include <asm/udbg.h>
diff --git a/arch/powerpc/platforms/86xx/mpc86xx_smp.c b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
index 014e26c..d84bbb5 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_smp.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
@@ -20,7 +20,6 @@ 
 #include <asm/pgtable.h>
 #include <asm/pci-bridge.h>
 #include <asm/mpic.h>
-#include <asm/mpc86xx.h>
 #include <asm/cacheflush.h>
 
 #include <sysdev/fsl_soc.h>
@@ -30,6 +29,11 @@ 
 extern void __secondary_start_mpc86xx(void);
 extern unsigned long __secondary_hold_acknowledge;
 
+#define MCM_PORT_CONFIG_OFFSET	0x10
+
+/* Offset from CCSRBAR */
+#define MPC86xx_MCM_OFFSET      (0x1000)
+#define MPC86xx_MCM_SIZE        (0x1000)
 
 static void __init
 smp_86xx_release_core(int nr)
@@ -48,6 +52,8 @@  smp_86xx_release_core(int nr)
 	pcr = in_be32(mcm_vaddr + (MCM_PORT_CONFIG_OFFSET >> 2));
 	pcr |= 1 << (nr + 24);
 	out_be32(mcm_vaddr + (MCM_PORT_CONFIG_OFFSET >> 2), pcr);
+
+	iounmap(mcm_vaddr);
 }
 
 
diff --git a/arch/powerpc/platforms/86xx/sbc8641d.c b/arch/powerpc/platforms/86xx/sbc8641d.c
index 2886a36..51c8f33 100644
--- a/arch/powerpc/platforms/86xx/sbc8641d.c
+++ b/arch/powerpc/platforms/86xx/sbc8641d.c
@@ -25,7 +25,6 @@ 
 #include <asm/time.h>
 #include <asm/machdep.h>
 #include <asm/pci-bridge.h>
-#include <asm/mpc86xx.h>
 #include <asm/prom.h>
 #include <mm/mmu_decl.h>
 #include <asm/udbg.h>