[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Tom Rini]

This introduces 200MHz Micron parts timing information based on x-loader
and re-organizes the file slightly for grouping.  The memory init logic
is also based on what x-loader does in these cases.  Note that while
previously u-boot would be flashed in with SW ECC in this case it now
must be flashed with HW ECC.

Beagleboard rev C5, xM rev A:
Tested-by: Tom Rini <trini@ti.com>
Beagleboard xM rev C:
Tested-by: Matt Ranostay <mranostay@gmail.com>
Beagleboard rev B7, C2, xM rev B:
Tested-by: Matt Porter <mporter@ti.com>
Signed-off-by: Tom Rini <trini@ti.com>
---
 arch/arm/include/asm/arch-omap3/mem.h |   24 +++++
 board/ti/beagle/beagle.c              |  160 ++++++++++++++++++++++++++++++++-
 board/ti/beagle/config.mk             |   33 -------
 include/configs/omap3_beagle.h        |   60 ++++++++++++-
 4 files changed, 242 insertions(+), 35 deletions(-)
 delete mode 100644 board/ti/beagle/config.mk

diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
index af3504c..a784813 100644
--- a/arch/arm/include/asm/arch-omap3/mem.h
+++ b/arch/arm/include/asm/arch-omap3/mem.h
@@ -171,6 +171,30 @@ enum {
 #define MICRON_V_MR ((MICRON_WBST << 9) | (MICRON_CASL << 4) | \
 	(MICRON_SIL << 3) | (MICRON_BL))
 
+
+/* Micron part (200MHz optimized) 5 ns
+  */
+#define MICRON_TDAL_200   6
+#define MICRON_TDPL_200   3
+#define MICRON_TRRD_200   2
+#define MICRON_TRCD_200   3
+#define MICRON_TRP_200    3
+#define MICRON_TRAS_200   8
+#define MICRON_TRC_200   11
+#define MICRON_TRFC_200  15
+#define MICRON_V_ACTIMA_200 ((MICRON_TRFC_200 << 27) | (MICRON_TRC_200 << 22) | (MICRON_TRAS_200 << 18) \
+		| (MICRON_TRP_200 << 15) | (MICRON_TRCD_200 << 12) |(MICRON_TRRD_200 << 9) | \
+		(MICRON_TDPL_200 << 6) | (MICRON_TDAL_200))
+
+#define MICRON_TWTR_200   2
+#define MICRON_TCKE_200   4
+#define MICRON_TXP_200    2
+#define MICRON_XSR_200   23
+#define MICRON_V_ACTIMB_200 ((MICRON_TCKE_200 << 12) | (MICRON_XSR_200 << 0)) | \
+				(MICRON_TXP_200 << 8) | (MICRON_TWTR_200 << 16)
+#define MICRON_ARCV_200		0x5e6
+#define MICRON_V_RFR_CTRL_200	((MICRON_ARCV_200 << 8) | (MICRON_ARE))
+
 /*
  * NUMONYX part of IGEP v2 (165MHz optimized) 6.06ns
  *   ACTIMA
diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
index aa5047c..a77b0cb 100644
--- a/board/ti/beagle/beagle.c
+++ b/board/ti/beagle/beagle.c
@@ -34,9 +34,11 @@
 #include <status_led.h>
 #endif
 #include <twl4030.h>
+#include <linux/mtd/nand.h>
 #include <asm/io.h>
 #include <asm/arch/mmc_host_def.h>
 #include <asm/arch/mux.h>
+#include <asm/arch/mem.h>
 #include <asm/arch/sys_proto.h>
 #include <asm/gpio.h>
 #include <asm/mach-types.h>
@@ -139,6 +141,160 @@ int get_board_revision(void)
 	return revision;
 }
 
+#ifdef CONFIG_SPL_BUILD
+
+#define MICRON_DDR	0
+#define NUMONYX_MCP	1
+#define MICRON_MCP	2
+
+#define NAND_CMD_STATUS		0x70
+#define NAND_CMD_READID		0x90
+#define NAND_CMD_RESET		0xff
+
+#define GPMC_NAND_COMMAND_0      (OMAP34XX_GPMC_BASE+0x7C)
+#define GPMC_NAND_ADDRESS_0      (OMAP34XX_GPMC_BASE+0x80)
+#define GPMC_NAND_DATA_0	 (OMAP34XX_GPMC_BASE+0x84)
+
+#define WRITE_NAND_COMMAND(d, adr) \
+	do {*(volatile u16 *)GPMC_NAND_COMMAND_0 = d;} while(0)
+#define WRITE_NAND_ADDRESS(d, adr) \
+	do {*(volatile u16 *)GPMC_NAND_ADDRESS_0 = d;} while(0)
+#define READ_NAND(adr)          (*(volatile u16 *)GPMC_NAND_DATA_0)
+
+/* nand_command: Send a flash command to the flash chip */
+static void nand_command(unsigned char command)
+{
+ 	WRITE_NAND_COMMAND(command, NAND_ADDR);
+
+  	if (command == NAND_CMD_RESET) {
+		unsigned char ret_val;
+		nand_command(NAND_CMD_STATUS);
+		do {
+			ret_val = READ_NAND(NAND_ADDR);/* wait till ready */
+  		} while ((ret_val & 0x40) != 0x40);
+ 	}
+}
+
+/* 
+ * In order to find out what DDR we have we need to see what NAND we
+ * may have.  This relies on having already initalized GPMC earlier
+ * in the sequence.
+ */
+static void nand_readid(int *mfr, int *id)
+{
+ 	nand_command(NAND_CMD_RESET);
+ 	nand_command(NAND_CMD_READID);
+
+	WRITE_NAND_ADDRESS(0x0, NAND_ADDR);
+
+	/* Read off the manufacturer and device id. */
+	*mfr = READ_NAND(NAND_ADDR);
+	*id = READ_NAND(NAND_ADDR);
+}
+
+#define GPMC_CONFIG_CS0_CONFIG1		0x6E000060
+#define GPMC_CONFIG_CS0_CONFIG2		0x6E000064
+#define GPMC_CONFIG_CS0_CONFIG3		0x6E000068
+#define GPMC_CONFIG_CS0_CONFIG4		0x6E00006C
+#define GPMC_CONFIG_CS0_CONFIG5		0x6E000070
+#define GPMC_CONFIG_CS0_CONFIG6		0x6E000074
+#define GPMC_CONFIG_CS0_CONFIG7		0x6E000078
+#define OMAP34XX_GPMC_CS0_SIZE		0x8
+
+static int identify_xm_ddr(void)
+{
+	int mfr, id;
+
+	/* Make sure that we have setup GPMC for NAND correctly. */
+	writel(M_NAND_GPMC_CONFIG1, GPMC_CONFIG_CS0_CONFIG1);
+	writel(M_NAND_GPMC_CONFIG2, GPMC_CONFIG_CS0_CONFIG2);
+	writel(M_NAND_GPMC_CONFIG3, GPMC_CONFIG_CS0_CONFIG3);
+	writel(M_NAND_GPMC_CONFIG4, GPMC_CONFIG_CS0_CONFIG4);
+	writel(M_NAND_GPMC_CONFIG5, GPMC_CONFIG_CS0_CONFIG5);
+	writel(M_NAND_GPMC_CONFIG6, GPMC_CONFIG_CS0_CONFIG6);
+
+	/* Enable the GPMC Mapping */
+	writel((((OMAP34XX_GPMC_CS0_SIZE & 0xF) << 8) |
+			     ((NAND_BASE >> 24) & 0x3F) |
+			     (1 << 6)),  (GPMC_CONFIG_CS0_CONFIG7));
+
+	sdelay(2000);
+
+	nand_readid(&mfr, &id);
+	if (mfr == 0)
+		return MICRON_DDR;
+	if ((mfr == 0x20) && (id == 0xba))
+		return NUMONYX_MCP;
+	if ((mfr == 0x2c) && (id == 0xbc))
+		return MICRON_MCP;
+
+	/* Unknown. */
+	return -1;
+}
+
+/* 
+ * Routine: board_early_sdrc_init
+ * Description: If we use SPL then there is no x-loader nor config header
+ * so we have to setup the DDR timings outself on both banks.
+ */
+void get_board_mem_timings(u32 *cs_cfg, u32 *mcfg, u32 *ctrla, u32 *ctrlb,
+		u32 *rfr_ctrl, u32 *mr)
+{
+	/* TODO XXX REWORD */
+	/* We have magic hard coded values here for V_MCFG which come from
+	 * x-loader as they do not match how the OMAP35x TRM says to
+	 * calculate them values. */
+	*mr = MICRON_V_MR;
+	switch (get_board_revision()) {
+	case REVISION_C4:
+		if (identify_xm_ddr() == NUMONYX_MCP) {
+			*cs_cfg = 0x4;
+			*mcfg = 0x04590099;
+			*ctrla = NUMONYX_V_ACTIMA_165;
+			*ctrlb = NUMONYX_V_ACTIMB_165;
+			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+		} else if (identify_xm_ddr() == MICRON_MCP) {
+			/* Beagleboard Rev C5 */
+			*cs_cfg = 0x2;
+			*mcfg = 0x03588099;
+			*ctrla = MICRON_V_ACTIMA_200;
+			*ctrlb = MICRON_V_ACTIMB_200;
+			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_200MHz;
+		} else {
+			*cs_cfg = 0x1;
+			*mcfg = 0x02584099;
+			*ctrla = MICRON_V_ACTIMA_165;
+			*ctrlb = MICRON_V_ACTIMB_165;
+			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+		}
+		break;
+	case REVISION_XM_A:
+	case REVISION_XM_B:
+	case REVISION_XM_C:
+		if (identify_xm_ddr() == MICRON_DDR) {
+			*cs_cfg = 0x2;
+			*mcfg = 0x03588099;
+			*ctrla = MICRON_V_ACTIMA_200;
+			*ctrlb = MICRON_V_ACTIMB_200;
+			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_200MHz;
+		} else {
+			*cs_cfg = 0x4;
+			*mcfg = 0x04590099;
+			*ctrla = NUMONYX_V_ACTIMA_165;
+			*ctrlb = NUMONYX_V_ACTIMB_165;
+			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+		}
+		break;
+	default:
+		*cs_cfg = 0x1;
+		*mcfg = 0x02584099;
+		*ctrla = MICRON_V_ACTIMA_165;
+		*ctrlb = MICRON_V_ACTIMB_165;
+		*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+	}
+}
+#endif
+
 /*
  * Routine: get_expansion_id
  * Description: This function checks for expansion board by checking I2C
@@ -371,7 +527,7 @@ void set_muxconf_regs(void)
 	MUX_BEAGLE();
 }
 
-#ifdef CONFIG_GENERIC_MMC
+#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD)
 int board_mmc_init(bd_t *bis)
 {
 	omap_mmc_init(0);
@@ -480,6 +636,7 @@ int ehci_hcd_init(void)
 
 #endif /* CONFIG_USB_EHCI */
 
