Patchwork [U-Boot] SPEAr: Add ARM relocation support

login
register
mail settings
Submitter Amit Virdi
Date Feb. 16, 2012, 11:43 a.m.
Message ID <1329392635-22460-1-git-send-email-amit.virdi@st.com>
Download mbox | patch
Permalink /patch/141576/
State Accepted
Commit 5cca72f8b342c8a65eaebda22647572394d98535
Headers show

Comments

Amit Virdi - Feb. 16, 2012, 11:43 a.m.
Signed-off-by: Amit Virdi <amit.virdi@st.com>
---
 board/spear/common/spr_misc.c |   20 +-------------------
 1 files changed, 1 insertions(+), 19 deletions(-)
Marek Vasut - Feb. 16, 2012, 7:43 p.m.
> Signed-off-by: Amit Virdi <amit.virdi@st.com>

Uh, are you sure the Subject correlates with the patch ?

M

> ---
>  board/spear/common/spr_misc.c |   20 +-------------------
>  1 files changed, 1 insertions(+), 19 deletions(-)
> 
> diff --git a/board/spear/common/spr_misc.c b/board/spear/common/spr_misc.c
> index 0812c20..3ab278f 100644
> --- a/board/spear/common/spr_misc.c
> +++ b/board/spear/common/spr_misc.c
> @@ -40,27 +40,9 @@ static struct chip_data chip_data;
> 
>  int dram_init(void)
>  {
> -	struct xloader_table *xloader_tb =
> -	    (struct xloader_table *)XLOADER_TABLE_ADDRESS;
> -	struct xloader_table_1_1 *table_1_1;
> -	struct xloader_table_1_2 *table_1_2;
> -	struct chip_data *chip = &chip_data;
> -
> +	/* Store complete RAM size and return */
>  	gd->ram_size = get_ram_size(PHYS_SDRAM_1, PHYS_SDRAM_1_MAXSIZE);
> 
> -	if (XLOADER_TABLE_VERSION_1_1 == xloader_tb->table_version) {
> -		table_1_1 = &xloader_tb->table.table_1_1;
> -		chip->dramfreq = table_1_1->ddrfreq;
> -		chip->dramtype = table_1_1->ddrtype;
> -
> -	} else if (XLOADER_TABLE_VERSION_1_2 == xloader_tb->table_version) {
> -		table_1_2 = &xloader_tb->table.table_1_2;
> -		chip->dramfreq = table_1_2->ddrfreq;
> -		chip->dramtype = table_1_2->ddrtype;
> -	} else {
> -		chip->dramfreq = -1;
> -	}
> -
>  	return 0;
>  }
Amit Virdi - Feb. 17, 2012, 4:06 a.m.
On 2/17/2012 1:13 AM, Marek Vasut wrote:
>> Signed-off-by: Amit Virdi<amit.virdi@st.com>
>
> Uh, are you sure the Subject correlates with the patch ?
>
> M
>

Yeah. The fix is a part of the Relocation feature. For relocation, it is 
essential that dram_init stores RAM size and do nothing more i.e. no 
access to bss etc.

~Amit Virdi

>> ---
>>   board/spear/common/spr_misc.c |   20 +-------------------
>>   1 files changed, 1 insertions(+), 19 deletions(-)
>>
>> diff --git a/board/spear/common/spr_misc.c b/board/spear/common/spr_misc.c
>> index 0812c20..3ab278f 100644
>> --- a/board/spear/common/spr_misc.c
>> +++ b/board/spear/common/spr_misc.c
>> @@ -40,27 +40,9 @@ static struct chip_data chip_data;
>>
>>   int dram_init(void)
>>   {
>> -	struct xloader_table *xloader_tb =
>> -	    (struct xloader_table *)XLOADER_TABLE_ADDRESS;
>> -	struct xloader_table_1_1 *table_1_1;
>> -	struct xloader_table_1_2 *table_1_2;
>> -	struct chip_data *chip =&chip_data;
>> -
>> +	/* Store complete RAM size and return */
>>   	gd->ram_size = get_ram_size(PHYS_SDRAM_1, PHYS_SDRAM_1_MAXSIZE);
>>
>> -	if (XLOADER_TABLE_VERSION_1_1 == xloader_tb->table_version) {
>> -		table_1_1 =&xloader_tb->table.table_1_1;
>> -		chip->dramfreq = table_1_1->ddrfreq;
>> -		chip->dramtype = table_1_1->ddrtype;
>> -
>> -	} else if (XLOADER_TABLE_VERSION_1_2 == xloader_tb->table_version) {
>> -		table_1_2 =&xloader_tb->table.table_1_2;
>> -		chip->dramfreq = table_1_2->ddrfreq;
>> -		chip->dramtype = table_1_2->ddrtype;
>> -	} else {
>> -		chip->dramfreq = -1;
>> -	}
>> -
>>   	return 0;
>>   }
> .
>
Marek Vasut - Feb. 17, 2012, 8:58 a.m.
> On 2/17/2012 1:13 AM, Marek Vasut wrote:
> >> Signed-off-by: Amit Virdi<amit.virdi@st.com>
> > 
> > Uh, are you sure the Subject correlates with the patch ?
> > 
> > M
> 
> Yeah. The fix is a part of the Relocation feature. For relocation, it is
> essential that dram_init stores RAM size and do nothing more i.e. no
> access to bss etc.

