[U-Boot] [PATCH 0/2] Add a new Nomadik board

[U-Boot] [PATCH 0/2] Add a new Nomadik board

[Alessandro Rubini]

These two patches add the USB-S8815 board by Calao.

The board is very similar to the nhk8815 evaluation kit,
but the NAND is connected on a 16-bit bus. Therefore, the first patch
adds the option to have 16-bit nand, and the second one uses it.

The patch depends on the LCD support patches I sent on Dec 05.  Some
of them were picked up in the video-next branch (Dec 05), but I had no
feedback about the other ones (actually, only those two are needed for
this board, the other ones are for nhk8815 devices that are missing
here).

Also, for the board to work the pach for reset timer is needed.  I
sent V4 on Nov 25th and I had no feedback (nor has it been applied).
The trivial patch is needed for NAND operations to work reliably on
both nhk8815 and usb-s8815 (this one).

Alessandro Rubini (2):
  NAND Nomadik: add CONFIG_NAND_NOMADIK_16BIT option
  Add Nomadik board "usb-s8815" by Calao vendor

 MAINTAINERS                                        |    4 +
 Makefile                                           |    4 +
 board/{st/nhk8815 => calao/usb-s8815}/Makefile     |    3 +-
 board/{st/nhk8815 => calao/usb-s8815}/config.mk    |    0
 board/{st/nhk8815 => calao/usb-s8815}/lcd.c        |   34 +------
 board/{st/nhk8815 => calao/usb-s8815}/platform.S   |    0
 .../nhk8815.c => calao/usb-s8815/usb-s8815.c}      |   40 +--------
 drivers/mtd/nand/nomadik.c                         |   15 ++-
 include/configs/{nhk8815.h => usb-s8815.h}         |   98 ++++----------------
 9 files changed, 47 insertions(+), 151 deletions(-)
 copy board/{st/nhk8815 => calao/usb-s8815}/Makefile (96%)
 copy board/{st/nhk8815 => calao/usb-s8815}/config.mk (100%)
 copy board/{st/nhk8815 => calao/usb-s8815}/lcd.c (68%)
 copy board/{st/nhk8815 => calao/usb-s8815}/platform.S (100%)
 copy board/{st/nhk8815/nhk8815.c => calao/usb-s8815/usb-s8815.c} (67%)
 copy include/configs/{nhk8815.h => usb-s8815.h} (61%)

[U-Boot] [PATCH 1/2] NAND Nomadik: add CONFIG_NAND_NOMADIK_16BIT option

[Alessandro Rubini]

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
Cc: Gregory Hermant <gregory.hermant@calao-systems.com>
---
 drivers/mtd/nand/nomadik.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/nomadik.c b/drivers/mtd/nand/nomadik.c
index b76f4cb..149f68b 100644
--- a/drivers/mtd/nand/nomadik.c
+++ b/drivers/mtd/nand/nomadik.c
@@ -196,13 +196,20 @@ static void nomadik_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 
 int board_nand_init(struct nand_chip *chip)
 {
-	/* Set up the FSMC_PCR0 for nand access*/
-	writel(0x0000004a, REG_FSMC_PCR0);
-	/* Set up FSMC_PMEM0, FSMC_PATT0 with timing data for access */
+	u32 pcr0_val;
+
+	chip->options = NAND_COPYBACK |	NAND_CACHEPRG | NAND_NO_PADDING;
+	pcr0_val = 0x0000004a; /* Enabled, nand, enable ecc hw */
+
+#ifdef CONFIG_NAND_NOMADIK_16BIT
+	chip->options |= NAND_BUSWIDTH_16;
+	pcr0_val |= 0x00000010; /* 16-bit access */
+#endif
+	/* Set up FSMC control0, PMEM0, PATT0 with timing data for access */
+	writel(pcr0_val, REG_FSMC_PCR0);
 	writel(0x00020401, REG_FSMC_PMEM0);
 	writel(0x00020404, REG_FSMC_PATT0);
 
-	chip->options = NAND_COPYBACK |	NAND_CACHEPRG | NAND_NO_PADDING;
 	chip->cmd_ctrl = nomadik_nand_hwcontrol;
 	chip->dev_ready = nomadik_nand_ready;
 	/* The chip allows 32bit reads, so avoid the default 8bit copy */
-- 
1.6.0.2

[U-Boot] [PATCH 2/2] Add Nomadik board "usb-s8815" by Calao vendor

[Alessandro Rubini]

Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
Cc: Gregory Hermant <gregory.hermant@calao-systems.com>
---
 MAINTAINERS                                        |    4 +
 Makefile                                           |    4 +
 board/{st/nhk8815 => calao/usb-s8815}/Makefile     |    3 +-
 board/{st/nhk8815 => calao/usb-s8815}/config.mk    |    0
 board/{st/nhk8815 => calao/usb-s8815}/lcd.c        |   34 +------
 board/{st/nhk8815 => calao/usb-s8815}/platform.S   |    0
 .../nhk8815.c => calao/usb-s8815/usb-s8815.c}      |   40 +--------
 include/configs/{nhk8815.h => usb-s8815.h}         |   98 ++++----------------
 8 files changed, 36 insertions(+), 147 deletions(-)
 copy board/{st/nhk8815 => calao/usb-s8815}/Makefile (96%)
 copy board/{st/nhk8815 => calao/usb-s8815}/config.mk (100%)
 copy board/{st/nhk8815 => calao/usb-s8815}/lcd.c (68%)
 copy board/{st/nhk8815 => calao/usb-s8815}/platform.S (100%)
 copy board/{st/nhk8815/nhk8815.c => calao/usb-s8815/usb-s8815.c} (67%)
 copy include/configs/{nhk8815.h => usb-s8815.h} (61%)

diff --git a/MAINTAINERS b/MAINTAINERS
index e8ba4bc..9d97b40 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -699,6 +699,10 @@ Nomadik Linux Team <STN_WMM_nomadik_linux@list.st.com>
 
 	nhk8815		ARM926EJS (Nomadik 8815 Soc)
 
+Alessandro Rubini <rubini@gnudd.com>
+
+	usb-s8815	ARM926EJS (Nomadik 8815 Soc)
+
 Steve Sakoman <sakoman@gmail.com>
 
 	omap3_overo	ARM CORTEX-A8 (OMAP3xx SoC)
diff --git a/Makefile b/Makefile
index 793fcec..39e9a09 100644
--- a/Makefile
+++ b/Makefile
@@ -3068,6 +3068,10 @@ trab_old_config:	unconfig
 		}
 	@$(MKCONFIG) -a $(call xtract_trab,$@) arm arm920t trab NULL s3c24x0
 
+usb-s8815_config:	unconfig
+	@$(MKCONFIG) $(@:_config=) arm arm926ejs usb-s8815 calao nomadik
+
+
 VCMA9_config	:	unconfig
 	@$(MKCONFIG) $(@:_config=) arm arm920t vcma9 mpl s3c24x0
 
diff --git a/board/st/nhk8815/Makefile b/board/calao/usb-s8815/Makefile
similarity index 96%
copy from board/st/nhk8815/Makefile
copy to board/calao/usb-s8815/Makefile
index 7155f12..730d374 100644
--- a/board/st/nhk8815/Makefile
+++ b/board/calao/usb-s8815/Makefile
@@ -29,8 +29,7 @@ include $(TOPDIR)/config.mk
 
 LIB	= $(obj)lib$(BOARD).a
 
-COBJS-y	:= nhk8815.o
-COBJS-$(CONFIG_NHK8815_KEYPAD) += keypad.o
+COBJS-y	:= usb-s8815.o
 COBJS-$(CONFIG_LCD) += lcd.o
 
 COBJS	:= $(COBJS-y)
