[U-Boot] [PATCH 1/4] mx7: psci: add cpu hotplug support

[Anson Huang]

Hi, Bryan

Anson Huang
Best Regards!


> -----Original Message-----
> From: Bryan O'Donoghue [mailto:bryan.odonoghue at linaro.org]
> Sent: Monday, May 28, 2018 4:59 PM
> To: Anson Huang <anson.huang at nxp.com>; sbabic at denx.de; Fabio Estevam
> <fabio.estevam at nxp.com>; albert.u.boot at aribaud.net;
> christian.gmeiner at gmail.com; Peng Fan <peng.fan at nxp.com>;
> patrick.delaunay at st.com; sjg at chromium.org
> Cc: dl-linux-imx <linux-imx at nxp.com>; u-boot at lists.denx.de
> Subject: Re: [PATCH 1/4] mx7: psci: add cpu hotplug support
> 
> 
> 
> On 28/05/18 07:17, Anson Huang wrote:
> > This patch adds cpu hotplug support, previous imx_cpu_off
> > implementation is NOT safe, a CPU can NOT power down itself in
> > runtime, it will cause system bus hang due to pending transaction. So
> > need to use other online CPU to kill it when it is ready for killed.
> >
> > Here use SRC parameter register and a magic number of ~0 as handshake
> > for killing a offline CPU, when the online CPU checks the
> > psci_affinity_info, it will help kill the offline CPU according to the
> > magic number stored in SRC parameter register.
> >
> > Signed-off-by: Anson Huang <Anson.Huang at nxp.com>
> > ---
> >   arch/arm/mach-imx/mx7/psci-mx7.c | 31
> ++++++++++++++++++++++++++++---
> >   arch/arm/mach-imx/mx7/psci.S     | 14 ++++++++++++++
> >   2 files changed, 42 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/mx7/psci-mx7.c
> > b/arch/arm/mach-imx/mx7/psci-mx7.c
> > index 7dc49bd..6f69fd3 100644
> > --- a/arch/arm/mach-imx/mx7/psci-mx7.c
> > +++ b/arch/arm/mach-imx/mx7/psci-mx7.c
> > @@ -77,12 +77,37 @@ __secure int imx_cpu_on(int fn, int cpu, int pc)
> >
> >   __secure int imx_cpu_off(int cpu)
> >   {
> > -	imx_enable_cpu_ca7(cpu, false);
> > -	imx_gpcv2_set_core1_power(false);
> > -	writel(0, SRC_BASE_ADDR + cpu * 8 + SRC_GPR1_MX7D + 4);
> > +	/*
> > +	 * We use the cpu jumping argument register to sync with
> > +	 * imx_cpu_affinity() which is running on cpu0 to kill the cpu.
> > +	 */
> > +	writel(~0, SRC_BASE_ADDR + cpu * 8 + SRC_GPR1_MX7D + 4);
> 
> Small suggestion. Make that "SRC_BASE_ADDR + cpu * 8 + SRC_GPR1_MX7D +
> 4" into some sort of macro
> 
> #define imx_cpu_gpr_offset(cpu) \
> 	SRC_BASE_ADDR + cpu * 8 + SRC_GPR1_MX7D + 4
> 
> you use it several times so its a good candidate for functional decompisition
> IMO.
> 
> Also instead of having magic numbers - define them.
> 
> #define IMX_CPU_SYNC_ON ~0
> #define IMX_CPU_SYNC_OFF 0

Thanks, will improve them in patch V2 later.

Anson.
 



> 
> ---
> bod