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