+#ifndef CONFIG_SPL_BUILD
 /*
  * This command returns the status of the user button on beagle xM
  * Input - none
@@ -534,3 +691,4 @@ U_BOOT_CMD(
 	"Return the status of the BeagleBoard USER button",
 	""
 );
+#endif
diff --git a/board/ti/beagle/config.mk b/board/ti/beagle/config.mk
deleted file mode 100644
index cf055db..0000000
--- a/board/ti/beagle/config.mk
+++ /dev/null
@@ -1,33 +0,0 @@
-#
-# (C) Copyright 2006
-# Texas Instruments, <www.ti.com>
-#
-# Beagle Board uses OMAP3 (ARM-CortexA8) cpu
-# see http://www.ti.com/ for more information on Texas Instruments
-#
-# See file CREDITS for list of people who contributed to this
-# project.
-#
-# 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
-#
-# Physical Address:
-# 8000'0000 (bank0)
-# A000/0000 (bank1)
-# Linux-Kernel is expected to be at 8000'8000, entry 8000'8000
-# (mem base + reserved)
-
-# For use with external or internal boots.
-CONFIG_SYS_TEXT_BASE = 0x80008000
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index a026478..a126566 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -347,7 +347,7 @@
  */
 #define CONFIG_NR_DRAM_BANKS	2	/* CS1 may or may not be populated */
 #define PHYS_SDRAM_1		OMAP34XX_SDRC_CS0
-#define PHYS_SDRAM_1_SIZE	(32 << 20)	/* at least 32 MiB */
+#define PHYS_SDRAM_1_SIZE	(32 << 20)	/* At least 32 MiB */
 #define PHYS_SDRAM_2		OMAP34XX_SDRC_CS1
 
 /* SDRAM Bank Allocation method */
@@ -390,4 +390,62 @@
 
 #define CONFIG_OMAP3_SPI
 
+/* Defines for SPL */
+#define CONFIG_SPL
+#define CONFIG_SPL_NAND_SIMPLE
+#define CONFIG_SPL_TEXT_BASE		0x40200800
+#define CONFIG_SPL_MAX_SIZE		(45 * 1024)
+#define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
+
+#define CONFIG_SPL_BSS_START_ADDR	0x80000000
+#define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */
+
+#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR	0x300 /* address 0x60000 */
+#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS	0x200 /* 256 KB */
+#define CONFIG_SYS_MMC_SD_FAT_BOOT_PARTITION	1
+#define CONFIG_SPL_FAT_LOAD_PAYLOAD_NAME	"u-boot.img"
+
+#define CONFIG_SPL_LIBCOMMON_SUPPORT
+#define CONFIG_SPL_LIBDISK_SUPPORT
+#define CONFIG_SPL_I2C_SUPPORT
+#define CONFIG_SPL_LIBGENERIC_SUPPORT
+#define CONFIG_SPL_MMC_SUPPORT
+#define CONFIG_SPL_FAT_SUPPORT
+#define CONFIG_SPL_SERIAL_SUPPORT
+#define CONFIG_SPL_NAND_SUPPORT
+#define CONFIG_SPL_POWER_SUPPORT
+#define CONFIG_SPL_LDSCRIPT		"$(CPUDIR)/omap-common/u-boot-spl.lds"
+
+/* NAND boot config */
+#define CONFIG_SYS_NAND_5_ADDR_CYCLE
+#define CONFIG_SYS_NAND_PAGE_COUNT	64
+#define CONFIG_SYS_NAND_PAGE_SIZE	2048
+#define CONFIG_SYS_NAND_OOBSIZE		64
+#define CONFIG_SYS_NAND_BLOCK_SIZE	(128*1024)
+#define CONFIG_SYS_NAND_BAD_BLOCK_POS	0
+#define CONFIG_SYS_NAND_ECCPOS		{2, 3, 4, 5, 6, 7, 8, 9,\
+						10, 11, 12, 13}
+
+#define CONFIG_SYS_NAND_ECCSIZE		512
+#define CONFIG_SYS_NAND_ECCBYTES	3
+
+#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
+						CONFIG_SYS_NAND_ECCSIZE)
+#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
+						CONFIG_SYS_NAND_ECCSTEPS)
+
+#define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
+
+#define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
+
+/*
+ * 1MB into the SDRAM to allow for SPL's bss@the beginning of SDRAM
+ * 64 bytes before this address should be set aside for u-boot.img's
+ * header. That is 0x800FFFC0--0x80100000 should not be used for any
+ * other needs.
+ */
+#define CONFIG_SYS_TEXT_BASE		0x80100000
+#define CONFIG_SYS_SPL_MALLOC_START	0x80208000
+#define CONFIG_SYS_SPL_MALLOC_SIZE	0x100000
+
 #endif /* __CONFIG_H */
-- 
1.7.0.4

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Tom Rini]

On Wed, Oct 26, 2011 at 1:50 PM, Tom Rini <trini@ti.com> wrote:
> This introduces 200MHz Micron parts timing information based on x-loader
> and re-organizes the file slightly for grouping. ?The memory init logic
> is also based on what x-loader does in these cases. ?Note that while
> previously u-boot would be flashed in with SW ECC in this case it now
> must be flashed with HW ECC.

Arg, wasn't quite ready to post this, cursing git send-email and sending the
whole series + explanation shortly.

-- 
Tom

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Tom Rini]

On Thu, Oct 27, 2011 at 4:22 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 10/27/2011 06:13 PM, Tom Rini wrote:
>> On 10/27/2011 04:10 PM, Igor Grinberg wrote:
>>> On 10/27/2011 11:29 PM, Tom Rini wrote:
>>>> On 10/27/2011 02:18 PM, Igor Grinberg wrote:
>>>>> On 10/26/2011 11:13 PM, Tom Rini wrote:
>> [snip]
>>>>>> +/* nand_command: Send a flash command to the flash chip */
>>>>>> +static void nand_command(unsigned char command)
>>>>>> +{
>>>>>> + ? ? ? ? WRITE_NAND_COMMAND(command, NAND_ADDR);
>>>>>> +
>>>>>> + ? ? ? ? if (command == NAND_CMD_RESET) {
>>>>>> + ? ? ? ? unsigned char ret_val;
>>>>>> + ? ? ? ? nand_command(NAND_CMD_STATUS);
>>>>>> + ? ? ? ? do {
>>>>>> + ? ? ? ? ? ? ? ? ret_val = READ_NAND(NAND_ADDR);/* wait till ready */
>>>>>> + ? ? ? ? ? ? ? ? } while ((ret_val & 0x40) != 0x40);
>>>>> You should be using some kind of timeout, so you will not stuck in here
>>>>> without being noticed.
>>>> OK. ?I've been wondering if we shouldn't somehow make a
>>>> not-tied-to-full-mtd nand_command more available since I suspect a few
>>>> other boards will be in a similar situation, for probing early on.
>>>
>>> That would be much better solution.
>>
>> (Background if you've otherwise ignored the thread, on some omap3 boards
>> we need to take a whack at NAND to see what DDR we have)
>>
>> OK, so, Scott, where would you suggest we put a very trivial set of nand
>> infrastructure we need outside of the scope of loading u-boot itself
>> from NAND ?
>
> Can the SPL code be used (not an actual SPL build, just the NAND code
> associated with it)?

So, as I start down this path, nand_command needs nand_init() done and
that means we need the chip driver so at this point I'm wondering if
it's not better to just say that boards that need to poke NAND to
determine memory just need CONFIG_SPL_NAND_SIMPLE set and un-static
nand_command that we've got already (and make boards call nand_init()
before nand_command()), comments?

-- 
Tom

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Tom Rini]

On Thu, Oct 27, 2011 at 2:18 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> On 10/26/2011 11:13 PM, Tom Rini wrote:
[snip]
>> ?board/ti/beagle/config.mk ? ? ? ? ? ? | ? 33 -------
[snip]
> config.mk removal does not belong to that patch...
> It should be a separate one, say cleanup patch.

I can do this in its own patches but note that what's going on is
we're changing from CONFIG_SYS_TEXT_BASE at 0x80008000 to 0x80100000
and putting it into the header file.  That's why I didn't split it in
two to start with.

-- 
Tom

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Tom Rini]

On 10/28/2011 09:42 AM, Scott Wood wrote:
> On 10/28/2011 11:29 AM, Tom Rini wrote:
>> On 10/28/2011 09:00 AM, Scott Wood wrote:
>>> On 10/27/2011 06:33 PM, Tom Rini wrote:
>>>> On 10/27/2011 04:22 PM, Scott Wood wrote:
>>>>> On 10/27/2011 06:13 PM, Tom Rini wrote:
>>>>>> OK, so, Scott, where would you suggest we put a very trivial set of nand
>>>>>> infrastructure we need outside of the scope of loading u-boot itself
>>>>>> from NAND ?
>>>>>
>>>>> Can the SPL code be used (not an actual SPL build, just the NAND code
>>>>> associated with it)?
>>>>
>>>> As-is, no, it'll fail to compile unless you fill in a number of blanks
>>>> about ECC and so forth.
>>>
>>> "...with appropriate refactoring" was implied. :-)
>>>
>>>> If we split nand_spl_simple.c in two, we might
>>>> be able to re-use nand_command there (maybe with some re-thinking of the
>>>> #if logic for big or small pages).  Give me filenames and I'll do the
>>>> work :)
>>>
>>> Pick whatever name you think makes sense given what's going to be in it.
>>
>> nand_spl_{cmd,read}.c ?  _cmd gets the two nand_command() funcs, _read
>> gets everything else.  Work for you?
> 
> Could you stick a "simple" in there somewhere?
> 
> Maybe nand_spl_simple.c and nand_cmd_simple.c (or nand_spl_simple_cmd.c
> if you want to keep the reference to spl as its primary purpose)?

nand_spl_simple.c + nand_spl_simple_cmd.c it is then, thanks.  Putting
this on my v2012.03 TODO list (which is when I figure I should have this
all done and a merge window be available).

-- 
Tom

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Scott Wood]

On 10/28/2011 11:29 AM, Tom Rini wrote:
> On 10/28/2011 09:00 AM, Scott Wood wrote:
>> On 10/27/2011 06:33 PM, Tom Rini wrote:
>>> On 10/27/2011 04:22 PM, Scott Wood wrote:
>>>> On 10/27/2011 06:13 PM, Tom Rini wrote:
>>>>> OK, so, Scott, where would you suggest we put a very trivial set of nand
>>>>> infrastructure we need outside of the scope of loading u-boot itself
>>>>> from NAND ?
>>>>
>>>> Can the SPL code be used (not an actual SPL build, just the NAND code
>>>> associated with it)?
>>>
>>> As-is, no, it'll fail to compile unless you fill in a number of blanks
>>> about ECC and so forth.
>>
>> "...with appropriate refactoring" was implied. :-)
>>
>>> If we split nand_spl_simple.c in two, we might
>>> be able to re-use nand_command there (maybe with some re-thinking of the
>>> #if logic for big or small pages).  Give me filenames and I'll do the
>>> work :)
>>
>> Pick whatever name you think makes sense given what's going to be in it.
> 
> nand_spl_{cmd,read}.c ?  _cmd gets the two nand_command() funcs, _read
> gets everything else.  Work for you?

