Tom wrote: > kevin.morf...@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.muel...@elsoft.ch >> ^ >> As David's name correctly contains a non-UTF-8 character I've ignored this >> error. >> >> Signed-off-by: Kevin Morfitt <kevin.morf...@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 > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