diff --git a/board/st/nhk8815/config.mk b/board/calao/usb-s8815/config.mk
similarity index 100%
copy from board/st/nhk8815/config.mk
copy to board/calao/usb-s8815/config.mk
diff --git a/board/st/nhk8815/lcd.c b/board/calao/usb-s8815/lcd.c
similarity index 68%
copy from board/st/nhk8815/lcd.c
copy to board/calao/usb-s8815/lcd.c
index d3acb48..744da67 100644
--- a/board/st/nhk8815/lcd.c
+++ b/board/calao/usb-s8815/lcd.c
@@ -1,5 +1,5 @@
 /*
- * board/st/nhk8815/lcd.c: use amba clcd and STMPE2401 for backlight/reset
+ * board/calao/usb-s8815/lcd.c: use amba clcd and STMPE2401 for backlight/reset
  *
  * Copyright 2009 Alessandro Rubini <rubini@unipv.it>
  *
@@ -24,7 +24,7 @@
 #include <common.h>
 #include <lcd.h>
 #include <amba_clcd.h>
-#include <stmpe2401.h>
+#include <asm/arch/gpio.h>
 
 /* Two configurations are supported: 32bpp and 16bpp */
 #if LCD_BPP == LCD_COLOR32
@@ -38,7 +38,7 @@
 #endif
 
 /* Horribly, these are precomputed registers */
