diff mbox

[U-Boot,18/25] SPEAr: Correct the definition of CONFIG_SYS_MONITOR_BASE

Message ID 1331121854-20494-19-git-send-email-amit.virdi@st.com
State Superseded
Delegated to: Stefan Roese
Headers show

Commit Message

Amit Virdi March 7, 2012, 12:04 p.m. UTC 
From: Vipin Kumar <vipin.kumar@st.com>

The below text is copy pasted from README
- CONFIG_SYS_MONITOR_BASE:
	Physical start address of boot monitor code (set by
	make config files to be same as the text base address
	(TEXT_BASE) used when linking) - same as
	CONFIG_SYS_FLASH_BASE when booting from flash.

This patch corrects the definition of CONFIG_SYS_MONITOR_BASE and sets it to
TEXT_BASE

Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
Signed-off-by: Amit Virdi <amit.virdi@st.com>
---
 include/configs/spear-common.h |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

Comments

Stefan Roese March 7, 2012, 2:31 p.m. UTC | #1 
On Wednesday 07 March 2012 13:04:07 Amit Virdi wrote:
> From: Vipin Kumar <vipin.kumar@st.com>
> 
> The below text is copy pasted from README
> - CONFIG_SYS_MONITOR_BASE:
> 	Physical start address of boot monitor code (set by
> 	make config files to be same as the text base address
> 	(TEXT_BASE) used when linking) - same as
> 	CONFIG_SYS_FLASH_BASE when booting from flash.
> 
> This patch corrects the definition of CONFIG_SYS_MONITOR_BASE and sets it
> to TEXT_BASE
> 
> Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
> Signed-off-by: Amit Virdi <amit.virdi@st.com>
> ---
>  include/configs/spear-common.h |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/configs/spear-common.h
> b/include/configs/spear-common.h index 26642f1..0998157 100644
> --- a/include/configs/spear-common.h
> +++ b/include/configs/spear-common.h
> @@ -168,8 +168,7 @@
>  						"0x4C0000; bootm 
0x1600000"
>  #endif
> 
> -#define CONFIG_SYS_MONITOR_BASE			
CONFIG_SYS_FLASH_BASE
> -#define CONFIG_ENV_ADDR				
(CONFIG_SYS_MONITOR_BASE + \
> +#define CONFIG_ENV_ADDR				
(CONFIG_SYS_FLASH_BASE + \
>  						CONFIG_SYS_MONITOR_LEN)
>  #elif defined(CONFIG_ENV_IS_IN_NAND)
>  /*
> @@ -206,6 +205,8 @@
> 
> 
>  #define CONFIG_ENV_SIZE				0x02000
> +#define CONFIG_SYS_MONITOR_BASE			CONFIG_SYS_TEXT_BASE
> +#define CONFIG_MONITOR_IS_IN_RAM		1

Why is CONFIG_MONITOR_IS_IN_RAM defined? And if really needed, please without 
the 1.

Thanks,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
Amit Virdi March 27, 2012, 6:38 a.m. UTC | #2 
Stefan,

>>
>>
>>   #define CONFIG_ENV_SIZE				0x02000
>> +#define CONFIG_SYS_MONITOR_BASE			CONFIG_SYS_TEXT_BASE
>> +#define CONFIG_MONITOR_IS_IN_RAM		1
>
> Why is CONFIG_MONITOR_IS_IN_RAM defined? And if really needed, please without
> the 1.
>

CONFIG_MONITOR_IS_IN_RAM is defined with the understanding that since, 
u-boot is loaded by the BootROM (embedded in eROM) to RAM, so the 
monitor is present in RAM. Please confirm if this understanding is correct.

Thanks
Amit Virdi
Stefan Roese March 27, 2012, 7:05 a.m. UTC | #3 
Amit,

On Tuesday 27 March 2012 08:38:18 Amit Virdi wrote:
> >>   #define CONFIG_ENV_SIZE				0x02000
> >> 
> >> +#define CONFIG_SYS_MONITOR_BASE			CONFIG_SYS_TEXT_BASE
> >> +#define CONFIG_MONITOR_IS_IN_RAM		1
> > 
> > Why is CONFIG_MONITOR_IS_IN_RAM defined? And if really needed, please
> > without the 1.
> 
> CONFIG_MONITOR_IS_IN_RAM is defined with the understanding that since,
> u-boot is loaded by the BootROM (embedded in eROM) to RAM, so the
> monitor is present in RAM. Please confirm if this understanding is correct.

Hmmm, seems to be used on some Coldfire and Blackfin platforms. Its not needed 
for your platform. Please remove it.

Thanks,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
Amit Virdi March 27, 2012, 7:42 a.m. UTC | #4 
On 3/27/2012 12:35 PM, Stefan Roese wrote:
> Amit,
>
> On Tuesday 27 March 2012 08:38:18 Amit Virdi wrote:
>>>>    #define CONFIG_ENV_SIZE				0x02000
>>>>
>>>> +#define CONFIG_SYS_MONITOR_BASE			CONFIG_SYS_TEXT_BASE
>>>> +#define CONFIG_MONITOR_IS_IN_RAM		1
>>>
>>> Why is CONFIG_MONITOR_IS_IN_RAM defined? And if really needed, please
>>> without the 1.
>>
>> CONFIG_MONITOR_IS_IN_RAM is defined with the understanding that since,
>> u-boot is loaded by the BootROM (embedded in eROM) to RAM, so the
>> monitor is present in RAM. Please confirm if this understanding is correct.
>
> Hmmm, seems to be used on some Coldfire and Blackfin platforms. Its not needed
> for your platform. Please remove it.
>

Ok. Thanks for the clarification!

Best Regards
Amit Virdi
diff mbox

Patch

diff --git a/include/configs/spear-common.h b/include/configs/spear-common.h
index 26642f1..0998157 100644
--- a/include/configs/spear-common.h
+++ b/include/configs/spear-common.h
@@ -168,8 +168,7 @@ 
 						"0x4C0000; bootm 0x1600000"
 #endif
 
-#define CONFIG_SYS_MONITOR_BASE			CONFIG_SYS_FLASH_BASE
-#define CONFIG_ENV_ADDR				(CONFIG_SYS_MONITOR_BASE + \
+#define CONFIG_ENV_ADDR				(CONFIG_SYS_FLASH_BASE + \
 						CONFIG_SYS_MONITOR_LEN)
 #elif defined(CONFIG_ENV_IS_IN_NAND)
 /*
@@ -206,6 +205,8 @@ 
 
 
 #define CONFIG_ENV_SIZE				0x02000
+#define CONFIG_SYS_MONITOR_BASE			CONFIG_SYS_TEXT_BASE
+#define CONFIG_MONITOR_IS_IN_RAM		1
 
 /* Miscellaneous configurable options */
 #define CONFIG_ARCH_CPU_INIT