diff mbox

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

Message ID 1461764367-7760-3-git-send-email-Zhiqiang.Hou@nxp.com
State Superseded
Delegated to: York Sun
Headers show

Commit Message

Z.Q. Hou April 27, 2016, 1:39 p.m. UTC 
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(+)

Comments

York Sun May 10, 2016, 7:48 p.m. UTC | #1 
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
Z.Q. Hou May 18, 2016, 7:37 a.m. UTC | #2 
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
York Sun May 18, 2016, 3:16 p.m. UTC | #3 
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
Z.Q. Hou May 24, 2016, 2:22 a.m. UTC | #4 
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 mbox

Patch

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