-struct clcd_config nhk8815_clcd_config = {
+struct clcd_config s8815_clcd_config = {
 	.address =	(struct clcd_registers *)NOMADIK_CLCDC_BASE,
 	.tim0 =		0xd52600c4,	/* horizontal timings */
 	.tim1 =		0x220a01df,	/* vertical timings */
@@ -53,36 +53,14 @@ vidinfo_t panel_info = {
 	.vl_col =	800,
 	.vl_row =	480,
 	.vl_bpix =	CLCD_BPIX_VAL,
-	.priv =		&nhk8815_clcd_config,
+	.priv =		&s8815_clcd_config,
 };
 
-/* Don't turn on (too early), but configure data lines and remove reset */
+/* Set GPIO32..GPIO39 to ALTB, to enable LCD16..LCD23 */
 void lcd_enable(void)
 {
 	int i;
 
-	/* Turn the alternate functions as needed */
-	for (i = 32; i <= 39; i++)
+	for (i = 32; i < 40; i++)
 		nmk_gpio_af(i, GPIO_ALT_B);
-
-	/* EXP1_GPIO_5 = output high -- remove reset from display */
-	pe_gpio_af(STMPE1, 5, PE_GPIO_AF_GPIO);
-	pe_gpio_dir(STMPE1, 5, 1);
-	pe_gpio_set(STMPE1, 5, 1);
-}
-
-/* Called from late_init: we turn on the backlight through port expander */
-int nhk8815_backlight_on(void)
-{
-	int i;
-
-	/* Turn the alternate functions as needed */
-	for (i = 32; i <= 39; i++)
-		nmk_gpio_af(i, GPIO_ALT_B);
-
-	/* EXP0_GPIO_21 = output high -- backlight */
-	pe_gpio_af(STMPE0, 21, PE_GPIO_AF_GPIO);
-	pe_gpio_dir(STMPE0, 21, 1);
-	pe_gpio_set(STMPE0, 21, 1);
-	return 0;
 }
diff --git a/board/st/nhk8815/platform.S b/board/calao/usb-s8815/platform.S
similarity index 100%
copy from board/st/nhk8815/platform.S
copy to board/calao/usb-s8815/platform.S
diff --git a/board/st/nhk8815/nhk8815.c b/board/calao/usb-s8815/usb-s8815.c
similarity index 67%
copy from board/st/nhk8815/nhk8815.c
copy to board/calao/usb-s8815/usb-s8815.c
index fedb3c0..12ae810 100644
--- a/board/st/nhk8815/nhk8815.c
+++ b/board/calao/usb-s8815/usb-s8815.c
@@ -28,8 +28,6 @@
 #include <common.h>
 #include <netdev.h>
 #include <asm/io.h>
-#include <asm/arch/gpio.h>
-#include "nhk8815-devices.h"
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -45,7 +43,7 @@ void show_boot_progress(int progress)
  */
 int board_init(void)
 {
-	gd->bd->bi_arch_number = MACH_TYPE_NOMADIK;
+	gd->bd->bi_arch_number = MACH_TYPE_USB_S8815;
 	gd->bd->bi_boot_params = 0x00000100;
 	writel(0xC37800F0, NOMADIK_GPIO1_BASE + 0x20);
 	writel(0x00000000, NOMADIK_GPIO1_BASE + 0x24);
@@ -56,32 +54,8 @@ int board_init(void)
 	writel(0x0000305b, REG_FSMC_BCR1);
 	writel(0x00033f33, REG_FSMC_BTR1);
 
-	/* Set up SMCS0 for OneNand: sram-like once again */
-	writel(0x000030db, NOMADIK_FSMC_BASE + 0x00); /* FSMC_BCR0 */
-	writel(0x02100551, NOMADIK_FSMC_BASE + 0x04); /* FSMC_BTR0 */
-
 	icache_enable();
 
-	/*
-	 * Configure I2C pins, as we will use I2C in a later commit
-	 */
-
-	/* Set the two I2C gpio lines to be gpio high */
-	nmk_gpio_set(__SCL, 1);	nmk_gpio_set(__SDA, 1);
-	nmk_gpio_dir(__SCL, 1);	nmk_gpio_dir(__SDA, 1);
-	nmk_gpio_af(__SCL, GPIO_GPIO); nmk_gpio_af(__SDA, GPIO_GPIO);
-
-	/* Put the two I2C port expanders out of reset, on GPIO77 and 79 */
-	{
-		int n[2]={77, 79};
-		int i;
-		for (i = 0; i < ARRAY_SIZE(n); i++) {
-			nmk_gpio_af(n[i], GPIO_GPIO);
-			nmk_gpio_dir(n[i], 1);
-			nmk_gpio_set(n[i], 1);
-		}
-	}
-
 	return 0;
 }
 
@@ -107,15 +81,3 @@ int board_eth_init(bd_t *bis)
 }
 #endif
 
-/* Initialization callback, from lib_arm/board.c */
-int board_late_init(void)
-{
-#ifdef CONFIG_NHK8815_KEYPAD
-	nhk8815_keypad_init();
-#endif
-#ifdef CONFIG_LCD
-	nhk8815_backlight_on();
-#endif
-	return 0;
-}
-
diff --git a/include/configs/nhk8815.h b/include/configs/usb-s8815.h
similarity index 61%
copy from include/configs/nhk8815.h
copy to include/configs/usb-s8815.h
index bbeea91..83df9e7 100644
--- a/include/configs/nhk8815.h
+++ b/include/configs/usb-s8815.h
@@ -1,8 +1,8 @@
 /*
  * (C) Copyright 2005
  * STMicroelectronics.
- * Configuration settings for the "Nomadik Hardware Kit" NHK-8815,
- * the evaluation board for the Nomadik 8815 System on Chip.
+ * Configuration settings for the "USBkey" S8815 by Calao.
+ * The board is similar to the NHK8815 Nomadik Evaluation Board
  *
  * See file CREDITS for list of people who contributed to this
  * project.
@@ -31,7 +31,6 @@
 #define CONFIG_ARM926EJS
 #define CONFIG_NOMADIK
 #define CONFIG_NOMADIK_8815	/* cpu variant */
-#define CONFIG_NOMADIK_NHK8815	/* board variant */
 
 #define CONFIG_SKIP_LOWLEVEL_INIT /* we have already been loaded to RAM */
 
@@ -48,12 +47,13 @@
 #define CONFIG_SYS_NO_FLASH
 /* There is NAND storage */
 #define CONFIG_NAND_NOMADIK
+#define CONFIG_NAND_NOMADIK_16BIT
 #define CONFIG_CMD_JFFS2
 
 /* user interface */
 #define CONFIG_SYS_LONGHELP
 #define CONFIG_SYS_HUSH_PARSER
-#define CONFIG_SYS_PROMPT		"Nomadik> "
+#define CONFIG_SYS_PROMPT		"S8815> "
 #define CONFIG_SYS_PROMPT_HUSH_PS2	"> "
 #define CONFIG_CMDLINE_EDITING
 #define CONFIG_SYS_CBSIZE		256	/* Console I/O Buffer Size */
@@ -88,7 +88,7 @@
 #endif
 
 #define CONFIG_SYS_MEMTEST_START	0x00000000
-#define CONFIG_SYS_MEMTEST_END		0x0FFFFFFF
+#define CONFIG_SYS_MEMTEST_END		0x03b00000 /* we live at 0x3c0.0000 */
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + 256 * 1024)
 #define CONFIG_SYS_GBL_DATA_SIZE	128	/* for initial data */
 
