Patchwork [U-Boot] hwconfig: Move HWCONFIG_BUFFER_SIZE into hwconfig.h

login
register
mail settings
Submitter York Sun
Date Aug. 16, 2012, 12:53 a.m.
Message ID <1345078436-25476-1-git-send-email-yorksun@freescale.com>
Download mbox | patch
Permalink /patch/177884/
State Superseded
Headers show

Comments

York Sun - Aug. 16, 2012, 12:53 a.m.
Before proper environment is setup, we extract hwconfig and put it into a
buffer with size HWCONFIG_BUFFER_SIZE. We need to enlarge the buffer to
accommodate longer string. Since this macro is used in multiple files, we
move it into hwconfig.h.

Signed-off-by: York Sun <yorksun@freescale.com>
---

 arch/powerpc/cpu/mpc85xx/cpu_init.c           |    2 --
 arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c |    2 --
 arch/powerpc/cpu/mpc8xxx/ddr/options.c        |    1 -
 include/hwconfig.h                            |    2 ++
 4 files changed, 2 insertions(+), 5 deletions(-)
Kumar Gala - Aug. 17, 2012, 5:54 p.m.
On Aug 15, 2012, at 7:53 PM, York Sun wrote:

> Before proper environment is setup, we extract hwconfig and put it into a
> buffer with size HWCONFIG_BUFFER_SIZE. We need to enlarge the buffer to
> accommodate longer string. Since this macro is used in multiple files, we
> move it into hwconfig.h.
> 
> Signed-off-by: York Sun <yorksun@freescale.com>
> ---
> 
> arch/powerpc/cpu/mpc85xx/cpu_init.c           |    2 --
> arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c |    2 --
> arch/powerpc/cpu/mpc8xxx/ddr/options.c        |    1 -
> include/hwconfig.h                            |    2 ++
> 4 files changed, 2 insertions(+), 5 deletions(-)

1. I don't think HWCONFIG_BUFFER_SIZE should be getting set in include/hwconfig.h, this is unique to FSL 8xxx usage not to hwconfig
2. if you are doing to something like this it should be:

	#ifndef CONFIG_HWCONFIG_BUFFER_SIZE
	  #define CONFIG_HWCONFIG_BUFFER_SIZE 128
	#endif

This allowing a board port to set it as needed.

- k

