[U-Boot] [PATCH] blackfin: Fix using gd->baudrate before setting its value

[Axel Lin]

2013/6/29 Marek Vasut <marex at denx.de>:
> Dear Axel Lin,
>
>> Current code uses gd->baudrate before setting its value.
>> Besides, I got below build warning which is introduced by
>> commit ddb5c5be "blackfin: add baudrate to bdinfo".
>>
>> board.c:235:3: warning: passing argument 1 of 'simple_strtoul' makes
>> pointer from integer without a cast [enabled by default]
>> include/vsprintf.h:27:7: note: expected 'const char *' but argument is of
>> type 'unsigned int'
>>
>> This patch moves the code using gd->baudrate to be after init_baudrate()
>> call, this ensures we get the baudrate setting before using it.
>>
>> Signed-off-by: Axel Lin <axel.lin at ingics.com>
>> ---
>> I forgot to CC u-boot mail list. here is a resend.
>>
>> Hi,
>> I don't have this hardware to test.
>> I'd appreciate if someone can test it.
>>
>> Thanks,
>> Axel
>>  arch/blackfin/lib/board.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/blackfin/lib/board.c b/arch/blackfin/lib/board.c
>> index f1d5547..9e2e9de 100644
>> --- a/arch/blackfin/lib/board.c
>> +++ b/arch/blackfin/lib/board.c
>> @@ -231,8 +231,6 @@ static int global_board_data_init(void)
>>       bd->bi_sclk = get_sclk();
>>       bd->bi_memstart = CONFIG_SYS_SDRAM_BASE;
>>       bd->bi_memsize = CONFIG_SYS_MAX_RAM_SIZE;
>> -     bd->bi_baudrate = (gd->baudrate > 0)
>> -             ? simple_strtoul(gd->baudrate, NULL, 10) : CONFIG_BAUDRATE;
>
>
> I'd rather say the fix here is to use (gd->baudrate > 0) ? gd->baudrate ?
> CONFIG_BAUDRATE ;
>
> Otherwise you're changing the logic of the code and for that, you'd need Mikes'
> ack.
Hi Marek,

It's because gd->baudrate is set in init_baudrate().
And if it is not found in an environment variable, the default is
CONFIG_BAUDRATE.
It's unlikely gd->baudrate is 0.

So if we have below code:
init_baudrate();
gd->bd->bi_baudrate = (gd->baudrate > 0) ? gd->baudrate : CONFIG_BAUDRATE;

In an unlikely case where baudrate environment variable is set to 0,
then we have inconsistency setting:

gd->baudrate = 0;
gd->bd->bi_baudrate = CONFIG_BAUDRATE;


Comments?

Regards,
Axel