Patchwork [U-Boot] tools: make it possible to build tools unconfigured

login
register
mail settings
Submitter Ilya Yanok
Date June 15, 2011, 9:37 p.m.
Message ID <1308173853-20178-1-git-send-email-yanok@emcraft.com>
Download mbox | patch
Permalink /patch/100574/
State Changes Requested
Headers show

Comments

Ilya Yanok - June 15, 2011, 9:37 p.m.
Sometimes we want to build common tools without configuring for specific
target. Currently top Makefile has some support for this but it doesn't
work. This patch tries to fix this.
Things changed:
 - config.mk   disable 'ld script not found error' in case if we are
building tools only.
 - Makefile    mkimage relies on autogenerated version so we need to
move $(VERSION_FILE) rule out of ifeq and make tools rule depend on it.
 - tools/Makefile put common/env_embedded.o and envcrc.o to object list
conditionally. This fixes errors during dependency generation.

Signed-off-by: Ilya Yanok <yanok@emcraft.com>
---
 Makefile       |   34 +++++++++++++++++-----------------
 config.mk      |    2 ++
 tools/Makefile |   19 ++++++++++++++++---
 3 files changed, 35 insertions(+), 20 deletions(-)
Mike Frysinger - June 18, 2011, 7:03 p.m.
On Wednesday, June 15, 2011 17:37:33 Ilya Yanok wrote:
> Sometimes we want to build common tools without configuring for specific
> target. Currently top Makefile has some support for this but it doesn't
> work. This patch tries to fix this.

i think you'll need to split this up in logical sep changesets since you are 
fixing different problems.

> Things changed:
>  - config.mk   disable 'ld script not found error' in case if we are
> building tools only.

i dont like copying & pasting the same logic in multiple places.  this is how 
code rots and bug fixes diverge.

considering LDSCRIPT/CONFIG_SYS_LDSCRIPT only get used by the top level u-boot 
file, and only when the system is configured, i wonder if we shouldnt just rip 
it out of config.mk and into the top level Makefile.

let's see what Scott thinks ...

>  - Makefile    mkimage relies on autogenerated version so we need to
> move $(VERSION_FILE) rule out of ifeq and make tools rule depend on it.

the VERSION_FILE changes look fine

>  - tools/Makefile put common/env_embedded.o and envcrc.o to object list
> conditionally. This fixes errors during dependency generation.

pretty sure this breaks board builds.  if the only thing this fixes is a 
harmless warning when generating dependency files, then i say ignore it.  
after all, this is how it has always worked in the past and no one really 
cared.
-mike
Wolfgang Denk - June 18, 2011, 8:11 p.m.
Dear Mike Frysinger,

In message <201106181503.10550.vapier@gentoo.org> you wrote:
>
> pretty sure this breaks board builds.  if the only thing this fixes is a 
> harmless warning when generating dependency files, then i say ignore it.  
> after all, this is how it has always worked in the past and no one really 
> cared.

There have been no warnings when generating dependencies.

Changes that cause new warnings need to be fixed.

Best regards,

Wolfgang Denk
Mike Frysinger - June 18, 2011, 8:21 p.m.
On Saturday, June 18, 2011 16:11:17 Wolfgang Denk wrote:
> Mike Frysinger wrote:
> > pretty sure this breaks board builds.  if the only thing this fixes is a
> > harmless warning when generating dependency files, then i say ignore it.
> > after all, this is how it has always worked in the past and no one really
> > cared.
> 
> There have been no warnings when generating dependencies.

yes, there has.  you probably dont run `make tools` (at least, in an 
unconfigured tree), so you've never noticed.

> Changes that cause new warnings need to be fixed.

you'll have to define "new" here.  the warning he refers to has been there for 
many many releases, and for pretty much as long as i can remember.

i dont mind fixing warnings, but i do mind breaking the building of boards to 
fix a harmless warning.
-mike
Ilya Yanok - June 19, 2011, 5:55 p.m.
Hi Mike,

thanks for your comments.

On 18.06.2011 23:03, Mike Frysinger wrote:
> i think you'll need to split this up in logical sep changesets since you are 
> fixing different problems.

Ok, will do.

>> Things changed:
>>  - config.mk   disable 'ld script not found error' in case if we are
>> building tools only.
> 
> i dont like copying & pasting the same logic in multiple places.  this is how 
> code rots and bug fixes diverge.

