* [U-Boot] [PATCH 2/3] arm: omap3: spl: Fix problem with 8bit NAND devices
2013-06-14 8:54 [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Stefan Roese
@ 2013-06-14 8:55 ` Stefan Roese
2013-07-30 13:25 ` [U-Boot] [U-Boot, " Tom Rini
2013-06-14 8:55 ` [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35 Stefan Roese
` (6 subsequent siblings)
7 siblings, 1 reply; 38+ messages in thread
From: Stefan Roese @ 2013-06-14 8:55 UTC (permalink / raw)
To: u-boot
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)
--
1.8.2.3
^ permalink raw reply related [flat|nested] 38+ messages in thread* [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35
2013-06-14 8:54 [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Stefan Roese
2013-06-14 8:55 ` [U-Boot] [PATCH 2/3] arm: omap3: spl: Fix problem with 8bit NAND devices Stefan Roese
@ 2013-06-14 8:55 ` Stefan Roese
2013-06-17 11:53 ` Igor Grinberg
` (3 more replies)
2013-06-20 16:42 ` [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Albert ARIBAUD
` (5 subsequent siblings)
7 siblings, 4 replies; 38+ messages in thread
From: Stefan Roese @ 2013-06-14 8:55 UTC (permalink / raw)
To: u-boot
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 */
--
1.8.2.3
^ permalink raw reply related [flat|nested] 38+ messages in thread* [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35
2013-06-14 8:55 ` [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35 Stefan Roese
@ 2013-06-17 11:53 ` Igor Grinberg
2013-06-17 12:38 ` Tom Rini
2013-06-17 14:03 ` [U-Boot] [PATCH v2 " Stefan Roese
` (2 subsequent siblings)
3 siblings, 1 reply; 38+ messages in thread
From: Igor Grinberg @ 2013-06-17 11:53 UTC (permalink / raw)
To: u-boot
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 */
>
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 38+ messages in thread* [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35
2013-06-17 11:53 ` Igor Grinberg
@ 2013-06-17 12:38 ` Tom Rini
2013-06-17 13:38 ` Igor Grinberg
0 siblings, 1 reply; 38+ messages in thread
From: Tom Rini @ 2013-06-17 12:38 UTC (permalink / raw)
To: u-boot
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.
--
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>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35
2013-06-17 12:38 ` Tom Rini
@ 2013-06-17 13:38 ` Igor Grinberg
2013-06-17 13:52 ` Stefan Roese
0 siblings, 1 reply; 38+ messages in thread
From: Igor Grinberg @ 2013-06-17 13:38 UTC (permalink / raw)
To: u-boot
-----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.
-----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-----
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35
2013-06-17 13:38 ` Igor Grinberg
@ 2013-06-17 13:52 ` Stefan Roese
0 siblings, 0 replies; 38+ messages in thread
From: Stefan Roese @ 2013-06-17 13:52 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v2 3/3] arm: omap3: Add SPL support to cm_t35
2013-06-14 8:55 ` [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35 Stefan Roese
2013-06-17 11:53 ` Igor Grinberg
@ 2013-06-17 14:03 ` Stefan Roese
2013-06-18 6:14 ` Nikita Kiryanov
2013-07-30 10:52 ` [U-Boot] [PATCH v3 " Stefan Roese
2013-11-15 7:51 ` [U-Boot] [PATCH v4] " Stefan Roese
3 siblings, 1 reply; 38+ messages in thread
From: Stefan Roese @ 2013-06-17 14:03 UTC (permalink / raw)
To: u-boot
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 */
--
1.8.2.3
^ permalink raw reply related [flat|nested] 38+ messages in thread* [U-Boot] [PATCH v2 3/3] arm: omap3: Add SPL support to cm_t35
2013-06-17 14:03 ` [U-Boot] [PATCH v2 " Stefan Roese
@ 2013-06-18 6:14 ` Nikita Kiryanov
0 siblings, 0 replies; 38+ messages in thread
From: Nikita Kiryanov @ 2013-06-18 6:14 UTC (permalink / raw)
To: u-boot
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 */
>
--
Regards,
Nikita.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v3 3/3] arm: omap3: Add SPL support to cm_t35
2013-06-14 8:55 ` [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35 Stefan Roese
2013-06-17 11:53 ` Igor Grinberg
2013-06-17 14:03 ` [U-Boot] [PATCH v2 " Stefan Roese
@ 2013-07-30 10:52 ` Stefan Roese
2013-07-30 12:10 ` Albert ARIBAUD
2013-11-15 7:51 ` [U-Boot] [PATCH v4] " Stefan Roese
3 siblings, 1 reply; 38+ messages in thread
From: Stefan Roese @ 2013-07-30 10:52 UTC (permalink / raw)
To: u-boot
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 */
--
1.8.3.4
^ permalink raw reply related [flat|nested] 38+ messages in thread* [U-Boot] [PATCH v3 3/3] arm: omap3: Add SPL support to cm_t35
2013-07-30 10:52 ` [U-Boot] [PATCH v3 " Stefan Roese
@ 2013-07-30 12:10 ` Albert ARIBAUD
2013-07-30 12:14 ` Stefan Roese
0 siblings, 1 reply; 38+ messages in thread
From: Albert ARIBAUD @ 2013-07-30 12:10 UTC (permalink / raw)
To: u-boot
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,
--
Albert.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v3 3/3] arm: omap3: Add SPL support to cm_t35
2013-07-30 12:10 ` Albert ARIBAUD
@ 2013-07-30 12:14 ` Stefan Roese
0 siblings, 0 replies; 38+ messages in thread
From: Stefan Roese @ 2013-07-30 12:14 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v4] arm: omap3: Add SPL support to cm_t35
2013-06-14 8:55 ` [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35 Stefan Roese
` (2 preceding siblings ...)
2013-07-30 10:52 ` [U-Boot] [PATCH v3 " Stefan Roese
@ 2013-11-15 7:51 ` Stefan Roese
[not found] ` <5288BBAC.2020307@compulab.co.il>
3 siblings, 1 reply; 38+ messages in thread
From: Stefan Roese @ 2013-11-15 7:51 UTC (permalink / raw)
To: u-boot
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 */
--
1.8.4.3
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-14 8:54 [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Stefan Roese
2013-06-14 8:55 ` [U-Boot] [PATCH 2/3] arm: omap3: spl: Fix problem with 8bit NAND devices Stefan Roese
2013-06-14 8:55 ` [U-Boot] [PATCH 3/3] arm: omap3: Add SPL support to cm_t35 Stefan Roese
@ 2013-06-20 16:42 ` Albert ARIBAUD
2013-06-20 17:01 ` Stefan Roese
2013-06-21 2:13 ` [U-Boot] [PATCH v2 " Stefan Roese
` (4 subsequent siblings)
7 siblings, 1 reply; 38+ messages in thread
From: Albert ARIBAUD @ 2013-06-20 16:42 UTC (permalink / raw)
To: u-boot
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,
--
Albert.
^ permalink raw reply [flat|nested] 38+ messages in thread* [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-20 16:42 ` [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Albert ARIBAUD
@ 2013-06-20 17:01 ` Stefan Roese
2013-06-20 17:51 ` Albert ARIBAUD
0 siblings, 1 reply; 38+ messages in thread
From: Stefan Roese @ 2013-06-20 17:01 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-20 17:01 ` Stefan Roese
@ 2013-06-20 17:51 ` Albert ARIBAUD
2013-06-20 18:28 ` Stefan Roese
0 siblings, 1 reply; 38+ messages in thread
From: Albert ARIBAUD @ 2013-06-20 17:51 UTC (permalink / raw)
To: u-boot
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,
--
Albert.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-20 17:51 ` Albert ARIBAUD
@ 2013-06-20 18:28 ` Stefan Roese
2013-06-20 19:18 ` Albert ARIBAUD
0 siblings, 1 reply; 38+ messages in thread
From: Stefan Roese @ 2013-06-20 18:28 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-20 18:28 ` Stefan Roese
@ 2013-06-20 19:18 ` Albert ARIBAUD
0 siblings, 0 replies; 38+ messages in thread
From: Albert ARIBAUD @ 2013-06-20 19:18 UTC (permalink / raw)
To: u-boot
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,
--
Albert.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v2 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-14 8:54 [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Stefan Roese
` (2 preceding siblings ...)
2013-06-20 16:42 ` [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Albert ARIBAUD
@ 2013-06-21 2:13 ` Stefan Roese
2013-06-21 8:57 ` Albert ARIBAUD
2013-06-21 9:10 ` [U-Boot] [PATCH v3 " Stefan Roese
` (3 subsequent siblings)
7 siblings, 1 reply; 38+ messages in thread
From: Stefan Roese @ 2013-06-21 2:13 UTC (permalink / raw)
To: u-boot
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)
--
1.8.2.3
^ permalink raw reply related [flat|nested] 38+ messages in thread* [U-Boot] [PATCH v2 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-21 2:13 ` [U-Boot] [PATCH v2 " Stefan Roese
@ 2013-06-21 8:57 ` Albert ARIBAUD
0 siblings, 0 replies; 38+ messages in thread
From: Albert ARIBAUD @ 2013-06-21 8:57 UTC (permalink / raw)
To: u-boot
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,
--
Albert.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v3 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-14 8:54 [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Stefan Roese
` (3 preceding siblings ...)
2013-06-21 2:13 ` [U-Boot] [PATCH v2 " Stefan Roese
@ 2013-06-21 9:10 ` Stefan Roese
2013-06-21 10:30 ` Albert ARIBAUD
2013-06-21 10:42 ` [U-Boot] [PATCH v4 " Stefan Roese
` (2 subsequent siblings)
7 siblings, 1 reply; 38+ messages in thread
From: Stefan Roese @ 2013-06-21 9:10 UTC (permalink / raw)
To: u-boot
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
--
1.8.2.3
^ permalink raw reply related [flat|nested] 38+ messages in thread* [U-Boot] [PATCH v3 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-21 9:10 ` [U-Boot] [PATCH v3 " Stefan Roese
@ 2013-06-21 10:30 ` Albert ARIBAUD
2013-06-21 10:39 ` Stefan Roese
0 siblings, 1 reply; 38+ messages in thread
From: Albert ARIBAUD @ 2013-06-21 10:30 UTC (permalink / raw)
To: u-boot
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,
--
Albert.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v3 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-21 10:30 ` Albert ARIBAUD
@ 2013-06-21 10:39 ` Stefan Roese
0 siblings, 0 replies; 38+ messages in thread
From: Stefan Roese @ 2013-06-21 10:39 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v4 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-14 8:54 [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Stefan Roese
` (4 preceding siblings ...)
2013-06-21 9:10 ` [U-Boot] [PATCH v3 " Stefan Roese
@ 2013-06-21 10:42 ` Stefan Roese
2013-06-21 11:02 ` Albert ARIBAUD
2013-06-25 7:14 ` [U-Boot] [PATCH v5 " Stefan Roese
2013-07-15 14:33 ` [U-Boot] [PATCH " Tom Rini
7 siblings, 1 reply; 38+ messages in thread
From: Stefan Roese @ 2013-06-21 10:42 UTC (permalink / raw)
To: u-boot
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
--
1.8.2.3
^ permalink raw reply related [flat|nested] 38+ messages in thread* [U-Boot] [PATCH v4 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-21 10:42 ` [U-Boot] [PATCH v4 " Stefan Roese
@ 2013-06-21 11:02 ` Albert ARIBAUD
0 siblings, 0 replies; 38+ messages in thread
From: Albert ARIBAUD @ 2013-06-21 11:02 UTC (permalink / raw)
To: u-boot
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,
--
Albert.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v5 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-14 8:54 [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Stefan Roese
` (5 preceding siblings ...)
2013-06-21 10:42 ` [U-Boot] [PATCH v4 " Stefan Roese
@ 2013-06-25 7:14 ` Stefan Roese
2013-06-27 8:27 ` Albert ARIBAUD
2013-07-15 14:33 ` [U-Boot] [PATCH " Tom Rini
7 siblings, 1 reply; 38+ messages in thread
From: Stefan Roese @ 2013-06-25 7:14 UTC (permalink / raw)
To: u-boot
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
--
1.8.3.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [U-Boot] [PATCH v5 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-25 7:14 ` [U-Boot] [PATCH v5 " Stefan Roese
@ 2013-06-27 8:27 ` Albert ARIBAUD
2013-07-03 19:47 ` Tom Rini
0 siblings, 1 reply; 38+ messages in thread
From: Albert ARIBAUD @ 2013-06-27 8:27 UTC (permalink / raw)
To: u-boot
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,
--
Albert.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v5 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-27 8:27 ` Albert ARIBAUD
@ 2013-07-03 19:47 ` Tom Rini
2013-07-04 11:58 ` Albert ARIBAUD
0 siblings, 1 reply; 38+ messages in thread
From: Tom Rini @ 2013-07-03 19:47 UTC (permalink / raw)
To: u-boot
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!
--
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>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v5 1/3] arm: spl: Fix SPL booting for OMAP3
2013-07-03 19:47 ` Tom Rini
@ 2013-07-04 11:58 ` Albert ARIBAUD
0 siblings, 0 replies; 38+ messages in thread
From: Albert ARIBAUD @ 2013-07-04 11:58 UTC (permalink / raw)
To: u-boot
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,
--
Albert.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3
2013-06-14 8:54 [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3 Stefan Roese
` (6 preceding siblings ...)
2013-06-25 7:14 ` [U-Boot] [PATCH v5 " Stefan Roese
@ 2013-07-15 14:33 ` Tom Rini
2013-07-16 6:24 ` Stefan Roese
7 siblings, 1 reply; 38+ messages in thread
From: Tom Rini @ 2013-07-15 14:33 UTC (permalink / raw)
To: u-boot
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!
--
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>
^ permalink raw reply [flat|nested] 38+ messages in thread* [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3
2013-07-15 14:33 ` [U-Boot] [PATCH " Tom Rini
@ 2013-07-16 6:24 ` Stefan Roese
2013-07-16 14:36 ` Tom Rini
0 siblings, 1 reply; 38+ messages in thread
From: Stefan Roese @ 2013-07-16 6:24 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 1/3] arm: spl: Fix SPL booting for OMAP3
2013-07-16 6:24 ` Stefan Roese
@ 2013-07-16 14:36 ` Tom Rini
0 siblings, 0 replies; 38+ messages in thread
From: Tom Rini @ 2013-07-16 14:36 UTC (permalink / raw)
To: u-boot
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.
--
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>
^ permalink raw reply [flat|nested] 38+ messages in thread