ARM: actions: remove boot_lock and pen_release

PATCH In review Russell King 4 participants 4 messages Dec 13, 2018 – Jan 11, 2019

Subsystems (4)

Arm (32-bit) Arm sub-architectures Arm Qualcomm mailing list Arm/actions semi architecture

lore

Share & tools

lore

Russell King

author

2018-12-13 11:55 UTC #0 lore raw

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

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 <[email protected]>

---
 arch/arm/mach-actions/platsmp.c | 15 ---------------
 1 file changed, 15 deletions(-)

arch/arm/mach-actions/platsmp.c +0 ~~-15~~

index 3efaa10efc43..4fd479c948e6 100644
@@ -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;
 }

Signature · 1 line

2.7.4

Manivannan Sadhasivam

2018-12-13 12:01 UTC #1 lore raw

Quoted 5 lines · 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.

Quoted 7 lines

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 <[email protected]>

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

Quoted 52 lines

---
 arch/arm/mach-actions/platsmp.c | 15 ---------------
 1 file changed, 15 deletions(-)

arch/arm/mach-actions/platsmp.c +0 ~~-15~~

index 3efaa10efc43..4fd479c948e6 100644
@@ -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;
 }

Signature · 1 line

2.7.4

Russell King - ARM Linux

maintainer

2018-12-13 12:04 UTC #2 lore raw

Quoted 9 lines · Manivannan Sadhasivam

Quoted 8 lines · 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.

Quoted 8 lines

Quoted 4 lines
3. owl_secondary_startup() is not referenced nor defined, the prototype is redundant, remove it.
Signed-off-by Russell King <[email protected]>
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.

Signature · 3 lines

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

Linus Walleij

2019-01-11 08:19 UTC #3 lore raw

In reply to Manivannan Sadhasivam

Quoted 9 lines · Manivannan Sadhasivam

Quoted 8 lines · 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.

Quoted 8 lines

Quoted 4 lines
3. owl_secondary_startup() is not referenced nor defined, the prototype is redundant, remove it.
Signed-off-by Russell King <[email protected]>
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