Hi Simon,
Le 26/12/2011 19:24, Simon Glass a écrit :
(I am resending this rebased so I can continue with this board-unification
work and allow people to review patches. There were some comments on the
v2 series but my questions have been sitting on the list for 2 weeks so it
is probably time for a new series.)
This is the second patch series aiming to unify the various board.c files
in each architecture into a single one. This series implements a generic
relocation feature, which is the bridge between board_init_f() and
board_init_r(). It then moves ARM over to use this framework, as an
example.
On ARM the relocation code is duplicated for each CPU yet it
is the same. We can bring this up to the arch level. But since (I believe)
Elf relocation is basically the same process for all archs, there is no
reason not to bring it up to the generic level.
Each architecture which uses this framework needs to provide a function
called arch_elf_relocate_entry() which processes a single relocation
entry. This is a static inline function to reduce code size overhead.
Agreed.
For ARM, a new arch/arm/lib/proc.S file is created, which holds generic
ARM assembler code (things that cannot be written in C and are common
functions used by all ARM CPUs). This helps reduce duplication. Interrupt
handling code and perhaps even some startup code can move there later.
It may be useful for other architectures with a lot of different CPUs
to have a similar file.
NAK for several reasons:
1. The code that goes on proc.S is already in start.S, which already
contains "things which cannot be written in C and are common functions
used by all ARM CPUs". Granted, right now start.S is duplicated in
multiple CPU directories; so the thing to do is to merge all ARM start.S
files into a single one, rather than merging only one of its parts.
2. There is no interest in moving this segment of code from all start.S
files into a new proc.S file: there is no gain is code size obviously,
and there is an increase in source file count.
3. I consider that files should contain 'things' based on their common
/functionality/, not on their similar /nature/, and "going about
starting up U-Boot" is a functionality.
Note that eventually, having a single start.S will achieve the same
effect as this 'proc.S' patch.
Note also that this start.S commonalization should be a completely
different patch set.
Code size on my ARMv7 system increases by 54 bytes with generic
relocation. This overhead is mostly just literal pool access and setting
up to call the relocated U-Boot at the end.
On my system, execution time increases from 10.8ms to 15.6ms due to the
less efficient C implementations of the copy and zero loops. If execution
time is of concern, you can define CONFIG_USE_ARCH_MEMSET and
CONFIG_USE_ARCH_MEMCPY to reduce it. For met this reduces relocation time
to 5.4ms, i.e. twice as fast as the old system.
Side question: is there any reason not to define these CONFIG options
systematically?
One problem remains which causes mx31pdk to fail to build. It doesn't
have string.c in its SPL code and the architecture-specific versions of
memset()/memcpy() are too large. I propose to add a local change to
reloc.c that uses inline code for boards that use the old legacy SPL
framework. We can remove it later. This is not included in v2 but I am
interested in comments on this approach. An alternative would be just
to add simple memset()/memcpy() functions just for this board (and one
other affected MX31 board).
I am not too fond of the "later removal" approach. In my experience,
this invariably tends to the "old bloat in the code no one knows the
reason of" situations.
Changes in v2:
- Use CONFIG_SYS_SKIP_RELOC instead of CONFIG_SYS_LEGACY_BOARD
- Import asm-generic/sections.h from Linux and add U-Boot extras
- Squash generic link symbols patch into generic relocation patch
- Move reloc.c into common/
- Add function comments
- Use memset, memcpy instead of inline code
- Add README file for relocation
- Invalidate I-cache when we jump to relocated code
- Use an inline relocation function to reduce code size
- Make relocation symbols global so we can use them outside start.S
Changes in v3:
- Remove the 'reloc' tag from each commit
- Rebase to master
Simon Glass (6):
Create reloc.h and include it where needed
define CONFIG_SYS_SKIP_RELOC for all archs
Add generic relocation feature
arm: Add processor function library
arm: Move over to generic relocation
arm: Remove unused code in start.S
README | 4 +
arch/arm/cpu/arm1136/start.S | 133 ++------------
arch/arm/cpu/arm1176/start.S | 214 ++-------------------
arch/arm/cpu/arm720t/start.S | 127 ++-----------
arch/arm/cpu/arm920t/start.S | 135 ++------------
arch/arm/cpu/arm925t/start.S | 135 ++------------
arch/arm/cpu/arm926ejs/davinci/spl.c | 1 +
arch/arm/cpu/arm926ejs/start.S | 144 ++-------------
arch/arm/cpu/arm946es/start.S | 130 ++-----------
arch/arm/cpu/arm_intcm/start.S | 135 ++------------
arch/arm/cpu/armv7/omap-common/spl.c | 1 +
arch/arm/cpu/armv7/start.S | 140 ++------------
arch/arm/cpu/ixp/start.S | 127 ++-----------
arch/arm/cpu/lh7a40x/start.S | 124 ++-----------
arch/arm/cpu/pxa/start.S | 138 ++------------
arch/arm/cpu/s3c44b0/start.S | 127 ++-----------
arch/arm/cpu/sa1100/start.S | 124 ++-----------
arch/arm/include/asm/reloc.h | 56 ++++++
arch/arm/lib/Makefile | 2 +
arch/arm/lib/board.c | 1 +
arch/arm/lib/proc.S | 40 ++++
arch/avr32/config.mk | 3 +
arch/avr32/lib/board.c | 1 +
arch/blackfin/config.mk | 3 +
arch/m68k/config.mk | 3 +
arch/m68k/lib/board.c | 1 +
arch/microblaze/config.mk | 3 +
arch/mips/config.mk | 3 +
arch/mips/lib/board.c | 1 +
arch/nds32/config.mk | 3 +
arch/nds32/lib/board.c | 1 +
arch/nios2/config.mk | 3 +
arch/powerpc/config.mk | 3 +
arch/powerpc/lib/board.c | 1 +
arch/sandbox/config.mk | 3 +
arch/sh/config.mk | 3 +
arch/sparc/config.mk | 3 +
arch/x86/config.mk | 3 +
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 +
common/Makefile | 4 +
common/reloc.c | 121 ++++++++++++
doc/README.relocation | 87 +++++++++
include/asm-generic/sections.h | 92 +++++++++
include/common.h | 2 +-
include/reloc.h | 54 +++++
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/mx31pdk/Makefile | 8 +-
nand_spl/board/freescale/mx31pdk/u-boot.lds | 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/board/karo/tx25/Makefile | 8 +-
nand_spl/board/karo/tx25/u-boot.lds | 1 +
nand_spl/nand_boot_fsl_nfc.c | 1 +
61 files changed, 705 insertions(+), 1765 deletions(-)
create mode 100644 arch/arm/include/asm/reloc.h
create mode 100644 arch/arm/lib/proc.S
create mode 100644 common/reloc.c
create mode 100644 doc/README.relocation
create mode 100644 include/asm-generic/sections.h
create mode 100644 include/reloc.h
Amicalement,
--
Albert.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
