Skip to contentSkip to conversation

powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

PATCH New Sandeep Singh 3 participants 5 messages Jun 13, 2013 – Jul 2, 2013 PowerPC

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

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)

---
 arch/powerpc/cpu/mpc85xx/cpu_init.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)
arch/powerpc/cpu/mpc85xx/cpu_init.c +32 -0
index 185e0d5..ea75ce5 100644
@@ -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)
Signature · 1 line
1.7.6.GIT
#1 lore raw

PATCH 1/1 powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

Quoted 22 lines · 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)

---
 arch/powerpc/cpu/mpc85xx/cpu_init.c |   32  
++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)
arch/powerpc/cpu/mpc85xx/cpu_init.c +0 -0
index 185e0d5..ea75ce5 100644
@@ -159,11 +159,43 @@ void config_8560_ioports (volatile ccsr_cpm_t *  
 cpm)
 static void enable_cpc(void)

arch/powerpc/cpu/mpc85xx/cpu_init.c void config_8560_ioports (volatile ccsr_cpm_t * · line 170

Quoted 10 lines
arch/powerpc/cpu/mpc85xx/cpu_init.c +5 -0
 {
 	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

arch/powerpc/cpu/mpc85xx/cpu_init.c void config_8560_ioports (volatile ccsr_cpm_t * cpm) · line 180

Quoted 11 lines
arch/powerpc/cpu/mpc85xx/cpu_init.c +11 -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;
+	}

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

arch/powerpc/cpu/mpc85xx/cpu_init.c void config_8560_ioports (volatile ccsr_cpm_t * cpm) · line 189

Quoted 8 lines
arch/powerpc/cpu/mpc85xx/cpu_init.c +8 -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 = 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

#2 lore raw

PATCH 1/1 powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

Thanks for your comments.
Please find reply inline.

Regards,
Sandeep

Quoted 35 lines

-----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

Quoted 26 lines · 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)

---
 arch/powerpc/cpu/mpc85xx/cpu_init.c |   32
++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)
arch/powerpc/cpu/mpc85xx/cpu_init.c +2 -0
index 185e0d5..ea75ce5 100644
@@ -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;
 
Quoted 6 lines

arch/powerpc/cpu/mpc85xx/cpu_init.c void config_8560_ioports (volatile ccsr_cpm_t * cpm) · line 169

Quoted 4 lines
arch/powerpc/cpu/mpc85xx/cpu_init.c +3 -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

Quoted 15 lines

arch/powerpc/cpu/mpc85xx/cpu_init.c void config_8560_ioports (volatile ccsr_cpm_t * cpm) · line 180

Quoted 12 lines
arch/powerpc/cpu/mpc85xx/cpu_init.c +11 -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;
+	}

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

Right, will rectify

Quoted 11 lines

arch/powerpc/cpu/mpc85xx/cpu_init.c void config_8560_ioports (volatile ccsr_cpm_t * cpm) · line 189

Quoted 9 lines
arch/powerpc/cpu/mpc85xx/cpu_init.c +8 -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 = 1;

= true

ok

Quoted 4 lines

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;
}
Quoted 2 lines

-Scott

#3 lore raw

PATCH 1/1 powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

Quoted 6 lines · 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:

Quoted 6 lines
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

#4 lore raw

PATCH 1/1 powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options

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

Regards,
Sandeep

Quoted 22 lines

-----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

Quoted 6 lines · 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:

Quoted 6 lines
if_defined("en_cpc"){
	only_then
		check_for_cpc_options;
} else {
	enable_all_cpc;
}
Quoted 5 lines

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.

Quoted 2 lines

-Scott