[U-Boot] [PATCH v3 3/7] ARMv8/ls1043ardb: Integrate FSL PPA

[York Sun]

On 05/18/2016 12:37 AM, Zhiqiang Hou wrote:
> Hi York,
> 
> Thanks for your comments and sorry for my delay response due to PTO.
> 
>> -----Original Message-----
>> From: York Sun [mailto:york.sun at nxp.com]
>> Sent: 2016年5月11日 3:48
>> 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
>> Cc: Hou Zhiqiang <Zhiqiang.Hou at freescale.com>
>> Subject: Re: [PATCH v3 3/7] ARMv8/ls1043ardb: Integrate FSL PPA
>>
>> On 04/27/2016 06:49 AM, Zhiqiang Hou wrote:
>>> From: Hou Zhiqiang <Zhiqiang.Hou at freescale.com>
>>>
>>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou at freescale.com>
>>> ---
>>> V3:
>>>  - no change
>>>
>>>  board/freescale/ls1043ardb/ls1043ardb.c | 11 +++++++++++
>>>  include/configs/ls1043ardb.h            |  9 +++++++++
>>>  2 files changed, 20 insertions(+)
>>>
>>> diff --git a/board/freescale/ls1043ardb/ls1043ardb.c
>>> b/board/freescale/ls1043ardb/ls1043ardb.c
>>> index ec5fdbf..5f0a8e7 100644
>>> --- a/board/freescale/ls1043ardb/ls1043ardb.c
>>> +++ b/board/freescale/ls1043ardb/ls1043ardb.c
>>> @@ -9,6 +9,7 @@
>>>  #include <asm/io.h>
>>>  #include <asm/arch/clock.h>
>>>  #include <asm/arch/fsl_serdes.h>
>>> +#include <asm/arch/ppa.h>
>>>  #include <asm/arch/soc.h>
>>>  #include <fdt_support.h>
>>>  #include <hwconfig.h>
>>> @@ -84,6 +85,9 @@ int board_early_init_f(void)  int board_init(void)
>>> {
>>>  	struct ccsr_cci400 *cci = (struct ccsr_cci400
>>> *)CONFIG_SYS_CCI400_ADDR;
>>> +#ifdef CONFIG_FSL_LS_PPA
>>> +	u64 ppa_entry;
>>> +#endif
>>>
>>>  	/*
>>>  	 * Set CCI-400 control override register to enable barrier @@ -103,6
>>> +107,13 @@ int board_init(void)
>>>  	enable_layerscape_ns_access();
>>>  #endif
>>>
>>> +#ifdef CONFIG_FSL_LS_PPA
>>> +	ppa_init_pre(&ppa_entry);
>>> +
>>> +	if (ppa_entry)
>>> +		ppa_init_entry((void *)ppa_entry);
>>
>> ppa_init_pre() returns the error code. Why don't you use the return value here?
> 
> The function ppa_init_pre() will set the ppa_entry to 0x0 if any error occurred.
> 

Understood. My suggestion is to use the return error code if the function has
it, unless you have a good reason not to. Please add comment to explain for
future maintenance.

York