Beagleboard SPL support

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

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

arch/arm/cpu/armv7/omap3/mem.c +1 -0

index a01c303..cd5fe5c 100644
@@ -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;

arch/arm/cpu/armv7/omap3/sdrc.c +48 -28

index 0dd1955..4799787 100644
@@ -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);

arch/arm/cpu/armv7/omap3/sdrc.c +4 -22

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

arch/arm/cpu/armv7/omap3/sdrc.c +1 -6

-	/*
-	 * Test ram in this bank
-	 * Disable if bad or not present
-	 */
-	if (!mem_ok(cs))
-		writel(0, &sdrc_base->cs[cs].mcfg);
+	}
 }
 
 /*

arch/arm/cpu/armv7/omap3/sdrc.c +2 -0

+	 * 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);

arch/arm/include/asm/arch-omap3/mem.h +0 -0

index 8e28f77..af3504c 100644
@@ -37,12 +37,28 @@ enum {
 };
 #endif /* __ASSEMBLY__ */
 

arch/arm/include/asm/arch-omap3/mem.h +6 -0

 #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

arch/arm/include/asm/arch-omap3/mem.h +0 -26

 #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

arch/arm/include/asm/arch-omap3/sys_proto.h +1 -0

index 7b60051..a2c317a 100644
@@ -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,

arch/arm/include/asm/arch-omap3/sys_proto.h +0 -0

 u32 get_sysboot_value(void);
 u32 is_gpmc_muxed(void);
 u32 get_gpmc0_type(void);

board/timll/devkit8000/devkit8000.c +0 -0

index f50d113..568f02e 100644

board/timll/devkit8000/devkit8000.c +9 -0

+ * 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;
+

Hi Tom,

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

Quoted 2 lines · Tom Rini

This changes to making the board be responsible for providing the memory initialization timings in SPL

That probably would be one patch.

Quoted 2 lines

and converts the devkit 8000
to this framework.

That, would be another one.

Quoted 2 lines

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

Next patch.

Quoted 2 lines

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

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

Quoted 2 lines

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

Quoted 9 lines · Igor Grinberg

Hi Tom,

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

Quoted 2 lines · Tom Rini

This changes to making the board be responsible for providing the memory initialization timings in SPL

Quoted 5 lines

That probably would be one patch.

Quoted 2 lines

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

Quoted 3 lines

Quoted 2 lines

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

Next patch.

OK.

Quoted 2 lines

Quoted 1 line

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.

Quoted 3 lines

Quoted 2 lines

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?

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

Beagleboard xM rev C:

Tested-by Matt Ranostay <[email protected]>

Beagleboard rev B7, C2, xM rev B:

Tested-by Matt Porter <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---

board/ti/beagle/beagle.c +10 -0

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

board/ti/beagle/beagle.c +7 -0

+
+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);

Quoted 15 lines

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

Quoted 8 lines

Beagleboard rev C5, xM rev A:

Tested-by Tom Rini <[email protected]>

Beagleboard xM rev C:

Tested-by Matt Ranostay <[email protected]>

Beagleboard rev B7, C2, xM rev B:

Tested-by Matt Porter <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---

[snip]...[snip]

Quoted 11 lines

board/ti/beagle/beagle.c +10 -0

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

board/ti/beagle/beagle.c int get_board_revision(void) · line 163

Quoted 10 lines

board/ti/beagle/beagle.c +10 -0

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

board/ti/beagle/beagle.c int get_board_revision(void) · line 202

Quoted 9 lines

board/ti/beagle/beagle.c +9 -0

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

Quoted 12 lines

board/ti/beagle/beagle.c +12 -0

+
+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);

board/ti/beagle/beagle.c int get_board_revision(void) · line 219

Quoted 5 lines

board/ti/beagle/beagle.c +5 -0

+
+	/* 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.

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

Beagleboard xM rev C:

Tested-by Matt Ranostay <[email protected]>

Beagleboard rev B7, C2, xM rev B:

Tested-by Matt Porter <[email protected]>

Signed-off-by Tom Rini <[email protected]>

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

arch/arm/include/asm/arch-omap3/mem.h +4 -0

index af3504c..a784813 100644
@@ -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

arch/arm/include/asm/arch-omap3/mem.h +1 -0

+
 /*
  * NUMONYX part of IGEP v2 (165MHz optimized) 6.06ns
  *   ACTIMA

board/ti/beagle/beagle.c +5 -0

index aa5047c..a77b0cb 100644
@@ -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

board/ti/beagle/beagle.c +12 -0

+ * 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);

board/ti/beagle/beagle.c +7 -0

+
+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);

board/ti/beagle/beagle.c +42 -0

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

board/ti/beagle/beagle.c +2 -0

 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

board/ti/beagle/config.mk deleted +0 -33

deleted file mode 100644
index cf055db..0000000
@@ -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

include/configs/omap3_beagle.h +1 -1

index a026478..a126566 100644
@@ -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

include/configs/omap3_beagle.h +7 -0

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

include/configs/omap3_beagle.h +20 -0

+
+#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 */

