[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