| Message ID | 20181211080759.3150-5-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] ARM: actions: Put OF nodes after use | expand |
On Tue, Dec 11, 2018 at 09:07:59AM +0100, Linus Walleij wrote: > The Actions SMP boot contain some cargo cult code that > we need to get rid of: it uses a probably completely > unnecessary spinlock (the ARM core SMP code will lock > properly already) and tries to keep CPUs in the "pen" > (CPU enclosure) using the custom variable that should > only be used with ARM Versatile test chips. > > Simply drop this and also remove the smp_send_reschedule() > that no other ARM machine is using to start a secondary > CPU and just send arch_send_wakeup_ipi_mask() to kick > secondary CPUs out of WFI. > > Reported-by: Russell King <linux@armlinux.org.uk> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > This patch was made after talking to Russell about ARM > machines doing pointless "pen holding" and using > pointless boot locks. Would be great if you can test > if it simply works like this, make sure both CPUs > come up, I usually just start a few dummy processes > and cat /proc/interrupts to see that the different > CPUs are getting work done. Provided it does work: Acked-by: Russell King <rmk+kernel@armlinux.org.uk> Also, be sure to exercise the hotplug paths during testing, not simply "does it still boot". Thanks. > --- > arch/arm/mach-actions/platsmp.c | 14 +------------- > 1 file changed, 1 insertion(+), 13 deletions(-) > > diff --git a/arch/arm/mach-actions/platsmp.c b/arch/arm/mach-actions/platsmp.c > index 26ddc12ea4a7..c19477fed340 100644 > --- a/arch/arm/mach-actions/platsmp.c > +++ b/arch/arm/mach-actions/platsmp.c > @@ -36,7 +36,6 @@ > > static void __iomem *sps_base_addr; > static void __iomem *timer_base_addr; > -static DEFINE_SPINLOCK(boot_lock); > > static int s500_wakeup_secondary(unsigned int cpu) > { > @@ -79,7 +78,6 @@ static int s500_wakeup_secondary(unsigned int cpu) > > static int s500_smp_boot_secondary(unsigned int cpu, struct task_struct *idle) > { > - unsigned long timeout; > int ret; > > ret = s500_wakeup_secondary(cpu); > @@ -88,20 +86,10 @@ static int s500_smp_boot_secondary(unsigned int cpu, struct task_struct *idle) > > udelay(10); > > - spin_lock(&boot_lock); > - > - smp_send_reschedule(cpu); > - > - timeout = jiffies + (1 * HZ); > - while (time_before(jiffies, timeout)) { > - if (pen_release == -1) > - break; > - } > - > writel(0, timer_base_addr + OWL_CPU1_ADDR + (cpu - 1) * 4); > writel(0, timer_base_addr + OWL_CPU1_FLAG + (cpu - 1) * 4); > > - spin_unlock(&boot_lock); > + arch_send_wakeup_ipi_mask(cpumask_of(cpu)); > > return 0; > } > -- > 2.19.2 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Dec 12, 2018 at 12:30 PM Russell King - ARM Linux <linux@armlinux.org.uk> wrote: > Also, be sure to exercise the hotplug paths during testing, not > simply "does it still boot". Indeed. If the platform support suspend-to-ram then echo mem > /sys/power/state and then bring the system up again, as a specific test that offlines all CPUs and then brings them back online. Yiyrs, Linus Walleij
diff --git a/arch/arm/mach-actions/platsmp.c b/arch/arm/mach-actions/platsmp.c index 26ddc12ea4a7..c19477fed340 100644 --- a/arch/arm/mach-actions/platsmp.c +++ b/arch/arm/mach-actions/platsmp.c @@ -36,7 +36,6 @@ static void __iomem *sps_base_addr; static void __iomem *timer_base_addr; -static DEFINE_SPINLOCK(boot_lock); static int s500_wakeup_secondary(unsigned int cpu) { @@ -79,7 +78,6 @@ static int s500_wakeup_secondary(unsigned int cpu) static int s500_smp_boot_secondary(unsigned int cpu, struct task_struct *idle) { - unsigned long timeout; int ret; ret = s500_wakeup_secondary(cpu); @@ -88,20 +86,10 @@ static int s500_smp_boot_secondary(unsigned int cpu, struct task_struct *idle) udelay(10); - spin_lock(&boot_lock); - - smp_send_reschedule(cpu); - - timeout = jiffies + (1 * HZ); - while (time_before(jiffies, timeout)) { - if (pen_release == -1) - break; - } - writel(0, timer_base_addr + OWL_CPU1_ADDR + (cpu - 1) * 4); writel(0, timer_base_addr + OWL_CPU1_FLAG + (cpu - 1) * 4); - spin_unlock(&boot_lock); + arch_send_wakeup_ipi_mask(cpumask_of(cpu)); return 0; }
The Actions SMP boot contain some cargo cult code that we need to get rid of: it uses a probably completely unnecessary spinlock (the ARM core SMP code will lock properly already) and tries to keep CPUs in the "pen" (CPU enclosure) using the custom variable that should only be used with ARM Versatile test chips. Simply drop this and also remove the smp_send_reschedule() that no other ARM machine is using to start a secondary CPU and just send arch_send_wakeup_ipi_mask() to kick secondary CPUs out of WFI. Reported-by: Russell King <linux@armlinux.org.uk> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- This patch was made after talking to Russell about ARM machines doing pointless "pen holding" and using pointless boot locks. Would be great if you can test if it simply works like this, make sure both CPUs come up, I usually just start a few dummy processes and cat /proc/interrupts to see that the different CPUs are getting work done. --- arch/arm/mach-actions/platsmp.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)