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

[york sun]

On 06/13/2016 01:30 AM, Zhiqiang Hou wrote:
> 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.

Zhiqiang,

I didn't suggest to discard bit 31. If PSCI uses bit 31, you can't 
presume it otherwise. In this case, you cannot return negative value 
directly. You may process the version number to fit in an int variable, 
or find another way to indicate an error.

York