[U-Boot] [PATCH v2 1/6] reboard: Create reloc.h and include it where needed

[Graeme Russ]

Hi Simon,

On Mon, Dec 12, 2011 at 8:33 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Albert,
>
> On Sun, Dec 11, 2011 at 6:52 AM, Albert ARIBAUD
> <albert.u.boot at aribaud.net> wrote:
>> Le 10/12/2011 20:16, Simon Glass a écrit :
>>
>>> Before adding new relocation functions, move this prototype out of
>>> common.h where things are pretty crowded.
>>>
>>> Signed-off-by: Simon Glass<sjg at chromium.org>
>>> ---
>>>
>>>  arch/arm/cpu/arm926ejs/davinci/spl.c              |    1 +
>>>  arch/arm/cpu/armv7/omap-common/spl.c              |    1 +
>>>  arch/arm/lib/board.c                              |    1 +
>>>  arch/avr32/lib/board.c                            |    1 +
>>>  arch/m68k/lib/board.c                             |    1 +
>>>  arch/mips/lib/board.c                             |    1 +
>>>  arch/nds32/lib/board.c                            |    1 +
>>>  arch/powerpc/lib/board.c                          |    1 +
>>>  arch/x86/lib/board.c                              |    1 +
>>>  board/freescale/mpc8313erdb/mpc8313erdb.c         |    1 +
>>>  board/freescale/mpc8315erdb/mpc8315erdb.c         |    1 +
>>>  board/samsung/smdk6400/smdk6400_nand_spl.c        |    1 +
>>>  board/sheldon/simpc8313/simpc8313.c               |    1 +
>>>  include/common.h                                  |    2 +-
>>>  include/reloc.h                                   |   39
>>> +++++++++++++++++++++
>>>  nand_spl/board/freescale/mpc8536ds/nand_boot.c    |    1 +
>>>  nand_spl/board/freescale/mpc8569mds/nand_boot.c   |    1 +
>>>  nand_spl/board/freescale/mpc8572ds/nand_boot.c    |    1 +
>>>  nand_spl/board/freescale/p1010rdb/nand_boot.c     |    1 +
>>>  nand_spl/board/freescale/p1023rds/nand_boot.c     |    1 +
>>>  nand_spl/board/freescale/p1_p2_rdb/nand_boot.c    |    1 +
>>>  nand_spl/board/freescale/p1_p2_rdb_pc/nand_boot.c |    1 +
>>>  nand_spl/nand_boot_fsl_nfc.c                      |    1 +
>>>  23 files changed, 61 insertions(+), 1 deletions(-)
>>>  create mode 100644 include/reloc.h
>>
>>
>> Since this patch set indicates that ARM, and only ARM, is moved over to
>> relocation, I would prefer it that no other arch file be modified in this
>> patch, and that the move of ARM to the new relocation mechanism be done in
>> an atomic commit, so that other architectures can refer to a single commit
>> in order to do their own move.
>
> What specifically are you asking for in this patch? I added this at
> the request of a reviewer of v1, who felt that we should be removing
> code from common.h instead of adding it, and that relocation is done
> in only a few sites so should not be in common.h. Do you think this
> patch should be pulled out of the series and done on its own?

I think the point is that when we pull stuff out of common.h into,
say, foo.h then only include foo.h where the functionality moved into
foo.h is _currently_ used.

It looks like you've pulled out the functions definitions into reloc.h
then included reloc.h everywhere (including common.h). Better would be
to moved the definitions to reloc.h, do not include reloc.h in
common.h and include reloc.h only where it is strictly needed right
now - Add #include <reloc.h> to each arch file as and when it is
needed

Regards,

Graeme