public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Tom <Tom.Rix@windriver.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH-ARM 1/3] Add support for the s3c2440 cpu excluding nand driver
Date: Sun, 01 Nov 2009 16:31:19 -0600	[thread overview]
Message-ID: <4AEE0C37.1080806@windriver.com> (raw)
In-Reply-To: <4AEDDA50.4010807@fearnside-systems.co.uk>

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.


>  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.

>  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.

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

  reply	other threads:[~2009-11-01 22:31 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 [this message]
2009-11-02  4:55   ` Minkyu Kang
2009-11-02 12:56   ` kevin.morfitt at fearnside-systems.co.uk

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=4AEE0C37.1080806@windriver.com \
    --to=tom.rix@windriver.com \
    --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