Could you stick a "simple" in there somewhere?

Maybe nand_spl_simple.c and nand_cmd_simple.c (or nand_spl_simple_cmd.c
if you want to keep the reference to spl as its primary purpose)?

-Scott

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Tom Rini]

On 10/28/2011 09:00 AM, Scott Wood wrote:
> On 10/27/2011 06:33 PM, Tom Rini wrote:
>> On 10/27/2011 04:22 PM, Scott Wood wrote:
>>> On 10/27/2011 06:13 PM, Tom Rini wrote:
>>>> OK, so, Scott, where would you suggest we put a very trivial set of nand
>>>> infrastructure we need outside of the scope of loading u-boot itself
>>>> from NAND ?
>>>
>>> Can the SPL code be used (not an actual SPL build, just the NAND code
>>> associated with it)?
>>
>> As-is, no, it'll fail to compile unless you fill in a number of blanks
>> about ECC and so forth.
> 
> "...with appropriate refactoring" was implied. :-)
> 
>> If we split nand_spl_simple.c in two, we might
>> be able to re-use nand_command there (maybe with some re-thinking of the
>> #if logic for big or small pages).  Give me filenames and I'll do the
>> work :)
> 
> Pick whatever name you think makes sense given what's going to be in it.

nand_spl_{cmd,read}.c ?  _cmd gets the two nand_command() funcs, _read
gets everything else.  Work for you?

-- 
Tom

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Scott Wood]

On 10/27/2011 06:33 PM, Tom Rini wrote:
> On 10/27/2011 04:22 PM, Scott Wood wrote:
>> On 10/27/2011 06:13 PM, Tom Rini wrote:
>>> OK, so, Scott, where would you suggest we put a very trivial set of nand
>>> infrastructure we need outside of the scope of loading u-boot itself
>>> from NAND ?
>>
>> Can the SPL code be used (not an actual SPL build, just the NAND code
>> associated with it)?
> 
> As-is, no, it'll fail to compile unless you fill in a number of blanks
> about ECC and so forth.

"...with appropriate refactoring" was implied. :-)

> If we split nand_spl_simple.c in two, we might
> be able to re-use nand_command there (maybe with some re-thinking of the
> #if logic for big or small pages).  Give me filenames and I'll do the
> work :)

Pick whatever name you think makes sense given what's going to be in it.

-Scott

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Igor Grinberg]

On 10/28/2011 01:19 AM, Scott Wood wrote:
> On 10/27/2011 06:02 PM, Igor Grinberg wrote:
>> On 10/27/2011 11:27 PM, Wolfgang Denk wrote:
>>> Dear Igor Grinberg,
>>>
>>> In message <4EA9CA9D.6010209@compulab.co.il> you wrote:
>>>> On 10/26/2011 11:13 PM, Tom Rini wrote:
>>>>> This introduces 200MHz Micron parts timing information based on x-loader
>>>>> and re-organizes the file slightly for grouping.  The memory init logic
>>>>> is also based on what x-loader does in these cases.  Note that while
>>>>> previously u-boot would be flashed in with SW ECC in this case it now
>>>>> must be flashed with HW ECC.
>>>> You have two spaces between the sentences, why is that?
>>> For improved readability.  It should be standard formatting for any
>>> text typed in constant-width fonts.
>> I actually do not see how this improves readability...
> It's a common practice, and for many people is both a habit and more
> visually pleasing.  Nobody's forcing you to do it, and we don't complain
> when other people type sentences that are all squished together. :-)

Alright,  as  I've  already  said,  I  can  live  with  it  ;-).  

>> Is that documented somewhere? I don't think so...
> http://en.wikipedia.org/wiki/Sentence_spacing

Thanks,  Scott,  that  was  entertaining...  

Regards,  
Igor  

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Tom Rini]

On 10/27/2011 04:22 PM, Scott Wood wrote:
> On 10/27/2011 06:13 PM, Tom Rini wrote:
>> On 10/27/2011 04:10 PM, Igor Grinberg wrote:
>>> On 10/27/2011 11:29 PM, Tom Rini wrote:
>>>> On 10/27/2011 02:18 PM, Igor Grinberg wrote:
>>>>> On 10/26/2011 11:13 PM, Tom Rini wrote:
>> [snip]
>>>>>> +/* nand_command: Send a flash command to the flash chip */
>>>>>> +static void nand_command(unsigned char command)
>>>>>> +{
>>>>>> + 	WRITE_NAND_COMMAND(command, NAND_ADDR);
>>>>>> +
>>>>>> +  	if (command == NAND_CMD_RESET) {
>>>>>> +		unsigned char ret_val;
>>>>>> +		nand_command(NAND_CMD_STATUS);
>>>>>> +		do {
>>>>>> +			ret_val = READ_NAND(NAND_ADDR);/* wait till ready */
>>>>>> +  		} while ((ret_val & 0x40) != 0x40);
>>>>> You should be using some kind of timeout, so you will not stuck in here
>>>>> without being noticed.
>>>> OK.  I've been wondering if we shouldn't somehow make a
>>>> not-tied-to-full-mtd nand_command more available since I suspect a few
>>>> other boards will be in a similar situation, for probing early on.
>>>
>>> That would be much better solution.
>>
>> (Background if you've otherwise ignored the thread, on some omap3 boards
>> we need to take a whack at NAND to see what DDR we have)
>>
>> OK, so, Scott, where would you suggest we put a very trivial set of nand
>> infrastructure we need outside of the scope of loading u-boot itself
>> from NAND ?
> 
> Can the SPL code be used (not an actual SPL build, just the NAND code
> associated with it)?

As-is, no, it'll fail to compile unless you fill in a number of blanks
about ECC and so forth.  If we split nand_spl_simple.c in two, we might
be able to re-use nand_command there (maybe with some re-thinking of the
#if logic for big or small pages).  Give me filenames and I'll do the
work :)

-- 
Tom

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Scott Wood]

On 10/27/2011 06:13 PM, Tom Rini wrote:
> On 10/27/2011 04:10 PM, Igor Grinberg wrote:
>> On 10/27/2011 11:29 PM, Tom Rini wrote:
>>> On 10/27/2011 02:18 PM, Igor Grinberg wrote:
>>>> On 10/26/2011 11:13 PM, Tom Rini wrote:
> [snip]
>>>>> +/* nand_command: Send a flash command to the flash chip */
>>>>> +static void nand_command(unsigned char command)
>>>>> +{
>>>>> + 	WRITE_NAND_COMMAND(command, NAND_ADDR);
>>>>> +
>>>>> +  	if (command == NAND_CMD_RESET) {
>>>>> +		unsigned char ret_val;
>>>>> +		nand_command(NAND_CMD_STATUS);
>>>>> +		do {
>>>>> +			ret_val = READ_NAND(NAND_ADDR);/* wait till ready */
>>>>> +  		} while ((ret_val & 0x40) != 0x40);
>>>> You should be using some kind of timeout, so you will not stuck in here
>>>> without being noticed.
>>> OK.  I've been wondering if we shouldn't somehow make a
>>> not-tied-to-full-mtd nand_command more available since I suspect a few
>>> other boards will be in a similar situation, for probing early on.
>>
>> That would be much better solution.
> 
> (Background if you've otherwise ignored the thread, on some omap3 boards
> we need to take a whack at NAND to see what DDR we have)
> 
> OK, so, Scott, where would you suggest we put a very trivial set of nand
> infrastructure we need outside of the scope of loading u-boot itself
> from NAND ?

Can the SPL code be used (not an actual SPL build, just the NAND code
associated with it)?

-Scott

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Scott Wood]

On 10/27/2011 06:02 PM, Igor Grinberg wrote:
> On 10/27/2011 11:27 PM, Wolfgang Denk wrote:
>> Dear Igor Grinberg,
>>
>> In message <4EA9CA9D.6010209@compulab.co.il> you wrote:
>>> On 10/26/2011 11:13 PM, Tom Rini wrote:
>>>> This introduces 200MHz Micron parts timing information based on x-loader
>>>> and re-organizes the file slightly for grouping.  The memory init logic
>>>> is also based on what x-loader does in these cases.  Note that while
>>>> previously u-boot would be flashed in with SW ECC in this case it now
>>>> must be flashed with HW ECC.
>>> You have two spaces between the sentences, why is that?
>> For improved readability.  It should be standard formatting for any
>> text typed in constant-width fonts.
> 
> I actually do not see how this improves readability...

It's a common practice, and for many people is both a habit and more
visually pleasing.  Nobody's forcing you to do it, and we don't complain
when other people type sentences that are all squished together. :-)

> Is that documented somewhere? I don't think so...

http://en.wikipedia.org/wiki/Sentence_spacing

-Scott

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Tom Rini]

