[U-Boot] [PATCH RFT 0/2] Beagleboard SPL support

[U-Boot] [PATCH RFT 0/2] Beagleboard SPL support

[Tom Rini]

Hey all,

I'm looking for more testers for the following series of patches.
This switches Beagleboard from using x-loader to load U-Boot to
using SPL to provide that loader.  This depends on:
http://patchwork.ozlabs.org/patch/117648/ which is in u-boot-arm
and for devkit8000:
http://patchwork.ozlabs.org/patch/120500/ which is in u-boot-arm
and
http://patchwork.ozlabs.org/patch/121958/ which I posted earlier today.

For convenience I've placed all of this into:
https://github.com/trini/u-boot/tree/rft-beagle-spl-testing-oct26

I'm also looking for feedback on the memory initalization hook.  In the
near term I think we need to take a real good look at what's (not)
happening in the memory initalization code we have today and maybe
do some rework / pruneing and say if you're seeing problems afterwards
it's time to switch your board to SPL.  But I think that's a little
outside the scope of what needs to happen first, which is converting
as many boards to SPL as we can.  And I think for the most part this
is a mechanical process now (figure out what magic x-loader, or
whatever, does for memory timing values, fill those in to the hook).

-- 
Tom

[U-Boot] [PATCH 1/2] OMAP3 SPL: Rework memory initalization and devkit 8000 support

[Tom Rini]

This changes to making the board be responsible for providing the
memory initialization timings in SPL and converts the devkit 8000
to this framework.  As part of this suffix the Micron DDR settings
with their speed and add a few more timing values that will be needed.
We also make sure that in mem_ok() we clear the values off as we may be
testing the same banks multiple times.

Signed-off-by: Tom Rini <trini@ti.com>
---
 arch/arm/cpu/armv7/omap3/mem.c              |    1 +
 arch/arm/cpu/armv7/omap3/sdrc.c             |  132 +++++++++++++++------------
 arch/arm/include/asm/arch-omap3/mem.h       |   58 +++++-------
 arch/arm/include/asm/arch-omap3/sys_proto.h |    2 +-
 board/timll/devkit8000/devkit8000.c         |   24 +++++
 5 files changed, 123 insertions(+), 94 deletions(-)

diff --git a/arch/arm/cpu/armv7/omap3/mem.c b/arch/arm/cpu/armv7/omap3/mem.c
index a01c303..cd5fe5c 100644
--- a/arch/arm/cpu/armv7/omap3/mem.c
+++ b/arch/arm/cpu/armv7/omap3/mem.c
@@ -86,6 +86,7 @@ u32 mem_ok(u32 cs)
 	writel(0x0, addr + 4);		/* remove pattern off the bus */
 	val1 = readl(addr + 0x400);	/* get pos A value */
 	val2 = readl(addr);		/* get val2 */
+	writel(0x0, addr + 0x400);	/* clear pos A */
 
 	if ((val1 != 0) || (val2 != pattern))	/* see if pos A val changed */
 		return 0;