Then name it "fix dram size reporting?". Also, you don't need dram_init at all 
if you only call get_dram_size() if I recall correctly, check 
arch/arm/lib/board.c, I might be mistaken. And maybe explain why you removed all 
the code in the changelog, it's not clear at all.

Thanks!

> 
> ~Amit Virdi
> 
> >> ---
> >> 
> >>   board/spear/common/spr_misc.c |   20 +-------------------
> >>   1 files changed, 1 insertions(+), 19 deletions(-)
> >> 
> >> diff --git a/board/spear/common/spr_misc.c
> >> b/board/spear/common/spr_misc.c index 0812c20..3ab278f 100644
> >> --- a/board/spear/common/spr_misc.c
> >> +++ b/board/spear/common/spr_misc.c
> >> @@ -40,27 +40,9 @@ static struct chip_data chip_data;
> >> 
> >>   int dram_init(void)
> >>   {
> >> 
> >> -	struct xloader_table *xloader_tb =
> >> -	    (struct xloader_table *)XLOADER_TABLE_ADDRESS;
> >> -	struct xloader_table_1_1 *table_1_1;
> >> -	struct xloader_table_1_2 *table_1_2;
> >> -	struct chip_data *chip =&chip_data;
> >> -
> >> +	/* Store complete RAM size and return */
> >> 
> >>   	gd->ram_size = get_ram_size(PHYS_SDRAM_1, PHYS_SDRAM_1_MAXSIZE);
> >> 
> >> -	if (XLOADER_TABLE_VERSION_1_1 == xloader_tb->table_version) {
> >> -		table_1_1 =&xloader_tb->table.table_1_1;
> >> -		chip->dramfreq = table_1_1->ddrfreq;
> >> -		chip->dramtype = table_1_1->ddrtype;
> >> -
> >> -	} else if (XLOADER_TABLE_VERSION_1_2 == xloader_tb->table_version) {
> >> -		table_1_2 =&xloader_tb->table.table_1_2;
> >> -		chip->dramfreq = table_1_2->ddrfreq;
> >> -		chip->dramtype = table_1_2->ddrtype;
> >> -	} else {
> >> -		chip->dramfreq = -1;
> >> -	}
> >> -
> >> 
> >>   	return 0;
> >>   
> >>   }
> > 
> > .
Albert ARIBAUD - Feb. 17, 2012, 9:13 a.m.
Hi Amit,

Le 17/02/2012 05:06, Amit Virdi a écrit :
>
> On 2/17/2012 1:13 AM, Marek Vasut wrote:
>>> Signed-off-by: Amit Virdi<amit.virdi@st.com>
>>
>> Uh, are you sure the Subject correlates with the patch ?
>>
>> M
>>
>
> Yeah. The fix is a part of the Relocation feature. For relocation, it is
> essential that dram_init stores RAM size and do nothing more i.e. no
> access to bss etc.

So the subject *is* wrong: the patch does not add relocation support, it 
fixes it.

Also, please fix my e-mail address in the recipients list. Somemone has 
me as ...@aribaud.ne instead of "@aribaud.net", and did not notice it.

> ~Amit Virdi

Amicalement,
Amit Virdi - Feb. 18, 2012, 7:33 a.m.
Hello Marek,

On Fri, Feb 17, 2012 at 2:28 PM, Marek Vasut <marek.vasut@gmail.com> wrote:

> > On 2/17/2012 1:13 AM, Marek Vasut wrote:
> > >> Signed-off-by: Amit Virdi<amit.virdi@st.com>
> > >
> > > Uh, are you sure the Subject correlates with the patch ?
> > >
> > > M
> >
> > Yeah. The fix is a part of the Relocation feature. For relocation, it is
> > essential that dram_init stores RAM size and do nothing more i.e. no
> > access to bss etc.
>
> Then name it "fix dram size reporting?". Also, you don't need dram_init at
> all
> if you only call get_dram_size() if I recall correctly, check
> arch/arm/lib/board.c, I might be mistaken. And maybe explain why you
> removed all
> the code in the changelog, it's not clear at all.
>
>
Thanks for your suggestions. I got your point. I'll send V2 with correct
subject and clear message.

Regards
Amit Virdi


Thanks!
>
> >
> > ~Amit Virdi
> >
> > >> ---
> > >>
> > >>   board/spear/common/spr_misc.c |   20 +-------------------
> > >>   1 files changed, 1 insertions(+), 19 deletions(-)
> > >>
> > >> diff --git a/board/spear/common/spr_misc.c
> > >> b/board/spear/common/spr_misc.c index 0812c20..3ab278f 100644
> > >> --- a/board/spear/common/spr_misc.c
> > >> +++ b/board/spear/common/spr_misc.c
> > >> @@ -40,27 +40,9 @@ static struct chip_data chip_data;
> > >>
> > >>   int dram_init(void)
> > >>   {
> > >>
> > >> -  struct xloader_table *xloader_tb =
> > >> -      (struct xloader_table *)XLOADER_TABLE_ADDRESS;
> > >> -  struct xloader_table_1_1 *table_1_1;
> > >> -  struct xloader_table_1_2 *table_1_2;
> > >> -  struct chip_data *chip =&chip_data;
> > >> -
> > >> +  /* Store complete RAM size and return */
> > >>
> > >>    gd->ram_size = get_ram_size(PHYS_SDRAM_1, PHYS_SDRAM_1_MAXSIZE);
> > >>
> > >> -  if (XLOADER_TABLE_VERSION_1_1 == xloader_tb->table_version) {
> > >> -          table_1_1 =&xloader_tb->table.table_1_1;
> > >> -          chip->dramfreq = table_1_1->ddrfreq;
> > >> -          chip->dramtype = table_1_1->ddrtype;
> > >> -
> > >> -  } else if (XLOADER_TABLE_VERSION_1_2 == xloader_tb->table_version)
> {
> > >> -          table_1_2 =&xloader_tb->table.table_1_2;
> > >> -          chip->dramfreq = table_1_2->ddrfreq;
> > >> -          chip->dramtype = table_1_2->ddrtype;
> > >> -  } else {
> > >> -          chip->dramfreq = -1;
> > >> -  }
> > >> -
> > >>
> > >>    return 0;
> > >>
> > >>   }
> > >
> > > .
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
Amit Virdi - Feb. 18, 2012, 7:35 a.m.
Ciao Albert,

On Fri, Feb 17, 2012 at 2:43 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net>wrote:

> Hi Amit,
>
> Le 17/02/2012 05:06, Amit Virdi a écrit :
>
>
>> On 2/17/2012 1:13 AM, Marek Vasut wrote:
>>
>>> Signed-off-by: Amit Virdi<amit.virdi@st.com>
>>>>
>>>
>>> Uh, are you sure the Subject correlates with the patch ?
>>>
>>> M
>>>
>>>
>> Yeah. The fix is a part of the Relocation feature. For relocation, it is
>> essential that dram_init stores RAM size and do nothing more i.e. no
>> access to bss etc.
>>
>
> So the subject *is* wrong: the patch does not add relocation support, it
> fixes it.
>
>
Yes. I'll correct the subject and shall be sending V2.


> Also, please fix my e-mail address in the recipients list. Somemone has me
> as ...@aribaud.ne instead of "@aribaud.net", and did not notice it.
>
>
Sure!!

