[U-Boot] [PATCH] overo: add SPL support

[U-Boot] [PATCH] overo: add SPL support

[Andreas Müller]

* implemenatation based on ti beagleboard/omap3evm
* timing data taken from x-loader
* run-tested with overo release 0 and 1 / boot from NAND and SDcard

Signed-off-by: Andreas M?ller <schnitzeltony@gmx.de>
---
 arch/arm/include/asm/arch-omap3/mem.h |   27 ++++++++++++++++
 board/overo/config.mk                 |   28 ----------------
 board/overo/overo.c                   |   41 +++++++++++++++++++++++-
 include/configs/omap3_overo.h         |   56 ++++++++++++++++++++++++++++++++-
 4 files changed, 122 insertions(+), 30 deletions(-)
 delete mode 100644 board/overo/config.mk

diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
index 5fd02d4..18998d8 100644
--- a/arch/arm/include/asm/arch-omap3/mem.h
+++ b/arch/arm/include/asm/arch-omap3/mem.h
@@ -123,6 +123,33 @@ enum {
 		V_MCFG_BANKALLOCATION_RBC |			\
 		V_MCFG_B32NOT16_32 | V_MCFG_DEEPPD_EN | V_MCFG_RAMTYPE_DDR
 
+
+/* Hynix part of Overo (165MHz optimized) 6.06ns */
+#define HYNIX_TDAL_165   6
+#define HYNIX_TDPL_165   3
+#define HYNIX_TRRD_165   2
+#define HYNIX_TRCD_165   3
+#define HYNIX_TRP_165    3
+#define HYNIX_TRAS_165   7
+#define HYNIX_TRC_165   10
+#define HYNIX_TRFC_165  21
+#define HYNIX_V_ACTIMA_165	\
+		ACTIM_CTRLA(HYNIX_TRFC_165, HYNIX_TRC_165,	\
+				HYNIX_TRAS_165, HYNIX_TRP_165,	\
+				HYNIX_TRCD_165, HYNIX_TRRD_165,	\
+				HYNIX_TDPL_165, HYNIX_TDAL_165)
+
+#define HYNIX_TWTR_165   1
+#define HYNIX_TCKE_165   1
+#define HYNIX_TXP_165    2
+#define HYNIX_XSR_165    24
+#define HYNIX_V_ACTIMB_165	\
+		ACTIM_CTRLB(HYNIX_TWTR_165, HYNIX_TCKE_165,	\
+				HYNIX_TXP_165, HYNIX_XSR_165)
+
+#define HYNIX_RASWIDTH_165	0x2
+#define HYNIX_V_MCFG_165(size)	MCFG((size), HYNIX_RASWIDTH_165)
+
 /* Hynix part of AM/DM37xEVM (200MHz optimized) */
 #define HYNIX_TDAL_200		6
 #define HYNIX_TDPL_200		3
diff --git a/board/overo/config.mk b/board/overo/config.mk
deleted file mode 100644
index e7c471c..0000000
--- a/board/overo/config.mk
+++ /dev/null
@@ -1,28 +0,0 @@
-#
-# Overo uses OMAP3 (ARM-CortexA8) cpu
-#
-# 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)
-
-CONFIG_SYS_TEXT_BASE = 0x80008000
diff --git a/board/overo/overo.c b/board/overo/overo.c
index 3c60b06..dbe43ab 100644
--- a/board/overo/overo.c
+++ b/board/overo/overo.c
@@ -31,6 +31,7 @@
 #include <common.h>
 #include <netdev.h>
 #include <twl4030.h>
+#include <linux/mtd/nand.h>
 #include <asm/io.h>
 #include <asm/arch/mmc_host_def.h>
 #include <asm/arch/mux.h>
@@ -126,6 +127,44 @@ int get_board_revision(void)
 	return revision;
 }
 
+#ifdef CONFIG_SPL_BUILD
+/*
+ * 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 ourself on both banks.
+ */
+void get_board_mem_timings(u32 *mcfg, u32 *ctrla, u32 *ctrlb, u32 *rfr_ctrl,
+		u32 *mr)
+{
+	*mr = MICRON_V_MR_165;
+	switch (get_board_revision()) {
+	case 0: /* Micron 1286MB/256MB, 1/2 banks of 128MB */
+		*mcfg = MICRON_V_MCFG_165(128 << 20);
+		*ctrla = MICRON_V_ACTIMA_165;
+		*ctrlb = MICRON_V_ACTIMB_165;
+		*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+		break;
+	case 1: /* Micron 256MB/512MB, 1/2 banks of 256MB */
+		*mcfg = MICRON_V_MCFG_165(256 << 20);
+		*ctrla = MICRON_V_ACTIMA_165;
+		*ctrlb = MICRON_V_ACTIMB_165;
+		*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+		break;
+	case 2: /* Hynix 256MB/512MB, 1/2 banks of 256MB */
+		*mcfg = HYNIX_V_MCFG_165(256 << 20);
+		*ctrla = HYNIX_V_ACTIMA_165;
+		*ctrlb = HYNIX_V_ACTIMB_165;
+		*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+		break;
+	default:
+		*mcfg = MICRON_V_MCFG_165(128 << 20);
+		*ctrla = MICRON_V_ACTIMA_165;
+		*ctrlb = MICRON_V_ACTIMB_165;
+		*rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+	}
+}
+#endif
+
 /*
  * Routine: get_sdio2_config
  * Description: Return information about the wifi module connection
@@ -337,7 +376,7 @@ int board_eth_init(bd_t *bis)
 	return rc;
 }
 
-#ifdef CONFIG_GENERIC_MMC
+#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD)
 int board_mmc_init(bd_t *bis)
 {
 	omap_mmc_init(0);
diff --git a/include/configs/omap3_overo.h b/include/configs/omap3_overo.h
index 79eb466..d9f54d1 100644
--- a/include/configs/omap3_overo.h
+++ b/include/configs/omap3_overo.h
@@ -149,6 +149,25 @@
 #define CONFIG_JFFS2_PART_SIZE		0xf980000	/* size of jffs2 */
 							/* partition */
 
+/* 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
+
+
 /* Environment information */
 #define CONFIG_BOOTDELAY		5
 
@@ -249,7 +268,6 @@
  */
 #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_2		OMAP34XX_SDRC_CS1
 
 /*-----------------------------------------------------------------------
@@ -292,6 +310,13 @@
 
 #endif /* (CONFIG_CMD_NET) */
 
+/*
+ * 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_SDRAM_BASE		PHYS_SDRAM_1
 #define CONFIG_SYS_INIT_RAM_ADDR	0x4020f800
 #define CONFIG_SYS_INIT_RAM_SIZE	0x800
@@ -301,4 +326,33 @@
 
 #define CONFIG_SYS_CACHELINE_SIZE	64
 
+
+/* 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"
+#define CONFIG_SYS_SPL_MALLOC_START	0x80208000
+#define CONFIG_SYS_SPL_MALLOC_SIZE	0x100000
+
 #endif				/* __CONFIG_H */
