I configured v2 and TINT, then:
$ make /home/stuge/co/buildrom/buildrom-devel/packages/grub2/grub2.mk:28: *** To build GRUB2, you need to install 'ruby'. Stop.
I don't want to build GRUB2.
//Peter
On Thu, Apr 24, 2008 at 06:32:17AM +0200, Peter Stuge wrote:
I configured v2 and TINT, then:
Cannot reproduce using ALIX.1C target in buildrom. Which target did you use? Do you have some .config* .tmpconfig* .kconfig* files around? Try a fresh checkout, just in case.
$ make /home/stuge/co/buildrom/buildrom-devel/packages/grub2/grub2.mk:28: *** To build GRUB2, you need to install 'ruby'. Stop.
I don't want to build GRUB2.
Uwe.
On Thu, Apr 24, 2008 at 11:21:41AM +0200, Uwe Hermann wrote:
On Thu, Apr 24, 2008 at 06:32:17AM +0200, Peter Stuge wrote:
I configured v2 and TINT, then:
Cannot reproduce using ALIX.1C target in buildrom.
Maybe you have ruby installed.
Which target did you use?
I thought I picked 1C but it seems I was using DB800.
Do you have some .config* .tmpconfig* .kconfig* files around? Try a fresh checkout, just in case.
It was a fresh checkout. Am retrying.
$ make /home/stuge/co/buildrom/buildrom-devel/packages/grub2/grub2.mk:28: *** To build GRUB2, you need to install 'ruby'. Stop.
Still the same.
--8<-- grub2.mk HAVE_RUBY:=$(call find-tool,ruby)
ifeq ($(HAVE_RUBY),n) $(error To build GRUB2, you need to install 'ruby') endif -->8--
The problem is that this is executed even when a different payload has been selected.
//Peter
On Thu, Apr 24, 2008 at 12:54:20PM +0200, Peter Stuge wrote:
On Thu, Apr 24, 2008 at 11:21:41AM +0200, Uwe Hermann wrote:
On Thu, Apr 24, 2008 at 06:32:17AM +0200, Peter Stuge wrote:
I configured v2 and TINT, then:
Cannot reproduce using ALIX.1C target in buildrom.
Maybe you have ruby installed.
Which target did you use?
I thought I picked 1C but it seems I was using DB800.
Do you have some .config* .tmpconfig* .kconfig* files around? Try a fresh checkout, just in case.
It was a fresh checkout. Am retrying.
$ make /home/stuge/co/buildrom/buildrom-devel/packages/grub2/grub2.mk:28: *** To build GRUB2, you need to install 'ruby'. Stop.
Still the same.
It's a bug. This is a workaround (I'm still working on a proper fix, make should not include all payload makefiles all the time):
------------------------------------- Index: packages/grub2/grub2.mk =================================================================== --- packages/grub2/grub2.mk (revision 155) +++ packages/grub2/grub2.mk (working copy) @@ -24,9 +24,11 @@
HAVE_RUBY:=$(call find-tool,ruby)
+ifeq ($(PAYLOAD),grub2) ifeq ($(HAVE_RUBY),n) $(error To build GRUB2, you need to install 'ruby') endif +endif
$(SOURCE_DIR)/$(GRUB2_TAR): @ mkdir -p $(SOURCE_DIR) -------------------------------------
Thanks, Ward.
On 24/04/08 06:32 +0200, Peter Stuge wrote:
I configured v2 and TINT, then:
$ make /home/stuge/co/buildrom/buildrom-devel/packages/grub2/grub2.mk:28: *** To build GRUB2, you need to install 'ruby'. Stop.
I don't want to build GRUB2.
Oops! Thats my bad. Patch forthcoming.
Jordan
On 24/04/08 09:54 -0400, Ward Vandewege wrote:
On Thu, Apr 24, 2008 at 12:54:20PM +0200, Peter Stuge wrote:
On Thu, Apr 24, 2008 at 11:21:41AM +0200, Uwe Hermann wrote:
On Thu, Apr 24, 2008 at 06:32:17AM +0200, Peter Stuge wrote:
I configured v2 and TINT, then:
Cannot reproduce using ALIX.1C target in buildrom.
Maybe you have ruby installed.
Which target did you use?
I thought I picked 1C but it seems I was using DB800.
Do you have some .config* .tmpconfig* .kconfig* files around? Try a fresh checkout, just in case.
It was a fresh checkout. Am retrying.
$ make /home/stuge/co/buildrom/buildrom-devel/packages/grub2/grub2.mk:28: *** To build GRUB2, you need to install 'ruby'. Stop.
Still the same.
It's a bug. This is a workaround (I'm still working on a proper fix, make should not include all payload makefiles all the time):
Thanks - I ack this if we need to, but ++ on fixing the includes properly.
Index: packages/grub2/grub2.mk
--- packages/grub2/grub2.mk (revision 155) +++ packages/grub2/grub2.mk (working copy) @@ -24,9 +24,11 @@
HAVE_RUBY:=$(call find-tool,ruby)
+ifeq ($(PAYLOAD),grub2) ifeq ($(HAVE_RUBY),n) $(error To build GRUB2, you need to install 'ruby') endif +endif
$(SOURCE_DIR)/$(GRUB2_TAR): @ mkdir -p $(SOURCE_DIR)
Thanks, Ward.
-- Ward Vandewege ward@fsf.org Free Software Foundation - Senior System Administrator
-- coreboot mailing list coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
On Thu, Apr 24, 2008 at 8:38 AM, Jordan Crouse jordan.crouse@amd.com wrote:
On 24/04/08 09:54 -0400, Ward Vandewege wrote:
On Thu, Apr 24, 2008 at 12:54:20PM +0200, Peter Stuge wrote:
On Thu, Apr 24, 2008 at 11:21:41AM +0200, Uwe Hermann wrote:
On Thu, Apr 24, 2008 at 06:32:17AM +0200, Peter Stuge wrote:
I configured v2 and TINT, then:
Cannot reproduce using ALIX.1C target in buildrom.
Maybe you have ruby installed.
Which target did you use?
I thought I picked 1C but it seems I was using DB800.
Do you have some .config* .tmpconfig* .kconfig* files around? Try a fresh checkout, just in case.
It was a fresh checkout. Am retrying.
$ make /home/stuge/co/buildrom/buildrom-devel/packages/grub2/grub2.mk:28: *** To build GRUB2, you need to install 'ruby'. Stop.
Still the same.
It's a bug. This is a workaround (I'm still working on a proper fix, make should not include all payload makefiles all the time):
This patch changes the Makefile and the config/payloads/*.conf files so that only the payload .mk files that are needed are included. It still includes kernel.mk, roms.mk and geodevsa.mk every time, but that seemed like a separate change.
Thanks, Myles
Signed-off-by: Myles Watson mylesgw@gmail.com
Hi Myles,
On Fri, Apr 25, 2008 at 08:49:00AM -0600, Myles Watson wrote:
This patch changes the Makefile and the config/payloads/*.conf files so that only the payload .mk files that are needed are included. It still includes kernel.mk, roms.mk and geodevsa.mk every time, but that seemed like a separate change.
Great start.
Don't we need that PAYLOAD_AND_DEP_MK change for every payload?
Also; looks like we need one extra small change (see below).
Index: config/payloads/etherboot.conf
--- config/payloads/etherboot.conf (revision 160) +++ config/payloads/etherboot.conf (working copy) @@ -8,3 +8,4 @@ PAYLOAD_COMPRESSED=$(OUTPUT_DIR)/etherboot-payload.elf.lzma
PAYLOAD-y=etherboot +PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/etherboot/etherboot.mk Index: config/payloads/ofw.conf =================================================================== --- config/payloads/ofw.conf (revision 160) +++ config/payloads/ofw.conf (working copy) @@ -12,3 +12,4 @@
PAYLOAD-y=ofw #HOSTTOOLS-y=crc32sum +PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/ofw/ofw.mk Index: config/payloads/memtest.conf =================================================================== --- config/payloads/memtest.conf (revision 160) +++ config/payloads/memtest.conf (working copy) @@ -1,4 +1,4 @@ -# Configuration file for the etherboot payload +# Configuration file for the memtest payload
# Common configuration options
@@ -8,3 +8,5 @@ PAYLOAD_COMPRESSED=$(OUTPUT_DIR)/memtest-payload.elf.lzma
PAYLOAD-y=memtest
+PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/memtest/memtest.mk Index: config/payloads/tint.conf =================================================================== --- config/payloads/tint.conf (revision 160) +++ config/payloads/tint.conf (working copy) @@ -10,5 +10,7 @@ PAYLOAD-y=tint PAYLOAD=tint
+PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/libpayload/libpayload.mk $(PACKAGE_DIR)/tint/tint.mk
# Add libpayload as a dependency DEPENDS-y=libpayload Index: config/payloads/grub2.conf =================================================================== --- config/payloads/grub2.conf (revision 160) +++ config/payloads/grub2.conf (working copy) @@ -9,3 +9,5 @@
PAYLOAD-y=grub2 PAYLOAD=grub2
+PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/grub2/grub2.mk Index: config/payloads/coreinfo.conf =================================================================== --- config/payloads/coreinfo.conf (revision 160) +++ config/payloads/coreinfo.conf (working copy) @@ -11,4 +11,5 @@ PAYLOAD=coreinfo
# Add libpayload as a dependency +PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/coreinfo/coreinfo.mk $(PACKAGE_DIR)/libpayload/libpayload.mk DEPENDS-y=libpayload Index: config/payloads/filo.conf =================================================================== --- config/payloads/filo.conf (revision 160) +++ config/payloads/filo.conf (working copy) @@ -8,6 +8,7 @@ PAYLOAD_COMPRESSED=$(OUTPUT_DIR)/filo-payload.elf.lzma
PAYLOAD-y=filo +PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/filo/filo.mk
PAYLOAD=filo
Index: Makefile
--- Makefile (revision 160) +++ Makefile (working copy) @@ -117,9 +117,10 @@ # The following code gets all the make targets, but filters out the kernel # targets which are implicitly set by the platform configuration
-MKTARGETS:= $(shell ls $(PACKAGE_DIR)/*/*.mk) +include $(PAYLOAD_AND_DEP_MK)
-include $(filter-out $(PACKAGE_DIR)/kernel/% $(PACKAGE_DIR)/coreboot-v2/% $(PACKAGE_DIR)/coreboot-v3/%,$(MKTARGETS)) +include $(PACKAGE_DIR)/geodevsa/geodevsa.mk +include $(PACKAGE_DIR)/roms/roms.mk
I needed to add
+include $(PACKAGE_DIR)/utils/nrv2b.mk
to get make to behave.
This also brings up the question why nrv2b.mk is in the 'utils' directory, instead of the $(PACKAGE_DIR)/nrv2b/ directory?
Thanks, Ward.
Hi Myles,
On Fri, Apr 25, 2008 at 08:49:00AM -0600, Myles Watson wrote:
This patch changes the Makefile and the config/payloads/*.conf files so
that
only the payload .mk files that are needed are included. It still
includes
kernel.mk, roms.mk and geodevsa.mk every time, but that seemed like a
separate
change.
Great start.
Don't we need that PAYLOAD_AND_DEP_MK change for every payload?
Did I miss one? Most of the payloads don't have dependencies. The libpayload ones were the only ones I saw.
Also; looks like we need one extra small change (see below).
Index: Makefile
--- Makefile (revision 160) +++ Makefile (working copy) @@ -117,9 +117,10 @@ # The following code gets all the make targets, but filters out the
kernel
# targets which are implicitly set by the platform configuration
-MKTARGETS:= $(shell ls $(PACKAGE_DIR)/*/*.mk) +include $(PAYLOAD_AND_DEP_MK)
-include $(filter-out $(PACKAGE_DIR)/kernel/% $(PACKAGE_DIR)/coreboot-
v2/% $(PACKAGE_DIR)/coreboot-v3/%,$(MKTARGETS))
+include $(PACKAGE_DIR)/geodevsa/geodevsa.mk +include $(PACKAGE_DIR)/roms/roms.mk
I needed to add
+include $(PACKAGE_DIR)/utils/nrv2b.mk
Yep, I missed that one. Fixed.
This also brings up the question why nrv2b.mk is in the 'utils' directory, instead of the $(PACKAGE_DIR)/nrv2b/ directory?
Because it doesn't get included in the ROM?
Thanks, Myles
On Fri, Apr 25, 2008 at 09:20:23AM -0600, Myles Watson wrote:
Don't we need that PAYLOAD_AND_DEP_MK change for every payload?
Did I miss one? Most of the payloads don't have dependencies. The libpayload ones were the only ones I saw.
The LAB and kernel payloads?
+include $(PACKAGE_DIR)/utils/nrv2b.mk
Yep, I missed that one. Fixed.
Great.
This also brings up the question why nrv2b.mk is in the 'utils' directory, instead of the $(PACKAGE_DIR)/nrv2b/ directory?
Because it doesn't get included in the ROM?
Hmm, ok, yes.
Thanks, Ward.
On 25/04/08 11:37 -0400, Ward Vandewege wrote:
On Fri, Apr 25, 2008 at 09:20:23AM -0600, Myles Watson wrote:
Don't we need that PAYLOAD_AND_DEP_MK change for every payload?
Did I miss one? Most of the payloads don't have dependencies. The libpayload ones were the only ones I saw.
The LAB and kernel payloads?
+include $(PACKAGE_DIR)/utils/nrv2b.mk
Yep, I missed that one. Fixed.
Great.
This also brings up the question why nrv2b.mk is in the 'utils' directory, instead of the $(PACKAGE_DIR)/nrv2b/ directory?
Because it doesn't get included in the ROM?
Hmm, ok, yes.
I guess it all depends on how many directories we want to look at, and trade that off with how complex the scripts should be. I don't really have an opinion one way or the other on that.
Jordan
-----Original Message----- From: coreboot-bounces@coreboot.org [mailto:coreboot-bounces@coreboot.org] On Behalf Of Jordan Crouse Sent: Friday, April 25, 2008 9:40 AM To: Myles Watson; coreboot@coreboot.org Subject: Re: [coreboot] buildrom bug
On 25/04/08 11:37 -0400, Ward Vandewege wrote:
On Fri, Apr 25, 2008 at 09:20:23AM -0600, Myles Watson wrote:
Don't we need that PAYLOAD_AND_DEP_MK change for every payload?
Did I miss one? Most of the payloads don't have dependencies. The libpayload ones were the only ones I saw.
The LAB and kernel payloads?
+include $(PACKAGE_DIR)/utils/nrv2b.mk
Yep, I missed that one. Fixed.
Great.
This also brings up the question why nrv2b.mk is in the 'utils'
directory,
instead of the $(PACKAGE_DIR)/nrv2b/ directory?
Because it doesn't get included in the ROM?
Hmm, ok, yes.
I guess it all depends on how many directories we want to look at, and trade that off with how complex the scripts should be. I don't really have an opinion one way or the other on that.
I'm happy to move it. I was just speculating on the original reason it was there. Unfortunately, lzma is in $(PACKAGE_DIR)/lzma/, which shoots down my theory. I think that we should avoid automagically including make files. There are few enough of them that we should only include specific ones to avoid problems.
BTW I also added the include for lzma in the same place as the nrvb.mk file.
Myles
On 25/04/08 10:13 -0600, Myles Watson wrote:
-----Original Message----- From: coreboot-bounces@coreboot.org [mailto:coreboot-bounces@coreboot.org] On Behalf Of Jordan Crouse Sent: Friday, April 25, 2008 9:40 AM To: Myles Watson; coreboot@coreboot.org Subject: Re: [coreboot] buildrom bug
On 25/04/08 11:37 -0400, Ward Vandewege wrote:
On Fri, Apr 25, 2008 at 09:20:23AM -0600, Myles Watson wrote:
Don't we need that PAYLOAD_AND_DEP_MK change for every payload?
Did I miss one? Most of the payloads don't have dependencies. The libpayload ones were the only ones I saw.
The LAB and kernel payloads?
+include $(PACKAGE_DIR)/utils/nrv2b.mk
Yep, I missed that one. Fixed.
Great.
This also brings up the question why nrv2b.mk is in the 'utils'
directory,
instead of the $(PACKAGE_DIR)/nrv2b/ directory?
Because it doesn't get included in the ROM?
Hmm, ok, yes.
I guess it all depends on how many directories we want to look at, and trade that off with how complex the scripts should be. I don't really have an opinion one way or the other on that.
I'm happy to move it. I was just speculating on the original reason it was there. Unfortunately, lzma is in $(PACKAGE_DIR)/lzma/, which shoots down my theory. I think that we should avoid automagically including make files. There are few enough of them that we should only include specific ones to avoid problems.
Your mistake is that you assumed that we (I) followed any specific plan. :)
Yes, go ahead and move them where ever you think they work the best, and ack on the including make files comment.
Jordan
On Fri, Apr 25, 2008 at 10:54 AM, Jordan Crouse jordan.crouse@amd.com wrote:
On 25/04/08 10:13 -0600, Myles Watson wrote:
-----Original Message----- From: coreboot-bounces@coreboot.org [mailto:coreboot-bounces@coreboot.org] On Behalf Of Jordan Crouse Sent: Friday, April 25, 2008 9:40 AM To: Myles Watson; coreboot@coreboot.org Subject: Re: [coreboot] buildrom bug
On 25/04/08 11:37 -0400, Ward Vandewege wrote:
On Fri, Apr 25, 2008 at 09:20:23AM -0600, Myles Watson wrote:
Don't we need that PAYLOAD_AND_DEP_MK change for every payload?
Did I miss one? Most of the payloads don't have dependencies. The libpayload ones were the only ones I saw.
The LAB and kernel payloads?
You're right. I've included them in this patch.
+include $(PACKAGE_DIR)/utils/nrv2b.mk
Yep, I missed that one. Fixed.
Great.
This also brings up the question why nrv2b.mk is in the 'utils'
directory,
instead of the $(PACKAGE_DIR)/nrv2b/ directory?
Because it doesn't get included in the ROM?
Hmm, ok, yes.
I guess it all depends on how many directories we want to look at, and trade that off with how complex the scripts should be. I don't really have an opinion one way or the other on that.
I'm happy to move it. I was just speculating on the original reason it was there. Unfortunately, lzma is in $(PACKAGE_DIR)/lzma/, which shoots down my theory. I think that we should avoid automagically including make files. There are few enough of them that we should only include specific ones to avoid problems.
Your mistake is that you assumed that we (I) followed any specific plan. :)
If we could only see the future... design would be so much easier.
Yes, go ahead and move them where ever you think they work the best, and ack on the including make files comment.
Here's the updated patch. It needs to be preceded by svn mv packages/utils packages/nrv2b
Signed-off-by: Myles Watson mylesgw@gmail.com
Thanks, Myles
Here's the updated patch. It needs to be preceded by svn mv packages/utils packages/nrv2b
Sorry. Here's the new patch.
Signed-off-by: Myles Watson mylesgw@gmail.com
Thanks, Myles
On Fri, Apr 25, 2008 at 11:43:04AM -0600, Myles Watson wrote:
The LAB and kernel payloads?
You're right. I've included them in this patch.
Hmm, I don't see them in the attached patch?
Here's the updated patch. It needs to be preceded by svn mv packages/utils packages/nrv2b
Signed-off-by: Myles Watson mylesgw@gmail.com
With LAB and kernel:
Acked-by: Ward Vandewege ward@gnu.org
Thanks, Ward.
On 25/04/08 11:45 -0600, Myles Watson wrote:
Here's the updated patch. It needs to be preceded by svn mv packages/utils packages/nrv2b
Sorry. Here's the new patch.
So here is the million dollar question. If all the .mk files are packages/<package>/<package>.mk, is there anything stopping us from constructing PAYLOAD_AND_DEP_MK like so:
PAYLOAD_AND_DEP_MK = $(patsubst %,$(PACKAGE_DIR)/packages/%/%.mk, $(DEPENDS-y) $(PAYLOAD-y) $(HOSTTOOLS-y))
That will have the side effect of including the kernel packages, but we could filter those out. I'm keen on the psuedo-automagic way that buildrom just works when you add a new package.
Jordan
Index: config/payloads/etherboot.conf
--- config/payloads/etherboot.conf (revision 167) +++ config/payloads/etherboot.conf (working copy) @@ -8,3 +8,4 @@ PAYLOAD_COMPRESSED=$(OUTPUT_DIR)/etherboot-payload.elf.lzma
PAYLOAD-y=etherboot +PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/etherboot/etherboot.mk Index: config/payloads/ofw.conf =================================================================== --- config/payloads/ofw.conf (revision 167) +++ config/payloads/ofw.conf (working copy) @@ -12,3 +12,4 @@
PAYLOAD-y=ofw #HOSTTOOLS-y=crc32sum +PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/ofw/ofw.mk Index: config/payloads/lab.conf =================================================================== --- config/payloads/lab.conf (revision 167) +++ config/payloads/lab.conf (working copy) @@ -33,3 +33,19 @@ PAYLOAD=lab
HOSTTOOLS-y = mkelfimage unifdef +PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/mkelfimage/mkelfimage.mk +PAYLOAD_AND_DEP_MK+=$(PACKAGE_DIR)/uclibc/uclibc.mk +PAYLOAD_AND_DEP_MK+=$(PACKAGE_DIR)/unifdef/unifdef.mk
+ifeq ($(CONFIG_KBL),y)
- PAYLOAD_AND_DEP_MK+=$(PACKAGE_DIR)/kexec-boot-loader/kexec-boot-loader.mk
+endif +ifeq ($(CONFIG_BUSYBOX),y)
- PAYLOAD_AND_DEP_MK+=$(PACKAGE_DIR)/busybox/busybox.mk
+endif +ifeq ($(CONFIG_BOOTMENU),y)
- PAYLOAD_AND_DEP_MK+=$(PACKAGE_DIR)/bootmenu/bootmenu.mk
+endif +ifeq ($(CONFIG_OLPCFLASH),y)
- PAYLOAD_AND_DEP_MK+=$(PACKAGE_DIR)/olpcflash/olpcflash.mk
+endif Index: config/payloads/memtest.conf =================================================================== --- config/payloads/memtest.conf (revision 167) +++ config/payloads/memtest.conf (working copy) @@ -1,4 +1,4 @@ -# Configuration file for the etherboot payload +# Configuration file for the memtest payload
# Common configuration options
@@ -8,3 +8,5 @@ PAYLOAD_COMPRESSED=$(OUTPUT_DIR)/memtest-payload.elf.lzma
PAYLOAD-y=memtest
+PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/memtest/memtest.mk Index: config/payloads/tint.conf =================================================================== --- config/payloads/tint.conf (revision 167) +++ config/payloads/tint.conf (working copy) @@ -10,5 +10,7 @@ PAYLOAD-y=tint PAYLOAD=tint
+PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/libpayload/libpayload.mk $(PACKAGE_DIR)/tint/tint.mk
# Add libpayload as a dependency DEPENDS-y=libpayload Index: config/payloads/kernel.conf =================================================================== --- config/payloads/kernel.conf (revision 167) +++ config/payloads/kernel.conf (working copy) @@ -19,3 +19,4 @@ PAYLOAD=kernel
HOSTTOOLS-y = mkelfimage +PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/mkelfimage/mkelfimage.mk Index: config/payloads/grub2.conf =================================================================== --- config/payloads/grub2.conf (revision 167) +++ config/payloads/grub2.conf (working copy) @@ -9,3 +9,5 @@
PAYLOAD-y=grub2 PAYLOAD=grub2
+PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/grub2/grub2.mk Index: config/payloads/coreinfo.conf =================================================================== --- config/payloads/coreinfo.conf (revision 167) +++ config/payloads/coreinfo.conf (working copy) @@ -11,4 +11,5 @@ PAYLOAD=coreinfo
# Add libpayload as a dependency +PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/coreinfo/coreinfo.mk $(PACKAGE_DIR)/libpayload/libpayload.mk DEPENDS-y=libpayload Index: config/payloads/filo.conf =================================================================== --- config/payloads/filo.conf (revision 167) +++ config/payloads/filo.conf (working copy) @@ -8,6 +8,7 @@ PAYLOAD_COMPRESSED=$(OUTPUT_DIR)/filo-payload.elf.lzma
PAYLOAD-y=filo +PAYLOAD_AND_DEP_MK=$(PACKAGE_DIR)/filo/filo.mk
PAYLOAD=filo
Index: Makefile
--- Makefile (revision 167) +++ Makefile (working copy) @@ -115,9 +115,14 @@ # The following code gets all the make targets, but filters out the kernel # targets which are implicitly set by the platform configuration
-MKTARGETS:= $(shell ls $(PACKAGE_DIR)/*/*.mk) +ifneq ($(PAYLOAD_AND_DEP_MK),) +include $(PAYLOAD_AND_DEP_MK) +endif
-include $(filter-out $(PACKAGE_DIR)/kernel/% $(PACKAGE_DIR)/coreboot-v2/% $(PACKAGE_DIR)/coreboot-v3/%,$(MKTARGETS)) +include $(PACKAGE_DIR)/nrv2b/nrv2b.mk +include $(PACKAGE_DIR)/lzma/lzma.mk +include $(PACKAGE_DIR)/geodevsa/geodevsa.mk +include $(PACKAGE_DIR)/roms/roms.mk
include $(KERNEL_MK)
On Fri, Apr 25, 2008 at 11:50:22AM -0600, Jordan Crouse wrote:
PAYLOAD_AND_DEP_MK = $(patsubst %,$(PACKAGE_DIR)/packages/%/%.mk, $(DEPENDS-y) $(PAYLOAD-y) $(HOSTTOOLS-y))
Nice.
//Peter
On 25/04/08 11:45 -0600, Myles Watson wrote:
Here's the updated patch. It needs to be preceded by svn mv packages/utils packages/nrv2b
Sorry. Here's the new patch.
So here is the million dollar question. If all the .mk files are packages/<package>/<package>.mk, is there anything stopping us from constructing PAYLOAD_AND_DEP_MK like so:
PAYLOAD_AND_DEP_MK = $(patsubst %,$(PACKAGE_DIR)/packages/%/%.mk, $(DEPENDS-y) $(PAYLOAD-y) $(HOSTTOOLS-y))
Doesn't that put you back where you were with the "You need ruby to compile grub2"? The point of the PAYLOAD_AND_DEP_MK variable was supposed to be that it got set (purposefully) in the config/payloads/<payload>.conf file. I think it's reasonable to ask a developer to set the variable when he adds the .conf file for a new package.
That will have the side effect of including the kernel packages, but we could filter those out. I'm keen on the psuedo-automagic way that buildrom just works when you add a new package.
If my explanation above didn't address this, can you give me an example of a new package that would get included without a .conf file?
Thanks, Myles
On Fri, Apr 25, 2008 at 12:01:58PM -0600, Myles Watson wrote:
PAYLOAD_AND_DEP_MK = $(patsubst %,$(PACKAGE_DIR)/packages/%/%.mk, $(DEPENDS-y) $(PAYLOAD-y) $(HOSTTOOLS-y))
Doesn't that put you back where you were with the "You need ruby to compile grub2"?
I don't know how pathsubst works exactly but I assumed that $(PAYLOAD-y) would be used somehow to only apply the substitution on the selected payload?
//Peter
On 25/04/08 12:01 -0600, Myles Watson wrote:
On 25/04/08 11:45 -0600, Myles Watson wrote:
Here's the updated patch. It needs to be preceded by svn mv packages/utils packages/nrv2b
Sorry. Here's the new patch.
So here is the million dollar question. If all the .mk files are packages/<package>/<package>.mk, is there anything stopping us from constructing PAYLOAD_AND_DEP_MK like so:
PAYLOAD_AND_DEP_MK = $(patsubst %,$(PACKAGE_DIR)/packages/%/%.mk, $(DEPENDS-y) $(PAYLOAD-y) $(HOSTTOOLS-y))
Doesn't that put you back where you were with the "You need ruby to compile grub2"? The point of the PAYLOAD_AND_DEP_MK variable was supposed to be that it got set (purposefully) in the config/payloads/<payload>.conf file. I think it's reasonable to ask a developer to set the variable when he adds the .conf file for a new package.
DEPENDS-y PAYLOAD-y and HOSTTOOLS-y are the dependencies for what you are actually building.
Our original problem was that we were doing a $(wildcard */*/.mk), which was including the kitchen sink. So unless your package is somehow listing grub2 as one of its dependencies, then packages/grub2/grub2.mk won't make it into the list, and you won't have a problem.
I think it is pretty unreasonable to ask the developer to add the variable - it includes magic that he shouldn't have to think about, namely how packages are referenced and included.
And lets look at it this way - 99.99% of the time, its going to be packages/<package>/<package>.mk - we're just automating the process. If we want, we can make PAYLOAD_AND_DEP_MK ?= so that the developer can override it if they want, but I think most people won't want to.
Jordan
-----Original Message----- From: Jordan Crouse [mailto:jordan.crouse@amd.com] Sent: Friday, April 25, 2008 12:12 PM To: Myles Watson Cc: coreboot@coreboot.org Subject: Re: buildrom bug
On 25/04/08 12:01 -0600, Myles Watson wrote:
On 25/04/08 11:45 -0600, Myles Watson wrote:
Here's the updated patch. It needs to be preceded by svn mv packages/utils packages/nrv2b
Sorry. Here's the new patch.
So here is the million dollar question. If all the .mk files are packages/<package>/<package>.mk, is there anything stopping us from constructing PAYLOAD_AND_DEP_MK like so:
PAYLOAD_AND_DEP_MK = $(patsubst %,$(PACKAGE_DIR)/packages/%/%.mk, $(DEPENDS-y) $(PAYLOAD-y) $(HOSTTOOLS-y))
Doesn't that put you back where you were with the "You need ruby to
compile
grub2"? The point of the PAYLOAD_AND_DEP_MK variable was supposed to be that it got set (purposefully) in the config/payloads/<payload>.conf
file.
I think it's reasonable to ask a developer to set the variable when he
adds
the .conf file for a new package.
DEPENDS-y PAYLOAD-y and HOSTTOOLS-y are the dependencies for what you are actually building.
Thanks for the clarification. I looked at you code and saw the original wildcard. Sorry.
Our original problem was that we were doing a $(wildcard */*/.mk), which was including the kitchen sink. So unless your package is somehow listing grub2 as one of its dependencies, then packages/grub2/grub2.mk won't make it into the list, and you won't have a problem.
I think it is pretty unreasonable to ask the developer to add the variable - it includes magic that he shouldn't have to think about, namely how packages are referenced and included.
And lets look at it this way - 99.99% of the time, its going to be packages/<package>/<package>.mk - we're just automating the process. If we want, we can make PAYLOAD_AND_DEP_MK ?= so that the developer can override it if they want, but I think most people won't want to.
I agree. Do you want to create the patch or send me a patch against mine?
Thanks, Myles
On 25/04/08 12:15 -0600, Myles Watson wrote:
-----Original Message----- From: Jordan Crouse [mailto:jordan.crouse@amd.com] Sent: Friday, April 25, 2008 12:12 PM To: Myles Watson Cc: coreboot@coreboot.org Subject: Re: buildrom bug
On 25/04/08 12:01 -0600, Myles Watson wrote:
On 25/04/08 11:45 -0600, Myles Watson wrote:
Here's the updated patch. It needs to be preceded by svn mv packages/utils packages/nrv2b
Sorry. Here's the new patch.
So here is the million dollar question. If all the .mk files are packages/<package>/<package>.mk, is there anything stopping us from constructing PAYLOAD_AND_DEP_MK like so:
PAYLOAD_AND_DEP_MK = $(patsubst %,$(PACKAGE_DIR)/packages/%/%.mk, $(DEPENDS-y) $(PAYLOAD-y) $(HOSTTOOLS-y))
Doesn't that put you back where you were with the "You need ruby to
compile
grub2"? The point of the PAYLOAD_AND_DEP_MK variable was supposed to be that it got set (purposefully) in the config/payloads/<payload>.conf
file.
I think it's reasonable to ask a developer to set the variable when he
adds
the .conf file for a new package.
DEPENDS-y PAYLOAD-y and HOSTTOOLS-y are the dependencies for what you are actually building.
Thanks for the clarification. I looked at you code and saw the original wildcard. Sorry.
Our original problem was that we were doing a $(wildcard */*/.mk), which was including the kitchen sink. So unless your package is somehow listing grub2 as one of its dependencies, then packages/grub2/grub2.mk won't make it into the list, and you won't have a problem.
I think it is pretty unreasonable to ask the developer to add the variable - it includes magic that he shouldn't have to think about, namely how packages are referenced and included.
And lets look at it this way - 99.99% of the time, its going to be packages/<package>/<package>.mk - we're just automating the process. If we want, we can make PAYLOAD_AND_DEP_MK ?= so that the developer can override it if they want, but I think most people won't want to.
I agree. Do you want to create the patch or send me a patch against mine?
I'll make a patch based on yours.
Jordan
-----Original Message----- From: Ward Vandewege [mailto:ward@gnu.org] Sent: Friday, April 25, 2008 11:48 AM To: Myles Watson Cc: Jordan Crouse; coreboot@coreboot.org Subject: Re: [coreboot] buildrom bug
On Fri, Apr 25, 2008 at 11:43:04AM -0600, Myles Watson wrote:
The LAB and kernel payloads?
You're right. I've included them in this patch.
Hmm, I don't see them in the attached patch?
Here's the updated patch. It needs to be preceded by svn mv packages/utils packages/nrv2b
Signed-off-by: Myles Watson mylesgw@gmail.com
With LAB and kernel:
Acked-by: Ward Vandewege ward@gnu.org
Rev 168.
Thanks, Myles
On 25/04/08 12:15 -0600, Myles Watson wrote:
I agree. Do you want to create the patch or send me a patch against mine?
We can further clean this up by moving the platform specific ifdefs for kernel and coreboot-v2 into packages/kernel/kernel.mk and packages/coreboot-v2/coreboot-v2.mk respectively. Then we can avoid any filtering at the main level.
Thoughts? Jordan
On Fri, Apr 25, 2008 at 01:36:08PM -0600, Jordan Crouse wrote:
Then we can avoid any filtering at the main level.
Thoughts?
Sounds good.
//Peter
I am assuming a update / co from svn will not build until this patch is in place? =-O
It doesn't for me.. :(
********************* Marc Karasek MTS Sun Microsystems mailto:marc.karasek@sun.com ph:770.360.6415 *********************
Jordan Crouse wrote:
On 25/04/08 12:15 -0600, Myles Watson wrote:
-----Original Message----- From: Jordan Crouse [mailto:jordan.crouse@amd.com] Sent: Friday, April 25, 2008 12:12 PM To: Myles Watson Cc: coreboot@coreboot.org Subject: Re: buildrom bug
On 25/04/08 12:01 -0600, Myles Watson wrote:
On 25/04/08 11:45 -0600, Myles Watson wrote:
> Here's the updated patch. It needs to be preceded by > svn mv packages/utils packages/nrv2b > Sorry. Here's the new patch.
So here is the million dollar question. If all the .mk files are packages/<package>/<package>.mk, is there anything stopping us from constructing PAYLOAD_AND_DEP_MK like so:
PAYLOAD_AND_DEP_MK = $(patsubst %,$(PACKAGE_DIR)/packages/%/%.mk, $(DEPENDS-y) $(PAYLOAD-y) $(HOSTTOOLS-y))
Doesn't that put you back where you were with the "You need ruby to
compile
grub2"? The point of the PAYLOAD_AND_DEP_MK variable was supposed to be that it got set (purposefully) in the config/payloads/<payload>.conf
file.
I think it's reasonable to ask a developer to set the variable when he
adds
the .conf file for a new package.
DEPENDS-y PAYLOAD-y and HOSTTOOLS-y are the dependencies for what you are actually building.
Thanks for the clarification. I looked at you code and saw the original wildcard. Sorry.
Our original problem was that we were doing a $(wildcard */*/.mk), which was including the kitchen sink. So unless your package is somehow listing grub2 as one of its dependencies, then packages/grub2/grub2.mk won't make it into the list, and you won't have a problem.
I think it is pretty unreasonable to ask the developer to add the variable - it includes magic that he shouldn't have to think about, namely how packages are referenced and included.
And lets look at it this way - 99.99% of the time, its going to be packages/<package>/<package>.mk - we're just automating the process. If we want, we can make PAYLOAD_AND_DEP_MK ?= so that the developer can override it if they want, but I think most people won't want to.
I agree. Do you want to create the patch or send me a patch against mine?
I'll make a patch based on yours.
Jordan
On 25/04/08 16:06 -0400, Marc Karasek wrote:
I am assuming a update / co from svn will not build until this patch is in place? =-O
It doesn't for me.. :(
Um - no. As far as I know, it should work. Myles?
Marc Karasek MTS Sun Microsystems mailto:marc.karasek@sun.com ph:770.360.6415
Jordan Crouse wrote:
On 25/04/08 12:15 -0600, Myles Watson wrote:
-----Original Message----- From: Jordan Crouse [mailto:jordan.crouse@amd.com] Sent: Friday, April 25, 2008 12:12 PM To: Myles Watson Cc: coreboot@coreboot.org Subject: Re: buildrom bug
On 25/04/08 12:01 -0600, Myles Watson wrote:
On 25/04/08 11:45 -0600, Myles Watson wrote:
>> Here's the updated patch. It needs to be preceded by >> svn mv packages/utils packages/nrv2b >> > Sorry. Here's the new patch. > So here is the million dollar question. If all the .mk files are packages/<package>/<package>.mk, is there anything stopping us from constructing PAYLOAD_AND_DEP_MK like so:
PAYLOAD_AND_DEP_MK = $(patsubst %,$(PACKAGE_DIR)/packages/%/%.mk, $(DEPENDS-y) $(PAYLOAD-y) $(HOSTTOOLS-y))
Doesn't that put you back where you were with the "You need ruby to
compile
grub2"? The point of the PAYLOAD_AND_DEP_MK variable was supposed to be that it got set (purposefully) in the config/payloads/<payload>.conf
file.
I think it's reasonable to ask a developer to set the variable when he
adds
the .conf file for a new package.
DEPENDS-y PAYLOAD-y and HOSTTOOLS-y are the dependencies for what you are actually building.
Thanks for the clarification. I looked at you code and saw the original wildcard. Sorry.
Our original problem was that we were doing a $(wildcard */*/.mk), which was including the kitchen sink. So unless your package is somehow listing grub2 as one of its dependencies, then packages/grub2/grub2.mk won't make it into the list, and you won't have a problem.
I think it is pretty unreasonable to ask the developer to add the variable - it includes magic that he shouldn't have to think about, namely how packages are referenced and included.
And lets look at it this way - 99.99% of the time, its going to be packages/<package>/<package>.mk - we're just automating the process. If we want, we can make PAYLOAD_AND_DEP_MK ?= so that the developer can override it if they want, but I think most people won't want to.
I agree. Do you want to create the patch or send me a patch against mine?
I'll make a patch based on yours.
Jordan
On 25/04/08 12:15 -0600, Myles Watson wrote:
I agree. Do you want to create the patch or send me a patch against mine?
Okay - so here we go. This patch uses black make magic to construct the list of .mk files to be included. But wait, thats not all - it consolidates all of the platform specific coreboot .mk and kernel .mk includes into generic toplevel packages/coreboot-v2/coreboot-v2.mk and packages/kernel/kernel.mk files so that we don't have to specially include files from within the toplevel Makefile. Finally, to avoid automagically including roms.mk and geodevsa.mk, I revamped how geodevsa works. I'm not super happy with it, but its at least reasonable and gets rid of some ifdefs. We'll need to address this in more detail when we revamp the lar UI and have other platforms that need option ROMs.
Please test and report back - this isn't a really scary patch, but it touches lots of files and lots of platforms.
Jordan
On Fri, Apr 25, 2008 at 03:52:43PM -0600, Jordan Crouse wrote:
b=`echo $$file | sed -e s:$(ROM_DIR)\/*::`; \
Should the expression include ^ somehow so as to not fail on subdirs with matching names?
Or just use basename?
+# FIXME: Get rid of this ifeq somehow
ifeq ($(CONFIG_COREBOOT_V2),y) -include $(CBV2_MK) +INCMK += $(PACKAGE_DIR)/coreboot-v2/coreboot-v2.mk else -include $(PACKAGE_DIR)/coreboot-v3/coreboot-v3.mk +INCMK += $(PACKAGE_DIR)/coreboot-v3/coreboot-v3.mk endif
This will have to exist _somewhere_ though.
+# Platform specific dependencies +DEPENDS-$(CONFIG_PLATFORM_GEODE) += geodevsa
I like it!
=================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ buildrom-devel/packages/coreboot-v2/coreboot-v2.mk 2008-04-25 15:28:59.000000000 -0600 @@ -0,0 +1,24 @@ +# "toplevel" coreboot-v2.mk - this is where we decide +# which of the platform specific files to actually +# include
+CBV2MK-y=$(PACKAGE_DIR)/coreboot-v2/generic.mk +CBV2MK-$(CONFIG_PLATFORM_NORWICH) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_MSM800SEV) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_ALIX1C) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_DB800) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_DBE61) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk
Why is this mapping needed here? Coreboot already knows that they are lx boards? Sorry, I am a total buildrom noob so will ask many questions and try to point out not obvious things.
+CBV2MK-$(CONFIG_PLATFORM_TYAN_S2881) = $(PACKAGE_DIR)/coreboot-v2/tyan-s2881.mk +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2882) = $(PACKAGE_DIR)/coreboot-v2/tyan-s2882.mk +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2891) = $(PACKAGE_DIR)/coreboot-v2/tyan-s2891.mk +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2892) = $(PACKAGE_DIR)/coreboot-v2/tyan-generic.mk +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2895) = $(PACKAGE_DIR)/coreboot-v2/tyan-generic.mk
Err.. When is tyan-generic good and when is tyan-<$newboard> good?
@@ -31,13 +35,19 @@ $(GEODE_PADDED_VSA): $(GEODE_COMPRESSED_VSA) @ cp $< $@ @ (size=`stat -c %s $<`; count=`expr $(GEODE_VSA_SIZE) - $$size`; \
- @ dd if=/dev/zero bs=1 count=$$count >> $@ 2> /dev/null)
- dd if=/dev/zero bs=1 count=$$count >> $@ 2> /dev/null)
On purpose?
+ifeq($(KERNELMK-y),) +$(error "You do not have a kernel .mk file defined for this platform") +endif
Awesome!
So many removed lines of code must make a good patch! :)
Acked-by: Peter Stuge peter@stuge.se
..but please wait for another.
//Peter
On 26/04/08 00:05 +0200, Peter Stuge wrote:
On Fri, Apr 25, 2008 at 03:52:43PM -0600, Jordan Crouse wrote:
b=`echo $$file | sed -e s:$(ROM_DIR)\/*::`; \Should the expression include ^ somehow so as to not fail on subdirs with matching names?
Or just use basename?
Its not just basename - there are also sub directories in there, i.e. $(ROM_DIR)/blob/vsa which will end up as blob/vsa in the LAR. The ^ is a good idea though.
+# FIXME: Get rid of this ifeq somehow
ifeq ($(CONFIG_COREBOOT_V2),y) -include $(CBV2_MK) +INCMK += $(PACKAGE_DIR)/coreboot-v2/coreboot-v2.mk else -include $(PACKAGE_DIR)/coreboot-v3/coreboot-v3.mk +INCMK += $(PACKAGE_DIR)/coreboot-v3/coreboot-v3.mk endif
This will have to exist _somewhere_ though.
Yes, probably.
=================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ buildrom-devel/packages/coreboot-v2/coreboot-v2.mk 2008-04-25 15:28:59.000000000 -0600 @@ -0,0 +1,24 @@ +# "toplevel" coreboot-v2.mk - this is where we decide +# which of the platform specific files to actually +# include
+CBV2MK-y=$(PACKAGE_DIR)/coreboot-v2/generic.mk +CBV2MK-$(CONFIG_PLATFORM_NORWICH) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_MSM800SEV) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_ALIX1C) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_DB800) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_DBE61) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk
Why is this mapping needed here? Coreboot already knows that they are lx boards? Sorry, I am a total buildrom noob so will ask many questions and try to point out not obvious things.
No problem. These are for platform specific magic that we need to do around the coreboot-v2 build, such as add different Config.lb files, etc, etc. All of the LX boards have the same platform specific magic (add VSA), so they share a similar target.
+CBV2MK-$(CONFIG_PLATFORM_TYAN_S2881) = $(PACKAGE_DIR)/coreboot-v2/tyan-s2881.mk +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2882) = $(PACKAGE_DIR)/coreboot-v2/tyan-s2882.mk +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2891) = $(PACKAGE_DIR)/coreboot-v2/tyan-s2891.mk +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2892) = $(PACKAGE_DIR)/coreboot-v2/tyan-generic.mk +CBV2MK-$(CONFIG_PLATFORM_TYAN_S2895) = $(PACKAGE_DIR)/coreboot-v2/tyan-generic.mk
Err.. When is tyan-generic good and when is tyan-<$newboard> good?
Thats a question for Myles.
@@ -31,13 +35,19 @@ $(GEODE_PADDED_VSA): $(GEODE_COMPRESSED_VSA) @ cp $< $@ @ (size=`stat -c %s $<`; count=`expr $(GEODE_VSA_SIZE) - $$size`; \
- @ dd if=/dev/zero bs=1 count=$$count >> $@ 2> /dev/null)
- dd if=/dev/zero bs=1 count=$$count >> $@ 2> /dev/null)
On purpose?
yes - the @ is a makeism - the second @ was in the shell and freaking out sh.
Jordan
On Fri, Apr 25, 2008 at 04:32:34PM -0600, Jordan Crouse wrote:
+CBV2MK-y=$(PACKAGE_DIR)/coreboot-v2/generic.mk +CBV2MK-$(CONFIG_PLATFORM_NORWICH) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_MSM800SEV) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_ALIX1C) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_DB800) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_DBE61) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk
Why is this mapping needed here? Coreboot already knows that they are lx boards? Sorry, I am a total buildrom noob so will ask many questions and try to point out not obvious things.
No problem. These are for platform specific magic that we need to do around the coreboot-v2 build, such as add different Config.lb files, etc, etc. All of the LX boards have the same platform specific magic (add VSA), so they share a similar target.
My point is that buildrom is integrated fairly tightly with coreboot already and it should not need to duplicate this mapping between e.g. ALIX1C and geodelx.
For v3 read the dts. For v2, hm, maybe look for "cpu/amd/model_lx" in mainboard/v/b/Config.lb.
Or no?
@ (size=`stat -c %s $<`; count=`expr $(GEODE_VSA_SIZE) - $$size`; \
- @ dd if=/dev/zero bs=1 count=$$count >> $@ 2> /dev/null)
- dd if=/dev/zero bs=1 count=$$count >> $@ 2> /dev/null)
On purpose?
yes - the @ is a makeism - the second @ was in the shell and freaking out sh.
Oh yes - good one.
//Peter
On 26/04/08 00:47 +0200, Peter Stuge wrote:
On Fri, Apr 25, 2008 at 04:32:34PM -0600, Jordan Crouse wrote:
+CBV2MK-y=$(PACKAGE_DIR)/coreboot-v2/generic.mk +CBV2MK-$(CONFIG_PLATFORM_NORWICH) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_MSM800SEV) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_ALIX1C) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_DB800) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk +CBV2MK-$(CONFIG_PLATFORM_DBE61) = $(PACKAGE_DIR)/coreboot-v2/geodelx.mk
Why is this mapping needed here? Coreboot already knows that they are lx boards? Sorry, I am a total buildrom noob so will ask many questions and try to point out not obvious things.
No problem. These are for platform specific magic that we need to do around the coreboot-v2 build, such as add different Config.lb files, etc, etc. All of the LX boards have the same platform specific magic (add VSA), so they share a similar target.
My point is that buildrom is integrated fairly tightly with coreboot already and it should not need to duplicate this mapping between e.g. ALIX1C and geodelx.
For v3 read the dts. For v2, hm, maybe look for "cpu/amd/model_lx" in mainboard/v/b/Config.lb.
No - this is actually stuff external to coreboot itself. In this case its the code that appends VSA to the ROM.
it is named geodelx.mk because thats a handy name, not because there is any sort of mapping.
On Fri, Apr 25, 2008 at 04:51:12PM -0600, Jordan Crouse wrote:
My point is that buildrom is integrated fairly tightly with coreboot already and it should not need to duplicate this mapping between e.g. ALIX1C and geodelx.
For v3 read the dts. For v2, hm, maybe look for "cpu/amd/model_lx" in mainboard/v/b/Config.lb.
No - this is actually stuff external to coreboot itself.
I know.
In this case its the code that appends VSA to the ROM.
Yes.
it is named geodelx.mk because thats a handy name, not because there is any sort of mapping.
All boards in coreboot using cpu/amd/model_lx need VSA, so rather than teaching buildrom one coreboot target at a time, I am suggesting to once and for all teach buildrom that all coreboot targets using cpu/amd/model_lx need VSA, by reusing a mapping (between $target_name and lx) that can be found in coreboot instead of duplicating the mapping in buildrom.
This can be extrapolated to work for other things as well, but right now I don't think there are any other uses.
Put another way: buildrom should not need it's own list of boards that use model_lx when coreboot already has one. (Though the index is backwards.)
//Peter
-----Original Message----- From: Marc.Karasek@Sun.COM [mailto:Marc.Karasek@Sun.COM] Sent: Friday, April 25, 2008 2:07 PM To: Jordan Crouse Cc: Myles Watson; coreboot@coreboot.org Subject: Re: [coreboot] buildrom bug
I am assuming a update / co from svn will not build until this patch is in place? =-O
It doesn't for me.. :(
Sorry. I forgot the svn mv that went with the patch. It should work now.
Myles
+CBV2MK-$(CONFIG_PLATFORM_TYAN_S2881) = $(PACKAGE_DIR)/coreboot-
v2/tyan-s2881.mk
+CBV2MK-$(CONFIG_PLATFORM_TYAN_S2882) = $(PACKAGE_DIR)/coreboot-
v2/tyan-s2882.mk
+CBV2MK-$(CONFIG_PLATFORM_TYAN_S2891) = $(PACKAGE_DIR)/coreboot-
v2/tyan-s2891.mk
+CBV2MK-$(CONFIG_PLATFORM_TYAN_S2892) = $(PACKAGE_DIR)/coreboot-
v2/tyan-generic.mk
+CBV2MK-$(CONFIG_PLATFORM_TYAN_S2895) = $(PACKAGE_DIR)/coreboot-
v2/tyan-generic.mk
Err.. When is tyan-generic good and when is tyan-<$newboard> good?
Thats a question for Myles.
The patch that gets rid of tyan-<$newboard> hasn't been acked yet.
Myles
@@ -31,13 +35,19 @@ $(GEODE_PADDED_VSA): $(GEODE_COMPRESSED_VSA) @ cp $< $@ @ (size=`stat -c %s $<`; count=`expr $(GEODE_VSA_SIZE) - $$size`; \
- @ dd if=/dev/zero bs=1 count=$$count >> $@ 2> /dev/null)
- dd if=/dev/zero bs=1 count=$$count >> $@ 2> /dev/null)
On purpose?
yes - the @ is a makeism - the second @ was in the shell and freaking out sh.
Can you indent the dd so that it's obvious that it's part of the previous line?
Thanks, Myles
On 28/04/08 09:40 -0600, Myles Watson wrote:
@@ -31,13 +35,19 @@ $(GEODE_PADDED_VSA): $(GEODE_COMPRESSED_VSA) @ cp $< $@ @ (size=`stat -c %s $<`; count=`expr $(GEODE_VSA_SIZE) - $$size`; \
- @ dd if=/dev/zero bs=1 count=$$count >> $@ 2> /dev/null)
- dd if=/dev/zero bs=1 count=$$count >> $@ 2> /dev/null)
On purpose?
yes - the @ is a makeism - the second @ was in the shell and freaking out sh.
Can you indent the dd so that it's obvious that it's part of the previous line?
Yep, not a problem. Isn't shell inside of Make grand? </sarcasm>
-----Original Message----- From: Jordan Crouse [mailto:jordan.crouse@amd.com] Sent: Friday, April 25, 2008 3:53 PM To: Myles Watson Cc: coreboot@coreboot.org Subject: Re: buildrom bug
On 25/04/08 12:15 -0600, Myles Watson wrote:
I agree. Do you want to create the patch or send me a patch against
mine?
Okay - so here we go. This patch uses black make magic to construct the list of .mk files to be included. But wait, thats not all - it consolidates all of the platform specific coreboot .mk and kernel .mk includes into generic toplevel packages/coreboot-v2/coreboot-v2.mk and packages/kernel/kernel.mk files so that we don't have to specially include files from within the toplevel Makefile. Finally, to avoid automagically including roms.mk and geodevsa.mk, I revamped how geodevsa works. I'm not super happy with it, but its at least reasonable and gets rid of some ifdefs. We'll need to address this in more detail when we revamp the lar UI and have other platforms that need option ROMs.
Please test and report back - this isn't a really scary patch, but it touches lots of files and lots of platforms.
Jordan,
Could you send it again from the latest version? Sorry for the mix up.
Thanks, Myles
On 28/04/08 13:08 -0600, Myles Watson wrote:
-----Original Message----- From: Jordan Crouse [mailto:jordan.crouse@amd.com] Sent: Friday, April 25, 2008 3:53 PM To: Myles Watson Cc: coreboot@coreboot.org Subject: Re: buildrom bug
On 25/04/08 12:15 -0600, Myles Watson wrote:
I agree. Do you want to create the patch or send me a patch against
mine?
Okay - so here we go. This patch uses black make magic to construct the list of .mk files to be included. But wait, thats not all - it consolidates all of the platform specific coreboot .mk and kernel .mk includes into generic toplevel packages/coreboot-v2/coreboot-v2.mk and packages/kernel/kernel.mk files so that we don't have to specially include files from within the toplevel Makefile. Finally, to avoid automagically including roms.mk and geodevsa.mk, I revamped how geodevsa works. I'm not super happy with it, but its at least reasonable and gets rid of some ifdefs. We'll need to address this in more detail when we revamp the lar UI and have other platforms that need option ROMs.
Please test and report back - this isn't a really scary patch, but it touches lots of files and lots of platforms.
Jordan,
Could you send it again from the latest version? Sorry for the mix up.
It should apply against the latest version, I did the nrv2b.mk fixup locally in my tree.
Jordan
-----Original Message----- From: Jordan Crouse [mailto:jordan.crouse@amd.com] Sent: Monday, April 28, 2008 1:15 PM To: Myles Watson Cc: coreboot@coreboot.org Subject: Re: buildrom bug
On 28/04/08 13:08 -0600, Myles Watson wrote:
-----Original Message----- From: Jordan Crouse [mailto:jordan.crouse@amd.com] Sent: Friday, April 25, 2008 3:53 PM To: Myles Watson Cc: coreboot@coreboot.org Subject: Re: buildrom bug
On 25/04/08 12:15 -0600, Myles Watson wrote:
I agree. Do you want to create the patch or send me a patch against
mine?
Okay - so here we go. This patch uses black make magic to construct the list of .mk files to be included. But wait, thats not all - it consolidates all of the platform specific coreboot .mk and kernel .mk includes into generic toplevel packages/coreboot-v2/coreboot-v2.mk and packages/kernel/kernel.mk files so that we don't have to specially
include
files from within the toplevel Makefile. Finally, to avoid
automagically
including roms.mk and geodevsa.mk, I revamped how geodevsa works. I'm
not
super happy with it, but its at least reasonable and gets rid of some ifdefs. We'll need to address this in more detail when we revamp the lar UI and have other platforms that need option ROMs.
Please test and report back - this isn't a really scary patch, but it touches lots of files and lots of platforms.
Jordan,
Could you send it again from the latest version? Sorry for the mix up.
It should apply against the latest version, I did the nrv2b.mk fixup locally in my tree.
I was actually talking about the simplify-v2 patch. For example, there is no more tyan-generic.mk or tyan-s2891.mk.
Thanks, Myles
-
Jordan Crouse -
Marc Karasek -
Myles Watson -
Peter Stuge -
Uwe Hermann -
Ward Vandewege