Regards
Amit Virdi
Marek Vasut - Feb. 18, 2012, 7:45 a.m.
> Hello Marek,
> 
> On Fri, Feb 17, 2012 at 2:28 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > > On 2/17/2012 1:13 AM, Marek Vasut wrote:
> > > >> Signed-off-by: Amit Virdi<amit.virdi@st.com>
> > > > 
> > > > Uh, are you sure the Subject correlates with the patch ?
> > > > 
> > > > M
> > > 
> > > Yeah. The fix is a part of the Relocation feature. For relocation, it
> > > is essential that dram_init stores RAM size and do nothing more i.e.
> > > no access to bss etc.
> > 
> > Then name it "fix dram size reporting?". Also, you don't need dram_init
> > at all
> > if you only call get_dram_size() if I recall correctly, check
> > arch/arm/lib/board.c, I might be mistaken. And maybe explain why you
> > removed all
> > the code in the changelog, it's not clear at all.
> 
> Thanks for your suggestions. I got your point. I'll send V2 with correct
> subject and clear message.
> 
> Regards
> Amit Virdi

Good, thanks! :)

M

> 
> 
> Thanks!
> 
> > > ~Amit Virdi
> > > 
> > > >> ---
> > > >> 
> > > >>   board/spear/common/spr_misc.c |   20 +-------------------
> > > >>   1 files changed, 1 insertions(+), 19 deletions(-)
> > > >> 
> > > >> diff --git a/board/spear/common/spr_misc.c
> > > >> b/board/spear/common/spr_misc.c index 0812c20..3ab278f 100644
> > > >> --- a/board/spear/common/spr_misc.c
> > > >> +++ b/board/spear/common/spr_misc.c
> > > >> @@ -40,27 +40,9 @@ static struct chip_data chip_data;
> > > >> 
> > > >>   int dram_init(void)
> > > >>   {
> > > >> 
> > > >> -  struct xloader_table *xloader_tb =
> > > >> -      (struct xloader_table *)XLOADER_TABLE_ADDRESS;
> > > >> -  struct xloader_table_1_1 *table_1_1;
> > > >> -  struct xloader_table_1_2 *table_1_2;
> > > >> -  struct chip_data *chip =&chip_data;
> > > >> -
> > > >> +  /* Store complete RAM size and return */
> > > >> 
> > > >>    gd->ram_size = get_ram_size(PHYS_SDRAM_1, PHYS_SDRAM_1_MAXSIZE);
> > > >> 
> > > >> -  if (XLOADER_TABLE_VERSION_1_1 == xloader_tb->table_version) {
> > > >> -          table_1_1 =&xloader_tb->table.table_1_1;
> > > >> -          chip->dramfreq = table_1_1->ddrfreq;
> > > >> -          chip->dramtype = table_1_1->ddrtype;
> > > >> -
> > > >> -  } else if (XLOADER_TABLE_VERSION_1_2 ==
> > > >> xloader_tb->table_version)
> > 
> > {
> > 
> > > >> -          table_1_2 =&xloader_tb->table.table_1_2;
> > > >> -          chip->dramfreq = table_1_2->ddrfreq;
> > > >> -          chip->dramtype = table_1_2->ddrtype;
> > > >> -  } else {
> > > >> -          chip->dramfreq = -1;
> > > >> -  }
> > > >> -
> > > >> 
> > > >>    return 0;
> > > >>   
> > > >>   }
> > > > 
> > > > .
> > 
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot

Patch

diff --git a/board/spear/common/spr_misc.c b/board/spear/common/spr_misc.c
index 0812c20..3ab278f 100644
--- a/board/spear/common/spr_misc.c
+++ b/board/spear/common/spr_misc.c
@@ -40,27 +40,9 @@  static struct chip_data chip_data;
 
 int dram_init(void)
 {
-	struct xloader_table *xloader_tb =
-	    (struct xloader_table *)XLOADER_TABLE_ADDRESS;
-	struct xloader_table_1_1 *table_1_1;
-	struct xloader_table_1_2 *table_1_2;
-	struct chip_data *chip = &chip_data;
-
+	/* Store complete RAM size and return */
 	gd->ram_size = get_ram_size(PHYS_SDRAM_1, PHYS_SDRAM_1_MAXSIZE);
 
-	if (XLOADER_TABLE_VERSION_1_1 == xloader_tb->table_version) {
-		table_1_1 = &xloader_tb->table.table_1_1;
-		chip->dramfreq = table_1_1->ddrfreq;
-		chip->dramtype = table_1_1->ddrtype;
-
-	} else if (XLOADER_TABLE_VERSION_1_2 == xloader_tb->table_version) {
-		table_1_2 = &xloader_tb->table.table_1_2;
-		chip->dramfreq = table_1_2->ddrfreq;
-		chip->dramtype = table_1_2->ddrtype;
-	} else {
-		chip->dramfreq = -1;
-	}
-
 	return 0;
 }