[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

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

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

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 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)
 	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
 
 #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
 }
 
 /* return PCLK frequency */
@@ -97,9 +121,17 @@ ulong get_PCLK(void)
 /* return UCLK frequency */
 ulong get_UCLK(void)
 {
+#ifdef CONFIG_S3C2440
+	S3C24X0_CLOCK_POWER * const clk_power = S3C24X0_GetBase_CLOCK_POWER();
+
+	return (readl(&clk_power->CLKDIVN) & (1 << 3)) ?
+		get_PLLCLK(UPLL) / 2 : get_PLLCLK(UPLL);
+#else
 	return get_PLLCLK(UPLL);
+#endif
 }
 
 #endif /* defined(CONFIG_S3C2400) ||
-			  defined (CONFIG_S3C2410) ||
-			  defined (CONFIG_TRAB) */
+	  defined (CONFIG_S3C2410) ||
+	  defined (CONFIG_S3C2440) ||
+	  defined (CONFIG_TRAB) */
diff --git a/cpu/arm920t/s3c24x0/timer.c b/cpu/arm920t/s3c24x0/timer.c
index a5a784c..0f6b580 100644
--- a/cpu/arm920t/s3c24x0/timer.c
+++ b/cpu/arm920t/s3c24x0/timer.c
@@ -32,6 +32,7 @@
 #include <common.h>
 #if defined(CONFIG_S3C2400)  || \
     defined(CONFIG_S3C2410)  || \
+    defined(CONFIG_S3C2440)  || \
     defined(CONFIG_TRAB)
 
 #include <asm/io.h>
@@ -40,6 +41,8 @@
 #include <s3c2400.h>
 #elif defined(CONFIG_S3C2410)
 #include <s3c2410.h>
+#elif defined(CONFIG_S3C2440)
+#include <s3c2440.h>
 #endif
 
 int timer_load_val = 0;
@@ -230,5 +233,6 @@ void s3c2410_irq(void)
 #endif /* USE_IRQ */
 
 #endif /* defined(CONFIG_S3C2400)  ||
-			  defined (CONFIG_S3C2410) ||
-			  defined (CONFIG_TRAB) */
+	  defined (CONFIG_S3C2410) ||
+	  defined(CONFIG_S3C2440)  || \
+	  defined (CONFIG_TRAB) */
diff --git a/cpu/arm920t/s3c24x0/usb.c b/cpu/arm920t/s3c24x0/usb.c
index c2d2f70..b05520d 100644
--- a/cpu/arm920t/s3c24x0/usb.c
+++ b/cpu/arm920t/s3c24x0/usb.c
@@ -24,12 +24,16 @@
 #include <common.h>
 
 #if defined(CONFIG_USB_OHCI_NEW) && defined(CONFIG_SYS_USB_OHCI_CPU_INIT)
-# if defined(CONFIG_S3C2400) || defined(CONFIG_S3C2410)
+# if defined(CONFIG_S3C2400) || \
+     defined(CONFIG_S3C2410) || \
+     defined(CONFIG_S3C2440)
 
 #if defined(CONFIG_S3C2400)
 # include <s3c2400.h>
 #elif defined(CONFIG_S3C2410)
 # include <s3c2410.h>
+#elif defined(CONFIG_S3C2440)
+# include <s3c2440.h>
 #endif
 
 #include <asm/io.h>
@@ -70,5 +74,6 @@ int usb_cpu_init_fail(void)
 	return 0;
 }
 
-# endif	/* defined(CONFIG_S3C2400) || defined(CONFIG_S3C2410) */
+# endif	/* defined(CONFIG_S3C2400) || defined(CONFIG_S3C2410) ||
+	   defined(CONFIG_S3C2440))*/
 #endif /* defined(CONFIG_USB_OHCI_NEW) && defined(CONFIG_SYS_USB_OHCI_CPU_INIT) */
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)
 
 # 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:
 	mov	r1, #0xffffffff
 	ldr	r0, =INTMSK
 	str	r1, [r0]
-# if defined(CONFIG_S3C2410)
-	ldr	r1, =0x3ff
+# if defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
+	ldr	r1, =INTSMASK
 	ldr	r0, =INTSUBMSK
 	str	r1, [r0]
 # endif
@@ -165,9 +175,16 @@ copyex:
 	/* FCLK:HCLK:PCLK = 1:2:4 */
 	/* default FCLK is 120 MHz ! */
 	ldr	r0, =CLKDIVN
-	mov	r1, #3
+	mov	r1, #CLKDIVVAL
 	str	r1, [r0]
-#endif	/* CONFIG_S3C2400 || CONFIG_S3C2410 */
+
+#ifdef CONFIG_S3C2440
+	/* Set asynchronous bus mode */
+	mrc	p15, 0, r1, c1, c0, 0   /* read ctrl register */
+	orr	r1, r1, #0xc0000000     /* Asynchronous */
+	mcr	p15, 0, r1, c1, c0, 0   /* write ctrl register */
+#endif  /* CONFIG_S3C2440 */
+#endif  /* CONFIG_S3C2400 || CONFIG_S3C2410 || CONFIG_S3C2440 */
 
 	/*
 	 * we do sys-critical inits only at reboot,
diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
index 33cd3d4..f0647fb 100644
--- a/drivers/i2c/s3c24x0_i2c.c
+++ b/drivers/i2c/s3c24x0_i2c.c
@@ -31,6 +31,8 @@
 #include <s3c2400.h>
 #elif defined(CONFIG_S3C2410)
 #include <s3c2410.h>
+#elif defined(CONFIG_S3C2440)
+#include <s3c2440.h>
 #endif
 
 #include <asm/io.h>
@@ -61,7 +63,7 @@ static int GetI2CSDA(void)
 {
 	S3C24X0_GPIO * const gpio = S3C24X0_GetBase_GPIO();
 
-#ifdef CONFIG_S3C2410
+#if defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
 	return (readl(&gpio->GPEDAT) & 0x8000) >> 15;
 #endif
 #ifdef CONFIG_S3C2400
@@ -80,7 +82,7 @@ static void SetI2CSCL(int x)
 {
 	S3C24X0_GPIO * const gpio = S3C24X0_GetBase_GPIO();
 
-#ifdef CONFIG_S3C2410
+#if defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
 	writel((readl(&gpio->GPEDAT) & ~0x4000) | (x & 1) << 14, &gpio->GPEDAT);
 #endif
 #ifdef CONFIG_S3C2400
@@ -132,7 +134,7 @@ void i2c_init(int speed, int slaveadd)
 	}
 
 	if ((readl(&i2c->IICSTAT) & I2CSTAT_BSY) || GetI2CSDA() == 0) {
-#ifdef CONFIG_S3C2410
+#if defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
 		ulong old_gpecon = readl(&gpio->GPECON);
 #endif
 #ifdef CONFIG_S3C2400
@@ -141,7 +143,7 @@ void i2c_init(int speed, int slaveadd)
 		/* bus still busy probably by (most) previously interrupted
 		   transfer */
 
-#ifdef CONFIG_S3C2410
+#if defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
 		/* set I2CSDA and I2CSCL (GPE15, GPE14) to GPIO */
 		writel((readl(&gpio->GPECON) & ~0xF0000000) | 0x10000000,
 		       &gpio->GPECON);
@@ -167,7 +169,7 @@ void i2c_init(int speed, int slaveadd)
 		udelay(1000);
 
 		/* restore pin functions */
-#ifdef CONFIG_S3C2410
+#if defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
 		writel(old_gpecon, &gpio->GPECON);
 #endif
 #ifdef CONFIG_S3C2400
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 7dfbeb5..e8740fc 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -43,6 +43,7 @@ COBJS-$(CONFIG_NAND_FSL_UPM) += fsl_upm.o
 COBJS-$(CONFIG_NAND_MPC5121_NFC) += mpc5121_nfc.o
 COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o
 COBJS-$(CONFIG_NAND_S3C2410) += s3c2410_nand.o
+COBJS-$(CONFIG_NAND_S3C2440) += s3c2440_nand.o
 COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o
 COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
 endif
