From mboxrd@z Thu Jan 1 00:00:00 1970 From: nicolas.pitre@linaro.org (Nicolas Pitre) Date: Tue, 25 Nov 2014 13:59:17 -0500 (EST) Subject: [PATCH 3/9] ARM: MB86S7X: Add MCPM support In-Reply-To: <20141125184609.GA29425@e102568-lin.cambridge.arm.com> References: <1416486442-25200-1-git-send-email-Vincent.Yang@tw.fujitsu.com> <1416486920-25343-1-git-send-email-Vincent.Yang@tw.fujitsu.com> <20141125184609.GA29425@e102568-lin.cambridge.arm.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, 25 Nov 2014, Lorenzo Pieralisi wrote: > On Tue, Nov 25, 2014 at 05:42:32PM +0000, Nicolas Pitre wrote: > > On Thu, 20 Nov 2014, Vincent Yang wrote: > > > > > The remote firmware(SCB) owns the SMP control. This MCPM driver gets > > > CPU/CLUSTER power up/down done by SCB over mailbox. > > > > > > Signed-off-by: Andy Green > > > Signed-off-by: Jassi Brar > > > Signed-off-by: Vincent Yang > > > Signed-off-by: Tetsuya Nuriya > > > --- > > > arch/arm/mach-mb86s7x/Makefile | 2 +- > > > arch/arm/mach-mb86s7x/mcpm.c | 360 +++++++++++++++++++++++++++++++++++++++++ > > > arch/arm/mach-mb86s7x/smc.S | 27 ++++ > > > 3 files changed, 388 insertions(+), 1 deletion(-) > > > create mode 100644 arch/arm/mach-mb86s7x/mcpm.c > > > create mode 100644 arch/arm/mach-mb86s7x/smc.S > > > > > > diff --git a/arch/arm/mach-mb86s7x/Makefile b/arch/arm/mach-mb86s7x/Makefile > > > index 97640b6..b0fa34b 100644 > > > --- a/arch/arm/mach-mb86s7x/Makefile > > > +++ b/arch/arm/mach-mb86s7x/Makefile > > > @@ -1 +1 @@ > > > -obj-$(CONFIG_ARCH_MB86S7X) += board.o > > > +obj-$(CONFIG_ARCH_MB86S7X) += board.o mcpm.o smc.o > > > diff --git a/arch/arm/mach-mb86s7x/mcpm.c b/arch/arm/mach-mb86s7x/mcpm.c > > > new file mode 100644 > > > index 0000000..bf1b50a > > > --- /dev/null > > > +++ b/arch/arm/mach-mb86s7x/mcpm.c > > > @@ -0,0 +1,360 @@ > > > +/* > > > + * arch/arm/mach-mb86s7x/mcpm.c > > > + * Copyright: (C) 2013-2014 Fujitsu Semiconductor Limited > > > + * Copyright: (C) 2014 Linaro Ltd. > > > + * > > > + * This program is free software; you can redistribute it and/or modify > > > + * it under the terms of the GNU General Public License version 2 as > > > + * published by the Free Software Foundation. > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#include > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#define S7X_MAX_CLUSTER 2 > > > +#define S7X_MAX_CPU 2 > > > + > > > +#define MHU_SHM_OFFSET 0x3800 > > > +#define WFI_COLOR_OFFSET 0x3f00 > > > +#define TRAMPOLINE_OFFSET 0x3c00 > > > +#define RESET_OFFSET (TRAMPOLINE_OFFSET + 0x3fc) > > > + > > > +static arch_spinlock_t mb86s7x_pm_lock = __ARCH_SPIN_LOCK_UNLOCKED; > > > +static int mb86s7x_pm_use_count[S7X_MAX_CLUSTER][S7X_MAX_CPU]; > > > +extern void __iomem *mb86s7x_shm_base; > > > + > > > +#define AT_WFI_DO_NOTHING 0x0 > > > +#define AT_WFI_DO_SUSPEND 0x1 > > > +#define AT_WFI_DO_POWEROFF 0x2 > > > +#define AT_WFI_COLOR_MASK 0x3 > > > + > > > +struct mb86s7x_cpu_gate { > > > + u32 payload_size; > > > + u32 cluster_class; > > > + u32 cluster_id; > > > + u32 cpu_id; > > > +#define SCB_CPU_STATE_OFF 0x0 > > > +#define SCB_CPU_STATE_ON 0x1 > > > +#define SCB_CPU_STATE_SUSP 0x2 > > > + u32 cpu_state; > > > +}; > > > + > > > +asmlinkage void mb86s70evb_outer_flush_all(void) > > > +{ > > > + outer_flush_all(); > > > +} > > > + > > > +#define mb86s70evb_exit_coherency_flush(level) { \ > > > + asm volatile( \ > > > + "stmfd sp!, {fp, ip}\n\t" \ > > > + "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR\n\t" \ > > > + "bic r0, r0, #"__stringify(CR_C)"\n\t" \ > > > + "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR\n\t" \ > > > + "isb\n\t" \ > > > + "bl v7_flush_dcache_"__stringify(level)"\n\t" \ > > > + "bl mb86s70evb_outer_flush_all\n\t" \ > > > > This is wrong. As mentioned already, this unconditionally flushes L2 in > > all cases which shouldn't be necessary in the "louis" case. > > Is this a bL system with unified and architected L2s ? I think so, so > what's the outercache for ? > > > Furthermore, the safety of this macro is ensured by not having any > > memory writes in the middle of the whole sequence. By calling > > mb86s70evb_outer_flush_all() there could be the return address pushed > > onto the stack before calling outer_flush_all() if some tail call > > optimization is not applied. And this is without saying what > > outer_flush_all() actually does. > > > > Why can't you simply do this instead: > > > > v7_exit_coherency_flush(all); > > outer_flush_all(); > > > > Of course you'll have to audit everything in the outer_flush_all() path > > to make sure no atomic instructions such as LDREX/STREX are invoked. > > Those have undefined behavior after CR_C is cleared. > > What you are saying is correct but first of all I would like to > understand what outercache we are talking about here. > > Code snippet above is fragile, as you said, and I'd rather disable > the outercache before executing the power down sequence (which implies > cache is cleaned/invalidated and quiescent before we clear the C bit) > if there is an outercache to be managed. Good question. That opens the potential for an even simpler fix! ;-) Nicolas