Quoted 5 lines · 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.

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

Dear Igor Grinberg,

Quoted 1 line · In message <[email protected]> you wrote:

On 10/26/2011 11:13 PM, Tom Rini wrote:

Quoted 6 lines

Quoted 5 lines

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.

Quoted 9 lines · Wolfgang Denk

Dear Igor Grinberg,

Quoted 6 lines · In message <[email protected]> you wrote:

Quoted 5 lines · 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.

Quoted 3 lines

Quoted 1 line

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

Quoted 10 lines · Igor Grinberg

Quoted 9 lines · Wolfgang Denk

Dear Igor Grinberg,

Quoted 6 lines · In message <[email protected]> you wrote:

Quoted 5 lines · 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.

Quoted 4 lines

Quoted 3 lines

Quoted 1 line

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. :-)

Quoted 1 line

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

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

Quoted 5 lines · 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.

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

Quoted 7 lines

Beagleboard rev C5, xM rev A:

Tested-by Tom Rini <[email protected]>

Beagleboard xM rev C:

Tested-by Matt Ranostay <[email protected]>

Beagleboard rev B7, C2, xM rev B:

Tested-by Matt Porter <[email protected]>

Signed-off-by Tom Rini <[email protected]>

Quoted 7 lines

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

Quoted 9 lines

arch/arm/include/asm/arch-omap3/mem.h +0 -0

index af3504c..a784813 100644
@@ -171,6 +171,30 @@ enum {
 #define MICRON_V_MR ((MICRON_WBST << 9) | (MICRON_CASL << 4) | \
 	(MICRON_SIL << 3) | (MICRON_BL))
 

Quoted 4 lines

arch/arm/include/asm/arch-omap3/mem.h +4 -0

+
+/* Micron part (200MHz optimized) 5 ns
+  */
+#define MICRON_TDAL_200   6