diff --git a/drivers/mtd/nand/s3c2410_nand.c b/drivers/mtd/nand/s3c2410_nand.c
index 60bfd10..b93787c 100644
--- a/drivers/mtd/nand/s3c2410_nand.c
+++ b/drivers/mtd/nand/s3c2410_nand.c
@@ -36,7 +36,7 @@
 static void s3c2410_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 {
 	struct nand_chip *chip = mtd->priv;
-	S3C24X0_NAND * const nand = S3C24X0_GetBase_NAND();
+	S3C2410_NAND * const nand = S3C2410_GetBase_NAND();
 
 	debugX(1, "hwcontrol(): 0x%02x 0x%02x\n", cmd, ctrl);
 
@@ -64,7 +64,7 @@ static void s3c2410_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 
 static int s3c2410_dev_ready(struct mtd_info *mtd)
 {
-	S3C24X0_NAND * const nand = S3C24X0_GetBase_NAND();
+	S3C2410_NAND * const nand = S3C2410_GetBase_NAND();
 
 	debugX(1, "dev_ready\n");
 	return readl(&nand->NFSTAT) & 0x01;
@@ -82,7 +82,7 @@ void s3c2410_nand_enable_hwecc(struct mtd_info *mtd, int mode)
 static int s3c2410_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
 				      u_char *ecc_code)
 {
-	S3C24X0_NAND * const nand = S3C24X0_GetBase_NAND();
+	S3C2410_NAND * const nand = S3C2410_GetBase_NAND();
 	unsigned long ecc = readl(&nand->NFECC);
 
 	ecc_code[0] = ecc;
@@ -112,7 +112,7 @@ int board_nand_init(struct nand_chip *nand)
 	u_int32_t cfg;
 	u_int8_t tacls, twrph0, twrph1;
 	S3C24X0_CLOCK_POWER * const clk_power = S3C24X0_GetBase_CLOCK_POWER();
-	S3C24X0_NAND * const nand_reg = S3C24X0_GetBase_NAND();
+	S3C2410_NAND * const nand_reg = S3C2410_GetBase_NAND();
 
 	debugX(1, "board_nand_init()\n");
 
diff --git a/drivers/mtd/nand/s3c2440_nand.c b/drivers/mtd/nand/s3c2440_nand.c
new file mode 100644
index 0000000..3328da6
--- /dev/null
+++ b/drivers/mtd/nand/s3c2440_nand.c
@@ -0,0 +1,241 @@
+/*
+ * (C) Copyright 2006 OpenMoko, Inc.
+ * Author: Harald Welte <laforge@openmoko.org>
+ *
+ * Modified for the S3C2440 by
+ * (C) Copyright 2009
+ * Kevin Morfitt, Fearnside Systems Ltd, <kevin.morfitt@fearnside-systems.co.uk>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+
+#include <nand.h>
+#include <s3c2440.h>
+#include <asm/io.h>
+
+#define S3C2440_NFCONT_EN		(1<<0)
+#define S3C2440_NFCONT_nFCE		(1<<1)
+#define S3C2440_NFCONT_INITECC		(1<<4)
+#define S3C2440_NFCONT_MAINECCLOCK	(1<<5)
+#define S3C2440_NFCONT_SPAREECCLOCK	(1<<6)
+#define S3C2440_NFCONT_RNBMODE		(1<<8)
+#define S3C2440_NFCONT_RNBINT		(1<<9)
+#define S3C2440_NFCONT_ILLEGALINT	(1<<10)
+#define S3C2440_NFCONT_SOFTLOCK		(1<<12)
+#define S3C2440_NFCONT_LOCKTIGHT	(1<<13)
+
+#define S3C2440_NFCONF_TACLS(x)		((x)<<12)
+#define S3C2440_NFCONF_TWRPH0(x)	((x)<<8)
+#define S3C2440_NFCONF_TWRPH1(x)	((x)<<4)
+
+
+#ifdef CONFIG_S3C2440_NAND_HWECC
+/* new oob placement block for use with hardware ecc generation
+ */
+static struct nand_ecclayout nand_hw_eccoob = {
+	.eccbytes = 3,
+	.eccpos = {0, 1, 2},
+	.oobfree = { {8, 8} }
+};
+#endif
+
+static void s3c2440_nand_select_chip(struct mtd_info *mtd, int chip)
+{
+	S3C2440_NAND * const nand = S3C2440_GetBase_NAND();
+
+	if (chip == -1) {
+		debugX(1, "Negating nFCE\n");
+		writel(readl(&nand->NFCONT) | S3C2440_NFCONT_nFCE,
+		       &nand->NFCONT);
+	} else {
+		debugX(1, "Asserting nFCE\n");
+		writel(readl(&nand->NFCONT) & ~S3C2440_NFCONT_nFCE,
+		       &nand->NFCONT);
+	}
+}
+
+static void s3c2440_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
+{
+	S3C2440_NAND * const nand = S3C2440_GetBase_NAND();
+
+	debugX(1, "hwcontrol(): 0x%02x 0x%02x\n", cmd, ctrl);
+
+	if (cmd == NAND_CMD_NONE)
+		return;
+
+	if (ctrl & NAND_CLE) {
+		debugX(1, "NFCMD = 0x%08X\n", cmd);
+		writel(cmd, &nand->NFCMD);
+	}
+
+	if (ctrl & NAND_ALE) {
+		debugX(1, "NFADDR = 0x%08X\n", cmd);
+		writel(cmd, &nand->NFADDR);
+	}
+}
+
+static int s3c2440_dev_ready(struct mtd_info *mtd)
+{
+	S3C2440_NAND * const nand = S3C2440_GetBase_NAND();
+
+	debugX(1, "dev_ready\n");
+	return readl(&nand->NFSTAT) & 0x01;
+}
+
+#ifdef CONFIG_S3C2440_NAND_HWECC
+void s3c2440_nand_enable_hwecc(struct mtd_info *mtd, int mode)
+{
+	S3C2440_UART * const nand = S3C2440_GetBase_NAND();
+
+	debugX(1, "s3c2440_nand_enable_hwecc(%p, %d)\n", mtd, mode);
+	writel(readl(&nand->NFCONT) | S3C2440_NFCONT_INITECC,
+	       &nand->NFCONT);
+}
+
+static int s3c2440_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
+				      u_char *ecc_code)
+{
+	S3C2440_NAND * const nand = S3C2440_GetBase_NAND();
+	unsigned long ecc = readl(&nand->NFECC);
+
+	ecc_code[0] = ecc;
+	ecc_code[1] = ecc >> 8;
+	ecc_code[2] = ecc >> 16;
+	debugX(1, "s3c2440_nand_calculate_hwecc(%p,): 0x%02x 0x%02x 0x%02x\n",
+	       mtd, ecc_code[0], ecc_code[1], ecc_code[2]);
+
+	return 0;
+}
+
+static int s3c2440_nand_correct_data(struct mtd_info *mtd, u_char *dat,
+				     u_char *read_ecc, u_char *calc_ecc)
+{
+	unsigned int diff0, diff1, diff2;
+	unsigned int bit, byte;
+
+	debugX(2, "s3c2440_nand_correct_data:\n");
+
+	diff0 = read_ecc[0] ^ calc_ecc[0];
+	diff1 = read_ecc[1] ^ calc_ecc[1];
+	diff2 = read_ecc[2] ^ calc_ecc[2];
+
+	debugX(3, "rd %02x%02x%02x calc %02x%02x%02x diff %02x%02x%02x\n",
+		read_ecc[0], read_ecc[1], read_ecc[2],
+		calc_ecc[0], calc_ecc[1], calc_ecc[2],
+		diff0, diff1, diff2);
+
+	if (diff0 == 0 && diff1 == 0 && diff2 == 0)
+		return 0;	/* ECC is ok */
+
+	/* Can we correct this ECC (ie, one row and column change).
+	 * Note, this is similar to the 256 error code on smartmedia */
+
+	if (((diff0 ^ (diff0 >> 1)) & 0x55) == 0x55 &&
+	    ((diff1 ^ (diff1 >> 1)) & 0x55) == 0x55 &&
+	    ((diff2 ^ (diff2 >> 1)) & 0x55) == 0x55) {
+		/* calculate the bit position of the error */
+		bit  = ((diff2 >> 3) & 1) |
+		       ((diff2 >> 4) & 2) |
+		       ((diff2 >> 5) & 4);
+
+		/* calculate the byte position of the error */
+		byte = ((diff2 << 7) & 0x100) |
+		       ((diff1 << 0) & 0x80)  |
+		       ((diff1 << 1) & 0x40)  |
+		       ((diff1 << 2) & 0x20)  |
+		       ((diff1 << 3) & 0x10)  |
+		       ((diff0 >> 4) & 0x08)  |
+		       ((diff0 >> 3) & 0x04)  |
+		       ((diff0 >> 2) & 0x02)  |
+		       ((diff0 >> 1) & 0x01);
+
+		debugX(2, "correcting error bit %d, byte %d\n", bit, byte);
+
+		dat[byte] ^= (1 << bit);
+		return 1;
+	}
+
+	debugX(2, "Failed to correct ECC error\n");
+	return -1;
+}
+#endif
+
+int board_nand_init(struct nand_chip *nand)
+{
+	u_int32_t cfg;
+	u_int8_t tacls, twrph0, twrph1;
+	S3C24X0_CLOCK_POWER * const clk_power = S3C24X0_GetBase_CLOCK_POWER();
+	S3C2440_NAND * const nand_reg = S3C2440_GetBase_NAND();
+
+	debugX(1, "board_nand_init()\n");
+
+	writel(readl(&clk_power->CLKCON) | (1 << 4),
+	       &clk_power->CLKCON);
+
+	/* initialize hardware */
+	twrph0 = 3; twrph1 = 1; tacls = 0;
+
+	cfg = S3C2440_NFCONF_TACLS(tacls);
+	cfg |= S3C2440_NFCONF_TWRPH0(twrph0);
+	cfg |= S3C2440_NFCONF_TWRPH1(twrph1);
+	writel(cfg, &nand->NFCONF);
+
+	cfg = S3C2440_NFCONT_EN;
+	cfg |= S3C2440_NFCONT_nFCE;
+	writel(cfg, &nand->NFCONT);
+
+	/* initialize nand_chip data structure */
+	nand->IO_ADDR_R = nand->IO_ADDR_W = (void *)&nand->NFDATA;
+
+	/* read_buf and write_buf are default */
+	/* read_byte and write_byte are default */
+
+	/* hwcontrol always must be implemented */
+	nand->cmd_ctrl = s3c2440_hwcontrol;
+	nand->dev_ready = s3c2440_dev_ready;
+
+#ifdef CONFIG_S3C2440_NAND_HWECC
+	nand->ecc.correct    = s3c2440_nand_correct_data;
+	nand->ecc.hwctl      = s3c2440_nand_enable_hwecc;
+	nand->ecc.calculate  = s3c2440_nand_calculate_ecc;
+	nand->ecc.mode = NAND_ECC_HW;
+	/* change the behaviour depending on whether we are using
+	 * the large or small page nand device */
+	if (nand->page_shift > 10) {
+		nand->ecc.size    = 256;
+		nand->ecc.bytes   = 3;
+	} else {
+		nand->ecc.size    = 512;
+		nand->ecc.bytes   = 3;
+		nand->ecc.layout  = &nand_hw_eccoob;
+	}
+	debugX(2, "ecc: size: %d bytes: %d\n", nand->ecc.size, nand->ecc.bytes);
+#else
+	nand->ecc.mode = NAND_ECC_SOFT;
+#endif
+
+#ifdef CONFIG_S3C2440_NAND_BBT
+	nand->options = NAND_USE_FLASH_BBT;
+#else
+	nand->options = 0;
+#endif
+
+	debugX(1, "end of nand_init\n");
+
+	return 0;
+}
diff --git a/drivers/rtc/s3c24x0_rtc.c b/drivers/rtc/s3c24x0_rtc.c
index f8e1111..9cdceae 100644
--- a/drivers/rtc/s3c24x0_rtc.c
+++ b/drivers/rtc/s3c24x0_rtc.c
@@ -34,6 +34,8 @@
 #include <s3c2400.h>
 #elif defined(CONFIG_S3C2410)
 #include <s3c2410.h>
+#elif defined(CONFIG_S3C2440)
+#include <s3c2440.h>
 #endif
 
 #include <rtc.h>
diff --git a/drivers/serial/serial_s3c24x0.c b/drivers/serial/serial_s3c24x0.c
index af77db8..f883c7d 100644
--- a/drivers/serial/serial_s3c24x0.c
+++ b/drivers/serial/serial_s3c24x0.c
@@ -23,6 +23,8 @@
 #include <s3c2400.h>
 #elif defined(CONFIG_S3C2410)
 #include <s3c2410.h>
+#elif defined(CONFIG_S3C2440)
+#include <s3c2440.h>
 #endif
 
 DECLARE_GLOBAL_DATA_PTR;
diff --git a/include/common.h b/include/common.h
index ff4f821..a48c60e 100644
--- a/include/common.h
+++ b/include/common.h
@@ -492,7 +492,8 @@ ulong	get_OPB_freq (void);
 ulong	get_PCI_freq (void);
 #endif
 #if defined(CONFIG_S3C2400) || defined(CONFIG_S3C2410) || \
-	defined(CONFIG_LH7A40X) || defined(CONFIG_S3C6400)
+    defined(CONFIG_LH7A40X) || defined(CONFIG_S3C6400) || \
+    defined(CONFIG_S3C2440)
 void	s3c2410_irq(void);
 #define ARM920_IRQ_CALLBACK s3c2410_irq
 ulong	get_FCLK (void);
diff --git a/include/s3c2440.h b/include/s3c2440.h
new file mode 100644
index 0000000..97a4c9c
--- /dev/null
+++ b/include/s3c2440.h
@@ -0,0 +1,232 @@
+/*
+ * (C) Copyright 2003
+ * David M?ller ELSOFT AG Switzerland. d.mueller at elsoft.ch
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * Modified for the Embest SBC2440-II by
+ * (C) Copyright 2009
+ * Kevin Morfitt, Fearnside Systems Ltd, <kevin.morfitt@fearnside-systems.co.uk>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+/************************************************
+ * NAME     : s3c2440.h
+ * Version  : 24.10.2008
+ *
+ * Based on S3C2440X User's manual Rev 1.3
+ ************************************************/
+
+#ifndef __S3C2440_H__
+#define __S3C2440_H__
+
+#define S3C24X0_UART_CHANNELS	3
+#define S3C24X0_SPI_CHANNELS	2
+
+typedef enum {
+	S3C24X0_UART0,
+	S3C24X0_UART1,
+	S3C24X0_UART2
+} S3C24X0_UARTS_NR;
+
+/* S3C2440 device base addresses */
+#define S3C24X0_MEMCTL_BASE		0x48000000
+#define S3C24X0_USB_HOST_BASE		0x49000000
+#define S3C24X0_INTERRUPT_BASE		0x4A000000
+#define S3C24X0_DMA_BASE		0x4B000000
+#define S3C24X0_CLOCK_POWER_BASE	0x4C000000
+#define S3C24X0_LCD_BASE		0x4D000000
+#define S3C2440_NAND_BASE		0x4E000000
+#define S3C2440_CAM_BASE		0x4F000000
+#define S3C24X0_UART_BASE		0x50000000
+#define S3C24X0_TIMER_BASE		0x51000000
+#define S3C24X0_USB_DEVICE_BASE		0x52000140
+#define S3C24X0_WATCHDOG_BASE		0x53000000
+#define S3C24X0_I2C_BASE		0x54000000
+#define S3C24X0_I2S_BASE		0x55000000
+#define S3C24X0_GPIO_BASE		0x56000000
+#define S3C24X0_RTC_BASE		0x57000000
+#define S3C2440_ADC_BASE		0x58000000
+#define S3C24X0_SPI_BASE		0x59000000
+#define S3C2440_SDI_BASE		0x5A000000
+#define S3C2440_AC97_BASE		0x5B000000
+
+/* include common stuff */
+#include <s3c24x0.h>
+
+static inline S3C24X0_MEMCTL *S3C24X0_GetBase_MEMCTL(void)
+{
+	return (S3C24X0_MEMCTL * const)S3C24X0_MEMCTL_BASE;
+}
+static inline S3C24X0_USB_HOST *S3C24X0_GetBase_USB_HOST(void)
+{
+	return (S3C24X0_USB_HOST * const)S3C24X0_USB_HOST_BASE;
+}
+static inline S3C24X0_INTERRUPT *S3C24X0_GetBase_INTERRUPT(void)
+{
+	return (S3C24X0_INTERRUPT * const)S3C24X0_INTERRUPT_BASE;
+}
+static inline S3C24X0_DMAS *S3C24X0_GetBase_DMAS(void)
+{
+	return (S3C24X0_DMAS * const)S3C24X0_DMA_BASE;
+}
+static inline S3C24X0_CLOCK_POWER *S3C24X0_GetBase_CLOCK_POWER(void)
+{
+	return (S3C24X0_CLOCK_POWER * const)S3C24X0_CLOCK_POWER_BASE;
+}
+static inline S3C24X0_LCD *S3C24X0_GetBase_LCD(void)
+{
+	return (S3C24X0_LCD * const)S3C24X0_LCD_BASE;
+}
+static inline S3C2440_NAND *S3C2440_GetBase_NAND(void)
+{
+	return (S3C2440_NAND * const)S3C2440_NAND_BASE;
+}
+static inline S3C2440_CAM *S3C2440_GetBase_CAM(void)
+{
+	return (S3C2440_CAM * const)S3C2440_CAM_BASE;
+}
+static inline S3C24X0_UART *S3C24X0_GetBase_UART(S3C24X0_UARTS_NR nr)
+{
+	return (S3C24X0_UART * const)(S3C24X0_UART_BASE + (nr * 0x4000));
+}
+static inline S3C24X0_TIMERS *S3C24X0_GetBase_TIMERS(void)
+{
+	return (S3C24X0_TIMERS * const)S3C24X0_TIMER_BASE;
+}
+static inline S3C24X0_USB_DEVICE *S3C24X0_GetBase_USB_DEVICE(void)
+{
+	return (S3C24X0_USB_DEVICE * const)S3C24X0_USB_DEVICE_BASE;
+}
+static inline S3C24X0_WATCHDOG *S3C24X0_GetBase_WATCHDOG(void)
+{
+	return (S3C24X0_WATCHDOG * const)S3C24X0_WATCHDOG_BASE;
+}
+static inline S3C24X0_I2C *S3C24X0_GetBase_I2C(void)
+{
+	return (S3C24X0_I2C * const)S3C24X0_I2C_BASE;
+}
+static inline S3C24X0_I2S *S3C24X0_GetBase_I2S(void)
+{
+	return (S3C24X0_I2S * const)S3C24X0_I2S_BASE;
+}
+static inline S3C24X0_GPIO *S3C24X0_GetBase_GPIO(void)
+{
+	return (S3C24X0_GPIO * const)S3C24X0_GPIO_BASE;
+}
+static inline S3C24X0_RTC *S3C24X0_GetBase_RTC(void)
+{
+	return (S3C24X0_RTC * const)S3C24X0_RTC_BASE;
+}
+static inline S3C2440_ADC *S3C2440_GetBase_ADC(void)
+{
+	return (S3C2440_ADC * const)S3C2440_ADC_BASE;
+}
+static inline S3C24X0_SPI *S3C24X0_GetBase_SPI(void)
+{
+	return (S3C24X0_SPI * const)S3C24X0_SPI_BASE;
+}
+static inline S3C2440_SDI *S3C2440_GetBase_SDI(void)
+{
+	return (S3C2440_SDI * const)S3C2440_SDI_BASE;
+}
+static inline S3C2440_AC97 *S3C2440_GetBase_AC97M(void)
+{
+	return (S3C2440_AC97 * const)S3C2440_AC97_BASE;
+}
+
+/* ISR */
+#define pISR_RESET	(*(unsigned *)(_ISR_STARTADDRESS+0x0))
+#define pISR_UNDEF	(*(unsigned *)(_ISR_STARTADDRESS+0x4))
+#define pISR_SWI	(*(unsigned *)(_ISR_STARTADDRESS+0x8))
+#define pISR_PABORT	(*(unsigned *)(_ISR_STARTADDRESS+0xC))
+#define pISR_DABORT	(*(unsigned *)(_ISR_STARTADDRESS+0x10))
+#define pISR_RESERVED	(*(unsigned *)(_ISR_STARTADDRESS+0x14))
+#define pISR_IRQ	(*(unsigned *)(_ISR_STARTADDRESS+0x18))
+#define pISR_FIQ	(*(unsigned *)(_ISR_STARTADDRESS+0x1C))
+
+#define pISR_EINT0	(*(unsigned *)(_ISR_STARTADDRESS+0x20))
+#define pISR_EINT1	(*(unsigned *)(_ISR_STARTADDRESS+0x24))
+#define pISR_EINT2	(*(unsigned *)(_ISR_STARTADDRESS+0x28))
+#define pISR_EINT3	(*(unsigned *)(_ISR_STARTADDRESS+0x2C))
+#define pISR_EINT4_7	(*(unsigned *)(_ISR_STARTADDRESS+0x30))
+#define pISR_EINT8_23	(*(unsigned *)(_ISR_STARTADDRESS+0x34))
+#define pISR_BAT_FLT	(*(unsigned *)(_ISR_STARTADDRESS+0x3C))
+#define pISR_TICK	(*(unsigned *)(_ISR_STARTADDRESS+0x40))
+#define pISR_WDT	(*(unsigned *)(_ISR_STARTADDRESS+0x44))
+#define pISR_TIMER0	(*(unsigned *)(_ISR_STARTADDRESS+0x48))
+#define pISR_TIMER1	(*(unsigned *)(_ISR_STARTADDRESS+0x4C))
+#define pISR_TIMER2	(*(unsigned *)(_ISR_STARTADDRESS+0x50))
+#define pISR_TIMER3	(*(unsigned *)(_ISR_STARTADDRESS+0x54))
+#define pISR_TIMER4	(*(unsigned *)(_ISR_STARTADDRESS+0x58))
+#define pISR_UART2	(*(unsigned *)(_ISR_STARTADDRESS+0x5C))
+#define pISR_NOTUSED	(*(unsigned *)(_ISR_STARTADDRESS+0x60))
+#define pISR_DMA0	(*(unsigned *)(_ISR_STARTADDRESS+0x64))
+#define pISR_DMA1	(*(unsigned *)(_ISR_STARTADDRESS+0x68))
+#define pISR_DMA2	(*(unsigned *)(_ISR_STARTADDRESS+0x6C))
+#define pISR_DMA3	(*(unsigned *)(_ISR_STARTADDRESS+0x70))
+#define pISR_SDI 	(*(unsigned *)(_ISR_STARTADDRESS+0x74))
+#define pISR_SPI0	(*(unsigned *)(_ISR_STARTADDRESS+0x78))
+#define pISR_UART1	(*(unsigned *)(_ISR_STARTADDRESS+0x7C))
+#define pISR_USBD	(*(unsigned *)(_ISR_STARTADDRESS+0x84))
+#define pISR_USBH	(*(unsigned *)(_ISR_STARTADDRESS+0x88))
+#define pISR_IIC	(*(unsigned *)(_ISR_STARTADDRESS+0x8C))
+#define pISR_UART0	(*(unsigned *)(_ISR_STARTADDRESS+0x90))
+#define pISR_SPI1	(*(unsigned *)(_ISR_STARTADDRESS+0x94))
+#define pISR_RTC	(*(unsigned *)(_ISR_STARTADDRESS+0x98))
+#define pISR_ADC	(*(unsigned *)(_ISR_STARTADDRESS+0xA0))
+
+/* PENDING BIT */
+#define BIT_EINT0	(0x1)
+#define BIT_EINT1	(0x1<<1)
+#define BIT_EINT2	(0x1<<2)
+#define BIT_EINT3	(0x1<<3)
+#define BIT_EINT4_7	(0x1<<4)
+#define BIT_EINT8_23	(0x1<<5)
+#define BIT_BAT_FLT	(0x1<<7)
+#define BIT_TICK	(0x1<<8)
+#define BIT_WDT		(0x1<<9)
+#define BIT_TIMER0	(0x1<<10)
+#define BIT_TIMER1	(0x1<<11)
+#define BIT_TIMER2	(0x1<<12)
+#define BIT_TIMER3	(0x1<<13)
+#define BIT_TIMER4	(0x1<<14)
+#define BIT_UART2	(0x1<<15)
+#define BIT_LCD		(0x1<<16)
+#define BIT_DMA0	(0x1<<17)
+#define BIT_DMA1	(0x1<<18)
+#define BIT_DMA2	(0x1<<19)
+#define BIT_DMA3	(0x1<<20)
+#define BIT_SDI		(0x1<<21)
+#define BIT_SPI0	(0x1<<22)
+#define BIT_UART1	(0x1<<23)
+#define BIT_USBD	(0x1<<25)
+#define BIT_USBH	(0x1<<26)
+#define BIT_IIC		(0x1<<27)
+#define BIT_UART0	(0x1<<28)
+#define BIT_SPI1	(0x1<<29)
+#define BIT_RTC		(0x1<<30)
+#define BIT_ADC		(0x1<<31)
+#define BIT_ALLMSK	(0xFFFFFFFF)
+
+#define ClearPending(bit) { \
+	rSRCPND = bit; \
+	rINTPND = bit; \
+	rINTPND;\
+}
+#endif /*__S3C2440_H__*/
diff --git a/include/s3c24x0.h b/include/s3c24x0.h
index da984c0..c521d04 100644
--- a/include/s3c24x0.h
+++ b/include/s3c24x0.h
@@ -80,7 +80,7 @@ typedef struct {
 	S3C24X0_REG32 PRIORITY;
 	S3C24X0_REG32 INTPND;
 	S3C24X0_REG32 INTOFFSET;
-#ifdef CONFIG_S3C2410
+#if defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
 	S3C24X0_REG32 SUBSRCPND;
 	S3C24X0_REG32 INTSUBMSK;
 #endif
@@ -89,11 +89,11 @@ typedef struct {
 /* DMAS (see manual chapter 8) */
 typedef struct {
 	S3C24X0_REG32 DISRC;
-#ifdef CONFIG_S3C2410
+#if defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
 	S3C24X0_REG32 DISRCC;
 #endif
 	S3C24X0_REG32 DIDST;
-#ifdef CONFIG_S3C2410
+#if defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
 	S3C24X0_REG32 DIDSTC;
 #endif
 	S3C24X0_REG32 DCON;
@@ -104,7 +104,7 @@ typedef struct {
 #ifdef CONFIG_S3C2400
 	S3C24X0_REG32 res[1];
 #endif
-#ifdef CONFIG_S3C2410
+#if defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
 	S3C24X0_REG32 res[7];
 #endif
 } /*__attribute__((__packed__))*/ S3C24X0_DMA;
@@ -122,6 +122,9 @@ typedef struct {
 	S3C24X0_REG32 CLKCON;
 	S3C24X0_REG32 CLKSLOW;
 	S3C24X0_REG32 CLKDIVN;
+#ifdef CONFIG_S3C2440
+	S3C24X0_REG32 CAMDIVN;
+#endif
 } /*__attribute__((__packed__))*/ S3C24X0_CLOCK_POWER;
 
 /* LCD CONTROLLER (see manual chapter 15) */
@@ -140,7 +143,7 @@ typedef struct {
 	S3C24X0_REG32 res[8];
 	S3C24X0_REG32 DITHMODE;
 	S3C24X0_REG32 TPAL;
-#ifdef CONFIG_S3C2410
+#if defined(CONFIG_S3C2410) || defined(CONFIG_S3C2440)
 	S3C24X0_REG32 LCDINTPND;
 	S3C24X0_REG32 LCDSRCPND;
 	S3C24X0_REG32 LCDINTMSK;
@@ -158,6 +161,26 @@ typedef struct {
 	S3C24X0_REG32 NFECC;
 } /*__attribute__((__packed__))*/ S3C2410_NAND;
 
+/* NAND FLASH (see S3C2440 manual chapter 6) */
+typedef struct {
+	S3C24X0_REG32 NFCONF;
+	S3C24X0_REG32 NFCONT;
+	S3C24X0_REG32 NFCMD;
+	S3C24X0_REG32 NFADDR;
+	S3C24X0_REG32 NFDATA;
+	S3C24X0_REG32 NFMECCD0;
+	S3C24X0_REG32 NFMECCD1;
+	S3C24X0_REG32 NFSECCD;
+	S3C24X0_REG32 NFSTAT;
+	S3C24X0_REG32 NFESTAT0;
+	S3C24X0_REG32 NFESTAT1;
+	S3C24X0_REG32 NFMECC0;
+	S3C24X0_REG32 NFMECC1;
+	S3C24X0_REG32 NFSECC;
+	S3C24X0_REG32 NFSBLOCK;
+	S3C24X0_REG32 NFEBLOCK;
+} /*__attribute__((__packed__))*/ S3C2440_NAND;
+
 /* UART (see manual chapter 11) */
 typedef struct {
 	S3C24X0_REG32 ULCON;
@@ -324,6 +347,9 @@ typedef struct {
 	S3C24X0_REG32 IICSTAT;
 	S3C24X0_REG32 IICADD;
 	S3C24X0_REG32 IICDS;
+#ifdef CONFIG_S3C2440
+	S3C24X0_REG32 IICLC;
+#endif
 } /*__attribute__((__packed__))*/ S3C24X0_I2C;
 
 /* IIS (see manual chapter 21) */
@@ -439,6 +465,64 @@ typedef struct {
 	S3C24X0_REG32 GSTATUS3;
 	S3C24X0_REG32 GSTATUS4;
 #endif
+#ifdef CONFIG_S3C2440
+	S3C24X0_REG32 GPACON;
+	S3C24X0_REG32 GPADAT;
+	S3C24X0_REG32 res1[2];
+	S3C24X0_REG32 GPBCON;
+	S3C24X0_REG32 GPBDAT;
+	S3C24X0_REG32 GPBUP;
+	S3C24X0_REG32 res2;
+	S3C24X0_REG32 GPCCON;
+	S3C24X0_REG32 GPCDAT;
+	S3C24X0_REG32 GPCUP;
+	S3C24X0_REG32 res3;
+	S3C24X0_REG32 GPDCON;
+	S3C24X0_REG32 GPDDAT;
+	S3C24X0_REG32 GPDUP;
+	S3C24X0_REG32 res4;
+	S3C24X0_REG32 GPECON;
+	S3C24X0_REG32 GPEDAT;
+	S3C24X0_REG32 GPEUP;
+	S3C24X0_REG32 res5;
+	S3C24X0_REG32 GPFCON;
+	S3C24X0_REG32 GPFDAT;
+	S3C24X0_REG32 GPFUP;
+	S3C24X0_REG32 res6;
+	S3C24X0_REG32 GPGCON;
+	S3C24X0_REG32 GPGDAT;
+	S3C24X0_REG32 GPGUP;
+	S3C24X0_REG32 res7;
+	S3C24X0_REG32 GPHCON;
+	S3C24X0_REG32 GPHDAT;
+	S3C24X0_REG32 GPHUP;
+	S3C24X0_REG32 res8;
+
+	S3C24X0_REG32 MISCCR;
+	S3C24X0_REG32 DCLKCON;
+	S3C24X0_REG32 EXTINT0;
+	S3C24X0_REG32 EXTINT1;
+	S3C24X0_REG32 EXTINT2;
+	S3C24X0_REG32 EINTFLT0;
+	S3C24X0_REG32 EINTFLT1;
+	S3C24X0_REG32 EINTFLT2;
+	S3C24X0_REG32 EINTFLT3;
+	S3C24X0_REG32 EINTMASK;
+	S3C24X0_REG32 EINTPEND;
+	S3C24X0_REG32 GSTATUS0;
+	S3C24X0_REG32 GSTATUS1;
+	S3C24X0_REG32 GSTATUS2;
+	S3C24X0_REG32 GSTATUS3;
+	S3C24X0_REG32 GSTATUS4;
+	S3C24X0_REG32 res9;
+
+	S3C24X0_REG32 DSC0;
+	S3C24X0_REG32 DSC1;
+	S3C24X0_REG32 MSLCON;
+	S3C24X0_REG32 GPJCON;
+	S3C24X0_REG32 GPJDAT;
+	S3C24X0_REG32 GPJUP;
+#endif
 } /*__attribute__((__packed__))*/ S3C24X0_GPIO;
 
 
@@ -533,6 +617,16 @@ typedef struct {
 	S3C24X0_REG32 ADCDAT1;
 } /*__attribute__((__packed__))*/ S3C2410_ADC;
 
+/* ADC (see S3C2440 manual chapter 16) */
+typedef struct {
+	S3C24X0_REG32 ADCCON;
+	S3C24X0_REG32 ADCTSC;
+	S3C24X0_REG32 ADCDLY;
+	S3C24X0_REG32 ADCDAT0;
+	S3C24X0_REG32 ADCDAT1;
+	S3C24X0_REG32 ADCUPDN;
+} /*__attribute__((__packed__))*/ S3C2440_ADC;
+
 /* SPI (see manual chapter 22) */
 typedef struct {
 	S3C24X0_REG32 SPCON;
@@ -630,4 +724,86 @@ typedef struct {
 	S3C24X0_REG32 SDIIMSK;
 } /*__attribute__((__packed__))*/ S3C2410_SDI;
 
+/* SD INTERFACE (see S3C2440 manual chapter 19) */
+typedef struct {
+	S3C24X0_REG32 SDICON;
+	S3C24X0_REG32 SDIPRE;
+	S3C24X0_REG32 SDICARG;
+	S3C24X0_REG32 SDICCON;
+	S3C24X0_REG32 SDICSTA;
+	S3C24X0_REG32 SDIRSP0;
+	S3C24X0_REG32 SDIRSP1;
+	S3C24X0_REG32 SDIRSP2;
+	S3C24X0_REG32 SDIRSP3;
+	S3C24X0_REG32 SDIDTIMER;
+	S3C24X0_REG32 SDIBSIZE;
+	S3C24X0_REG32 SDIDCON;
+	S3C24X0_REG32 SDIDCNT;
+	S3C24X0_REG32 SDIDSTA;
+	S3C24X0_REG32 SDIFSTA;
+	S3C24X0_REG32 SDIIMSK;
+#ifdef __BIG_ENDIAN
+	S3C24X0_REG8 res[3];
+	S3C24X0_REG8 SDIDAT;
+#else
+	S3C24X0_REG8 SDIDAT;
+	S3C24X0_REG8 res[3];
+#endif
+} /*__attribute__((__packed__))*/ S3C2440_SDI;
+
+/* CAMERA INTERFACE (see S3C2440 manual chapter 23) */
+typedef struct {
+	S3C24X0_REG32 CISRCFMT;
+	S3C24X0_REG32 CIWDOFST;
+	S3C24X0_REG32 CIGCTRL;
+	S3C24X0_REG32 res1[3];
+	S3C24X0_REG32 CICOYSA1;
+	S3C24X0_REG32 CICOYSA2;
+	S3C24X0_REG32 CICOYSA3;
+	S3C24X0_REG32 CICOYSA4;
+	S3C24X0_REG32 CICOCBSA1;
+	S3C24X0_REG32 CICOCBSA2;
+	S3C24X0_REG32 CICOCBSA3;
+	S3C24X0_REG32 CICOCBSA4;
+	S3C24X0_REG32 CICOCRSA1;
+	S3C24X0_REG32 CICOCRSA2;
+	S3C24X0_REG32 CICOCRSA3;
+	S3C24X0_REG32 CICOCRSA4;
+	S3C24X0_REG32 CICOTRGFMT;
+	S3C24X0_REG32 CICOCTRL;
+	S3C24X0_REG32 CICOSCPRERATIO;
+	S3C24X0_REG32 CICOSCPREDST;
+	S3C24X0_REG32 CICOSCCTRL;
+	S3C24X0_REG32 CICOTAREA;
+	S3C24X0_REG32 res2;
+	S3C24X0_REG32 CICOSTATUS;
+	S3C24X0_REG32 res3;
+	S3C24X0_REG32 CIPRCLRSA1;
+	S3C24X0_REG32 CIPRCLRSA2;
+	S3C24X0_REG32 CIPRCLRSA3;
+	S3C24X0_REG32 CIPRCLRSA4;
+	S3C24X0_REG32 CIPRTRGFMT;
+	S3C24X0_REG32 CIPRCTRL;
+	S3C24X0_REG32 CIPRSCPRERATIO;
+	S3C24X0_REG32 CIPRSCPREDST;
+	S3C24X0_REG32 CIPRSCCTRL;
+	S3C24X0_REG32 CIPRTAREA;
+	S3C24X0_REG32 res4;
+	S3C24X0_REG32 CIPRSTATUS;
+	S3C24X0_REG32 res5;
+	S3C24X0_REG32 CIIMGCPT;
+} /*__attribute__((__packed__))*/ S3C2440_CAM;
+
+/* 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;
+
 #endif /*__S3C24X0_H__*/
-- 
1.6.0.6

[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

[Wolfgang Denk]

Dear "kevin.morfitt at fearnside-systems.co.uk",

In message <4A3BC001.3010103@fearnside-systems.co.uk> you 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@fearnside-systems.co.uk>

Reviewed-by: Wolfgang Denk <wd@denx.de>

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Monday is an awful way to spend one seventh of your life.

[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

[Jean-Christophe PLAGNIOL-VILLARD]

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@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
> 
> 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
>  #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
>  
>  #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

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

Best Regards,
J.

[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

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

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

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

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.

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
- [U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board
(ie this
  patch)
- [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
  June
 
Best Regards
Kevin
> Best Regards,
> J.
>
> __________ Information from ESET NOD32 Antivirus, version of virus signature database 4173 (20090620) __________
>
> The message was checked by ESET NOD32 Antivirus.
>
> http://www.eset.com
>
>
>
>
>   



__________ Information from ESET NOD32 Antivirus, version of virus signature database 4174 (20090620) __________

The message was checked by ESET NOD32 Antivirus.

http://www.eset.com

[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

[Jean-Christophe PLAGNIOL-VILLARD]

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

[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

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

On 21/06/2009 10:46, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 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.
>>
<snip>
>> 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

when you say 'thread all of them' do you mean send them separately but 
numbered in the Subject field like 1/5, 2/5 etc?

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

The patch above does a code style clean-up of the s3c24x0 drivers code. 
This includes the mtd nand s3c2410 driver. Do I need to put the mtd nand 
s3c2410 driver clean-up in a separate patch or can I leave it in the 
same patch but just include something like PATCH-ARM,MTD-NAND in the 
Subject field?

> 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.
> 
> __________ Information from ESET NOD32 Antivirus, version of virus signature database 4174 (20090620) __________
> 
> The message was checked by ESET NOD32 Antivirus.
> 
> http://www.eset.com
> 



__________ Information from ESET NOD32 Antivirus, version of virus signature database 4174 (20090620) __________

The message was checked by ESET NOD32 Antivirus.

http://www.eset.com

[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

[Scott Wood]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> 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

If a NAND patch is sandwiched in the middle of other patches that need 
to go via an arch tree, and depends on some of those patches, I'd rather 
ack the NAND bits to go through the arch tree rather than try to merge 
everything separately while still maintaining dependencies.  In that 
case, keeping the NAND bits in a separate patch is nice, but not 
mandatory IMHO.

I'll look at that patch soon.

-Scott

[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

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

On 22/06/2009 20:04, Scott Wood wrote:
> Jean-Christophe PLAGNIOL-VILLARD wrote:
>> 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
> 
> If a NAND patch is sandwiched in the middle of other patches that need 
> to go via an arch tree, and depends on some of those patches, I'd rather 
> ack the NAND bits to go through the arch tree rather than try to merge 
> everything separately while still maintaining dependencies.  In that 
> case, keeping the NAND bits in a separate patch is nice, but not 
> mandatory IMHO.
> 

Thanks, leaving the NAND changes in the arch-specific patch would be simpler 
for me. I have to make some changes to the SBC2440 patch anyway so I'll make 
sure I cc you when I re-send it.

> I'll look at that patch soon.
> 
> -Scott
> 



__________ Information from ESET NOD32 Antivirus, version of virus signature database 4179 (20090622) __________

The message was checked by ESET NOD32 Antivirus.

http://www.eset.com

[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

[Jean-Christophe PLAGNIOL-VILLARD]

On 14:04 Mon 22 Jun     , Scott Wood wrote:
> Jean-Christophe PLAGNIOL-VILLARD wrote:
> >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
> 
> If a NAND patch is sandwiched in the middle of other patches that
> need to go via an arch tree, and depends on some of those patches,
> I'd rather ack the NAND bits to go through the arch tree rather than
> try to merge everything separately while still maintaining
> dependencies.  In that case, keeping the NAND bits in a separate
> patch is nice, but not mandatory IMHO.
To simplify bisect and merge I prefer to have the nand adding in an other
patch

Best Regards,
J.

[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

[Scott Wood]

Keven Morfitt wrote:
> diff --git a/drivers/mtd/nand/s3c2410_nand.c b/drivers/mtd/nand/s3c2410_nand.c
> index 60bfd10..b93787c 100644
> --- a/drivers/mtd/nand/s3c2410_nand.c
> +++ b/drivers/mtd/nand/s3c2410_nand.c
> @@ -36,7 +36,7 @@
>  static void s3c2410_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
>  {
>  	struct nand_chip *chip = mtd->priv;
> -	S3C24X0_NAND * const nand = S3C24X0_GetBase_NAND();
> +	S3C2410_NAND * const nand = S3C2410_GetBase_NAND();

This isn't new to this patch, but please don't make type names look like
preprocessor macros (i.e. not ALL CAPS).  Also, no space after * when
it means pointer and not multiply.

Is there any particular reason to declare the pointer itself as const? 
It's just a local variable.

> +#ifdef CONFIG_S3C2440_NAND_HWECC
> +/* new oob placement block for use with hardware ecc generation
> + */
> +static struct nand_ecclayout nand_hw_eccoob = {
> +	.eccbytes = 3,
> +	.eccpos = {0, 1, 2},
> +	.oobfree = { {8, 8} }
> +};

Any reason why bytes 3, 4, 6, and 7 aren't free?

> +static int s3c2440_dev_ready(struct mtd_info *mtd)
> +{
> +	S3C2440_NAND * const nand = S3C2440_GetBase_NAND();
> +
> +	debugX(1, "dev_ready\n");
> +	return readl(&nand->NFSTAT) & 0x01;
> +}
> +
> +#ifdef CONFIG_S3C2440_NAND_HWECC
> +void s3c2440_nand_enable_hwecc(struct mtd_info *mtd, int mode)
> +{
> +	S3C2440_UART * const nand = S3C2440_GetBase_NAND();

UART?

> +static int s3c2440_nand_correct_data(struct mtd_info *mtd, u_char *dat,
> +				     u_char *read_ecc, u_char *calc_ecc)
> +{
> +	unsigned int diff0, diff1, diff2;
> +	unsigned int bit, byte;
> +
> +	debugX(2, "s3c2440_nand_correct_data:\n");
> +
> +	diff0 = read_ecc[0] ^ calc_ecc[0];
> +	diff1 = read_ecc[1] ^ calc_ecc[1];
> +	diff2 = read_ecc[2] ^ calc_ecc[2];
> +
> +	debugX(3, "rd %02x%02x%02x calc %02x%02x%02x diff %02x%02x%02x\n",
> +		read_ecc[0], read_ecc[1], read_ecc[2],
> +		calc_ecc[0], calc_ecc[1], calc_ecc[2],
> +		diff0, diff1, diff2);
> +
> +	if (diff0 == 0 && diff1 == 0 && diff2 == 0)
> +		return 0;	/* ECC is ok */
> +
> +	/* Can we correct this ECC (ie, one row and column change).
> +	 * Note, this is similar to the 256 error code on smartmedia */
> +
> +	if (((diff0 ^ (diff0 >> 1)) & 0x55) == 0x55 &&
> +	    ((diff1 ^ (diff1 >> 1)) & 0x55) == 0x55 &&
> +	    ((diff2 ^ (diff2 >> 1)) & 0x55) == 0x55) {
> +		/* calculate the bit position of the error */
> +		bit  = ((diff2 >> 3) & 1) |
> +		       ((diff2 >> 4) & 2) |
> +		       ((diff2 >> 5) & 4);
> +
> +		/* calculate the byte position of the error */
> +		byte = ((diff2 << 7) & 0x100) |
> +		       ((diff1 << 0) & 0x80)  |
> +		       ((diff1 << 1) & 0x40)  |
> +		       ((diff1 << 2) & 0x20)  |
> +		       ((diff1 << 3) & 0x10)  |
> +		       ((diff0 >> 4) & 0x08)  |
> +		       ((diff0 >> 3) & 0x04)  |
> +		       ((diff0 >> 2) & 0x02)  |
> +		       ((diff0 >> 1) & 0x01);
> +
> +		debugX(2, "correcting error bit %d, byte %d\n", bit, byte);
> +
> +		dat[byte] ^= (1 << bit);
> +		return 1;
> +	}
> +
> +	debugX(2, "Failed to correct ECC error\n");
> +	return -1;
> +}

Hmm, this looks very similar to the generic correct_data, except that it
uses a 512-byte block (and is missing the final countbits check -- not
sure what that's for, perhaps a single bit error in the ECC itself?). 

Perhaps it should be factored out into the generic code?

> +int board_nand_init(struct nand_chip *nand)
> +{
> +	u_int32_t cfg;
> +	u_int8_t tacls, twrph0, twrph1;
> +	S3C24X0_CLOCK_POWER * const clk_power = S3C24X0_GetBase_CLOCK_POWER();
> +	S3C2440_NAND * const nand_reg = S3C2440_GetBase_NAND();
> +
> +	debugX(1, "board_nand_init()\n");
> +
> +	writel(readl(&clk_power->CLKCON) | (1 << 4),
> +	       &clk_power->CLKCON);
> +
> +	/* initialize hardware */
> +	twrph0 = 3; twrph1 = 1; tacls = 0;

Put each statement on its own line.  Do these really need to be held in
local variables rather than just passing them directly to these macros:

> +	cfg = S3C2440_NFCONF_TACLS(tacls);
> +	cfg |= S3C2440_NFCONF_TWRPH0(twrph0);
> +	cfg |= S3C2440_NFCONF_TWRPH1(twrph1);

...which include the same name, so it doesn't have any descriptive value?

Should the values come from the board config file?

-Scott

[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

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

On 22/06/2009 20:26, Scott Wood wrote:
> Keven Morfitt wrote:
>> diff --git a/drivers/mtd/nand/s3c2410_nand.c b/drivers/mtd/nand/s3c2410_nand.c
>> index 60bfd10..b93787c 100644
>> --- a/drivers/mtd/nand/s3c2410_nand.c
>> +++ b/drivers/mtd/nand/s3c2410_nand.c
>> @@ -36,7 +36,7 @@
>>  static void s3c2410_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
>>  {
>>  	struct nand_chip *chip = mtd->priv;
>> -	S3C24X0_NAND * const nand = S3C24X0_GetBase_NAND();
>> +	S3C2410_NAND * const nand = S3C2410_GetBase_NAND();
> 
> This isn't new to this patch, but please don't make type names look like
> preprocessor macros (i.e. not ALL CAPS).  Also, no space after * when
> it means pointer and not multiply.
> 
> Is there any particular reason to declare the pointer itself as const? 
> It's just a local variable.
> 

These type names (and the 'const') are in the existing s3c24x0 code so I 
just made my new code follow the same style and Lindent and checkpatch 
didn't complain. The u-boot coding style guidelines say we should use the 
Linux coding style and this says that 'mixed case names are frowned upon' 
and 'It's a _mistake_ to use typedef for structures' so it doesn't meet 
the coding style, at least for the use of typedef if not for the upper 
case names. I'll change it throughout the s3c24x0 code.

>> +#ifdef CONFIG_S3C2440_NAND_HWECC
>> +/* new oob placement block for use with hardware ecc generation
>> + */
>> +static struct nand_ecclayout nand_hw_eccoob = {
>> +	.eccbytes = 3,
>> +	.eccpos = {0, 1, 2},
>> +	.oobfree = { {8, 8} }
>> +};
> 
> Any reason why bytes 3, 4, 6, and 7 aren't free?
> 

I ported this from the Linux s3c2410 NAND driver (which covers s3c2440
as well as s3c2410). It worked when I tested it (after I enabled hardware
ECC and fixed the problem below), but I don't know enough about how mtd
hardware ecc works to understand why it was done this way in the Linux 
kernel. A comment in the kernel code says that nand_ecclayout is 
'Exported to userspace for diagnosis and to allow creation of raw 
images' so it's likely I haven't tested this bit as all I did was check 
that NAND read/write worked. I'll have a look at it in more detail.

>> +static int s3c2440_dev_ready(struct mtd_info *mtd)
>> +{
>> +	S3C2440_NAND * const nand = S3C2440_GetBase_NAND();
>> +
>> +	debugX(1, "dev_ready\n");
>> +	return readl(&nand->NFSTAT) & 0x01;
>> +}
>> +
>> +#ifdef CONFIG_S3C2440_NAND_HWECC
>> +void s3c2440_nand_enable_hwecc(struct mtd_info *mtd, int mode)
>> +{
>> +	S3C2440_UART * const nand = S3C2440_GetBase_NAND();
> 
> UART?
> 

Thanks, I should obviously be more careful with cut and paste! I also 
forgot to test the patch with hardware ECC enabled.

>> +static int s3c2440_nand_correct_data(struct mtd_info *mtd, u_char *dat,
>> +				     u_char *read_ecc, u_char *calc_ecc)
>> +{
>> +	unsigned int diff0, diff1, diff2;
>> +	unsigned int bit, byte;
>> +
>> +	debugX(2, "s3c2440_nand_correct_data:\n");
>> +
>> +	diff0 = read_ecc[0] ^ calc_ecc[0];
>> +	diff1 = read_ecc[1] ^ calc_ecc[1];
>> +	diff2 = read_ecc[2] ^ calc_ecc[2];
>> +
>> +	debugX(3, "rd %02x%02x%02x calc %02x%02x%02x diff %02x%02x%02x\n",
>> +		read_ecc[0], read_ecc[1], read_ecc[2],
>> +		calc_ecc[0], calc_ecc[1], calc_ecc[2],
>> +		diff0, diff1, diff2);
>> +
>> +	if (diff0 == 0 && diff1 == 0 && diff2 == 0)
>> +		return 0;	/* ECC is ok */
>> +
>> +	/* Can we correct this ECC (ie, one row and column change).
>> +	 * Note, this is similar to the 256 error code on smartmedia */
>> +
>> +	if (((diff0 ^ (diff0 >> 1)) & 0x55) == 0x55 &&
>> +	    ((diff1 ^ (diff1 >> 1)) & 0x55) == 0x55 &&
>> +	    ((diff2 ^ (diff2 >> 1)) & 0x55) == 0x55) {
>> +		/* calculate the bit position of the error */
>> +		bit  = ((diff2 >> 3) & 1) |
>> +		       ((diff2 >> 4) & 2) |
>> +		       ((diff2 >> 5) & 4);
>> +
>> +		/* calculate the byte position of the error */
>> +		byte = ((diff2 << 7) & 0x100) |
>> +		       ((diff1 << 0) & 0x80)  |
>> +		       ((diff1 << 1) & 0x40)  |
>> +		       ((diff1 << 2) & 0x20)  |
>> +		       ((diff1 << 3) & 0x10)  |
>> +		       ((diff0 >> 4) & 0x08)  |
>> +		       ((diff0 >> 3) & 0x04)  |
>> +		       ((diff0 >> 2) & 0x02)  |
>> +		       ((diff0 >> 1) & 0x01);
>> +
>> +		debugX(2, "correcting error bit %d, byte %d\n", bit, byte);
>> +
>> +		dat[byte] ^= (1 << bit);
>> +		return 1;
>> +	}
>> +
>> +	debugX(2, "Failed to correct ECC error\n");
>> +	return -1;
>> +}
> 
> Hmm, this looks very similar to the generic correct_data, except that it
> uses a 512-byte block (and is missing the final countbits check -- not
> sure what that's for, perhaps a single bit error in the ECC itself?). 
> 
> Perhaps it should be factored out into the generic code?
> 

I'll have a look at doing that. I obviously need to look at it in more
detail.

>> +int board_nand_init(struct nand_chip *nand)
>> +{
>> +	u_int32_t cfg;
>> +	u_int8_t tacls, twrph0, twrph1;
>> +	S3C24X0_CLOCK_POWER * const clk_power = S3C24X0_GetBase_CLOCK_POWER();
>> +	S3C2440_NAND * const nand_reg = S3C2440_GetBase_NAND();
>> +
>> +	debugX(1, "board_nand_init()\n");
>> +
>> +	writel(readl(&clk_power->CLKCON) | (1 << 4),
>> +	       &clk_power->CLKCON);
>> +
>> +	/* initialize hardware */
>> +	twrph0 = 3; twrph1 = 1; tacls = 0;
> 
> Put each statement on its own line.  Do these really need to be held in
> local variables rather than just passing them directly to these macros:
> 
>> +	cfg = S3C2440_NFCONF_TACLS(tacls);
>> +	cfg |= S3C2440_NFCONF_TWRPH0(twrph0);
>> +	cfg |= S3C2440_NFCONF_TWRPH1(twrph1);
> 
> ...which include the same name, so it doesn't have any descriptive value?
> 

Again, this comes from the existing s3c24x0 code so I'll change it
throughout.

> Should the values come from the board config file?
> 

Yes. I'll add these to the board config file.

Thanks
Kevin

> -Scott
> 



__________ Information from ESET NOD32 Antivirus, version of virus signature database 4179 (20090622) __________

The message was checked by ESET NOD32 Antivirus.

http://www.eset.com

[U-Boot] [PATCH-ARM 1/2] Add support for the Embest SBC2440-II Board

[Scott Wood]

kevin.morfitt at fearnside-systems.co.uk wrote:
> These type names (and the 'const') are in the existing s3c24x0 code so I 
> just made my new code follow the same style and Lindent and checkpatch 
> didn't complain. The u-boot coding style guidelines say we should use the 
> Linux coding style and this says that 'mixed case names are frowned upon' 
> and 'It's a _mistake_ to use typedef for structures'

I love it when someone justifies their opinion by asserting that the 
alternative is "a _mistake_". :-)

> so it doesn't meet 
> the coding style, at least for the use of typedef if not for the upper 
> case names.

Upper case names are for macros in the Linux/u-boot code style.

> I ported this from the Linux s3c2410 NAND driver (which covers s3c2440
> as well as s3c2410). It worked when I tested it (after I enabled hardware
> ECC and fixed the problem below), but I don't know enough about how mtd
> hardware ecc works to understand why it was done this way in the Linux 
> kernel. A comment in the kernel code says that nand_ecclayout is 
> 'Exported to userspace for diagnosis and to allow creation of raw 
> images' so it's likely I haven't tested this bit as all I did was check 
> that NAND read/write worked. I'll have a look at it in more detail.

It's relevant for things like JFFS2, which use the free area for their 
own markers.  It looks like 8 bytes is enough for that, though -- and 
being in sync with Linux is the most important.  It may also be useful 
to reserve some bytes for alterate ECC schemes.

-Scott