[PATCH 1/5] ARM: EXYNOS: fix the hotplug for Cortex-A15

[Lorenzo Pieralisi]

On Tue, Nov 06, 2012 at 12:17:02PM +0000, Santosh Shilimkar wrote:
> On Tuesday 06 November 2012 12:12 AM, Abhilash Kesavan wrote:
> > The sequence of cpu_enter_lowpower() for Cortex-A15
> > is different from the sequence for Cortex-A9.
> Are you sure ? Apart from integrated cache vs external, there
> should be no change. And L2 doesn't need to come into picture
> while powering down just a CPU.

Reiterating Santosh point in here. v7 shutdown procedure is and has to
be identical across all v7 cores. There is not such a thing as "A15
specific" shutdown procedure.

Embedded L2 will come into the picture on multi-cluster systems, for the
time being L2 must not be flushed when hotplugging a CPU in a single cluster
so the LoUIS API is to be used here.

> > This patch implements cpu_enter_lowpower() for EXYNOS5
> > SoC which has Cortex-A15 cores.
> >
> > Signed-off-by: Changhwan Youn <chaos.youn at samsung.com>
> > Cc: Russell King <rmk+kernel at arm.linux.org.uk>
> > Signed-off-by: Kukjin Kim <kgene.kim at samsung.com>
> > Tested-by: Abhilash Kesavan <a.kesavan at samsung.com>
> > ---
> >   arch/arm/mach-exynos/hotplug.c |   45 +++++++++++++++++++++++++++++++++++++--
> >   1 files changed, 42 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
> > index f4d7dd2..8c06c4f 100644
> > --- a/arch/arm/mach-exynos/hotplug.c
> > +++ b/arch/arm/mach-exynos/hotplug.c
> > @@ -20,10 +20,11 @@
> >   #include <asm/smp_plat.h>
> >
> >   #include <mach/regs-pmu.h>
> > +#include <plat/cpu.h>
> >
> >   #include "common.h"
> >
> > -static inline void cpu_enter_lowpower(void)
> > +static inline void cpu_enter_lowpower_a9(void)
> >   {
> >   	unsigned int v;
> >
> > @@ -45,6 +46,35 @@ static inline void cpu_enter_lowpower(void)
> >   	  : "cc");
> >   }
> >
> > +static inline void cpu_enter_lowpower_a15(void)
> > +{
> > +	unsigned int v;
> > +
> > +	asm volatile(
> > +	"	mrc	p15, 0, %0, c1, c0, 0\n"
> > +	"	bic	%0, %0, %1\n"
> > +	"	mcr	p15, 0, %0, c1, c0, 0\n"
> > +	  : "=&r" (v)
> > +	  : "Ir" (CR_C)
> > +	  : "cc");
> > +
> > +	flush_cache_all();
> > +
> Why are flushing all the cache levels ?
> flush_kern_louis() should be enough for CPU power
> down.

Agree with Santosh again.

> 
> > +	asm volatile(
> > +	/*
> > +	* Turn off coherency
> > +	*/
> > +	"	mrc	p15, 0, %0, c1, c0, 1\n"
> > +	"	bic	%0, %0, %1\n"
> > +	"	mcr	p15, 0, %0, c1, c0, 1\n"
> > +	: "=&r" (v)
> > +	: "Ir" (0x40)
> > +	: "cc");
> > +
> > +	isb();
> > +	dsb();
> > +}
> > +
> The above sequence should work on A9 as well. In general you should have
> CPU power down code under one code block and avoid making use of stack 
> in between. Otherwise you will end up with stack corruption because of
> the memory view change after C bit is disabled.
> 
> Regards
> Santosh

The above sequence does not work on A9 since A9 does not look-up the
caches when the C bit is cleared. It is an accident waiting to happen,
as Santosh explained.

The sequence:

- clear C bit
- clean L1
- exit SMP

must be written in assembly with no access to any data whatsoever, no stack,
_nothing_.

There is some code in the works to consolidate this procedure once for all but
all bits and pieces are already in the kernel.

Lorenzo