-- 
1.7.6.4

[U-Boot] [PATCH] overo: add SPL support

[Steve Sakoman]

On Wed, Dec 14, 2011 at 8:27 AM, Andreas M?ller <schnitzeltony@gmx.de> wrote:
> * implemenatation based on ti beagleboard/omap3evm
> * timing data taken from x-loader
> * run-tested with overo release 0 and 1 / boot from NAND and SDcard

Thanks for doing this!

I took a quick look and only see one addition to Tom's comments.  See below.

> Signed-off-by: Andreas M?ller <schnitzeltony@gmx.de>
> ---
> ?arch/arm/include/asm/arch-omap3/mem.h | ? 27 ++++++++++++++++
> ?board/overo/config.mk ? ? ? ? ? ? ? ? | ? 28 ----------------
> ?board/overo/overo.c ? ? ? ? ? ? ? ? ? | ? 41 +++++++++++++++++++++++-
> ?include/configs/omap3_overo.h ? ? ? ? | ? 56 ++++++++++++++++++++++++++++++++-
> ?4 files changed, 122 insertions(+), 30 deletions(-)
> ?delete mode 100644 board/overo/config.mk
>
> diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
> index 5fd02d4..18998d8 100644
> --- a/arch/arm/include/asm/arch-omap3/mem.h
> +++ b/arch/arm/include/asm/arch-omap3/mem.h
> @@ -123,6 +123,33 @@ enum {
> ? ? ? ? ? ? ? ?V_MCFG_BANKALLOCATION_RBC | ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ? ? ? ? ?V_MCFG_B32NOT16_32 | V_MCFG_DEEPPD_EN | V_MCFG_RAMTYPE_DDR
>
> +
> +/* Hynix part of Overo (165MHz optimized) 6.06ns */
> +#define HYNIX_TDAL_165 ? 6
> +#define HYNIX_TDPL_165 ? 3
> +#define HYNIX_TRRD_165 ? 2
> +#define HYNIX_TRCD_165 ? 3
> +#define HYNIX_TRP_165 ? ?3
> +#define HYNIX_TRAS_165 ? 7
> +#define HYNIX_TRC_165 ? 10
> +#define HYNIX_TRFC_165 ?21
> +#define HYNIX_V_ACTIMA_165 ? ? \
> + ? ? ? ? ? ? ? ACTIM_CTRLA(HYNIX_TRFC_165, HYNIX_TRC_165, ? ? ?\
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? HYNIX_TRAS_165, HYNIX_TRP_165, ?\
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? HYNIX_TRCD_165, HYNIX_TRRD_165, \
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? HYNIX_TDPL_165, HYNIX_TDAL_165)
> +
> +#define HYNIX_TWTR_165 ? 1
> +#define HYNIX_TCKE_165 ? 1
> +#define HYNIX_TXP_165 ? ?2
> +#define HYNIX_XSR_165 ? ?24
> +#define HYNIX_V_ACTIMB_165 ? ? \
> + ? ? ? ? ? ? ? ACTIM_CTRLB(HYNIX_TWTR_165, HYNIX_TCKE_165, ? ? \
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? HYNIX_TXP_165, HYNIX_XSR_165)
> +
> +#define HYNIX_RASWIDTH_165 ? ? 0x2
> +#define HYNIX_V_MCFG_165(size) MCFG((size), HYNIX_RASWIDTH_165)
> +
> ?/* Hynix part of AM/DM37xEVM (200MHz optimized) */
> ?#define HYNIX_TDAL_200 ? ? ? ? 6
> ?#define HYNIX_TDPL_200 ? ? ? ? 3
> diff --git a/board/overo/config.mk b/board/overo/config.mk
> deleted file mode 100644
> index e7c471c..0000000
> --- a/board/overo/config.mk
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -#
> -# Overo uses OMAP3 (ARM-CortexA8) cpu
> -#
> -# 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)
> -
> -CONFIG_SYS_TEXT_BASE = 0x80008000
> diff --git a/board/overo/overo.c b/board/overo/overo.c
> index 3c60b06..dbe43ab 100644
> --- a/board/overo/overo.c
> +++ b/board/overo/overo.c
> @@ -31,6 +31,7 @@
> ?#include <common.h>
> ?#include <netdev.h>
> ?#include <twl4030.h>
> +#include <linux/mtd/nand.h>
> ?#include <asm/io.h>
> ?#include <asm/arch/mmc_host_def.h>
> ?#include <asm/arch/mux.h>
> @@ -126,6 +127,44 @@ int get_board_revision(void)
> ? ? ? ?return revision;
> ?}
>
> +#ifdef CONFIG_SPL_BUILD
> +/*
> + * 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 ourself on both banks.
> + */
> +void get_board_mem_timings(u32 *mcfg, u32 *ctrla, u32 *ctrlb, u32 *rfr_ctrl,
> + ? ? ? ? ? ? ? u32 *mr)
> +{
> + ? ? ? *mr = MICRON_V_MR_165;
> + ? ? ? switch (get_board_revision()) {

I think you will also need to update the get_board_revision function
to ensure that SPL works with very early Overo revisions.

Note this excerpt from the X-loader get_board_revision funtion:

	/* board revisions <= R2410 connect 4030 irq_1 to gpio112             */
	/* these boards should return a revision number of 0                  */
	/* the code below forces a 4030 RTC irq to ensure that gpio112 is low */
#ifdef CONFIG_DRIVER_OMAP34XX_I2C
	i2c_init(CFG_I2C_SPEED, CFG_I2C_SLAVE);
	data = 0x01;
	i2c_write(0x4B, 0x29, 1, &data, 1);
	data = 0x0c;
	i2c_write(0x4B, 0x2b, 1, &data, 1);
	i2c_read(0x4B, 0x2a, 1, &data, 1);
#endif

Yup, ugly, but this is the only way to detect revision properly on those boards.

Regards,

Steve

[U-Boot] [PATCH] overo: add SPL support

[Andreas Müller]

On Wednesday, December 14, 2011 06:24:13 PM you wrote:
> 
> I think you will also need to update the get_board_revision function
> to ensure that SPL works with very early Overo revisions.
> 
> Note this excerpt from the X-loader get_board_revision funtion:
> 
> 	/* board revisions <= R2410 connect 4030 irq_1 to gpio112             */
> 	/* these boards should return a revision number of 0                  */
> 	/* the code below forces a 4030 RTC irq to ensure that gpio112 is low */
> #ifdef CONFIG_DRIVER_OMAP34XX_I2C
> 	i2c_init(CFG_I2C_SPEED, CFG_I2C_SLAVE);
> 	data = 0x01;
> 	i2c_write(0x4B, 0x29, 1, &data, 1);
> 	data = 0x0c;
> 	i2c_write(0x4B, 0x2b, 1, &data, 1);
> 	i2c_read(0x4B, 0x2a, 1, &data, 1);
> #endif
> 
> Yup, ugly, but this is the only way to detect revision properly on those
> boards.
That explains why my Rev0-board is sometimes detected with Rev 1 and the whole 
i2c stuff is not working because it was not initialized in u-boot :)

will prepare V2 & test tomorrow & thanks for this hint!

Andreas

[U-Boot] [PATCH] overo: add SPL support

[Andreas Müller]

On Wednesday, December 14, 2011 06:24:13 PM Steve Sakoman wrote:
> 
> I think you will also need to update the get_board_revision function
> to ensure that SPL works with very early Overo revisions.
> 
> Note this excerpt from the X-loader get_board_revision funtion:
> 
> 	/* board revisions <= R2410 connect 4030 irq_1 to gpio112             */
> 	/* these boards should return a revision number of 0                  */
> 	/* the code below forces a 4030 RTC irq to ensure that gpio112 is low */
> #ifdef CONFIG_DRIVER_OMAP34XX_I2C
> 	i2c_init(CFG_I2C_SPEED, CFG_I2C_SLAVE);
> 	data = 0x01;
> 	i2c_write(0x4B, 0x29, 1, &data, 1);
> 	data = 0x0c;
> 	i2c_write(0x4B, 0x2b, 1, &data, 1);
> 	i2c_read(0x4B, 0x2a, 1, &data, 1);
> #endif
> 
> Yup, ugly, but this is the only way to detect revision properly on those
> boards.
I tried the following (as you can see I already commented out the i2c-read-write 
for test):

int get_board_revision(void)
{
#ifdef CONFIG_DRIVER_OMAP34XX_I2C
	i2c_set_bus_num(TWL4030_I2C_BUS);
	/*data = 0x01;
	i2c_write(0x4B, 0x29, 1, &data, 1);
	data = 0x0c;
	i2c_write(0x4B, 0x2b, 1, &data, 1);
	i2c_read(0x4B, 0x2a, 1, &data, 1);*/
#endif
....
}

SPL Boot process hangs on i2c_set_bus_num ( tested by removing i2c_set_bus_num -
> proper operation ) with console freeze:

| U-Boot SPL 2011.12-rc1-00004-g06e42c6-dirty (Dec 15 2011 - 14:03:34)
| Texas Instruments Revision detection unimplemented

The call stack for get_board_revision() is for SPL

s_init()
mem_init()
do_sdrc_init(..)
get_board_mem_timings(..)
get_board_revision(..)

It seems that the call to i2c_set_bus_num comes too early. Before I dive into x-
loader analysis: Has anybody an idea what goes wrong?

Andreas

[U-Boot] [PATCH] overo: add SPL support

[Tom Rini]

On Thu, Dec 15, 2011 at 7:34 AM, Andreas M?ller <schnitzeltony@gmx.de> wrote:
> On Wednesday, December 14, 2011 06:24:13 PM Steve Sakoman wrote:
>>
>> I think you will also need to update the get_board_revision function
>> to ensure that SPL works with very early Overo revisions.
>>
>> Note this excerpt from the X-loader get_board_revision funtion:
>>
>> ? ? ? /* board revisions <= R2410 connect 4030 irq_1 to gpio112 ? ? ? ? ? ? */
>> ? ? ? /* these boards should return a revision number of 0 ? ? ? ? ? ? ? ? ?*/
>> ? ? ? /* the code below forces a 4030 RTC irq to ensure that gpio112 is low */
>> #ifdef CONFIG_DRIVER_OMAP34XX_I2C
>> ? ? ? i2c_init(CFG_I2C_SPEED, CFG_I2C_SLAVE);
>> ? ? ? data = 0x01;
>> ? ? ? i2c_write(0x4B, 0x29, 1, &data, 1);
>> ? ? ? data = 0x0c;
>> ? ? ? i2c_write(0x4B, 0x2b, 1, &data, 1);
>> ? ? ? i2c_read(0x4B, 0x2a, 1, &data, 1);
>> #endif
>>
>> Yup, ugly, but this is the only way to detect revision properly on those
>> boards.
> I tried the following (as you can see I already commented out the i2c-read-write
> for test):
>
> int get_board_revision(void)
> {
> #ifdef CONFIG_DRIVER_OMAP34XX_I2C
> ? ? ? ?i2c_set_bus_num(TWL4030_I2C_BUS);
> ? ? ? ?/*data = 0x01;
> ? ? ? ?i2c_write(0x4B, 0x29, 1, &data, 1);
> ? ? ? ?data = 0x0c;
> ? ? ? ?i2c_write(0x4B, 0x2b, 1, &data, 1);
> ? ? ? ?i2c_read(0x4B, 0x2a, 1, &data, 1);*/
> #endif
> ....
> }
>
> SPL Boot process hangs on i2c_set_bus_num ( tested by removing i2c_set_bus_num -
>> proper operation ) with console freeze:
>
> | U-Boot SPL 2011.12-rc1-00004-g06e42c6-dirty (Dec 15 2011 - 14:03:34)
> | Texas Instruments Revision detection unimplemented
>
> The call stack for get_board_revision() is for SPL
>
> s_init()
> mem_init()
> do_sdrc_init(..)
> get_board_mem_timings(..)
> get_board_revision(..)
>
> It seems that the call to i2c_set_bus_num comes too early. Before I dive into x-
> loader analysis: Has anybody an idea what goes wrong?

Without digging too hard into overo stuff, we've only called
i2c_init() on the main bus and the x-load snippet is on the slave bus.

-- 
Tom

[U-Boot] [PATCH] overo: add SPL support

[Andreas Müller]

On Thu, Dec 15, 2011 at 7:34 AM, Andreas M?ller <schnitzeltony@gmx.de> wrote:
> I tried the following (as you can see I already commented out the i2c-read-
write
> for test):
>
> int get_board_revision(void)
> {
> #ifdef CONFIG_DRIVER_OMAP34XX_I2C
>        i2c_set_bus_num(TWL4030_I2C_BUS);
>        /*data = 0x01;
>        i2c_write(0x4B, 0x29, 1, &data, 1);
>        data = 0x0c;
>        i2c_write(0x4B, 0x2b, 1, &data, 1);
>        i2c_read(0x4B, 0x2a, 1, &data, 1);*/
> #endif
> ....
> }
>
> SPL Boot process hangs on i2c_set_bus_num ( tested by removing
> i2c_set_bus_num -> proper operation ) with console freeze:
>
> | U-Boot SPL 2011.12-rc1-00004-g06e42c6-dirty (Dec 15 2011 - 14:03:34)
> | Texas Instruments Revision detection unimplemented
>
> The call stack for get_board_revision() is for SPL
>
> s_init()
> mem_init()
> do_sdrc_init(..)
> get_board_mem_timings(..)
> get_board_revision(..)
>
> It seems that the call to i2c_set_bus_num comes too early.

Sorry for spamming but I face black magic: 

I added debug messages in omap24xx_i2c.c / i2c_set_bus_num().

* Version 1:

int i2c_set_bus_num(unsigned int bus)
{
	puts("i2c_set_bus_num called 1\n");
	if ((bus < 0) || (bus >= I2C_BUS_MAX)) {
		printf("Bad bus: %d\n", bus);
		return -1;
	}
	printf("Bus: %d\n", bus);

#if I2C_BUS_MAX == 3
	if (bus == 2) {
		puts("i2c_set_bus_num called 2\n");
		i2c_base = (struct i2c *)I2C_BASE3;
		puts("i2c_set_bus_num called 3\n");
	}
	else
#endif
	if (bus == 1) {
		puts("i2c_set_bus_num called 4\n");
		i2c_base = (struct i2c *)I2C_BASE2;
		puts("i2c_set_bus_num called 5\n");
	}
	else {
		puts("i2c_set_bus_num called 6\n");
		i2c_base = (struct i2c *)I2C_BASE1;
		puts("i2c_set_bus_num called 7\n");
	}

	puts("i2c_set_bus_num called 8\n");
-->	current_bus = bus;

	puts("i2c_set_bus_num called 9\n");
	if (!bus_initialized[current_bus])
		i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);

	return 0;
}

