| Message ID | 20201130131047.2648960-10-daniel@0x0f.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: mstar: Add basic support for i2m and SMP | expand |
On Mon, Nov 30, 2020 at 2:10 PM Daniel Palmer <daniel@0x0f.com> wrote: > +#ifdef CONFIG_SMP > +static int mstarv7_boot_secondary(unsigned int cpu, struct task_struct *idle) > +{ > + struct device_node *np; > + u32 bootaddr = (u32) __pa_symbol(secondary_startup_arm); > + void __iomem *smpctrl = 0; The initialization is wrong here: it's not a pointer and the value '0' is not useful. > +struct smp_operations __initdata mstarv7_smp_ops = { > + .smp_boot_secondary = mstarv7_boot_secondary, > +}; > +#endif So no hotplug operations? > @@ -78,4 +125,7 @@ static void __init mstarv7_init(void) > DT_MACHINE_START(MSTARV7_DT, "MStar/Sigmastar Armv7 (Device Tree)") > .dt_compat = mstarv7_board_dt_compat, > .init_machine = mstarv7_init, > +#ifdef CONFIG_SMP > + .smp = smp_ops(mstarv7_smp_ops), > +#endif > MACHINE_END Drop the #ifdef, smp_ops() already makes the assignment conditional. Or better, use CPU_METHOD_OF_DECLARE() instead of smp_ops. Arnd
Hi Arnd, On Mon, 30 Nov 2020 at 22:42, Arnd Bergmann <arnd@kernel.org> wrote: > > +struct smp_operations __initdata mstarv7_smp_ops = { > > + .smp_boot_secondary = mstarv7_boot_secondary, > > +}; > > +#endif > > So no hotplug operations? Not yet. There are controls to power down different bits of the chip, assert internal resets and so on so it might be possible to add that later but I haven't worked out where those bits are yet for the second cpu. > Or better, use CPU_METHOD_OF_DECLARE() instead of smp_ops. I'll do that for the v2. Was there anything else that looked fishy? Every other platform seems to have a lot of code for moving secondary CPUs from the boot ROM into somewhere the kernel can control the order in which they come online (vendor code has a copy/paste of the vexpress code) so I was worried I missed something. Thanks, Daniel
On Mon, Nov 30, 2020 at 3:25 PM Daniel Palmer <daniel@0x0f.com> wrote: > > Hi Arnd, > > On Mon, 30 Nov 2020 at 22:42, Arnd Bergmann <arnd@kernel.org> wrote: > > > +struct smp_operations __initdata mstarv7_smp_ops = { > > > + .smp_boot_secondary = mstarv7_boot_secondary, > > > +}; > > > +#endif > > > > So no hotplug operations? > > Not yet. There are controls to power down different bits of the chip, > assert internal resets and so on so it might be possible to add that > later but I haven't worked out where those bits are yet for the second > cpu. > > > Or better, use CPU_METHOD_OF_DECLARE() instead of smp_ops. > > I'll do that for the v2. Ok. > Was there anything else that looked fishy? Every other platform seems > to have a lot of code for moving secondary CPUs from the boot ROM into > somewhere the kernel can control the order in which they come online > (vendor code has a copy/paste of the vexpress code) so I was worried I > missed something. No, it looks fine to me, but I'm not an expert in this area. As far as I can tell, platforms will either execute from bootrom with the secondary_startup function in a register like you have, or they start from SRAM, with that function somewhere else, but you wouldn't need both. Arnd
diff --git a/arch/arm/mach-mstar/mstarv7.c b/arch/arm/mach-mstar/mstarv7.c index 1aa748fa006e..23fe47a8f1a5 100644 --- a/arch/arm/mach-mstar/mstarv7.c +++ b/arch/arm/mach-mstar/mstarv7.c @@ -31,6 +31,13 @@ #define MSTARV7_L3BRIDGE_FLUSH_TRIGGER BIT(0) #define MSTARV7_L3BRIDGE_STATUS_DONE BIT(12) +#ifdef CONFIG_SMP +#define MSTARV7_CPU1_BOOT_ADDR_HIGH 0x4c +#define MSTARV7_CPU1_BOOT_ADDR_LOW 0x50 +#define MSTARV7_CPU1_UNLOCK 0x58 +#define MSTARV7_CPU1_UNLOCK_MAGIC 0xbabe +#endif + static void __iomem *l3bridge; static const char * const mstarv7_board_dt_compat[] __initconst = { @@ -63,6 +70,46 @@ static void mstarv7_mb(void) } } +#ifdef CONFIG_SMP +static int mstarv7_boot_secondary(unsigned int cpu, struct task_struct *idle) +{ + struct device_node *np; + u32 bootaddr = (u32) __pa_symbol(secondary_startup_arm); + void __iomem *smpctrl = 0; + + /* + * right now we don't know how to boot anything except + * cpu 1. + */ + if (cpu != 1) + return -EINVAL; + + np = of_find_compatible_node(NULL, NULL, "mstar,smpctrl"); + smpctrl = of_iomap(np, 0); + + if (!smpctrl) + return -ENODEV; + + /* set the boot address for the second cpu */ + writew(bootaddr & 0xffff, smpctrl + MSTARV7_CPU1_BOOT_ADDR_LOW); + writew((bootaddr >> 16) & 0xffff, smpctrl + MSTARV7_CPU1_BOOT_ADDR_HIGH); + + /* unlock the second cpu */ + writew(MSTARV7_CPU1_UNLOCK_MAGIC, smpctrl + MSTARV7_CPU1_UNLOCK); + + /* and away we go...*/ + arch_send_wakeup_ipi_mask(cpumask_of(cpu)); + + iounmap(smpctrl); + + return 0; +} + +struct smp_operations __initdata mstarv7_smp_ops = { + .smp_boot_secondary = mstarv7_boot_secondary, +}; +#endif + static void __init mstarv7_init(void) { struct device_node *np; @@ -78,4 +125,7 @@ static void __init mstarv7_init(void) DT_MACHINE_START(MSTARV7_DT, "MStar/Sigmastar Armv7 (Device Tree)") .dt_compat = mstarv7_board_dt_compat, .init_machine = mstarv7_init, +#ifdef CONFIG_SMP + .smp = smp_ops(mstarv7_smp_ops), +#endif MACHINE_END
This patch adds SMP support for MStar/Sigmastar chips that have a second core like those in the infinity2m family. So far only single and dual core chips have been found so this does the bare minimum to boot the second core. From what I can tell not having the "holding pen" code to handle multiple cores is fine if there is only one core the will get booted. This might need to be reconsidered if chips with more cores turn up. Signed-off-by: Daniel Palmer <daniel@0x0f.com> --- arch/arm/mach-mstar/mstarv7.c | 50 +++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)