On 10/27/2011 04:10 PM, Igor Grinberg wrote:
> On 10/27/2011 11:29 PM, Tom Rini wrote:
>> On 10/27/2011 02:18 PM, Igor Grinberg wrote:
>>> On 10/26/2011 11:13 PM, Tom Rini wrote:
[snip]
>>>> +/* nand_command: Send a flash command to the flash chip */
>>>> +static void nand_command(unsigned char command)
>>>> +{
>>>> + 	WRITE_NAND_COMMAND(command, NAND_ADDR);
>>>> +
>>>> +  	if (command == NAND_CMD_RESET) {
>>>> +		unsigned char ret_val;
>>>> +		nand_command(NAND_CMD_STATUS);
>>>> +		do {
>>>> +			ret_val = READ_NAND(NAND_ADDR);/* wait till ready */
>>>> +  		} while ((ret_val & 0x40) != 0x40);
>>> You should be using some kind of timeout, so you will not stuck in here
>>> without being noticed.
>> OK.  I've been wondering if we shouldn't somehow make a
>> not-tied-to-full-mtd nand_command more available since I suspect a few
>> other boards will be in a similar situation, for probing early on.
> 
> That would be much better solution.

(Background if you've otherwise ignored the thread, on some omap3 boards
we need to take a whack at NAND to see what DDR we have)

OK, so, Scott, where would you suggest we put a very trivial set of nand
infrastructure we need outside of the scope of loading u-boot itself
from NAND ?

-- 
Tom

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Igor Grinberg]

On 10/27/2011 11:29 PM, Tom Rini wrote:
> On 10/27/2011 02:18 PM, Igor Grinberg wrote:
>> On 10/26/2011 11:13 PM, Tom Rini wrote:
>>> This introduces 200MHz Micron parts timing information based on x-loader
>>> and re-organizes the file slightly for grouping.  The memory init logic
>>> is also based on what x-loader does in these cases.  Note that while
>>> previously u-boot would be flashed in with SW ECC in this case it now
>>> must be flashed with HW ECC.
>> You have two spaces between the sentences, why is that?
> Old habit.
>
>>> Beagleboard rev C5, xM rev A:
>>> Tested-by: Tom Rini <trini@ti.com>
>>> Beagleboard xM rev C:
>>> Tested-by: Matt Ranostay <mranostay@gmail.com>
>>> Beagleboard rev B7, C2, xM rev B:
>>> Tested-by: Matt Porter <mporter@ti.com>
>>> Signed-off-by: Tom Rini <trini@ti.com>
>>> ---
>>>  arch/arm/include/asm/arch-omap3/mem.h |   24 +++++
>>>  board/ti/beagle/beagle.c              |  160 ++++++++++++++++++++++++++++++++-
>>>  board/ti/beagle/config.mk             |   33 -------
>>>  include/configs/omap3_beagle.h        |   60 ++++++++++++-
>>>  4 files changed, 242 insertions(+), 35 deletions(-)
>>>  delete mode 100644 board/ti/beagle/config.mk
>> config.mk removal does not belong to that patch...
>> It should be a separate one, say cleanup patch.
> I'll see if I can restructure things and kill that off first then.
>
>>> diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
>>> index af3504c..a784813 100644
>>> --- a/arch/arm/include/asm/arch-omap3/mem.h
>>> +++ b/arch/arm/include/asm/arch-omap3/mem.h
>>> @@ -171,6 +171,30 @@ enum {
>>>  #define MICRON_V_MR ((MICRON_WBST << 9) | (MICRON_CASL << 4) | \
>>>  	(MICRON_SIL << 3) | (MICRON_BL))
>>>  
>>> +
>>> +/* Micron part (200MHz optimized) 5 ns
>>> +  */
>>> +#define MICRON_TDAL_200   6
>>> +#define MICRON_TDPL_200   3
>>> +#define MICRON_TRRD_200   2
>>> +#define MICRON_TRCD_200   3
>>> +#define MICRON_TRP_200    3
>>> +#define MICRON_TRAS_200   8
>>> +#define MICRON_TRC_200   11
>>> +#define MICRON_TRFC_200  15
>>> +#define MICRON_V_ACTIMA_200 ((MICRON_TRFC_200 << 27) | (MICRON_TRC_200 << 22) | (MICRON_TRAS_200 << 18) \
>>> +		| (MICRON_TRP_200 << 15) | (MICRON_TRCD_200 << 12) |(MICRON_TRRD_200 << 9) | \
>>> +		(MICRON_TDPL_200 << 6) | (MICRON_TDAL_200))
>> MICRON_TDAL_200 does not need parenthesis.
> As I said to Sanjeev this is all yanked right from X-loader so yes,
> there's spacing and so on problems (and I swore git send-email --subject
> modified subject for all patches, but I guess not).
>
> [snip]
>>> +#define WRITE_NAND_COMMAND(d, adr) \
>>> +	do {*(volatile u16 *)GPMC_NAND_COMMAND_0 = d;} while(0)
>>> +#define WRITE_NAND_ADDRESS(d, adr) \
>>> +	do {*(volatile u16 *)GPMC_NAND_ADDRESS_0 = d;} while(0)
>>> +#define READ_NAND(adr)          (*(volatile u16 *)GPMC_NAND_DATA_0)
>> This is definitely needs a cleanup...
>> Consider Sanjeev's proposal. If it will not work for some reason,
>> you need at least to use writel() readl() io accessors.
> At first pass you can't call gpmc_init() as-is this early, but I'll
> switch over to readl/writel as a start.
>
>>> +
>>> +/* nand_command: Send a flash command to the flash chip */
>>> +static void nand_command(unsigned char command)
>>> +{
>>> + 	WRITE_NAND_COMMAND(command, NAND_ADDR);
>>> +
>>> +  	if (command == NAND_CMD_RESET) {
>>> +		unsigned char ret_val;
>>> +		nand_command(NAND_CMD_STATUS);
>>> +		do {
>>> +			ret_val = READ_NAND(NAND_ADDR);/* wait till ready */
>>> +  		} while ((ret_val & 0x40) != 0x40);
>> You should be using some kind of timeout, so you will not stuck in here
>> without being noticed.
> OK.  I've been wondering if we shouldn't somehow make a
> not-tied-to-full-mtd nand_command more available since I suspect a few
> other boards will be in a similar situation, for probing early on.

That would be much better solution.

> [snip]
>>> +	*mr = MICRON_V_MR;
>>> +	switch (get_board_revision()) {
>>> +	case REVISION_C4:
>>> +		if (identify_xm_ddr() == NUMONYX_MCP) {
>>> +			*cs_cfg = 0x4;
>>> +			*mcfg = 0x04590099;
>>> +			*ctrla = NUMONYX_V_ACTIMA_165;
>>> +			*ctrlb = NUMONYX_V_ACTIMB_165;
>>> +			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
>> aligning the assignments above (and below) will make it much more readable
> I'm really not a fan of spacing out assignments but I'll see what
> CodingSytle says.

Yeah, me too, but it looks like these, can benefit from it...
It is not a blocker - up to you ;)

> [snip]
>>> -#ifdef CONFIG_GENERIC_MMC
>>> +#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD)
>> This should be another patch.
> Really?  I'm adding SPL support here and that's what introduces the need
> to not build this on just CONFIG_GENERIC_MMC.

No, not really ;) I've been disrupted and did a mistake - sorry for that.

> [snip]
>>> +#define CONFIG_SPL_BSS_START_ADDR	0x80000000
>>> +#define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */
>> alignment
> I'll double check but all of them but this one is fine once applied.
>
> Thanks.
>

Regards,
Igor

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Igor Grinberg]

On 10/27/2011 11:27 PM, Wolfgang Denk wrote:
> Dear Igor Grinberg,
>
> In message <4EA9CA9D.6010209@compulab.co.il> you wrote:
>> On 10/26/2011 11:13 PM, Tom Rini wrote:
>>> This introduces 200MHz Micron parts timing information based on x-loader
>>> and re-organizes the file slightly for grouping.  The memory init logic
>>> is also based on what x-loader does in these cases.  Note that while
>>> previously u-boot would be flashed in with SW ECC in this case it now
>>> must be flashed with HW ECC.
>> You have two spaces between the sentences, why is that?
> For improved readability.  It should be standard formatting for any
> text typed in constant-width fonts.

I actually do not see how this improves readability...
Is that documented somewhere? I don't think so...
For me it is just an annoying white space error.
A dot and a space is a nice separator between the sentences - it is
what people do all over the world for quite a while ;) and with the
fixed width
fonts, you almost get two spaces, because dot is small, but
if you want this to be two spaces - I can live with it.

Regards,
Igor

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Tom Rini]

On 10/27/2011 02:18 PM, Igor Grinberg wrote:
> On 10/26/2011 11:13 PM, Tom Rini wrote:
>> This introduces 200MHz Micron parts timing information based on x-loader
>> and re-organizes the file slightly for grouping.  The memory init logic
>> is also based on what x-loader does in these cases.  Note that while
>> previously u-boot would be flashed in with SW ECC in this case it now
>> must be flashed with HW ECC.
> 
> You have two spaces between the sentences, why is that?

Old habit.

>> Beagleboard rev C5, xM rev A:
>> Tested-by: Tom Rini <trini@ti.com>
>> Beagleboard xM rev C:
>> Tested-by: Matt Ranostay <mranostay@gmail.com>
>> Beagleboard rev B7, C2, xM rev B:
>> Tested-by: Matt Porter <mporter@ti.com>
>> Signed-off-by: Tom Rini <trini@ti.com>
>> ---
>>  arch/arm/include/asm/arch-omap3/mem.h |   24 +++++
>>  board/ti/beagle/beagle.c              |  160 ++++++++++++++++++++++++++++++++-
>>  board/ti/beagle/config.mk             |   33 -------
>>  include/configs/omap3_beagle.h        |   60 ++++++++++++-
>>  4 files changed, 242 insertions(+), 35 deletions(-)
>>  delete mode 100644 board/ti/beagle/config.mk
> 
> config.mk removal does not belong to that patch...
> It should be a separate one, say cleanup patch.

I'll see if I can restructure things and kill that off first then.

> 
>> diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
>> index af3504c..a784813 100644
>> --- a/arch/arm/include/asm/arch-omap3/mem.h
>> +++ b/arch/arm/include/asm/arch-omap3/mem.h
>> @@ -171,6 +171,30 @@ enum {
>>  #define MICRON_V_MR ((MICRON_WBST << 9) | (MICRON_CASL << 4) | \
>>  	(MICRON_SIL << 3) | (MICRON_BL))
>>  
>> +
>> +/* Micron part (200MHz optimized) 5 ns
>> +  */
>> +#define MICRON_TDAL_200   6
>> +#define MICRON_TDPL_200   3
>> +#define MICRON_TRRD_200   2
>> +#define MICRON_TRCD_200   3
>> +#define MICRON_TRP_200    3
>> +#define MICRON_TRAS_200   8
>> +#define MICRON_TRC_200   11
>> +#define MICRON_TRFC_200  15
>> +#define MICRON_V_ACTIMA_200 ((MICRON_TRFC_200 << 27) | (MICRON_TRC_200 << 22) | (MICRON_TRAS_200 << 18) \
>> +		| (MICRON_TRP_200 << 15) | (MICRON_TRCD_200 << 12) |(MICRON_TRRD_200 << 9) | \
>> +		(MICRON_TDPL_200 << 6) | (MICRON_TDAL_200))
> 
> MICRON_TDAL_200 does not need parenthesis.

As I said to Sanjeev this is all yanked right from X-loader so yes,
there's spacing and so on problems (and I swore git send-email --subject
modified subject for all patches, but I guess not).

[snip]
>> +#define WRITE_NAND_COMMAND(d, adr) \
>> +	do {*(volatile u16 *)GPMC_NAND_COMMAND_0 = d;} while(0)
>> +#define WRITE_NAND_ADDRESS(d, adr) \
>> +	do {*(volatile u16 *)GPMC_NAND_ADDRESS_0 = d;} while(0)
>> +#define READ_NAND(adr)          (*(volatile u16 *)GPMC_NAND_DATA_0)
> 
> This is definitely needs a cleanup...
> Consider Sanjeev's proposal. If it will not work for some reason,
> you need at least to use writel() readl() io accessors.

At first pass you can't call gpmc_init() as-is this early, but I'll
switch over to readl/writel as a start.

