[U-Boot] [RFC PATCH 3/7] reboard: Add generic relocation feature

[Simon Glass]

Hi Graeme,

On Wed, Dec 7, 2011 at 2:54 PM, Graeme Russ <graeme.russ at gmail.com> wrote:
> Hi Simon,
>
> On Thu, Dec 8, 2011 at 9:45 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Mike,
>>
>> On Mon, Nov 28, 2011 at 7:07 PM, Mike Frysinger <vapier at gentoo.org> wrote:
>>> On Monday 21 November 2011 18:57:56 Simon Glass wrote:
>>>>  board/Makefile   |   45
>>>>  board/reloc.c    |  101
>>>
>>> not to bikeshed, but i don't think we want files in board/.  how about
>>> board/common/ or board/generic/ instead ?
>>>
>>>> --- /dev/null
>>>> +++ b/board/Makefile
>>>>
>>>> +ifndef CONFIG_SYS_LEGACY_BOARD
>>>> +COBJS        += reloc.o
>>>> +endif
>>>
>>> i don't think relocation should be tied "legacy board".  not all arches do
>>> relocation at all, which means they might never opt in to this aspect.
>>>
>>>> --- a/include/common.h
>>>> +++ b/include/common.h
>>>>
>>>> -void relocate_code (ulong, gd_t *, ulong) __attribute__ ((noreturn));
>>>> +#include <reloc.h>
>>>
>>> i'd think very few funcs would need this.  so maybe we should make the few
>>> places include reloc.h explicitly.
>>> -mike
>>
>> I found that 27 files call relocate_code(). Now we can discuss whether
>> the number should be that high, but for now I would prefer to move
>> this out of common.h in a later patch. What do you think?
>
> 27 out of how many source code files? Thousands?
>
> Quite honestly, include/common.h has become a bit of a dumping ground
> for one-off function definitions that should have been put in a
> stand-alone header.
>
> common.h is included nearly everywhere - is all this really needed everywhere:
>
> /* Multicore arch functions */
> #ifdef CONFIG_MP
> int cpu_status(int nr);
> int cpu_reset(int nr);
> int cpu_disable(int nr);
> int cpu_release(int nr, int argc, char * const argv[]);
> #endif
>
> ...
>
> int     dpram_init (void);
> uint    dpram_base(void);
> uint    dpram_base_align(uint align);
> uint    dpram_alloc(uint size);
> uint    dpram_alloc_align(uint size,uint align);
>
> ...
>
> #if defined(CONFIG_8260)
> int     prt_8260_rsr  (void);
> #elif defined(CONFIG_MPC83xx)
> int     prt_83xx_rsr  (void);
> #endif
>
> ...
>
> /* $(CPU)/cpu_init.c */
> #if defined(CONFIG_8xx) || defined(CONFIG_8260)
> void    cpu_init_f    (volatile immap_t *immr);
> #endif
> #if defined(CONFIG_4xx) || defined(CONFIG_MPC85xx) ||
> defined(CONFIG_MCF52x2) ||defined(CONFIG_MPC86xx)
> void    cpu_init_f    (void);
> #endif
>
> ...
>
> #if defined(CONFIG_IMX)
> ulong get_systemPLLCLK(void);
> ulong get_FCLK(void);
> ulong get_HCLK(void);
> ulong get_BCLK(void);
> ulong get_PERCLK1(void);
> ulong get_PERCLK2(void);
> ulong get_PERCLK3(void);
> #endif
>
>
> The list goes on - please, lets stop the rot

You are preaching to the choir...

OK I will add an initial patch to the series to clean this up.

Regards,
Simon

>
> Regards,
>
> Graeme