[U-Boot] [PATCH 1/2] arm, lib/board.c: Coding Style cleanup

Heiko Schocher hs at denx.de
Fri Jul 15 08:57:52 CEST 2011


Hello Albert,

Albert ARIBAUD wrote:
> Hi Heiko,
> 
> Sorry for having kept this patch unanswered so long. I am now on
> holiday, which means *a bit* more time for U-Boot, so here comes:
> 
> Re: patch title, can you make it "arm: libboard.c: ..." rather than
> "arm, libboard.c" ?

Yep, do this. Also add "cosmetic" as "macpaul at gmail.com" commented.

> Also, make sure you rebase onto latest u-boot-arm/master for next
> version: this one does not apply cleanly ATM.

Yes, I saw this too now, but at the time I posted the patch,
it applied clean...

> Le 03/06/2011 10:11, Heiko Schocher a écrit :
> 
>> --- a/arch/arm/lib/board.c
>> +++ b/arch/arm/lib/board.c
>> @@ -92,26 +92,28 @@ extern void rtl8019_get_enetaddr (uchar * addr);
>>   
>> ************************************************************************
>>    * May be supplied by boards if desired
>>    */
>> -void inline __coloured_LED_init (void) {}
>> -void coloured_LED_init (void) __attribute__((weak,
>> alias("__coloured_LED_init")));
>> -void inline __red_LED_on (void) {}
>> -void red_LED_on (void) __attribute__((weak, alias("__red_LED_on")));
>> -void inline __red_LED_off(void) {}
>> +inline void __coloured_LED_init(void) {}
>> +void coloured_LED_init(void)
>> +__attribute__((weak, alias("__coloured_LED_init")));
> 
> If you break the declaration in several lines, please leave some
> indentation in the second and subsequent lines.

Ok, fixed.

>> +inline void __red_LED_on(void) {}
>> +void red_LED_on(void) __attribute__((weak, alias("__red_LED_on")));
>> +inline void __red_LED_off(void) {}
>>   void red_LED_off(void) __attribute__((weak, alias("__red_LED_off")));
>> -void inline __green_LED_on(void) {}
>> +inline void __green_LED_on(void) {}
>>   void green_LED_on(void) __attribute__((weak, alias("__green_LED_on")));
>> -void inline __green_LED_off(void) {}
>> +inline void __green_LED_off(void) {}
>>   void green_LED_off(void) __attribute__((weak,
>> alias("__green_LED_off")));
>> -void inline __yellow_LED_on(void) {}
>> +inline void __yellow_LED_on(void) {}
>>   void yellow_LED_on(void) __attribute__((weak,
>> alias("__yellow_LED_on")));
>> -void inline __yellow_LED_off(void) {}
>> +inline void __yellow_LED_off(void) {}
>>   void yellow_LED_off(void) __attribute__((weak,
>> alias("__yellow_LED_off")));
>> -void inline __blue_LED_on(void) {}
>> +inline void __blue_LED_on(void) {}
>>   void blue_LED_on(void) __attribute__((weak, alias("__blue_LED_on")));
>> -void inline __blue_LED_off(void) {}
>> +inline void __blue_LED_off(void) {}
>>   void blue_LED_off(void) __attribute__((weak, alias("__blue_LED_off")));
>>
>> -/************************************************************************
>>
>> +/*
>> +
>> ************************************************************************
>>    * Init Utilities                            *
>>   
>> ************************************************************************
>>    * Some of this code should be moved into the core functions,
>> @@ -485,34 +490,37 @@ void board_init_r (gd_t *id, ulong dest_addr)
>>       mem_malloc_init (malloc_start, TOTAL_MALLOC_LEN);
>>
>>   #if !defined(CONFIG_SYS_NO_FLASH)
>> -    puts ("Flash: ");
>> +    puts("Flash: ");
>>
>> -    if ((flash_size = flash_init ())>  0) {
>> +    flash_size = flash_init();
>> +    if (flash_size>  0) {
>>   # ifdef CONFIG_SYS_FLASH_CHECKSUM
>> -        print_size (flash_size, "");
>> +        print_size(flash_size, "");
>>           /*
>>            * Compute and print flash CRC if flashchecksum is set to 'y'
>>            *
>>            * NOTE: Maybe we should add some WATCHDOG_RESET()? XXX
>>            */
>> -        s = getenv ("flashchecksum");
>> +        s = getenv("flashchecksum");
>>           if (s&&  (*s == 'y')) {
>> -            printf ("  CRC: %08X",
>> -                crc32 (0, (const unsigned char *)
>> CONFIG_SYS_FLASH_BASE, flash_size)
>> +            printf("  CRC: %08X",
>> +                crc32(0,
>> +                (const unsigned char *) CONFIG_SYS_FLASH_BASE,
>> +                flash_size)
> 
> Pleas indent deeper for the last two lines that belong to the call to
> crc32.

Ups, I think, the crc32 call could be in the previous line! Fixed.

Thanks for the review!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


More information about the U-Boot mailing list