leads to

i2c_set_bus_num called 1
Bus: 0
i2c_set_bus_num called 6
i2c_set_bus_num called 7
i2c_set_bus_num called 8


* Version 2:

int i2c_set_bus_num(unsigned int bus)
{
	puts("i2c_set_bus_num called 1\n");
	if ((bus < 0) || (bus >= I2C_BUS_MAX)) {
		printf("Bad bus: %d\n", bus);
		return -1;
	}
	printf("Bus: %d\n", bus);
-->	printf("CurrentBus: %d\n", current_bus);
	printf("AdrCurrentBus: %X\n", &current_bus);

#if I2C_BUS_MAX == 3
	if (bus == 2) {
		puts("i2c_set_bus_num called 2\n");
		i2c_base = (struct i2c *)I2C_BASE3;
		puts("i2c_set_bus_num called 3\n");
	}
	else
#endif
	if (bus == 1) {
		puts("i2c_set_bus_num called 4\n");
		i2c_base = (struct i2c *)I2C_BASE2;
		puts("i2c_set_bus_num called 5\n");
	}
	else {
		puts("i2c_set_bus_num called 6\n");
		i2c_base = (struct i2c *)I2C_BASE1;
		puts("i2c_set_bus_num called 7\n");
	}

	puts("i2c_set_bus_num called 8\n");
	current_bus = bus;

	puts("i2c_set_bus_num called 9\n");
	if (!bus_initialized[current_bus])
		i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);

	return 0;
}

