[U-Boot] [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly

[Zhiqiang Hou]

Hi York,

Thanks for your comments!

> -----Original Message-----
> From: York Sun [mailto:york.sun at nxp.com]
> Sent: 2016年6月8日 8:51
> To: Zhiqiang Hou <zhiqiang.hou at nxp.com>; u-boot at lists.denx.de;
> albert.u.boot at aribaud.net; scottwood at freescale.com;
> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
> prabhakar at freescale.com; bhupesh.sharma at freescale.com
> Subject: Re: [PATCHV5 4/6] ARMv8/Layerscape: switch SMP method accordingly
> 
> On 06/04/2016 11:40 PM, Zhiqiang Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> >
> > If the PSCI and PPA is ready, skip the fixup for spin-table and waking
> > secondary cores. If not, change SMP method to spin-table, and the
> > device node of PSCI will be removed.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> > ---
> > V5:
> >  - Changed the checking if the PSCI feature is ready to read the psci version.
> >
> > V4:
> >  - Reordered this patch.
> >
> >  arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 17 +++++++++++++---
> > arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 36
> > +++++++++++++++++++++++++++++++++
> >  2 files changed, 50 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > index 672a453..eb566cd 100644
> > --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > @@ -23,6 +23,9 @@
> >  #ifdef CONFIG_FSL_ESDHC
> >  #include <fsl_esdhc.h>
> >  #endif
> > +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include
> > +<asm/armv8/sec_firmware.h> #endif
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -618,6 +621,7 @@ int arch_early_init_r(void)  {  #ifdef CONFIG_MP
> >  	int rv = 1;
> > +	bool psci_support = false;
> >  #endif
> >
> >  #ifdef CONFIG_SYS_FSL_ERRATUM_A009635 @@ -625,9 +629,16 @@ int
> > arch_early_init_r(void)  #endif
> >
> >  #ifdef CONFIG_MP
> > -	rv = fsl_layerscape_wake_seconday_cores();
> > -	if (rv)
> > -		printf("Did not wake secondary cores\n");
> > +#if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) &&
> defined(CONFIG_ARMV8_PSCI)
> > +	/* Check the psci version to determine if the psci is supported */
> > +	psci_support = (int)sec_firmware_support_psci_version() > 0 ?
> > +			true : false;
> > +#endif
> > +	if (!psci_support) {
> > +		rv = fsl_layerscape_wake_seconday_cores();
> > +		if (rv)
> > +			printf("Did not wake secondary cores\n");
> > +	}
> >  #endif
> >
> 
> Zhiqiang,
> 
> Here you presume the psci version returned by
> sec_firmware_support_psci_version() is a proof of a functional psci. I think that is
> flawed. This sec_firmware_support_psci_version() can return a positive number as
> far as the image in specified location is valid. It doesn't guarantee the image is
> running. The only place you would know the result of loading such image is by
> calling sec_firmware_init() in board_init() in your 6th patch. But you didn't check
> the return. That means you blindly believe a valid image would be successfully
> loaded and started to run. Later when you need to decide to use spin-table or PSCI
> for secondary cores, you don't know if such image is running.
> 
> I wonder if you can make a psci call to return the version number.

So far, the function sec_firmware_support_psci_version() do the psci call to get the
supported psci version. And if the returned psci version is valid, then we assume the
PPA is running, otherwise if the returned value is an error number, we assume the
PPA isn't running regularly. The return value of the function was casted to type 'int',
because the psci_version call will return the version by pattern major[31:16]:minor[15:0]
in normal, but a negative error number while in exception.

Thanks,
Zhiqiang