Message ID | 1461764367-7760-3-git-send-email-Zhiqiang.Hou@nxp.com |
---|---|
State | Superseded |
Delegated to: | York Sun |
Headers | show |
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? York
Hi York, Thanks for your comments and sorry for my delay response due to PTO. > -----Original Message----- > From: York Sun [mailto:york.sun@nxp.com] > Sent: 2016年5月11日 3:48 > To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot@lists.denx.de; > albert.u.boot@aribaud.net; scottwood@freescale.com; > Mingkai.hu@freescale.com; yorksun@freescale.com; leoli@freescale.com; > prabhakar@freescale.com; bhupesh.sharma@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. Thanks, Zhiqiang
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@nxp.com] >> Sent: 2016年5月11日 3:48 >> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot@lists.denx.de; >> albert.u.boot@aribaud.net; scottwood@freescale.com; >> Mingkai.hu@freescale.com; yorksun@freescale.com; leoli@freescale.com; >> prabhakar@freescale.com; bhupesh.sharma@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
Hi York, Thanks for your comments! > -----Original Message----- > From: York Sun [mailto:york.sun@nxp.com] > Sent: 2016年5月18日 23:16 > To: Zhiqiang Hou <zhiqiang.hou@nxp.com> > Cc: u-boot@lists.denx.de; albert.u.boot@aribaud.net; Scott Wood > <oss@buserror.net>; Mingkai.hu@freescale.com; leoli@freescale.com; > prabhakar@freescale.com; bhupesh.sharma@freescale.com; Hou Zhiqiang > <Zhiqiang.Hou@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@nxp.com] > >> Sent: 2016年5月11日 3:48 > >> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot@lists.denx.de; > >> albert.u.boot@aribaud.net; scottwood@freescale.com; > >> Mingkai.hu@freescale.com; yorksun@freescale.com; leoli@freescale.com; > >> prabhakar@freescale.com; bhupesh.sharma@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. 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
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); +#endif + #ifdef CONFIG_U_QE u_qe_init(); #endif diff --git a/include/configs/ls1043ardb.h b/include/configs/ls1043ardb.h index 6d35be2..4d22b63 100644 --- a/include/configs/ls1043ardb.h +++ b/include/configs/ls1043ardb.h @@ -9,6 +9,15 @@ #include "ls1043a_common.h" +#if defined(CONFIG_FSL_LS_PPA) +#define CONFIG_SYS_LS_PPA_DRAM_BLOCK_MIN_SIZE (1UL * 1024 * 1024) + +#define CONFIG_SYS_LS_PPA_FW_IN_NOR +#ifdef CONFIG_SYS_LS_PPA_FW_IN_NOR +#define CONFIG_SYS_LS_PPA_FW_ADDR 0x60500000 +#endif +#endif + #define CONFIG_DISPLAY_CPUINFO #define CONFIG_DISPLAY_BOARDINFO