leads to

i2c_set_bus_num called 1
Bus: 0


It seems that accessing 'current_bus' causes trouble. Has anybody an 
explanation for that? I don't think this is related to i2c or overo. For a 
better picture I attached memory mappings.

Since I do not have JTAG for debugging, I think x-loader is also nice :(

Regards

Andreas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: u-boot-spl.map.zip
Type: application/zip
Size: 10148 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20111215/2581ad4e/attachment.zip>

[U-Boot] [PATCH] overo: add SPL support

[Andreas Müller]

On Thursday, December 15, 2011 10:12:59 PM Andreas M?ller wrote:
> On Thu, Dec 15, 2011 at 7:34 AM, Andreas M?ller <schnitzeltony@gmx.de> wrote:
> > I tried the following (as you can see I already commented out the
> > i2c-read-
> 
> write
> 
> > for test):
> > 
> > int get_board_revision(void)
> > {
> > #ifdef CONFIG_DRIVER_OMAP34XX_I2C
> > 
> >        i2c_set_bus_num(TWL4030_I2C_BUS);
> >        /*data = 0x01;
> >        i2c_write(0x4B, 0x29, 1, &data, 1);
> >        data = 0x0c;
> >        i2c_write(0x4B, 0x2b, 1, &data, 1);
> >        i2c_read(0x4B, 0x2a, 1, &data, 1);*/
> > 
> > #endif
> > ....
> > }
> > 
> > SPL Boot process hangs on i2c_set_bus_num ( tested by removing
> > 
> > i2c_set_bus_num -> proper operation ) with console freeze:
> > | U-Boot SPL 2011.12-rc1-00004-g06e42c6-dirty (Dec 15 2011 - 14:03:34)
> > | Texas Instruments Revision detection unimplemented
> > 
> > The call stack for get_board_revision() is for SPL
> > 
> > s_init()
> > mem_init()
> > do_sdrc_init(..)
> > get_board_mem_timings(..)
> > get_board_revision(..)
> > 
> > It seems that the call to i2c_set_bus_num comes too early.
> 
> Sorry for spamming but I face black magic:
> 
> I added debug messages in omap24xx_i2c.c / i2c_set_bus_num().
> 
> * Version 1:
> 
> int i2c_set_bus_num(unsigned int bus)
> {
> 	puts("i2c_set_bus_num called 1\n");
> 	if ((bus < 0) || (bus >= I2C_BUS_MAX)) {
> 		printf("Bad bus: %d\n", bus);
> 		return -1;
> 	}
> 	printf("Bus: %d\n", bus);
> 
> #if I2C_BUS_MAX == 3
> 	if (bus == 2) {
> 		puts("i2c_set_bus_num called 2\n");
> 		i2c_base = (struct i2c *)I2C_BASE3;
> 		puts("i2c_set_bus_num called 3\n");
> 	}
> 	else
> #endif
> 	if (bus == 1) {
> 		puts("i2c_set_bus_num called 4\n");
> 		i2c_base = (struct i2c *)I2C_BASE2;
> 		puts("i2c_set_bus_num called 5\n");
> 	}
> 	else {
> 		puts("i2c_set_bus_num called 6\n");
> 		i2c_base = (struct i2c *)I2C_BASE1;
> 		puts("i2c_set_bus_num called 7\n");
> 	}
> 
> 	puts("i2c_set_bus_num called 8\n");
> -->	current_bus = bus;
> 
> 	puts("i2c_set_bus_num called 9\n");
> 	if (!bus_initialized[current_bus])
> 		i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
> 
> 	return 0;
> }
> 
> leads to
> 
> i2c_set_bus_num called 1
> Bus: 0
> i2c_set_bus_num called 6
> i2c_set_bus_num called 7
> i2c_set_bus_num called 8
> 
> 
> * Version 2:
> 
> int i2c_set_bus_num(unsigned int bus)
> {
> 	puts("i2c_set_bus_num called 1\n");
> 	if ((bus < 0) || (bus >= I2C_BUS_MAX)) {
> 		printf("Bad bus: %d\n", bus);
> 		return -1;
> 	}
> 	printf("Bus: %d\n", bus);
> -->	printf("CurrentBus: %d\n", current_bus);
> 	printf("AdrCurrentBus: %X\n", &current_bus);
> 
> #if I2C_BUS_MAX == 3
> 	if (bus == 2) {
> 		puts("i2c_set_bus_num called 2\n");
> 		i2c_base = (struct i2c *)I2C_BASE3;
> 		puts("i2c_set_bus_num called 3\n");
> 	}
> 	else
> #endif
> 	if (bus == 1) {
> 		puts("i2c_set_bus_num called 4\n");
> 		i2c_base = (struct i2c *)I2C_BASE2;
> 		puts("i2c_set_bus_num called 5\n");
> 	}
> 	else {
> 		puts("i2c_set_bus_num called 6\n");
> 		i2c_base = (struct i2c *)I2C_BASE1;
> 		puts("i2c_set_bus_num called 7\n");
> 	}
> 
> 	puts("i2c_set_bus_num called 8\n");
> 	current_bus = bus;
> 
> 	puts("i2c_set_bus_num called 9\n");
> 	if (!bus_initialized[current_bus])
> 		i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
> 
> 	return 0;
> }
> 
> leads to
> 
> i2c_set_bus_num called 1
> Bus: 0
> 
> 
> It seems that accessing 'current_bus' causes trouble. Has anybody an
> explanation for that? I don't think this is related to i2c or overo. For a
> better picture I attached memory mappings.
> 
'objdump -dSt' shows (the memory mappings I attached were not really helpful - 
sorry next time I know):

    4020ae14 l     O .data      00000004 i2c_base
    80000068 l     O .bss       00000004 current_bus
    8000006c l     O .bss       0000000c bus_initialized

'i2c_base' is correctly located in SRAM but 'current_bus' and 'bus_initialized' 
are located in CS0 SDRAM which is at the time of call not yet initalized. This 
fits to the crash behaviour: Accessing 'i2c_base' does not cause trouble.
How can I move 'current_bus' and 'bus_initialized' to SRAM?

Andreas

[U-Boot] [PATCH] overo: add SPL support

[Tom Rini]

On Mon, Dec 19, 2011 at 6:03 PM, Andreas M?ller <schnitzeltony@gmx.de> wrote:
> On Thursday, December 15, 2011 10:12:59 PM Andreas M?ller wrote:
>> On Thu, Dec 15, 2011 at 7:34 AM, Andreas M?ller <schnitzeltony@gmx.de> wrote:
>> > I tried the following (as you can see I already commented out the
>> > i2c-read-
>>
>> write
>>
>> > for test):
>> >
>> > int get_board_revision(void)
>> > {
>> > #ifdef CONFIG_DRIVER_OMAP34XX_I2C
>> >
>> > ? ? ? ?i2c_set_bus_num(TWL4030_I2C_BUS);
>> > ? ? ? ?/*data = 0x01;
>> > ? ? ? ?i2c_write(0x4B, 0x29, 1, &data, 1);
>> > ? ? ? ?data = 0x0c;
>> > ? ? ? ?i2c_write(0x4B, 0x2b, 1, &data, 1);
>> > ? ? ? ?i2c_read(0x4B, 0x2a, 1, &data, 1);*/
>> >
>> > #endif
>> > ....
>> > }
>> >
>> > SPL Boot process hangs on i2c_set_bus_num ( tested by removing
>> >
>> > i2c_set_bus_num -> proper operation ) with console freeze:
>> > | U-Boot SPL 2011.12-rc1-00004-g06e42c6-dirty (Dec 15 2011 - 14:03:34)
>> > | Texas Instruments Revision detection unimplemented
>> >
>> > The call stack for get_board_revision() is for SPL
>> >
>> > s_init()
>> > mem_init()
>> > do_sdrc_init(..)
>> > get_board_mem_timings(..)
>> > get_board_revision(..)
>> >
>> > It seems that the call to i2c_set_bus_num comes too early.
>>
>> Sorry for spamming but I face black magic:
>>
>> I added debug messages in omap24xx_i2c.c / i2c_set_bus_num().
>>
>> * Version 1:
>>
>> int i2c_set_bus_num(unsigned int bus)
>> {
>> ? ? ? puts("i2c_set_bus_num called 1\n");
>> ? ? ? if ((bus < 0) || (bus >= I2C_BUS_MAX)) {
>> ? ? ? ? ? ? ? printf("Bad bus: %d\n", bus);
>> ? ? ? ? ? ? ? return -1;
>> ? ? ? }
>> ? ? ? printf("Bus: %d\n", bus);
>>
>> #if I2C_BUS_MAX == 3
>> ? ? ? if (bus == 2) {
>> ? ? ? ? ? ? ? puts("i2c_set_bus_num called 2\n");
>> ? ? ? ? ? ? ? i2c_base = (struct i2c *)I2C_BASE3;
>> ? ? ? ? ? ? ? puts("i2c_set_bus_num called 3\n");
>> ? ? ? }
>> ? ? ? else
>> #endif
>> ? ? ? if (bus == 1) {
>> ? ? ? ? ? ? ? puts("i2c_set_bus_num called 4\n");
>> ? ? ? ? ? ? ? i2c_base = (struct i2c *)I2C_BASE2;
>> ? ? ? ? ? ? ? puts("i2c_set_bus_num called 5\n");
>> ? ? ? }
>> ? ? ? else {
>> ? ? ? ? ? ? ? puts("i2c_set_bus_num called 6\n");
>> ? ? ? ? ? ? ? i2c_base = (struct i2c *)I2C_BASE1;
>> ? ? ? ? ? ? ? puts("i2c_set_bus_num called 7\n");
>> ? ? ? }
>>
>> ? ? ? puts("i2c_set_bus_num called 8\n");
>> --> ? current_bus = bus;
>>
>> ? ? ? puts("i2c_set_bus_num called 9\n");
>> ? ? ? if (!bus_initialized[current_bus])
>> ? ? ? ? ? ? ? i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
>>
>> ? ? ? return 0;
>> }
>>
>> leads to
>>
>> i2c_set_bus_num called 1
>> Bus: 0
>> i2c_set_bus_num called 6
>> i2c_set_bus_num called 7
>> i2c_set_bus_num called 8
>>
>>
>> * Version 2:
>>
>> int i2c_set_bus_num(unsigned int bus)
>> {
>> ? ? ? puts("i2c_set_bus_num called 1\n");
>> ? ? ? if ((bus < 0) || (bus >= I2C_BUS_MAX)) {
>> ? ? ? ? ? ? ? printf("Bad bus: %d\n", bus);
>> ? ? ? ? ? ? ? return -1;
>> ? ? ? }
>> ? ? ? printf("Bus: %d\n", bus);
>> --> ? printf("CurrentBus: %d\n", current_bus);
>> ? ? ? printf("AdrCurrentBus: %X\n", &current_bus);
>>
>> #if I2C_BUS_MAX == 3
>> ? ? ? if (bus == 2) {
>> ? ? ? ? ? ? ? puts("i2c_set_bus_num called 2\n");
>> ? ? ? ? ? ? ? i2c_base = (struct i2c *)I2C_BASE3;
>> ? ? ? ? ? ? ? puts("i2c_set_bus_num called 3\n");
>> ? ? ? }
>> ? ? ? else
>> #endif
>> ? ? ? if (bus == 1) {
>> ? ? ? ? ? ? ? puts("i2c_set_bus_num called 4\n");
>> ? ? ? ? ? ? ? i2c_base = (struct i2c *)I2C_BASE2;
>> ? ? ? ? ? ? ? puts("i2c_set_bus_num called 5\n");
>> ? ? ? }
>> ? ? ? else {
>> ? ? ? ? ? ? ? puts("i2c_set_bus_num called 6\n");
>> ? ? ? ? ? ? ? i2c_base = (struct i2c *)I2C_BASE1;
>> ? ? ? ? ? ? ? puts("i2c_set_bus_num called 7\n");
>> ? ? ? }
>>
>> ? ? ? puts("i2c_set_bus_num called 8\n");
>> ? ? ? current_bus = bus;
>>
>> ? ? ? puts("i2c_set_bus_num called 9\n");
>> ? ? ? if (!bus_initialized[current_bus])
>> ? ? ? ? ? ? ? i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
>>
>> ? ? ? return 0;
>> }
>>
>> leads to
>>
>> i2c_set_bus_num called 1
>> Bus: 0
>>
>>
>> It seems that accessing 'current_bus' causes trouble. Has anybody an
>> explanation for that? I don't think this is related to i2c or overo. For a
>> better picture I attached memory mappings.
>>
> 'objdump -dSt' shows (the memory mappings I attached were not really helpful -
> sorry next time I know):
>
> ? ?4020ae14 l ? ? O .data ? ? ?00000004 i2c_base
> ? ?80000068 l ? ? O .bss ? ? ? 00000004 current_bus
> ? ?8000006c l ? ? O .bss ? ? ? 0000000c bus_initialized
>
> 'i2c_base' is correctly located in SRAM but 'current_bus' and 'bus_initialized'
> are located in CS0 SDRAM which is at the time of call not yet initalized. This
> fits to the crash behaviour: Accessing 'i2c_base' does not cause trouble.
> How can I move 'current_bus' and 'bus_initialized' to SRAM?

Ah-ha!  Good work.  If you initialize them to a non-zero value,
statically (and make sure the code doesn't assume they're 0 by
default), this will change.

-- 
Tom

[U-Boot] [PATCH] overo: add SPL support

[Andreas Müller]

On Tuesday, December 20, 2011 02:08:18 AM Tom Rini wrote:
> > 'objdump -dSt' shows (the memory mappings I attached were not really
> > helpful - sorry next time I know):
> > 
> >    4020ae14 l     O .data      00000004 i2c_base
> >    80000068 l     O .bss       00000004 current_bus
> >    8000006c l     O .bss       0000000c bus_initialized
> > 
> > 'i2c_base' is correctly located in SRAM but 'current_bus' and
> > 'bus_initialized' are located in CS0 SDRAM which is at the time of call
> > not yet initalized. This fits to the crash behaviour: Accessing
> > 'i2c_base' does not cause trouble. How can I move 'current_bus' and
> > 'bus_initialized' to SRAM?
> 
> Ah-ha!  Good work.  If you initialize them to a non-zero value,
> statically (and make sure the code doesn't assume they're 0 by
> default), this will change.
LOL: I tried already to set them to 0!

[U-Boot] [PATCH] overo: add SPL support

[Wolfgang Denk]

Dear Tom Rini,

In message <CA+M6bX=bBFBaVm4aD2hVJfLXovtGjmKO9V40E+NyWtMtoaOV6A@mail.gmail.com> you wrote:
>
> > 'i2c_base' is correctly located in SRAM but 'current_bus' and 'bus_initialized'
> > are located in CS0 SDRAM which is at the time of call not yet initalized. This
> > fits to the crash behaviour: Accessing 'i2c_base' does not cause trouble.
> > How can I move 'current_bus' and 'bus_initialized' to SRAM?

I guess you really, really must use i2c before relocation?  If
possible, this should be avoided in the first place.

> Ah-ha!  Good work.  If you initialize them to a non-zero value,
> statically (and make sure the code doesn't assume they're 0 by
> default), this will change.

No, this is justy hiding the problem, and the next one who believes
the value doesn't fit and changes it back to 0 runs into the issue
again.  Rather use an explicit

	int foo __attribute__ ((section(".data"))) = 0;

so everybody sees the real intention.

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
Ein weiser Herrscher kann in einem gro?en Land  mehr  Gutes  bewirken
als  in  einem kleinen - ein dummer Herrscher aber auch viel mehr Un-
fug. Da weise Herrscher seltener sind als dumme, war ich schon  immer
gegen gro?e Reiche skeptisch.                   - Herbert Rosendorfer

[U-Boot] [PATCH] overo: add SPL support

[Tom Rini]

On Mon, Dec 19, 2011 at 10:43 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Tom Rini,
>
> In message <CA+M6bX=bBFBaVm4aD2hVJfLXovtGjmKO9V40E+NyWtMtoaOV6A@mail.gmail.com> you wrote:
>>
>> > 'i2c_base' is correctly located in SRAM but 'current_bus' and 'bus_initialized'
>> > are located in CS0 SDRAM which is at the time of call not yet initalized. This
>> > fits to the crash behaviour: Accessing 'i2c_base' does not cause trouble.
>> > How can I move 'current_bus' and 'bus_initialized' to SRAM?
>
> I guess you really, really must use i2c before relocation? ?If
> possible, this should be avoided in the first place.

Yes, board rev detection to know how to configure SDRAM.

>> Ah-ha! ?Good work. ?If you initialize them to a non-zero value,
>> statically (and make sure the code doesn't assume they're 0 by
>> default), this will change.
>
> No, this is justy hiding the problem, and the next one who believes
> the value doesn't fit and changes it back to 0 runs into the issue
> again. ?Rather use an explicit
>
> ? ? ? ?int foo __attribute__ ((section(".data"))) = 0;
>
> so everybody sees the real intention.

And I'll hit up <linux/compiler.h> and see if there's a shortcut and
put it on my TODO list to clean up the existing areas (there's a few,
but commented at least) that have the same need, but set to non-zero
values.  Thanks!

-- 
Tom

[U-Boot] [PATCH] overo: add SPL support

[Wolfgang Denk]

Dear Tom Rini,

In message <CA+M6bXn_ZBqA8rosE4L4O+4Eu+grAhWgCZ=u=2Nomb6WcyL6-w@mail.gmail.com> you wrote:
>
> > I guess you really, really must use i2c before relocation? =A0If
> > possible, this should be avoided in the first place.
> 
> Yes, board rev detection to know how to configure SDRAM.

I don't consider this a valid reason (reading the SPD EEPROM would be
such a reason).  In almost all other cases it should be suffucient to
configure the maximum number of memory banks and the maximum size of
the memory banks and then use get_ram_size() to determine the actual
amount of memory and to correctly initialize the memory controller.

