[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

[Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>]

On 00:56 Sun 21 Jun     , kevin.morfitt at fearnside-systems.co.uk wrote:
> Hi Jean-Christophe, comments below:
> 
> Also, see note at the end regarding re-structuring of the existing
> s3c24x0 and
> Embest SBC2440-II Board patches.
> 
> On 20/06/2009 18:36, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 17:42 Fri 19 Jun     , kevin.morfitt at fearnside-systems.co.uk wrote:
> >   
> >> This is the first of two patches that will add support for the Embest 
> >> SBC2440-II Board. This one adds generic support for the S3C2440 CPU. Tested by 
> >> running MAKEALL for ARM9 boards - no new warnings or errors were found.
> >>
> >> This patch set assumes that the following patches have already been applied:
> >>
> >> - Clean-up of ARM920T S3C24x0 code, submitted on 5th June
> >> - Clean-up of ARM920T S3C24x0 drivers code, submitted on 5th June
> >> - Bug-fix in drivers mtd nand Makefile, submitted on 18th June
> >>
> >> Signed-off-by: Kevin Morfitt <kevin.morfitt@fearnside-systems.co.uk>
> >> ---
> >>  common/serial.c                 |    4 +-
> >>  cpu/arm920t/s3c24x0/speed.c     |   38 ++++++-
> >>  cpu/arm920t/s3c24x0/timer.c     |    8 +-
> >>  cpu/arm920t/s3c24x0/usb.c       |    9 +-
> >>  cpu/arm920t/start.S             |   29 ++++-
> >>  drivers/i2c/s3c24x0_i2c.c       |   12 +-
> >>  drivers/mtd/nand/Makefile       |    1 +
> >>  drivers/mtd/nand/s3c2410_nand.c |    8 +-
> >>  drivers/mtd/nand/s3c2440_nand.c |  241 +++++++++++++++++++++++++++++++++++++++
> >>  drivers/rtc/s3c24x0_rtc.c       |    2 +
> >>  drivers/serial/serial_s3c24x0.c |    2 +
> >>  include/common.h                |    3 +-
> >>  include/s3c2440.h               |  232 +++++++++++++++++++++++++++++++++++++
> >>  include/s3c24x0.h               |  186 +++++++++++++++++++++++++++++-
> >>  14 files changed, 745 insertions(+), 30 deletions(-)
> >>  create mode 100644 drivers/mtd/nand/s3c2440_nand.c
> >>  create mode 100644 include/s3c2440.h
> >>     
> > first general comment
> >
> > Please split this patch as something like this
> > patch 1 add the s3c24x0 support
> > patch 2 add the nand
> > patch 3 your boards
> >   
> s3c24x0 refers to the Samsung s3c type of cpu's - s3c2400, s3c2410 and
> s3c2440.
> Support for the s3c2400 and s3c2410 cpu's already exists in the current
> u-boot
> code base in cpu/arm920t/s3c24x0 but there is currently no support for the
> s3c2440 cpu.
> 
> However, the format of the current s3c24x0 code doesn't meet the u-boot
> coding
> style so I've already submitted 2 patches to clean up the current
> s3c24x0 code
> - see reference at the top of this patch to the patches "Clean-up of
> ARM920T
> S3C24x0 code, submitted on 5th June" and "Clean-up of ARM920T S3C24x0
> drivers
> code, submitted on 5th June".
> 
> This patch, 1/2, just adds support for the s3c2440 cpu (including the new
> s3c2440 NAND driver) to the current s3c24x0 code - it doesn't actually
> add the
> Embest SBC2440-II Board support. Patch 2/2 is the one that adds the
> support for
> the Embest SBC2440-II Board itself.
> 
> So, the patches are already structured as you requested.
no as you add the nand in this patch
the nand need to be add in a seperate patch,
this one need to only add the s3c2440 support
and the nand will be handle by Scott the nand Maintainer
> 
> It's getting a bit complicated because I initially sent a patch to add
> all of
> the Embest SBC2440-II Board changes in one go. Wolfgang and yourself
> asked me
> to create patches to clean up the existing code base first and to add the
> s3c2440 support separately to the Embest SBC2440-II Board so I now have 4
> patches all dependent on one another and it's not too clear how they fit
> together.

> >> diff --git a/common/serial.c b/common/serial.c
> >> index dd80e7c..6548b8b 100644
> >> --- a/common/serial.c
> >> +++ b/common/serial.c
> >> @@ -58,7 +58,7 @@ struct serial_device *__default_serial_console (void)
> >>  #else
> >>  		return &serial0_device;
> >>  #endif
> >> -#elif defined(CONFIG_S3C2410)
> >> +#elif defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
> >>     
> > if it's really s3c24x0 please use a correspondig macro IIRC yes
> >   
> Could you explain a bit more about how the macro would work please?
CONFIG_SERIAL_S3C24X0 or CONFIG_S3C24X0 as example
> >>  #if defined(CONFIG_SERIAL1)
> >>  	return &s3c24xx_serial0_device;
> >>  #elif defined(CONFIG_SERIAL2)
> >> @@ -133,7 +133,7 @@ void serial_initialize (void)
> >>  #if defined (CONFIG_STUART)
> >>  	serial_register(&serial_stuart_device);
> >>  #endif
> >> -#if defined(CONFIG_S3C2410)
> >> +#if defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
> >>     
> > ditto
> >   
> >>  	serial_register(&s3c24xx_serial0_device);
> >>  	serial_register(&s3c24xx_serial1_device);
> >>  	serial_register(&s3c24xx_serial2_device);
> >> diff --git a/cpu/arm920t/s3c24x0/speed.c b/cpu/arm920t/s3c24x0/speed.c
> >> index 3d7c8cf..b8b183e 100644
> >> --- a/cpu/arm920t/s3c24x0/speed.c
> >> +++ b/cpu/arm920t/s3c24x0/speed.c
> >> @@ -30,7 +30,8 @@
> >>   */
> >>  
> >>  #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)
> >>  
> >>  #include <asm/io.h>
> >>  
> >> @@ -38,6 +39,8 @@
> >>  #include <s3c2400.h>
> >>  #elif defined(CONFIG_S3C2410)
> >>  #include <s3c2410.h>
> >> +#elif defined(CONFIG_S3C2440)
> >> +#include <s3c2440.h>
> >>  #endif
> >>     
> > please create a cpu.h file to avoid those ifdef
> >   
> OK. There is already an s3c24x0.h header file so I could modify it to
> include
> the correct s3c2400.h, s3c2410.h or s3c2440.h header file.
> >>  
> >>  #define MPLL 0
> >> @@ -69,6 +72,11 @@ static ulong get_PLLCLK(int pllreg)
> >>  	p = ((r & 0x003F0) >> 4) + 2;
> >>  	s = r & 0x3;
> >>  
> >> +#ifdef CONFIG_S3C2440
> >> +	if (pllreg == MPLL)
> >> +		return (2 * CONFIG_SYS_CLK_FREQ * m) / (p << s);
> >> +	else
> >> +#endif
> >>  	return (CONFIG_SYS_CLK_FREQ * m) / (p << s);
> >>  }
> >>  
> >> @@ -83,7 +91,23 @@ ulong get_HCLK(void)
> >>  {
> >>  	S3C24X0_CLOCK_POWER * const clk_power = S3C24X0_GetBase_CLOCK_POWER();
> >>  
> >> +#ifdef CONFIG_S3C2440
> >> +	switch (clk_power->CLKDIVN & 0x6) {
> >> +	default:
> >> +	case 0:
> >> +		return get_FCLK();
> >> +	case 2:
> >> +		return get_FCLK() / 2;
> >> +	case 4:
> >> +		return (readl(&clk_power->CAMDIVN) & (1 << 9)) ?
> >> +			get_FCLK() / 8 : get_FCLK() / 4;
> >> +	case 6:
> >> +		return (readl(&clk_power->CAMDIVN) & (1 << 8)) ?
> >> +			get_FCLK() / 6 : get_FCLK() / 3;
> >> +	}
> >> +#else
> >>  	return (readl(&clk_power->CLKDIVN) & 2) ? get_FCLK() / 2 : get_FCLK();
> >> +#endif
> >>  }
> >>  
> >>     
> >
> >
> >   
> >> diff --git a/cpu/arm920t/start.S b/cpu/arm920t/start.S
> >> index 810d402..5f7aa33 100644
> >> --- a/cpu/arm920t/start.S
> >> +++ b/cpu/arm920t/start.S
> >> @@ -132,8 +132,9 @@ copyex:
> >>  	bne	copyex
> >>  #endif
> >>  
> >> -#if defined(CONFIG_S3C2400) || defined(CONFIG_S3C2410)
> >> -	/* turn off the watchdog */
> >> +#if defined(CONFIG_S3C2400) || \
> >> +    defined(CONFIG_S3C2410) || \
> >> +    defined(CONFIG_S3C2440)
> >>  
> >>     
> > please move this code to cpu/arm920t/s3c24x0/
> >
> > the start.S need to generic but you can call arch code from it
> > a branch or if not possible a assembly macro
> >
> > please call it arch_pre_lowlevel_init
> > tks
> >   
> OK. I can do something similar to the existing arch-specific
> 'lowlevel_init'
> function call.
please do this in 2 steps
first you move the code patch 1
and the you add the s3c2440 (this patch)
> >   
> >>  # if defined(CONFIG_S3C2400)
> >>  #  define pWTCON	0x15300000
> >> @@ -146,6 +147,15 @@ copyex:
> >>  #  define CLKDIVN	0x4C000014	/* clock divisor register */
> >>  # endif
> >>  
> >> +# if defined(CONFIG_S3C2440)
> >> +#  define INTSMASK  0xffff
> >> +#  define CLKDIVVAL 0x5
> >> +#else
> >> +#  define INTSMASK  0x3ff
> >> +#  define CLKDIVVAL 0x3
> >> +# endif
> >> +
> >> +	/* turn off the watchdog */
> >>  	ldr	r0, =pWTCON
> >>  	mov	r1, #0x0
> >>  	str	r1, [r0]
> >>     
> >
> >   
> >> @@ -156,8 +166,8 @@ copyex:
> >>     
> > <snip>
> >   
> >> +
> >> +/* AC97 INTERFACE (see S3C2440 manual chapter 24) */
> >> +typedef struct {
> >> +	S3C24X0_REG32 ACGLBCTRL;
> >> +	S3C24X0_REG32 ACGLBSTAT;
> >> +	S3C24X0_REG32 AC_CODEC_CMD;
> >> +	S3C24X0_REG32 AC_CODEC_STAT;
> >> +	S3C24X0_REG32 AC_PCMADDR;
> >> +	S3C24X0_REG32 AC_MICADDR;
> >> +	S3C24X0_REG32 AC_PCMDATA;
> >> +	S3C24X0_REG32 AC_MICDATA;
> >> +} /*__attribute__((__packed__))*/ S3C2440_AC97;
> >>     
> > if no need please remove
> >   
> OK. The ADC isn't used either so I'll remove that as well.
ok but I've in mind the dead code /* */
> 
> Wolfgang has also asked me to make the CONFIG_SYS_HZ change in the current
> s3c2400 and s3c2410 boards before I add support for the s3c2440. I've
> already
> submitted 5 patches as part of the s3c24x0 clean up and Embest
> SBC2440-II Board
> so this would make a 6th and it's getting difficult to track them all.
no just thread all of them
> 
> If you agree, I'd like to withdraw the following patches for now as they
> all
> need changes:
> 
> - [U-Boot] [PATCH-ARM] Clean-up of ARM920T S3C24x0 code, submitted on
> 5th June
> - [U-Boot] [PATCH-ARM] Clean-up of ARM920T S3C24x0 drivers code,
> submitted on
>   5th June
patch 3
	move the start.S code
> - [U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board
> (ie this
>   patch)
patch 5
	add the nand
> - [U-Boot] [PATCH-ARM 2/2] Add support for the Embest SBC2440-II Board,
>   submitted on 19th June
> 
> I'll create the new CONFIG_SYS_HZ patch and submit it, then make the
> required
> changes to the 4 patches above, then re-submit these 4 together, probably
> marked 1/4, 2/4, 3/4 and 4/4, all version 2.
> 
> However, the following patch doesn't need any changes so I'd like to
> leave it
> out for comment:
> 
> - [U-Boot] [PATCH-ARM] Bug-fix in drivers mtd nand Makefile, submitted
> on 18th
	This will be handle by Scott with the nand patch

Best Regards,
J.