arch/arm/include/asm/arch-omap3/mem.h enum { · line 187

Quoted 10 lines

arch/arm/include/asm/arch-omap3/mem.h +10 -0

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

arch/arm/include/asm/arch-omap3/mem.h enum { · line 192

Quoted 5 lines

arch/arm/include/asm/arch-omap3/mem.h +5 -0

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

arch/arm/include/asm/arch-omap3/mem.h enum {

Quoted 13 lines

arch/arm/include/asm/arch-omap3/mem.h +12 -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);

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

arch/arm/include/asm/arch-omap3/mem.h

Quoted 9 lines

arch/arm/include/asm/arch-omap3/mem.h +9 -0

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

arch/arm/include/asm/arch-omap3/mem.h

Quoted 2 lines

arch/arm/include/asm/arch-omap3/mem.h +1 -1

-#ifdef CONFIG_GENERIC_MMC
+#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD)

This should be another patch.

arch/arm/include/asm/arch-omap3/mem.h

Quoted 2 lines

arch/arm/include/asm/arch-omap3/mem.h +2 -0

+#define CONFIG_SPL_BSS_START_ADDR	0x80000000
+#define CONFIG_SPL_BSS_MAX_SIZE		0x80000		/* 512 KB */

alignment

Quoted 7 lines · Igor Grinberg

Quoted 5 lines · 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.

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

Old habit.

Quoted 14 lines

Quoted 7 lines

Beagleboard rev C5, xM rev A:

Tested-by Tom Rini <[email protected]>

Beagleboard xM rev C:

Tested-by Matt Ranostay <[email protected]>

Beagleboard rev B7, C2, xM rev B:

Tested-by Matt Porter <[email protected]>

Signed-off-by Tom Rini <[email protected]>

Quoted 7 lines

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

Quoted 2 lines

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.

Quoted 8 lines

Quoted 8 lines

arch/arm/include/asm/arch-omap3/mem.h +0 -0

index af3504c..a784813 100644
@@ -171,6 +171,30 @@ enum {
 #define MICRON_V_MR ((MICRON_WBST << 9) | (MICRON_CASL << 4) | \
 	(MICRON_SIL << 3) | (MICRON_BL))
 

Quoted 15 lines

Quoted 4 lines

arch/arm/include/asm/arch-omap3/mem.h +4 -0

+
+/* Micron part (200MHz optimized) 5 ns
+  */
+#define MICRON_TDAL_200   6

arch/arm/include/asm/arch-omap3/mem.h enum { · line 187

Quoted 10 lines

arch/arm/include/asm/arch-omap3/mem.h +10 -0

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

Quoted 8 lines

arch/arm/include/asm/arch-omap3/mem.h enum { · line 192

Quoted 5 lines

arch/arm/include/asm/arch-omap3/mem.h +5 -0

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

Quoted 6 lines

Quoted 6 lines

arch/arm/include/asm/arch-omap3/mem.h +6 -0

+
+/* nand_command: Send a flash command to the flash chip */
+static void nand_command(unsigned char command)
+{
+ 	WRITE_NAND_COMMAND(command, NAND_ADDR);
+

Quoted 8 lines

arch/arm/include/asm/arch-omap3/mem.h enum {

Quoted 6 lines

arch/arm/include/asm/arch-omap3/mem.h +6 -0

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

Quoted 10 lines

arch/arm/include/asm/arch-omap3/mem.h

Quoted 9 lines

arch/arm/include/asm/arch-omap3/mem.h +9 -0

+	*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]

Quoted 3 lines

arch/arm/include/asm/arch-omap3/mem.h

Quoted 2 lines

arch/arm/include/asm/arch-omap3/mem.h +1 -1

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

Quoted 3 lines

arch/arm/include/asm/arch-omap3/mem.h

Quoted 2 lines

arch/arm/include/asm/arch-omap3/mem.h +2 -0

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

Quoted 2 lines · Tom Rini

Quoted 1 line · Igor Grinberg

On 10/26/2011 11:13 PM, Tom Rini wrote:

Quoted 13 lines

Quoted 13 lines

board/ti/beagle/beagle.c int get_board_revision(void) · line 174

Quoted 11 lines

board/ti/beagle/beagle.c +11 -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);

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

Quoted 3 lines

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.

Quoted 3 lines · Igor Grinberg

Quoted 2 lines · Tom Rini

Quoted 1 line · Igor Grinberg

On 10/26/2011 11:13 PM, Tom Rini wrote:

[snip]

Quoted 17 lines

Quoted 16 lines

Quoted 5 lines

Quoted 5 lines

board/ti/beagle/beagle.c +5 -0

+/* nand_command: Send a flash command to the flash chip */
+static void nand_command(unsigned char command)
+{
+ 	WRITE_NAND_COMMAND(command, NAND_ADDR);
+

Quoted 8 lines

board/ti/beagle/beagle.c int get_board_revision(void) · line 174

Quoted 6 lines

board/ti/beagle/beagle.c +6 -0

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

Quoted 1 line

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 ?

Quoted 27 lines · Tom Rini

Quoted 3 lines · Igor Grinberg

Quoted 2 lines · Tom Rini

Quoted 1 line · Igor Grinberg

On 10/26/2011 11:13 PM, Tom Rini wrote:

[snip]

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

board/ti/beagle/beagle.c int get_board_revision(void) · line 174

Quoted 11 lines

board/ti/beagle/beagle.c +11 -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);

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

Quoted 5 lines

Quoted 3 lines

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)

Quoted 3 lines

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

Quoted 7 lines · Scott Wood

Quoted 3 lines · Tom Rini

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

Quoted 11 lines · Tom Rini

Quoted 7 lines · Scott Wood

Quoted 3 lines · Tom Rini

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. :-)

Quoted 4 lines

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.

Quoted 20 lines · Scott Wood

Quoted 11 lines · Tom Rini

Quoted 7 lines · Scott Wood

Quoted 3 lines · Tom Rini

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. :-)

Quoted 4 lines

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

Quoted 1 line

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?

Quoted 23 lines · Tom Rini

Quoted 21 lines · Scott Wood

Quoted 11 lines · Tom Rini

Quoted 7 lines · Scott Wood

Quoted 3 lines · Tom Rini

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. :-)

Quoted 4 lines

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.

Quoted 2 lines

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

Quoted 27 lines · Tom Rini

Quoted 3 lines · Igor Grinberg

Quoted 2 lines · Tom Rini

Quoted 1 line · Igor Grinberg

On 10/26/2011 11:13 PM, Tom Rini wrote:

[snip]

Quoted 13 lines

Quoted 13 lines

Quoted 13 lines

Review

Quoted 11 lines

(quoted diff) +11 -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);

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

Quoted 5 lines

Quoted 3 lines

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)

Quoted 3 lines

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

On 10/26/2011 11:13 PM, Tom Rini wrote:

Quoted 1 line

?board/ti/beagle/config.mk ? ? ? ? ? ? | ? 33 -------

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