| Message ID | E1gXPav-00064t-KY@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] ARM: omap2: remove unnecessary boot_lock | expand |
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 >
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.
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
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; }
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(-)