[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 at 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