From: kevin.morfitt at fearnside-systems.co.uk <kevin.morfitt@fearnside-systems.co.uk>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH-ARM 1/3] Add support for the s3c2440 cpu excluding nand driver
Date: Mon, 02 Nov 2009 12:56:22 +0000 [thread overview]
Message-ID: <4AEED6F6.2060705@fearnside-systems.co.uk> (raw)
In-Reply-To: <4AEE0C37.1080806@windriver.com>
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@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
>
prev parent reply other threads:[~2009-11-02 12:56 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-11-01 18:58 [U-Boot] [PATCH-ARM 1/3] Add support for the s3c2440 cpu excluding nand driver kevin.morfitt at fearnside-systems.co.uk
2009-11-01 22:31 ` Tom
2009-11-02 4:55 ` Minkyu Kang
2009-11-02 12:56 ` kevin.morfitt at fearnside-systems.co.uk [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4AEED6F6.2060705@fearnside-systems.co.uk \
--to=kevin.morfitt@fearnside-systems.co.uk \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox