On 06/11/2016 08:49 PM, Zhiqiang Hou wrote: > 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. > I see. It verifies the image is valid first, then it makes an smc call. Thanks.
York _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
