From mboxrd@z Thu Jan  1 00:00:00 1970
From: York Sun <york.sun@nxp.com>
Date: Wed, 18 May 2016 08:16:05 -0700
Subject: [U-Boot] [PATCH v3 3/7] ARMv8/ls1043ardb: Integrate FSL PPA
In-Reply-To: <DBXPR04MB2066BA97672C9EFBED98CD784490@DBXPR04MB206.eurprd04.prod.outlook.com>
References: <1461764367-7760-1-git-send-email-Zhiqiang.Hou@nxp.com>
 <1461764367-7760-3-git-send-email-Zhiqiang.Hou@nxp.com>
 <57323B07.1050603@nxp.com>
 <DBXPR04MB2066BA97672C9EFBED98CD784490@DBXPR04MB206.eurprd04.prod.outlook.com>
Message-ID: <573C8735.3060104@nxp.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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@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@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@freescale.com>
>>>
>>> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@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