Note that I don't insist on any changes to existing code here.  This
is just a recommendation which you may (or may not) consider for any
future ports / implementations.


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
How much net work could a network work, if a network could net work?

[U-Boot] [PATCH] overo: add SPL support

[Andreas Müller]

On Tuesday, December 20, 2011 12:41:08 PM you wrote:
> Dear Tom Rini,
> 
> In message <CA+M6bXn_ZBqA8rosE4L4O+4Eu+grAhWgCZ=u=2Nomb6WcyL6-
w@mail.gmail.com> you wrote:
> > > I guess you really, really must use i2c before relocation? =A0If
> > > possible, this should be avoided in the first place.
> > 
> > Yes, board rev detection to know how to configure SDRAM.
> 
> I don't consider this a valid reason (reading the SPD EEPROM would be
> such a reason).  In almost all other cases it should be suffucient to
> configure the maximum number of memory banks and the maximum size of
> the memory banks and then use get_ram_size() to determine the actual
> amount of memory and to correctly initialize the memory controller.
> 
> Note that I don't insist on any changes to existing code here.  This
> is just a recommendation which you may (or may not) consider for any
> future ports / implementations.
> 
>
Dear Wolfgang Denk,
 
I agree to your concerns but - as I understood Steve Sakoman - here the 
situation is slightly different: 
At elder overo boards TWL4030 RTC irq is connected to gpio112. Unfortunately 
this pin is also used for binary revision detection. Therefore we need to send 
'shut-up' to TWL4030 via i2c to avoid reading wrong revision. In SPL this must 
be done *before* SDRAM (timing) is set up, because the type of SDRAM is 
revision dependent.