Agreed. How should I fix this?

> considering LDSCRIPT/CONFIG_SYS_LDSCRIPT only get used by the top level u-boot 
> file, and only when the system is configured, i wonder if we shouldnt just rip 
> it out of config.mk and into the top level Makefile.
> 
> let's see what Scott thinks ...

I see. Let's wait for Scott's comments then.

>>  - tools/Makefile put common/env_embedded.o and envcrc.o to object list
>> conditionally. This fixes errors during dependency generation.
> 
> pretty sure this breaks board builds.  if the only thing this fixes is a

I'm sorry but I can't see how this can break the builds. Could you
please be more specific? I've tried to build some boards, it actually
works...

> harmless warning when generating dependency files, then i say ignore it.  
> after all, this is how it has always worked in the past and no one really 
> cared.

Yep, they are harmless but they are not warnings but rather scary errors
actually. ;) I think it's better to fix them.

Regards, Ilya.
Scott Wood - June 20, 2011, 5:52 p.m.
On Sat, 18 Jun 2011 15:03:09 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> considering LDSCRIPT/CONFIG_SYS_LDSCRIPT only get used by the top level u-boot 
> file, and only when the system is configured, i wonder if we shouldnt just rip 
> it out of config.mk and into the top level Makefile.
> 
> let's see what Scott thinks ...

Sounds fine.

-Scott
Wolfgang Denk - Oct. 17, 2011, 7:42 p.m.
Dear Ilya,

In message <1308173853-20178-1-git-send-email-yanok@emcraft.com> you wrote:
> Sometimes we want to build common tools without configuring for specific
> target. Currently top Makefile has some support for this but it doesn't
> work. This patch tries to fix this.
> Things changed:
>  - config.mk   disable 'ld script not found error' in case if we are
> building tools only.
>  - Makefile    mkimage relies on autogenerated version so we need to
> move $(VERSION_FILE) rule out of ifeq and make tools rule depend on it.
>  - tools/Makefile put common/env_embedded.o and envcrc.o to object list
> conditionally. This fixes errors during dependency generation.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> ---
>  Makefile       |   34 +++++++++++++++++-----------------
>  config.mk      |    2 ++
>  tools/Makefile |   19 ++++++++++++++++---
>  3 files changed, 35 insertions(+), 20 deletions(-)

Is it correct to assume that this commit has been obsolteted by
the following commits:

249b53a   2011-10-06 20:19:42 +0200   Build timestamp_autogenerated.h without config
a76406f   2011-10-06 20:20:15 +0200   Safer timestamp_autogenerated.h generation
9f87658   2011-10-06 20:21:16 +0200   ublimage: NAND block size isn't set at build-time

?


Best regards,

Wolfgang Denk
Ilya Yanok - Oct. 17, 2011, 8:24 p.m.
Dear Wolfgang,

On 17.10.2011 23:42, Wolfgang Denk wrote:
>> Sometimes we want to build common tools without configuring for specific
>> target. Currently top Makefile has some support for this but it doesn't
>> work. This patch tries to fix this.
>> Things changed:
>>  - config.mk   disable 'ld script not found error' in case if we are
>> building tools only.
>>  - Makefile    mkimage relies on autogenerated version so we need to
>> move $(VERSION_FILE) rule out of ifeq and make tools rule depend on it.
>>  - tools/Makefile put common/env_embedded.o and envcrc.o to object list
>> conditionally. This fixes errors during dependency generation.
>>
>> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
>> ---
>>  Makefile       |   34 +++++++++++++++++-----------------
>>  config.mk      |    2 ++
>>  tools/Makefile |   19 ++++++++++++++++---
>>  3 files changed, 35 insertions(+), 20 deletions(-)
> 
> Is it correct to assume that this commit has been obsolteted by
> the following commits:
> 
> 249b53a   2011-10-06 20:19:42 +0200   Build timestamp_autogenerated.h without config
> a76406f   2011-10-06 20:20:15 +0200   Safer timestamp_autogenerated.h generation
> 9f87658   2011-10-06 20:21:16 +0200   ublimage: NAND block size isn't set at build-time

Not exactly. I've reposted my patch splitting it into three as Mike
suggested, and all these patches are already applied.

The commits you mention deal with the related problems that were
introduced later during the development.

Regards, Ilya.
Wolfgang Denk - Oct. 17, 2011, 8:35 p.m.
Dear Ilya Yanok,

