arm: spl: Fix SPL booting for OMAP3
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).
--- arch/arm/lib/crt0.S | 6 ++++++ 1 file changed, 6 insertions(+)
arch/arm/lib/crt0.S +6 -0
@@ -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
Signature · 1 line
1.8.2.3
PATCH 2/3 arm: omap3: spl: Fix problem with 8bit NAND devices
Currently in OMAP3 SPL, the GPMC for NAND is configured for 16bit access. This patch adds support for 8bit NAND devices as well.
--- arch/arm/cpu/armv7/omap3/mem.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
arch/arm/cpu/armv7/omap3/mem.c +12 -0
@@ -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 )
Signature · 1 line
1.8.2.3
PATCH 2/3 arm: omap3: spl: Fix problem with 8bit NAND devices
Quoted 5 lines · Stefan Roese
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 <[email protected]>Cc Tom Rini <[email protected]>
Applied to u-boot-ti/master, thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130730/d268f253/attachment.pgp>
PATCH 3/3 arm: omap3: Add SPL support to cm_t35
In reply to Stefan RoeseAdd 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.
--- board/compulab/cm_t35/cm_t35.c | 18 +++++++++++- include/configs/cm_t35.h | 64 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 3 deletions(-)
board/compulab/cm_t35/cm_t35.c +17 -1
@@ -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;
include/configs/cm_t35.h +62 -2
74 lines not on this page · raw message
Signature · 1 line
1.8.2.3
PATCH 3/3 arm: omap3: Add SPL support to cm_t35
Hi Stefan,
Quoted 26 lines · Stefan Roese
Add SPL U-Boot support to replace x-loader on the Compulab
cm_t35board. Currently only the 256MiB SDRAM board versions are supported.Tested by booting via MMC and NAND.
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Cc Igor Grinberg <[email protected]>--- board/compulab/cm_t35/cm_t35.c | 18 +++++++++++- include/configs/cm_t35.h | 64 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 3 deletions(-)board/compulab/cm_t35/cm_t35.c +5
-0@@ -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.
board/compulab/cm_t35/cm_t35.c static inline int splash_load_from_nand(void) · line 137
Quoted 10 lines
board/compulab/cm_t35/cm_t35.c +10
-0+*/ +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.
Quoted 79 lines
board/compulab/cm_t35/cm_t35.c +2
-1+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;include/configs/cm_t35.h +49
-2@@ -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 +
include/configs/cm_t35.h line 400
Quoted 10 lines
include/configs/cm_t35.h +10
-0+#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...
Quoted 5 lines
include/configs/cm_t35.h +3
-0+#define CONFIG_SYS_SPL_MALLOC_START 0x80208000 +#define CONFIG_SYS_SPL_MALLOC_SIZE 0x100000 +#endif /* __CONFIG_H */
Signature · 2 lines
Regards, Igor.
PATCH 3/3 arm: omap3: Add SPL support to cm_t35
Quoted 11 lines · Igor Grinberg
Hi Stefan,
Quoted 8 lines · Stefan Roese
Add SPL U-Boot support to replace x-loader on the Compulab
cm_t35board. Currently only the 256MiB SDRAM board versions are supported.Tested by booting via MMC and NAND.
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Cc Igor Grinberg <[email protected]>
[snip]
Quoted 11 lines
include/configs/cm_t35.h line 400
Quoted 7 lines
include/configs/cm_t35.h +7
-0+/* +* 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 0x80100000Now 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.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130617/02c25f47/attachment.pgp>
PATCH 3/3 arm: omap3: Add SPL support to cm_t35
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 25 lines · Tom Rini
Quoted 11 lines · Igor Grinberg
Hi Stefan,
Quoted 8 lines · Stefan Roese
Add SPL U-Boot support to replace x-loader on the Compulab
cm_t35board. Currently only the 256MiB SDRAM board versions are supported.Tested by booting via MMC and NAND.
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Cc Igor Grinberg <[email protected]>[snip]
Quoted 11 lines
include/configs/cm_t35.h line 400
Quoted 7 lines
include/configs/cm_t35.h +7
-0+/* +* 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 0x80100000Now this is a problem.
This breaks the backward compatibility with our X-Loader and we cannot just switch to 80100000...
Quoted 4 lines
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.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)
iQIcBAEBAgAGBQJRvxFwAAoJEBDE8YO64Efa0DsP/R0053VMz10juUsP38dFb7lb xcKQFmvJe0W1skIJZmFAa+1gUNYf1EMcHGx7JqIwtsVDBsHxK5fzAbj1mU4RbQUx zY5nmGtKlfiV1xPMQSrwo6rDxX3pPRId5rfC9sxpCwdliCFzvypfP6mPTcGm10db 2MygHvm+cXU1hNi7JPF3eRCVuqDhO/mrnES0MD2tkxfF88sJOYvvCBSLndJy+cF2 8FrnAC1w+4xnUqT33VTAf/P1+/2qzFcnrVXRNBn/fRfvahT91EuGwWrzqRrvvwNd lMTwSreO4RvvG6pa0RGgu6I6/6Ao5CYbjkznDzeyMl+001a2hxIlwuXb+yJaNneN lISSWSAnTT6AcoIRDGVOmFkWOzfmgZoqsikeBKA22jyzZymoXWwhJ5RM5/l0/OKu V7/X2vWuzTeAzbOlRMof25DtLk/vXdYA4vt6b9iyIffVPSq9WWGGJhPhgSfQ5rW1 S8k1JXbIn99TY6igyaK+7rf42FxOMZLyPdKH53qCM2G80XGy+Df7y2ZHyv8ssQRY f5w2CYSDo4ph01tGo3hI5bvz671yAoBUs6+/7ERofJ9gx2R0XOKUhfY6znIXeDMl o8AqyYpXTboDG9FRH0cE6Lud4CUt2sY2NQ/CkfdXyYIfeOGVqWUbLefc9cZNBGDe 4AoUHehr5tgIFNiT5yP+
=X70e
-----END PGP SIGNATURE-----
PATCH 3/3 arm: omap3: Add SPL support to cm_t35
Quoted 12 lines · Igor Grinberg
Quoted 12 lines
Quoted 11 lines
include/configs/cm_t35.h line 400
Quoted 7 lines
include/configs/cm_t35.h +7
-0+/* +* 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 0x80100000Now this is a problem.
This breaks the backward compatibility with our X-Loader and we cannot just switch to 80100000...
Quoted 8 lines
Quoted 4 lines
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
PATCH v2 3/3 arm: omap3: Add SPL support to cm_t35
In reply to Stefan RoeseAdd 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.
---
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(-)
board/compulab/cm_t35/cm_t35.c +17 -1
@@ -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;
include/configs/cm_t35.h +62 -2
74 lines not on this page · raw message
Signature · 1 line
1.8.2.3
PATCH v2 3/3 arm: omap3: Add SPL support to cm_t35
This version works when booting with X-Loader.
Quoted 136 lines · Stefan Roese
Add SPL U-Boot support to replace x-loader on the Compulab
cm_t35board. Currently only the 256MiB SDRAM board versions are supported.Tested by booting via MMC and NAND.
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Cc Igor Grinberg <[email protected]>---
v2:
- ChangeCONFIG_SYS_TEXT_BASEback to0x80008000for x-loader compatibility
- ChangeCONFIG_SPL_BSS_START_ADDRto0x80100000to not overlap withTEXT_BASEnowboard/compulab/cm_t35/cm_t35.c | 18 +++++++++++- include/configs/cm_t35.h | 64 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 3 deletions(-)board/compulab/cm_t35/cm_t35.c +17
-1@@ -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;include/configs/cm_t35.h +62
-2@@ -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 */
Signature · 2 lines
Regards, Nikita.
PATCH v3 3/3 arm: omap3: Add SPL support to cm_t35
In reply to Stefan RoeseAdd 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.
---
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(-)
board/compulab/cm_t35/cm_t35.c +17 -1
@@ -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;
include/configs/cm_t35.h +62 -2
74 lines not on this page · raw message
Signature · 1 line
1.8.3.4
PATCH v3 3/3 arm: omap3: Add SPL support to cm_t35
Hi Stefan,
Quoted 16 lines · Stefan Roese
Add SPL U-Boot support to replace x-loader on the Compulab
cm_t35board. Currently only the 256MiB SDRAM board versions are supported.Tested by booting via MMC and NAND.
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Cc Igor Grinberg <[email protected]>---
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_T35needs RAS-width of 14 instead of
13. So use the defineMICRON_V_MCFG_200which implements this 14 as RAS width.
Quoted 1 line
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,
Signature · 1 line
Albert.
PATCH v3 3/3 arm: omap3: Add SPL support to cm_t35
Hi Albert,
Quoted 16 lines · Albert ARIBAUD
Quoted 16 lines
Add SPL U-Boot support to replace x-loader on the Compulab
cm_t35board. Currently only the 256MiB SDRAM board versions are supported.Tested by booting via MMC and NAND.
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Cc Igor Grinberg <[email protected]>---
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_T35needs RAS-width of 14 instead of
13. So use the defineMICRON_V_MCFG_200which implements this 14 as RAS width.
Quoted 4 lines
Quoted 1 line
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
PATCH v4 1/1 arm: omap3: Add SPL support to cm_t35
In reply to Stefan RoeseAdd 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.
---
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(-)
board/compulab/cm_t35/cm_t35.c +17 -1
@@ -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;
include/configs/cm_t35.h +62 -2
74 lines not on this page · raw message
Signature · 1 line
1.8.4.3
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v4 1/1 arm: omap3: Add SPL support to cm_t35
Hi Nikita!
(Added Pekon to Cc)
Quoted 51 lines · Nikita Kiryanov
Hi Stefan,
Quoted 37 lines · Stefan Roese
Hi Nikita!
Quoted 8 lines · Nikita Kiryanov
I compiled SPL with your patch and it does boot U-Boot from an SD card. U-Boot functionality seems to be intact too.
However, I was unable to install it on the NAND flash. When booting from NAND, SPL does start (the SPL header is displayed), but it fails to load U-Boot with the following errors:Error: Bad compare! failed
Quoted 7 lines
I used similar commands as the ones I use to install X-Loader:
CM-T3x # mmc rescan && fatload mmc 080a00000mlo && nandecc hw && nand erase.chip && nand scrub.chip -y && nand write80a000000 30000 CM-T3x # fatload mmc 080a00000u-boot.img && nandecc sw && nand erase 80000 60000 && nand write80a0000080000 60000Perhaps some of the recent changes regarding NAND ECC messed things up?
Might be. I have not tested the latest U-Boot version on this board yet. The last version I did test was:
63c4f17b2f8017d22241522a48c765073b8791b0Author: Nikita Kiryanov <[email protected]> 2013-10-16 16:23:29
Committer: Anatolij Gustschin <[email protected]> 2013-11-12 10:12:07
Parent:f109a6e73e6bf177eabedfa5fb622ed4809e145c(omap3_dss: defineDSS_ONOFF)
Child:3f0b8b4da58c702c92652981a1e563c129108316(arm: omap3: Add SPL support tocm_t35)
Branches: cm-t35-2013-11-15, cm-t35-2013-11-26, dxr2-v1, master, remotes/origin/master
Follows: v2013.10
Precedes: v2014.01-rc1
cm_t35: use scf0403 driverUse scf0403 driver to add scf0403x LCD support for cm-t35 and cm-t3730 boards.
Quoted 6 lines
Signed-off-by Nikita Kiryanov <[email protected]>Acked-by Igor Grinberg <[email protected]>With this patch attached. Perhaps you might test this version as well. To see if this works for you too.
I tried the above, but SPL still can't boot U-Boot from NAND. I get an SPL header followed by a stream of "Error: Bad compare! failed".
Quoted 1 line
Quoted 5 lines
Quoted 2 lines
I'll try to test with the latest U-Boot version (hopefully) later this week.
When based on v2014.01-rc1, SPL itself doesn't boot from NAND (no SPL header or any visible activity when booting from NAND).
Yes. I have now tested this as well. I have been digging into this a bit this morning. And it seems that this patch is causing the regression:
d016dc42 [mtd: nand: omap: enable BCH ECC scheme using ELM for generic platform]
I can only suspect that with this patch applied, U-Boot writes the SPL (MLO) to NAND in an incompatible way for the BootROM. I have switched to 1bit HW ECC of course.
Very strangely, the Technexion TAO3530 board works fine with SPL. Even with current mainline. The only difference I can see right now is, that TAO3530 has an 16bit NAND chip and the CM_T35 has an 8bit NAND chip.
Unfortunately I have to stop debugging this issue now. Perhaps Pekon has an idea on whats going on here. Pekon, do you have an OMAP3530 board with an 8bit NAND chip? Or do you have any other idea, what might cause this problem?
Thanks,
Stefan
PATCH v4 1/1 arm: omap3: Add SPL support to cm_t35
Quoted 24 lines · Stefan Roese
Hi Nikita!
(Added Pekon to Cc)
Quoted 19 lines · Nikita Kiryanov
Hi Stefan,
Quoted 16 lines · Stefan Roese
Hi Nikita!
Quoted 13 lines · Nikita Kiryanov
I compiled SPL with your patch and it does boot U-Boot from an SD card. U-Boot functionality seems to be intact too.
However, I was unable to install it on the NAND flash. When booting from NAND, SPL does start (the SPL header is displayed), but it fails to load U-Boot with the following errors:Error: Bad compare! failed
I used similar commands as the ones I use to install X-Loader:
CM-T3x # mmc rescan && fatload mmc 080a00000mlo && nandecc hw && nand erase.chip && nand scrub.chip -y && nand write80a000000 30000 CM-T3x # fatload mmc 080a00000u-boot.img && nandecc sw && nand erase 80000 60000 && nand write80a0000080000 60000
OK, but why the nandecc switching around? On omap3_beagle _now_ (and for a year or two, but a change from long ago) we do nandecc hw for MLO and U-Boot and get the same 1bit ECC scheme the kernel uses too. I bet the problem is that SPL is loading and expecting the HW scheme, but you wrote in the SW scheme...
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20131204/fb205ebf/attachment.pgp>
PATCH v4 1/1 arm: omap3: Add SPL support to cm_t35
Quoted 15 lines · Tom Rini
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines · Nikita Kiryanov
Quoted 6 lines
I compiled SPL with your patch and it does boot U-Boot from an SD card. U-Boot functionality seems to be intact too.
However, I was unable to install it on the NAND flash. When booting from NAND, SPL does start (the SPL header is displayed), but it fails to load U-Boot with the following errors:Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Error: Bad compare! failed
I used similar commands as the ones I use to install X-Loader:
CM-T3x # mmc rescan && fatload mmc 080a00000mlo && nandecc hw && nand erase.chip && nand scrub.chip -y && nand write80a000000 30000 CM-T3x # fatload mmc 080a00000u-boot.img && nandecc sw && nand erase 80000 60000 && nand write80a0000080000 60000
Quoted 5 lines
OK, but why the nandecc switching around? On
omap3_beagle_now_ (and for a year or two, but a change from long ago) we do nandecc hw for MLO and U-Boot and get the same 1bit ECC scheme the kernel uses too. I bet the problem is that SPL is loading and expecting the HW scheme, but you wrote in the SW scheme...
No (please read my mail to the end). The problem is that the BootROM doesn't boot the SPL which is written with the latest U-Boot and with 1bit hw ecc. And this works again when the patch I mention is reverted.
Thanks,
Stefan
Gupta
PATCH v4 1/1 arm: omap3: Add SPL support to cm_t35
In reply to Stefan RoeseHi Stefan,
Quoted 2 lines
From: Stefan Roese [mailto:sr at denx.de]
Quoted 1 line · Nikita Kiryanov
[...]
Quoted 9 lines
Quoted 2 lines
When based on v2014.01-rc1, SPL itself doesn't boot from NAND (no SPL header or any visible activity when booting from NAND).
Yes. I have now tested this as well. I have been digging into this a bit this morning. And it seems that this patch is causing the regression:
d016dc42[mtd: nand: omap: enable BCH ECC scheme using ELM for generic platform]
Quoted 8 lines
I can only suspect that with this patch applied, U-Boot writes the SPL (MLO) to NAND in an incompatible way for the BootROM. I have switched to 1bit HW ECC of course.
Very strangely, the Technexion TAO3530 board works fine with SPL. Even with current mainline. The only difference I can see right now is, that TAO3530 has an 16bit NAND chip and the
CM_T35has an 8bit NAND chip.
For HAM1_HW this selects ecc-scheme as below..
@@omap_select_ecc_scheme()
case OMAP_ECC_HAM1_CODE_HW:
+ /* define ecc-layout */ + ecclayout->eccbytes = nand->ecc.bytes * eccsteps; + for (i = 0; i < ecclayout->eccbytes; i++)
+ ecclayout->eccpos[i] = i + BADBLOCK_MARKER_LENGTH;
+ ecclayout->oobfree[0].offset = i + BADBLOCK_MARKER_LENGTH;
+ ecclayout->oobfree[0].length = oobsize - ecclayout->eccbytes -
+ BADBLOCK_MARKER_LENGTH;
Aah!!... May be I understand the issue..
Reference: As per OMAP3530 TRM given below
http://www.ti.com/product/omap3530 http://www.ti.com/litv/pdf/spruf98x Chapter-25: Initialization Sub-topic: Memory Booting Section: 25.4.7.4 NAND Figure 25-19. ECC Locations in NAND Spare Areas
For large-page NAND
(a) x8 Device: ECC signature starts from byte[1] in OOB (offset of 1 *byte*)
(b) x16 Device: ECC signature starts from byte[2] in OOB (offset of 1 *word*)
And in omap_gpmc.c .. BADBLOCK_MARKER_LENGTH = 2. So ECC signature starts from offset
ecclayout->eccpos[0] = 0 + BADBLOCK_MARKER_LENGTH = 0x2;
which is actually for (b) that is x16 device.
Thus, Technexion TAO3530 board with x16 NAND device is booting correctly, Whereas CM_T35 with x8 NAND device fails..
Quoted 5 lines
Unfortunately I have to stop debugging this issue now. Perhaps Pekon has an idea on whats going on here. Pekon, do you have an OMAP3530 board with an 8bit NAND chip? Or do you have any other idea, what might cause this problem?
It's a bug in driver. I should have taken care of device-width while defining the layout. I think this is the issue with HAM1 scheme only, because ROM code has same ecc-layout for other schemes (like BCH8) whether it's a x8 or x16 devices.
Please patch following to see if CM_T35 boots fine, with latest u-boot.
drivers/mtd/nand/omap_gpmc.c +4 -1
@@ -626,7 +626,10 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,/* define ecc-layout */ ecclayout->eccbytes = nand->ecc.bytes * eccsteps;for (i =0 ; i < ecclayout->eccbytes; i++) - ecclayout->eccpos[i] = i +BADBLOCK_MARKER_LENGTH ; +if (nand->options &NAND_BUSWIDTH_16 ) + ecclayout->eccpos[i] = i +2 ; +else + ecclayout->eccpos[i] = i +1 ; ecclayout->oobfree[0 ].offset = i +BADBLOCK_MARKER_LENGTH ; ecclayout->oobfree[0 ].length = oobsize - ecclayout->eccbytes -BADBLOCK_MARKER_LENGTH ;
Thanks for pointing out this..
Sorry I don't have many OMAP3 boards to boot-test HAM1, so this error crept in. Once you confirm its working, I'll submit a formal patch.
with regards, pekon
PATCH v4 1/1 arm: omap3: Add SPL support to cm_t35
Pekon!
Quoted 27 lines · Pekon
Quoted 4 lines
Very strangely, the Technexion TAO3530 board works fine with SPL. Even with current mainline. The only difference I can see right now is, that TAO3530 has an 16bit NAND chip and the
CM_T35has an 8bit NAND chip.For
HAM1_HWthis selects ecc-scheme as below..
@@omap_select_ecc_scheme()
caseOMAP_ECC_HAM1_CODE_HW:(quoted diff) +7
-0+ /* define ecc-layout */ + ecclayout->eccbytes = nand->ecc.bytes * eccsteps; + for (i = 0; i < ecclayout->eccbytes; i++) + ecclayout->eccpos[i] = i + BADBLOCK_MARKER_LENGTH; + ecclayout->oobfree[0].offset = i + BADBLOCK_MARKER_LENGTH; + ecclayout->oobfree[0].length = oobsize - ecclayout->eccbytes - + BADBLOCK_MARKER_LENGTH;Aah!!... May be I understand the issue..
Reference: As per OMAP3530 TRM given belowhttp://www.ti.com/product/omap3530 http://www.ti.com/litv/pdf/spruf98x Chapter-25: Initialization Sub-topic: Memory Booting Section: 25.4.7.4 NAND Figure 25-19. ECC Locations in NAND Spare AreasFor large-page NAND
(a) x8 Device: ECC signature starts from byte[1] in OOB (offset of 1 *byte*)
(b) x16 Device: ECC signature starts from byte[2] in OOB (offset of 1 *word*)
Quoted 7 lines
And in
omap_gpmc.c..BADBLOCK_MARKER_LENGTH= 2. So ECC signature starts from offsetecclayout->eccpos[0] = 0 + BADBLOCK_MARKER_LENGTH = 0x2;which is actually for (b) that is x16 device.
Thus, Technexion TAO3530 board with x16 NAND device is booting correctly, Whereas
CM_T35with x8 NAND device fails..
Sounds like a very good explanation. Thanks!
Quoted 28 lines
Quoted 5 lines
Unfortunately I have to stop debugging this issue now. Perhaps Pekon has an idea on whats going on here. Pekon, do you have an OMAP3530 board with an 8bit NAND chip? Or do you have any other idea, what might cause this problem?
It's a bug in driver. I should have taken care of device-width while defining the layout. I think this is the issue with HAM1 scheme only, because ROM code has same ecc-layout for other schemes (like BCH8) whether it's a x8 or x16 devices.
Please patch following to see ifCM_T35boots fine, with latest u-boot.drivers/mtd/nand/omap_gpmc.c +4
-1@@ -626,7 +626,10 @@ static int omap_select_ecc_scheme(struct nand_chip *nand,/* define ecc-layout */ ecclayout->eccbytes = nand->ecc.bytes * eccsteps;for (i =0 ; i < ecclayout->eccbytes; i++) - ecclayout->eccpos[i] = i +BADBLOCK_MARKER_LENGTH ; +if (nand->options &NAND_BUSWIDTH_16 ) + ecclayout->eccpos[i] = i +2 ; +else + ecclayout->eccpos[i] = i +1 ; ecclayout->oobfree[0 ].offset = i +BADBLOCK_MARKER_LENGTH ; ecclayout->oobfree[0 ].length = oobsize - ecclayout->eccbytes -BADBLOCK_MARKER_LENGTH ;
Quoted 3 lines
Thanks for pointing out this..
Sorry I don't have many OMAP3 boards to boot-test HAM1, so this error crept in. Once you confirm its working, I'll submit a formal patch.
Yes. This fixes this issue on the cm-t35 board. So please submit a proper patch. :)
Thanks again for this quick fix!
Stefan
PATCH 1/3 arm: spl: Fix SPL booting for OMAP3
In reply to Stefan RoeseHi Stefan,
Quoted 21 lines · Stefan Roese
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 <[email protected]>Cc Tom Rini <[email protected]>Cc Albert ARIBAUD <[email protected]>--- arch/arm/lib/crt0.S | 6 ++++++ 1 file changed, 6 insertions(+)arch/arm/lib/crt0.S +0
-0@@ -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 */
arch/arm/lib/crt0.S ENTRY(_main) · line 97
Quoted 10 lines
arch/arm/lib/crt0.S +6
-0+#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,
Signature · 1 line
Albert.
PATCH 1/3 arm: spl: Fix SPL booting for OMAP3
Hi Albert,
Quoted 22 lines · Albert ARIBAUD
Quoted 8 lines
arch/arm/lib/crt0.S +0
-0@@ -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 */arch/arm/lib/crt0.S ENTRY(_main) · line 97
Quoted 10 lines
arch/arm/lib/crt0.S +6
-0+#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_fNAK 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.
Quoted 8 lines
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
PATCH 1/3 arm: spl: Fix SPL booting for OMAP3
Hi Stefan,
Quoted 45 lines · Stefan Roese
Hi Albert,
Quoted 22 lines · Albert ARIBAUD
Quoted 8 lines
arch/arm/lib/crt0.S +0
-0@@ -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 */arch/arm/lib/crt0.S ENTRY(_main) · line 97
Quoted 10 lines
arch/arm/lib/crt0.S +6
-0+#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_fNAK 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.Quoted 8 lines
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
_mainis 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();
Quoted 4 lines
#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 inlowlevel_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< ---
Quoted 3 lines
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.
Quoted 3 lines
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?
Quoted 2 lines
Thanks,
Stefan
Amicalement,
Signature · 1 line
Albert.
PATCH 1/3 arm: spl: Fix SPL booting for OMAP3
Hi Albert,
Quoted 43 lines · Albert ARIBAUD
Quoted 16 lines
Quoted 4 lines
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
_mainis 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();Quoted 4 lines
#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_initis called at the very end oflowlevel_init, which was branched straight into fromcpu_init_critwhich itself is called just before_main.In other words, after
s_init(),_mainimmediately kicks in, sets up sp and r8 (which was done also inlowlevel_initand will thus be a no-op once gdata is handled incrt0.Stoo) and callsboard_init_f().So, calling
s_init()last inlowlevel_init()would be the same as calling it first inboard_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 enteringboard_init_f(). This is the only reason why sp and r8 are set up inlowlevel_init: becauses_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.Sdoes *exactly* the same thing aslowlevel_init-- or will, once the gdata stuff is added incrt0.Sas I suggest.
Quoted 6 lines
--- 8< ---
So you just need to add the gdata stuff incrt0.Sas I previously suggested, move thes_init()call incrt0.S(conditioned on building SPL) just before the call toboard_init_f(), and then makelowlevel_initempty since it would then be useless.
--- 8< ---
Useless? Its still calling cpy_clk_code(). Isn't this needed anymore?
Quoted 7 lines
Quoted 3 lines
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.
Quoted 9 lines
Quoted 3 lines
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
PATCH 1/3 arm: spl: Fix SPL booting for OMAP3
Hi Stefan,
Quoted 46 lines · Stefan Roese
Hi Albert,
Quoted 43 lines · Albert ARIBAUD
Quoted 16 lines
Quoted 4 lines
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
_mainis 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();Quoted 4 lines
#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_initis called at the very end oflowlevel_init, which was branched straight into fromcpu_init_critwhich itself is called just before_main.In other words, after
s_init(),_mainimmediately kicks in, sets up sp and r8 (which was done also inlowlevel_initand will thus be a no-op once gdata is handled incrt0.Stoo) and callsboard_init_f().So, calling
s_init()last inlowlevel_init()would be the same as calling it first inboard_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 enteringboard_init_f(). This is the only reason why sp and r8 are set up inlowlevel_init: becauses_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.Sdoes *exactly* the same thing aslowlevel_init-- or will, once the gdata stuff is added incrt0.Sas I suggest.
Quoted 8 lines
Quoted 6 lines
--- 8< ---
So you just need to add the gdata stuff incrt0.Sas I previously suggested, move thes_init()call incrt0.S(conditioned on building SPL) just before the call toboard_init_f(), and then makelowlevel_initempty 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.
Quoted 14 lines
Quoted 7 lines
Quoted 3 lines
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.
Quoted 4 lines
Quoted 3 lines
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...
Quoted 8 lines
Quoted 5 lines
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.
Quoted 2 lines
Thanks,
Stefan
Amicalement,
Signature · 1 line
Albert.
PATCH v2 1/3 arm: spl: Fix SPL booting for OMAP3
In reply to Stefan RoeseFix 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).
---
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(-)
arch/arm/cpu/armv7/omap3/board.c +0 -2
@@ -256,8 +256,6 @@ void s_init(void)#endif #ifdef CONFIG_SPL_BUILD - gd = &gdata; -preloader_console_init ();timer_init ();
arch/arm/cpu/armv7/omap3/lowlevel_init.S +1 -2
@@ -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)
arch/arm/lib/crt0.S +6 -1
@@ -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)
Signature · 1 line
1.8.2.3
PATCH v2 1/3 arm: spl: Fix SPL booting for OMAP3
Hi Stefan,
Quoted 63 lines · Stefan Roese
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 callingboard_init_f(). And makes sure that r8 is correctly initialized befores_init()is called. r8 now is only written incrt0.S.This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Cc Albert ARIBAUD <[email protected]>---
v2:
- Change handling/initializing of r8 as suggested by Albert. It should only be written incrt0.S.Tom, while working on this version one question came up:
Islowlevel_init()(filearch/arm/cpu/armv7/omap3/lowlevel_init.S) needed any more? It callscpy_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(-)arch/arm/cpu/armv7/omap3/board.c +0
-2@@ -256,8 +256,6 @@ void s_init(void)#endif #ifdef CONFIG_SPL_BUILD - gd = &gdata; -preloader_console_init ();timer_init ();arch/arm/cpu/armv7/omap3/lowlevel_init.S +1
-2@@ -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)arch/arm/lib/crt0.S +0
-0
arch/arm/lib/crt0.S ENTRY(_main) · line 85
Quoted 1 line
arch/arm/lib/crt0.S +0
-0@@ -85,8 +85,13 @@ ENTRY(_main) bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
Actually, this...
arch/arm/lib/crt0.S ENTRY(_main) · line 87
Quoted 2 lines
arch/arm/lib/crt0.S +0
-0sub 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.
arch/arm/lib/crt0.S ENTRY(_main) · line 88
Quoted 1 line
arch/arm/lib/crt0.S +0
-1- mov r8, sp /* GD is above SP */
Also, this...
arch/arm/lib/crt0.S ENTRY(_main) · line 88
Quoted 1 line
arch/arm/lib/crt0.S +0
-0mov 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.
arch/arm/lib/crt0.S ENTRY(_main) · line 97
Quoted 9 lines
arch/arm/lib/crt0.S +6
-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)
Amicalement,
Signature · 1 line
Albert.
PATCH v3 1/3 arm: spl: Fix SPL booting for OMAP3
In reply to Stefan RoeseSPL 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).
---
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(-)
arch/arm/cpu/armv7/omap3/board.c +0 -2
@@ -256,8 +256,6 @@ void s_init(void)#endif #ifdef CONFIG_SPL_BUILD - gd = &gdata; -preloader_console_init ();timer_init ();
arch/arm/cpu/armv7/omap3/lowlevel_init.S +1 -2
@@ -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)
arch/arm/lib/crt0.S +5 -0
@@ -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
Signature · 1 line
1.8.2.3
PATCH v3 1/3 arm: spl: Fix SPL booting for OMAP3
Hi Stefan,
Quoted 2 lines · Stefan Roese
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:
Amicalement,
Signature · 1 line
Albert.
PATCH v3 1/3 arm: spl: Fix SPL booting for OMAP3
Quoted 6 lines · Albert ARIBAUD
Quoted 2 lines
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_initshould 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
PATCH v4 1/3 arm: spl: Fix SPL booting for OMAP3
In reply to Stefan RoeseFix 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).
---
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(-)
arch/arm/cpu/armv7/omap3/board.c +0 -2
@@ -256,8 +256,6 @@ void s_init(void)#endif #ifdef CONFIG_SPL_BUILD - gd = &gdata; -preloader_console_init ();timer_init ();
arch/arm/cpu/armv7/omap3/lowlevel_init.S +1 -2
@@ -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)
arch/arm/lib/crt0.S +5 -0
@@ -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
Signature · 1 line
1.8.2.3
PATCH v4 1/3 arm: spl: Fix SPL booting for OMAP3
Hi Stefan,
Quoted 73 lines · Stefan Roese
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 callingboard_init_f(). And makes sure that r8 is correctly initialized befores_init()is called. r8 now is only written incrt0.S.This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Acked-by Albert ARIBAUD <[email protected]>---
v4:
- Corrected commit text to reflect changed patchv3:
- Some code shuffling incrt0.Sas requested by Albertv2:
- Change handling/initializing of r8 as suggested by Albert. It should only be written incrt0.S.Tom, while working on this version one question came up:
Islowlevel_init()(filearch/arm/cpu/armv7/omap3/lowlevel_init.S) needed any more? It callscpy_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(-)arch/arm/cpu/armv7/omap3/board.c +0
-2@@ -256,8 +256,6 @@ void s_init(void)#endif #ifdef CONFIG_SPL_BUILD - gd = &gdata; -preloader_console_init ();timer_init ();arch/arm/cpu/armv7/omap3/lowlevel_init.S +1
-2@@ -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)arch/arm/lib/crt0.S +1
-0@@ -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)
arch/arm/lib/crt0.S ENTRY(_main) · line 96
Quoted 10 lines
arch/arm/lib/crt0.S +4
-0+ 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
Amicalement,
Signature · 1 line
Albert.
PATCH v5 1/3 arm: spl: Fix SPL booting for OMAP3
In reply to Stefan RoeseFix 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).
---
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(-)
arch/arm/cpu/armv7/omap3/board.c +0 -2
@@ -256,8 +256,6 @@ void s_init(void)#endif #ifdef CONFIG_SPL_BUILD - gd = &gdata; -preloader_console_init ();timer_init ();
arch/arm/cpu/armv7/omap3/lowlevel_init.S +1 -2
@@ -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)
arch/arm/lib/crt0.S +8 -0
@@ -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
Signature · 1 line
1.8.3.1
PATCH v5 1/3 arm: spl: Fix SPL booting for OMAP3
Hi Stefan,
Quoted 12 lines · Stefan Roese
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 callingboard_init_f(). And makes sure that r8 is correctly initialized befores_init()is called. r8 now is only written incrt0.S.This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Cc Albert ARIBAUD <[email protected]>
Quoted 8 lines
---
Albert, I'm not really happy with this patch as it evolves now. As you will see, I had to make some further additions tocrt0.Sto 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,
Signature · 1 line
Albert.
PATCH v5 1/3 arm: spl: Fix SPL booting for OMAP3
Quoted 39 lines · Albert ARIBAUD
Hi Stefan,
Quoted 12 lines · Stefan Roese
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 callingboard_init_f(). And makes sure that r8 is correctly initialized befores_init()is called. r8 now is only written incrt0.S.This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Cc Albert ARIBAUD <[email protected]>Quoted 8 lines
---
Albert, I'm not really happy with this patch as it evolves now. As you will see, I had to make some further additions tocrt0.Sto 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()incrt0.Sright between the initial environment setting and the call toboard_init_f(), we could simply move thes_init()call insideboard_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 duringboard_init_f()to call functions.Moving the call to
s_init()intoboard_init_f()removes any changes tocrt0.S, which were my essential NAK reason and saves you some ugliness.
Quoted 8 lines
I would even hazard that you could place
s_initin init_sequence[], for instance as a first entry to be called (beforearch_cpu_init). After all, the only difference in execution is that gdata is going to be initialized properly befores_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 fromboard_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!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130703/5d1f8bfa/attachment.pgp>
PATCH v5 1/3 arm: spl: Fix SPL booting for OMAP3
Hi Tom,
Quoted 49 lines · Tom Rini
Quoted 39 lines · Albert ARIBAUD
Hi Stefan,
Quoted 12 lines · Stefan Roese
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 callingboard_init_f(). And makes sure that r8 is correctly initialized befores_init()is called. r8 now is only written incrt0.S.This error was detected on the SPL port for the Compulab CM-T35 board (OMAP3530).
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Cc Albert ARIBAUD <[email protected]>Quoted 8 lines
---
Albert, I'm not really happy with this patch as it evolves now. As you will see, I had to make some further additions tocrt0.Sto 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()incrt0.Sright between the initial environment setting and the call toboard_init_f(), we could simply move thes_init()call insideboard_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 duringboard_init_f()to call functions.Moving the call to
s_init()intoboard_init_f()removes any changes tocrt0.S, which were my essential NAK reason and saves you some ugliness.Quoted 8 lines
I would even hazard that you could place
s_initin init_sequence[], for instance as a first entry to be called (beforearch_cpu_init). After all, the only difference in execution is that gdata is going to be initialized properly befores_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 fromboard_init_f()it needs a more meaningful name.
Quoted 6 lines
So, this is one of the things that needs resolving for v2013.07. What do you want to call the renamed
s_initfunction? I think we need to go theinit_sequenceroute, and keepcommon/board_f.cin 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,
Signature · 1 line
Albert.
PATCH 1/3 arm: spl: Fix SPL booting for OMAP3
In reply to Stefan RoeseQuoted 9 lines · Stefan Roese
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 <[email protected]>Cc Tom Rini <[email protected]>Cc Albert ARIBAUD <[email protected]>
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!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130715/96924771/attachment.pgp>
PATCH 1/3 arm: spl: Fix SPL booting for OMAP3
Hi Tom,
Quoted 6 lines · Tom Rini
Quoted 6 lines
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).
Quoted 6 lines
Quoted 3 lines
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Cc Albert ARIBAUD <[email protected]>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.
Quoted 2 lines
I just gave my
beagleboard (classic and xM) a spin and they're OK.
Did you also power-cycle the board (cold boot)?
Quoted 5 lines
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 (changes_inittosystem_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
PATCH 1/3 arm: spl: Fix SPL booting for OMAP3
Quoted 16 lines · Stefan Roese
Hi Tom,
Quoted 6 lines · Tom Rini
Quoted 6 lines
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).
Quoted 6 lines
Quoted 3 lines
Signed-off-by Stefan Roese <[email protected]>Cc Tom Rini <[email protected]>Cc Albert ARIBAUD <[email protected]>While we have a problem here, it's not a problem that's visible on current SPL using platforms, just the CM-T35, yes?
Quoted 7 lines
Thats what I noticed as well. I was a bit astonished that I didn't see it on beagle. It should hit there too.
Quoted 2 lines
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.
Quoted 9 lines
Quoted 5 lines
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 (changes_inittosystem_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.
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130716/0059178f/attachment.pgp>