Hope this helps

Andreas

[U-Boot] [PATCH] overo: add SPL support

[Wolfgang Denk]

Dear Andreas,

In message <201112201253.46991.schnitzeltony@gmx.de> you wrote:
>
> I agree to your concerns but - as I understood Steve Sakoman - here the 
> situation is slightly different: 

I think you misunderstand.

> At elder overo boards TWL4030 RTC irq is connected to gpio112. Unfortunately 
> this pin is also used for binary revision detection. Therefore we need to send 
> 'shut-up' to TWL4030 via i2c to avoid reading wrong revision. In SPL this must 
> be done *before* SDRAM (timing) is set up, because the type of SDRAM is 
> revision dependent.

My suggestion was to check if memory initialization can not rather be
done _without_ reading (and without otherwise knowing) the board type
or revision.  Usually this is possible, and I always prefer such
auto-adjusting solutions over hard-wired approaches that break down
when any of the expected inout data is not correct or not available.

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
We are Microsoft. Unix is irrelevant. Openness is futile.  Prepare to
be assimilated.

[U-Boot] [PATCH] overo: add SPL support

[Igor Grinberg]

Hi Andreas,

On 12/20/11 13:53, Andreas M?ller wrote:
> On Tuesday, December 20, 2011 12:41:08 PM you wrote:
>> Dear Tom Rini,
>>
>> In message <CA+M6bXn_ZBqA8rosE4L4O+4Eu+grAhWgCZ=u=2Nomb6WcyL6-
> w at mail.gmail.com> you wrote:
>>>> I guess you really, really must use i2c before relocation? =A0If
>>>> possible, this should be avoided in the first place.
>>>
>>> Yes, board rev detection to know how to configure SDRAM.
>>
>> I don't consider this a valid reason (reading the SPD EEPROM would be
>> such a reason).  In almost all other cases it should be suffucient to
>> configure the maximum number of memory banks and the maximum size of
>> the memory banks and then use get_ram_size() to determine the actual
>> amount of memory and to correctly initialize the memory controller.
>>
>> Note that I don't insist on any changes to existing code here.  This
>> is just a recommendation which you may (or may not) consider for any
>> future ports / implementations.
>>
>>
> Dear Wolfgang Denk,
>  
> I agree to your concerns but - as I understood Steve Sakoman - here the 
> situation is slightly different: 
> At elder overo boards TWL4030 RTC irq is connected to gpio112. Unfortunately 
> this pin is also used for binary revision detection. Therefore we need to send 
> 'shut-up' to TWL4030 via i2c to avoid reading wrong revision. In SPL this must 
> be done *before* SDRAM (timing) is set up, because the type of SDRAM is 
> revision dependent.

