[5/5] RFT: ARM: actions: Simplify secondary boot
diff mbox series

Message ID 20181211080759.3150-5-linus.walleij@linaro.org
State New
Headers show
Series
  • [1/5] ARM: actions: Put OF nodes after use
Related show

Commit Message

Linus Walleij Dec. 11, 2018, 8:07 a.m. UTC
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(-)

Comments

Russell King - ARM Linux admin Dec. 12, 2018, 11:30 a.m. UTC | #1
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
Linus Walleij Dec. 12, 2018, 12:03 p.m. UTC | #2
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

Patch
diff mbox series

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;
 }