In message <4E9C8EF9.3030506@emcraft.com> you wrote:
> 
> > Is it correct to assume that this commit has been obsolteted by
> > the following commits:
...
> Not exactly. I've reposted my patch splitting it into three as Mike
> suggested, and all these patches are already applied.
> 
> The commits you mention deal with the related problems that were
> introduced later during the development.

So it should still be applied?

Best regards,

Wolfgang Denk
Ilya Yanok - Oct. 17, 2011, 8:48 p.m.
Dear Wolfgang,

On 18.10.2011 00:35, Wolfgang Denk wrote:
>>> Is it correct to assume that this commit has been obsolteted by
>>> the following commits:
> ...
>> Not exactly. I've reposted my patch splitting it into three as Mike
>> suggested, and all these patches are already applied.
>>
>> The commits you mention deal with the related problems that were
>> introduced later during the development.
> 
> So it should still be applied?

No, you can just drop it. It's already applied long ago (well, its
second version).

Regards, Ilya.

Patch

diff --git a/Makefile b/Makefile
index dcf5d93..4f7326e 100644
--- a/Makefile
+++ b/Makefile
@@ -140,7 +140,7 @@  SUBDIRS	= tools \
 	  examples/standalone \
 	  examples/api
 
-.PHONY : $(SUBDIRS)
+.PHONY : $(SUBDIRS) $(VERSION_FILE)
 
 ifeq ($(obj)include/config.mk,$(wildcard $(obj)include/config.mk))
 
@@ -422,19 +422,6 @@  mmc_spl:	$(TIMESTAMP_FILE) $(VERSION_FILE) depend
 
 $(obj)mmc_spl/u-boot-mmc-spl.bin:	mmc_spl
 
-$(VERSION_FILE):
-		@( localvers='$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ; \
-		   printf '#define PLAIN_VERSION "%s%s"\n' \
-			"$(U_BOOT_VERSION)" "$${localvers}" ; \
-		   printf '#define U_BOOT_VERSION "U-Boot %s%s"\n' \
-			"$(U_BOOT_VERSION)" "$${localvers}" ; \
-		) > $@.tmp
-		@( printf '#define CC_VERSION_STRING "%s"\n' \
-		 '$(shell $(CC) --version | head -n 1)' )>>  $@.tmp
-		@( printf '#define LD_VERSION_STRING "%s"\n' \
-		 '$(shell $(LD) -v | head -n 1)' )>>  $@.tmp
-		@cmp -s $@ $@.tmp && rm -f $@.tmp || mv -f $@.tmp $@
-
 $(TIMESTAMP_FILE):
 		@LC_ALL=C date +'#define U_BOOT_DATE "%b %d %C%y"' > $@
 		@LC_ALL=C date +'#define U_BOOT_TIME "%T"' >> $@
@@ -509,20 +496,33 @@  $(obj)lib/asm-offsets.s:	$(obj)include/autoconf.mk.dep \
 else	# !config.mk
 all $(obj)u-boot.hex $(obj)u-boot.srec $(obj)u-boot.bin \
 $(obj)u-boot.img $(obj)u-boot.dis $(obj)u-boot \
-$(filter-out tools,$(SUBDIRS)) $(TIMESTAMP_FILE) $(VERSION_FILE) \
+$(filter-out tools,$(SUBDIRS)) $(TIMESTAMP_FILE) \
 updater depend dep tags ctags etags cscope $(obj)System.map:
 	@echo "System not configured - see README" >&2
 	@ exit 1
 
-tools:
+tools:	$(VERSION_FILE)
 	$(MAKE) -C $@ all
 endif	# config.mk
 
+$(VERSION_FILE):
+		@( localvers='$(shell $(TOPDIR)/tools/setlocalversion $(TOPDIR))' ; \
+		   printf '#define PLAIN_VERSION "%s%s"\n' \
+			"$(U_BOOT_VERSION)" "$${localvers}" ; \
+		   printf '#define U_BOOT_VERSION "U-Boot %s%s"\n' \
+			"$(U_BOOT_VERSION)" "$${localvers}" ; \
+		) > $@.tmp
+		@( printf '#define CC_VERSION_STRING "%s"\n' \
+		 '$(shell $(CC) --version | head -n 1)' )>>  $@.tmp
+		@( printf '#define LD_VERSION_STRING "%s"\n' \
+		 '$(shell $(LD) -v | head -n 1)' )>>  $@.tmp
+		@cmp -s $@ $@.tmp && rm -f $@.tmp || mv -f $@.tmp $@
+
 easylogo env gdb:
 	$(MAKE) -C tools/$@ all MTD_VERSION=${MTD_VERSION}
 gdbtools: gdb
 
