Patchwork [U-Boot,v4,6/6] SMDK6400: Fix SMDK6400 SDRAM init

login
register
mail settings
Submitter seedshope
Date Jan. 21, 2011, 3:34 p.m.
Message ID <1295624053-8060-7-git-send-email-bocui107@gmail.com>
Download mbox | patch
Permalink /patch/79856/
State Rejected
Headers show

Comments

seedshope - Jan. 21, 2011, 3:34 p.m.
Since SDRAM init function have already change, So the SDRAM
initial function must be change.

Signed-off-by: seedshope <bocui107@gmail.com>
---
 board/samsung/smdk6400/smdk6400.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
Sergei Shtylyov - Jan. 21, 2011, 5:52 p.m.
Hello.

seedshope wrote:

> Since SDRAM init function have already change, So the SDRAM
> initial function must be change.

    This description sounds somewhat tautological...

> Signed-off-by: seedshope <bocui107@gmail.com>

    Your real name is required in the signoff.

> ---
>  board/samsung/smdk6400/smdk6400.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)

> diff --git a/board/samsung/smdk6400/smdk6400.c b/board/samsung/smdk6400/smdk6400.c
> index 35aa40b..1d03b7a 100644
> --- a/board/samsung/smdk6400/smdk6400.c
> +++ b/board/samsung/smdk6400/smdk6400.c
> @@ -78,10 +78,18 @@ int board_init(void)
>  	return 0;
>  }
>  
> -int dram_init(void)
> +void dram_init_banksize(void)
>  {
> +	DECLARE_GLOBAL_DATA_PTR;
> +
>  	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
>  	gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
> +}
> +
> +int dram_init(void)
> +{
> +	gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
> +			PHYS_SDRAM_1_SIZE);

   Could you move this last line more to the right?

WBR, Sergei
seedshope - Jan. 21, 2011, 6:05 p.m.
On 01/22/2011 01:52 AM, Sergei Shtylyov wrote:
> Hello.
>
> seedshope wrote:
>
>> Since SDRAM init function have already change, So the SDRAM
>> initial function must be change.
>
>    This description sounds somewhat tautological...
If I describe  as following:
Since SDRAM init function have already change, Modify SDRAM inital
function to adapt to it.

How about it?
>
>> Signed-off-by: seedshope <bocui107@gmail.com>
>
>    Your real name is required in the signoff.
I use the name for my pen name. It is not problem.
>
>> ---
>>  board/samsung/smdk6400/smdk6400.c |   10 +++++++++-
>>  1 files changed, 9 insertions(+), 1 deletions(-)
>
>> diff --git a/board/samsung/smdk6400/smdk6400.c 
>> b/board/samsung/smdk6400/smdk6400.c
>> index 35aa40b..1d03b7a 100644
>> --- a/board/samsung/smdk6400/smdk6400.c
>> +++ b/board/samsung/smdk6400/smdk6400.c
>> @@ -78,10 +78,18 @@ int board_init(void)
>>      return 0;
>>  }
>>
>> -int dram_init(void)
>> +void dram_init_banksize(void)
>>  {
>> +    DECLARE_GLOBAL_DATA_PTR;
>> +
>>      gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
>>      gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
>> +}
>> +
>> +int dram_init(void)
>> +{
>> +    gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
>> +            PHYS_SDRAM_1_SIZE);
>
>   Could you move this last line more to the right?
Ya, the orig is ok, But I re-do the patch, It is miss. sorry.

