[PATCH v8 01/11] ARM: brcmstb: add infrastructure for ARM-based Broadcom STB SoCs

Brian Norris computersforpeace at gmail.com
Wed Jul 30 19:36:35 PDT 2014


Hi Russell,

On Wed, Jul 30, 2014 at 10:26:35AM +0100, Russell King wrote:
> On Mon, Jul 21, 2014 at 02:07:56PM -0700, Brian Norris wrote:
> > +static DEFINE_SPINLOCK(boot_lock);
> > +
> > +static void brcmstb_secondary_init(unsigned int cpu)
> > +{
> > +	/*
> > +	 * Synchronise with the boot thread.
> > +	 */
> > +	spin_lock(&boot_lock);
> > +	spin_unlock(&boot_lock);
> > +}
> > +
> > +static int brcmstb_boot_secondary(unsigned int cpu, struct task_struct *idle)
> > +{
> > +	/*
> > +	 * set synchronisation state between this boot processor
> > +	 * and the secondary one
> > +	 */
> > +	spin_lock(&boot_lock);
> > +
> > +	/* Bring up power to the core if necessary */
> > +	if (brcmstb_cpu_get_power_state(cpu) == 0)
> > +		brcmstb_cpu_power_on(cpu);
> > +
> > +	brcmstb_cpu_boot(cpu);
> > +
> > +	/*
> > +	 * now the secondary core is starting up let it run its
> > +	 * calibrations, then wait for it to finish
> > +	 */
> > +	spin_unlock(&boot_lock);
> 
> I've just read through this code (because it caused my allmodconfig to
> break) and spotted this.

Sorry about the allmodconfig problems. I never compile-tested with ARMv6
enabled. This look OK?

diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index f3665121729b..5ce82b4ba931 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_ARCH_BCM2835)	+= board_bcm2835.o
 obj-$(CONFIG_ARCH_BCM_5301X)	+= bcm_5301x.o
 
 ifeq ($(CONFIG_ARCH_BRCMSTB),y)
+CFLAGS_platsmp-brcmstb.o	+= -march=armv7-a
 obj-y				+= brcmstb.o
 obj-$(CONFIG_SMP)		+= headsmp-brcmstb.o platsmp-brcmstb.o
 endif

> What function does boot_lock perform here?  Please, don't quote the
> comments (I know where the comments came from) but what I want to hear
> is your comments about why you decided to retain this.

You might glean a little more from my response to Rob, but I'm not sure
there was a good reason for retaining this. We do need to be sure the
CPU is fully powered online before bringing it out of reset, but the
spinlock seems overkill AFAICT.

Brian



More information about the linux-arm-kernel mailing list