From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vipin Kumar Date: Thu, 6 Dec 2012 15:26:37 +0530 Subject: [U-Boot] [PATCH resend 2/2] arm/boards: Define a new config option CONFIG_BOOT_PARAMS_P In-Reply-To: <50C068F4.3000107@denx.de> References: <0914ca07f3fa7549188e349e7a0a32c3d6521321.1354786138.git.vipin.kumar@st.com> <50C068F4.3000107@denx.de> Message-ID: <50C06BD5.7010507@st.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 12/6/2012 3:14 PM, Stefan Roese wrote: > On 12/06/2012 10:29 AM, Vipin Kumar wrote: >> A lot of ARM boards are using board_init routine just to initialize boot_params >> variable in the global data structure. >> >> This patch lets the board config files to define a CONFIG_BOOT_PARAMS_P option >> which is assigned to gd->bd->bi_boot_params automatically >> >> Consequently, many board_init routines would not be required in the respective >> board directories and a weak definition becomes necessary before their removal >> from the code. >> >> Signed-off-by: Vipin Kumar >> --- >> README | 6 ++++++ >> arch/arm/lib/board.c | 12 ++++++++++++ >> 2 files changed, 18 insertions(+) >> >> diff --git a/README b/README >> index 037513a..2077c3b 100644 >> --- a/README >> +++ b/README >> @@ -550,6 +550,12 @@ The following options need to be configured: >> in a single configuration file and the machine type is >> runtime discoverable, do not have to use this setting. >> >> + CONFIG_BOOT_PARAMS_P [relevant for ARM only] >> + >> + This config option can provide a way to initialize >> + bi_boot_params from the u-boot infrastructure itself. The >> + board still has the option to override it in board_init routine >> + >> - vxWorks boot parameters: >> >> bootvx constructs a valid bootline using the following >> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c >> index 92cad9a..fa161b8 100644 >> --- a/arch/arm/lib/board.c >> +++ b/arch/arm/lib/board.c >> @@ -399,6 +399,11 @@ void board_init_f(ulong bootflag) >> gd->bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */ >> #endif >> >> +#ifdef CONFIG_BOOT_PARAMS_P >> + /* Boot params passed to Linux */ >> + gd->bd->bi_boot_params = CONFIG_BOOT_PARAMS_P; >> +#endif > > Again an ugly #ifdef. Why not something like this instead: > > Define a default earlier in the code (is 0x100 the best default?): > > #ifndef CONFIG_BOOT_PARAMS_P > #define CONFIG_BOOT_PARAMS_P 0x100 > #endif > > then here just: > > /* Boot params passed to Linux */ > gd->bd->bi_boot_params = CONFIG_BOOT_PARAMS_P; > This would mean that I am forcing the boot params at 0x100 for all boards. Is that the right thing to do Off-course, all of them might already be initializing gd->bd->bi_boot_params on their own > without the #ifdef. > >> + >> addr_sp -= sizeof (gd_t); >> id = (gd_t *) addr_sp; >> debug("Reserving %zu Bytes for Global Data at: %08lx\n", >> @@ -468,6 +473,13 @@ void board_init_f(ulong bootflag) >> static char *failed = "*** failed ***\n"; >> #endif >> >> +static int __def_board_init(bd_t *bis) >> +{ >> + return -1; > > Is -1 a good value to return as default board_init()? > The return value is not checked as of today >> +} >> + >> +int board_init(void) __attribute__((weak, alias("__def_board_init"))); >> + > > Use __weak from inlcude/linux/compiler.h instead: > > int __weak board_init(bd_t *bis) > ... OK, I would do that in v2 Vipin > > > Thanks, > Stefan > . >