[U-Boot] [PATCH resend 2/2] arm/boards: Define a new config option CONFIG_BOOT_PARAMS_P

[Vipin Kumar <vipin.kumar@st.com>]

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<vipin.kumar@st.com>
>> ---
>>   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
> .
>