Hi Graeme,
On Tue, Oct 9, 2012 at 3:58 PM, Graeme Russ graeme.russ@gmail.com wrote:
Hi Simon,
On Wed, Oct 10, 2012 at 8:15 AM, Simon Glass sjg@chromium.org wrote:
diff --git a/arch/x86/cpu/resetvec.S b/arch/x86/cpu/resetvec.S index 44aee5f..5b359ff 100644 --- a/arch/x86/cpu/resetvec.S +++ b/arch/x86/cpu/resetvec.S @@ -25,6 +25,10 @@
/* Reset vector, jumps to start16.S */
+#include <config.h>
+#ifndef CONFIG_NO_RESET_CODE
.extern start16
.section .resetvec, "ax" @@ -36,3 +40,5 @@ reset_vector:
.org 0xf nop
+#endif
Condition it out in the Makefile instead
I suspect the reason it was done here is these lines in the top-level Makefile.
OBJS = $(CPUDIR)/start.o ifeq ($(CPU),x86) OBJS += $(CPUDIR)/start16.o OBJS += $(CPUDIR)/resetvec.o endif
I have often wondered about these lines in the top-level Makefile considering they are also in arch/x86/cpu/Makefile. I keep meaning to test if they are actually needed in the top-level Makefile but keep forgetting
(I see why now - see below)
If we just take it out of the .lds file then start16.o and resetvec.o are not included in the image. But they will still be built. We could add an additional condition here perhaps, like:
I don't see a huge problem with that. Yes, it's a waste of CPU cycles during the build but really, who cares.
OBJS = $(CPUDIR)/start.o ifeq ($(CPU),x86) ifneq ($(CONFIG_NO_RESET_CODE),y) OBJS += $(CPUDIR)/start16.o OBJS += $(CPUDIR)/resetvec.o endif endif
Looks good for the time being (again, see beloW).
Here is the menu as I see it - what would you prefer?
- top level Makefile change
- arch/arm/cpu/Makefile change (pointless if top level Makefile
includes these files anyway)
- building everything but removing unneeded object files in the link script
Can we not invert the logic of CONFIG_X86_NO_RESET_VECTOR using some Makefile magic and then do this in arch/x86/cpu/Makefile:
START-$(INCLUDE_X86_RESET_VECTOR) += resetvec.o START-y = start.o START-$(INCLUDE_X86_RESET_VECTOR) += start16.o
Actuall, to be honest, it should be:
SOBJS-y += start.o
SOBJS16-$(INCLUDE_X86_RESET_VECTOR) += resetvec.o SOBJS16-$(INCLUDE_X86_RESET_VECTOR) += start16.o
SRCS := $(SOBJS16-y:.o=.S) $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c) OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS)) OBJS16 := $(addprefix $(obj),$(SOBJS16))
all: $(obj).depend $(OBJS16) $(LIB)
start.S is not at all related to the reset vector / protected mode switch, and so can safely be moved into the main (32-bit) lib. ENTRY(_start) in the linker script and:
.section .text .code32 .globl _start .type _start, @function .globl _start .type _start, @function
in start.S will always guarantee that the code in start.S appears first in u-boot.bin.
Ah Ha! now I get it - Now I see why the top-level Makefile includes:
OBJS = $(CPUDIR)/start.o ifeq ($(CPU),x86) OBJS += $(CPUDIR)/start16.o OBJS += $(CPUDIR)/resetvec.o endif
These files are not in $(CPUDIR)/lib$(CPU).o so they must be pulled in individually!
OK, by moving start.o into the lib we can drop the first line...
Now, if we create a 16-bit lib in arch/x86/cpu/Makefile:
LIB = $(obj)lib$(CPU).o LIB16 = $(obj)lib16$(CPU).o
SOBJS16-$(INCLUDE_X86_RESET_VECTOR) += resetvec.o SOBJS16-$(INCLUDE_X86_RESET_VECTOR) += start16.o
SOBJS-y += start.o COBJS-y += cpu.o COBJS-y += interrupts.o
SRCS := $(SOBJS16-y:.o=.S) $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c) OBJS16 := $(addprefix $(obj),$(SOBJS16)) OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS))
all: $(obj).depend $(LIB) $(LIB16)
$(LIB): $(OBJS) $(call cmd_link_o_target, $(OBJS))
$(LIB16): $(OBJS16) $(call cmd_link_o_target, $(OBJS16))
And then in the top-level Makefile:
LIBS-$(INCLUDE_X86_RESET_VECTOR) += $(CPUDIR)/lib16$(CPU).o
Much cleaner :)
(Hmmm, looking at arch/x86/lib/Makefile is appears that it is safe to mix 16- and 32-bit code in the same lib - maybe that is a better solution...)
But don't worry too much about all that in these patches - Make the changes as you have already suggested and I will tweak the rest later
Looks good - yes I will prepare a new series to send likely on Thursday.
Not that it matters for now, but who will define INCLUDE_X86_RESET_VECTOR?
Regards, Simon
Regards,
Graeme