[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board
Jean-Christophe PLAGNIOL-VILLARD
plagnioj at jcrosoft.com
Sun Jun 21 11:46:35 CEST 2009
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 at 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.
More information about the U-Boot
mailing list