What about forging some very not optimized default DRAM settings,
that suit any assembled DRAM and then when you have I2C access,
reconfigure it - is it possible?


-- 
Regards,
Igor.

[U-Boot] [PATCH] overo: add SPL support

[Andreas Müller]

On Tuesday, December 20, 2011 01:06:07 PM you wrote:
> Dear Andreas,
> 
> In message <201112201253.46991.schnitzeltony@gmx.de> you wrote:
> > I agree to your concerns but - as I understood Steve Sakoman - here the
> 
> > situation is slightly different:
> I think you misunderstand.
Yes I think too :-)
> 
> > At elder overo boards TWL4030 RTC irq is connected to gpio112.
> > Unfortunately this pin is also used for binary revision detection.
> > Therefore we need to send 'shut-up' to TWL4030 via i2c to avoid reading
> > wrong revision. In SPL this must be done *before* SDRAM (timing) is set
> > up, because the type of SDRAM is revision dependent.
> 
> My suggestion was to check if memory initialization can not rather be
> done _without_ reading (and without otherwise knowing) the board type
> or revision.  Usually this is possible, and I always prefer such
> auto-adjusting solutions over hard-wired approaches that break down
> when any of the expected inout data is not correct or not available.
> 
Dear Wolfgang,

I don't know if I want to jump also into these changes now - especially since 
I am quite new here..
But for my intererst - since it seems more error tolerant: How is SDRAM timing 
set up without exactly knowing what type is connected? Is there a good example 
implementation in u-boot(-spl)?

Best regards

Andreas

[U-Boot] [PATCH] overo: add SPL support

[Steve Sakoman]