Thanks
seedshope
>
> WBR, Sergei
seedshope - Jan. 21, 2011, 6:15 p.m.
On 01/22/2011 02:05 AM, seedshope wrote:
> On 01/22/2011 01:52 AM, Sergei Shtylyov wrote:
>> Hello.
>>
>> seedshope wrote:
>>
>>> Since SDRAM init function have already change, So the SDRAM
>>> initial function must be change.
>>
>>    This description sounds somewhat tautological...
> If I describe  as following:
> Since SDRAM init function have already change, Modify SDRAM inital
> function to adapt to it.
>
> How about it?
>>
>>> Signed-off-by: seedshope <bocui107@gmail.com>
>>
>>    Your real name is required in the signoff.
> I use the name for my pen name. It is not problem.
>>
>>> ---
>>>  board/samsung/smdk6400/smdk6400.c |   10 +++++++++-
>>>  1 files changed, 9 insertions(+), 1 deletions(-)
>>
>>> diff --git a/board/samsung/smdk6400/smdk6400.c 
>>> b/board/samsung/smdk6400/smdk6400.c
>>> index 35aa40b..1d03b7a 100644
>>> --- a/board/samsung/smdk6400/smdk6400.c
>>> +++ b/board/samsung/smdk6400/smdk6400.c
>>> @@ -78,10 +78,18 @@ int board_init(void)
>>>      return 0;
>>>  }
>>>
>>> -int dram_init(void)
>>> +void dram_init_banksize(void)
>>>  {
>>> +    DECLARE_GLOBAL_DATA_PTR;
>>> +
>>>      gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
>>>      gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
>>> +}
>>> +
>>> +int dram_init(void)
>>> +{
>>> +    gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
>>> +            PHYS_SDRAM_1_SIZE);
>>
>>   Could you move this last line more to the right?
> Ya, the orig is ok, But I re-do the patch, It is miss. sorry.
Hi Sergei,

I feel this may be you e-mail issue. I open my patch, It is display as 
following:
+         gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
+                                    PHYS_SDRAM_1_SIZE);

Thanks,
seedshope
>
> Thanks
> seedshope
>>
>> WBR, Sergei
>
Albert ARIBAUD - Jan. 21, 2011, 6:29 p.m.
Le 21/01/2011 19:15, seedshope a écrit :
> On 01/22/2011 02:05 AM, seedshope wrote:
>> On 01/22/2011 01:52 AM, Sergei Shtylyov wrote:
>>> Hello.
>>>
>>> seedshope wrote:
>>>
>>>> Since SDRAM init function have already change, So the SDRAM
>>>> initial function must be change.
>>>
>>>     This description sounds somewhat tautological...
>> If I describe  as following:
>> Since SDRAM init function have already change, Modify SDRAM inital
>> function to adapt to it.
>>
>> How about it?

Still unclear, due to the fact you're using the same three terms 
("init/initial, RAM, function") for two apparently different things.

>>>> Signed-off-by: seedshope<bocui107@gmail.com>
>>>
>>>     Your real name is required in the signoff.
>> I use the name for my pen name. It is not problem.

I think Sergei means pen names should not be used. I won't personally 
pass judgment, but so far I've always seen contributors using their 
actual names.

> I feel this may be you e-mail issue. I open my patch, It is display as
> following:
>
> +         gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
> +                                    PHYS_SDRAM_1_SIZE);

Your patch, pulled from patchwork and viewed in vi, has three tabs on 
that second line, which does not align properly. You should check your 
code editor settings re: tabs.

Amicalement,
seedshope - Jan. 21, 2011, 6:43 p.m.
On 01/22/2011 02:29 AM, Albert ARIBAUD wrote:
> Le 21/01/2011 19:15, seedshope a écrit :
>> On 01/22/2011 02:05 AM, seedshope wrote:
>>> On 01/22/2011 01:52 AM, Sergei Shtylyov wrote:
>>>> Hello.
>>>>
>>>> seedshope wrote:
>>>>
>>>>> Since SDRAM init function have already change, So the SDRAM
>>>>> initial function must be change.
>>>>      This description sounds somewhat tautological...
>>> If I describe  as following:
>>> Since SDRAM init function have already change, Modify SDRAM inital
>>> function to adapt to it.
>>>
>>> How about it?
> Still unclear, due to the fact you're using the same three terms
> ("init/initial, RAM, function") for two apparently different things.
Ya, Maybe, But I don't know to describe it.

The patch is only to modify the dram_init() and dram_init_banksize(),
Could you help me to describe?

Thank you very much!
seedshope
>>>>> Signed-off-by: seedshope<bocui107@gmail.com>
>>>>      Your real name is required in the signoff.
>>> I use the name for my pen name. It is not problem.
> I think Sergei means pen names should not be used. I won't personally
> pass judgment, but so far I've always seen contributors using their
> actual names.
>
ok
>> I feel this may be you e-mail issue. I open my patch, It is display as
>> following:
>>
>> +         gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
>> +                                    PHYS_SDRAM_1_SIZE);
> Your patch, pulled from patchwork and viewed in vi, has three tabs on
> that second line, which does not align properly. You should check your
> code editor settings re: tabs.
My patch is ok, I just two tabs in my e-mail, But I sent the mail,
It is change.

Thanks
hongbo
> Amicalement,
Albert ARIBAUD - Jan. 21, 2011, 7:11 p.m.
Le 21/01/2011 19:43, seedshope a écrit :

> On 01/22/2011 02:29 AM, Albert ARIBAUD wrote:
>> Le 21/01/2011 19:15, seedshope a écrit :
>>> On 01/22/2011 02:05 AM, seedshope wrote:
>>>> On 01/22/2011 01:52 AM, Sergei Shtylyov wrote:
>>>>> Hello.
>>>>>
>>>>> seedshope wrote:
>>>>>
>>>>>> Since SDRAM init function have already change, So the SDRAM
>>>>>> initial function must be change.
>>>>>       This description sounds somewhat tautological...
>>>> If I describe  as following:
>>>> Since SDRAM init function have already change, Modify SDRAM inital
>>>> function to adapt to it.
>>>>
>>>> How about it?
>> Still unclear, due to the fact you're using the same three terms
>> ("init/initial, RAM, function") for two apparently different things.
> Ya, Maybe, But I don't know to describe it.
>
> The patch is only to modify the dram_init() and dram_init_banksize(),
> Could you help me to describe?
>
> Thank you very much!
> seedshope

The reason for the change to dram_init is not actually about DRAM. If 
you look up similar patches, you'll find out it is about not being able 
to access gd->bd because bd does not exist, and this is so since the ELF 
relocation was introduced. So some good descriptions could be "do not 
use gd->bd any more" or "fix dram_init for relocation support", for 
instance.

>>>>>> Signed-off-by: seedshope<bocui107@gmail.com>
>>>>>       Your real name is required in the signoff.
>>>> I use the name for my pen name. It is not problem.
>> I think Sergei means pen names should not be used. I won't personally
>> pass judgment, but so far I've always seen contributors using their
>> actual names.
>>
> ok
>>> I feel this may be you e-mail issue. I open my patch, It is display as
>>> following:
>>>
>>> +         gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
>>> +                                    PHYS_SDRAM_1_SIZE);
>> Your patch, pulled from patchwork and viewed in vi, has three tabs on
>> that second line, which does not align properly. You should check your
>> code editor settings re: tabs.
> My patch is ok, I just two tabs in my e-mail, But I sent the mail,
> It is change.

Do you send the patch through git format-patch and git send-email? Many 
e-mail softwares have weird issues when posting git patches, which is 
why git has its own tools for sending patches via e-mail.

> Thanks
> hongbo

Amicalement,
seedshope - Jan. 22, 2011, 1:56 a.m.
On 01/22/2011 03:11 AM, Albert ARIBAUD wrote:
> Le 21/01/2011 19:43, seedshope a écrit :
>
>> On 01/22/2011 02:29 AM, Albert ARIBAUD wrote:
>>> Le 21/01/2011 19:15, seedshope a écrit :
>>>> On 01/22/2011 02:05 AM, seedshope wrote:
>>>>> On 01/22/2011 01:52 AM, Sergei Shtylyov wrote:
>>>>>> Hello.
>>>>>>
>>>>>> seedshope wrote:
>>>>>>
>>>>>>> Since SDRAM init function have already change, So the SDRAM
>>>>>>> initial function must be change.
>>>>>>        This description sounds somewhat tautological...
>>>>> If I describe  as following:
>>>>> Since SDRAM init function have already change, Modify SDRAM inital
>>>>> function to adapt to it.
>>>>>
>>>>> How about it?
>>> Still unclear, due to the fact you're using the same three terms
>>> ("init/initial, RAM, function") for two apparently different things.
>> Ya, Maybe, But I don't know to describe it.
>>
>> The patch is only to modify the dram_init() and dram_init_banksize(),
>> Could you help me to describe?
>>
>> Thank you very much!
>> seedshope
> The reason for the change to dram_init is not actually about DRAM. If
> you look up similar patches, you'll find out it is about not being able
> to access gd->bd because bd does not exist, and this is so since the ELF
> relocation was introduced. So some good descriptions could be "do not
> use gd->bd any more" or "fix dram_init for relocation support", for
> instance.
>
ok,
>>>>>>> Signed-off-by: seedshope<bocui107@gmail.com>
>>>>>>        Your real name is required in the signoff.
>>>>> I use the name for my pen name. It is not problem.
>>> I think Sergei means pen names should not be used. I won't personally
>>> pass judgment, but so far I've always seen contributors using their
>>> actual names.
>>>
>> ok
>>>> I feel this may be you e-mail issue. I open my patch, It is display as
>>>> following:
>>>>
>>>> +         gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
>>>> +                                    PHYS_SDRAM_1_SIZE);
>>> Your patch, pulled from patchwork and viewed in vi, has three tabs on
>>> that second line, which does not align properly. You should check your
>>> code editor settings re: tabs.
>> My patch is ok, I just two tabs in my e-mail, But I sent the mail,
>> It is change.
> Do you send the patch through git format-patch and git send-email?
Yes, I use the git format-patch and git send-email
> Many
> e-mail softwares have weird issues when posting git patches, which is
> why git has its own tools for sending patches via e-mail.
ok