> 
> diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c
> index c13c45f..2397547 100644
> --- a/arch/powerpc/cpu/mpc85xx/cpu_init.c
> +++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c
> @@ -56,8 +56,6 @@
> 
> #include "../../../../drivers/block/fsl_sata.h"
> 
> -#define HWCONFIG_BUFFER_SIZE 128
> -
> DECLARE_GLOBAL_DATA_PTR;
> 
> #ifdef CONFIG_SECURE_BOOT
> diff --git a/arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c b/arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c
> index 9300e7f..69399aa 100644
> --- a/arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c
> +++ b/arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c
> @@ -46,8 +46,6 @@
> 
> static u32 serdes_prtcl_map;
> 
> -#define HWCONFIG_BUFFER_SIZE	128
> -
> #ifdef DEBUG
> static const char *serdes_prtcl_str[] = {
> 	[NONE] = "NA",
> diff --git a/arch/powerpc/cpu/mpc8xxx/ddr/options.c b/arch/powerpc/cpu/mpc8xxx/ddr/options.c
> index 00ec57b..24c8a51 100644
> --- a/arch/powerpc/cpu/mpc8xxx/ddr/options.c
> +++ b/arch/powerpc/cpu/mpc8xxx/ddr/options.c
> @@ -19,7 +19,6 @@
> * This is pretty fragile on both the use of stack and if the buffer is big
> * enough. However we will get a warning from getenv_f for the later.
> */
> -#define HWCONFIG_BUFFER_SIZE	128
> 
> /* Board-specific functions defined in each board's ddr.c */
> extern void fsl_ddr_board_options(memctl_options_t *popts,
> diff --git a/include/hwconfig.h b/include/hwconfig.h
> index a037ed8..64861c4 100644
> --- a/include/hwconfig.h
> +++ b/include/hwconfig.h
> @@ -18,6 +18,8 @@
> #include <linux/types.h>
> #include <asm/errno.h>
> 
> +#define HWCONFIG_BUFFER_SIZE 256
> +
> #ifdef CONFIG_HWCONFIG
> 
> extern int hwconfig_f(const char *opt, char *buf);
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
York Sun - Aug. 17, 2012, 6 p.m.
On Fri, 2012-08-17 at 12:54 -0500, Kumar Gala wrote:
> On Aug 15, 2012, at 7:53 PM, York Sun wrote:
> 
> > Before proper environment is setup, we extract hwconfig and put it into a
> > buffer with size HWCONFIG_BUFFER_SIZE. We need to enlarge the buffer to
> > accommodate longer string. Since this macro is used in multiple files, we
> > move it into hwconfig.h.
> > 
> > Signed-off-by: York Sun <yorksun@freescale.com>
> > ---
> > 
> > arch/powerpc/cpu/mpc85xx/cpu_init.c           |    2 --
> > arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c |    2 --
> > arch/powerpc/cpu/mpc8xxx/ddr/options.c        |    1 -
> > include/hwconfig.h                            |    2 ++
> > 4 files changed, 2 insertions(+), 5 deletions(-)
> 
> 1. I don't think HWCONFIG_BUFFER_SIZE should be getting set in include/hwconfig.h, this is unique to FSL 8xxx usage not to hwconfig

If no one else uses this feature, I can move it to
arch/powerpc/include/asm/config.h. How about that?

> 2. if you are doing to something like this it should be:
> 
> 	#ifndef CONFIG_HWCONFIG_BUFFER_SIZE
> 	  #define CONFIG_HWCONFIG_BUFFER_SIZE 128
> 	#endif
> 
> This allowing a board port to set it as needed.
> 

This is a good idea. Will do.

York
Wolfgang Denk - Sept. 2, 2012, 12:43 p.m.
Dear York Sun,

In message <1345226412.6510.38.camel@oslab-l1> you wrote:
> On Fri, 2012-08-17 at 12:54 -0500, Kumar Gala wrote:
> > On Aug 15, 2012, at 7:53 PM, York Sun wrote:
> > 
> > > Before proper environment is setup, we extract hwconfig and put it into a
> > > buffer with size HWCONFIG_BUFFER_SIZE. We need to enlarge the buffer to
> > > accommodate longer string. Since this macro is used in multiple files, we
> > > move it into hwconfig.h.
> > > 
> > > Signed-off-by: York Sun <yorksun@freescale.com>
> > > ---
> > > 
> > > arch/powerpc/cpu/mpc85xx/cpu_init.c           |    2 --
> > > arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c |    2 --
> > > arch/powerpc/cpu/mpc8xxx/ddr/options.c        |    1 -
> > > include/hwconfig.h                            |    2 ++
> > > 4 files changed, 2 insertions(+), 5 deletions(-)
> > 
> > 1. I don't think HWCONFIG_BUFFER_SIZE should be getting set in include/hwconfig.h, this is unique to FSL 8xxx usage not to hwconfig
> 
> If no one else uses this feature, I can move it to
> arch/powerpc/include/asm/config.h. How about that?

I cannot see in which way arch/powerpc/include/asm/config.h would be
more FSL 8xxx specific ?

Best regards,

Wolfgang Denk
York Sun - Sept. 4, 2012, 4:16 p.m.
On 09/02/2012 05:43 AM, Wolfgang Denk wrote:
> Dear York Sun,
>
> In message <1345226412.6510.38.camel@oslab-l1> you wrote:
>> On Fri, 2012-08-17 at 12:54 -0500, Kumar Gala wrote:
>>> On Aug 15, 2012, at 7:53 PM, York Sun wrote:
>>>
>>>> Before proper environment is setup, we extract hwconfig and put it into a
>>>> buffer with size HWCONFIG_BUFFER_SIZE. We need to enlarge the buffer to
>>>> accommodate longer string. Since this macro is used in multiple files, we
>>>> move it into hwconfig.h.
>>>>
>>>> Signed-off-by: York Sun <yorksun@freescale.com>
>>>> ---
>>>>
>>>> arch/powerpc/cpu/mpc85xx/cpu_init.c           |    2 --
>>>> arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c |    2 --
>>>> arch/powerpc/cpu/mpc8xxx/ddr/options.c        |    1 -
>>>> include/hwconfig.h                            |    2 ++
>>>> 4 files changed, 2 insertions(+), 5 deletions(-)
>>>
>>> 1. I don't think HWCONFIG_BUFFER_SIZE should be getting set in include/hwconfig.h, this is unique to FSL 8xxx usage not to hwconfig
>>
>> If no one else uses this feature, I can move it to
>> arch/powerpc/include/asm/config.h. How about that?
>
> I cannot see in which way arch/powerpc/include/asm/config.h would be
> more FSL 8xxx specific ?
>

I can put it within

#if defined(CONFIG_MPC85xx) || \
     defined(CONFIG_MPC83xx) || \
     defined(CONFIG_MPC86xx)
#ednif

How about that?

York

Patch

diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c
index c13c45f..2397547 100644
--- a/arch/powerpc/cpu/mpc85xx/cpu_init.c
+++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c
@@ -56,8 +56,6 @@ 
 
 #include "../../../../drivers/block/fsl_sata.h"
 
-#define HWCONFIG_BUFFER_SIZE 128
-
 DECLARE_GLOBAL_DATA_PTR;
 
 #ifdef CONFIG_SECURE_BOOT
diff --git a/arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c b/arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c
index 9300e7f..69399aa 100644
--- a/arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c
+++ b/arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c
@@ -46,8 +46,6 @@ 
 
 static u32 serdes_prtcl_map;
 
-#define HWCONFIG_BUFFER_SIZE	128
-
 #ifdef DEBUG
 static const char *serdes_prtcl_str[] = {
 	[NONE] = "NA",
diff --git a/arch/powerpc/cpu/mpc8xxx/ddr/options.c b/arch/powerpc/cpu/mpc8xxx/ddr/options.c
index 00ec57b..24c8a51 100644
--- a/arch/powerpc/cpu/mpc8xxx/ddr/options.c
+++ b/arch/powerpc/cpu/mpc8xxx/ddr/options.c
@@ -19,7 +19,6 @@ 
  * This is pretty fragile on both the use of stack and if the buffer is big
  * enough. However we will get a warning from getenv_f for the later.
  */
-#define HWCONFIG_BUFFER_SIZE	128
 
 /* Board-specific functions defined in each board's ddr.c */
 extern void fsl_ddr_board_options(memctl_options_t *popts,
diff --git a/include/hwconfig.h b/include/hwconfig.h
index a037ed8..64861c4 100644
--- a/include/hwconfig.h
+++ b/include/hwconfig.h
@@ -18,6 +18,8 @@ 
 #include <linux/types.h>
 #include <asm/errno.h>
 
+#define HWCONFIG_BUFFER_SIZE 256
+
 #ifdef CONFIG_HWCONFIG
 
 extern int hwconfig_f(const char *opt, char *buf);