| Message ID | 1359651943-21752-5-git-send-email-dinguyen@altera.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Thu, Jan 31, 2013 at 11:05:43AM -0600, dinguyen@altera.com wrote: > From: Dinh Nguyen <dinguyen@altera.com> > > Because the CPU1 start address is different for socfpga-vt and > socfpga-cyclone5, we add code to use the correct CPU1 start addr. > > Signed-off-by: Dinh Nguyen <dinguyen@altera.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Olof Johansson <olof@lixom.net> > Cc: Pavel Machek <pavel@denx.de> > --- > .../bindings/arm/altera/socfpga-system.txt | 2 ++ > arch/arm/boot/dts/socfpga_cyclone5.dts | 4 ++++ > arch/arm/boot/dts/socfpga_vt.dts | 4 ++++ > arch/arm/mach-socfpga/core.h | 4 +++- > arch/arm/mach-socfpga/headsmp.S | 16 ++++++++++++---- > arch/arm/mach-socfpga/platsmp.c | 3 ++- > arch/arm/mach-socfpga/socfpga.c | 8 ++++++++ > 7 files changed, 35 insertions(+), 6 deletions(-) > @@ -72,6 +73,13 @@ void __init socfpga_sysmgr_init(void) > struct device_node *np; > > np = of_find_compatible_node(NULL, NULL, "altr,sys-mgr"); > + > + if (of_property_read_u32(np, "cpu1-start-addr", > + (u32 *) &cpu1start_addr)) { > + early_printk("Need cpu1-start-addr in device tree.\n"); > + panic("Need cpu1-start-addr in device tree.\n"); > + } > + > sys_manager_base_addr = of_iomap(np, 0); Wouldn't it be easier to diagnose this failure if you just printed the error and continued booting without the second CPU? An early panic is usually really hard to debug since you might not get early console without extra work. -Olof
Hi! > > Because the CPU1 start address is different for socfpga-vt and > > socfpga-cyclone5, we add code to use the correct CPU1 start addr. > > @@ -72,6 +73,13 @@ void __init socfpga_sysmgr_init(void) > > struct device_node *np; > > > > np = of_find_compatible_node(NULL, NULL, "altr,sys-mgr"); > > + > > + if (of_property_read_u32(np, "cpu1-start-addr", > > + (u32 *) &cpu1start_addr)) { > > + early_printk("Need cpu1-start-addr in device tree.\n"); > > + panic("Need cpu1-start-addr in device tree.\n"); > > + } > > + > > sys_manager_base_addr = of_iomap(np, 0); > > Wouldn't it be easier to diagnose this failure if you just printed the error > and continued booting without the second CPU? An early panic is usually really > hard to debug since you might not get early console without extra work. I actually thought about that... but could not think of non-ugly way of doing that. I hope dts will normally be "right" for any production system... Pavel
On Thu 2013-01-31 11:05:43, dinguyen@altera.com wrote: > From: Dinh Nguyen <dinguyen@altera.com> > > Because the CPU1 start address is different for socfpga-vt and > socfpga-cyclone5, we add code to use the correct CPU1 start addr. > > Signed-off-by: Dinh Nguyen <dinguyen@altera.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Olof Johansson <olof@lixom.net> > Cc: Pavel Machek <pavel@denx.de> Signed-off-by: Pavel Machek <pavel@denx.de>
Hi Olof, On Fri, 2013-02-01 at 11:46 +0100, ZY - pavel wrote: > Hi! > > > > Because the CPU1 start address is different for socfpga-vt and > > > socfpga-cyclone5, we add code to use the correct CPU1 start addr. > > > > @@ -72,6 +73,13 @@ void __init socfpga_sysmgr_init(void) > > > struct device_node *np; > > > > > > np = of_find_compatible_node(NULL, NULL, "altr,sys-mgr"); > > > + > > > + if (of_property_read_u32(np, "cpu1-start-addr", > > > + (u32 *) &cpu1start_addr)) { > > > + early_printk("Need cpu1-start-addr in device tree.\n"); > > > + panic("Need cpu1-start-addr in device tree.\n"); > > > + } > > > + > > > sys_manager_base_addr = of_iomap(np, 0); > > > > Wouldn't it be easier to diagnose this failure if you just printed the error > > and continued booting without the second CPU? An early panic is usually really > > hard to debug since you might not get early console without extra work. > > I actually thought about that... but could not think of non-ugly way > of doing that. I hope dts will normally be "right" for any production > system... I think a panic is better just for the reason that if someone is expecting SMP, but missed the warning message, and later finds out that the secondary core never came up, it would save some debugging time. Since I have to send out a v3 from the 1st patch anyways, let me verify that I can get the early warning. Thanks, Dinh > Pavel Confidentiality Notice. This message may contain information that is confidential or otherwise protected from disclosure. If you are not the intended recipient, you are hereby notified that any use, disclosure, dissemination, distribution, or copying of this message, or any attachments, is strictly prohibited. If you have received this message in error, please advise the sender by reply e-mail, and delete the message and any attachments. Thank you.
On Fri, Feb 01, 2013 at 07:27:46AM -0800, Dinh Nguyen wrote: > Hi Olof, > On Fri, 2013-02-01 at 11:46 +0100, ZY - pavel wrote: > > Hi! > > > > > > Because the CPU1 start address is different for socfpga-vt and > > > > socfpga-cyclone5, we add code to use the correct CPU1 start addr. > > > > > > @@ -72,6 +73,13 @@ void __init socfpga_sysmgr_init(void) > > > > struct device_node *np; > > > > > > > > np = of_find_compatible_node(NULL, NULL, "altr,sys-mgr"); > > > > + > > > > + if (of_property_read_u32(np, "cpu1-start-addr", > > > > + (u32 *) &cpu1start_addr)) { > > > > + early_printk("Need cpu1-start-addr in device tree.\n"); > > > > + panic("Need cpu1-start-addr in device tree.\n"); > > > > + } > > > > + > > > > sys_manager_base_addr = of_iomap(np, 0); > > > > > > Wouldn't it be easier to diagnose this failure if you just printed the error > > > and continued booting without the second CPU? An early panic is usually really > > > hard to debug since you might not get early console without extra work. > > > > I actually thought about that... but could not think of non-ugly way > > of doing that. I hope dts will normally be "right" for any production > > system... > > I think a panic is better just for the reason that if someone is > expecting SMP, but missed the warning message, and later finds out that > the secondary core never came up, it would save some debugging time. > > Since I have to send out a v3 from the 1st patch anyways, let me verify > that I can get the early warning. The choice is between a panic() at a point where the only way to find out is to throw in printascii() or a working printk, and ending up with an unbootable kernel, vs continuing the boot and having an almost working system which can be logged into and the messages viewed. If you have an application which relies on the second CPU coming up, why not have it verify that the second CPU came up (it's quite easy to do - there's POSIX standard libc calls to get the number of online CPUs).
On Fri, 2013-02-01 at 15:31 +0000, Russell King - ARM Linux wrote: > On Fri, Feb 01, 2013 at 07:27:46AM -0800, Dinh Nguyen wrote: > > Hi Olof, > > On Fri, 2013-02-01 at 11:46 +0100, ZY - pavel wrote: > > > Hi! > > > > > > > > Because the CPU1 start address is different for socfpga-vt and > > > > > socfpga-cyclone5, we add code to use the correct CPU1 start addr. > > > > > > > > @@ -72,6 +73,13 @@ void __init socfpga_sysmgr_init(void) > > > > > struct device_node *np; > > > > > > > > > > np = of_find_compatible_node(NULL, NULL, "altr,sys-mgr"); > > > > > + > > > > > + if (of_property_read_u32(np, "cpu1-start-addr", > > > > > + (u32 *) &cpu1start_addr)) { > > > > > + early_printk("Need cpu1-start-addr in device tree.\n"); > > > > > + panic("Need cpu1-start-addr in device tree.\n"); > > > > > + } > > > > > + > > > > > sys_manager_base_addr = of_iomap(np, 0); > > > > > > > > Wouldn't it be easier to diagnose this failure if you just printed the error > > > > and continued booting without the second CPU? An early panic is usually really > > > > hard to debug since you might not get early console without extra work. > > > > > > I actually thought about that... but could not think of non-ugly way > > > of doing that. I hope dts will normally be "right" for any production > > > system... > > > > I think a panic is better just for the reason that if someone is > > expecting SMP, but missed the warning message, and later finds out that > > the secondary core never came up, it would save some debugging time. > > > > Since I have to send out a v3 from the 1st patch anyways, let me verify > > that I can get the early warning. > > The choice is between a panic() at a point where the only way to find > out is to throw in printascii() or a working printk, and ending up with > an unbootable kernel, vs continuing the boot and having an almost > working system which can be logged into and the messages viewed. > > If you have an application which relies on the second CPU coming up, > why not have it verify that the second CPU came up (it's quite easy > to do - there's POSIX standard libc calls to get the number of online > CPUs). Point taken...thanks Russell. Dinh >
diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-system.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-system.txt index 07c65e3..f4d04a0 100644 --- a/Documentation/devicetree/bindings/arm/altera/socfpga-system.txt +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-system.txt @@ -3,9 +3,11 @@ Altera SOCFPGA System Manager Required properties: - compatible : "altr,sys-mgr" - reg : Should contain 1 register ranges(address and length) +- cpu1-start-addr : CPU1 start address in hex. Example: sysmgr@ffd08000 { compatible = "altr,sys-mgr"; reg = <0xffd08000 0x1000>; + cpu1-start-addr = <0xffd080c4>; }; diff --git a/arch/arm/boot/dts/socfpga_cyclone5.dts b/arch/arm/boot/dts/socfpga_cyclone5.dts index 7ad3cc6..3ae8a83 100644 --- a/arch/arm/boot/dts/socfpga_cyclone5.dts +++ b/arch/arm/boot/dts/socfpga_cyclone5.dts @@ -56,5 +56,9 @@ serial1@ffc03000 { clock-frequency = <100000000>; }; + + sysmgr@ffd08000 { + cpu1-start-addr = <0xffd080c4>; + }; }; }; diff --git a/arch/arm/boot/dts/socfpga_vt.dts b/arch/arm/boot/dts/socfpga_vt.dts index a0c6c65..1036eba 100644 --- a/arch/arm/boot/dts/socfpga_vt.dts +++ b/arch/arm/boot/dts/socfpga_vt.dts @@ -56,5 +56,9 @@ serial1@ffc03000 { clock-frequency = <7372800>; }; + + sysmgr@ffd08000 { + cpu1-start-addr = <0xffd08010>; + }; }; }; diff --git a/arch/arm/mach-socfpga/core.h b/arch/arm/mach-socfpga/core.h index 9941caa..5b76dd4 100644 --- a/arch/arm/mach-socfpga/core.h +++ b/arch/arm/mach-socfpga/core.h @@ -20,7 +20,7 @@ #ifndef __MACH_CORE_H #define __MACH_CORE_H -extern void secondary_startup(void); +extern void v7_secondary_startup(void); extern void __iomem *socfpga_scu_base_addr; extern void socfpga_init_clocks(void); @@ -29,6 +29,8 @@ extern void socfpga_sysmgr_init(void); extern struct smp_operations socfpga_smp_ops; extern char secondary_trampoline, secondary_trampoline_end; +extern unsigned long cpu1start_addr; + #define SOCFPGA_SCU_VIRT_BASE 0xfffec000 #endif diff --git a/arch/arm/mach-socfpga/headsmp.S b/arch/arm/mach-socfpga/headsmp.S index f09b128..3c83582 100644 --- a/arch/arm/mach-socfpga/headsmp.S +++ b/arch/arm/mach-socfpga/headsmp.S @@ -13,13 +13,21 @@ __CPUINIT .arch armv7-a -#define CPU1_START_ADDR 0xffd08010 - ENTRY(secondary_trampoline) - movw r0, #:lower16:CPU1_START_ADDR - movt r0, #:upper16:CPU1_START_ADDR + movw r2, #:lower16:cpu1start_addr + movt r2, #:upper16:cpu1start_addr + + /* The socfpga VT cannot handle a 0xC0000000 page offset when loading + the cpu1start_addr, we bit clear it. Tested on HW and VT. */ + bic r2, r2, #0x40000000 + ldr r0, [r2] ldr r1, [r0] bx r1 ENTRY(secondary_trampoline_end) + +ENTRY(v7_secondary_startup) + bl v7_invalidate_l1 + b secondary_startup +ENDPROC(v7_secondary_startup) diff --git a/arch/arm/mach-socfpga/platsmp.c b/arch/arm/mach-socfpga/platsmp.c index 68dd1b6..c428519 100644 --- a/arch/arm/mach-socfpga/platsmp.c +++ b/arch/arm/mach-socfpga/platsmp.c @@ -49,7 +49,8 @@ static int __cpuinit socfpga_boot_secondary(unsigned int cpu, struct task_struct memcpy(phys_to_virt(0), &secondary_trampoline, trampoline_size); - __raw_writel(virt_to_phys(secondary_startup), (sys_manager_base_addr+0x10)); + __raw_writel(virt_to_phys(v7_secondary_startup), + (sys_manager_base_addr + (cpu1start_addr & 0x000000ff))); flush_cache_all(); smp_wmb(); diff --git a/arch/arm/mach-socfpga/socfpga.c b/arch/arm/mach-socfpga/socfpga.c index 198f491..5a56370 100644 --- a/arch/arm/mach-socfpga/socfpga.c +++ b/arch/arm/mach-socfpga/socfpga.c @@ -29,6 +29,7 @@ void __iomem *socfpga_scu_base_addr = ((void __iomem *)(SOCFPGA_SCU_VIRT_BASE)); void __iomem *sys_manager_base_addr; void __iomem *rst_manager_base_addr; +unsigned long cpu1start_addr; static struct map_desc scu_io_desc __initdata = { .virtual = SOCFPGA_SCU_VIRT_BASE, @@ -72,6 +73,13 @@ void __init socfpga_sysmgr_init(void) struct device_node *np; np = of_find_compatible_node(NULL, NULL, "altr,sys-mgr"); + + if (of_property_read_u32(np, "cpu1-start-addr", + (u32 *) &cpu1start_addr)) { + early_printk("Need cpu1-start-addr in device tree.\n"); + panic("Need cpu1-start-addr in device tree.\n"); + } + sys_manager_base_addr = of_iomap(np, 0); np = of_find_compatible_node(NULL, NULL, "altr,rst-mgr");