On Tue, Dec 20, 2011 at 4:20 AM, Igor Grinberg <grinberg@compulab.co.il>wrote:


> What about forging some very not optimized default DRAM settings,
> that suit any assembled DRAM and then when you have I2C access,
> reconfigure it - is it possible?
>

The board ID is used to determine some fairly fundamental things like how
the address bits are multiplexed, bank size, number of banks, and timing.

Perhaps it might be possible to determine some non-optimal settings that
can work with the current set of POP memories used, and also a scheme to
modify the above on the fly while executing from said ram, but then one
would have to revisit this every time a new vendor/type of POP was used.

That seems a lot more complex than the current method.

I suppose we could just drop support for the old boards in u-boot.  Those
folks could continue to use the current x-load solution.  Or perhaps
someone will come up with a more clever idea!

Steve

[U-Boot] [PATCH] overo: add SPL support

[Aneesh V]

On Tuesday 20 December 2011 06:09 PM, Andreas M?ller wrote:
> On Tuesday, December 20, 2011 01:06:07 PM you wrote:
>> Dear Andreas,
>>
>> In message<201112201253.46991.schnitzeltony@gmx.de>  you wrote:
>>> I agree to your concerns but - as I understood Steve Sakoman - here the
>>
>>> situation is slightly different:
>> I think you misunderstand.
> Yes I think too :-)
>>
>>> At elder overo boards TWL4030 RTC irq is connected to gpio112.
>>> Unfortunately this pin is also used for binary revision detection.
>>> Therefore we need to send 'shut-up' to TWL4030 via i2c to avoid reading
>>> wrong revision. In SPL this must be done *before* SDRAM (timing) is set
>>> up, because the type of SDRAM is revision dependent.
>>
>> My suggestion was to check if memory initialization can not rather be
>> done _without_ reading (and without otherwise knowing) the board type
>> or revision.  Usually this is possible, and I always prefer such
>> auto-adjusting solutions over hard-wired approaches that break down
>> when any of the expected inout data is not correct or not available.
>>
> Dear Wolfgang,
>
> I don't know if I want to jump also into these changes now - especially since
> I am quite new here..
> But for my intererst - since it seems more error tolerant: How is SDRAM timing
> set up without exactly knowing what type is connected? Is there a good example
> implementation in u-boot(-spl)?

Here is an example for automatic configuration with LPDDR2 memories in
OMAP4.

http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/omap-common/emif-common.c;h=62678ff5d30739ce6dc0a4fdb5ea398acc3d31ea;hb=HEAD

Look at the function emif_get_device_details().

The automatic settings make use of timing values from JEDEC spec
that is guaranteed to be safe for all devices, but not necessarily
optimal(but for all LPDDR2 devices I have seen so far their timings are
not different from the JEDEC defaults). In addition, we need to know
density etc. which can be read from the LPDDR2 device itself.

Something similar may be possible for LPDDR (I presume you are using
LPDDR), but will be somewhat complex, to say the least. Please note
that we have compiled out the above code from OMAP4 SPL due to space
constraints!

If something like 'int foo __attribute__ ((section(".data"))) = 0;' is
solving your problem that may be the easier solution at the moment.

br,
Aneesh

[U-Boot] [PATCH] overo: add SPL support

[Wolfgang Denk]

Dear Andreas,

In message <201112201339.39460.schnitzeltony@gmx.de> you wrote:
>
> I don't know if I want to jump also into these changes now - especially since 
> I am quite new here..
> But for my intererst - since it seems more error tolerant: How is SDRAM timing 
> set up without exactly knowing what type is connected? Is there a good example 
> implementation in u-boot(-spl)?

In most cases thare is not an arbitrary range of RAM types, but only a
pretty small selection; even more often they differ only in size, not
in timings.  It depends on the set of possible configurations and
capabilities of the meory controller how to proceed then.  Typically
you will start with very conservative settings, and from the sizes
found you will know the correct configuration.

As for examples: probably not much in SPL, but a number of PPC board
use this. See for example "board/tqc/tqm8xx/tqm8xx.c" which probes
several banks of memory in 8, 9, and 10 column mode and then adjusts
timing, even taking into account that the CPU and bus clock (and thus
the refresh timings) are variable on some of these boards (you can set
the CPU clock through the "cpuclk" environment variable).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"There is no statute of limitations on stupidity."
- Randomly produced by a computer program called Markov3.

[U-Boot] [PATCH] overo: add SPL support

[Andreas Müller]

On Tuesday, December 20, 2011 02:55:50 PM you wrote:
> On Tue, Dec 20, 2011 at 4:20 AM, Igor Grinberg <grinberg@compulab.co.il>wrote:
> > What about forging some very not optimized default DRAM settings,
> > that suit any assembled DRAM and then when you have I2C access,
> > reconfigure it - is it possible?
> 
> The board ID is used to determine some fairly fundamental things like how
> the address bits are multiplexed, bank size, number of banks, and timing.
> 
> Perhaps it might be possible to determine some non-optimal settings that
> can work with the current set of POP memories used, and also a scheme to
> modify the above on the fly while executing from said ram, but then one
> would have to revisit this every time a new vendor/type of POP was used.
> 
> That seems a lot more complex than the current method.
> 
> I suppose we could just drop support for the old boards in u-boot.  Those
> folks could continue to use the current x-load solution. 
I am afraid you are right. It seems that the current U-boot/OMAP/SPL environment 
it is not designed to have i2c in early state when SDRAM is set up. 
My experience:
* After moving the i2c variables to SDRAM, i2c_set_bus_num (and thereby 
i2c_init) work but i2c_write hangs when calling udelay because timer is not yet 
running.
* This can be worked around by calling timer_init() within s_init(). With this 
setup booting is possible but I get MMC timeout messages from u-boot although 
kernel is loaded properly.
* I don't know if this is the reason for timeout but I think successful booting 
is just by accident because the register pointer to global data is not yet set 
(i2c and timer use global data). I tried to setup global data earlier but up to 
now without success. Even if successful I think the number of friends for such a 
deep change is limited...
Not to be misunderstood: This is no criticism to anybody. The experince that a 
design does not fit for a (corner case) request I have had in several projects :)
> Or perhaps someone will come up with a more clever idea!
> 
My suggestion: 

* Soon: Clean up the patch already sent as reviewed and leave i2c TWL4030 shut-
up out. As fallback for old boards leave x-load in oe meta-gumstix  (maybe just 
create a different machine configuration).
* Later: Think about a 'more probing' approach as suggested in this thread (or a 
combination e.g if revision 1 detected, check size of SDRAM and with the result 
deciding if this is a revision 1/0).

Anyway: The informations regarding SDRAM I copied from x-load. Since this is a 
bit fishing in the dark: Are there documents from gumstix available, describing 
which memories were used (with which revision :) or are these secret IP?

Regards

Andreas