@@ -109,30 +109,6 @@
 #define CONFIG_PL01x_PORTS	{ (void *)CFG_SERIAL0, (void *)CFG_SERIAL1 }
 #define CONFIG_PL011_CLOCK	48000000
 
-/* i2c, for the stmpe2401 port extenders (uses gpio.c in board directory) */
-#ifndef __ASSEMBLY__
-#include <asm/arch/gpio.h>
-#define CONFIG_CMD_I2C
-#define CONFIG_SOFT_I2C
-#define CONFIG_SYS_I2C_SPEED	400000
-#define __SDA			63
-#define __SCL			62
-#define I2C_SDA(x)		nmk_gpio_set(__SDA, x)
-#define I2C_SCL(x)		nmk_gpio_set(__SCL, x)
-#define I2C_READ		(nmk_gpio_get(__SDA)!=0)
-#define I2C_ACTIVE		nmk_gpio_dir(__SDA, 1)
-#define I2C_TRISTATE		nmk_gpio_dir(__SDA, 0)
-#define I2C_DELAY     (udelay(2))
-#endif /* __ASSEMBLY__ */
-
-/* Activate port extenders and define their i2c address */
-#define CONFIG_STMPE2401
-#define STMPE0  0x43
-#define STMPE1  0x44
-
-/* Keypad using stmpe2401 */
-#define CONFIG_NHK8815_KEYPAD
-
 /* Display support */
 #define CONFIG_LCD
 #define CONFIG_LCD_LOGO
@@ -160,59 +136,25 @@
 #define CONFIG_TFTP_BLOCKSIZE	4096
 #define CONFIG_NFS_READ_SIZE	4096
 
-/* Storage information: onenand and nand */
-#define CONFIG_CMD_ONENAND
-#define CONFIG_MTD_ONENAND_VERIFY_WRITE
-#define CONFIG_SYS_ONENAND_BASE		0x30000000
-
+/* Storage information: internal nand */
 #define CONFIG_CMD_NAND
 #define CONFIG_SYS_MAX_NAND_DEVICE	1
 #define CONFIG_SYS_NAND_BASE		0x40000000 /* SMPS0n */
 
-/*
- * Filesystem information
- *
- * Since U-Boot has been loaded to RAM by vendor code, we could use
- * either or both OneNand and Nand. However, we need to know where the
- * filesystem lives. Comments below report vendor-selected partitions
+/* Partition				Size	Start
+ * XloaderTOC + X-Loader		256KB	0x00000000
+ * Memory init function		256KB	0x00040000
+ * U-Boot + env			2MB	0x00080000
+ * Kernel Image			3MB	0x00280000
+ * JFFS2 Root filesystem		22MB	0x00580000
+ * JFFS2 User Data			100.5MB	0x01b80000
  */