> 
>> +
>> +/* nand_command: Send a flash command to the flash chip */
>> +static void nand_command(unsigned char command)
>> +{
>> + 	WRITE_NAND_COMMAND(command, NAND_ADDR);
>> +
>> +  	if (command == NAND_CMD_RESET) {
>> +		unsigned char ret_val;
>> +		nand_command(NAND_CMD_STATUS);
>> +		do {
>> +			ret_val = READ_NAND(NAND_ADDR);/* wait till ready */
>> +  		} while ((ret_val & 0x40) != 0x40);
> 
> You should be using some kind of timeout, so you will not stuck in here
> without being noticed.

OK.  I've been wondering if we shouldn't somehow make a
not-tied-to-full-mtd nand_command more available since I suspect a few
other boards will be in a similar situation, for probing early on.

[snip]
>> +	*mr = MICRON_V_MR;
>> +	switch (get_board_revision()) {
>> +	case REVISION_C4:
>> +		if (identify_xm_ddr() == NUMONYX_MCP) {
>> +			*cs_cfg = 0x4;
>> +			*mcfg = 0x04590099;
>> +			*ctrla = NUMONYX_V_ACTIMA_165;
>> +			*ctrlb = NUMONYX_V_ACTIMB_165;
>> +			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
> 
> aligning the assignments above (and below) will make it much more readable

I'm really not a fan of spacing out assignments but I'll see what
CodingSytle says.

[snip]
>> -#ifdef CONFIG_GENERIC_MMC
>> +#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD)
> 
> This should be another patch.

Really?  I'm adding SPL support here and that's what introduces the need
to not build this on just CONFIG_GENERIC_MMC.

[snip]
>> +#define CONFIG_SPL_BSS_START_ADDR	0x80000000
>> +#define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */
> 
> alignment

I'll double check but all of them but this one is fine once applied.

Thanks.

-- 
Tom

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Wolfgang Denk]

Dear Igor Grinberg,

In message <4EA9CA9D.6010209@compulab.co.il> you wrote:
> On 10/26/2011 11:13 PM, Tom Rini wrote:
> > This introduces 200MHz Micron parts timing information based on x-loader
> > and re-organizes the file slightly for grouping.  The memory init logic
> > is also based on what x-loader does in these cases.  Note that while
> > previously u-boot would be flashed in with SW ECC in this case it now
> > must be flashed with HW ECC.
> 
> You have two spaces between the sentences, why is that?

For improved readability.  It should be standard formatting for any
text typed in constant-width fonts.

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
"Well I don't see why I have to make one man  miserable  when  I  can
make so many men happy."              - Ellyn Mustard, about marriage

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Igor Grinberg]

On 10/26/2011 11:13 PM, Tom Rini wrote:
> This introduces 200MHz Micron parts timing information based on x-loader
> and re-organizes the file slightly for grouping.  The memory init logic
> is also based on what x-loader does in these cases.  Note that while
> previously u-boot would be flashed in with SW ECC in this case it now
> must be flashed with HW ECC.

You have two spaces between the sentences, why is that?

> Beagleboard rev C5, xM rev A:
> Tested-by: Tom Rini <trini@ti.com>
> Beagleboard xM rev C:
> Tested-by: Matt Ranostay <mranostay@gmail.com>
> Beagleboard rev B7, C2, xM rev B:
> Tested-by: Matt Porter <mporter@ti.com>
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
>  arch/arm/include/asm/arch-omap3/mem.h |   24 +++++
>  board/ti/beagle/beagle.c              |  160 ++++++++++++++++++++++++++++++++-
>  board/ti/beagle/config.mk             |   33 -------
>  include/configs/omap3_beagle.h        |   60 ++++++++++++-
>  4 files changed, 242 insertions(+), 35 deletions(-)
>  delete mode 100644 board/ti/beagle/config.mk

config.mk removal does not belong to that patch...
It should be a separate one, say cleanup patch.

> diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
> index af3504c..a784813 100644
> --- a/arch/arm/include/asm/arch-omap3/mem.h
> +++ b/arch/arm/include/asm/arch-omap3/mem.h
> @@ -171,6 +171,30 @@ enum {
>  #define MICRON_V_MR ((MICRON_WBST << 9) | (MICRON_CASL << 4) | \
>  	(MICRON_SIL << 3) | (MICRON_BL))
>  
> +
> +/* Micron part (200MHz optimized) 5 ns
> +  */
> +#define MICRON_TDAL_200   6
> +#define MICRON_TDPL_200   3
> +#define MICRON_TRRD_200   2
> +#define MICRON_TRCD_200   3
> +#define MICRON_TRP_200    3
> +#define MICRON_TRAS_200   8
> +#define MICRON_TRC_200   11
> +#define MICRON_TRFC_200  15
> +#define MICRON_V_ACTIMA_200 ((MICRON_TRFC_200 << 27) | (MICRON_TRC_200 << 22) | (MICRON_TRAS_200 << 18) \
> +		| (MICRON_TRP_200 << 15) | (MICRON_TRCD_200 << 12) |(MICRON_TRRD_200 << 9) | \
> +		(MICRON_TDPL_200 << 6) | (MICRON_TDAL_200))

MICRON_TDAL_200 does not need parenthesis.


> +
> +#define MICRON_TWTR_200   2
> +#define MICRON_TCKE_200   4
> +#define MICRON_TXP_200    2
> +#define MICRON_XSR_200   23
> +#define MICRON_V_ACTIMB_200 ((MICRON_TCKE_200 << 12) | (MICRON_XSR_200 << 0)) | \
> +				(MICRON_TXP_200 << 8) | (MICRON_TWTR_200 << 16)
> +#define MICRON_ARCV_200		0x5e6
> +#define MICRON_V_RFR_CTRL_200	((MICRON_ARCV_200 << 8) | (MICRON_ARE))

same here with MICRON_ARE

> +
>  /*
>   * NUMONYX part of IGEP v2 (165MHz optimized) 6.06ns
>   *   ACTIMA
> diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
> index aa5047c..a77b0cb 100644
> --- a/board/ti/beagle/beagle.c
> +++ b/board/ti/beagle/beagle.c
> @@ -34,9 +34,11 @@
>  #include <status_led.h>
>  #endif
>  #include <twl4030.h>
> +#include <linux/mtd/nand.h>
>  #include <asm/io.h>
>  #include <asm/arch/mmc_host_def.h>
>  #include <asm/arch/mux.h>
> +#include <asm/arch/mem.h>
>  #include <asm/arch/sys_proto.h>
>  #include <asm/gpio.h>
>  #include <asm/mach-types.h>
> @@ -139,6 +141,160 @@ int get_board_revision(void)
>  	return revision;
>  }
>  
> +#ifdef CONFIG_SPL_BUILD
> +
> +#define MICRON_DDR	0
> +#define NUMONYX_MCP	1
> +#define MICRON_MCP	2
> +
> +#define NAND_CMD_STATUS		0x70
> +#define NAND_CMD_READID		0x90
> +#define NAND_CMD_RESET		0xff
> +
> +#define GPMC_NAND_COMMAND_0      (OMAP34XX_GPMC_BASE+0x7C)
> +#define GPMC_NAND_ADDRESS_0      (OMAP34XX_GPMC_BASE+0x80)
> +#define GPMC_NAND_DATA_0	 (OMAP34XX_GPMC_BASE+0x84)

Why not use tabs for alignment?

> +
> +#define WRITE_NAND_COMMAND(d, adr) \
> +	do {*(volatile u16 *)GPMC_NAND_COMMAND_0 = d;} while(0)
> +#define WRITE_NAND_ADDRESS(d, adr) \
> +	do {*(volatile u16 *)GPMC_NAND_ADDRESS_0 = d;} while(0)
> +#define READ_NAND(adr)          (*(volatile u16 *)GPMC_NAND_DATA_0)

This is definitely needs a cleanup...
Consider Sanjeev's proposal. If it will not work for some reason,
you need at least to use writel() readl() io accessors.

> +
> +/* nand_command: Send a flash command to the flash chip */
> +static void nand_command(unsigned char command)
> +{
> + 	WRITE_NAND_COMMAND(command, NAND_ADDR);
> +
> +  	if (command == NAND_CMD_RESET) {
> +		unsigned char ret_val;
> +		nand_command(NAND_CMD_STATUS);
> +		do {
> +			ret_val = READ_NAND(NAND_ADDR);/* wait till ready */
> +  		} while ((ret_val & 0x40) != 0x40);

You should be using some kind of timeout, so you will not stuck in here
without being noticed.

> + 	}
> +}
> +
> +/* 
> + * In order to find out what DDR we have we need to see what NAND we
> + * may have.  This relies on having already initalized GPMC earlier

Two spaces?

> + * in the sequence.
> + */
> +static void nand_readid(int *mfr, int *id)
> +{
> + 	nand_command(NAND_CMD_RESET);
> + 	nand_command(NAND_CMD_READID);
> +
> +	WRITE_NAND_ADDRESS(0x0, NAND_ADDR);
> +
> +	/* Read off the manufacturer and device id. */
> +	*mfr = READ_NAND(NAND_ADDR);
> +	*id = READ_NAND(NAND_ADDR);
> +}
> +
> +#define GPMC_CONFIG_CS0_CONFIG1		0x6E000060
> +#define GPMC_CONFIG_CS0_CONFIG2		0x6E000064
> +#define GPMC_CONFIG_CS0_CONFIG3		0x6E000068
> +#define GPMC_CONFIG_CS0_CONFIG4		0x6E00006C
> +#define GPMC_CONFIG_CS0_CONFIG5		0x6E000070
> +#define GPMC_CONFIG_CS0_CONFIG6		0x6E000074
> +#define GPMC_CONFIG_CS0_CONFIG7		0x6E000078
> +#define OMAP34XX_GPMC_CS0_SIZE		0x8

Why do you need two tabs in all the above defines?
And yes, Sanjeev is right.

> +
> +static int identify_xm_ddr(void)
> +{
> +	int mfr, id;
> +
> +	/* Make sure that we have setup GPMC for NAND correctly. */
> +	writel(M_NAND_GPMC_CONFIG1, GPMC_CONFIG_CS0_CONFIG1);
> +	writel(M_NAND_GPMC_CONFIG2, GPMC_CONFIG_CS0_CONFIG2);
> +	writel(M_NAND_GPMC_CONFIG3, GPMC_CONFIG_CS0_CONFIG3);
> +	writel(M_NAND_GPMC_CONFIG4, GPMC_CONFIG_CS0_CONFIG4);
> +	writel(M_NAND_GPMC_CONFIG5, GPMC_CONFIG_CS0_CONFIG5);
> +	writel(M_NAND_GPMC_CONFIG6, GPMC_CONFIG_CS0_CONFIG6);
> +
> +	/* Enable the GPMC Mapping */
> +	writel((((OMAP34XX_GPMC_CS0_SIZE & 0xF) << 8) |
> +			     ((NAND_BASE >> 24) & 0x3F) |
> +			     (1 << 6)),  (GPMC_CONFIG_CS0_CONFIG7));

White space problems, unneeded parenthesis.

> +
> +	sdelay(2000);
> +
> +	nand_readid(&mfr, &id);
> +	if (mfr == 0)
> +		return MICRON_DDR;
> +	if ((mfr == 0x20) && (id == 0xba))

parenthesis

> +		return NUMONYX_MCP;
> +	if ((mfr == 0x2c) && (id == 0xbc))

parenthesis

> +		return MICRON_MCP;
> +
> +	/* Unknown. */
> +	return -1;
> +}
> +
> +/* 
> + * Routine: board_early_sdrc_init

wrong

> + * Description: If we use SPL then there is no x-loader nor config header
> + * so we have to setup the DDR timings outself on both banks.
> + */
> +void get_board_mem_timings(u32 *cs_cfg, u32 *mcfg, u32 *ctrla, u32 *ctrlb,
> +		u32 *rfr_ctrl, u32 *mr)
> +{
> +	/* TODO XXX REWORD */
> +	/* We have magic hard coded values here for V_MCFG which come from
> +	 * x-loader as they do not match how the OMAP35x TRM says to
> +	 * calculate them values. */

multi line comment should be:
/*
 * blabla
 */

> +	*mr = MICRON_V_MR;
> +	switch (get_board_revision()) {
> +	case REVISION_C4:
> +		if (identify_xm_ddr() == NUMONYX_MCP) {
> +			*cs_cfg = 0x4;
> +			*mcfg = 0x04590099;
> +			*ctrla = NUMONYX_V_ACTIMA_165;
> +			*ctrlb = NUMONYX_V_ACTIMB_165;
> +			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;

aligning the assignments above (and below) will make it much more readable

> +		} else if (identify_xm_ddr() == MICRON_MCP) {
> +			/* Beagleboard Rev C5 */
> +			*cs_cfg = 0x2;
> +			*mcfg = 0x03588099;
> +			*ctrla = MICRON_V_ACTIMA_200;
> +			*ctrlb = MICRON_V_ACTIMB_200;
> +			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_200MHz;
> +		} else {
> +			*cs_cfg = 0x1;
> +			*mcfg = 0x02584099;
> +			*ctrla = MICRON_V_ACTIMA_165;
> +			*ctrlb = MICRON_V_ACTIMB_165;
> +			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
> +		}
> +		break;
> +	case REVISION_XM_A:
> +	case REVISION_XM_B:
> +	case REVISION_XM_C:
> +		if (identify_xm_ddr() == MICRON_DDR) {
> +			*cs_cfg = 0x2;
> +			*mcfg = 0x03588099;
> +			*ctrla = MICRON_V_ACTIMA_200;
> +			*ctrlb = MICRON_V_ACTIMB_200;
> +			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_200MHz;
> +		} else {
> +			*cs_cfg = 0x4;
> +			*mcfg = 0x04590099;
> +			*ctrla = NUMONYX_V_ACTIMA_165;
> +			*ctrlb = NUMONYX_V_ACTIMB_165;
> +			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
> +		}
> +		break;
> +	default:
> +		*cs_cfg = 0x1;
> +		*mcfg = 0x02584099;
> +		*ctrla = MICRON_V_ACTIMA_165;
> +		*ctrlb = MICRON_V_ACTIMB_165;
> +		*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
> +	}
> +}
> +#endif
> +
>  /*
>   * Routine: get_expansion_id
>   * Description: This function checks for expansion board by checking I2C
> @@ -371,7 +527,7 @@ void set_muxconf_regs(void)
>  	MUX_BEAGLE();
>  }
>  
> -#ifdef CONFIG_GENERIC_MMC
> +#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD)

This should be another patch.

>  int board_mmc_init(bd_t *bis)
>  {
>  	omap_mmc_init(0);
> @@ -480,6 +636,7 @@ int ehci_hcd_init(void)
>  
>  #endif /* CONFIG_USB_EHCI */
>  
> +#ifndef CONFIG_SPL_BUILD
>  /*
>   * This command returns the status of the user button on beagle xM
>   * Input - none
> @@ -534,3 +691,4 @@ U_BOOT_CMD(
>  	"Return the status of the BeagleBoard USER button",
>  	""
>  );
> +#endif
> diff --git a/board/ti/beagle/config.mk b/board/ti/beagle/config.mk
> deleted file mode 100644
> index cf055db..0000000
> --- a/board/ti/beagle/config.mk
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -#
> -# (C) Copyright 2006
> -# Texas Instruments, <www.ti.com>
> -#
> -# Beagle Board uses OMAP3 (ARM-CortexA8) cpu
> -# see http://www.ti.com/ for more information on Texas Instruments
> -#
> -# See file CREDITS for list of people who contributed to this
> -# project.
> -#
> -# 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
> -#
> -# Physical Address:
> -# 8000'0000 (bank0)
> -# A000/0000 (bank1)
> -# Linux-Kernel is expected to be at 8000'8000, entry 8000'8000
> -# (mem base + reserved)
> -
> -# For use with external or internal boots.
> -CONFIG_SYS_TEXT_BASE = 0x80008000
> diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
> index a026478..a126566 100644
> --- a/include/configs/omap3_beagle.h
> +++ b/include/configs/omap3_beagle.h
> @@ -347,7 +347,7 @@
>   */
>  #define CONFIG_NR_DRAM_BANKS	2	/* CS1 may or may not be populated */
>  #define PHYS_SDRAM_1		OMAP34XX_SDRC_CS0
> -#define PHYS_SDRAM_1_SIZE	(32 << 20)	/*@least 32 MiB */
> +#define PHYS_SDRAM_1_SIZE	(32 << 20)	/* At least 32 MiB */
>  #define PHYS_SDRAM_2		OMAP34XX_SDRC_CS1
>  
>  /* SDRAM Bank Allocation method */
> @@ -390,4 +390,62 @@
>  
>  #define CONFIG_OMAP3_SPI
>  
> +/* Defines for SPL */
> +#define CONFIG_SPL
> +#define CONFIG_SPL_NAND_SIMPLE
> +#define CONFIG_SPL_TEXT_BASE		0x40200800
> +#define CONFIG_SPL_MAX_SIZE		(45 * 1024)
> +#define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
> +
> +#define CONFIG_SPL_BSS_START_ADDR	0x80000000
> +#define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */

alignment

> +
> +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR	0x300 /* address 0x60000 */
> +#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS	0x200 /* 256 KB */
> +#define CONFIG_SYS_MMC_SD_FAT_BOOT_PARTITION	1
> +#define CONFIG_SPL_FAT_LOAD_PAYLOAD_NAME	"u-boot.img"

ditto

> +
> +#define CONFIG_SPL_LIBCOMMON_SUPPORT
> +#define CONFIG_SPL_LIBDISK_SUPPORT
> +#define CONFIG_SPL_I2C_SUPPORT
> +#define CONFIG_SPL_LIBGENERIC_SUPPORT
> +#define CONFIG_SPL_MMC_SUPPORT
> +#define CONFIG_SPL_FAT_SUPPORT
> +#define CONFIG_SPL_SERIAL_SUPPORT
> +#define CONFIG_SPL_NAND_SUPPORT
> +#define CONFIG_SPL_POWER_SUPPORT
> +#define CONFIG_SPL_LDSCRIPT		"$(CPUDIR)/omap-common/u-boot-spl.lds"
> +
> +/* NAND boot config */
> +#define CONFIG_SYS_NAND_5_ADDR_CYCLE
> +#define CONFIG_SYS_NAND_PAGE_COUNT	64
> +#define CONFIG_SYS_NAND_PAGE_SIZE	2048
> +#define CONFIG_SYS_NAND_OOBSIZE		64

ditto

> +#define CONFIG_SYS_NAND_BLOCK_SIZE	(128*1024)
> +#define CONFIG_SYS_NAND_BAD_BLOCK_POS	0
> +#define CONFIG_SYS_NAND_ECCPOS		{2, 3, 4, 5, 6, 7, 8, 9,\
> +						10, 11, 12, 13}
> +
> +#define CONFIG_SYS_NAND_ECCSIZE		512
> +#define CONFIG_SYS_NAND_ECCBYTES	3

