[U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3
SPL already has GD set to the correct location (in s_init), we mustn't move it around now since some data (clocks etc) is already present.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net --- arch/arm/lib/crt0.S | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index a9657d1..b05f66a 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -85,7 +85,13 @@ ENTRY(_main) bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ +#if !defined(CONFIG_SPL_BUILD) +/* + * SPL already has GD set to the correct location (in s_init), we mustn't + * move it around now since some data (clocks etc) is already present. + */ mov r8, sp /* GD is above SP */ +#endif mov r0, #0 bl board_init_f
Currently in OMAP3 SPL, the GPMC for NAND is configured for 16bit access. This patch adds support for 8bit NAND devices as well.
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com --- arch/arm/cpu/armv7/omap3/mem.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/arch/arm/cpu/armv7/omap3/mem.c b/arch/arm/cpu/armv7/omap3/mem.c index d04a5a1..378ba81 100644 --- a/arch/arm/cpu/armv7/omap3/mem.c +++ b/arch/arm/cpu/armv7/omap3/mem.c @@ -34,6 +34,17 @@ struct gpmc *gpmc_cfg;
#if defined(CONFIG_CMD_NAND) +#if defined(GPMC_NAND_ECC_SP_x8_LAYOUT) || defined(GPMC_NAND_ECC_LP_x8_LAYOUT) +static const u32 gpmc_m_nand[GPMC_MAX_REG] = { + SMNAND_GPMC_CONFIG1, + SMNAND_GPMC_CONFIG2, + SMNAND_GPMC_CONFIG3, + SMNAND_GPMC_CONFIG4, + SMNAND_GPMC_CONFIG5, + SMNAND_GPMC_CONFIG6, + 0, +}; +#else static const u32 gpmc_m_nand[GPMC_MAX_REG] = { M_NAND_GPMC_CONFIG1, M_NAND_GPMC_CONFIG2, @@ -42,6 +53,7 @@ static const u32 gpmc_m_nand[GPMC_MAX_REG] = { M_NAND_GPMC_CONFIG5, M_NAND_GPMC_CONFIG6, 0 }; +#endif #endif /* CONFIG_CMD_NAND */
#if defined(CONFIG_CMD_ONENAND)
On Fri, Jun 14, 2013 at 10:55:00AM +0200, Stefan Roese wrote:
Currently in OMAP3 SPL, the GPMC for NAND is configured for 16bit access. This patch adds support for 8bit NAND devices as well.
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com
Applied to u-boot-ti/master, thanks!
Add SPL U-Boot support to replace x-loader on the Compulab cm_t35 board. Currently only the 256MiB SDRAM board versions are supported.
Tested by booting via MMC and NAND.
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Igor Grinberg grinberg@compulab.co.il --- board/compulab/cm_t35/cm_t35.c | 18 +++++++++++- include/configs/cm_t35.h | 64 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/board/compulab/cm_t35/cm_t35.c b/board/compulab/cm_t35/cm_t35.c index b0b80e5..cd7882e 100644 --- a/board/compulab/cm_t35/cm_t35.c +++ b/board/compulab/cm_t35/cm_t35.c @@ -120,6 +120,22 @@ static inline int splash_load_from_nand(void) } #endif /* CONFIG_CMD_NAND */
+#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(struct board_sdrc_timings *timings) +{ + timings->mr = MICRON_V_MR_165; + timings->mcfg = MICRON_V_MCFG_165(256 << 20); + timings->ctrla = MICRON_V_ACTIMA_165; + timings->ctrlb = MICRON_V_ACTIMB_165; + timings->rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz; +} +#endif + int board_splash_screen_prepare(void) { char *env_splashimage_value; @@ -443,7 +459,7 @@ void set_muxconf_regs(void) cm_t3730_set_muxconf(); }
-#ifdef CONFIG_GENERIC_MMC +#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD) int board_mmc_getcd(struct mmc *mmc) { u8 val; diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h index c6e357a..b76810d 100644 --- a/include/configs/cm_t35.h +++ b/include/configs/cm_t35.h @@ -40,8 +40,6 @@ #define CONFIG_OMAP_GPIO #define CONFIG_CM_T3X /* working with CM-T35 and CM-T3730 */
-#define CONFIG_SYS_TEXT_BASE 0x80008000 - #define CONFIG_SDRC /* The chip has SDRC controller */
#include <asm/arch/cpu.h> /* get chip and board defs */ @@ -341,4 +339,66 @@ #define CONFIG_BMP_16BPP #define CONFIG_SPLASH_SCREEN_PREPARE
+/* Defines for SPL */ +#define CONFIG_SPL +#define CONFIG_SPL_FRAMEWORK +#define CONFIG_SPL_NAND_SIMPLE +#define CONFIG_SPL_TEXT_BASE 0x40200800 +#define CONFIG_SPL_MAX_SIZE (54 * 1024) /* 8 KB for stack */ +#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_BOARD_INIT +#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_NAND_BASE +#define CONFIG_SPL_NAND_DRIVERS +#define CONFIG_SPL_NAND_ECC +#define CONFIG_SPL_GPIO_SUPPORT +#define CONFIG_SPL_POWER_SUPPORT +#define CONFIG_SPL_OMAP3_ID_NAND +#define CONFIG_SPL_LDSCRIPT "$(CPUDIR)/omap-common/u-boot-spl.lds" + +/* NAND boot config */ +#define CONFIG_SYS_NAND_5_ADDR_CYCLE +#define CONFIG_SYS_NAND_PAGE_COUNT 64 +#define CONFIG_SYS_NAND_PAGE_SIZE 2048 +#define CONFIG_SYS_NAND_OOBSIZE 64 +#define CONFIG_SYS_NAND_BLOCK_SIZE (128 * 1024) +#define CONFIG_SYS_NAND_BAD_BLOCK_POS NAND_LARGE_BADBLOCK_POS +/* + * Use the ECC/OOB layout from omap_gpmc.h that matches your chip: + * SP vs LP, 8bit vs 16bit: GPMC_NAND_HW_ECC_LAYOUT + */ +#define CONFIG_SYS_NAND_ECCPOS { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ + 10, 11, 12 } +#define CONFIG_SYS_NAND_ECCSIZE 512 +#define CONFIG_SYS_NAND_ECCBYTES 3 + +#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 */
Hi Stefan,
On 06/14/13 11:55, Stefan Roese wrote:
Add SPL U-Boot support to replace x-loader on the Compulab cm_t35 board. Currently only the 256MiB SDRAM board versions are supported.
Tested by booting via MMC and NAND.
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Igor Grinberg grinberg@compulab.co.il
board/compulab/cm_t35/cm_t35.c | 18 +++++++++++- include/configs/cm_t35.h | 64 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/board/compulab/cm_t35/cm_t35.c b/board/compulab/cm_t35/cm_t35.c index b0b80e5..cd7882e 100644 --- a/board/compulab/cm_t35/cm_t35.c +++ b/board/compulab/cm_t35/cm_t35.c @@ -120,6 +120,22 @@ static inline int splash_load_from_nand(void) } #endif /* CONFIG_CMD_NAND */
+#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(struct board_sdrc_timings *timings) +{
- timings->mr = MICRON_V_MR_165;
- timings->mcfg = MICRON_V_MCFG_165(256 << 20);
- timings->ctrla = MICRON_V_ACTIMA_165;
- timings->ctrlb = MICRON_V_ACTIMB_165;
- timings->rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+} +#endif
I still haven't checked the timings... Hopefully, I will sometime during this week.
int board_splash_screen_prepare(void) { char *env_splashimage_value; @@ -443,7 +459,7 @@ void set_muxconf_regs(void) cm_t3730_set_muxconf(); }
-#ifdef CONFIG_GENERIC_MMC +#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD) int board_mmc_getcd(struct mmc *mmc) { u8 val; diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h index c6e357a..b76810d 100644 --- a/include/configs/cm_t35.h +++ b/include/configs/cm_t35.h @@ -40,8 +40,6 @@ #define CONFIG_OMAP_GPIO #define CONFIG_CM_T3X /* working with CM-T35 and CM-T3730 */
-#define CONFIG_SYS_TEXT_BASE 0x80008000
#define CONFIG_SDRC /* The chip has SDRC controller */
#include <asm/arch/cpu.h> /* get chip and board defs */ @@ -341,4 +339,66 @@ #define CONFIG_BMP_16BPP #define CONFIG_SPLASH_SCREEN_PREPARE
+/* Defines for SPL */ +#define CONFIG_SPL +#define CONFIG_SPL_FRAMEWORK +#define CONFIG_SPL_NAND_SIMPLE +#define CONFIG_SPL_TEXT_BASE 0x40200800 +#define CONFIG_SPL_MAX_SIZE (54 * 1024) /* 8 KB for stack */ +#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_BOARD_INIT +#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_NAND_BASE +#define CONFIG_SPL_NAND_DRIVERS +#define CONFIG_SPL_NAND_ECC +#define CONFIG_SPL_GPIO_SUPPORT +#define CONFIG_SPL_POWER_SUPPORT +#define CONFIG_SPL_OMAP3_ID_NAND +#define CONFIG_SPL_LDSCRIPT "$(CPUDIR)/omap-common/u-boot-spl.lds"
+/* NAND boot config */ +#define CONFIG_SYS_NAND_5_ADDR_CYCLE +#define CONFIG_SYS_NAND_PAGE_COUNT 64 +#define CONFIG_SYS_NAND_PAGE_SIZE 2048 +#define CONFIG_SYS_NAND_OOBSIZE 64 +#define CONFIG_SYS_NAND_BLOCK_SIZE (128 * 1024) +#define CONFIG_SYS_NAND_BAD_BLOCK_POS NAND_LARGE_BADBLOCK_POS +/*
- Use the ECC/OOB layout from omap_gpmc.h that matches your chip:
- SP vs LP, 8bit vs 16bit: GPMC_NAND_HW_ECC_LAYOUT
- */
+#define CONFIG_SYS_NAND_ECCPOS { 1, 2, 3, 4, 5, 6, 7, 8, 9, \
10, 11, 12 }+#define CONFIG_SYS_NAND_ECCSIZE 512 +#define CONFIG_SYS_NAND_ECCBYTES 3
+#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
Now this is a problem. This breaks the backward compatibility with our X-Loader and we cannot just switch to 80100000...
+#define CONFIG_SYS_SPL_MALLOC_START 0x80208000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000
#endif /* __CONFIG_H */
On Mon, Jun 17, 2013 at 02:53:27PM +0300, Igor Grinberg wrote:
Hi Stefan,
On 06/14/13 11:55, Stefan Roese wrote:
Add SPL U-Boot support to replace x-loader on the Compulab cm_t35 board. Currently only the 256MiB SDRAM board versions are supported.
Tested by booting via MMC and NAND.
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Igor Grinberg grinberg@compulab.co.il
[snip]
+/*
- 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
Now this is a problem. This breaks the backward compatibility with our X-Loader and we cannot just switch to 80100000...
And thinking back to when I was doing more OMAP3 conversions, there's no requirement to break compatibility with x-loader either. You just need to take care where you place things, see doc/SPL/README.omap3 for the SPL and X-Loader compatible setup.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 06/17/13 15:38, Tom Rini wrote:
On Mon, Jun 17, 2013 at 02:53:27PM +0300, Igor Grinberg wrote:
Hi Stefan,
On 06/14/13 11:55, Stefan Roese wrote:
Add SPL U-Boot support to replace x-loader on the Compulab cm_t35 board. Currently only the 256MiB SDRAM board versions are supported.
Tested by booting via MMC and NAND.
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Igor Grinberg grinberg@compulab.co.il
[snip]
+/*
- 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
Now this is a problem. This breaks the backward compatibility with our X-Loader and we cannot just switch to 80100000...
And thinking back to when I was doing more OMAP3 conversions, there's no requirement to break compatibility with x-loader either. You just need to take care where you place things, see doc/SPL/README.omap3 for the SPL and X-Loader compatible setup.
Actually, I was thinking about adding a target in boards.cfg, but if we can make both (X-Loader and SPL) live in piece, then IMO, we should go for it.
Thanks for the pointer!
- -- Regards, Igor.
On 17.06.2013 15:38, Igor Grinberg wrote:
+/*
- 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
Now this is a problem. This breaks the backward compatibility with our X-Loader and we cannot just switch to 80100000...
And thinking back to when I was doing more OMAP3 conversions, there's no requirement to break compatibility with x-loader either. You just need to take care where you place things, see doc/SPL/README.omap3 for the SPL and X-Loader compatible setup.
Actually, I was thinking about adding a target in boards.cfg, but if we can make both (X-Loader and SPL) live in piece, then IMO, we should go for it.
I just did a quick check with moving CONFIG_SYS_TEXT_BASE back to 0x80008000 and CONFIG_SPL_BSS_START_ADDR up to 0x80100000. Seems to work just fine.
I'll submit a new version in a short while.
Thanks, Stefan
Add SPL U-Boot support to replace x-loader on the Compulab cm_t35 board. Currently only the 256MiB SDRAM board versions are supported.
Tested by booting via MMC and NAND.
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Igor Grinberg grinberg@compulab.co.il --- v2: - Change CONFIG_SYS_TEXT_BASE back to 0x80008000 for x-loader compatibility - Change CONFIG_SPL_BSS_START_ADDR to 0x80100000 to not overlap with TEXT_BASE now
board/compulab/cm_t35/cm_t35.c | 18 +++++++++++- include/configs/cm_t35.h | 64 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/board/compulab/cm_t35/cm_t35.c b/board/compulab/cm_t35/cm_t35.c index b0b80e5..cd7882e 100644 --- a/board/compulab/cm_t35/cm_t35.c +++ b/board/compulab/cm_t35/cm_t35.c @@ -120,6 +120,22 @@ static inline int splash_load_from_nand(void) } #endif /* CONFIG_CMD_NAND */
+#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(struct board_sdrc_timings *timings) +{ + timings->mr = MICRON_V_MR_165; + timings->mcfg = MICRON_V_MCFG_165(256 << 20); + timings->ctrla = MICRON_V_ACTIMA_165; + timings->ctrlb = MICRON_V_ACTIMB_165; + timings->rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz; +} +#endif + int board_splash_screen_prepare(void) { char *env_splashimage_value; @@ -443,7 +459,7 @@ void set_muxconf_regs(void) cm_t3730_set_muxconf(); }
-#ifdef CONFIG_GENERIC_MMC +#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD) int board_mmc_getcd(struct mmc *mmc) { u8 val; diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h index c6e357a..1dc2d5b 100644 --- a/include/configs/cm_t35.h +++ b/include/configs/cm_t35.h @@ -40,8 +40,6 @@ #define CONFIG_OMAP_GPIO #define CONFIG_CM_T3X /* working with CM-T35 and CM-T3730 */
-#define CONFIG_SYS_TEXT_BASE 0x80008000 - #define CONFIG_SDRC /* The chip has SDRC controller */
#include <asm/arch/cpu.h> /* get chip and board defs */ @@ -341,4 +339,66 @@ #define CONFIG_BMP_16BPP #define CONFIG_SPLASH_SCREEN_PREPARE
+/* Defines for SPL */ +#define CONFIG_SPL +#define CONFIG_SPL_FRAMEWORK +#define CONFIG_SPL_NAND_SIMPLE + +#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_BOARD_INIT +#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_NAND_BASE +#define CONFIG_SPL_NAND_DRIVERS +#define CONFIG_SPL_NAND_ECC +#define CONFIG_SPL_GPIO_SUPPORT +#define CONFIG_SPL_POWER_SUPPORT +#define CONFIG_SPL_OMAP3_ID_NAND +#define CONFIG_SPL_LDSCRIPT "$(CPUDIR)/omap-common/u-boot-spl.lds" + +/* NAND boot config */ +#define CONFIG_SYS_NAND_5_ADDR_CYCLE +#define CONFIG_SYS_NAND_PAGE_COUNT 64 +#define CONFIG_SYS_NAND_PAGE_SIZE 2048 +#define CONFIG_SYS_NAND_OOBSIZE 64 +#define CONFIG_SYS_NAND_BLOCK_SIZE (128 * 1024) +#define CONFIG_SYS_NAND_BAD_BLOCK_POS NAND_LARGE_BADBLOCK_POS +/* + * Use the ECC/OOB layout from omap_gpmc.h that matches your chip: + * SP vs LP, 8bit vs 16bit: GPMC_NAND_HW_ECC_LAYOUT + */ +#define CONFIG_SYS_NAND_ECCPOS { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ + 10, 11, 12 } +#define CONFIG_SYS_NAND_ECCSIZE 512 +#define CONFIG_SYS_NAND_ECCBYTES 3 + +#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE +#define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000 + +#define CONFIG_SPL_TEXT_BASE 0x40200800 +#define CONFIG_SPL_MAX_SIZE (54 * 1024) /* 8 KB for stack */ +#define CONFIG_SPL_STACK LOW_LEVEL_SRAM_STACK + +/* + * Use 0x80008000 as TEXT_BASE here for compatibility reasons with the + * older x-loader implementations. And move the BSS area so that it + * doesn't overlap with TEXT_BASE. + */ +#define CONFIG_SYS_TEXT_BASE 0x80008000 +#define CONFIG_SPL_BSS_START_ADDR 0x80100000 +#define CONFIG_SPL_BSS_MAX_SIZE 0x80000 /* 512 KB */ + +#define CONFIG_SYS_SPL_MALLOC_START 0x80208000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000 + #endif /* __CONFIG_H */
This version works when booting with X-Loader.
Tested-by: Nikita Kiryanov nikita@compulab.co.il
On 06/17/2013 05:03 PM, Stefan Roese wrote:
Add SPL U-Boot support to replace x-loader on the Compulab cm_t35 board. Currently only the 256MiB SDRAM board versions are supported.
Tested by booting via MMC and NAND.
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Igor Grinberg grinberg@compulab.co.il
v2:
Change CONFIG_SYS_TEXT_BASE back to 0x80008000 for x-loader compatibility
Change CONFIG_SPL_BSS_START_ADDR to 0x80100000 to not overlap with TEXT_BASE now
board/compulab/cm_t35/cm_t35.c | 18 +++++++++++- include/configs/cm_t35.h | 64 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/board/compulab/cm_t35/cm_t35.c b/board/compulab/cm_t35/cm_t35.c index b0b80e5..cd7882e 100644 --- a/board/compulab/cm_t35/cm_t35.c +++ b/board/compulab/cm_t35/cm_t35.c @@ -120,6 +120,22 @@ static inline int splash_load_from_nand(void) } #endif /* CONFIG_CMD_NAND */
+#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(struct board_sdrc_timings *timings) +{
- timings->mr = MICRON_V_MR_165;
- timings->mcfg = MICRON_V_MCFG_165(256 << 20);
- timings->ctrla = MICRON_V_ACTIMA_165;
- timings->ctrlb = MICRON_V_ACTIMB_165;
- timings->rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
+} +#endif
- int board_splash_screen_prepare(void) { char *env_splashimage_value;
@@ -443,7 +459,7 @@ void set_muxconf_regs(void) cm_t3730_set_muxconf(); }
-#ifdef CONFIG_GENERIC_MMC +#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD) int board_mmc_getcd(struct mmc *mmc) { u8 val; diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h index c6e357a..1dc2d5b 100644 --- a/include/configs/cm_t35.h +++ b/include/configs/cm_t35.h @@ -40,8 +40,6 @@ #define CONFIG_OMAP_GPIO #define CONFIG_CM_T3X /* working with CM-T35 and CM-T3730 */
-#define CONFIG_SYS_TEXT_BASE 0x80008000
#define CONFIG_SDRC /* The chip has SDRC controller */
#include <asm/arch/cpu.h> /* get chip and board defs */
@@ -341,4 +339,66 @@ #define CONFIG_BMP_16BPP #define CONFIG_SPLASH_SCREEN_PREPARE
+/* Defines for SPL */ +#define CONFIG_SPL +#define CONFIG_SPL_FRAMEWORK +#define CONFIG_SPL_NAND_SIMPLE
+#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_BOARD_INIT +#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_NAND_BASE +#define CONFIG_SPL_NAND_DRIVERS +#define CONFIG_SPL_NAND_ECC +#define CONFIG_SPL_GPIO_SUPPORT +#define CONFIG_SPL_POWER_SUPPORT +#define CONFIG_SPL_OMAP3_ID_NAND +#define CONFIG_SPL_LDSCRIPT "$(CPUDIR)/omap-common/u-boot-spl.lds"
+/* NAND boot config */ +#define CONFIG_SYS_NAND_5_ADDR_CYCLE +#define CONFIG_SYS_NAND_PAGE_COUNT 64 +#define CONFIG_SYS_NAND_PAGE_SIZE 2048 +#define CONFIG_SYS_NAND_OOBSIZE 64 +#define CONFIG_SYS_NAND_BLOCK_SIZE (128 * 1024) +#define CONFIG_SYS_NAND_BAD_BLOCK_POS NAND_LARGE_BADBLOCK_POS +/*
- Use the ECC/OOB layout from omap_gpmc.h that matches your chip:
- SP vs LP, 8bit vs 16bit: GPMC_NAND_HW_ECC_LAYOUT
- */
+#define CONFIG_SYS_NAND_ECCPOS { 1, 2, 3, 4, 5, 6, 7, 8, 9, \
10, 11, 12 }+#define CONFIG_SYS_NAND_ECCSIZE 512 +#define CONFIG_SYS_NAND_ECCBYTES 3
+#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE +#define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000
+#define CONFIG_SPL_TEXT_BASE 0x40200800 +#define CONFIG_SPL_MAX_SIZE (54 * 1024) /* 8 KB for stack */ +#define CONFIG_SPL_STACK LOW_LEVEL_SRAM_STACK
+/*
- Use 0x80008000 as TEXT_BASE here for compatibility reasons with the
- older x-loader implementations. And move the BSS area so that it
- doesn't overlap with TEXT_BASE.
- */
+#define CONFIG_SYS_TEXT_BASE 0x80008000 +#define CONFIG_SPL_BSS_START_ADDR 0x80100000 +#define CONFIG_SPL_BSS_MAX_SIZE 0x80000 /* 512 KB */
+#define CONFIG_SYS_SPL_MALLOC_START 0x80208000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000
- #endif /* __CONFIG_H */
Add SPL U-Boot support to replace x-loader on the Compulab cm_t35 board. Currently only the 256MiB SDRAM board versions are supported.
Tested by booting via MMC and NAND.
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Igor Grinberg grinberg@compulab.co.il --- v3: - Some instability of this SDRAM setup has been detected while running Linux. Comparison with the x-loader setup showed that mcfg is configured slightly differently here. CM_T35 needs RAS-width of 14 instead of 13. So use the define MICRON_V_MCFG_200 which implements this 14 as RAS width.
v2: - Change CONFIG_SYS_TEXT_BASE back to 0x80008000 for x-loader compatibility - Change CONFIG_SPL_BSS_START_ADDR to 0x80100000 to not overlap with TEXT_BASE now
board/compulab/cm_t35/cm_t35.c | 18 +++++++++++- include/configs/cm_t35.h | 64 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/board/compulab/cm_t35/cm_t35.c b/board/compulab/cm_t35/cm_t35.c index 3caa5be..c3d064c 100644 --- a/board/compulab/cm_t35/cm_t35.c +++ b/board/compulab/cm_t35/cm_t35.c @@ -105,6 +105,22 @@ static inline int splash_load_from_nand(void) } #endif /* CONFIG_CMD_NAND */
+#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(struct board_sdrc_timings *timings) +{ + timings->mr = MICRON_V_MR_165; + timings->mcfg = MICRON_V_MCFG_200(256 << 20); /* raswidth 14 needed */ + timings->ctrla = MICRON_V_ACTIMA_165; + timings->ctrlb = MICRON_V_ACTIMB_165; + timings->rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz; +} +#endif + int splash_screen_prepare(void) { char *env_splashimage_value; @@ -428,7 +444,7 @@ void set_muxconf_regs(void) cm_t3730_set_muxconf(); }
-#ifdef CONFIG_GENERIC_MMC +#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD) int board_mmc_getcd(struct mmc *mmc) { u8 val; diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h index 39a216e..1eb57ba 100644 --- a/include/configs/cm_t35.h +++ b/include/configs/cm_t35.h @@ -25,8 +25,6 @@ #define CONFIG_OMAP_GPIO #define CONFIG_CM_T3X /* working with CM-T35 and CM-T3730 */
-#define CONFIG_SYS_TEXT_BASE 0x80008000 - #define CONFIG_SDRC /* The chip has SDRC controller */
#include <asm/arch/cpu.h> /* get chip and board defs */ @@ -325,4 +323,66 @@ #define CONFIG_CMD_BMP #define CONFIG_BMP_16BPP
+/* Defines for SPL */ +#define CONFIG_SPL +#define CONFIG_SPL_FRAMEWORK +#define CONFIG_SPL_NAND_SIMPLE + +#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_BOARD_INIT +#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_NAND_BASE +#define CONFIG_SPL_NAND_DRIVERS +#define CONFIG_SPL_NAND_ECC +#define CONFIG_SPL_GPIO_SUPPORT +#define CONFIG_SPL_POWER_SUPPORT +#define CONFIG_SPL_OMAP3_ID_NAND +#define CONFIG_SPL_LDSCRIPT "$(CPUDIR)/omap-common/u-boot-spl.lds" + +/* NAND boot config */ +#define CONFIG_SYS_NAND_5_ADDR_CYCLE +#define CONFIG_SYS_NAND_PAGE_COUNT 64 +#define CONFIG_SYS_NAND_PAGE_SIZE 2048 +#define CONFIG_SYS_NAND_OOBSIZE 64 +#define CONFIG_SYS_NAND_BLOCK_SIZE (128 * 1024) +#define CONFIG_SYS_NAND_BAD_BLOCK_POS NAND_LARGE_BADBLOCK_POS +/* + * Use the ECC/OOB layout from omap_gpmc.h that matches your chip: + * SP vs LP, 8bit vs 16bit: GPMC_NAND_HW_ECC_LAYOUT + */ +#define CONFIG_SYS_NAND_ECCPOS { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ + 10, 11, 12 } +#define CONFIG_SYS_NAND_ECCSIZE 512 +#define CONFIG_SYS_NAND_ECCBYTES 3 + +#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE +#define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000 + +#define CONFIG_SPL_TEXT_BASE 0x40200800 +#define CONFIG_SPL_MAX_SIZE (54 * 1024) /* 8 KB for stack */ +#define CONFIG_SPL_STACK LOW_LEVEL_SRAM_STACK + +/* + * Use 0x80008000 as TEXT_BASE here for compatibility reasons with the + * older x-loader implementations. And move the BSS area so that it + * doesn't overlap with TEXT_BASE. + */ +#define CONFIG_SYS_TEXT_BASE 0x80008000 +#define CONFIG_SPL_BSS_START_ADDR 0x80100000 +#define CONFIG_SPL_BSS_MAX_SIZE 0x80000 /* 512 KB */ + +#define CONFIG_SYS_SPL_MALLOC_START 0x80208000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000 + #endif /* __CONFIG_H */
Hi Stefan,
On Tue, 30 Jul 2013 12:52:10 +0200, Stefan Roese sr@denx.de wrote:
Add SPL U-Boot support to replace x-loader on the Compulab cm_t35 board. Currently only the 256MiB SDRAM board versions are supported.
Tested by booting via MMC and NAND.
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Igor Grinberg grinberg@compulab.co.il
v3:
- Some instability of this SDRAM setup has been detected while running Linux. Comparison with the x-loader setup showed that mcfg is configured slightly differently here. CM_T35 needs RAS-width of 14 instead of 13. So use the define MICRON_V_MCFG_200 which implements this 14 as RAS width.
v2:
I assume this means that your request to Tom about PRing V2 is withdrawn and I can apply his PR as-is. Correct?
Amicalement,
Hi Albert,
On 07/30/2013 02:10 PM, Albert ARIBAUD wrote:
Add SPL U-Boot support to replace x-loader on the Compulab cm_t35 board. Currently only the 256MiB SDRAM board versions are supported.
Tested by booting via MMC and NAND.
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Igor Grinberg grinberg@compulab.co.il
v3:
- Some instability of this SDRAM setup has been detected while running Linux. Comparison with the x-loader setup showed that mcfg is configured slightly differently here. CM_T35 needs RAS-width of 14 instead of 13. So use the define MICRON_V_MCFG_200 which implements this 14 as RAS width.
v2:
I assume this means that your request to Tom about PRing V2 is withdrawn and I can apply his PR as-is. Correct?
Yes. Please go ahead and pull. We can apply this patch at a later time.
Thanks, Stefan
Add SPL U-Boot support to replace x-loader on the Compulab cm_t35 board. Currently only the 256MiB SDRAM board versions are supported.
Tested by booting via MMC and NAND.
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Igor Grinberg grinberg@compulab.co.il --- v4: - Rebased and retested on current mainline version
v3: - Some instability of this SDRAM setup has been detected while running Linux. Comparison with the x-loader setup showed that mcfg is configured slightly differently here. CM_T35 needs RAS-width of 14 instead of 13. So use the define MICRON_V_MCFG_200 which implements this 14 as RAS width.
v2: - Change CONFIG_SYS_TEXT_BASE back to 0x80008000 for x-loader compatibility - Change CONFIG_SPL_BSS_START_ADDR to 0x80100000 to not overlap with TEXT_BASE now
board/compulab/cm_t35/cm_t35.c | 18 +++++++++++- include/configs/cm_t35.h | 64 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/board/compulab/cm_t35/cm_t35.c b/board/compulab/cm_t35/cm_t35.c index 9f2937a..2b4a64a 100644 --- a/board/compulab/cm_t35/cm_t35.c +++ b/board/compulab/cm_t35/cm_t35.c @@ -105,6 +105,22 @@ static inline int splash_load_from_nand(void) } #endif /* CONFIG_CMD_NAND */
+#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(struct board_sdrc_timings *timings) +{ + timings->mr = MICRON_V_MR_165; + timings->mcfg = MICRON_V_MCFG_200(256 << 20); /* raswidth 14 needed */ + timings->ctrla = MICRON_V_ACTIMA_165; + timings->ctrlb = MICRON_V_ACTIMB_165; + timings->rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz; +} +#endif + int splash_screen_prepare(void) { char *env_splashimage_value; @@ -440,7 +456,7 @@ void set_muxconf_regs(void) cm_t3730_set_muxconf(); }
-#ifdef CONFIG_GENERIC_MMC +#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD) int board_mmc_getcd(struct mmc *mmc) { u8 val; diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h index a6c63cb..d3c278f 100644 --- a/include/configs/cm_t35.h +++ b/include/configs/cm_t35.h @@ -27,8 +27,6 @@ #define CONFIG_CM_T3X /* working with CM-T35 and CM-T3730 */ #define CONFIG_OMAP_COMMON
-#define CONFIG_SYS_TEXT_BASE 0x80008000 - #define CONFIG_SDRC /* The chip has SDRC controller */
#include <asm/arch/cpu.h> /* get chip and board defs */ @@ -329,4 +327,66 @@
#define CONFIG_OMAP3_SPI
+/* Defines for SPL */ +#define CONFIG_SPL +#define CONFIG_SPL_FRAMEWORK +#define CONFIG_SPL_NAND_SIMPLE + +#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_BOARD_INIT +#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_NAND_BASE +#define CONFIG_SPL_NAND_DRIVERS +#define CONFIG_SPL_NAND_ECC +#define CONFIG_SPL_GPIO_SUPPORT +#define CONFIG_SPL_POWER_SUPPORT +#define CONFIG_SPL_OMAP3_ID_NAND +#define CONFIG_SPL_LDSCRIPT "$(CPUDIR)/omap-common/u-boot-spl.lds" + +/* NAND boot config */ +#define CONFIG_SYS_NAND_5_ADDR_CYCLE +#define CONFIG_SYS_NAND_PAGE_COUNT 64 +#define CONFIG_SYS_NAND_PAGE_SIZE 2048 +#define CONFIG_SYS_NAND_OOBSIZE 64 +#define CONFIG_SYS_NAND_BLOCK_SIZE (128 * 1024) +#define CONFIG_SYS_NAND_BAD_BLOCK_POS NAND_LARGE_BADBLOCK_POS +/* + * Use the ECC/OOB layout from omap_gpmc.h that matches your chip: + * SP vs LP, 8bit vs 16bit: GPMC_NAND_HW_ECC_LAYOUT + */ +#define CONFIG_SYS_NAND_ECCPOS { 1, 2, 3, 4, 5, 6, 7, 8, 9, \ + 10, 11, 12 } +#define CONFIG_SYS_NAND_ECCSIZE 512 +#define CONFIG_SYS_NAND_ECCBYTES 3 + +#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE +#define CONFIG_SYS_NAND_U_BOOT_OFFS 0x80000 + +#define CONFIG_SPL_TEXT_BASE 0x40200800 +#define CONFIG_SPL_MAX_SIZE (54 * 1024) /* 8 KB for stack */ +#define CONFIG_SPL_STACK LOW_LEVEL_SRAM_STACK + +/* + * Use 0x80008000 as TEXT_BASE here for compatibility reasons with the + * older x-loader implementations. And move the BSS area so that it + * doesn't overlap with TEXT_BASE. + */ +#define CONFIG_SYS_TEXT_BASE 0x80008000 +#define CONFIG_SPL_BSS_START_ADDR 0x80100000 +#define CONFIG_SPL_BSS_MAX_SIZE 0x80000 /* 512 KB */ + +#define CONFIG_SYS_SPL_MALLOC_START 0x80208000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000 + #endif /* __CONFIG_H */
Hi Stefan,
On Fri, 14 Jun 2013 10:54:59 +0200, Stefan Roese sr@denx.de wrote:
SPL already has GD set to the correct location (in s_init), we mustn't move it around now since some data (clocks etc) is already present.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net
arch/arm/lib/crt0.S | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index a9657d1..b05f66a 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -85,7 +85,13 @@ ENTRY(_main) bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ +#if !defined(CONFIG_SPL_BUILD) +/*
- SPL already has GD set to the correct location (in s_init), we mustn't
- move it around now since some data (clocks etc) is already present.
- */ mov r8, sp /* GD is above SP */
+#endif mov r0, #0 bl board_init_f
NAK in this form. I don't want gd to be set "somewhere in the code" depending on the actual target; I want it set in crt0.S, period.
I see there are several locations in ARM architecture or board code which set up GD themselves in the same manner as OMAP does. Luckily all these locations set it to the same value, the address of gdata.
The correct fix (read: the one I won't NAK) is thus to add a #else clause in the code above, in which r8 will be set to =gdata, and to remove the corresponding assignments in the various places where they reside.
(also, maybe not all SPLs want GD in gdata rather than on the stack; for instance, those SPLs loaded in DDR by some ROM code. Therefore, the whole gdata thing could possibly be placed under a specific condition such as CONFIG_SPL_GD_GLOBAL)
Amicalement,
Hi Albert,
On 20.06.2013 18:42, Albert ARIBAUD wrote:
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index a9657d1..b05f66a 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -85,7 +85,13 @@ ENTRY(_main) bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ +#if !defined(CONFIG_SPL_BUILD) +/*
- SPL already has GD set to the correct location (in s_init), we mustn't
- move it around now since some data (clocks etc) is already present.
- */ mov r8, sp /* GD is above SP */
+#endif mov r0, #0 bl board_init_f
NAK in this form. I don't want gd to be set "somewhere in the code" depending on the actual target; I want it set in crt0.S, period.
I see there are several locations in ARM architecture or board code which set up GD themselves in the same manner as OMAP does. Luckily all these locations set it to the same value, the address of gdata.
The correct fix (read: the one I won't NAK) is thus to add a #else clause in the code above, in which r8 will be set to =gdata, and to remove the corresponding assignments in the various places where they reside.
Here's the problem. Setting r8 in _main is too late. As it has already been used (in the current implementation) to store some data (e.g. clocks for baudrate generation etc). Here the code from arch/arm/cpu/armv7/omap3/board.c:s_init():
#ifdef CONFIG_SPL_BUILD gd = &gdata;
preloader_console_init();
timer_init(); #endif
Note that this is done *before* _main() is called (we are talking about SPL for OMAP here). And it did cost me quite some time to find this problem, that r8 was re-configured in _main() and all the already set values disappeared again (no serial output etc).
Yes, this needs some cleanup/fixup. Unfortunately I won't find the time to look into such a cleanup in the next days. Perhaps somebody else might jump in...
Thanks, Stefan
Hi Stefan,
On Thu, 20 Jun 2013 19:01:22 +0200, Stefan Roese sr@denx.de wrote:
Hi Albert,
On 20.06.2013 18:42, Albert ARIBAUD wrote:
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index a9657d1..b05f66a 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -85,7 +85,13 @@ ENTRY(_main) bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ +#if !defined(CONFIG_SPL_BUILD) +/*
- SPL already has GD set to the correct location (in s_init), we mustn't
- move it around now since some data (clocks etc) is already present.
- */ mov r8, sp /* GD is above SP */
+#endif mov r0, #0 bl board_init_f
NAK in this form. I don't want gd to be set "somewhere in the code" depending on the actual target; I want it set in crt0.S, period.
I see there are several locations in ARM architecture or board code which set up GD themselves in the same manner as OMAP does. Luckily all these locations set it to the same value, the address of gdata.
The correct fix (read: the one I won't NAK) is thus to add a #else clause in the code above, in which r8 will be set to =gdata, and to remove the corresponding assignments in the various places where they reside.
Here's the problem. Setting r8 in _main is too late. As it has already been used (in the current implementation) to store some data (e.g. clocks for baudrate generation etc). Here the code from arch/arm/cpu/armv7/omap3/board.c:s_init():
#ifdef CONFIG_SPL_BUILD gd = &gdata;
preloader_console_init();
timer_init(); #endif
Note that this is done *before* _main() is called (we are talking about SPL for OMAP here).
Yes, it is done before _main... right before it. Like, really *right* *before* *it*. Like, s_init is called at the very end of lowlevel_init, which was branched straight into from cpu_init_crit which itself is called just before _main.
In other words, after s_init(), _main immediately kicks in, sets up sp and r8 (which was done also in lowlevel_init and will thus be a no-op once gdata is handled in crt0.S too) and calls board_init_f().
So, calling s_init() last in lowlevel_init() would be the same as calling it first in board_init_f().
The difference with the current situation is, s_init() is C code, and C runtime is supposed not to be available until just before entering board_init_f(). This is the only reason why sp and r8 are set up in lowlevel_init: because s_init() is called at a time when C runtime is not officially set up yet.
Note that as far as setting the C runtime environment is concerned, crt0.S does *exactly* the same thing as lowlevel_init -- or will, once the gdata stuff is added in crt0.S as I suggest.
--- 8< --- So you just need to add the gdata stuff in crt0.S as I previously suggested, move the s_init() call in crt0.S (conditioned on building SPL) just before the call to board_init_f(), and then make lowlevel_init empty since it would then be useless. --- 8< ---
And it did cost me quite some time to find this problem, that r8 was re-configured in _main() and all the already set values disappeared again (no serial output etc).
Actually your trouble precisely shows why gd/r8 should only be touched in a single file and nowhere else: because it is set in many places, its value was hard to control.
Yes, this needs some cleanup/fixup. Unfortunately I won't find the time to look into such a cleanup in the next days. Perhaps somebody else might jump in...
There'll have to be a V2 for this patch anyway.
Here's my offer: you submit V2 of this patch as described above between the '--- 8< ---' markers, and I handle the scrubbing of all spurious assignments to gd myself. Deal?
Thanks, Stefan
Amicalement,
Hi Albert,
On 20.06.2013 19:51, Albert ARIBAUD wrote:
The correct fix (read: the one I won't NAK) is thus to add a #else clause in the code above, in which r8 will be set to =gdata, and to remove the corresponding assignments in the various places where they reside.
Here's the problem. Setting r8 in _main is too late. As it has already been used (in the current implementation) to store some data (e.g. clocks for baudrate generation etc). Here the code from arch/arm/cpu/armv7/omap3/board.c:s_init():
#ifdef CONFIG_SPL_BUILD gd = &gdata;
preloader_console_init();
timer_init(); #endif
Note that this is done *before* _main() is called (we are talking about SPL for OMAP here).
Yes, it is done before _main... right before it. Like, really *right* *before* *it*. Like, s_init is called at the very end of lowlevel_init, which was branched straight into from cpu_init_crit which itself is called just before _main.
In other words, after s_init(), _main immediately kicks in, sets up sp and r8 (which was done also in lowlevel_init and will thus be a no-op once gdata is handled in crt0.S too) and calls board_init_f().
So, calling s_init() last in lowlevel_init() would be the same as calling it first in board_init_f().
The difference with the current situation is, s_init() is C code, and C runtime is supposed not to be available until just before entering board_init_f(). This is the only reason why sp and r8 are set up in lowlevel_init: because s_init() is called at a time when C runtime is not officially set up yet.
Note that as far as setting the C runtime environment is concerned, crt0.S does *exactly* the same thing as lowlevel_init -- or will, once the gdata stuff is added in crt0.S as I suggest.
--- 8< --- So you just need to add the gdata stuff in crt0.S as I previously suggested, move the s_init() call in crt0.S (conditioned on building SPL) just before the call to board_init_f(), and then make lowlevel_init empty since it would then be useless. --- 8< ---
Useless? Its still calling cpy_clk_code(). Isn't this needed anymore?
And it did cost me quite some time to find this problem, that r8 was re-configured in _main() and all the already set values disappeared again (no serial output etc).
Actually your trouble precisely shows why gd/r8 should only be touched in a single file and nowhere else: because it is set in many places, its value was hard to control.
Full ack on this.
Yes, this needs some cleanup/fixup. Unfortunately I won't find the time to look into such a cleanup in the next days. Perhaps somebody else might jump in...
There'll have to be a V2 for this patch anyway.
Here's my offer: you submit V2 of this patch as described above between the '--- 8< ---' markers, and I handle the scrubbing of all spurious assignments to gd myself. Deal?
Yes, I'll try to squeeze a few cycles from the other projects to get this done hopefully tomorrow.
Thanks, Stefan
Hi Stefan,
On Thu, 20 Jun 2013 20:28:01 +0200, Stefan Roese sr@denx.de wrote:
Hi Albert,
On 20.06.2013 19:51, Albert ARIBAUD wrote:
The correct fix (read: the one I won't NAK) is thus to add a #else clause in the code above, in which r8 will be set to =gdata, and to remove the corresponding assignments in the various places where they reside.
Here's the problem. Setting r8 in _main is too late. As it has already been used (in the current implementation) to store some data (e.g. clocks for baudrate generation etc). Here the code from arch/arm/cpu/armv7/omap3/board.c:s_init():
#ifdef CONFIG_SPL_BUILD gd = &gdata;
preloader_console_init();
timer_init(); #endif
Note that this is done *before* _main() is called (we are talking about SPL for OMAP here).
Yes, it is done before _main... right before it. Like, really *right* *before* *it*. Like, s_init is called at the very end of lowlevel_init, which was branched straight into from cpu_init_crit which itself is called just before _main.
In other words, after s_init(), _main immediately kicks in, sets up sp and r8 (which was done also in lowlevel_init and will thus be a no-op once gdata is handled in crt0.S too) and calls board_init_f().
So, calling s_init() last in lowlevel_init() would be the same as calling it first in board_init_f().
The difference with the current situation is, s_init() is C code, and C runtime is supposed not to be available until just before entering board_init_f(). This is the only reason why sp and r8 are set up in lowlevel_init: because s_init() is called at a time when C runtime is not officially set up yet.
Note that as far as setting the C runtime environment is concerned, crt0.S does *exactly* the same thing as lowlevel_init -- or will, once the gdata stuff is added in crt0.S as I suggest.
--- 8< --- So you just need to add the gdata stuff in crt0.S as I previously suggested, move the s_init() call in crt0.S (conditioned on building SPL) just before the call to board_init_f(), and then make lowlevel_init empty since it would then be useless. --- 8< ---
Useless? Its still calling cpy_clk_code(). Isn't this needed anymore?
My bad -- I was looking at arch/arm/cpu/armv7/lowlevel_init.S, not arch/arm/cpu/armv7/omap3/lowlevel_init.S.
And it did cost me quite some time to find this problem, that r8 was re-configured in _main() and all the already set values disappeared again (no serial output etc).
Actually your trouble precisely shows why gd/r8 should only be touched in a single file and nowhere else: because it is set in many places, its value was hard to control.
Full ack on this.
Yes, this needs some cleanup/fixup. Unfortunately I won't find the time to look into such a cleanup in the next days. Perhaps somebody else might jump in...
There'll have to be a V2 for this patch anyway.
Here's my offer: you submit V2 of this patch as described above between the '--- 8< ---' markers, and I handle the scrubbing of all spurious assignments to gd myself. Deal?
Yes, I'll try to squeeze a few cycles from the other projects to get this done hopefully tomorrow.
Thank you.
Thanks, Stefan
Amicalement,
Fix a problem with a re-assignment of r8 in the SPL version.
This patch now moves the call to s_init() to a later stage, right before calling board_init_f(). And makes sure that r8 is correctly initialized before s_init() is called. r8 now is only written in crt0.S.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net --- v2: - Change handling/initializing of r8 as suggested by Albert. It should only be written in crt0.S.
Tom, while working on this version one question came up: Is lowlevel_init() (file arch/arm/cpu/armv7/omap3/lowlevel_init.S) needed any more? It calls cpy_clk_code() to copy some clk init code into SRAM. But I fail to see if and where this code is really executed from SRAM. Maybe I missed something. Perhaps you could shed some light into this.
Thanks, Stefan
arch/arm/cpu/armv7/omap3/board.c | 2 -- arch/arm/cpu/armv7/omap3/lowlevel_init.S | 3 +-- arch/arm/lib/crt0.S | 7 ++++++- 3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c index b72fadc..8f41dcd 100644 --- a/arch/arm/cpu/armv7/omap3/board.c +++ b/arch/arm/cpu/armv7/omap3/board.c @@ -256,8 +256,6 @@ void s_init(void) #endif
#ifdef CONFIG_SPL_BUILD - gd = &gdata; - preloader_console_init();
timer_init(); diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index eacfef8..8539093 100644 --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S @@ -226,8 +226,7 @@ ENTRY(lowlevel_init) #endif /* NAND Boot */ mov lr, ip /* restore link reg */ ldr ip, [sp] /* restore save ip */ - /* tail-call s_init to setup pll, mux, memory */ - b s_init + mov pc, lr
ENDPROC(lowlevel_init)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index a5bffb8..0f8d9f5 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -85,8 +85,13 @@ ENTRY(_main) bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ - mov r8, sp /* GD is above SP */ mov r0, #0 +#if defined(CONFIG_SPL_BUILD) + ldr r8, =gdata /* SPL assigns r8 directly to &gdata */ + bl s_init /* s_init() needs GD to be setup */ +#else + mov r8, sp /* GD is above SP */ +#endif bl board_init_f
#if ! defined(CONFIG_SPL_BUILD)
Hi Stefan,
On Fri, 21 Jun 2013 04:13:17 +0200, Stefan Roese sr@denx.de wrote:
Fix a problem with a re-assignment of r8 in the SPL version.
This patch now moves the call to s_init() to a later stage, right before calling board_init_f(). And makes sure that r8 is correctly initialized before s_init() is called. r8 now is only written in crt0.S.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net
v2:
- Change handling/initializing of r8 as suggested by Albert. It should only be written in crt0.S.
Tom, while working on this version one question came up: Is lowlevel_init() (file arch/arm/cpu/armv7/omap3/lowlevel_init.S) needed any more? It calls cpy_clk_code() to copy some clk init code into SRAM. But I fail to see if and where this code is really executed from SRAM. Maybe I missed something. Perhaps you could shed some light into this.
Thanks, Stefan
arch/arm/cpu/armv7/omap3/board.c | 2 -- arch/arm/cpu/armv7/omap3/lowlevel_init.S | 3 +-- arch/arm/lib/crt0.S | 7 ++++++- 3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c index b72fadc..8f41dcd 100644 --- a/arch/arm/cpu/armv7/omap3/board.c +++ b/arch/arm/cpu/armv7/omap3/board.c @@ -256,8 +256,6 @@ void s_init(void) #endif
#ifdef CONFIG_SPL_BUILD
gd = &gdata;
preloader_console_init();
timer_init();
diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index eacfef8..8539093 100644 --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S @@ -226,8 +226,7 @@ ENTRY(lowlevel_init) #endif /* NAND Boot */ mov lr, ip /* restore link reg */ ldr ip, [sp] /* restore save ip */
- /* tail-call s_init to setup pll, mux, memory */
- b s_init
- mov pc, lr
ENDPROC(lowlevel_init)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index a5bffb8..0f8d9f5 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -85,8 +85,13 @@ ENTRY(_main) bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
Actually, this...
sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
... should also be moved under the #else clause below -- no need to eat up valuable stack space when GD is in gdata.
- mov r8, sp /* GD is above SP */
Also, this...
mov r0, #0
... should remain just before the call to board_init_f which needs it. Otherwise you run the (non-null) risk that r0 be non-zero on return from s_init and then this non-zero value be passed to board_init_f.
+#if defined(CONFIG_SPL_BUILD)
- ldr r8, =gdata /* SPL assigns r8 directly to &gdata */
- bl s_init /* s_init() needs GD to be setup */
+#else
- mov r8, sp /* GD is above SP */
+#endif bl board_init_f
#if ! defined(CONFIG_SPL_BUILD)
Amicalement,
SPL already has GD set to the correct location (in s_init), we mustn't move it around now since some data (clocks etc) is already present.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net --- v3: - Some code shuffling in crt0.S as requested by Albert
v2: - Change handling/initializing of r8 as suggested by Albert. It should only be written in crt0.S.
Tom, while working on this version one question came up: Is lowlevel_init() (file arch/arm/cpu/armv7/omap3/lowlevel_init.S) needed any more? It calls cpy_clk_code() to copy some clk init code into SRAM. But I fail to see if and where this code is really executed from SRAM. Maybe I missed something. Perhaps you could shed some light into this.
Thanks, Stefan
arch/arm/cpu/armv7/omap3/board.c | 2 -- arch/arm/cpu/armv7/omap3/lowlevel_init.S | 3 +-- arch/arm/lib/crt0.S | 5 +++++ 3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c index b72fadc..8f41dcd 100644 --- a/arch/arm/cpu/armv7/omap3/board.c +++ b/arch/arm/cpu/armv7/omap3/board.c @@ -256,8 +256,6 @@ void s_init(void) #endif
#ifdef CONFIG_SPL_BUILD - gd = &gdata; - preloader_console_init();
timer_init(); diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index eacfef8..8539093 100644 --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S @@ -226,8 +226,7 @@ ENTRY(lowlevel_init) #endif /* NAND Boot */ mov lr, ip /* restore link reg */ ldr ip, [sp] /* restore save ip */ - /* tail-call s_init to setup pll, mux, memory */ - b s_init + mov pc, lr
ENDPROC(lowlevel_init)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index a5bffb8..9bd7c24 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -83,9 +83,14 @@ ENTRY(_main) ldr sp, =(CONFIG_SYS_INIT_SP_ADDR) #endif bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ +#if defined(CONFIG_SPL_BUILD) + ldr r8, =gdata /* SPL assigns r8 directly to &gdata */ + bl s_init /* s_init() needs GD to be setup */ +#else sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ mov r8, sp /* GD is above SP */ +#endif mov r0, #0 bl board_init_f
Hi Stefan,
On Fri, 21 Jun 2013 11:10:10 +0200, Stefan Roese sr@denx.de wrote:
SPL already has GD set to the correct location (in s_init), we mustn't move it around now since some data (clocks etc) is already present.
Actually the commit message is not accurate any more, as it states s_init should keep setting gd -- sorry for missing this so far, maybe Tom can fix the message while applying?
Otherwise:
Acked-by: Albert ARIBAUD albert.u.boot@aribaud.net
Amicalement,
On 21.06.2013 12:30, Albert ARIBAUD wrote:
SPL already has GD set to the correct location (in s_init), we mustn't move it around now since some data (clocks etc) is already present.
Actually the commit message is not accurate any more, as it states s_init should keep setting gd -- sorry for missing this so far, maybe Tom can fix the message while applying?
Argh! I had the commit message changed in v2 but forgot it in v3. Will send v4 shortly.
Thanks, Stefan
Fix a problem with a re-assignment of r8 in the SPL version.
This patch now moves the call to s_init() to a later stage, right before calling board_init_f(). And makes sure that r8 is correctly initialized before s_init() is called. r8 now is only written in crt0.S.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Acked-by: Albert ARIBAUD albert.u.boot@aribaud.net --- v4: - Corrected commit text to reflect changed patch
v3: - Some code shuffling in crt0.S as requested by Albert
v2: - Change handling/initializing of r8 as suggested by Albert. It should only be written in crt0.S.
Tom, while working on this version one question came up: Is lowlevel_init() (file arch/arm/cpu/armv7/omap3/lowlevel_init.S) needed any more? It calls cpy_clk_code() to copy some clk init code into SRAM. But I fail to see if and where this code is really executed from SRAM. Maybe I missed something. Perhaps you could shed some light into this.
Thanks, Stefan
arch/arm/cpu/armv7/omap3/board.c | 2 -- arch/arm/cpu/armv7/omap3/lowlevel_init.S | 3 +-- arch/arm/lib/crt0.S | 5 +++++ 3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c index b72fadc..8f41dcd 100644 --- a/arch/arm/cpu/armv7/omap3/board.c +++ b/arch/arm/cpu/armv7/omap3/board.c @@ -256,8 +256,6 @@ void s_init(void) #endif
#ifdef CONFIG_SPL_BUILD - gd = &gdata; - preloader_console_init();
timer_init(); diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index eacfef8..8539093 100644 --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S @@ -226,8 +226,7 @@ ENTRY(lowlevel_init) #endif /* NAND Boot */ mov lr, ip /* restore link reg */ ldr ip, [sp] /* restore save ip */ - /* tail-call s_init to setup pll, mux, memory */ - b s_init + mov pc, lr
ENDPROC(lowlevel_init)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index a5bffb8..9bd7c24 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -83,9 +83,14 @@ ENTRY(_main) ldr sp, =(CONFIG_SYS_INIT_SP_ADDR) #endif bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ +#if defined(CONFIG_SPL_BUILD) + ldr r8, =gdata /* SPL assigns r8 directly to &gdata */ + bl s_init /* s_init() needs GD to be setup */ +#else sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ mov r8, sp /* GD is above SP */ +#endif mov r0, #0 bl board_init_f
Hi Stefan,
On Fri, 21 Jun 2013 12:42:46 +0200, Stefan Roese sr@denx.de wrote:
Fix a problem with a re-assignment of r8 in the SPL version.
This patch now moves the call to s_init() to a later stage, right before calling board_init_f(). And makes sure that r8 is correctly initialized before s_init() is called. r8 now is only written in crt0.S.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Acked-by: Albert ARIBAUD albert.u.boot@aribaud.net
v4:
- Corrected commit text to reflect changed patch
v3:
- Some code shuffling in crt0.S as requested by Albert
v2:
- Change handling/initializing of r8 as suggested by Albert. It should only be written in crt0.S.
Tom, while working on this version one question came up: Is lowlevel_init() (file arch/arm/cpu/armv7/omap3/lowlevel_init.S) needed any more? It calls cpy_clk_code() to copy some clk init code into SRAM. But I fail to see if and where this code is really executed from SRAM. Maybe I missed something. Perhaps you could shed some light into this.
Thanks, Stefan
arch/arm/cpu/armv7/omap3/board.c | 2 -- arch/arm/cpu/armv7/omap3/lowlevel_init.S | 3 +-- arch/arm/lib/crt0.S | 5 +++++ 3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c index b72fadc..8f41dcd 100644 --- a/arch/arm/cpu/armv7/omap3/board.c +++ b/arch/arm/cpu/armv7/omap3/board.c @@ -256,8 +256,6 @@ void s_init(void) #endif
#ifdef CONFIG_SPL_BUILD
gd = &gdata;
preloader_console_init();
timer_init();
diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index eacfef8..8539093 100644 --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S @@ -226,8 +226,7 @@ ENTRY(lowlevel_init) #endif /* NAND Boot */ mov lr, ip /* restore link reg */ ldr ip, [sp] /* restore save ip */
- /* tail-call s_init to setup pll, mux, memory */
- b s_init
- mov pc, lr
ENDPROC(lowlevel_init)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index a5bffb8..9bd7c24 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -83,9 +83,14 @@ ENTRY(_main) ldr sp, =(CONFIG_SYS_INIT_SP_ADDR) #endif bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ +#if defined(CONFIG_SPL_BUILD)
- ldr r8, =gdata /* SPL assigns r8 directly to &gdata */
- bl s_init /* s_init() needs GD to be setup */
+#else sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ mov r8, sp /* GD is above SP */ +#endif mov r0, #0 bl board_init_f
Acked-by: Albert ARIBAUD albert.u.boot@aribaud.net
Amicalement,
Fix a problem with a re-assignment of r8 in the SPL version.
This patch now moves the call to s_init() to a later stage, right before calling board_init_f(). And makes sure that r8 is correctly initialized before s_init() is called. r8 now is only written in crt0.S.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net --- Albert, I'm not really happy with this patch as it evolves now. As you will see, I had to make some further additions to crt0.S to fix a problem for non-SPL builds and to fix compilation errors for non-OMAP platforms. This gets quite ugly now. Looking back at my patch v1, this looks much less intrusive.
What do you think?
v5: - Fix a problem with non-SPL build on OMAP3 where s_init() needs to get called (if CONFIG_SKIP_LOWLEVEL_INIT is not defined). With the "code shuffling" in v3, s_init() was only called in the SPL U-Boot version. - Fix compilation problem for non-OMAP platforms with assigning gdata to r8 (e.g. tx25).
v4: - Corrected commit text to reflect changed patch
v3: - Some code shuffling in crt0.S as requested by Albert
v2: - Change handling/initializing of r8 as suggested by Albert. It should only be written in crt0.S.
arch/arm/cpu/armv7/omap3/board.c | 2 -- arch/arm/cpu/armv7/omap3/lowlevel_init.S | 3 +-- arch/arm/lib/crt0.S | 8 ++++++++ 3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap3/board.c b/arch/arm/cpu/armv7/omap3/board.c index b72fadc..8f41dcd 100644 --- a/arch/arm/cpu/armv7/omap3/board.c +++ b/arch/arm/cpu/armv7/omap3/board.c @@ -256,8 +256,6 @@ void s_init(void) #endif
#ifdef CONFIG_SPL_BUILD - gd = &gdata; - preloader_console_init();
timer_init(); diff --git a/arch/arm/cpu/armv7/omap3/lowlevel_init.S b/arch/arm/cpu/armv7/omap3/lowlevel_init.S index eacfef8..8539093 100644 --- a/arch/arm/cpu/armv7/omap3/lowlevel_init.S +++ b/arch/arm/cpu/armv7/omap3/lowlevel_init.S @@ -226,8 +226,7 @@ ENTRY(lowlevel_init) #endif /* NAND Boot */ mov lr, ip /* restore link reg */ ldr ip, [sp] /* restore save ip */ - /* tail-call s_init to setup pll, mux, memory */ - b s_init + mov pc, lr
ENDPROC(lowlevel_init)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index a5bffb8..78eb951 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -83,9 +83,17 @@ ENTRY(_main) ldr sp, =(CONFIG_SYS_INIT_SP_ADDR) #endif bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ +#if defined(CONFIG_OMAP34XX) && defined(CONFIG_SPL_BUILD) + ldr r8, =gdata /* SPL assigns r8 directly to &gdata */ +#else sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ mov r8, sp /* GD is above SP */ +#endif +#if defined(CONFIG_OMAP34XX) && \ + (defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SKIP_LOWLEVEL_INIT)) + bl s_init /* s_init() needs GD to be setup */ +#endif mov r0, #0 bl board_init_f
Hi Stefan,
On Tue, 25 Jun 2013 09:14:12 +0200, Stefan Roese sr@denx.de wrote:
Fix a problem with a re-assignment of r8 in the SPL version.
This patch now moves the call to s_init() to a later stage, right before calling board_init_f(). And makes sure that r8 is correctly initialized before s_init() is called. r8 now is only written in crt0.S.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net
Albert, I'm not really happy with this patch as it evolves now. As you will see, I had to make some further additions to crt0.S to fix a problem for non-SPL builds and to fix compilation errors for non-OMAP platforms. This gets quite ugly now. Looking back at my patch v1, this looks much less intrusive.
What do you think?
I said the first patch was NAK, and the reasons I NAKed it remain.
However, there might be another solution: instead of squeezing the call to s_init() in crt0.S right between the initial environment setting and the call to board_init_f(), we could simply move the s_init() call inside board_init_f().
From a running conditions perspective, the only change would be that s_init() is going to run from a non-empty stack, but we know that there is free stack enough during board_init_f() to call functions.
Moving the call to s_init() into board_init_f() removes any changes to crt0.S, which were my essential NAK reason and saves you some ugliness.
I would even hazard that you could place s_init in init_sequence[], for instance as a first entry to be called (before arch_cpu_init). After all, the only difference in execution is that gdata is going to be initialized properly before s_init() kicks in.
Also, a name change would be in order, because s_init() as a private OMAP function is ok, but as an init function invoked from board_init_f() it needs a more meaningful name.
Amicalement,
On Thu, Jun 27, 2013 at 10:27:26AM +0200, Albert ARIBAUD wrote:
Hi Stefan,
On Tue, 25 Jun 2013 09:14:12 +0200, Stefan Roese sr@denx.de wrote:
Fix a problem with a re-assignment of r8 in the SPL version.
This patch now moves the call to s_init() to a later stage, right before calling board_init_f(). And makes sure that r8 is correctly initialized before s_init() is called. r8 now is only written in crt0.S.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net
Albert, I'm not really happy with this patch as it evolves now. As you will see, I had to make some further additions to crt0.S to fix a problem for non-SPL builds and to fix compilation errors for non-OMAP platforms. This gets quite ugly now. Looking back at my patch v1, this looks much less intrusive.
What do you think?
I said the first patch was NAK, and the reasons I NAKed it remain.
However, there might be another solution: instead of squeezing the call to s_init() in crt0.S right between the initial environment setting and the call to board_init_f(), we could simply move the s_init() call inside board_init_f().
From a running conditions perspective, the only change would be that s_init() is going to run from a non-empty stack, but we know that there is free stack enough during board_init_f() to call functions.
Moving the call to s_init() into board_init_f() removes any changes to crt0.S, which were my essential NAK reason and saves you some ugliness.
I would even hazard that you could place s_init in init_sequence[], for instance as a first entry to be called (before arch_cpu_init). After all, the only difference in execution is that gdata is going to be initialized properly before s_init() kicks in.
Also, a name change would be in order, because s_init() as a private OMAP function is ok, but as an init function invoked from board_init_f() it needs a more meaningful name.
So, this is one of the things that needs resolving for v2013.07. What do you want to call the renamed s_init function? I think we need to go the init_sequence route, and keep common/board_f.c in sync (I did a trivial test the other week about moving am335x into the generic board framework and it went fine, so I'll want to move all the TI boards I can over soon). Thanks!
Hi Tom,
On Wed, 3 Jul 2013 15:47:27 -0400, Tom Rini trini@ti.com wrote:
On Thu, Jun 27, 2013 at 10:27:26AM +0200, Albert ARIBAUD wrote:
Hi Stefan,
On Tue, 25 Jun 2013 09:14:12 +0200, Stefan Roese sr@denx.de wrote:
Fix a problem with a re-assignment of r8 in the SPL version.
This patch now moves the call to s_init() to a later stage, right before calling board_init_f(). And makes sure that r8 is correctly initialized before s_init() is called. r8 now is only written in crt0.S.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net
Albert, I'm not really happy with this patch as it evolves now. As you will see, I had to make some further additions to crt0.S to fix a problem for non-SPL builds and to fix compilation errors for non-OMAP platforms. This gets quite ugly now. Looking back at my patch v1, this looks much less intrusive.
What do you think?
I said the first patch was NAK, and the reasons I NAKed it remain.
However, there might be another solution: instead of squeezing the call to s_init() in crt0.S right between the initial environment setting and the call to board_init_f(), we could simply move the s_init() call inside board_init_f().
From a running conditions perspective, the only change would be that s_init() is going to run from a non-empty stack, but we know that there is free stack enough during board_init_f() to call functions.
Moving the call to s_init() into board_init_f() removes any changes to crt0.S, which were my essential NAK reason and saves you some ugliness.
I would even hazard that you could place s_init in init_sequence[], for instance as a first entry to be called (before arch_cpu_init). After all, the only difference in execution is that gdata is going to be initialized properly before s_init() kicks in.
Also, a name change would be in order, because s_init() as a private OMAP function is ok, but as an init function invoked from board_init_f() it needs a more meaningful name.
So, this is one of the things that needs resolving for v2013.07. What do you want to call the renamed s_init function? I think we need to go the init_sequence route, and keep common/board_f.c in sync (I did a trivial test the other week about moving am335x into the generic board framework and it went fine, so I'll want to move all the TI boards I can over soon). Thanks!
I have no strong opinion on the name... As is does mux and clock inits needed by the 'system' (which for all I know may well be where the "s" of s_init comes from), we could simply name it system_init().
Amicalement,
On Fri, Jun 14, 2013 at 10:54:59AM +0200, Stefan Roese wrote:
SPL already has GD set to the correct location (in s_init), we mustn't move it around now since some data (clocks etc) is already present.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net
While we have a problem here, it's not a problem that's visible on current SPL using platforms, just the CM-T35, yes? I just gave my beagleboard (classic and xM) a spin and they're OK. I've got a few more platforms I can dig out if needed, but I'm inclined to hold this until after v2013.07 and we can take one of the paths Albert outlined (change s_init to system_init, add to the function table, call that way). Does that work or have I underestimated the impact of this issue? Thanks!
Hi Tom,
On 07/15/2013 04:33 PM, Tom Rini wrote:
SPL already has GD set to the correct location (in s_init), we mustn't move it around now since some data (clocks etc) is already present.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net
While we have a problem here, it's not a problem that's visible on current SPL using platforms, just the CM-T35, yes?
Thats what I noticed as well. I was a bit astonished that I didn't see it on beagle. It should hit there too.
I just gave my beagleboard (classic and xM) a spin and they're OK.
Did you also power-cycle the board (cold boot)?
I've got a few more platforms I can dig out if needed, but I'm inclined to hold this until after v2013.07 and we can take one of the paths Albert outlined (change s_init to system_init, add to the function table, call that way). Does that work or have I underestimated the impact of this issue? Thanks!
I can definitely live with postponing this solution/fix to after this release. Since your tests on the beagle boards are also working fine, then lets just hold this patch and release v2013.07 now.
Thanks, Stefan
On Tue, Jul 16, 2013 at 08:24:48AM +0200, Stefan Roese wrote:
Hi Tom,
On 07/15/2013 04:33 PM, Tom Rini wrote:
SPL already has GD set to the correct location (in s_init), we mustn't move it around now since some data (clocks etc) is already present.
This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by: Stefan Roese sr@denx.de Cc: Tom Rini trini@ti.com Cc: Albert ARIBAUD albert.u.boot@aribaud.net
While we have a problem here, it's not a problem that's visible on current SPL using platforms, just the CM-T35, yes?
Thats what I noticed as well. I was a bit astonished that I didn't see it on beagle. It should hit there too.
I just gave my beagleboard (classic and xM) a spin and they're OK.
Did you also power-cycle the board (cold boot)?
Yes, both SD and NAND boot on classic and SD boot on xM were cold boots.
I've got a few more platforms I can dig out if needed, but I'm inclined to hold this until after v2013.07 and we can take one of the paths Albert outlined (change s_init to system_init, add to the function table, call that way). Does that work or have I underestimated the impact of this issue? Thanks!
I can definitely live with postponing this solution/fix to after this release. Since your tests on the beagle boards are also working fine, then lets just hold this patch and release v2013.07 now.
OK, thanks.
participants (5)
-
Albert ARIBAUD -
Igor Grinberg -
Nikita Kiryanov -
Stefan Roese -
Tom Rini