-#ifdef CONFIG_BOOT_ONENAND
-   /* Partition				Size	Start
-    * XloaderTOC + X-Loader		256KB	0x00000000
-    * Memory init function		256KB	0x00040000
-    * U-Boot + env			2MB	0x00080000
-    * Sysimage (kernel + ramdisk)	4MB	0x00280000
-    * JFFS2 Root filesystem		22MB	0x00680000
-    * JFFS2 User Data			227.5MB	0x01C80000
-    */
-#   define CONFIG_JFFS2_DEV		"onenand0"
-#   define CONFIG_JFFS2_PART_SIZE	0x01600000
-#   define CONFIG_JFFS2_PART_OFFSET	0x00680000
-#   define CONFIG_ENV_IS_IN_ONENAND
-#   define CONFIG_ENV_SIZE		0x20000 /* 128 Kb - one sector */
-#   define CONFIG_ENV_ADDR		(0x00280000 - CONFIG_ENV_SIZE)
-
-#else /*  BOOT_NAND */
-   /* Partition				Size	Start
-    * XloaderTOC + X-Loader		256KB	0x00000000
-    * Memory init function		256KB	0x00040000
-    * U-Boot + env			2MB	0x00080000
-    * Kernel Image			3MB	0x00280000
-    * JFFS2 Root filesystem		22MB	0x00580000
-    * JFFS2 User Data			100.5MB	0x01b80000
-    */
-#   define CONFIG_JFFS2_DEV		"nand0"
-#   define CONFIG_JFFS2_NAND		1 /* For the jffs2 support*/
-#   define CONFIG_JFFS2_PART_SIZE	0x01600000
-#   define CONFIG_JFFS2_PART_OFFSET	0x00580000
-#   define CONFIG_ENV_IS_IN_NAND
-#   define CONFIG_ENV_SIZE		0x20000 /* 128 Kb - one sector */
-#   define CONFIG_ENV_OFFSET		(0x00280000 - CONFIG_ENV_SIZE)
-
-#endif /* CONFIG_BOOT_ONENAND */
-
-/* this is needed to make hello_world.c and other stuff happy */
-#define CONFIG_SYS_MAX_FLASH_SECT	512
-#define CONFIG_SYS_MAX_FLASH_BANKS	1
+#define CONFIG_JFFS2_DEV		"nand0"
+#define CONFIG_JFFS2_NAND		1 /* For the jffs2 support*/
+#define CONFIG_JFFS2_PART_SIZE	0x01600000
+#define CONFIG_JFFS2_PART_OFFSET	0x00580000
+#define CONFIG_ENV_IS_IN_NAND
+#define CONFIG_ENV_SIZE		0x20000 /* 128 Kb - one sector */
+#define CONFIG_ENV_OFFSET		(0x00280000 - CONFIG_ENV_SIZE)
 
 #endif /* __CONFIG_H */
-- 
1.6.0.2

[U-Boot] [PATCH 1/2] NAND Nomadik: add CONFIG_NAND_NOMADIK_16BIT option

[Wolfgang Denk]

Dear Alessandro Rubini,

In message <a14c1bde15838a59c60e177a43a4ebd35f28adea.1263851321.git.rubini@unipv.it> you wrote:
> 
> Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
> Cc: Gregory Hermant <gregory.hermant@calao-systems.com>
> ---
>  drivers/mtd/nand/nomadik.c |   15 +++++++++++----
>  1 files changed, 11 insertions(+), 4 deletions(-)

New config options should be documented. If not in the REDME, so at
last in the commit message. You may know what
CONFIG_NAND_NOMADIK_16BIT is supposed to mean, others may not.

Also, I suggest to change the name into CONFIG_NOMADIK_NAND_16BIT
which seems more logical to me.

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
"There is no statute of limitations on stupidity."
- Randomly produced by a computer program called Markov3.

[U-Boot] [PATCH 1/2] NAND Nomadik: add CONFIG_NAND_NOMADIK_16BIT option

