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

[Zhiqiang Hou]

Hi York,

> -----Original Message-----
> From: Zhiqiang Hou
> Sent: 2016年6月12日 12:31
> To: york sun <york.sun 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
> 
> Hi York,
> 
> Thanks for your comments!
> 
> > -----Original Message-----
> > From: york sun
> > Sent: 2016年6月12日 12:07
> > 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/11/2016 08:58 PM, Zhiqiang Hou wrote:
> > > Hi York,
> > >
> > > Thanks for your comments!
> > >
> > >> -----Original Message-----
> > >> From: York Sun [mailto:york.sun at nxp.com]
> > >> Sent: 2016年6月8日 8:56
> > >> 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;
> > >>
> > >> Another comment, even if the function can be used to indicate if
> > >> psci is available, do you have to cast it to (int)? I think this can be simplified as
> > >> 	psci_support = sec_firmware_support_psci_version() > 0;
> > >
> > > The type of this func return value is 'unsigned int', so the cast is necessary.
> >
> > The return value of function sec_firmware_support_psci_version() may
> > need some work. It has three results
> >
> > Positive numbers mean success (presuming bit 31 is not used by major number.
> > Need to check with PPA code) Zero means image is not valid Negative
> > numbers means errors
> 
> In PSCI spec v1.0, the bit 31 is used by major number, and the type of the return
> value is uint, but there isn't any other description for the return value. I don't know
> why the PPA isn't consistent with PSCI spec.

I misunderstand your words, and the PPA is consistent with PSCI spec.
Will take your suggestion that presuming bit 31 isn't used by major number to handle
the return value.
 
> > If the error code doesn't include -EINVAL, you can return -EINVAL in
> > case of an invalid image. Then you can have
> > sec_firmware_support_psci_version() return int.
> >

Thanks,
Zhiqiang