Hi Graeme, On Wed, Dec 7, 2011 at 2:54 PM, Graeme Russ <graeme.r...@gmail.com> wrote: > Hi Simon, > > On Thu, Dec 8, 2011 at 9:45 AM, Simon Glass <s...@chromium.org> wrote: >> Hi Mike, >> >> On Mon, Nov 28, 2011 at 7:07 PM, Mike Frysinger <vap...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot