[PATCH 5/5] ARM: actions: remove boot_lock and pen_release

[PATCH 5/5] ARM: actions: remove boot_lock and pen_release

[Russell King]

The actions SMP implementation has several issues:

1. pen_release is only ever read and compared to -1, and is defined in
   arch/arm/kernel/smp.c to be -1.  This test will always succeed.

2. we are already guaranteed to be single threaded while bringing up a
   CPU, so the spinlock makes no sense, remove it.

3. owl_secondary_startup() is not referenced nor defined, the prototype
   is redundant, remove it.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mach-actions/platsmp.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/arch/arm/mach-actions/platsmp.c b/arch/arm/mach-actions/platsmp.c
index 3efaa10efc43..4fd479c948e6 100644
--- a/arch/arm/mach-actions/platsmp.c
+++ b/arch/arm/mach-actions/platsmp.c
@@ -39,10 +39,6 @@ static void __iomem *sps_base_addr;
 static void __iomem *timer_base_addr;
 static int ncores;
 
-static DEFINE_SPINLOCK(boot_lock);
-
-void owl_secondary_startup(void);
-
 static int s500_wakeup_secondary(unsigned int cpu)
 {
 	int ret;
@@ -84,7 +80,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);
@@ -93,21 +88,11 @@ 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);
-
 	return 0;
 }
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/5] ARM: actions: remove boot_lock and pen_release

[Manivannan Sadhasivam]

On Thu, Dec 13, 2018 at 11:55:57AM +0000, Russell King wrote:
> The actions SMP implementation has several issues:
> 
> 1. pen_release is only ever read and compared to -1, and is defined in
>    arch/arm/kernel/smp.c to be -1.  This test will always succeed.
> 
> 2. we are already guaranteed to be single threaded while bringing up a
>    CPU, so the spinlock makes no sense, remove it.
> 
> 3. owl_secondary_startup() is not referenced nor defined, the prototype
>    is redundant, remove it.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Hi Russel,

Is this patch a superset of https://www.spinics.net/lists/arm-kernel/msg694546.html?

Andreas: Will you be able to test this patch or Linus's series on S500 based board?

Thanks,
Mani

> ---
>  arch/arm/mach-actions/platsmp.c | 15 ---------------
>  1 file changed, 15 deletions(-)
> 
> diff --git a/arch/arm/mach-actions/platsmp.c b/arch/arm/mach-actions/platsmp.c
> index 3efaa10efc43..4fd479c948e6 100644
> --- a/arch/arm/mach-actions/platsmp.c
> +++ b/arch/arm/mach-actions/platsmp.c
> @@ -39,10 +39,6 @@ static void __iomem *sps_base_addr;
>  static void __iomem *timer_base_addr;
>  static int ncores;
>  
> -static DEFINE_SPINLOCK(boot_lock);
> -
> -void owl_secondary_startup(void);
> -
>  static int s500_wakeup_secondary(unsigned int cpu)
>  {
>  	int ret;
> @@ -84,7 +80,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);
> @@ -93,21 +88,11 @@ 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);
> -
>  	return 0;
>  }
>  
> -- 
> 2.7.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/5] ARM: actions: remove boot_lock and pen_release

[Russell King - ARM Linux]

On Thu, Dec 13, 2018 at 05:31:53PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Dec 13, 2018 at 11:55:57AM +0000, Russell King wrote:
> > The actions SMP implementation has several issues:
> > 
> > 1. pen_release is only ever read and compared to -1, and is defined in
> >    arch/arm/kernel/smp.c to be -1.  This test will always succeed.
> > 
> > 2. we are already guaranteed to be single threaded while bringing up a
> >    CPU, so the spinlock makes no sense, remove it.
> > 
> > 3. owl_secondary_startup() is not referenced nor defined, the prototype
> >    is redundant, remove it.
> > 
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> 
> Hi Russel,
> 
> Is this patch a superset of https://www.spinics.net/lists/arm-kernel/msg694546.html?

Probably not, I couldn't find which platform Linus had already done in
my mailbox, so I just did those that I could see in the kernel tree.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/5] ARM: actions: remove boot_lock and pen_release

[Linus Walleij]

On Thu, Dec 13, 2018 at 1:02 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:

> On Thu, Dec 13, 2018 at 11:55:57AM +0000, Russell King wrote:
> > The actions SMP implementation has several issues:
> >
> > 1. pen_release is only ever read and compared to -1, and is defined in
> >    arch/arm/kernel/smp.c to be -1.  This test will always succeed.
> >
> > 2. we are already guaranteed to be single threaded while bringing up a
> >    CPU, so the spinlock makes no sense, remove it.
> >
> > 3. owl_secondary_startup() is not referenced nor defined, the prototype
> >    is redundant, remove it.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>
> Hi Russel,
>
> Is this patch a superset of https://www.spinics.net/lists/arm-kernel/msg694546.html?

Please apply Russell's patch first, I can easily apply my refactorings on
top later.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel