[U-Boot] Fwd: [PATCH] arm/lib: Add get_effective_memsize() to board.c

[U-Boot] Fwd: [PATCH] arm/lib: Add get_effective_memsize() to board.c

[York Sun]

-------- Original Message --------
Subject: [PATCH] arm/lib: Add get_effective_memsize() to board.c
Date: Mon, 10 Feb 2014 14:04:19 -0800
From: York Sun <yorksun@freescale.com>
To: <albert.u.boot@aribaud.net>
CC: <scottwood@freescale.com>, York Sun <yorksun@freescale.com>

This function has been around for powerpc. It is used for systems with
memory more than CONFIG_MAX_MEM_MAPPED. In case of non-contiguous memory,
this feature can limit U-boot to one block without going over the limit.

Signed-off-by: York Sun <yorksun@freescale.com>
---
 arch/arm/lib/board.c |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index b770e25..b31c8ff 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -263,6 +263,24 @@ init_fnc_t *init_sequence[] = {
 	NULL,
 };

+phys_size_t get_effective_memsize(void)
+{
+#ifndef	CONFIG_VERY_BIG_RAM
+	return gd->ram_size;
+#else
+	/* limit stack to what we can reasonable map */
+	return ((gd->ram_size > CONFIG_MAX_MEM_MAPPED) ?
+		CONFIG_MAX_MEM_MAPPED : gd->ram_size);
+#endif
+}
+
+#ifdef CONFIG_LOGBUFFER
+unsigned long logbuffer_base(void)
+{
+	return CONFIG_SYS_SDRAM_BASE + get_effective_memsize() - LOGBUFF_LEN;
+}
+#endif
+
 void board_init_f(ulong bootflag)
 {
 	bd_t *bd;
@@ -322,7 +340,7 @@ void board_init_f(ulong bootflag)
 	gd->ram_size -= CONFIG_SYS_MEM_TOP_HIDE;
 #endif

-	addr = CONFIG_SYS_SDRAM_BASE + gd->ram_size;
+	addr = CONFIG_SYS_SDRAM_BASE + get_effective_memsize();

 #ifdef CONFIG_LOGBUFFER
 #ifndef CONFIG_ALT_LB_ADDR
-- 
1.7.9.5

[U-Boot] [PATCH] arm/lib: Add get_effective_memsize() to board.c

[York Sun]

On 02/10/2014 02:19 PM, Scott Wood wrote:
> On Mon, 2014-02-10 at 14:04 -0800, York Sun wrote:
>> This function has been around for powerpc. It is used for systems with
>> memory more than CONFIG_MAX_MEM_MAPPED. In case of non-contiguous memory,
>> this feature can limit U-boot to one block without going over the limit.
>>
>> Signed-off-by: York Sun <yorksun@freescale.com>
>> ---
>>  arch/arm/lib/board.c |   20 +++++++++++++++++++-
>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>> index b770e25..b31c8ff 100644
>> --- a/arch/arm/lib/board.c
>> +++ b/arch/arm/lib/board.c
>> @@ -263,6 +263,24 @@ init_fnc_t *init_sequence[] = {
>>  	NULL,
>>  };
>>  
>> +phys_size_t get_effective_memsize(void)
>> +{
>> +#ifndef	CONFIG_VERY_BIG_RAM
> 
> Whitespace

I think I have put a tab instead of a space.

> 
>> +	return gd->ram_size;
>> +#else
>> +	/* limit stack to what we can reasonable map */
>> +	return ((gd->ram_size > CONFIG_MAX_MEM_MAPPED) ?
>> +		CONFIG_MAX_MEM_MAPPED : gd->ram_size);
>> +#endif
>> +}
> 
> Is there anything really arch-specific about this function?
> 

Not at all. It is not called by any other file either. I tried to move the
function phys_size_t get_effective_memsize(void) out, but I don't see which
common file fits this purpose.

York

[U-Boot] [PATCH] arm/lib: Add get_effective_memsize() to board.c

[Scott Wood]

On Mon, 2014-02-10 at 14:33 -0800, York Sun wrote:
> On 02/10/2014 02:19 PM, Scott Wood wrote:
> > On Mon, 2014-02-10 at 14:04 -0800, York Sun wrote:
> >> This function has been around for powerpc. It is used for systems with
> >> memory more than CONFIG_MAX_MEM_MAPPED. In case of non-contiguous memory,
> >> this feature can limit U-boot to one block without going over the limit.
> >>
> >> Signed-off-by: York Sun <yorksun@freescale.com>
> >> ---
> >>  arch/arm/lib/board.c |   20 +++++++++++++++++++-
> >>  1 file changed, 19 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> >> index b770e25..b31c8ff 100644
> >> --- a/arch/arm/lib/board.c
> >> +++ b/arch/arm/lib/board.c
> >> @@ -263,6 +263,24 @@ init_fnc_t *init_sequence[] = {
> >>  	NULL,
> >>  };
> >>  
> >> +phys_size_t get_effective_memsize(void)
> >> +{
> >> +#ifndef	CONFIG_VERY_BIG_RAM
> > 
> > Whitespace
> 
> I think I have put a tab instead of a space.

Yes.

> >> +	return gd->ram_size;
> >> +#else
> >> +	/* limit stack to what we can reasonable map */
> >> +	return ((gd->ram_size > CONFIG_MAX_MEM_MAPPED) ?
> >> +		CONFIG_MAX_MEM_MAPPED : gd->ram_size);
> >> +#endif
> >> +}
> > 
> > Is there anything really arch-specific about this function?
> > 
> 
> Not at all. It is not called by any other file either. I tried to move the
> function phys_size_t get_effective_memsize(void) out, but I don't see which
> common file fits this purpose.

Not having an obvious place to put it isn't a good reason to keep it
duplicated in arch code.  Moving it to generic code would let you also
avoid overriding things like logbuffer_base().

-Scott

[U-Boot] [PATCH] arm/lib: Add get_effective_memsize() to board.c

[York Sun]

On 02/10/2014 02:37 PM, Scott Wood wrote:
> On Mon, 2014-02-10 at 14:33 -0800, York Sun wrote:
>> On 02/10/2014 02:19 PM, Scott Wood wrote:
>>> On Mon, 2014-02-10 at 14:04 -0800, York Sun wrote:
>>>> This function has been around for powerpc. It is used for systems with
>>>> memory more than CONFIG_MAX_MEM_MAPPED. In case of non-contiguous memory,
>>>> this feature can limit U-boot to one block without going over the limit.
>>>>
>>>> Signed-off-by: York Sun <yorksun@freescale.com>
>>>> ---
>>>>  arch/arm/lib/board.c |   20 +++++++++++++++++++-
>>>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>> index b770e25..b31c8ff 100644
>>>> --- a/arch/arm/lib/board.c
>>>> +++ b/arch/arm/lib/board.c
>>>> @@ -263,6 +263,24 @@ init_fnc_t *init_sequence[] = {
>>>>  	NULL,
>>>>  };
>>>>  
>>>> +phys_size_t get_effective_memsize(void)
>>>> +{
>>>> +#ifndef	CONFIG_VERY_BIG_RAM
>>>
>>> Whitespace
>>
>> I think I have put a tab instead of a space.
> 
> Yes.
> 
>>>> +	return gd->ram_size;
>>>> +#else
>>>> +	/* limit stack to what we can reasonable map */
>>>> +	return ((gd->ram_size > CONFIG_MAX_MEM_MAPPED) ?
>>>> +		CONFIG_MAX_MEM_MAPPED : gd->ram_size);
>>>> +#endif
>>>> +}
>>>
>>> Is there anything really arch-specific about this function?
>>>
>>
>> Not at all. It is not called by any other file either. I tried to move the
>> function phys_size_t get_effective_memsize(void) out, but I don't see which
>> common file fits this purpose.
> 
> Not having an obvious place to put it isn't a good reason to keep it
> duplicated in arch code.  Moving it to generic code would let you also
> avoid overriding things like logbuffer_base().
> 

I see the benefit of putting the code to common file and modify the
common/cmd_log.c for logbuffer_base(). Can you suggest a location for
get_effective_memsize(void)? Only powerpc uses this function before this patch.
Putting into common.h seems too generic.

York

[U-Boot] [PATCH] arm/lib: Add get_effective_memsize() to board.c

[Scott Wood]

On Mon, 2014-02-10 at 14:55 -0800, York Sun wrote:
> On 02/10/2014 02:37 PM, Scott Wood wrote:
> > On Mon, 2014-02-10 at 14:33 -0800, York Sun wrote:
> >> On 02/10/2014 02:19 PM, Scott Wood wrote:
> >>> On Mon, 2014-02-10 at 14:04 -0800, York Sun wrote:
> >>>> This function has been around for powerpc. It is used for systems with
> >>>> memory more than CONFIG_MAX_MEM_MAPPED. In case of non-contiguous memory,
> >>>> this feature can limit U-boot to one block without going over the limit.
> >>>>
> >>>> Signed-off-by: York Sun <yorksun@freescale.com>
> >>>> ---
> >>>>  arch/arm/lib/board.c |   20 +++++++++++++++++++-
> >>>>  1 file changed, 19 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> >>>> index b770e25..b31c8ff 100644
> >>>> --- a/arch/arm/lib/board.c
> >>>> +++ b/arch/arm/lib/board.c
> >>>> @@ -263,6 +263,24 @@ init_fnc_t *init_sequence[] = {
> >>>>  	NULL,
> >>>>  };
> >>>>  
> >>>> +phys_size_t get_effective_memsize(void)
> >>>> +{
> >>>> +#ifndef	CONFIG_VERY_BIG_RAM
> >>>
> >>> Whitespace
> >>
> >> I think I have put a tab instead of a space.
> > 
> > Yes.
> > 
> >>>> +	return gd->ram_size;
> >>>> +#else
> >>>> +	/* limit stack to what we can reasonable map */
> >>>> +	return ((gd->ram_size > CONFIG_MAX_MEM_MAPPED) ?
> >>>> +		CONFIG_MAX_MEM_MAPPED : gd->ram_size);
> >>>> +#endif
> >>>> +}
> >>>
> >>> Is there anything really arch-specific about this function?
> >>>
> >>
> >> Not at all. It is not called by any other file either. I tried to move the
> >> function phys_size_t get_effective_memsize(void) out, but I don't see which
> >> common file fits this purpose.
> > 
> > Not having an obvious place to put it isn't a good reason to keep it
> > duplicated in arch code.  Moving it to generic code would let you also
> > avoid overriding things like logbuffer_base().
> > 
> 
> I see the benefit of putting the code to common file and modify the
> common/cmd_log.c for logbuffer_base(). Can you suggest a location for
> get_effective_memsize(void)? Only powerpc uses this function before this patch.
> Putting into common.h seems too generic.

What's wrong with common.h?  That's where get_ram_size() is.

-Scott

[U-Boot] [PATCH] arm/lib: Add get_effective_memsize() to board.c

[York Sun]

On 02/10/2014 03:00 PM, Scott Wood wrote:
> On Mon, 2014-02-10 at 14:55 -0800, York Sun wrote:
>> On 02/10/2014 02:37 PM, Scott Wood wrote:
>>> On Mon, 2014-02-10 at 14:33 -0800, York Sun wrote:
>>>> On 02/10/2014 02:19 PM, Scott Wood wrote:
>>>>> On Mon, 2014-02-10 at 14:04 -0800, York Sun wrote:
>>>>>> This function has been around for powerpc. It is used for systems with
>>>>>> memory more than CONFIG_MAX_MEM_MAPPED. In case of non-contiguous memory,
>>>>>> this feature can limit U-boot to one block without going over the limit.
>>>>>>
>>>>>> Signed-off-by: York Sun <yorksun@freescale.com>
>>>>>> ---
>>>>>>  arch/arm/lib/board.c |   20 +++++++++++++++++++-
>>>>>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
>>>>>> index b770e25..b31c8ff 100644
>>>>>> --- a/arch/arm/lib/board.c
>>>>>> +++ b/arch/arm/lib/board.c
>>>>>> @@ -263,6 +263,24 @@ init_fnc_t *init_sequence[] = {
>>>>>>  	NULL,
>>>>>>  };
>>>>>>  
>>>>>> +phys_size_t get_effective_memsize(void)
>>>>>> +{
>>>>>> +#ifndef	CONFIG_VERY_BIG_RAM
>>>>>
>>>>> Whitespace
>>>>
>>>> I think I have put a tab instead of a space.
>>>
>>> Yes.
>>>
>>>>>> +	return gd->ram_size;
>>>>>> +#else
>>>>>> +	/* limit stack to what we can reasonable map */
>>>>>> +	return ((gd->ram_size > CONFIG_MAX_MEM_MAPPED) ?
>>>>>> +		CONFIG_MAX_MEM_MAPPED : gd->ram_size);
>>>>>> +#endif
>>>>>> +}
>>>>>
>>>>> Is there anything really arch-specific about this function?
>>>>>
>>>>
>>>> Not at all. It is not called by any other file either. I tried to move the
>>>> function phys_size_t get_effective_memsize(void) out, but I don't see which
>>>> common file fits this purpose.
>>>
>>> Not having an obvious place to put it isn't a good reason to keep it
>>> duplicated in arch code.  Moving it to generic code would let you also
>>> avoid overriding things like logbuffer_base().
>>>
>>
>> I see the benefit of putting the code to common file and modify the
>> common/cmd_log.c for logbuffer_base(). Can you suggest a location for
>> get_effective_memsize(void)? Only powerpc uses this function before this patch.
>> Putting into common.h seems too generic.
> 
> What's wrong with common.h?  That's where get_ram_size() is.
> 

You are right. I will put the function in common/memsize.c and put the prototype
in common.h.

York

[U-Boot] [PATCH] arm/lib: Add get_effective_memsize() to board.c

[Wolfgang Denk]

Dear York Sun,

In message <52F958D0.4050004@freescale.com> you wrote:
>
> I see the benefit of putting the code to common file and modify the
> common/cmd_log.c for logbuffer_base(). Can you suggest a location for
> get_effective_memsize(void)? Only powerpc uses this function before this patch.
> Putting into common.h seems too generic.

common/memsize.c ?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Software suppliers are trying to make their  software  packages  more
``user-friendly''.  .  .  .  Their best approach, so far, has been to
take all the old brochures, and stamp the words, ``user-friendly'' on
the cover.                                               - Bill Gates

[U-Boot] [PATCH] arm/lib: Add get_effective_memsize() to board.c

[York Sun]

On 02/11/2014 12:21 AM, Wolfgang Denk wrote:
> Dear York Sun,
> 
> In message <52F958D0.4050004@freescale.com> you wrote:
>>
>> I see the benefit of putting the code to common file and modify the
>> common/cmd_log.c for logbuffer_base(). Can you suggest a location for
>> get_effective_memsize(void)? Only powerpc uses this function before this patch.
>> Putting into common.h seems too generic.
> 
> common/memsize.c ?
> 

Yes. Thanks. I am working on this file.

York