-tools-all: easylogo env gdb
+tools-all: easylogo env gdb $(VERSION_FILE)
 	$(MAKE) -C tools HOST_TOOLS_ALL=y
 
 .PHONY : CHANGELOG
diff --git a/config.mk b/config.mk
index 7ce554e..51994f2 100644
--- a/config.mk
+++ b/config.mk
@@ -180,7 +180,9 @@  ifndef LDSCRIPT
 		LDSCRIPT := $(TOPDIR)/$(CPUDIR)/u-boot.lds
 	endif
 	ifeq ($(wildcard $(LDSCRIPT)),)
+		ifeq ($(obj)include/config.mk,$(wildcard $(obj)include/config.mk))
 $(error could not find linker script)
+		endif
 	endif
 endif
 
diff --git a/tools/Makefile b/tools/Makefile
index 623f908..30ae0b5 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -67,7 +67,14 @@  BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX)
 BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX)
 
 # Source files which exist outside the tools directory
-EXT_OBJ_FILES-y += common/env_embedded.o
+EXT_OBJ_FILES-$(CONFIG_ENV_IS_EMBEDDED) += common/env_embedded.o
+EXT_OBJ_FILES-$(CONFIG_ENV_IS_IN_DATAFLASH) += common/env_embedded.o
+EXT_OBJ_FILES-$(CONFIG_ENV_IS_IN_EEPROM) += common/env_embedded.o
+EXT_OBJ_FILES-$(CONFIG_ENV_IS_IN_FLASH) += common/env_embedded.o
+EXT_OBJ_FILES-$(CONFIG_ENV_IS_IN_ONENAND) += common/env_embedded.o
+EXT_OBJ_FILES-$(CONFIG_ENV_IS_IN_NAND) += common/env_embedded.o
+EXT_OBJ_FILES-$(CONFIG_ENV_IS_IN_NVRAM) += common/env_embedded.o
+EXT_OBJ_FILES-$(CONFIG_ENV_IS_IN_SPI_FLASH) += common/env_embedded.o
 EXT_OBJ_FILES-y += common/image.o
 EXT_OBJ_FILES-y += lib/crc32.o
 EXT_OBJ_FILES-y += lib/md5.o
@@ -77,7 +84,14 @@  EXT_OBJ_FILES-y += lib/sha1.o
 OBJ_FILES-$(CONFIG_LCD_LOGO) += bmp_logo.o
 OBJ_FILES-$(CONFIG_VIDEO_LOGO) += bmp_logo.o
 NOPED_OBJ_FILES-y += default_image.o
-OBJ_FILES-y += envcrc.o
+OBJ_FILES-$(CONFIG_ENV_IS_EMBEDDED) += envcrc.o
+OBJ_FILES-$(CONFIG_ENV_IS_IN_DATAFLASH) += envcrc.o
+OBJ_FILES-$(CONFIG_ENV_IS_IN_EEPROM) += envcrc.o
+OBJ_FILES-$(CONFIG_ENV_IS_IN_FLASH) += envcrc.o
+OBJ_FILES-$(CONFIG_ENV_IS_IN_ONENAND) += envcrc.o
+OBJ_FILES-$(CONFIG_ENV_IS_IN_NAND) += envcrc.o
+OBJ_FILES-$(CONFIG_ENV_IS_IN_NVRAM) += envcrc.o
+OBJ_FILES-$(CONFIG_ENV_IS_IN_SPI_FLASH) += envcrc.o
 NOPED_OBJ_FILES-y += fit_image.o
 OBJ_FILES-$(CONFIG_CMD_NET) += gen_eth_addr.o
 OBJ_FILES-$(CONFIG_CMD_LOADS) += img2srec.o
@@ -149,7 +163,6 @@  HOSTCPPFLAGS =	-idirafter $(SRCTREE)/include \
 		-DUSE_HOSTCC \
 		-D__KERNEL_STRICT_NAMES
 
-
 all:	$(obj).depend $(BINS) $(LOGO-y) subdirs
 
 $(obj)bin2header$(SFX): $(obj)bin2header.o