Dear "Hui.Tang", In message <e934d0d28a38c4eec2b08862b258bf4b1e5e3403.1256898456.git.zetal...@gmail.com> you wrote: > New Board GEC2410 Setup.
Please see below for checkpatch.pl output for your patch series (note that you should have resolved all tehse issues _before_ submitting the patches). > Signed-off-by: Hui.Tang <zetal...@gmail.com> > --- > board/gec/gec2410/Makefile | 54 +++++ > board/gec/gec2410/README | 85 ++++++++ > board/gec/gec2410/config.mk | 32 +++ > board/gec/gec2410/flash.c | 417 > +++++++++++++++++++++++++++++++++++++ > board/gec/gec2410/gec2410.c | 150 +++++++++++++ > board/gec/gec2410/lowlevel_init.S | 171 +++++++++++++++ > board/gec/gec2410/u-boot-nand.lds | 61 ++++++ > 7 files changed, 970 insertions(+), 0 deletions(-) > create mode 100644 board/gec/gec2410/Makefile > create mode 100644 board/gec/gec2410/README > create mode 100644 board/gec/gec2410/config.mk > create mode 100644 board/gec/gec2410/flash.c > create mode 100644 board/gec/gec2410/gec2410.c > create mode 100644 board/gec/gec2410/lowlevel_init.S > create mode 100644 board/gec/gec2410/u-boot-nand.lds For a new board support, entries to the top level Makefile and to the MAINTAINERS and MAKEALL files are missing. > diff --git a/board/gec/gec2410/flash.c b/board/gec/gec2410/flash.c > new file mode 100644 > index 0000000..ab418b1 > --- /dev/null > +++ b/board/gec/gec2410/flash.c This looks very much like a CFI compatible flash device. Why do you think you need a custom flash driver? > diff --git a/board/gec/gec2410/gec2410.c b/board/gec/gec2410/gec2410.c > new file mode 100644 > index 0000000..543ceeb > --- /dev/null > +++ b/board/gec/gec2410/gec2410.c > @@ -0,0 +1,150 @@ > +/* > + * (C) Copyright 2002 > + * Sysgo Real-Time Solutions, GmbH <www.elinos.com> > + * Marius Groeger <mgroe...@sysgo.de> > + * > + * (C) Copyright 2002 > + * David Mueller, ELSOFT AG, <d.muel...@elsoft.ch> > + * > + * (C) Copyright 2009 > + * Hui Tang, <zetal...@gmail.com> > + * > + * See file CREDITS for list of people who contributed to this > + * project. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of > + * the License, or (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, > + * MA 02111-1307 USA > + */ > + > +#include <common.h> > +#include <netdev.h> > +#include <s3c2410.h> > + > +DECLARE_GLOBAL_DATA_PTR; > + > +#define FCLK_SPEED 1 > + > +#if FCLK_SPEED == 0 /* Fout = 203MHz, Fin = 12MHz for Audio */ > +#define M_MDIV 0xC3 > +#define M_PDIV 0x4 > +#define M_SDIV 0x1 > +#elif FCLK_SPEED == 1 /* Fout = 202.8MHz */ > +#define M_MDIV 0xA1 > +#define M_PDIV 0x3 > +#define M_SDIV 0x1 > +#endif > + > +#define USB_CLOCK 1 > + > +#if USB_CLOCK == 0 > +#define U_M_MDIV 0xA1 > +#define U_M_PDIV 0x3 > +#define U_M_SDIV 0x1 > +#elif USB_CLOCK == 1 > +#define U_M_MDIV 0x48 > +#define U_M_PDIV 0x3 > +#define U_M_SDIV 0x2 > +#endif > + > +static inline void delay(unsigned long loops) > +{ > + __asm__ volatile ("1:\n" > + "subs %0, %1, #1\n" > + "bne 1b" : "=r" (loops) : "0" (loops)); > +} > + > +/* > + * Miscellaneous platform dependent initialisations > + */ > + > +int board_init(void) > +{ > + struct s3c24x0_clock_power * const clk_power = > + s3c24x0_get_base_clock_power(); > + struct s3c24x0_gpio * const gpio = s3c24x0_get_base_gpio(); > + > + /* to reduce PLL lock time, adjust the LOCKTIME register */ > + clk_power->LOCKTIME = 0xFFFFFF; > + > + /* configure MPLL */ > + clk_power->MPLLCON = ((M_MDIV << 12) + (M_PDIV << 4) + M_SDIV); > + > + /* some delay between MPLL and UPLL */ > + delay(4000); > + > + /* configure UPLL */ > + clk_power->UPLLCON = ((U_M_MDIV << 12) + (U_M_PDIV << 4) + U_M_SDIV); Please use I/O accessors to access device registers, here and everwhere else in this patch series. > + /* set up the I/O ports */ > + gpio->GPACON = 0x007FFFFF; > + gpio->GPBCON = 0x00044555; > + gpio->GPBUP = 0x000007FF; > + gpio->GPCCON = 0xAAAAAAAA; > + gpio->GPCUP = 0x0000FFFF; > + gpio->GPDCON = 0xAAAAAAAA; > + gpio->GPDUP = 0x0000FFFF; > + gpio->GPECON = 0xAAAAAAAA; > + gpio->GPEUP = 0x0000FFFF; > + gpio->GPFCON = 0x000055AA; > + gpio->GPFUP = 0x000000FF; > + gpio->GPGCON = 0xFF95FFBA; > + gpio->GPGUP = 0x0000FFFF; > + gpio->GPHCON = 0x002AFAAA; > + gpio->GPHUP = 0x000007FF; Please define some symbolic constants for these data (in your board config file), and explain in comments what these mean. > +int dram_init(void) > +{ > + gd->bd->bi_dram[0].start = PHYS_SDRAM_1; > + gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE; Please use get_ram_size() for auto-sizing and memory testing. > +ulong board_flash_get_legacy(ulong base, int banknum, flash_info_t *info) > +{ > + if (banknum == 0) { /* non-CFI boot flash */ > + info->portwidth = FLASH_CFI_16BIT; > + info->chipwidth = FLASH_CFI_BY16; > + info->interface = FLASH_CFI_X16; > + return 1; > + } else > + return 0; > +} Why would that be needed? > diff --git a/board/gec/gec2410/lowlevel_init.S > b/board/gec/gec2410/lowlevel_init.S > new file mode 100644 ... > +#include <config.h> > +#include <version.h> > + > + > +/* some parameters for the board */ What is this comment supposed to explain? > +/* > + * > + * Taken from linux/arch/arm/boot/compressed/head-s3c2410.S > + * > + * Copyright (C) 2002 Samsung Electronics SW.LEE <hitch...@sec.samsung.com> > + * > + */ > + > +#define BWSCON 0x48000000 > + > +/* BWSCON */ > +#define DW8 (0x0) > +#define DW16 (0x1) > +#define DW32 (0x2) > +#define WAIT (0x1<<2) > +#define UBLB (0x1<<3) > + > +#define B1_BWSCON (DW32) > +#define B2_BWSCON (DW16) > +#define B3_BWSCON (DW16 + WAIT + UBLB) > +#define B4_BWSCON (DW16) > +#define B5_BWSCON (DW16) > +#define B6_BWSCON (DW32) > +#define B7_BWSCON (DW32) > + > +/* BANK0CON */ > +#define B0_Tacs 0x0 /* 0clk */ > +#define B0_Tcos 0x0 /* 0clk */ > +#define B0_Tacc 0x7 /* 14clk */ > +#define B0_Tcoh 0x0 /* 0clk */ > +#define B0_Tah 0x0 /* 0clk */ > +#define B0_Tacp 0x0 > +#define B0_PMC 0x0 /* normal */ > + > +/* BANK1CON */ > +#define B1_Tacs 0x0 /* 0clk */ > +#define B1_Tcos 0x0 /* 0clk */ > +#define B1_Tacc 0x7 /* 14clk */ > +#define B1_Tcoh 0x0 /* 0clk */ > +#define B1_Tah 0x0 /* 0clk */ > +#define B1_Tacp 0x0 > +#define B1_PMC 0x0 > + > +#define B2_Tacs 0x0 > +#define B2_Tcos 0x0 > +#define B2_Tacc 0x7 > +#define B2_Tcoh 0x0 > +#define B2_Tah 0x0 > +#define B2_Tacp 0x0 > +#define B2_PMC 0x0 > + > +#define B3_Tacs 0x0 /* 0clk */ > +#define B3_Tcos 0x3 /* 4clk */ > +#define B3_Tacc 0x7 /* 14clk */ > +#define B3_Tcoh 0x1 /* 1clk */ > +#define B3_Tah 0x0 /* 0clk */ > +#define B3_Tacp 0x3 /* 6clk */ > +#define B3_PMC 0x0 /* normal */ > + > +#define B4_Tacs 0x0 /* 0clk */ > +#define B4_Tcos 0x0 /* 0clk */ > +#define B4_Tacc 0x7 /* 14clk */ > +#define B4_Tcoh 0x0 /* 0clk */ > +#define B4_Tah 0x0 /* 0clk */ > +#define B4_Tacp 0x0 > +#define B4_PMC 0x0 /* normal */ > + > +#define B5_Tacs 0x0 /* 0clk */ > +#define B5_Tcos 0x0 /* 0clk */ > +#define B5_Tacc 0x7 /* 14clk */ > +#define B5_Tcoh 0x0 /* 0clk */ > +#define B5_Tah 0x0 /* 0clk */ > +#define B5_Tacp 0x0 > +#define B5_PMC 0x0 /* normal */ > + > +#define B6_MT 0x3 /* SDRAM */ > +#define B6_Trcd 0x1 > +#define B6_SCAN 0x1 /* 9bit */ > + > +#define B7_MT 0x3 /* SDRAM */ > +#define B7_Trcd 0x1 /* 3clk */ > +#define B7_SCAN 0x1 /* 9bit */ > + > +/* REFRESH parameter */ > +#define REFEN 0x1 /* Refresh enable */ > +#define TREFMD 0x0 /* CBR(CAS before RAS)/Auto > refresh */ > +#define Trp 0x0 /* 2clk */ > +#define Trc 0x3 /* 7clk */ > +#define Tchr 0x2 /* 3clk */ > +#define REFCNT 1113 /* period=15.6us, HCLK=60Mhz, > (2048+1-15.6*60) */ These #defines belong into some header file. > +SMRDATA: > + .word > (0+(B1_BWSCON<<4)+(B2_BWSCON<<8)+(B3_BWSCON<<12)+(B4_BWSCON<<16)+(B5_BWSCON<<20)+(B6_BWSCON<<24)+(B7_BWSCON<<28)) > + .word > ((B0_Tacs<<13)+(B0_Tcos<<11)+(B0_Tacc<<8)+(B0_Tcoh<<6)+(B0_Tah<<4)+(B0_Tacp<<2)+(B0_PMC)) > + .word > ((B1_Tacs<<13)+(B1_Tcos<<11)+(B1_Tacc<<8)+(B1_Tcoh<<6)+(B1_Tah<<4)+(B1_Tacp<<2)+(B1_PMC)) > + .word > ((B2_Tacs<<13)+(B2_Tcos<<11)+(B2_Tacc<<8)+(B2_Tcoh<<6)+(B2_Tah<<4)+(B2_Tacp<<2)+(B2_PMC)) > + .word > ((B3_Tacs<<13)+(B3_Tcos<<11)+(B3_Tacc<<8)+(B3_Tcoh<<6)+(B3_Tah<<4)+(B3_Tacp<<2)+(B3_PMC)) > + .word > ((B4_Tacs<<13)+(B4_Tcos<<11)+(B4_Tacc<<8)+(B4_Tcoh<<6)+(B4_Tah<<4)+(B4_Tacp<<2)+(B4_PMC)) > + .word > ((B5_Tacs<<13)+(B5_Tcos<<11)+(B5_Tacc<<8)+(B5_Tcoh<<6)+(B5_Tah<<4)+(B5_Tacp<<2)+(B5_PMC)) Lines too long. Please fix globally. PATCH 01/10: WARNING: line over 80 characters #336: FILE: board/gec/gec2410/flash.c:43: +#define MEM_FLASH_ADDR1 (*(volatile u16 *)(CONFIG_SYS_FLASH_BASE + (0x00000555 << 1))) WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt #336: FILE: board/gec/gec2410/flash.c:43: +#define MEM_FLASH_ADDR1 (*(volatile u16 *)(CONFIG_SYS_FLASH_BASE + (0x00000555 << 1))) WARNING: line over 80 characters #337: FILE: board/gec/gec2410/flash.c:44: +#define MEM_FLASH_ADDR2 (*(volatile u16 *)(CONFIG_SYS_FLASH_BASE + (0x000002AA << 1))) WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt #337: FILE: board/gec/gec2410/flash.c:44: +#define MEM_FLASH_ADDR2 (*(volatile u16 *)(CONFIG_SYS_FLASH_BASE + (0x000002AA << 1))) WARNING: line over 80 characters #1000: FILE: board/gec/gec2410/lowlevel_init.S:128: +#define REFCNT 1113 /* period=15.6us, HCLK=60Mhz, (2048+1-15.6*60) */ WARNING: line over 80 characters #1031: FILE: board/gec/gec2410/lowlevel_init.S:159: + .word (0+(B1_BWSCON<<4)+(B2_BWSCON<<8)+(B3_BWSCON<<12)+(B4_BWSCON<<16)+(B5_BWSCON<<20)+(B6_BWSCON<<24)+(B7_BWSCON<<28)) WARNING: line over 80 characters #1032: FILE: board/gec/gec2410/lowlevel_init.S:160: + .word ((B0_Tacs<<13)+(B0_Tcos<<11)+(B0_Tacc<<8)+(B0_Tcoh<<6)+(B0_Tah<<4)+(B0_Tacp<<2)+(B0_PMC)) WARNING: line over 80 characters #1033: FILE: board/gec/gec2410/lowlevel_init.S:161: + .word ((B1_Tacs<<13)+(B1_Tcos<<11)+(B1_Tacc<<8)+(B1_Tcoh<<6)+(B1_Tah<<4)+(B1_Tacp<<2)+(B1_PMC)) WARNING: line over 80 characters #1034: FILE: board/gec/gec2410/lowlevel_init.S:162: + .word ((B2_Tacs<<13)+(B2_Tcos<<11)+(B2_Tacc<<8)+(B2_Tcoh<<6)+(B2_Tah<<4)+(B2_Tacp<<2)+(B2_PMC)) WARNING: line over 80 characters #1035: FILE: board/gec/gec2410/lowlevel_init.S:163: + .word ((B3_Tacs<<13)+(B3_Tcos<<11)+(B3_Tacc<<8)+(B3_Tcoh<<6)+(B3_Tah<<4)+(B3_Tacp<<2)+(B3_PMC)) WARNING: line over 80 characters #1036: FILE: board/gec/gec2410/lowlevel_init.S:164: + .word ((B4_Tacs<<13)+(B4_Tcos<<11)+(B4_Tacc<<8)+(B4_Tcoh<<6)+(B4_Tah<<4)+(B4_Tacp<<2)+(B4_PMC)) WARNING: line over 80 characters #1037: FILE: board/gec/gec2410/lowlevel_init.S:165: + .word ((B5_Tacs<<13)+(B5_Tcos<<11)+(B5_Tacc<<8)+(B5_Tcoh<<6)+(B5_Tah<<4)+(B5_Tacp<<2)+(B5_PMC)) total: 0 errors, 12 warnings, 970 lines checked PATCH 01/10 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. PATCH 03/10 WARNING: line over 80 characters #135: FILE: include/configs/gec2410.h:38: +#define CONFIG_GEC2410 1 /* on a GD-Embedded Software Center GEC2410 Board */ WARNING: line over 80 characters #161: FILE: include/configs/gec2410.h:64: +#define CONFIG_SYS_GBL_DATA_SIZE 128 /* size in bytes reserved for initial data */ ERROR: spaces required around that ':' (ctx:VxV) #214: FILE: include/configs/gec2410.h:117: +#define CONFIG_ETHADDR 08:00:3e:26:0a:5b ^ ERROR: spaces required around that ':' (ctx:VxV) #214: FILE: include/configs/gec2410.h:117: +#define CONFIG_ETHADDR 08:00:3e:26:0a:5b ^ ERROR: spaces required around that ':' (ctx:VxV) #214: FILE: include/configs/gec2410.h:117: +#define CONFIG_ETHADDR 08:00:3e:26:0a:5b ^ ERROR: spaces required around that ':' (ctx:VxV) #214: FILE: include/configs/gec2410.h:117: +#define CONFIG_ETHADDR 08:00:3e:26:0a:5b ^ ERROR: spaces required around that ':' (ctx:VxV) #214: FILE: include/configs/gec2410.h:117: +#define CONFIG_ETHADDR 08:00:3e:26:0a:5b ^ WARNING: line over 80 characters #222: FILE: include/configs/gec2410.h:125: +#define CONFIG_KGDB_BAUDRATE 115200 /* speed to run kgdb serial port */ WARNING: line over 80 characters #230: FILE: include/configs/gec2410.h:133: +#define CONFIG_SYS_LONGHELP /* undef to save memory */ WARNING: line over 80 characters #231: FILE: include/configs/gec2410.h:134: +#define CONFIG_SYS_PROMPT "GEC2410#" /* Monitor Command Prompt */ WARNING: line over 80 characters #232: FILE: include/configs/gec2410.h:135: +#define CONFIG_SYS_CBSIZE 256 /* Console I/O Buffer Size */ WARNING: line over 80 characters #233: FILE: include/configs/gec2410.h:136: +#define CONFIG_SYS_PBSIZE 384 /* Print Buffer Size */ WARNING: line over 80 characters #234: FILE: include/configs/gec2410.h:137: +#define CONFIG_SYS_MAXARGS 16 /* max number of command args */ WARNING: line over 80 characters #235: FILE: include/configs/gec2410.h:138: +#define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE /* Boot Argument Buffer Size */ WARNING: line over 80 characters #237: FILE: include/configs/gec2410.h:140: +#define CONFIG_SYS_MEMTEST_START CONFIG_SYS_SDRAM_BASE /* memtest works on */ WARNING: line over 80 characters #238: FILE: include/configs/gec2410.h:141: +#define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_SDRAM_BASE + 0x3e00000) /* 62 MB in DRAM */ WARNING: line over 80 characters #240: FILE: include/configs/gec2410.h:143: +#define CONFIG_SYS_LOAD_ADDR CONFIG_SYS_SDRAM_BASE /* default load address */ WARNING: line over 80 characters #264: FILE: include/configs/gec2410.h:167: +#define PHYS_SDRAM_1 CONFIG_SYS_SDRAM_BASE /* SDRAM Bank #1 */ WARNING: line over 80 characters #275: FILE: include/configs/gec2410.h:178: +#define CONFIG_SYS_MAX_FLASH_BANKS 1 /* max number of memory banks */ WARNING: line over 80 characters #276: FILE: include/configs/gec2410.h:179: +#define CONFIG_SYS_MAX_FLASH_SECT 512 /* max number of sectors on one chip */ WARNING: line over 80 characters #278: FILE: include/configs/gec2410.h:181: +#define CONFIG_SYS_FLASH_CFI 1 /* Use CFI parameters (needed?) */ WARNING: line over 80 characters #285: FILE: include/configs/gec2410.h:188: +#define CONFIG_ENV_ADDR (CONFIG_SYS_FLASH_BASE + 0x0F0000) /* addr of environment */ WARNING: line over 80 characters #288: FILE: include/configs/gec2410.h:191: +#define CONFIG_SYS_FLASH_ERASE_TOUT (5 * CONFIG_SYS_HZ) /* Timeout for Flash Erase */ WARNING: line over 80 characters #289: FILE: include/configs/gec2410.h:192: +#define CONFIG_SYS_FLASH_WRITE_TOUT (5 * CONFIG_SYS_HZ) /* Timeout for Flash Write */ WARNING: line over 80 characters #318: FILE: include/configs/gec2410.h:221: +#define CONFIG_SYS_UBOOT_BASE (CONFIG_SYS_MAPPED_RAM_BASE + 0x03e00000) WARNING: line over 80 characters #320: FILE: include/configs/gec2410.h:223: +#define CONFIG_ENV_OFFSET 0x0040000 /* Offset of Environment Sector */ WARNING: line over 80 characters #324: FILE: include/configs/gec2410.h:227: +#define CONFIG_SYS_NAND_BASE 0x4e00000c /* NFDATA 0x4e00000c R/W NAND Flash data register */ WARNING: line over 80 characters #329: FILE: include/configs/gec2410.h:232: +#define CONFIG_SYS_NAND_U_BOOT_DST CONFIG_SYS_PHY_UBOOT_BASE /* NUB load-addr */ WARNING: line over 80 characters #330: FILE: include/configs/gec2410.h:233: +#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_NAND_U_BOOT_DST /* NUB start-addr */ WARNING: line over 80 characters #332: FILE: include/configs/gec2410.h:235: +#define CONFIG_SYS_NAND_U_BOOT_OFFS (4 * 1024) /* Offset to RAM U-Boot image */ WARNING: line over 80 characters #333: FILE: include/configs/gec2410.h:236: +#define CONFIG_SYS_NAND_U_BOOT_SIZE (252 * 1024) /* Size of RAM U-Boot image */ WARNING: line over 80 characters #344: FILE: include/configs/gec2410.h:247: +#define CONFIG_SYS_NAND_4_ADDR_CYCLE 1 /* Fourth addr used (>32MB) */ WARNING: line over 80 characters #352: FILE: include/configs/gec2410.h:255: +#define CONFIG_SYS_NAND_ECCSTEPS (CONFIG_SYS_NAND_PAGE_SIZE / CONFIG_SYS_NAND_ECCSIZE) WARNING: line over 80 characters #356: FILE: include/configs/gec2410.h:259: +#define CONFIG_SYS_NAND_ECCTOTAL (CONFIG_SYS_NAND_ECCBYTES * CONFIG_SYS_NAND_ECCSTEPS) total: 5 errors, 29 warnings, 275 lines checked PATCH 03/10 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Time is fluid ... like a river with currents, eddies, backwash. -- Spock, "The City on the Edge of Forever", stardate 3134.0 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot