Hi York,
Thanks for your comments!
-----Original Message----- From: York Sun [mailto:york.sun@nxp.com] Sent: 2016年6月8日 8:51 To: Zhiqiang Hou zhiqiang.hou@nxp.com; u-boot@lists.denx.de; albert.u.boot@aribaud.net; scottwood@freescale.com; Mingkai.hu@freescale.com; yorksun@freescale.com; leoli@freescale.com; prabhakar@freescale.com; bhupesh.sharma@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@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@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