[U-Boot] [PATCH ARM 2/3 v2] s3c24x0 code style changes

kevin.morfitt at fearnside-systems.co.uk kevin.morfitt at fearnside-systems.co.uk
Thu Feb 11 10:40:41 CET 2010


Hi Minkyu

Thanks for your comments...

On 11/02/2010 08:37, Minkyu Kang wrote:
> Dear Kevin Morfitt,
>
> On 11 February 2010 00:20, Kevin Morfitt
> <kevin.morfitt at fearnside-systems.co.uk>  wrote:
>> Changes the s3c24x0 files to meet the code style requirements.
>>
>> Signed-off-by: Kevin Morfitt<kevin.morfitt at fearnside-systems.co.uk>
>> ---
>>
>> v2 changes:
>> - re-number to be 2/3
>>
>> checkpatch.pl reports no errors.
>>
>>   board/MAI/AmigaOneG3SE/video.c    |    1 -
>>   board/mpl/common/common_util.h    |    1 +
>>   board/mpl/common/flash.c          |    1 -
>>   board/mpl/vcma9/cmd_vcma9.c       |    9 -
>>   board/mpl/vcma9/vcma9.c           |   27 ++--
>>   board/mpl/vcma9/vcma9.h           |    6 +-
>>   board/samsung/smdk2400/smdk2400.c |   12 +-
>>   board/sbc2410x/sbc2410x.c         |    6 +-
>>   board/trab/auto_update.c          |   30 +---
>>   board/trab/cmd_trab.c             |  213 +++++++++++-------------
>>   board/trab/rs485.c                |   16 +-
>>   board/trab/trab.c                 |   43 ++---
>>   board/trab/trab.h                 |   38 +++++
>>   board/trab/trab_fkt.c             |  331 ++++++++++++++++---------------------
>>   board/trab/tsc2000.c              |   12 +-
>>   board/trab/tsc2000.h              |    1 +
>>   board/trab/vfd.c                  |   70 ++++----
>>   common/cmd_version.c              |    2 -
>>   common/cmd_vfd.c                  |    2 -
>>   common/main.c                     |    5 +-
>>   drivers/i2c/s3c24x0_i2c.c         |   71 ++++-----
>>   drivers/video/cfb_console.c       |    1 -
>>   include/common.h                  |   14 ++
>>   lib_generic/display_options.c     |    2 -
>>   24 files changed, 418 insertions(+), 496 deletions(-)
>>   create mode 100644 board/trab/trab.h
>>
<snip>
>>
>> -extern void print_vcma9_info(void);
>> -extern int vcma9_cantest(int);
>> -extern int vcma9_nandtest(void);
>> -extern int vcma9_nanderase(void);
>> -extern int vcma9_nandread(ulong);
>> -extern int vcma9_nandwrite(ulong);
>> -extern int vcma9_dactest(int);
>> -extern int do_mplcommon(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
>> -
>>   /* ------------------------------------------------------------------------- */
>>
>
> removing extern functions and values are ok.
> but I wonder if it occur compile warnings.
> Did you test it?

I added a new file, trab.h, with the function prototypes for the
external functions. There were no new build warnings.

>
>>   int do_vcma9(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>> diff --git a/board/mpl/vcma9/vcma9.c b/board/mpl/vcma9/vcma9.c
>> index 9a7c0fa..5401664 100644
>> --- a/board/mpl/vcma9/vcma9.c
>> +++ b/board/mpl/vcma9/vcma9.c
>> @@ -137,22 +137,19 @@ int board_init(void)
>>   * NAND flash initialization.
>>   */
>>   #if defined(CONFIG_CMD_NAND)
>> -extern ulong
>> -nand_probe(ulong physadr);
>> -
>> -
>>   static inline void NF_Reset(void)
>>   {
>>         int i;
>>
>>         NF_SetCE(NFCE_LOW);
>> -       NF_Cmd(0xFF);                   /* reset command */
>> -       for (i = 0; i<  10; i++);       /* tWB = 100ns. */
>> -       NF_WaitRB();                    /* wait 200~500us; */
>> +       NF_Cmd(0xFF);           /* reset command */
>> +       /* tWB = 100ns. */
>> +       for (i = 0; i<  10; i++)
>> +               /* delay */;
>> +       NF_WaitRB();            /* wait 200~500us; */
>>         NF_SetCE(NFCE_HIGH);
>>   }
>>
<snip>
>>   static u8 Get_PLD_Revision(void)
>>   {
>> -       return (Get_PLD_ID()&  0x0F);
>> +       return Get_PLD_ID()&  0x0F;
>>   }
>>
>>   static uchar Get_Board_PCB(void)
>>   {
>> -       return (((Get_PLD_BOARD()>>  4)&  0x03) + 'A');
>> +       return ((Get_PLD_BOARD()>>  4)&  0x03) + 'A';
>>   }
>>
>>   static u8 Get_SDRAM_ChipNr(void)
>
> Do you have plan for clean up such upper case function names?

I didn't notice those, probably because checkpatch doesn't warn about
them. I'll change it.

>
>> @@ -307,7 +304,7 @@ int checkboard(void)
>>                 s[5] = 0;
>>                 Show_VCMA9_Info(s,&s[6]);
>>         }
>> -       return (0);
>> +       return 0;
>>   }
>>
>>   int last_stage_init(void)
>> diff --git a/board/mpl/vcma9/vcma9.h b/board/mpl/vcma9/vcma9.h
>> index e1ec1fe..adc7673 100644
>> --- a/board/mpl/vcma9/vcma9.h
>> +++ b/board/mpl/vcma9/vcma9.h
>> @@ -27,7 +27,8 @@
>>
>>   #include<asm/arch/s3c24x0_cpu.h>
>>
>> -extern int mem_test(unsigned long start, unsigned long ramsize, int mode);
>> +int mem_test(unsigned long start, unsigned long ramsize, int mode);
>> +int vcma9_cantest(int);
>>   void print_vcma9_info(void);
>>
>>   #if defined(CONFIG_CMD_NAND)
>> @@ -81,7 +82,8 @@ static inline void NF_WaitRB(void)
>>   {
>>         struct s3c2410_nand * const nand = s3c2410_get_base_nand();
>>
>> -       while (!(nand->NFSTAT&  (1<<  0)));
>> +       while (!(nand->NFSTAT&  (1<<  0)))
>> +               /* Wait */;
>>   }
>>
>>   static inline void NF_Write(u8 data)
>> diff --git a/board/samsung/smdk2400/smdk2400.c b/board/samsung/smdk2400/smdk2400.c
>> index 5825ce9..a1b0e0c 100644
>> --- a/board/samsung/smdk2400/smdk2400.c
>> +++ b/board/samsung/smdk2400/smdk2400.c
>> @@ -33,18 +33,13 @@ DECLARE_GLOBAL_DATA_PTR;
>>
>>   #ifdef CONFIG_MODEM_SUPPORT
>>   static int key_pressed(void);
>> -int mdm_init (bd_t *);
>> -extern void disable_putc(void);
>> -extern void enable_putc(void);
>> -extern int hwflow_onoff(int);
>> -extern int do_mdm_init; /* defined in common/main.c */
>>   #endif /* CONFIG_MODEM_SUPPORT */
>>
>>   /*
>>   * Miscellaneous platform dependent initialisations
>>   */
>>
>> -int board_init (void)
>> +int board_init(void)
>>   {
>>         struct s3c24x0_clock_power * const clk_power =
>>                                         s3c24x0_get_base_clock_power();
>> @@ -93,7 +88,7 @@ int board_init (void)
>>         return 0;
>>   }
>>
>> -int dram_init (void)
>> +int dram_init(void)
>>   {
>>         gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
>>         gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
>> @@ -105,9 +100,8 @@ int dram_init (void)
>>   static int key_pressed(void)
>>   {
>>         int rc;
>> -       if (1) {        /* check for button push here, now just return 1 */
>> +       if (1)  /* check for button push here, now just return 1 */
>
> Please remove this if (1).

OK, I'll also clear up the upper case function names mentioned above.

>
>>                 rc = 1;
>> -       }
>>
>>         return rc;
<snip>
>>   }
>>   #ifdef CONFIG_PREBOOT
>> @@ -297,11 +287,11 @@ static char *key_match(ulong kbd_data)
>>                               cmd_name,
>>                               getenv(cmd_name) ?
>>                               getenv(cmd_name) : "<UNDEFINED>");
>> -                       return (getenv(cmd_name));
>> +                       return getenv(cmd_name);
>>                 }
>>         }
>>         debug("key_match: no match\n");
>> -       return (NULL);
>> +       return NULL;
>>   }
>>   #endif /* CONFIG_PREBOOT */
>>
>> @@ -328,7 +318,7 @@ U_BOOT_CMD(kbd, 1, 1, do_kbd, "read keyboard status", "");
>>   #ifdef CONFIG_MODEM_SUPPORT
>>   static int key_pressed(void)
>>   {
>> -       return (compare_magic(KBD_DATA, CONFIG_MODEM_KEY_MAGIC) == 0);
>> +       return compare_magic(KBD_DATA, CONFIG_MODEM_KEY_MAGIC) == 0;
>
> return !compare_magic(KBD_DATA, CONFIG_MODEM_KEY_MAGIC); is better.

I changed it because checkpatch warned that 'return is not a function',
but I think you're right, it's better as it was - I'll change it.

>
>>   }
>>   #endif /* CONFIG_MODEM_SUPPORT */
>>
>> @@ -370,9 +360,8 @@ static void spi_init(void)
>>         spi->ch[0].SPCON = 0x1A;
>>
>>         /* Dummy byte ensures clock to be low. */
>> -       for (i = 0; i<  10; i++) {
>> +       for (i = 0; i<  10; i++)
>>                 spi->ch[0].SPTDAT = 0xFF;
>> -       }
>>         wait_transmit_done();
>>   }
>>
>> @@ -380,8 +369,8 @@ static void wait_transmit_done(void)
>>   {
>>         struct s3c24x0_spi * const spi = s3c24x0_get_base_spi();
>>
>> -       /* wait until transfer is done */
>> -       while (!(spi->ch[0].SPSTA&  0x01));
>> +       while (!(spi->ch[0].SPSTA&  0x01))
>> +               /* wait until transfer is done */;
>>   }
>>
>>   static void tsc2000_write(unsigned int page, unsigned int reg,
>> @@ -417,7 +406,7 @@ static void tsc2000_set_brightness(void)
>>
>>         i = getenv_r("brightness", tmp, sizeof(tmp));
>>         br = (i>  0) ? (int)simple_strtoul(tmp, NULL, 10) :
>> -                       CONFIG_SYS_BRIGHTNESS;
>> +            CONFIG_SYS_BRIGHTNESS;
>>
>>         tsc2000_write(0, 0xb, br&  0xff);
>>   }
>
> Thanks
> Minkyu Kang


More information about the U-Boot mailing list