Thanks
seedshope

>> Thanks
>> hongbo
> Amicalement,
Albert ARIBAUD - Jan. 22, 2011, 7:31 a.m.
Hi seedshope,

Le 22/01/2011 02:56, seedshope a écrit :

>>> My patch is ok, I just two tabs in my e-mail, But I sent the mail,
>>> It is change.
>> Do you send the patch through git format-patch and git send-email?

> Yes, I use the git format-patch and git send-email

>> Many e-mail softwares have weird issues when posting git patches,
>> which is why git has its own tools for sending patches via e-mail.

> ok

Since you're using git format-patch and git send-email, then your 
original change is not correctly aligned. I suggest you check your code 
editor's settings on indentation and use of tabulations, notably the 
"tab size": tabs should align on 8-space multiples; also check that your 
editor uses a fixed font -- you never know.

>>> Thanks
>>> hongbo

Amicalement,
Minkyu Kang - Jan. 22, 2011, 8:26 a.m.
Dear seedshope,

On 22 January 2011 00:34, seedshope <bocui107@gmail.com> wrote:
> Since SDRAM init function have already change, So the SDRAM
> initial function must be change.
>
> Signed-off-by: seedshope <bocui107@gmail.com>
> ---
>  board/samsung/smdk6400/smdk6400.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/board/samsung/smdk6400/smdk6400.c b/board/samsung/smdk6400/smdk6400.c
> index 35aa40b..1d03b7a 100644
> --- a/board/samsung/smdk6400/smdk6400.c
> +++ b/board/samsung/smdk6400/smdk6400.c
> @@ -78,10 +78,18 @@ int board_init(void)
>        return 0;
>  }
>
> -int dram_init(void)
> +void dram_init_banksize(void)
>  {
> +       DECLARE_GLOBAL_DATA_PTR;

Please remove it.

> +
>        gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
>        gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;

Thanks
Minkyu Kang
Thomas Langer - Jan. 22, 2011, 9:15 a.m.
Hi seedshope,

seedshope <bocui107 <at> gmail.com> writes:

> -int dram_init(void)
> +void dram_init_banksize(void)
>  {
> +	DECLARE_GLOBAL_DATA_PTR;

This declaration should be done on file scope, not in a function.

> +
>  	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
>  	gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
> +}
> +

Best regards,
Thomas
seedshope - Jan. 22, 2011, 7:23 p.m.
On 01/22/2011 03:31 PM, Albert ARIBAUD wrote:
> Hi seedshope,
>
> Le 22/01/2011 02:56, seedshope a écrit :
>
>>>> My patch is ok, I just two tabs in my e-mail, But I sent the mail,
>>>> It is change.
>>> Do you send the patch through git format-patch and git send-email?
>
>> Yes, I use the git format-patch and git send-email
>
>>> Many e-mail softwares have weird issues when posting git patches,
>>> which is why git has its own tools for sending patches via e-mail.
>
>> ok
>
> Since you're using git format-patch and git send-email, then your 
> original change is not correctly aligned. I suggest you check your 
> code editor's settings on indentation and use of tabulations, notably 
> the "tab size": tabs should align on 8-space multiples; also check 
> that your editor uses a fixed font -- you never know.
Hi Amicalement

I check my patch 6 on the 
http://news.gmane.org/gmane.comp.boot-loaders.u-boot, It look fine.
I have a bit despondent. Why do you think it has a format problem.

Thanks
seedshope
>
> Amicalement,
seedshope - Jan. 22, 2011, 7:29 p.m.
On 01/22/2011 01:52 AM, Sergei Shtylyov wrote:
> Hello.
>
> seedshope wrote:
>
>> Since SDRAM init function have already change, So the SDRAM
>> initial function must be change.
>
> This description sounds somewhat tautological...
>
>> Signed-off-by: seedshope <bocui107@gmail.com>
>
> Your real name is required in the signoff.
>
>> ---
>> board/samsung/smdk6400/smdk6400.c | 10 +++++++++-
>> 1 files changed, 9 insertions(+), 1 deletions(-)
>
>> diff --git a/board/samsung/smdk6400/smdk6400.c 
>> b/board/samsung/smdk6400/smdk6400.c
>> index 35aa40b..1d03b7a 100644
>> --- a/board/samsung/smdk6400/smdk6400.c
>> +++ b/board/samsung/smdk6400/smdk6400.c
>> @@ -78,10 +78,18 @@ int board_init(void)
>> return 0;
>> }
>>
>> -int dram_init(void)
>> +void dram_init_banksize(void)
>> {
>> + DECLARE_GLOBAL_DATA_PTR;
>> +
>> gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
>> gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
>> +}
>> +
>> +int dram_init(void)
>> +{
>> + gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
>> + PHYS_SDRAM_1_SIZE);
You can look at the web 
site(http://news.gmane.org/gmane.comp.boot-loaders.u-boot) for the patch,
It is inconsistent with your description.

Thanks
seedshope
>
> Could you move this last line more to the right?
>
> WBR, Sergei
Albert ARIBAUD - Jan. 22, 2011, 8:28 p.m.
Hi seedshope,

Le 22/01/2011 20:23, seedshope a écrit :

> Hi Amicalement

That's Albert, actually. :)

> I check my patch 6 on the
> http://news.gmane.org/gmane.comp.boot-loaders.u-boot, It look fine.
> I have a bit despondent. Why do you think it has a format problem.

V5 of your patch has one more tab on as V4 had on the line we're 
discussion. It is a bit better ; Sergei will tell if that's enough for him.

> Thanks
> seedshope

Amicalement,
seedshope - Jan. 22, 2011, 8:40 p.m.
On 01/23/2011 04:28 AM, Albert ARIBAUD wrote:
> Hi seedshope,
>
> Le 22/01/2011 20:23, seedshope a écrit :
>
>> Hi Amicalement
>
> That's Albert, actually. :)
>
>> I check my patch 6 on the
>> http://news.gmane.org/gmane.comp.boot-loaders.u-boot, It look fine.
>> I have a bit despondent. Why do you think it has a format problem.
>
> V5 of your patch has one more tab on as V4 had on the line we're 
> discussion. It is a bit better ; Sergei will tell if that's enough for 
> him.
yes,  I just found the error in web site. I miss something in my 
thunderbird. such as tab convert space, So the format is change.
Here, I beg you to forgot my miss.

BR
seedshope
>> Thanks
>> seedshope
>
> Amicalement,
Wolfgang Denk - Jan. 22, 2011, 9:30 p.m.
Dear seedshope,

In message <4D3B40AC.8090105@gmail.com> you wrote:
>
> yes,  I just found the error in web site. I miss something in my
> thunderbird. such as tab convert space, So the format is change.
> Here, I beg you to forgot my miss.

It is usually helpful to search for and read the available documentation.

See file Documentation/email-clients.txt in your Linux source tree, or:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt

Best regards,

Wolfgang Denk

Patch

diff --git a/board/samsung/smdk6400/smdk6400.c b/board/samsung/smdk6400/smdk6400.c
index 35aa40b..1d03b7a 100644
--- a/board/samsung/smdk6400/smdk6400.c
+++ b/board/samsung/smdk6400/smdk6400.c
@@ -78,10 +78,18 @@  int board_init(void)
 	return 0;
 }
 
-int dram_init(void)
+void dram_init_banksize(void)
 {
+	DECLARE_GLOBAL_DATA_PTR;
+
 	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
 	gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
+}
+
+int dram_init(void)
+{
+	gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
+			PHYS_SDRAM_1_SIZE);
 
 	return 0;
 }