Hi York, Thanks for your comments!
> -----Original Message----- > From: York Sun [mailto:york....@nxp.com] > Sent: 2016年6月8日 8:51 > To: Zhiqiang Hou <zhiqiang....@nxp.com>; u-boot@lists.denx.de; > albert.u.b...@aribaud.net; scottw...@freescale.com; > mingkai...@freescale.com; york...@freescale.com; le...@freescale.com; > prabha...@freescale.com; bhupesh.sha...@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....@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....@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
