| Message ID | 1371106791-27581-1-git-send-email-Sandeep@freescale.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Andy Fleming |
| Headers | show |
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
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@lists.denx.de; Singh Sandeep-B37400; afleming@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
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@lists.denx.de; afleming@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
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)
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(-)