ditto

> +
> +#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
> +						CONFIG_SYS_NAND_ECCSIZE)
> +#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
> +						CONFIG_SYS_NAND_ECCSTEPS)
> +
> +#define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
> +
> +#define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
> +
> +/*
> + * 1MB into the SDRAM to allow for SPL's bss at the beginning of SDRAM
> + * 64 bytes before this address should be set aside for u-boot.img's
> + * header. That is 0x800FFFC0--0x80100000 should not be used for any
> + * other needs.
> + */
> +#define CONFIG_SYS_TEXT_BASE		0x80100000
> +#define CONFIG_SYS_SPL_MALLOC_START	0x80208000
> +#define CONFIG_SYS_SPL_MALLOC_SIZE	0x100000
> +
>  #endif /* __CONFIG_H */

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Tom Rini]

On 10/27/2011 05:46 AM, Premi, Sanjeev wrote:
>> -----Original Message-----
>> From: u-boot-bounces at lists.denx.de 
>> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Rini, Tom
>> Sent: Thursday, October 27, 2011 2:44 AM
>> To: u-boot at lists.denx.de
>> Subject: [U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard
>>
>> This introduces 200MHz Micron parts timing information based 
>> on x-loader
>> and re-organizes the file slightly for grouping.  The memory 
>> init logic
>> is also based on what x-loader does in these cases.  Note that while
>> previously u-boot would be flashed in with SW ECC in this case it now
>> must be flashed with HW ECC.
>>
>> Beagleboard rev C5, xM rev A:
>> Tested-by: Tom Rini <trini@ti.com>
>> Beagleboard xM rev C:
>> Tested-by: Matt Ranostay <mranostay@gmail.com>
>> Beagleboard rev B7, C2, xM rev B:
>> Tested-by: Matt Porter <mporter@ti.com>
>> Signed-off-by: Tom Rini <trini@ti.com>
>> ---
> 
> [snip]...[snip]
> 
>>  
>> +#ifdef CONFIG_SPL_BUILD
>> +
>> +#define MICRON_DDR	0
>> +#define NUMONYX_MCP	1
>> +#define MICRON_MCP	2
>> +
>> +#define NAND_CMD_STATUS		0x70
>> +#define NAND_CMD_READID		0x90
>> +#define NAND_CMD_RESET		0xff
>> +
>> +#define GPMC_NAND_COMMAND_0      (OMAP34XX_GPMC_BASE+0x7C)
>> +#define GPMC_NAND_ADDRESS_0      (OMAP34XX_GPMC_BASE+0x80)
>> +#define GPMC_NAND_DATA_0	 (OMAP34XX_GPMC_BASE+0x84)
>> +
>> +#define WRITE_NAND_COMMAND(d, adr) \
>> +	do {*(volatile u16 *)GPMC_NAND_COMMAND_0 = d;} while(0)
>> +#define WRITE_NAND_ADDRESS(d, adr) \
>> +	do {*(volatile u16 *)GPMC_NAND_ADDRESS_0 = d;} while(0)
>> +#define READ_NAND(adr)          (*(volatile u16 *)GPMC_NAND_DATA_0)
>> +
> 
> I am not yet familiar with SPL code, but I would suggest using
> "struct gpmc" instead of hardcoded offsets above.
> 
> [snip]...[snip]
> 
>> +
>> +#define GPMC_CONFIG_CS0_CONFIG1		0x6E000060
>> +#define GPMC_CONFIG_CS0_CONFIG2		0x6E000064
>> +#define GPMC_CONFIG_CS0_CONFIG3		0x6E000068
>> +#define GPMC_CONFIG_CS0_CONFIG4		0x6E00006C
>> +#define GPMC_CONFIG_CS0_CONFIG5		0x6E000070
>> +#define GPMC_CONFIG_CS0_CONFIG6		0x6E000074
>> +#define GPMC_CONFIG_CS0_CONFIG7		0x6E000078
>> +#define OMAP34XX_GPMC_CS0_SIZE		0x8
> 
> Suggest using "struct gpmc_cs" instead of defining offsets.
> 
> [snip]...[snip]
> 
>> +
>> +static int identify_xm_ddr(void)
>> +{
>> +	int mfr, id;
>> +
>> +	/* Make sure that we have setup GPMC for NAND correctly. */
>> +	writel(M_NAND_GPMC_CONFIG1, GPMC_CONFIG_CS0_CONFIG1);
>> +	writel(M_NAND_GPMC_CONFIG2, GPMC_CONFIG_CS0_CONFIG2);
>> +	writel(M_NAND_GPMC_CONFIG3, GPMC_CONFIG_CS0_CONFIG3);
>> +	writel(M_NAND_GPMC_CONFIG4, GPMC_CONFIG_CS0_CONFIG4);
>> +	writel(M_NAND_GPMC_CONFIG5, GPMC_CONFIG_CS0_CONFIG5);
>> +	writel(M_NAND_GPMC_CONFIG6, GPMC_CONFIG_CS0_CONFIG6);
>> +
>> +	/* Enable the GPMC Mapping */
>> +	writel((((OMAP34XX_GPMC_CS0_SIZE & 0xF) << 8) |
>> +			     ((NAND_BASE >> 24) & 0x3F) |
>> +			     (1 << 6)),  (GPMC_CONFIG_CS0_CONFIG7));
> 
> Same comment as before. Looks like there may be few more places
> where hardcoded addresses/ offsets are being used.

Yeah, this is all a drop-in of the x-loader code which needs some clean
up.  But the overall logic (poke this to determine that) is what I want
to make sure we're going to be OK with.

-- 
Tom

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Premi, Sanjeev]

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de 
> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Rini, Tom
> Sent: Thursday, October 27, 2011 2:44 AM
> To: u-boot at lists.denx.de
> Subject: [U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard
> 
> This introduces 200MHz Micron parts timing information based 
> on x-loader
> and re-organizes the file slightly for grouping.  The memory 
> init logic
> is also based on what x-loader does in these cases.  Note that while
> previously u-boot would be flashed in with SW ECC in this case it now
> must be flashed with HW ECC.
> 
> Beagleboard rev C5, xM rev A:
> Tested-by: Tom Rini <trini@ti.com>
> Beagleboard xM rev C:
> Tested-by: Matt Ranostay <mranostay@gmail.com>
> Beagleboard rev B7, C2, xM rev B:
> Tested-by: Matt Porter <mporter@ti.com>
> Signed-off-by: Tom Rini <trini@ti.com>
> ---

[snip]...[snip]

>  
> +#ifdef CONFIG_SPL_BUILD
> +
> +#define MICRON_DDR	0
> +#define NUMONYX_MCP	1
> +#define MICRON_MCP	2
> +
> +#define NAND_CMD_STATUS		0x70
> +#define NAND_CMD_READID		0x90
> +#define NAND_CMD_RESET		0xff
> +
> +#define GPMC_NAND_COMMAND_0      (OMAP34XX_GPMC_BASE+0x7C)
> +#define GPMC_NAND_ADDRESS_0      (OMAP34XX_GPMC_BASE+0x80)
> +#define GPMC_NAND_DATA_0	 (OMAP34XX_GPMC_BASE+0x84)
> +
> +#define WRITE_NAND_COMMAND(d, adr) \
> +	do {*(volatile u16 *)GPMC_NAND_COMMAND_0 = d;} while(0)
> +#define WRITE_NAND_ADDRESS(d, adr) \
> +	do {*(volatile u16 *)GPMC_NAND_ADDRESS_0 = d;} while(0)
> +#define READ_NAND(adr)          (*(volatile u16 *)GPMC_NAND_DATA_0)
> +

I am not yet familiar with SPL code, but I would suggest using
"struct gpmc" instead of hardcoded offsets above.

[snip]...[snip]

> +
> +#define GPMC_CONFIG_CS0_CONFIG1		0x6E000060
> +#define GPMC_CONFIG_CS0_CONFIG2		0x6E000064
> +#define GPMC_CONFIG_CS0_CONFIG3		0x6E000068
> +#define GPMC_CONFIG_CS0_CONFIG4		0x6E00006C
> +#define GPMC_CONFIG_CS0_CONFIG5		0x6E000070
> +#define GPMC_CONFIG_CS0_CONFIG6		0x6E000074
> +#define GPMC_CONFIG_CS0_CONFIG7		0x6E000078
> +#define OMAP34XX_GPMC_CS0_SIZE		0x8

Suggest using "struct gpmc_cs" instead of defining offsets.

[snip]...[snip]

> +
> +static int identify_xm_ddr(void)
> +{
> +	int mfr, id;
> +
> +	/* Make sure that we have setup GPMC for NAND correctly. */
> +	writel(M_NAND_GPMC_CONFIG1, GPMC_CONFIG_CS0_CONFIG1);
> +	writel(M_NAND_GPMC_CONFIG2, GPMC_CONFIG_CS0_CONFIG2);
> +	writel(M_NAND_GPMC_CONFIG3, GPMC_CONFIG_CS0_CONFIG3);
> +	writel(M_NAND_GPMC_CONFIG4, GPMC_CONFIG_CS0_CONFIG4);
> +	writel(M_NAND_GPMC_CONFIG5, GPMC_CONFIG_CS0_CONFIG5);
> +	writel(M_NAND_GPMC_CONFIG6, GPMC_CONFIG_CS0_CONFIG6);
> +
> +	/* Enable the GPMC Mapping */
> +	writel((((OMAP34XX_GPMC_CS0_SIZE & 0xF) << 8) |
> +			     ((NAND_BASE >> 24) & 0x3F) |
> +			     (1 << 6)),  (GPMC_CONFIG_CS0_CONFIG7));

Same comment as before. Looks like there may be few more places
where hardcoded addresses/ offsets are being used.

~sanjeev

[snip]...[snip]

[U-Boot] [PATCH 2/2] OMAP3: Add SPL support to Beagleboard

[Tom Rini]

This introduces 200MHz Micron parts timing information based on x-loader
and re-organizes the file slightly for grouping.  The memory init logic
is also based on what x-loader does in these cases.  Note that while
previously u-boot would be flashed in with SW ECC in this case it now
must be flashed with HW ECC.

Beagleboard rev C5, xM rev A:
Tested-by: Tom Rini <trini@ti.com>
Beagleboard xM rev C:
Tested-by: Matt Ranostay <mranostay@gmail.com>
Beagleboard rev B7, C2, xM rev B:
Tested-by: Matt Porter <mporter@ti.com>
Signed-off-by: Tom Rini <trini@ti.com>
---
 arch/arm/include/asm/arch-omap3/mem.h |   24 +++++
 board/ti/beagle/beagle.c              |  160 ++++++++++++++++++++++++++++++++-
 board/ti/beagle/config.mk             |   33 -------
 include/configs/omap3_beagle.h        |   60 ++++++++++++-
 4 files changed, 242 insertions(+), 35 deletions(-)
 delete mode 100644 board/ti/beagle/config.mk

diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
index af3504c..a784813 100644
--- a/arch/arm/include/asm/arch-omap3/mem.h
+++ b/arch/arm/include/asm/arch-omap3/mem.h
@@ -171,6 +171,30 @@ enum {
 #define MICRON_V_MR ((MICRON_WBST << 9) | (MICRON_CASL << 4) | \
 	(MICRON_SIL << 3) | (MICRON_BL))
 
+
+/* Micron part (200MHz optimized) 5 ns
+  */
+#define MICRON_TDAL_200   6
+#define MICRON_TDPL_200   3
+#define MICRON_TRRD_200   2
+#define MICRON_TRCD_200   3
+#define MICRON_TRP_200    3
+#define MICRON_TRAS_200   8
+#define MICRON_TRC_200   11
+#define MICRON_TRFC_200  15
+#define MICRON_V_ACTIMA_200 ((MICRON_TRFC_200 << 27) | (MICRON_TRC_200 << 22) | (MICRON_TRAS_200 << 18) \
+		| (MICRON_TRP_200 << 15) | (MICRON_TRCD_200 << 12) |(MICRON_TRRD_200 << 9) | \
+		(MICRON_TDPL_200 << 6) | (MICRON_TDAL_200))
+
+#define MICRON_TWTR_200   2
+#define MICRON_TCKE_200   4
+#define MICRON_TXP_200    2
+#define MICRON_XSR_200   23
+#define MICRON_V_ACTIMB_200 ((MICRON_TCKE_200 << 12) | (MICRON_XSR_200 << 0)) | \
+				(MICRON_TXP_200 << 8) | (MICRON_TWTR_200 << 16)
+#define MICRON_ARCV_200		0x5e6
+#define MICRON_V_RFR_CTRL_200	((MICRON_ARCV_200 << 8) | (MICRON_ARE))
+
 /*
  * NUMONYX part of IGEP v2 (165MHz optimized) 6.06ns
  *   ACTIMA
diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
index aa5047c..a77b0cb 100644
--- a/board/ti/beagle/beagle.c
+++ b/board/ti/beagle/beagle.c
@@ -34,9 +34,11 @@
 #include <status_led.h>
 #endif
 #include <twl4030.h>
+#include <linux/mtd/nand.h>
 #include <asm/io.h>
 #include <asm/arch/mmc_host_def.h>
 #include <asm/arch/mux.h>
+#include <asm/arch/mem.h>
 #include <asm/arch/sys_proto.h>
 #include <asm/gpio.h>
 #include <asm/mach-types.h>
@@ -139,6 +141,160 @@ int get_board_revision(void)
 	return revision;
 }
 
+#ifdef CONFIG_SPL_BUILD
+
+#define MICRON_DDR	0
+#define NUMONYX_MCP	1
+#define MICRON_MCP	2
+
+#define NAND_CMD_STATUS		0x70
+#define NAND_CMD_READID		0x90
+#define NAND_CMD_RESET		0xff
+
+#define GPMC_NAND_COMMAND_0      (OMAP34XX_GPMC_BASE+0x7C)
+#define GPMC_NAND_ADDRESS_0      (OMAP34XX_GPMC_BASE+0x80)
+#define GPMC_NAND_DATA_0	 (OMAP34XX_GPMC_BASE+0x84)
+
+#define WRITE_NAND_COMMAND(d, adr) \
+	do {*(volatile u16 *)GPMC_NAND_COMMAND_0 = d;} while(0)
+#define WRITE_NAND_ADDRESS(d, adr) \
+	do {*(volatile u16 *)GPMC_NAND_ADDRESS_0 = d;} while(0)
+#define READ_NAND(adr)          (*(volatile u16 *)GPMC_NAND_DATA_0)
+
+/* nand_command: Send a flash command to the flash chip */
+static void nand_command(unsigned char command)
+{
+ 	WRITE_NAND_COMMAND(command, NAND_ADDR);
+
+  	if (command == NAND_CMD_RESET) {
+		unsigned char ret_val;
+		nand_command(NAND_CMD_STATUS);
+		do {
+			ret_val = READ_NAND(NAND_ADDR);/* wait till ready */
+  		} while ((ret_val & 0x40) != 0x40);
+ 	}
+}
+
+/* 
+ * In order to find out what DDR we have we need to see what NAND we
+ * may have.  This relies on having already initalized GPMC earlier
+ * in the sequence.
+ */
+static void nand_readid(int *mfr, int *id)
+{
+ 	nand_command(NAND_CMD_RESET);
+ 	nand_command(NAND_CMD_READID);
+
+	WRITE_NAND_ADDRESS(0x0, NAND_ADDR);
+
+	/* Read off the manufacturer and device id. */
+	*mfr = READ_NAND(NAND_ADDR);
+	*id = READ_NAND(NAND_ADDR);
+}
+
+#define GPMC_CONFIG_CS0_CONFIG1		0x6E000060
+#define GPMC_CONFIG_CS0_CONFIG2		0x6E000064
+#define GPMC_CONFIG_CS0_CONFIG3		0x6E000068
+#define GPMC_CONFIG_CS0_CONFIG4		0x6E00006C
+#define GPMC_CONFIG_CS0_CONFIG5		0x6E000070
+#define GPMC_CONFIG_CS0_CONFIG6		0x6E000074
+#define GPMC_CONFIG_CS0_CONFIG7		0x6E000078
+#define OMAP34XX_GPMC_CS0_SIZE		0x8
+
+static int identify_xm_ddr(void)
+{
+	int mfr, id;
+
+	/* Make sure that we have setup GPMC for NAND correctly. */
+	writel(M_NAND_GPMC_CONFIG1, GPMC_CONFIG_CS0_CONFIG1);
+	writel(M_NAND_GPMC_CONFIG2, GPMC_CONFIG_CS0_CONFIG2);
+	writel(M_NAND_GPMC_CONFIG3, GPMC_CONFIG_CS0_CONFIG3);
+	writel(M_NAND_GPMC_CONFIG4, GPMC_CONFIG_CS0_CONFIG4);
+	writel(M_NAND_GPMC_CONFIG5, GPMC_CONFIG_CS0_CONFIG5);
+	writel(M_NAND_GPMC_CONFIG6, GPMC_CONFIG_CS0_CONFIG6);
+
+	/* Enable the GPMC Mapping */
+	writel((((OMAP34XX_GPMC_CS0_SIZE & 0xF) << 8) |
+			     ((NAND_BASE >> 24) & 0x3F) |
+			     (1 << 6)),  (GPMC_CONFIG_CS0_CONFIG7));
+
+	sdelay(2000);
+
+	nand_readid(&mfr, &id);
+	if (mfr == 0)
+		return MICRON_DDR;
+	if ((mfr == 0x20) && (id == 0xba))
+		return NUMONYX_MCP;
+	if ((mfr == 0x2c) && (id == 0xbc))
+		return MICRON_MCP;
+
+	/* Unknown. */
+	return -1;
+}
+
+/* 
+ * Routine: board_early_sdrc_init
+ * Description: If we use SPL then there is no x-loader nor config header
+ * so we have to setup the DDR timings outself on both banks.
+ */
+void get_board_mem_timings(u32 *cs_cfg, u32 *mcfg, u32 *ctrla, u32 *ctrlb,
+		u32 *rfr_ctrl, u32 *mr)
+{
+	/* TODO XXX REWORD */
+	/* We have magic hard coded values here for V_MCFG which come from
+	 * x-loader as they do not match how the OMAP35x TRM says to
+	 * calculate them values. */
+	*mr = MICRON_V_MR;
+	switch (get_board_revision()) {
+	case REVISION_C4:
+		if (identify_xm_ddr() == NUMONYX_MCP) {
+			*cs_cfg = 0x4;
+			*mcfg = 0x04590099;
+			*ctrla = NUMONYX_V_ACTIMA_165;
+			*ctrlb = NUMONYX_V_ACTIMB_165;
+			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+		} else if (identify_xm_ddr() == MICRON_MCP) {
+			/* Beagleboard Rev C5 */
+			*cs_cfg = 0x2;
+			*mcfg = 0x03588099;
+			*ctrla = MICRON_V_ACTIMA_200;
+			*ctrlb = MICRON_V_ACTIMB_200;
+			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_200MHz;
+		} else {
+			*cs_cfg = 0x1;
+			*mcfg = 0x02584099;
+			*ctrla = MICRON_V_ACTIMA_165;
+			*ctrlb = MICRON_V_ACTIMB_165;
+			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+		}
+		break;
+	case REVISION_XM_A:
+	case REVISION_XM_B:
+	case REVISION_XM_C:
+		if (identify_xm_ddr() == MICRON_DDR) {
+			*cs_cfg = 0x2;
+			*mcfg = 0x03588099;
+			*ctrla = MICRON_V_ACTIMA_200;
+			*ctrlb = MICRON_V_ACTIMB_200;
+			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_200MHz;
+		} else {
+			*cs_cfg = 0x4;
+			*mcfg = 0x04590099;
+			*ctrla = NUMONYX_V_ACTIMA_165;
+			*ctrlb = NUMONYX_V_ACTIMB_165;
+			*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+		}
+		break;
+	default:
+		*cs_cfg = 0x1;
+		*mcfg = 0x02584099;
+		*ctrla = MICRON_V_ACTIMA_165;
+		*ctrlb = MICRON_V_ACTIMB_165;
+		*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+	}
+}
+#endif
+
 /*
  * Routine: get_expansion_id
  * Description: This function checks for expansion board by checking I2C
@@ -371,7 +527,7 @@ void set_muxconf_regs(void)
 	MUX_BEAGLE();
 }
 
-#ifdef CONFIG_GENERIC_MMC
+#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD)
 int board_mmc_init(bd_t *bis)
 {
 	omap_mmc_init(0);
@@ -480,6 +636,7 @@ int ehci_hcd_init(void)
 
 #endif /* CONFIG_USB_EHCI */
 
+#ifndef CONFIG_SPL_BUILD
 /*
  * This command returns the status of the user button on beagle xM
  * Input - none
@@ -534,3 +691,4 @@ U_BOOT_CMD(
 	"Return the status of the BeagleBoard USER button",
 	""
 );
+#endif
diff --git a/board/ti/beagle/config.mk b/board/ti/beagle/config.mk
deleted file mode 100644
index cf055db..0000000
--- a/board/ti/beagle/config.mk
+++ /dev/null
@@ -1,33 +0,0 @@
-#
-# (C) Copyright 2006
-# Texas Instruments, <www.ti.com>
-#
-# Beagle Board uses OMAP3 (ARM-CortexA8) cpu
-# see http://www.ti.com/ for more information on Texas Instruments
-#
-# See file CREDITS for list of people who contributed to this
-# project.
-#
-# 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
-#
-# Physical Address:
-# 8000'0000 (bank0)
-# A000/0000 (bank1)
-# Linux-Kernel is expected to be at 8000'8000, entry 8000'8000
-# (mem base + reserved)
-
-# For use with external or internal boots.
-CONFIG_SYS_TEXT_BASE = 0x80008000
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index a026478..a126566 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -347,7 +347,7 @@
  */
 #define CONFIG_NR_DRAM_BANKS	2	/* CS1 may or may not be populated */
 #define PHYS_SDRAM_1		OMAP34XX_SDRC_CS0
-#define PHYS_SDRAM_1_SIZE	(32 << 20)	/* at least 32 MiB */
+#define PHYS_SDRAM_1_SIZE	(32 << 20)	/* At least 32 MiB */
 #define PHYS_SDRAM_2		OMAP34XX_SDRC_CS1
 
 /* SDRAM Bank Allocation method */
@@ -390,4 +390,62 @@
 
 #define CONFIG_OMAP3_SPI
 
+/* Defines for SPL */
+#define CONFIG_SPL
+#define CONFIG_SPL_NAND_SIMPLE
+#define CONFIG_SPL_TEXT_BASE		0x40200800
+#define CONFIG_SPL_MAX_SIZE		(45 * 1024)
+#define CONFIG_SPL_STACK		LOW_LEVEL_SRAM_STACK
+
+#define CONFIG_SPL_BSS_START_ADDR	0x80000000
+#define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */
+
+#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR	0x300 /* address 0x60000 */
+#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS	0x200 /* 256 KB */
+#define CONFIG_SYS_MMC_SD_FAT_BOOT_PARTITION	1
+#define CONFIG_SPL_FAT_LOAD_PAYLOAD_NAME	"u-boot.img"
+
+#define CONFIG_SPL_LIBCOMMON_SUPPORT
+#define CONFIG_SPL_LIBDISK_SUPPORT
+#define CONFIG_SPL_I2C_SUPPORT
+#define CONFIG_SPL_LIBGENERIC_SUPPORT
+#define CONFIG_SPL_MMC_SUPPORT
+#define CONFIG_SPL_FAT_SUPPORT
+#define CONFIG_SPL_SERIAL_SUPPORT
+#define CONFIG_SPL_NAND_SUPPORT
+#define CONFIG_SPL_POWER_SUPPORT
+#define CONFIG_SPL_LDSCRIPT		"$(CPUDIR)/omap-common/u-boot-spl.lds"
+
+/* NAND boot config */
+#define CONFIG_SYS_NAND_5_ADDR_CYCLE
+#define CONFIG_SYS_NAND_PAGE_COUNT	64
+#define CONFIG_SYS_NAND_PAGE_SIZE	2048
+#define CONFIG_SYS_NAND_OOBSIZE		64
+#define CONFIG_SYS_NAND_BLOCK_SIZE	(128*1024)
+#define CONFIG_SYS_NAND_BAD_BLOCK_POS	0
+#define CONFIG_SYS_NAND_ECCPOS		{2, 3, 4, 5, 6, 7, 8, 9,\
+						10, 11, 12, 13}
+
+#define CONFIG_SYS_NAND_ECCSIZE		512
+#define CONFIG_SYS_NAND_ECCBYTES	3
+
+#define CONFIG_SYS_NAND_ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
+						CONFIG_SYS_NAND_ECCSIZE)
+#define CONFIG_SYS_NAND_ECCTOTAL       (CONFIG_SYS_NAND_ECCBYTES * \
+						CONFIG_SYS_NAND_ECCSTEPS)
+
+#define CONFIG_SYS_NAND_U_BOOT_START   CONFIG_SYS_TEXT_BASE
+
+#define CONFIG_SYS_NAND_U_BOOT_OFFS	0x80000
+
+/*
+ * 1MB into the SDRAM to allow for SPL's bss@the beginning of SDRAM
+ * 64 bytes before this address should be set aside for u-boot.img's
+ * header. That is 0x800FFFC0--0x80100000 should not be used for any
+ * other needs.
+ */
+#define CONFIG_SYS_TEXT_BASE		0x80100000
+#define CONFIG_SYS_SPL_MALLOC_START	0x80208000
+#define CONFIG_SYS_SPL_MALLOC_SIZE	0x100000
+
 #endif /* __CONFIG_H */
-- 
1.7.0.4