diff --git a/arch/arm/cpu/armv7/omap3/sdrc.c b/arch/arm/cpu/armv7/omap3/sdrc.c
index 0dd1955..4799787 100644
--- a/arch/arm/cpu/armv7/omap3/sdrc.c
+++ b/arch/arm/cpu/armv7/omap3/sdrc.c
@@ -109,15 +109,56 @@ u32 get_sdr_cs_offset(u32 cs)
 }
 
 /*
+ * write_sdrc_timings -
+ *  - Takes CS and associated timings and initalize SDRAM
+ *  - Test CS to make sure it's OK for use
+ */
+static void write_sdrc_timings(u32 cs, struct sdrc_actim *sdrc_actim_base,
+		u32 mcfg, u32 ctrla, u32 ctrlb, u32 rfr_ctrl, u32 mr)
+{
+	/* Setup timings we got from the board. */
+	writel(mcfg, &sdrc_base->cs[cs].mcfg);
+	writel(ctrla, &sdrc_actim_base->ctrla);
+	writel(ctrlb, &sdrc_actim_base->ctrlb);
+	writel(rfr_ctrl, &sdrc_base->cs[cs].rfr_ctrl);
+	writel(CMD_NOP, &sdrc_base->cs[cs].manual);
+	writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual);
+	writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
+	writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
+	writel(mr, &sdrc_base->cs[cs].mr);
+
+	/*
+	 * Test ram in this bank
+	 * Disable if bad or not present
+	 */
+	if (!mem_ok(cs))
+		writel(0, &sdrc_base->cs[cs].mcfg);
+}
+
+/*
  * do_sdrc_init -
- *  - Initialize the SDRAM for use.
- *  - code called once in C-Stack only context for CS0 and a possible 2nd
- *    time depending on memory configuration from stack+global context
+ *  - Code called once in C-Stack only context for CS0 and with early being
+ *    true and a possible 2nd time depending on memory configuration from
+ *    stack+global context.
  */
 void do_sdrc_init(u32 cs, u32 early)
 {
-	struct sdrc_actim *sdrc_actim_base0, *sdrc_actim_base1;
+	struct sdrc_actim *sdrc_actim_base0;
+	u32 mcfg, ctrla, ctrlb, rfr_ctrl, mr;
+#ifdef CONFIG_SPL_BUILD
+	u32 cs_cfg;
+#endif
 
+	/*
+	 * When called in the early context this may be SPL and we will
+	 * need to set all of the timings.  This ends up being board
+	 * specific so we call a helper function to take care of this
+	 * for us.  Otherwise, to be safe, we need to copy the settings
+	 * from the first bank to the second.
+	 */
+#ifdef CONFIG_SPL_BUILD
+	get_board_mem_timings(&cs_cfg, &mcfg, &ctrla, &ctrlb, &rfr_ctrl, &mr);
+#endif
 	if (early) {
 		/* reset sdrc controller */
 		writel(SOFTRESET, &sdrc_base->sysconfig);
@@ -128,73 +169,45 @@ void do_sdrc_init(u32 cs, u32 early)
 		/* setup sdrc to ball mux */
 		writel(SDRC_SHARING, &sdrc_base->sharing);
 
-		/* Disable Power Down of CKE cuz of 1 CKE on combo part */
+		/* Disable Power Down of CKE because of 1 CKE on combo part */
 		writel(WAKEUPPROC | SRFRONRESET | PAGEPOLICY_HIGH,
 				&sdrc_base->power);
 
 		writel(ENADLL | DLLPHASE_90, &sdrc_base->dlla_ctrl);
 		sdelay(0x20000);
-	}
-
-/* As long as V_MCFG and V_RFR_CTRL is not defined for all OMAP3 boards we need
- * to prevent this to be build in non-SPL build */
 #ifdef CONFIG_SPL_BUILD
-	/* If we use a SPL there is no x-loader nor config header so we have
-	 * to do the job ourselfs
-	 */
-	if (cs == CS0) {
-		sdrc_actim_base0 = (struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE;
-
-		/* General SDRC config */
-		writel(V_MCFG, &sdrc_base->cs[cs].mcfg);
-		writel(V_RFR_CTRL, &sdrc_base->cs[cs].rfr_ctrl);
-
-		/* AC timings */
-		writel(V_ACTIMA_165, &sdrc_actim_base0->ctrla);
-		writel(V_ACTIMB_165, &sdrc_actim_base0->ctrlb);
-
-		/* Initialize */
-		writel(CMD_NOP, &sdrc_base->cs[cs].manual);
-		writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual);
-		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
-		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
+		/* We only set cs_cfg in this case */
+		writel(cs_cfg, &sdrc_base->cs_cfg);
+
+		/* We need to do both banks now, in many cases. */
+		write_sdrc_timings(CS0,
+				(struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE,
+				mcfg, ctrla, ctrlb, rfr_ctrl, mr);
+		write_sdrc_timings(CS1,
+				(struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE,
+				mcfg, ctrla, ctrlb, rfr_ctrl, mr);
+#endif
 
-		writel(V_MR, &sdrc_base->cs[cs].mr);
 	}
-#endif
 
 	/*
-	 * SDRC timings are set up by x-load or config header
-	 * We don't need to redo them here.
-	 * Older x-loads configure only CS0
-	 * configure CS1 to handle this ommission
+	 * If we aren't using SPL we have been loaded by some
+	 * other means which may not have correctly initialized
+	 * both CS0 and CS1 (such as some older versions of x-loader)
+	 * so we may be asked now to setup CS1.
 	 */
 	if (cs == CS1) {
 		sdrc_actim_base0 = (struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE;
-		sdrc_actim_base1 = (struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE;
-		writel(readl(&sdrc_base->cs[CS0].mcfg),
-			&sdrc_base->cs[CS1].mcfg);
-		writel(readl(&sdrc_base->cs[CS0].rfr_ctrl),
-			&sdrc_base->cs[CS1].rfr_ctrl);
-		writel(readl(&sdrc_actim_base0->ctrla),
-			&sdrc_actim_base1->ctrla);
-		writel(readl(&sdrc_actim_base0->ctrlb),
-			&sdrc_actim_base1->ctrlb);
-
-		writel(CMD_NOP, &sdrc_base->cs[cs].manual);
-		writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual);
-		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
-		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
-		writel(readl(&sdrc_base->cs[CS0].mr),
-			&sdrc_base->cs[CS1].mr);
-	}
+		mcfg = readl(&sdrc_base->cs[CS0].mcfg),
+		rfr_ctrl = readl(&sdrc_base->cs[CS0].rfr_ctrl);
+		ctrla = readl(&sdrc_actim_base0->ctrla),
+		ctrlb = readl(&sdrc_actim_base0->ctrlb);
+		mr = readl(&sdrc_base->cs[CS0].mr);
+		write_sdrc_timings(cs,
+				(struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE,
+				mcfg, ctrla, ctrlb, rfr_ctrl, mr);
 
-	/*
-	 * Test ram in this bank
-	 * Disable if bad or not present
-	 */
-	if (!mem_ok(cs))
-		writel(0, &sdrc_base->cs[cs].mcfg);
+	}
 }
 
 /*
@@ -208,8 +221,9 @@ int dram_init(void)
 	size0 = get_sdr_cs_size(CS0);
 	/*
 	 * If a second bank of DDR is attached to CS1 this is
-	 * where it can be started.  Early init code will init
-	 * memory on CS0.
+	 * where it can be found.  If we have SPL that code will have
+	 * initalized it already, otherwise early init code will init
+	 * memory on CS0 only.
 	 */
 	if ((sysinfo.mtype == DDR_COMBO) || (sysinfo.mtype == DDR_STACKED)) {
 		do_sdrc_init(CS1, NOT_EARLY);
diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
index 8e28f77..af3504c 100644
--- a/arch/arm/include/asm/arch-omap3/mem.h
+++ b/arch/arm/include/asm/arch-omap3/mem.h
@@ -37,12 +37,28 @@ enum {
 };
 #endif /* __ASSEMBLY__ */
 
+/* Memory that can be connected to GPMC */
+#define GPMC_NOR            0
+#define GPMC_NAND           1
+#define GPMC_MDOC           2
+#define GPMC_ONENAND        3
+#define MMC_NAND            4
+#define MMC_ONENAND         5
+#define GPMC_NONE           6
+#define GPMC_ONENAND_TRY    7
+
 #define EARLY_INIT	1
 
 /* Slower full frequency range default timings for x32 operation*/
 #define SDRC_SHARING	0x00000100
 #define SDRC_MR_0_SDR	0x00000031
 
+/* optimized timings good for current shipping parts */
+#define SDP_3430_SDRC_RFR_CTRL_100MHz   0x0002da01
+#define SDP_3430_SDRC_RFR_CTRL_133MHz   0x0003de01 /* 7.8us/7.5ns - 50=0x3de */
+#define SDP_3430_SDRC_RFR_CTRL_165MHz   0x0004e201 /* 7.8us/6ns - 50=0x4e2 */
+#define SDP_3430_SDRC_RFR_CTRL_200MHz   0x0005e601 /* 7.8us/5ns - 50=0x5e6 */
+
 #define DLL_OFFSET		0
 #define DLL_WRITEDDRCLKX2DIS	1
 #define DLL_ENADLL		1
@@ -138,15 +154,15 @@ enum {
 #define MICRON_CASWIDTH			0x5
 #define MICRON_RASWIDTH			0x2
 #define MICRON_LOCKSTATUS		0x0
-#define MICRON_V_MCFG ((MICRON_LOCKSTATUS << 30) | (MICRON_RASWIDTH << 24) | \
-	(MICRON_CASWIDTH << 20) | (MICRON_ADDRMUXLEGACY << 19) | \
-	(MICRON_RAMSIZE << 8) | (MICRON_BANKALLOCATION << 6) | \
-	(MICRON_B32NOT16 << 4) | (MICRON_DEEPPD << 3) | \
-	(MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE))
+#define MICRON_V_MCFG_165 		((MICRON_LOCKSTATUS << 30) | \
+	(MICRON_RASWIDTH << 24) | (MICRON_CASWIDTH << 20) | \
+	(MICRON_ADDRMUXLEGACY << 19) | (MICRON_RAMSIZE << 8) | \
+	(MICRON_BANKALLOCATION << 6) | (MICRON_B32NOT16 << 4) | \
+	(MICRON_DEEPPD << 3) | (MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE))
 
-#define MICRON_ARCV				2030
-#define MICRON_ARE				0x1
-#define MICRON_V_RFR_CTRL ((MICRON_ARCV << 8) | (MICRON_ARE))
+#define MICRON_ARCV_165		0x4e2
+#define MICRON_ARE		0x1
+#define MICRON_V_RFR_CTRL_165	((MICRON_ARCV_165 << 8) | (MICRON_ARE))
 
 #define MICRON_BL				0x2
 #define MICRON_SIL				0x0
@@ -194,32 +210,6 @@ enum {
 		(NUMONYX_XSR_165 << 0) | (NUMONYX_TXP_165 << 8) | \
 		(NUMONYX_TWTR_165 << 16))
 
-#ifdef CONFIG_OMAP3_INFINEON_DDR
-#define V_ACTIMA_165 INFINEON_V_ACTIMA_165
-#define V_ACTIMB_165 INFINEON_V_ACTIMB_165
-#endif
-
-#ifdef CONFIG_OMAP3_MICRON_DDR
-#define V_ACTIMA_165 MICRON_V_ACTIMA_165
-#define V_ACTIMB_165 MICRON_V_ACTIMB_165
-#define V_MCFG			MICRON_V_MCFG
-#define V_RFR_CTRL		MICRON_V_RFR_CTRL
-#define V_MR			MICRON_V_MR
-#endif
-
-#ifdef CONFIG_OMAP3_NUMONYX_DDR
-#define V_ACTIMA_165 NUMONYX_V_ACTIMA_165
-#define V_ACTIMB_165 NUMONYX_V_ACTIMB_165
-#endif
-
-#if !defined(V_ACTIMA_165) || !defined(V_ACTIMB_165)
-#error "Please choose the right DDR type in config header"
-#endif
-
-#if defined(CONFIG_SPL_BUILD) && (!defined(V_MCFG) || !defined(V_RFR_CTRL))
-#error "Please choose the right DDR type in config header"
-#endif
-
 /*
  * GPMC settings -
  * Definitions is as per the following format
diff --git a/arch/arm/include/asm/arch-omap3/sys_proto.h b/arch/arm/include/asm/arch-omap3/sys_proto.h
index 7b60051..a2c317a 100644
--- a/arch/arm/include/asm/arch-omap3/sys_proto.h
+++ b/arch/arm/include/asm/arch-omap3/sys_proto.h
@@ -38,6 +38,7 @@ void per_clocks_enable(void);
 void memif_init(void);
 void sdrc_init(void);
 void do_sdrc_init(u32, u32);
+void get_board_mem_timings(u32 *, u32 *, u32 *, u32 *, u32 *, u32 *);
 void emif4_init(void);
 void gpmc_init(void);
 void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
@@ -49,7 +50,6 @@ void set_muxconf_regs(void);
 u32 get_cpu_family(void);
 u32 get_cpu_rev(void);
 u32 get_sku_id(void);
-u32 get_mem_type(void);
 u32 get_sysboot_value(void);
 u32 is_gpmc_muxed(void);
 u32 get_gpmc0_type(void);
diff --git a/board/timll/devkit8000/devkit8000.c b/board/timll/devkit8000/devkit8000.c
index f50d113..568f02e 100644
--- a/board/timll/devkit8000/devkit8000.c
+++ b/board/timll/devkit8000/devkit8000.c
@@ -138,3 +138,27 @@ int board_eth_init(bd_t *bis)
 	return dm9000_initialize(bis);
 }
 #endif
+
+/* 
+ * Routine: get_board_mem_timings
+ * Description: If we use SPL then there is no x-loader nor config header
+ * so we have to setup the DDR timings outself on the first bank.  This
+ * provides the timing values back to the function that configures
+ * the memory.
+ */
+void get_board_mem_timings(u32 *cs_cfg, u32 *mcfg, u32 *ctrla, u32 *ctrlb,
+		u32 *rfr_ctrl, u32 *mr)
+{
+	/* 128MiB/bank */
+	*cs_cfg = 0x1;
+
+	/* General SDRC config */
+	*mcfg = MICRON_V_MCFG_165;
+	*rfr_ctrl = MICRON_V_RFR_CTRL_165;
+
+	/* AC timings */
+	*ctrla = MICRON_V_ACTIMA_165;
+	*ctrlb = MICRON_V_ACTIMB_165;
+
+	*mr = MICRON_V_MR;
+}
-- 
1.7.0.4

[U-Boot] [PATCH 1/2] OMAP3 SPL: Rework memory initalization and devkit 8000 support

[Igor Grinberg]

Hi Tom,

That seems like a good change, though I would recommend to split it into
smaller
patches, please, see below:

On 10/26/2011 11:13 PM, Tom Rini wrote:
> This changes to making the board be responsible for providing the
> memory initialization timings in SPL

That probably would be one patch.

>  and converts the devkit 8000
> to this framework.

That, would be another one.

>   As part of this suffix the Micron DDR settings
> with their speed

Next patch.

>  and add a few more timing values that will be needed.

This can go along with the patch that uses those settings.

> We also make sure that in mem_ok() we clear the values off as we may be
> testing the same banks multiple times.

That's should be another patch (although it has only one line).

> Signed-off-by: Tom Rini <trini@ti.com>
> ---
>  arch/arm/cpu/armv7/omap3/mem.c              |    1 +
>  arch/arm/cpu/armv7/omap3/sdrc.c             |  132 +++++++++++++++------------
>  arch/arm/include/asm/arch-omap3/mem.h       |   58 +++++-------
>  arch/arm/include/asm/arch-omap3/sys_proto.h |    2 +-
>  board/timll/devkit8000/devkit8000.c         |   24 +++++
>  5 files changed, 123 insertions(+), 94 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/omap3/mem.c b/arch/arm/cpu/armv7/omap3/mem.c
> index a01c303..cd5fe5c 100644
> --- a/arch/arm/cpu/armv7/omap3/mem.c
> +++ b/arch/arm/cpu/armv7/omap3/mem.c
> @@ -86,6 +86,7 @@ u32 mem_ok(u32 cs)
>  	writel(0x0, addr + 4);		/* remove pattern off the bus */
>  	val1 = readl(addr + 0x400);	/* get pos A value */
>  	val2 = readl(addr);		/* get val2 */
> +	writel(0x0, addr + 0x400);	/* clear pos A */
>  
>  	if ((val1 != 0) || (val2 != pattern))	/* see if pos A val changed */
>  		return 0;
> diff --git a/arch/arm/cpu/armv7/omap3/sdrc.c b/arch/arm/cpu/armv7/omap3/sdrc.c
> index 0dd1955..4799787 100644
> --- a/arch/arm/cpu/armv7/omap3/sdrc.c
> +++ b/arch/arm/cpu/armv7/omap3/sdrc.c
> @@ -109,15 +109,56 @@ u32 get_sdr_cs_offset(u32 cs)
>  }
>  
>  /*
> + * write_sdrc_timings -
> + *  - Takes CS and associated timings and initalize SDRAM
> + *  - Test CS to make sure it's OK for use
> + */
> +static void write_sdrc_timings(u32 cs, struct sdrc_actim *sdrc_actim_base,
> +		u32 mcfg, u32 ctrla, u32 ctrlb, u32 rfr_ctrl, u32 mr)
> +{
> +	/* Setup timings we got from the board. */
> +	writel(mcfg, &sdrc_base->cs[cs].mcfg);
> +	writel(ctrla, &sdrc_actim_base->ctrla);
> +	writel(ctrlb, &sdrc_actim_base->ctrlb);
> +	writel(rfr_ctrl, &sdrc_base->cs[cs].rfr_ctrl);
> +	writel(CMD_NOP, &sdrc_base->cs[cs].manual);
> +	writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual);
> +	writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
> +	writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
> +	writel(mr, &sdrc_base->cs[cs].mr);
> +
> +	/*
> +	 * Test ram in this bank
> +	 * Disable if bad or not present
> +	 */
> +	if (!mem_ok(cs))
> +		writel(0, &sdrc_base->cs[cs].mcfg);
> +}
> +
> +/*
>   * do_sdrc_init -
> - *  - Initialize the SDRAM for use.
> - *  - code called once in C-Stack only context for CS0 and a possible 2nd
> - *    time depending on memory configuration from stack+global context
> + *  - Code called once in C-Stack only context for CS0 and with early being
> + *    true and a possible 2nd time depending on memory configuration from
> + *    stack+global context.
>   */
>  void do_sdrc_init(u32 cs, u32 early)
>  {
> -	struct sdrc_actim *sdrc_actim_base0, *sdrc_actim_base1;
> +	struct sdrc_actim *sdrc_actim_base0;
> +	u32 mcfg, ctrla, ctrlb, rfr_ctrl, mr;
> +#ifdef CONFIG_SPL_BUILD
> +	u32 cs_cfg;
> +#endif
>  
> +	/*
> +	 * When called in the early context this may be SPL and we will
> +	 * need to set all of the timings.  This ends up being board
> +	 * specific so we call a helper function to take care of this
> +	 * for us.  Otherwise, to be safe, we need to copy the settings
> +	 * from the first bank to the second.
> +	 */
> +#ifdef CONFIG_SPL_BUILD
> +	get_board_mem_timings(&cs_cfg, &mcfg, &ctrla, &ctrlb, &rfr_ctrl, &mr);
> +#endif
>  	if (early) {
>  		/* reset sdrc controller */
>  		writel(SOFTRESET, &sdrc_base->sysconfig);
> @@ -128,73 +169,45 @@ void do_sdrc_init(u32 cs, u32 early)
>  		/* setup sdrc to ball mux */
>  		writel(SDRC_SHARING, &sdrc_base->sharing);
>  
> -		/* Disable Power Down of CKE cuz of 1 CKE on combo part */
> +		/* Disable Power Down of CKE because of 1 CKE on combo part */
>  		writel(WAKEUPPROC | SRFRONRESET | PAGEPOLICY_HIGH,
>  				&sdrc_base->power);
>  
>  		writel(ENADLL | DLLPHASE_90, &sdrc_base->dlla_ctrl);
>  		sdelay(0x20000);
> -	}
> -
> -/* As long as V_MCFG and V_RFR_CTRL is not defined for all OMAP3 boards we need
> - * to prevent this to be build in non-SPL build */
>  #ifdef CONFIG_SPL_BUILD
> -	/* If we use a SPL there is no x-loader nor config header so we have
> -	 * to do the job ourselfs
> -	 */
> -	if (cs == CS0) {
> -		sdrc_actim_base0 = (struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE;
> -
> -		/* General SDRC config */
> -		writel(V_MCFG, &sdrc_base->cs[cs].mcfg);
> -		writel(V_RFR_CTRL, &sdrc_base->cs[cs].rfr_ctrl);
> -
> -		/* AC timings */
> -		writel(V_ACTIMA_165, &sdrc_actim_base0->ctrla);
> -		writel(V_ACTIMB_165, &sdrc_actim_base0->ctrlb);
> -
> -		/* Initialize */
> -		writel(CMD_NOP, &sdrc_base->cs[cs].manual);
> -		writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual);
> -		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
> -		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
> +		/* We only set cs_cfg in this case */
> +		writel(cs_cfg, &sdrc_base->cs_cfg);
> +
> +		/* We need to do both banks now, in many cases. */
> +		write_sdrc_timings(CS0,
> +				(struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE,
> +				mcfg, ctrla, ctrlb, rfr_ctrl, mr);
> +		write_sdrc_timings(CS1,
> +				(struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE,
> +				mcfg, ctrla, ctrlb, rfr_ctrl, mr);
> +#endif
>  

You probably can remove that empty line.

> -		writel(V_MR, &sdrc_base->cs[cs].mr);
>  	}
> -#endif
>  
>  	/*
> -	 * SDRC timings are set up by x-load or config header
> -	 * We don't need to redo them here.
> -	 * Older x-loads configure only CS0
> -	 * configure CS1 to handle this ommission
> +	 * If we aren't using SPL we have been loaded by some
> +	 * other means which may not have correctly initialized
> +	 * both CS0 and CS1 (such as some older versions of x-loader)
> +	 * so we may be asked now to setup CS1.
>  	 */
>  	if (cs == CS1) {
>  		sdrc_actim_base0 = (struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE;
> -		sdrc_actim_base1 = (struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE;
> -		writel(readl(&sdrc_base->cs[CS0].mcfg),
> -			&sdrc_base->cs[CS1].mcfg);
> -		writel(readl(&sdrc_base->cs[CS0].rfr_ctrl),
> -			&sdrc_base->cs[CS1].rfr_ctrl);
> -		writel(readl(&sdrc_actim_base0->ctrla),
> -			&sdrc_actim_base1->ctrla);
> -		writel(readl(&sdrc_actim_base0->ctrlb),
> -			&sdrc_actim_base1->ctrlb);
> -
> -		writel(CMD_NOP, &sdrc_base->cs[cs].manual);
> -		writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual);
> -		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
> -		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
> -		writel(readl(&sdrc_base->cs[CS0].mr),
> -			&sdrc_base->cs[CS1].mr);
> -	}
> +		mcfg = readl(&sdrc_base->cs[CS0].mcfg),
> +		rfr_ctrl = readl(&sdrc_base->cs[CS0].rfr_ctrl);
> +		ctrla = readl(&sdrc_actim_base0->ctrla),
> +		ctrlb = readl(&sdrc_actim_base0->ctrlb);
> +		mr = readl(&sdrc_base->cs[CS0].mr);
> +		write_sdrc_timings(cs,
> +				(struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE,
> +				mcfg, ctrla, ctrlb, rfr_ctrl, mr);
>  

same here

> -	/*
> -	 * Test ram in this bank
> -	 * Disable if bad or not present
> -	 */
> -	if (!mem_ok(cs))
> -		writel(0, &sdrc_base->cs[cs].mcfg);
> +	}
>  }
>  
>  /*
> @@ -208,8 +221,9 @@ int dram_init(void)
>  	size0 = get_sdr_cs_size(CS0);
>  	/*
>  	 * If a second bank of DDR is attached to CS1 this is
> -	 * where it can be started.  Early init code will init
> -	 * memory on CS0.
> +	 * where it can be found.  If we have SPL that code will have

Why do you need two spaces before the if?

> +	 * initalized it already, otherwise early init code will init
> +	 * memory on CS0 only.
>  	 */
>  	if ((sysinfo.mtype == DDR_COMBO) || (sysinfo.mtype == DDR_STACKED)) {
>  		do_sdrc_init(CS1, NOT_EARLY);
> diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
> index 8e28f77..af3504c 100644
> --- a/arch/arm/include/asm/arch-omap3/mem.h
> +++ b/arch/arm/include/asm/arch-omap3/mem.h
> @@ -37,12 +37,28 @@ enum {
>  };
>  #endif /* __ASSEMBLY__ */
>  
> +/* Memory that can be connected to GPMC */
> +#define GPMC_NOR            0
> +#define GPMC_NAND           1
> +#define GPMC_MDOC           2
> +#define GPMC_ONENAND        3
> +#define MMC_NAND            4
> +#define MMC_ONENAND         5
> +#define GPMC_NONE           6
> +#define GPMC_ONENAND_TRY    7
> +

The comment is misleading.
This probably was copied from the X-Loader, and each define specifies
the boot storage devices probing order, right?

I think the above does not belong here as all the GPMC code in this file,
we have omap-gpmc.h file in this directory for the GPMC stuff,
but as the GPMC code is already here, I don't think it is right to ask
you to
move it... So volunteers are welcome...

>  #define EARLY_INIT	1
>  
>  /* Slower full frequency range default timings for x32 operation*/
>  #define SDRC_SHARING	0x00000100
>  #define SDRC_MR_0_SDR	0x00000031
>  
> +/* optimized timings good for current shipping parts */
> +#define SDP_3430_SDRC_RFR_CTRL_100MHz   0x0002da01
> +#define SDP_3430_SDRC_RFR_CTRL_133MHz   0x0003de01 /* 7.8us/7.5ns - 50=0x3de */
> +#define SDP_3430_SDRC_RFR_CTRL_165MHz   0x0004e201 /* 7.8us/6ns - 50=0x4e2 */
> +#define SDP_3430_SDRC_RFR_CTRL_200MHz   0x0005e601 /* 7.8us/5ns - 50=0x5e6 */
> +
>  #define DLL_OFFSET		0
>  #define DLL_WRITEDDRCLKX2DIS	1
>  #define DLL_ENADLL		1
> @@ -138,15 +154,15 @@ enum {
>  #define MICRON_CASWIDTH			0x5
>  #define MICRON_RASWIDTH			0x2
>  #define MICRON_LOCKSTATUS		0x0
> -#define MICRON_V_MCFG ((MICRON_LOCKSTATUS << 30) | (MICRON_RASWIDTH << 24) | \
> -	(MICRON_CASWIDTH << 20) | (MICRON_ADDRMUXLEGACY << 19) | \
> -	(MICRON_RAMSIZE << 8) | (MICRON_BANKALLOCATION << 6) | \
> -	(MICRON_B32NOT16 << 4) | (MICRON_DEEPPD << 3) | \
> -	(MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE))
> +#define MICRON_V_MCFG_165 		((MICRON_LOCKSTATUS << 30) | \

The above line should be generating a white space error.
There is a mixture of tabs and spaces.
Also, why don't you just use one tab or space?

> +	(MICRON_RASWIDTH << 24) | (MICRON_CASWIDTH << 20) | \
> +	(MICRON_ADDRMUXLEGACY << 19) | (MICRON_RAMSIZE << 8) | \
> +	(MICRON_BANKALLOCATION << 6) | (MICRON_B32NOT16 << 4) | \
> +	(MICRON_DEEPPD << 3) | (MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE))
>  
> -#define MICRON_ARCV				2030
> -#define MICRON_ARE				0x1
> -#define MICRON_V_RFR_CTRL ((MICRON_ARCV << 8) | (MICRON_ARE))
> +#define MICRON_ARCV_165		0x4e2
> +#define MICRON_ARE		0x1

The above values are not aligned with all the others.

> +#define MICRON_V_RFR_CTRL_165	((MICRON_ARCV_165 << 8) | (MICRON_ARE))

The MICRON_ARE macro does not need a parenthesis.

>  
>  #define MICRON_BL				0x2
>  #define MICRON_SIL				0x0
> @@ -194,32 +210,6 @@ enum {
>  		(NUMONYX_XSR_165 << 0) | (NUMONYX_TXP_165 << 8) | \
>  		(NUMONYX_TWTR_165 << 16))
>  
> -#ifdef CONFIG_OMAP3_INFINEON_DDR
> -#define V_ACTIMA_165 INFINEON_V_ACTIMA_165
> -#define V_ACTIMB_165 INFINEON_V_ACTIMB_165
> -#endif
> -
> -#ifdef CONFIG_OMAP3_MICRON_DDR
> -#define V_ACTIMA_165 MICRON_V_ACTIMA_165
> -#define V_ACTIMB_165 MICRON_V_ACTIMB_165
> -#define V_MCFG			MICRON_V_MCFG
> -#define V_RFR_CTRL		MICRON_V_RFR_CTRL
> -#define V_MR			MICRON_V_MR
> -#endif
> -
> -#ifdef CONFIG_OMAP3_NUMONYX_DDR
> -#define V_ACTIMA_165 NUMONYX_V_ACTIMA_165
> -#define V_ACTIMB_165 NUMONYX_V_ACTIMB_165
> -#endif
> -
> -#if !defined(V_ACTIMA_165) || !defined(V_ACTIMB_165)
> -#error "Please choose the right DDR type in config header"
> -#endif
> -
> -#if defined(CONFIG_SPL_BUILD) && (!defined(V_MCFG) || !defined(V_RFR_CTRL))
> -#error "Please choose the right DDR type in config header"
> -#endif
> -
>  /*
>   * GPMC settings -
>   * Definitions is as per the following format
> diff --git a/arch/arm/include/asm/arch-omap3/sys_proto.h b/arch/arm/include/asm/arch-omap3/sys_proto.h
> index 7b60051..a2c317a 100644
> --- a/arch/arm/include/asm/arch-omap3/sys_proto.h
> +++ b/arch/arm/include/asm/arch-omap3/sys_proto.h
> @@ -38,6 +38,7 @@ void per_clocks_enable(void);
>  void memif_init(void);
>  void sdrc_init(void);
>  void do_sdrc_init(u32, u32);
> +void get_board_mem_timings(u32 *, u32 *, u32 *, u32 *, u32 *, u32 *);
>  void emif4_init(void);
>  void gpmc_init(void);
>  void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
> @@ -49,7 +50,6 @@ void set_muxconf_regs(void);
>  u32 get_cpu_family(void);
>  u32 get_cpu_rev(void);
>  u32 get_sku_id(void);
> -u32 get_mem_type(void);

I think the above will break omap2420h4 and apollon boards.
Also, how is this related to that patch?

>  u32 get_sysboot_value(void);
>  u32 is_gpmc_muxed(void);
>  u32 get_gpmc0_type(void);
> diff --git a/board/timll/devkit8000/devkit8000.c b/board/timll/devkit8000/devkit8000.c
> index f50d113..568f02e 100644
> --- a/board/timll/devkit8000/devkit8000.c
> +++ b/board/timll/devkit8000/devkit8000.c
> @@ -138,3 +138,27 @@ int board_eth_init(bd_t *bis)
>  	return dm9000_initialize(bis);
>  }
>  #endif
> +
> +/* 
> + * Routine: get_board_mem_timings
> + * Description: If we use SPL then there is no x-loader nor config header
> + * so we have to setup the DDR timings outself on the first bank.  This

s/outself on/our self from/
Also two spaces before "This".

> + * provides the timing values back to the function that configures
> + * the memory.
> + */
> +void get_board_mem_timings(u32 *cs_cfg, u32 *mcfg, u32 *ctrla, u32 *ctrlb,
> +		u32 *rfr_ctrl, u32 *mr)
> +{
> +	/* 128MiB/bank */
> +	*cs_cfg = 0x1;
> +
> +	/* General SDRC config */
> +	*mcfg = MICRON_V_MCFG_165;
> +	*rfr_ctrl = MICRON_V_RFR_CTRL_165;
> +
> +	/* AC timings */
> +	*ctrla = MICRON_V_ACTIMA_165;
> +	*ctrlb = MICRON_V_ACTIMB_165;
> +
> +	*mr = MICRON_V_MR;
> +}

This looks good.

Regards,
Igor

[U-Boot] [PATCH 1/2] OMAP3 SPL: Rework memory initalization and devkit 8000 support

[Tom Rini]

On 10/27/2011 01:46 PM, Igor Grinberg wrote:
> Hi Tom,
> 
> That seems like a good change, though I would recommend to split it into
> smaller
> patches, please, see below:
> 
> On 10/26/2011 11:13 PM, Tom Rini wrote:
>> This changes to making the board be responsible for providing the
>> memory initialization timings in SPL
> 
> That probably would be one patch.
> 
>>  and converts the devkit 8000
>> to this framework.
> 
> That, would be another one.

But breaking it up like this breaks bisect'ability as devkit8000 builds
today (well, outside of the scope of this series).

>>   As part of this suffix the Micron DDR settings
>> with their speed
> 
> Next patch.

OK.

> 
>>  and add a few more timing values that will be needed.
> 
> This can go along with the patch that uses those settings.

I'll double check where they're really neded.

>> We also make sure that in mem_ok() we clear the values off as we may be
>> testing the same banks multiple times.
> 
> That's should be another patch (although it has only one line).

Sure.  Again, the bigger picture right now I'm hoping for is:
(a) does it work on all of the other beagleboard revs?  The logic is a
little twisty but it's what x-loader does today and (b) does the overall
approach look sane to everyone?

Thanks!

-- 
Tom

[U-Boot] [PATCH 1/2] OMAP3 SPL: Rework memory initalization and devkit 8000 support

[Igor Grinberg]

On 10/27/2011 11:00 PM, Tom Rini wrote:
> On 10/27/2011 01:46 PM, Igor Grinberg wrote:
>> Hi Tom,
>>
>> That seems like a good change, though I would recommend to split it into
>> smaller
>> patches, please, see below:
>>
>> On 10/26/2011 11:13 PM, Tom Rini wrote:
>>> This changes to making the board be responsible for providing the
>>> memory initialization timings in SPL
>> That probably would be one patch.
>>
>>>  and converts the devkit 8000
>>> to this framework.
>> That, would be another one.
> But breaking it up like this breaks bisect'ability as devkit8000 builds
> today (well, outside of the scope of this series).

Well the rule for keeping bisectability was always:
1) add the new functionality
2) switch to it
3) remove the unneeded (old) one

And yes, you don't just move code to split the patch if you want
the bisectability kept, it could be more then that.

>>>   As part of this suffix the Micron DDR settings
>>> with their speed
>> Next patch.
> OK.
>
>>>  and add a few more timing values that will be needed.
>> This can go along with the patch that uses those settings.
> I'll double check where they're really neded.
>
>>> We also make sure that in mem_ok() we clear the values off as we may be
>>> testing the same banks multiple times.
>> That's should be another patch (although it has only one line).
> Sure.  Again, the bigger picture right now I'm hoping for is:
> (a) does it work on all of the other beagleboard revs?  The logic is a
> little twisty but it's what x-loader does today and (b) does the overall
> approach look sane to everyone?

a) Testing is always welcome.
    I don't have those boards, but there are many people who do.
