[U-Boot] [PATCH] powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

[U-Boot] [PATCH] powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

[Shaveta Leekha]

If hwconfig does not contains "en_cpc" then by default all cpcs are enabled
If this config is defined then only those individual cpcs which are defined
in the subargument of "en_cpc" will be enabled e.g en_cpc:cpc1,cpc2; (this
will enable cpc1 and cpc2) or en_cpc:cpc2; (this enables just cpc2)

Signed-off-by: Shaveta Leekha <shaveta@freescale.com>
Signed-off-by: Sandeep Singh <Sandeep@freescale.com>
---
 arch/powerpc/cpu/mpc85xx/cpu_init.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c
index 78316a6..ecde00b 100644
--- a/arch/powerpc/cpu/mpc85xx/cpu_init.c
+++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c
@@ -254,11 +254,38 @@ static void enable_tdm_law(void)
 static void enable_cpc(void)
 {
 	int i;
+	int ret;
 	u32 size = 0;
 
 	cpc_corenet_t *cpc = (cpc_corenet_t *)CONFIG_SYS_FSL_CPC_ADDR;
+	char buffer[HWCONFIG_BUFFER_SIZE];
+	char cpc_subarg[16];
+	bool have_hwconfig = false;
+	int cpc_args = 0;
+
+	/*
+	 * Extract hwconfig from environment since environmen
+	 * is not setup properly yet
+	 */
+	ret = getenv_f("hwconfig", buffer, sizeof(buffer));
+	if (ret > 0) {
+		/*
+		 * If "en_cpc" is not defined in hwconfig then by default all
+		 * cpcs are enable. If this config is defined then individual
+		 * cpcs which have to be enabled should also be defined.
+		 * e.g en_cpc:cpc1,cpc2;
+		 */
+		if (hwconfig_f("en_cpc", buffer))
+			have_hwconfig = true;
+	}
 
 	for (i = 0; i < CONFIG_SYS_NUM_CPC; i++, cpc++) {
+		if (have_hwconfig) {
+			sprintf(cpc_subarg, "cpc%u", i + 1);
+			cpc_args = hwconfig_sub_f("en_cpc", cpc_subarg, buffer);
+			if (cpc_args == 0)
+				continue;
+		}
 		u32 cpccfg0 = in_be32(&cpc->cpccfg0);
 		size += CPC_CFG0_SZ_K(cpccfg0);
 
-- 
1.7.6.GIT

[U-Boot] [PATCH] powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

[York Sun]

On 07/01/2014 11:14 PM, Shaveta Leekha wrote:
> If hwconfig does not contains "en_cpc" then by default all cpcs are enabled
> If this config is defined then only those individual cpcs which are defined
> in the subargument of "en_cpc" will be enabled e.g en_cpc:cpc1,cpc2; (this
> will enable cpc1 and cpc2) or en_cpc:cpc2; (this enables just cpc2)
> 

What's the user's case for enabling selected CPC?

York

[U-Boot] [PATCH] powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

[shaveta at freescale.com]

Hi York,

This change was required to provide the flexibility of enabling DDRC1/CPC1 by
SC3900/DSP core as DDRC1 is used by Starcore. SC enables CPC1 as per their requirement.
PPC core use DDRC2, so it enables DDRC2/CPC2.

Do you suggest mentioning it in the commit message also?

Thanks and Regards,
Shaveta

-----Original Message-----
From: Sun York-R58495 
Sent: Wednesday, August 06, 2014 11:04 PM
To: Leekha Shaveta-B20052; u-boot at lists.denx.de
Cc: Wood Scott-B07421; Singh Sandeep-B37400
Subject: Re: [PATCH] powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

On 07/01/2014 11:14 PM, Shaveta Leekha wrote:
> If hwconfig does not contains "en_cpc" then by default all cpcs are 
> enabled If this config is defined then only those individual cpcs 
> which are defined in the subargument of "en_cpc" will be enabled e.g 
> en_cpc:cpc1,cpc2; (this will enable cpc1 and cpc2) or en_cpc:cpc2; 
> (this enables just cpc2)
> 

What's the user's case for enabling selected CPC?

York

[U-Boot] [PATCH] powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

[York Sun]

On 07/01/2014 11:14 PM, Shaveta Leekha wrote:
> If hwconfig does not contains "en_cpc" then by default all cpcs are enabled
> If this config is defined then only those individual cpcs which are defined
> in the subargument of "en_cpc" will be enabled e.g en_cpc:cpc1,cpc2; (this
> will enable cpc1 and cpc2) or en_cpc:cpc2; (this enables just cpc2)
> 
> Signed-off-by: Shaveta Leekha <shaveta@freescale.com>
> Signed-off-by: Sandeep Singh <Sandeep@freescale.com>
> ---

Applied to u-boot-mpc85xx master branch, awaiting for upstream.

York

[U-Boot] [PATCH] powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

[Sandeep Singh]

If hwconfig does not contains "en_cpc" then by default all cpcs are enabled
If this config is defined then only those individual cpcs which are defined
in the subargument of "en_cpc" will be enabled e.g en_cpc:cpc1,cpc2; (this
will enable cpc1 and cpc2) or en_cpc:cpc2; (this enables just cpc2)

Signed-off-by: Sandeep Singh <Sandeep@freescale.com>
---
 arch/powerpc/cpu/mpc85xx/cpu_init.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c
index 185e0d5..ea75ce5 100644
--- a/arch/powerpc/cpu/mpc85xx/cpu_init.c
+++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c
@@ -159,11 +159,43 @@ void config_8560_ioports (volatile ccsr_cpm_t * cpm)
 static void enable_cpc(void)
 {
 	int i;
+	int arglen;
+	int ret;
 	u32 size = 0;
 
 	cpc_corenet_t *cpc = (cpc_corenet_t *)CONFIG_SYS_FSL_CPC_ADDR;
+	char buffer[HWCONFIG_BUFFER_SIZE];
+	char cpc_subarg[16];
+	bool have_hwconfig = 0;
+	const char *cpc_args = NULL;
+
+	/*
+	 * Extract hwconfig from environment since environment
+	 * is not setup properly yet
+	 */
+	ret = getenv_f("hwconfig", buffer, sizeof(buffer));
+	if (ret == -1) {
+		printf("Error getting hwconfig\n");
+		return;
+	}
+
+	/*
+	 * If "en_cpc" is not defined in hwconfig then by default all
+	 * cpcs are enable. If this config is defined then individual
+	 * cpcs which have to be enabled should also be defined.
+	 * e.g en_cpc:cpc1,cpc2;
+	 */
+	if (hwconfig_f("en_cpc", buffer))
+		have_hwconfig = 1;
 
 	for (i = 0; i < CONFIG_SYS_NUM_CPC; i++, cpc++) {
+		sprintf(cpc_subarg, "cpc%u", i + 1);
+		if (have_hwconfig) {
+			cpc_args = hwconfig_sub_f("en_cpc", cpc_subarg, buffer);
+			if (cpc_args == NULL)
+				continue;
+		}
+
 		u32 cpccfg0 = in_be32(&cpc->cpccfg0);
 		size += CPC_CFG0_SZ_K(cpccfg0);
 #if defined(CONFIG_RAMBOOT_PBL) || defined(CONFIG_SECURE_HKAREA_CPC)
-- 
1.7.6.GIT

[U-Boot] [PATCH] powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

[Scott Wood]

On 06/13/2013 01:59:51 AM, Sandeep Singh wrote:
> If hwconfig does not contains "en_cpc" then by default all cpcs are  
> enabled
> If this config is defined then only those individual cpcs which are  
> defined
> in the subargument of "en_cpc" will be enabled e.g en_cpc:cpc1,cpc2;  
> (this
> will enable cpc1 and cpc2) or en_cpc:cpc2; (this enables just cpc2)
> 
> Signed-off-by: Sandeep Singh <Sandeep@freescale.com>
> ---
>  arch/powerpc/cpu/mpc85xx/cpu_init.c |   32  
> ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c  
> b/arch/powerpc/cpu/mpc85xx/cpu_init.c
> index 185e0d5..ea75ce5 100644
> --- a/arch/powerpc/cpu/mpc85xx/cpu_init.c
> +++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c
> @@ -159,11 +159,43 @@ void config_8560_ioports (volatile ccsr_cpm_t *  
> cpm)
>  static void enable_cpc(void)
>  {
>  	int i;
> +	int arglen;
> +	int ret;
>  	u32 size = 0;
> 
>  	cpc_corenet_t *cpc = (cpc_corenet_t *)CONFIG_SYS_FSL_CPC_ADDR;
> +	char buffer[HWCONFIG_BUFFER_SIZE];
> +	char cpc_subarg[16];
> +	bool have_hwconfig = 0;

= false

> +	const char *cpc_args = NULL;
> +
> +	/*
> +	 * Extract hwconfig from environment since environment
> +	 * is not setup properly yet
> +	 */
> +	ret = getenv_f("hwconfig", buffer, sizeof(buffer));
> +	if (ret == -1) {
> +		printf("Error getting hwconfig\n");
> +		return;
> +	}

It is not an error for hwconfig to be missing.  That just means that no  
options are set.

> +	/*
> +	 * If "en_cpc" is not defined in hwconfig then by default all
> +	 * cpcs are enable. If this config is defined then individual
> +	 * cpcs which have to be enabled should also be defined.
> +	 * e.g en_cpc:cpc1,cpc2;
> +	 */
> +	if (hwconfig_f("en_cpc", buffer))
> +		have_hwconfig = 1;

= true

have_hwconfig should be set based on the return of getenv_f.  Is there  
really any benefit to checking whether en_cpc is present, separately  
from the hwconfig_sub_f call?

-Scott

[U-Boot] [PATCH] powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

[Singh Sandeep-B37400]

Thanks for your comments.
Please find reply inline.

Regards,
Sandeep

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, June 13, 2013 10:09 PM
> To: Singh Sandeep-B37400
> Cc: u-boot at lists.denx.de; Singh Sandeep-B37400; afleming at gmail.com
> Subject: Re: [U-Boot] [PATCH] powerpc/mpc85xx: Enabling CPC conditionally
> based on hwconfig options
> 
> On 06/13/2013 01:59:51 AM, Sandeep Singh wrote:
> > If hwconfig does not contains "en_cpc" then by default all cpcs are
> > enabled If this config is defined then only those individual cpcs
> > which are defined in the subargument of "en_cpc" will be enabled e.g
> > en_cpc:cpc1,cpc2; (this will enable cpc1 and cpc2) or en_cpc:cpc2;
> > (this enables just cpc2)
> >
> > Signed-off-by: Sandeep Singh <Sandeep@freescale.com>
> > ---
> >  arch/powerpc/cpu/mpc85xx/cpu_init.c |   32
> > ++++++++++++++++++++++++++++++++
> >  1 files changed, 32 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c
> > b/arch/powerpc/cpu/mpc85xx/cpu_init.c
> > index 185e0d5..ea75ce5 100644
> > --- a/arch/powerpc/cpu/mpc85xx/cpu_init.c
> > +++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c
> > @@ -159,11 +159,43 @@ void config_8560_ioports (volatile ccsr_cpm_t *
> > cpm)
> >  static void enable_cpc(void)
> >  {
> >  	int i;
> > +	int arglen;
> > +	int ret;
> >  	u32 size = 0;
> >
> >  	cpc_corenet_t *cpc = (cpc_corenet_t *)CONFIG_SYS_FSL_CPC_ADDR;
> > +	char buffer[HWCONFIG_BUFFER_SIZE];
> > +	char cpc_subarg[16];
> > +	bool have_hwconfig = 0;
> 
> = false
Ok
> 
> > +	const char *cpc_args = NULL;
> > +
> > +	/*
> > +	 * Extract hwconfig from environment since environment
> > +	 * is not setup properly yet
> > +	 */
> > +	ret = getenv_f("hwconfig", buffer, sizeof(buffer));
> > +	if (ret == -1) {
> > +		printf("Error getting hwconfig\n");
> > +		return;
> > +	}
> 
> It is not an error for hwconfig to be missing.  That just means that no
> options are set.
Right, will rectify
> 
> > +	/*
> > +	 * If "en_cpc" is not defined in hwconfig then by default all
> > +	 * cpcs are enable. If this config is defined then individual
> > +	 * cpcs which have to be enabled should also be defined.
> > +	 * e.g en_cpc:cpc1,cpc2;
> > +	 */
> > +	if (hwconfig_f("en_cpc", buffer))
> > +		have_hwconfig = 1;
> 
> = true
ok
> 
> have_hwconfig should be set based on the return of getenv_f.  Is there
> really any benefit to checking whether en_cpc is present, separately from
> the hwconfig_sub_f call?
This was done with the intension of providing greater configurability. When
en_cpc is defined then it's entirely up to user to decide which cpcs are to
be enabled. Hence we do following:

if_defined("en_cpc"){
	only_then
		check_for_cpc_options;
} else {
	enable_all_cpc;
}

> 
> -Scott

[U-Boot] [PATCH] powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

[Scott Wood]

On 06/14/2013 12:26:18 AM, Singh Sandeep-B37400 wrote:
> This was done with the intension of providing greater  
> configurability. When
> en_cpc is defined then it's entirely up to user to decide which cpcs  
> are to
> be enabled. Hence we do following:
> 
> if_defined("en_cpc"){
> 	only_then
> 		check_for_cpc_options;
> } else {
> 	enable_all_cpc;
> }

OK, I see.

What is the use case for enabling one cpc but not another?  What is the  
use case for disabling cpc at all?  The answer should go in the commit  
message.

-Scott

[U-Boot] [PATCH] powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

[Singh Sandeep-B37400]

Sorry somehow I missed this earlier. Please find my reply inline.

Regards,
Sandeep


> -----Original Message-----
> From: Wood Scott-B07421
> Sent: 15 June 2013 01:09
> To: Singh Sandeep-B37400
> Cc: Wood Scott-B07421; u-boot at lists.denx.de; afleming at gmail.com
> Subject: Re: [U-Boot] [PATCH] powerpc/mpc85xx: Enabling CPC conditionally
> based on hwconfig options
> 
> On 06/14/2013 12:26:18 AM, Singh Sandeep-B37400 wrote:
> > This was done with the intension of providing greater
> > configurability. When
> > en_cpc is defined then it's entirely up to user to decide which cpcs
> > are to
> > be enabled. Hence we do following:
> >
> > if_defined("en_cpc"){
> > 	only_then
> > 		check_for_cpc_options;
> > } else {
> > 	enable_all_cpc;
> > }
> 
> OK, I see.
> 
> What is the use case for enabling one cpc but not another?  What is the
> use case for disabling cpc at all?  The answer should go in the commit
> message.

In B4860, Starcore owns one CPC and second CPC is owned by PowerPC. So we need to enable only one CPC at u-boot.

> 
> -Scott