[U-Boot] [PATCH-ARM] Add support for Embest SBC2440-II Board

Jean-Christophe PLAGNIOL-VILLARD plagnioj at jcrosoft.com
Wed Jun 3 01:35:29 CEST 2009


On 03:24 Sat 23 May     , kevin.morfitt at fearnside-systems.co.uk wrote:
> 
> Implementation based on the existing u-boot support for S3C2410-based boards. u-boot programmed into NOR flash.
> 
> Tested on an SBC2440-II Board using tftp to copy the files from a server and programming them into NAND flash.
> 
> MAKEALL used to build all LIST_ARM9 targets only - no other architectures built as the changes only affect ARM9-based boards.
> 
> Signed-off-by: Kevin Morfitt <kevin.morfitt at fearnside-systems.co.uk>
please limit your comments to 80 chars per line
could you split this patch in multiple subset as adding the 2440 support,
addind the nand drivers and then adding the board.

This will really simplify the review
> ---
>  MAKEALL                                |    1 +
please add MAINTAINERS entry
>  Makefile                               |    3 +
>  board/embest/sbc2440ii/Makefile        |   55 +++++++
>  board/embest/sbc2440ii/config.mk       |   25 +++
>  board/embest/sbc2440ii/lowlevel_init.S |  219 +++++++++++++++++++++++++++
>  board/embest/sbc2440ii/sbc2440ii.c     |  127 ++++++++++++++++
>  board/embest/sbc2440ii/u-boot.lds      |   56 +++++++
>  common/serial.c                        |    4 +-
>  cpu/arm920t/s3c24x0/speed.c            |   83 ++++++++---
>  cpu/arm920t/s3c24x0/timer.c            |    6 +-
>  cpu/arm920t/s3c24x0/usb.c              |    6 +-
>  cpu/arm920t/s3c24x0/usb_ohci.c         |    2 +
>  cpu/arm920t/start.S                    |   35 ++++-
>  drivers/i2c/s3c24x0_i2c.c              |   18 +++
>  drivers/mtd/nand/Makefile              |    2 +-
>  drivers/mtd/nand/s3c2410_nand.c        |  223 +++++++++++++++++++++++-----
>  drivers/rtc/s3c24x0_rtc.c              |    2 +
>  drivers/serial/serial_s3c24x0.c        |    2 +
>  include/common.h                       |    3 +-
>  include/configs/sbc2440ii.h            |  252 ++++++++++++++++++++++++++++++++
>  include/s3c2440.h                      |  231 +++++++++++++++++++++++++++++
>  include/s3c24x0.h                      |   83 +++++++++++
>  22 files changed, 1362 insertions(+), 76 deletions(-)
>  create mode 100644 board/embest/sbc2440ii/Makefile
>  create mode 100644 board/embest/sbc2440ii/config.mk
>  create mode 100644 board/embest/sbc2440ii/lowlevel_init.S
>  create mode 100644 board/embest/sbc2440ii/sbc2440ii.c
>  create mode 100644 board/embest/sbc2440ii/u-boot.lds
>  create mode 100644 include/configs/sbc2440ii.h
>  create mode 100644 include/s3c2440.h
> 
<snip>
> diff --git a/MAKEALL b/MAKEALL
> index c98d03a..e7235e4 100755
> + */
> +
> +#include <config.h>
> +#include <version.h>
> +
> +/*
> + * Taken from linux/arch/arm/boot/compressed/head-s3c2410.S
> + *
> + * Copyright (C) 2002 Samsung Electronics SW.LEE  <hitchcar at sec.samsung.com>
> + */
> +
> +#define BWSCON		0x48000000
> +
> +#define DW8		(0x0)
> +#define DW16		(0x1)
> +#define DW32		(0x2)
> +#define WAIT		(0x1 << 2)
> +#define UBLB		(0x1 << 3)
what is all theses macro? for what use?
> +
> +#define B1_BWSCON	(DW16)
> +#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)

