On 06/11/2016 08:58 PM, Zhiqiang Hou wrote:
> Hi York,
>
> Thanks for your comments!
>
>> -----Original Message-----
>> From: York Sun [mailto:york....@nxp.com]
>> Sent: 2016年6月8日 8:56
>> 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;
>>
>> 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

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.

York

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to