b) Yes, I think it is pretty sane.

Regards,
Igor

[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

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

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

[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

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

[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

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

[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

[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

[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

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

[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/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: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

[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 0/2]: OMAP3 SPL updates

[Tom Rini]

Hey all,

The following short series does two things.  First, it re-works where
we do memory initalization in SPL for OMAP3 to be board-specific.
This is needed since in many cases (beagleboard, omap3evm) what timings
we set depend on what hardware we detect ourself to be running on.  Second,
along those same lines what omap_rev_string tells us is doing to be
board specific as well so provide a weak version in the common omap SPL
code and lets it be replaced as needed.

-- 
Tom

[U-Boot] [PATCH 1/2] OMAP3 SPL: Rework memory initalization and devkit 8000 support

[Tom Rini]

This changes to making the board be responsible for providing a
memory initialization function for SPL and converts the existing
function to provide these services for devkit 8000.  As part of this
suffix the Micron DDR settings with their speed.

Cc: Frederik Kriewitz <frederik@kriewitz.eu>
Signed-off-by: Tom Rini <trini@ti.com>
---
 arch/arm/cpu/armv7/omap3/sdrc.c             |   46 +++++++++------------------
 arch/arm/include/asm/arch-omap3/mem.h       |   42 +++++--------------------
 arch/arm/include/asm/arch-omap3/sys_proto.h |    1 +
 board/timll/devkit8000/devkit8000.c         |   24 ++++++++++++++
 4 files changed, 48 insertions(+), 65 deletions(-)

diff --git a/arch/arm/cpu/armv7/omap3/sdrc.c b/arch/arm/cpu/armv7/omap3/sdrc.c
index 0dd1955..7f81eb9 100644
--- a/arch/arm/cpu/armv7/omap3/sdrc.c
+++ b/arch/arm/cpu/armv7/omap3/sdrc.c
@@ -111,13 +111,21 @@ u32 get_sdr_cs_offset(u32 cs)
 /*
  * do_sdrc_init -
  *  - Initialize the SDRAM for use.
- *  - code called once in C-Stack only context for CS0 and a possible 2nd
- *    time depending on memory configuration from stack+global context
+ *  - Code called once in C-Stack only context for CS0 and with early being
+ *    true and a possible 2nd time depending on memory configuration from
+ *    stack+global context.
  */
 void do_sdrc_init(u32 cs, u32 early)
 {
-	struct sdrc_actim *sdrc_actim_base0, *sdrc_actim_base1;
+	struct sdrc_actim *sdrc_actim_base0 = (struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE;
+	struct sdrc_actim *sdrc_actim_base1 = (struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE;
 
+	/*
+	 * When called in the early context this may be SPL and we will
+	 * need to set all of the timings.  This ends up being board
+	 * specific so we call a helper function to take care of this
+	 * for us.
+	 */
 	if (early) {
 		/* reset sdrc controller */
 		writel(SOFTRESET, &sdrc_base->sysconfig);
@@ -128,40 +136,18 @@ void do_sdrc_init(u32 cs, u32 early)
 		/* setup sdrc to ball mux */
 		writel(SDRC_SHARING, &sdrc_base->sharing);
 
-		/* Disable Power Down of CKE cuz of 1 CKE on combo part */
+		/* Disable Power Down of CKE because of 1 CKE on combo part */
 		writel(WAKEUPPROC | SRFRONRESET | PAGEPOLICY_HIGH,
 				&sdrc_base->power);
 
 		writel(ENADLL | DLLPHASE_90, &sdrc_base->dlla_ctrl);
 		sdelay(0x20000);
-	}
 
-/* As long as V_MCFG and V_RFR_CTRL is not defined for all OMAP3 boards we need
- * to prevent this to be build in non-SPL build */
 #ifdef CONFIG_SPL_BUILD
-	/* If we use a SPL there is no x-loader nor config header so we have
-	 * to do the job ourselfs
-	 */
-	if (cs == CS0) {
-		sdrc_actim_base0 = (struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE;
-
-		/* General SDRC config */
-		writel(V_MCFG, &sdrc_base->cs[cs].mcfg);
-		writel(V_RFR_CTRL, &sdrc_base->cs[cs].rfr_ctrl);
-
-		/* AC timings */
-		writel(V_ACTIMA_165, &sdrc_actim_base0->ctrla);
-		writel(V_ACTIMB_165, &sdrc_actim_base0->ctrlb);
-
-		/* Initialize */
-		writel(CMD_NOP, &sdrc_base->cs[cs].manual);
-		writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual);
-		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
-		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
-
-		writel(V_MR, &sdrc_base->cs[cs].mr);
-	}
+		/* setup timings */
+		board_early_sdrc_init(sdrc_base, sdrc_actim_base0);
 #endif
+	}
 
 	/*
 	 * SDRC timings are set up by x-load or config header
@@ -170,8 +156,6 @@ void do_sdrc_init(u32 cs, u32 early)
 	 * configure CS1 to handle this ommission
 	 */
 	if (cs == CS1) {
-		sdrc_actim_base0 = (struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE;
-		sdrc_actim_base1 = (struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE;
 		writel(readl(&sdrc_base->cs[CS0].mcfg),
 			&sdrc_base->cs[CS1].mcfg);
 		writel(readl(&sdrc_base->cs[CS0].rfr_ctrl),
diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
index 8e28f77..90ec573 100644
--- a/arch/arm/include/asm/arch-omap3/mem.h
+++ b/arch/arm/include/asm/arch-omap3/mem.h
@@ -138,15 +138,15 @@ enum {
 #define MICRON_CASWIDTH			0x5
 #define MICRON_RASWIDTH			0x2
 #define MICRON_LOCKSTATUS		0x0
-#define MICRON_V_MCFG ((MICRON_LOCKSTATUS << 30) | (MICRON_RASWIDTH << 24) | \
-	(MICRON_CASWIDTH << 20) | (MICRON_ADDRMUXLEGACY << 19) | \
-	(MICRON_RAMSIZE << 8) | (MICRON_BANKALLOCATION << 6) | \
-	(MICRON_B32NOT16 << 4) | (MICRON_DEEPPD << 3) | \
-	(MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE))
+#define MICRON_V_MCFG_165 		((MICRON_LOCKSTATUS << 30) | \
+	(MICRON_RASWIDTH << 24) | (MICRON_CASWIDTH << 20) | \
+	(MICRON_ADDRMUXLEGACY << 19) | (MICRON_RAMSIZE << 8) | \
+	(MICRON_BANKALLOCATION << 6) | (MICRON_B32NOT16 << 4) | \
+	(MICRON_DEEPPD << 3) | (MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE))
 
-#define MICRON_ARCV				2030
-#define MICRON_ARE				0x1
-#define MICRON_V_RFR_CTRL ((MICRON_ARCV << 8) | (MICRON_ARE))
+#define MICRON_ARCV_165		0x4e2
+#define MICRON_ARE		0x1
+#define MICRON_V_RFR_CTRL_165	((MICRON_ARCV_165 << 8) | (MICRON_ARE))
 
 #define MICRON_BL				0x2
 #define MICRON_SIL				0x0
@@ -194,32 +194,6 @@ enum {
 		(NUMONYX_XSR_165 << 0) | (NUMONYX_TXP_165 << 8) | \
 		(NUMONYX_TWTR_165 << 16))
 
-#ifdef CONFIG_OMAP3_INFINEON_DDR
-#define V_ACTIMA_165 INFINEON_V_ACTIMA_165
-#define V_ACTIMB_165 INFINEON_V_ACTIMB_165
-#endif
-
-#ifdef CONFIG_OMAP3_MICRON_DDR
-#define V_ACTIMA_165 MICRON_V_ACTIMA_165
-#define V_ACTIMB_165 MICRON_V_ACTIMB_165
-#define V_MCFG			MICRON_V_MCFG
-#define V_RFR_CTRL		MICRON_V_RFR_CTRL
-#define V_MR			MICRON_V_MR
-#endif
-
-#ifdef CONFIG_OMAP3_NUMONYX_DDR
-#define V_ACTIMA_165 NUMONYX_V_ACTIMA_165
-#define V_ACTIMB_165 NUMONYX_V_ACTIMB_165
-#endif
-
-#if !defined(V_ACTIMA_165) || !defined(V_ACTIMB_165)
-#error "Please choose the right DDR type in config header"
-#endif
-
-#if defined(CONFIG_SPL_BUILD) && (!defined(V_MCFG) || !defined(V_RFR_CTRL))
-#error "Please choose the right DDR type in config header"
-#endif
-
 /*
  * GPMC settings -
  * Definitions is as per the following format
diff --git a/arch/arm/include/asm/arch-omap3/sys_proto.h b/arch/arm/include/asm/arch-omap3/sys_proto.h
index 7b60051..7f2631e 100644
--- a/arch/arm/include/asm/arch-omap3/sys_proto.h
+++ b/arch/arm/include/asm/arch-omap3/sys_proto.h
@@ -38,6 +38,7 @@ void per_clocks_enable(void);
 void memif_init(void);
 void sdrc_init(void);
 void do_sdrc_init(u32, u32);
+void board_early_sdrc_init(struct sdrc *, struct sdrc_actim *);
 void emif4_init(void);
 void gpmc_init(void);
 void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
diff --git a/board/timll/devkit8000/devkit8000.c b/board/timll/devkit8000/devkit8000.c
index f50d113..100cc59 100644
--- a/board/timll/devkit8000/devkit8000.c
+++ b/board/timll/devkit8000/devkit8000.c
@@ -138,3 +138,27 @@ int board_eth_init(bd_t *bis)
 	return dm9000_initialize(bis);
 }
 #endif
+
+/*
+ * 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 the first bank.
+ */
+void board_early_sdrc_init(struct sdrc *sdrc_base, struct sdrc_actim *sdrc_actim_base0)
+{
+	/* General SDRC config */
+	writel(MICRON_V_MCFG_165, &sdrc_base->cs[CS0].mcfg);
+	writel(MICRON_V_RFR_CTRL_165, &sdrc_base->cs[CS0].rfr_ctrl);
+
+	/* AC timings */
+	writel(MICRON_V_ACTIMA_165, &sdrc_actim_base0->ctrla);
+	writel(MICRON_V_ACTIMB_165, &sdrc_actim_base0->ctrlb);
+
+	/* Initialize */
+	writel(CMD_NOP, &sdrc_base->cs[CS0].manual);
+	writel(CMD_PRECHARGE, &sdrc_base->cs[CS0].manual);
+	writel(CMD_AUTOREFRESH, &sdrc_base->cs[CS0].manual);
+	writel(CMD_AUTOREFRESH, &sdrc_base->cs[CS0].manual);
+
+	writel(MICRON_V_MR, &sdrc_base->cs[CS0].mr);
+}
-- 
1.7.0.4

[U-Boot] [PATCH 1/2] OMAP3 SPL: Rework memory initalization and devkit 8000 support

[Wolfgang Denk]

Dear Tom Rini,

In message <1317740363-7033-2-git-send-email-trini@ti.com> you wrote:
> This changes to making the board be responsible for providing a
> memory initialization function for SPL and converts the existing
> function to provide these services for devkit 8000.  As part of this
> suffix the Micron DDR settings with their speed.
> 
> Cc: Frederik Kriewitz <frederik@kriewitz.eu>
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
>  arch/arm/cpu/armv7/omap3/sdrc.c             |   46 +++++++++------------------
>  arch/arm/include/asm/arch-omap3/mem.h       |   42 +++++--------------------
>  arch/arm/include/asm/arch-omap3/sys_proto.h |    1 +
>  board/timll/devkit8000/devkit8000.c         |   24 ++++++++++++++
>  4 files changed, 48 insertions(+), 65 deletions(-)

Checkpatch says:

total: 0 errors, 4 warnings, 165 lines checked

Please clean up and resubmit.  Thanks.

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
The manager will be continually amazed that policies he took for com-
mon knowledge are totally unknown by some member of his  team.  Since
his fundamental job is to keep everybody going in the same direction,
his chief daily task will be communication, not decision-making.
                              - Fred Brooks, "The Mythical Man Month"