> diff --git a/board/embest/sbc2440ii/sbc2440ii.c b/board/embest/sbc2440ii/sbc2440ii.c
> new file mode 100644
> index 0000000..e0599dd
> --- /dev/null
> +++ b/board/embest/sbc2440ii/sbc2440ii.c
> @@ -0,0 +1,127 @@
> +/*
> + * (C) Copyright 2002
> + * Sysgo Real-Time Solutions, GmbH <www.elinos.com>
> + * Marius Groeger <mgroeger at sysgo.de>
> + *
> + * (C) Copyright 2002
> + * David Mueller, ELSOFT AG, <d.mueller at elsoft.ch>
> + *
> + * (C) Copyright 2005
> + * JinHua Luo, GuangDong Linux Center, <luo.jinhua at gd-linux.com>
> + *
> + * Modified for the Embest SBC2440-II by
> + * (C) Copyright 2009
> + * Kevin Morfitt, Fearnside Systems Ltd, <kevin.morfitt at fearnside-systems.co.uk>
> + *
> + * 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 <s3c2440.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/* Configure the PLLs for MPLL = 400MHz, UPLL = 48MHz
> +	The clock frequency ratios are set to 1:4:8 ie:
> +		PCLK = 50MHz
> +		HCLK = 100MHz
> +		FCLK = 400MHz
> + */
> +/* The MPLL values. */
> +#define M_MDIV	0x5C
> +#define M_PDIV	1
> +#define M_SDIV	1
> +
> +/* The UPLL values. */
> +#define U_MDIV	0x38
> +#define U_PDIV	2
> +#define U_SDIV	2
this need to be in the config.h as CONFIS_SYS_xxxxx
> +
> +/*
> + * Miscellaneous platform dependent initialisations
> + */
> +
> +static inline void pll_settle_delay(unsigned long loops)
> +{
> +	__asm__ volatile ("1:\n"
> +			  "subs %0, %1, #1\n"
> +			  "bne 1b" : "=r" (loops) : "0" (loops));
> +}
> +
> +int board_init(void)
> +{
> +	S3C24X0_CLOCK_POWER * const clk_power = S3C24X0_GetBase_CLOCK_POWER();
> +	S3C24X0_GPIO * const gpio = S3C24X0_GetBase_GPIO();
> +
> +	/* to reduce PLL lock time, adjust the LOCKTIME register */
> +	clk_power->LOCKTIME = 0xFFFFFFFF;
> +
> +	/* configure UPLL */
> +	clk_power->UPLLCON = ((U_MDIV << 12) + (U_PDIV << 4) + U_SDIV);
> +
> +	/* some delay between UPLL and MPLL */
> +	pll_settle_delay(8000);
> +
> +	/* configure MPLL */
> +	clk_power->MPLLCON = ((M_MDIV << 12) + (M_PDIV << 4) + M_SDIV);
> +
> +	/* configure the GPIO */
> +	gpio->GPACON = 0x007FFFFF;
what means all these hardcoded values?
> +	gpio->GPBCON = 0x00055555;
> +	gpio->GPBUP  = 0x000007FF;
> +	gpio->GPBDAT = 0x000001C0;		/* Switch on LED1. */
> +	gpio->GPCCON = 0xAAAAAAAA;
> +	gpio->GPCUP  = 0x0000FFFF;
> +	gpio->GPDCON = 0xAAAAAAAA;
<snip>
 +
> +	/* arch number of SBC2440-II Board */
> +   gd->bd->bi_arch_number = MACH_TYPE_SBC2440II;
please use tab for indent
> +
> +	/* adress of boot parameters */
> +	gd->bd->bi_boot_params = 0x30000100;
please use this style RAM_BASE + 0x100
> +
> +	icache_enable();
> +	dcache_enable();
do you support mmu? if not enable the dcache will have no effect
> +
> +	return 0;
> +}
> +
> +int dram_init(void)
> +{
> +	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
> +	gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
> +
> +	return 0;
> +}
> diff --git a/board/embest/sbc2440ii/u-boot.lds b/board/embest/sbc2440ii/u-boot.lds
please remove no more need
> new file mode 100644
> index 0000000..3b79776
<snip>
> --- a/cpu/arm920t/s3c24x0/speed.c
> +++ b/cpu/arm920t/s3c24x0/speed.c
> @@ -30,12 +30,15 @@
>   */
>  
>  #include <common.h>
> -#if defined(CONFIG_S3C2400) || defined (CONFIG_S3C2410) || defined (CONFIG_TRAB)
> +#if defined(CONFIG_S3C2400) || defined(CONFIG_S3C2410) || \
> +    defined(CONFIG_S3C2440) || defined(CONFIG_TRAB)
>  
>  #if defined(CONFIG_S3C2400)
>  #include <s3c2400.h>
>  #elif defined(CONFIG_S3C2410)
>  #include <s3c2410.h>
> +#elif defined(CONFIG_S3C2440)
> +#include <s3c2440.h>
>  #endif
>  
>  #define MPLL 0
> @@ -53,49 +56,81 @@
>  
>  static ulong get_PLLCLK(int pllreg)
>  {
> -    S3C24X0_CLOCK_POWER * const clk_power = S3C24X0_GetBase_CLOCK_POWER();
> -    ulong r, m, p, s;
> -
> -    if (pllreg == MPLL)
> -	r = clk_power->MPLLCON;
> -    else if (pllreg == UPLL)
> -	r = clk_power->UPLLCON;
> -    else
> -	hang();
> -
> -    m = ((r & 0xFF000) >> 12) + 8;
> -    p = ((r & 0x003F0) >> 4) + 2;
> -    s = r & 0x3;
> -
> -    return((CONFIG_SYS_CLK_FREQ * m) / (p << s));
> +	S3C24X0_CLOCK_POWER * const clk_power = S3C24X0_GetBase_CLOCK_POWER();
please do not mix codind style fix and new code in the same patch
> +	ulong r, m, p, s;
> +
> +	if (pllreg == MPLL)
> +		r = clk_power->MPLLCON;
> +	else if (pllreg == UPLL)
> +		r = clk_power->UPLLCON;
> +	else
> +		hang();
> +
> +	m = ((r & 0xFF000) >> 12) + 8;
> +	p = ((r & 0x003F0) >> 4) + 2;
> +	s = r & 0x3;
> +
> +#ifdef CONFIG_S3C2440
> +	if (pllreg == MPLL)
> +		return (2 * CONFIG_SYS_CLK_FREQ * m) / (p << s);
> +	else
please add the #else here
> +		return (CONFIG_SYS_CLK_FREQ * m) / (p << s);
> +#else
> +	return (CONFIG_SYS_CLK_FREQ * m) / (p << s);
> +#endif
>  }
>  
>  /* return FCLK frequency */
>  ulong get_FCLK(void)
>  {
> -    return(get_PLLCLK(MPLL));
> +	return get_PLLCLK(MPLL);
>  }
>  
>  /* return HCLK frequency */

please send a new version before continuing the review
as you mix codind style fix and new code in the same patch
it's really hard to review

Best Regards,
J.


More information about the U-Boot mailing list