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

[Zhiqiang Hou]

Hi York,

Thanks for your comments!

> -----Original Message-----
> From: York Sun [mailto:york.sun at nxp.com]
> Sent: 2016年5月18日 23:16
> To: Zhiqiang Hou <zhiqiang.hou at nxp.com>
> Cc: u-boot at lists.denx.de; albert.u.boot at aribaud.net; Scott Wood
> <oss at buserror.net>; Mingkai.hu at freescale.com; leoli at freescale.com;
> prabhakar at freescale.com; bhupesh.sharma at freescale.com; Hou Zhiqiang
> <Zhiqiang.Hou at freescale.com>
> Subject: Re: [PATCH v3 3/7] ARMv8/ls1043ardb: Integrate FSL PPA
> 
> 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.

Added the operation to check the return value. I have refactored the patchset and
reordered the sequence of the patches in version 4.

Thanks,
Zhiqiang