powerpc/mpc85xx: Enabling CPC conditionally based on hwconfig options
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
@@ -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
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)Signed-off-by Sandeep Singh <[email protected]>--- arch/powerpc/cpu/mpc85xx/cpu_init.c | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-)arch/powerpc/cpu/mpc85xx/cpu_init.c +0
-0@@ -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
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 optionsQuoted 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)Signed-off-by Sandeep Singh <[email protected]>--- arch/powerpc/cpu/mpc85xx/cpu_init.c | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-)arch/powerpc/cpu/mpc85xx/cpu_init.c +2
-0@@ -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
-0cpc_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_hwconfigshould be set based on the return ofgetenv_f. Is there really any benefit to checking whetheren_cpcis present, separately from thehwconfig_sub_fcall?
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
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_cpcis 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
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 optionsQuoted 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_cpcis 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