[Scott Wood]

Wolfgang Denk wrote:
> Dear Alessandro Rubini,
> 
> In message <a14c1bde15838a59c60e177a43a4ebd35f28adea.1263851321.git.rubini@unipv.it> you wrote:
>> Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
>> Cc: Gregory Hermant <gregory.hermant@calao-systems.com>
>> ---
>>  drivers/mtd/nand/nomadik.c |   15 +++++++++++----
>>  1 files changed, 11 insertions(+), 4 deletions(-)
> 
> New config options should be documented. If not in the REDME, so at
> last in the commit message. You may know what
> CONFIG_NAND_NOMADIK_16BIT is supposed to mean, others may not.
> 
> Also, I suggest to change the name into CONFIG_NOMADIK_NAND_16BIT
> which seems more logical to me.

It's an attribute of the hardware, so it should be 
CONFIG_SYS_NOMADIK_NAND_16BIT.

-Scott

[U-Boot] [PATCH 2/2] Add Nomadik board "usb-s8815" by Calao vendor

[Wolfgang Denk]

Dear Alessandro Rubini,

In message <f34ac6eeba7c2771468e8f7b08d1cdc9bdb04fe6.1263851321.git.rubini@unipv.it> you wrote:
> Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
> Cc: Gregory Hermant <gregory.hermant@calao-systems.com>
> ---
>  MAINTAINERS                                        |    4 +
>  Makefile                                           |    4 +
>  board/{st/nhk8815 => calao/usb-s8815}/Makefile     |    3 +-
>  board/{st/nhk8815 => calao/usb-s8815}/config.mk    |    0
>  board/{st/nhk8815 => calao/usb-s8815}/lcd.c        |   34 +------
>  board/{st/nhk8815 => calao/usb-s8815}/platform.S   |    0
>  .../nhk8815.c => calao/usb-s8815/usb-s8815.c}      |   40 +--------
>  include/configs/{nhk8815.h => usb-s8815.h}         |   98 ++++----------------
>  8 files changed, 36 insertions(+), 147 deletions(-)
>  copy board/{st/nhk8815 => calao/usb-s8815}/Makefile (96%)
>  copy board/{st/nhk8815 => calao/usb-s8815}/config.mk (100%)
>  copy board/{st/nhk8815 => calao/usb-s8815}/lcd.c (68%)
>  copy board/{st/nhk8815 => calao/usb-s8815}/platform.S (100%)
>  copy board/{st/nhk8815/nhk8815.c => calao/usb-s8815/usb-s8815.c} (67%)
>  copy include/configs/{nhk8815.h => usb-s8815.h} (61%)

I don't like to see such heavy copying of code.  This is a clear
indication that we should factor out the common parts ; Actually
differences are so small that common code can be used completely, and
no separate board directories are needed.

Note that many of the existing diffrences can be avoided easily and
without need for #ifdef's:

