Message ID | 1366038520-7492-4-git-send-email-r.sricharan@ti.com |
---|---|
State | Changes Requested |
Delegated to: | Tom Rini |
Headers | show |
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 04/15/2013 11:08 AM, Sricharan R wrote: > Currently save_boot_params saves the boot parameters passed from > romcode. But this is not stored in a writable location > consistently. So the current code would not work for a 'XIP' boot. > Change this by saving the boot parameters in 'gd' which is always > writable. Also add a 'C' function instead of an assembly code that > is more readable. > > Signed-off-by: Sricharan R <r.sricharan@ti.com> --- There is a > checkpatch warning because of multiple assignments. The code looks > readable this way. > What/where? [snip] > + if ((boot_device >= BOOT_DEVICE_XIP) && + (boot_device <= > BOOT_DEVICE_MMC2)) { This will need to be rebased to use MMC_BOOT_DEVICES_START/END and I know you didn't test from eMMC on omap5_uevm then. - -- Tom -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRbByPAAoJENk4IS6UOR1W9tgQAIO3+Ejnxgy+oqo10SieOsw0 n4dgRvYwppNgpaR4RND5ldjHnYAdr7g3TPOZ8X0tPuw/VwgvpaSkMfAtxzKF3S8m QjVlPj2+XCTHI9fbg3Wz3kiHRTBYjlBnQ4o3fbnL+h7Sunci8a0HGrLzmfPfz6eA SR/jLFNtBCQNfh3p7ZikH4pnTCcf/kTfHWkGjIImt5/2Viv8XoQnbwFyTeJS9upZ s3BPlST+Az7p/D6qFTGR70fmq55H/cIF8iW6KxmD8I8Ezwa2S+hj6FIJ+NmRtEw8 6SOzGw61hGV4y60NelfbRA4KR8GqiFpbb6NtX+bBhX6Vu8DGJpsU/PE1jngA9xSt RCWqUVxP3LOGCxgI3brZe26kCgHkI8hGx/tDK2e9LF9MspkujL7TVxOTWMEARdZ/ XdJVGljTjovfbIVdWwt0NMASs9aY9gm3lbFtWfST9uZ8esEI1xRc9i19s1aGUf3k ND87nLgsf6xW1hbJbC+f5+UdUkBizu3+MSCAivNcsc8Haqubm/Ue1s+e7Bh3rkzA 3fo4HfYGqJnAcZiD6EtXhtObOSCEIl3tHOpHkl2kf29G5H8SbPTosXuEZvNESjzx PSNqyn9jSJHUPQLmPxRhi9ltfm4forZgYyBqQjHMUOBhhMZGdlrqaCSwTdxmwN2L z62KZtWwP9g5Ac64fWJ+ =8BS/ -----END PGP SIGNATURE-----
On Monday 15 April 2013 08:58 PM, Tom Rini wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 04/15/2013 11:08 AM, Sricharan R wrote: >> Currently save_boot_params saves the boot parameters passed from >> romcode. But this is not stored in a writable location >> consistently. So the current code would not work for a 'XIP' boot. >> Change this by saving the boot parameters in 'gd' which is always >> writable. Also add a 'C' function instead of an assembly code that >> is more readable. >> >> Signed-off-by: Sricharan R <r.sricharan@ti.com> --- There is a >> checkpatch warning because of multiple assignments. The code looks >> readable this way. >> > > What/where? > In the below line pf the patch. gd->arch.omap_boot_params.omap_bootdevice = boot_device = > [snip] >> + if ((boot_device >= BOOT_DEVICE_XIP) && + (boot_device <= >> BOOT_DEVICE_MMC2)) { > > This will need to be rebased to use MMC_BOOT_DEVICES_START/END and I > know you didn't test from eMMC on omap5_uevm then. > Yes, i was aware of this. Infact i saw before this series that emmc was broken and your patch was fixing that. When i started this series, your patch was not merged then. I can rebase on V2. Regards, Sricharan
Hi Sricharan, I very much like how you've structured this. A vast improvement! I haven't yet tried to apply the whole series but have one quick comment. In the new function: static void save_omap_boot_params(void) { ... if (!(omap_hw_init_context() == OMAP_INIT_CONTEXT_UBOOT_AFTER_SPL)) { ... } else { ... } wouldn't it be clearer to drop the boolean negation "!" and exchange the if/else bodies? Best regards, -Michael Cashwell
On Monday 15 April 2013 09:52 PM, Michael Cashwell wrote: > Hi Sricharan, > > I very much like how you've structured this. A vast improvement! > > I haven't yet tried to apply the whole series but have one quick comment. In the new function: > > static void save_omap_boot_params(void) > { > ... > if (!(omap_hw_init_context() == > OMAP_INIT_CONTEXT_UBOOT_AFTER_SPL)) { > ... > } else { > ... > } > > wouldn't it be clearer to drop the boolean negation "!" and exchange the if/else bodies? > hmm, will do and add a comment as well. Regards, Sricharan
diff --git a/arch/arm/cpu/armv7/omap-common/hwinit-common.c b/arch/arm/cpu/armv7/omap-common/hwinit-common.c index 70d16a8..602e76e 100644 --- a/arch/arm/cpu/armv7/omap-common/hwinit-common.c +++ b/arch/arm/cpu/armv7/omap-common/hwinit-common.c @@ -101,11 +101,6 @@ void omap_rev_string(void) } #ifdef CONFIG_SPL_BUILD -static void init_boot_params(void) -{ - boot_params_ptr = (u32 *) &boot_params; -} - void spl_display_print(void) { omap_rev_string(); @@ -116,6 +111,42 @@ void __weak srcomp_enable(void) { } +static void save_omap_boot_params(void) +{ + u32 rom_params = *((u32 *)OMAP_SRAM_SCRATCH_BOOT_PARAMS); + u8 boot_device; + u32 dev_desc, dev_data; + + if ((rom_params < NON_SECURE_SRAM_START) || + (rom_params > NON_SECURE_SRAM_END)) + return; + + /* + * rom_params can be type casted to omap_boot_parameters and + * used. But it not correct to assume that romcode structure + * encoding would be same as u-boot. So use the defined offsets. + */ + gd->arch.omap_boot_params.omap_bootdevice = boot_device = + *((u8 *)(rom_params + BOOT_DEVICE_OFFSET)); + + gd->arch.omap_boot_params.ch_flags = + *((u8 *)(rom_params + CH_FLAGS_OFFSET)); + + if ((boot_device >= BOOT_DEVICE_XIP) && + (boot_device <= BOOT_DEVICE_MMC2)) { + if (!(omap_hw_init_context() == + OMAP_INIT_CONTEXT_UBOOT_AFTER_SPL)) { + dev_desc = *((u32 *)(rom_params + DEV_DESC_PTR_OFFSET)); + dev_data = *((u32 *)(dev_desc + DEV_DATA_PTR_OFFSET)); + gd->arch.omap_boot_params.omap_bootmode = + *((u32 *)(dev_data + BOOT_MODE_OFFSET)); + } else { + gd->arch.omap_boot_params.omap_bootmode = + *((u8 *)(rom_params + BOOT_MODE_OFFSET)); + } + } +} + /* * Routine: s_init * Description: Does early system init of watchdog, muxing, andclocks @@ -132,6 +163,14 @@ void __weak srcomp_enable(void) */ void s_init(void) { + /* + * Save the boot parameters passed from romcode. + * We cannot delay the saving further than this, + * to prevent overwrites. + */ +#ifdef CONFIG_SPL_BUILD + save_omap_boot_params(); +#endif init_omap_revision(); hw_data_init(); @@ -156,7 +195,6 @@ void s_init(void) /* For regular u-boot sdram_init() is called from dram_init() */ sdram_init(); - init_boot_params(); #endif } diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 37ac0da..7611d0a 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -24,6 +24,10 @@ #ifndef __ASM_GBL_DATA_H #define __ASM_GBL_DATA_H +#ifdef CONFIG_OMAP +#include <asm/omap_boot.h> +#endif + /* Architecture-specific global data */ struct arch_global_data { #if defined(CONFIG_FSL_ESDHC) @@ -51,6 +55,10 @@ struct arch_global_data { unsigned long tlb_addr; unsigned long tlb_size; #endif + +#ifdef CONFIG_OMAP + struct omap_boot_parameters omap_boot_params; +#endif }; #include <asm-generic/global_data.h> diff --git a/arch/arm/include/asm/omap_boot.h b/arch/arm/include/asm/omap_boot.h index 87a9530..a803965 100644 --- a/arch/arm/include/asm/omap_boot.h +++ b/arch/arm/include/asm/omap_boot.h @@ -45,5 +45,6 @@ struct omap_boot_parameters { unsigned char omap_bootdevice; unsigned char reset_reason; unsigned char ch_flags; + unsigned long omap_bootmode; }; #endif diff --git a/arch/arm/include/asm/omap_common.h b/arch/arm/include/asm/omap_common.h index 6b70dbb..6b73d86 100644 --- a/arch/arm/include/asm/omap_common.h +++ b/arch/arm/include/asm/omap_common.h @@ -596,5 +596,7 @@ static inline u32 omap_revision(void) #define OMAP_SRAM_SCRATCH_DPLLS_PTR (SRAM_SCRATCH_SPACE_ADDR + 0x18) #define OMAP_SRAM_SCRATCH_VCORES_PTR (SRAM_SCRATCH_SPACE_ADDR + 0x1C) #define OMAP_SRAM_SCRATCH_SYS_CTRL (SRAM_SCRATCH_SPACE_ADDR + 0x20) -#define OMAP_SRAM_SCRATCH_SPACE_END (SRAM_SCRATCH_SPACE_ADDR + 0x24) +#define OMAP_SRAM_SCRATCH_BOOT_PARAMS (SRAM_SCRATCH_SPACE_ADDR + 0x24) +#define OMAP5_SRAM_SCRATCH_SPACE_END (SRAM_SCRATCH_SPACE_ADDR + 0x28) + #endif /* _OMAP_COMMON_H_ */
Currently save_boot_params saves the boot parameters passed from romcode. But this is not stored in a writable location consistently. So the current code would not work for a 'XIP' boot. Change this by saving the boot parameters in 'gd' which is always writable. Also add a 'C' function instead of an assembly code that is more readable. Signed-off-by: Sricharan R <r.sricharan@ti.com> --- There is a checkpatch warning because of multiple assignments. The code looks readable this way. arch/arm/cpu/armv7/omap-common/hwinit-common.c | 50 +++++++++++++++++++++--- arch/arm/include/asm/global_data.h | 8 ++++ arch/arm/include/asm/omap_boot.h | 1 + arch/arm/include/asm/omap_common.h | 4 +- 4 files changed, 56 insertions(+), 7 deletions(-)