Thanks for the feedback. Comments and an updated (shorter!) patch are below. On 30 March 2012 07:15, Peter Maydell <peter.maydell at linaro.org> wrote: > On 27 March 2012 01:39, Michael Hope <michael.hope at linaro.org> wrote: >> (I wasn't sure where to send this or if just to push. ?Thoughts?) > > I've been using android-virt as a vaguely plausible place to > send boot-wrapper patches, on the basis that that gives an > opportunity for public review and the volume isn't so high that > it annoys anybody [I hope!]. (cc'd) > >> Add and modify Makefile rules so that it can be used in a Debian >> package. ?Add an explicit 'distclean' target. ?Add a 'semi' target >> that only builds the semihosting binary. ?Add stub commands to 'all' >> and 'semi' so the call doesn't fall through to the default kernel >> rule. ?Update 'clean' to also clean the semihosting files. >> >> Signed-off-by: Michael Hope <michael.hope at linaro.org> >> --- >> ?Makefile | ? 13 +++++++++++-- >> ?1 files changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index ebcbce5..584b404 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -28,10 +28,18 @@ LD ? ? ? ? ?= $(CROSS_COMPILE)ld >> ?# These are needed by the underlying kernel make >> ?export CROSS_COMPILE ARCH >> >> +# Build all wrappers >> ?all: $(IMAGE) $(SEMIIMG) >> + ? ? ? true > > This shouldn't be necessary. Yip. Deleted. >> -clean: >> - ? ? ? rm -f $(IMAGE) boot.o model.lds monitor.o uImage >> +# Build just the semihosting wrapper >> +semi: $(SEMIIMG) >> + ? ? ? true > > ...and nor should this 'true' -- if you add 'semi' to the > .PHONY target list at the bottom of the makefile it ought to > suppress the default goes-through-to-kernel-makefile rule. Yip. Deleted and added to .PHONY. >> +clean distclean: >> + ? ? ? rm -f $(IMAGE) $(SEMIIMG) \ >> + ? ? ? boot.o model.lds monitor.o uImage \ >> + ? ? ? bootsemi.o monitor.o modelsemi.lds > > You have monitor.o twice in this list. Yip, as these are a copy and paste of the $(IMAGE) dependancies on one line and the $(SEMIIMG) ones on the next. Makes it easier to check and keep up to date. IMAGE_OBJ would be better... It's not a big enough gain. Deleted the second monitor.o. > 'distclean' should be in the .PHONY list too. Added. >> ?$(KERNEL): $(KERNEL_SRC)/arch/arm/boot/uImage >> ? ? ? ?cp $< $@ >> @@ -62,6 +70,7 @@ $(KERNEL_SRC)/arch/arm/boot/uImage: force >> >> ?# Pass any target we don't know about through to the kernel makefile. >> ?# This is a convenience rule so we can say 'make menuconfig' etc here. >> +# Note that any rules in this file must have a command. > > ...or be listed as .PHONY. Updated. -- Michael Add and modify Makefile rules so that it can be used in a Debian package. Add an explicit 'distclean' target. Add a 'semi' target that only builds the semihosting binary. Add the commands to .PHONY so the call doesn't pass through to the default kernel rule. Update the clean rule to also clean the semihosting files. Signed-off-by: Michael Hope <michael.hope at linaro.org> --- Makefile | 14 +++++++++++--- 1 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index ebcbce5..310b2dd 100644 --- a/Makefile +++ b/Makefile @@ -28,10 +28,16 @@ LD = $(CROSS_COMPILE)ld # These are needed by the underlying kernel make export CROSS_COMPILE ARCH +# Build all wrappers all: $(IMAGE) $(SEMIIMG) -clean: - rm -f $(IMAGE) boot.o model.lds monitor.o uImage +# Build just the semihosting wrapper +semi: $(SEMIIMG) + +clean distclean: + rm -f $(IMAGE) $(SEMIIMG) \ + boot.o model.lds monitor.o $(KERNEL) \ + bootsemi.o modelsemi.lds $(KERNEL): $(KERNEL_SRC)/arch/arm/boot/uImage cp $< $@ @@ -62,6 +68,8 @@ $(KERNEL_SRC)/arch/arm/boot/uImage: force # Pass any target we don't know about through to the kernel makefile. # This is a convenience rule so we can say 'make menuconfig' etc here. +# Note that any rules in this file must have a command or be marked as +# .PHONY. %: force $(MAKE) -C $(KERNEL_SRC) $@ @@ -69,4 +77,4 @@ force: ; Makefile: ; -.PHONY: all clean config.mk config-default.mk +.PHONY: all semi clean distclean config.mk config-default.mk -- 1.7.9.1