> index d3acb48..744da67 100644
> --- a/board/st/nhk8815/lcd.c
> +++ b/board/calao/usb-s8815/lcd.c
> @@ -1,5 +1,5 @@
>  /*
> - * board/st/nhk8815/lcd.c: use amba clcd and STMPE2401 for backlight/reset
> + * board/calao/usb-s8815/lcd.c: use amba clcd and STMPE2401 for backlight/reset

No need to include the file name in the comment.

>  /* Horribly, these are precomputed registers */
> -struct clcd_config nhk8815_clcd_config = {
> +struct clcd_config s8815_clcd_config = {

Use a common name, then this...

>  	.address =	(struct clcd_registers *)NOMADIK_CLCDC_BASE,
>  	.tim0 =		0xd52600c4,	/* horizontal timings */
>  	.tim1 =		0x220a01df,	/* vertical timings */
> @@ -53,36 +53,14 @@ vidinfo_t panel_info = {
>  	.vl_col =	800,
>  	.vl_row =	480,
>  	.vl_bpix =	CLCD_BPIX_VAL,
> -	.priv =		&nhk8815_clcd_config,
> +	.priv =		&s8815_clcd_config,

...and this diff will go away.

> -	gd->bd->bi_arch_number = MACH_TYPE_NOMADIK;
> +	gd->bd->bi_arch_number = MACH_TYPE_USB_S8815;

Set the current MACH_TYPE in the board config file, and use this
setting here.

etc. etc.


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

[U-Boot] [PATCH 2/2] Add Nomadik board "usb-s8815" by Calao vendor

[Alessandro Rubini]

[I'm re-adding Gregory Hermant in Cc:, while my message had the Cc:
explicit, the copy I got from the list and your reply had no such header]

> I don't like to see such heavy copying of code.

I understand your point of view (although I didn't use
--find-copies-harder you probably wouldn't have noticed).

However, these are really different boards from different vendors.
The Calao one is a stripped-down version of the development system,
not uncommon to what happens elsewhere.

Where do you suggest to place common code?

While it is true that, currently, platform.S can be placed in common
nomadik code, the file could as well be cleaned up of unneeded or
redundant stuff, which is different in the two boards. (The file is
something I got from the vendor and would benefit from some cleanup,
now that I know more of the various register sets in the SoC).

> Note that many of the existing diffrences can be avoided easily and
> without need for #ifdef's:

You are right. But one LCD needs a turn-on-backlight method, while the
other does not.  If I place it on cpu/arm926ejs/nomadik/ we'll have
problems when another board mounts a different display.

I'll try to factorize as I can, but I'm not sure it's the right thing
to do. Will respin the patch by tomorrow night.

/alessandro

[U-Boot] [PATCH 1/2] NAND Nomadik: add CONFIG_NAND_NOMADIK_16BIT option

[Alessandro Rubini]

Wolfgang Denk:
>> Also, I suggest to change the name into CONFIG_NOMADIK_NAND_16BIT
>> which seems more logical to me.

Scott Wood:
> It's an attribute of the hardware, so it should be 
> CONFIG_SYS_NOMADIK_NAND_16BIT.

Actually, I would love to have this option passed from the board,
to avoid an ifdef in the SoC driver, which actually can handle both.
I haven't found how to do that. Any suggestion is welcome, otherwise
I'll use the name suggested by Scott Wood.

/alessandro

[U-Boot] [PATCH 2/2] Add Nomadik board "usb-s8815" by Calao vendor

[Alessandro Rubini]

Wolfgang Denk:
> I don't like to see such heavy copying of code.  This is a clear
> indication that we should factor out the common parts

While I disagree (as explained), I'm trying hard to do
it. Unfortunately what is currently a Makefile decision (COBJS-y and
such) will sometimes become ifdef in the code.

However, i've moved platform.S and lcd.c to the cpu directory, so they
can be shared (the assembly really ought to be personalized for each
board, as some cruft is still there from past history, and the lcd.c
is really two small data structures, but let's ignore this by now).

Now, reading better your message I see this that I don't understand.

> Actually differences are so small that common code can be used
> completely, and no separate board directories are needed.

On one side we are asked to use the vendor directories, and now I
should merge vendor/st/nhk8815 and vendor/calao/usb-8815.  Now,
I wonder where am I expected to place che board files. In
cpu/arm926ejs/nomadik ?  And using #ifdef in a single file to sort out
differences?

I got the idea a board should be a separate object (directory), which
bases on the available infrastructure to pick up pieces and define own
parameters.  Adding a board should be as easy as adding a directory.

Such parameters sometimes happen to match other parameters, but
coalescing two boards just to save a few lines is not an advantage, in
my opinion.

Could you please explain how to proceed to have both boards upstream in
the best possible way? I currently don't understand the policy.

Please forgive me if the message looks aggressive. It's not meant to
be, but ascii doesn't proprely convey tone.

/alessandro