[U-Boot] [PATCH-ARM 1/3] Add support for the s3c2440 cpu excluding nand driver

[kevin.morfitt at fearnside-systems.co.uk]

Tom wrote:
> kevin.morfitt at fearnside-systems.co.uk wrote:
>> This patch adds support for the s3c2440 cpu, excluding the nand driver.
>>
>> Tested on an Embest SBC2440-II Board with local u-boot patches as I don't have
>> any s3c2400 or s3c2410 boards but need this patch applying before I can submit
>> patches for the SBC2440-II Board. Also, ran MAKEALL for all ARM9 targets and no
>> new warnings or errors were found.
>>
>> Note that checkpatch.pl shows one error:
> 
> Thank you for using checkpatch.
> 
>> ERROR: Invalid UTF-8, patch and commit message should be encoded in UTF-8
>> #656: FILE: include/s3c2440.h:3:
>> + * David M�ller ELSOFT AG Switzerland. d.mueller at elsoft.ch
>>            ^
>> As David's name correctly contains a non-UTF-8 character I've ignored this error.
>>
>> Signed-off-by: Kevin Morfitt <kevin.morfitt at fearnside-systems.co.uk>
>> ---
>>  common/serial.c                              |    4 +-
>>  cpu/arm920t/s3c24x0/Makefile                 |    6 +-
>>  cpu/arm920t/s3c24x0/arch_pre_lowlevel_init.S |   81 +++++++++++++
> 
> Why not just lowlevel_init.S ?
> It looks like this is a common lowlevel_init but this looks like
> a mistake since the other s3c34x0 boards have not used it up to to this
> point.  Since it looks like this option is being enabled in the
> other boards, this change must be broken out at its own patch.
> 

There already is a lowlevel_init.S that's used in the s3c24x0 board 
directories to configure the SDRAM controller specific to the board.
arch_pre_lowlevel_init.S is intended to run first at start-up to let
a board configure the PLL's in a board specific way so I should really
have put arch_pre_lowlevel_init.S in the board directories and enabled
or disabled it in the board config files.
As you suggested, I'll break it out into its own patch.

> 
>>  cpu/arm920t/s3c24x0/speed.c                  |   41 +++++--
>>  cpu/arm920t/s3c24x0/timer.c                  |   19 +---
>>  cpu/arm920t/s3c24x0/usb.c                    |   17 +--
>>  cpu/arm920t/s3c24x0/usb_ohci.c               |   11 +--
>>  cpu/arm920t/start.S                          |   39 +------
>>  drivers/i2c/s3c24x0_i2c.c                    |   18 ++--
>>  drivers/mtd/nand/s3c2410_nand.c              |    2 +-
>>  drivers/rtc/s3c24x0_rtc.c                    |    7 +-
>>  drivers/serial/serial_s3c24x0.c              |    6 +-
>>  drivers/usb/host/ohci-hcd.c                  |    3 +-
>>  include/common.h                             |    5 +-
> 
>>  include/configs/VCMA9.h                      |    4 +-
>>  include/configs/sbc2410x.h                   |    4 +-
>>  include/configs/smdk2400.h                   |    4 +-
>>  include/configs/smdk2410.h                   |    4 +-
>>  include/configs/trab.h                       |    4 +-
> 
> This is typical of what you are doing with the config files.
>> +#define	CONFIG_S3C24X0		1	/* in a SAMSUNG S3C24x0-type SoC     */
>> +#define	CONFIG_S3C2410		1	/* specifically a SAMSUNG S3C2410 SoC */
> It is good that you are trying to generalize the boards, but
> this separate change and must be split.  This new patch should come first.

OK - I'll do that in a separate patch.

> 
>>  include/s3c2410.h                            |   25 ++++
>>  include/s3c2440.h                            |  163 ++++++++++++++++++++++++++
>>  include/s3c24x0.h                            |   94 ++++++++++++++-
>>  include/s3c24x0_cpu.h                        |   29 +++++
> 
> These 4 files belong in include/asm-arch/arch-s3c24x0 or
> where Minkyu thinks is appropriate.

I'll move these in a separate patch.

> 
> On your include file s3c2440.h
> 
> For your #defines, the whitespace between the identifier and the value
> must be tabs.  You have spaces.
> 
> The static inline functions need space beween one function definition
> and the next.  They also need to use tabs
> 
>>  23 files changed, 471 insertions(+), 119 deletions(-)
>>  create mode 100644 cpu/arm920t/s3c24x0/arch_pre_lowlevel_init.S
>>  create mode 100644 include/s3c2440.h
>>  create mode 100644 include/s3c24x0_cpu.h
>>
> 
> Tom
>