Change in coreboot[master]: arch/x86/Makefile.inc: Use the 'all' target to add common sources

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36002 )

Change subject: arch/x86/Makefile.inc: Use the 'all' target to add common sources
......................................................................

arch/x86/Makefile.inc: Use the 'all' target to add common sources

Change-Id: Ibbd418656c32f56be2b00481068e8499421b147c
Signed-off-by: Arthur Heymans 
---
M src/arch/x86/Makefile.inc
1 file changed, 10 insertions(+), 26 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/36002/1

diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc
index 612424d..7a846c4 100644
--- a/src/arch/x86/Makefile.inc
+++ b/src/arch/x86/Makefile.inc
@@ -47,12 +47,6 @@
 pci$(stripped_vgabios_dgpu_id).rom-file := $(call strip_quotes,$(CONFIG_VGA_BIOS_DGPU_FILE))
 pci$(stripped_vgabios_dgpu_id).rom-type := optionrom

-verstage-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
-bootblock-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
-romstage-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
-ramstage-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
-postcar-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
-
 ###############################################################################
 # common support for early assembly includes
 ###############################################################################
@@ -95,17 +89,23 @@
 endef

###############################################################################
+# all (bootblock,verstage,romstage,postcar,ramstage)
+###############################################################################
+
+all-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
+all-y += boot.c
+all-y += memcpy.c
+all-y += memset.c
+all-y += cpu_common.c
+
+###############################################################################
 # bootblock
 ###############################################################################

ifeq ($(CONFIG_ARCH_BOOTBLOCK_X86_32)$(CONFIG_ARCH_BOOTBLOCK_X86_64),y)

-bootblock-y += boot.c
-bootblock-y += cpu_common.c
 bootblock-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c
 bootblock-$(CONFIG_IDT_IN_EVERY_STAGE) += idt.S
-bootblock-y += memcpy.c
-bootblock-y += memset.c
 bootblock-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c
 bootblock-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c

@@ -183,14 +183,10 @@

ifeq ($(CONFIG_ARCH_VERSTAGE_X86_32)$(CONFIG_ARCH_VERSTAGE_X86_64),y)

-verstage-y += boot.c
 verstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += gdt_init.S
 verstage-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c
 verstage-$(CONFIG_IDT_IN_EVERY_STAGE) += idt.S

-verstage-y += cpu_common.c
-verstage-y += memset.c
-verstage-y += memcpy.c
 verstage-y += memmove.c
 verstage-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 # If verstage is a separate stage it means there's no need
@@ -219,19 +215,15 @@
 ifeq ($(CONFIG_ARCH_ROMSTAGE_X86_32)$(CONFIG_ARCH_ROMSTAGE_X86_64),y)

romstage-$(CONFIG_HAVE_ACPI_RESUME) += acpi_s3.c
-romstage-y += boot.c
 # gdt_init.S is included by entry32.inc when romstage is the first C
 # environment.
 romstage-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += gdt_init.S
 romstage-y += cbmem.c
 romstage-y += cbfs_and_run.c
-romstage-y += cpu_common.c
 romstage-$(CONFIG_EARLY_EBDA_INIT) += ebda.c
 romstage-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c
 romstage-$(CONFIG_IDT_IN_EVERY_STAGE) += idt.S
-romstage-y += memcpy.c
 romstage-y += memmove.c
-romstage-y += memset.c
 romstage-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 romstage-y += postcar_loader.c
 romstage-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c
@@ -261,18 +253,14 @@
 postcar-generic-ccopts += -D__POSTCAR__

postcar-$(CONFIG_HAVE_ACPI_RESUME) += acpi_s3.c
-postcar-y += boot.c
 postcar-y += gdt_init.S
 postcar-y += cbfs_and_run.c
 postcar-y += cbmem.c
-postcar-y += cpu_common.c
 postcar-$(CONFIG_EARLY_EBDA_INIT) += ebda.c
 postcar-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c
 postcar-$(CONFIG_IDT_IN_EVERY_STAGE) += idt.S
 postcar-y += exit_car.S
-postcar-y += memcpy.c
 postcar-y += memmove.c
-postcar-y += memset.c
 postcar-y += memlayout.ld
 postcar-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 postcar-y += postcar.c
@@ -306,20 +294,16 @@
 ramstage-$(CONFIG_HAVE_ACPI_TABLES) += acpi_pld.c
 ramstage-$(CONFIG_HAVE_ACPI_RESUME) += acpi_s3.c
 ramstage-$(CONFIG_ACPI_BERT) += acpi_bert_storage.c
-ramstage-y += boot.c
 ramstage-y += c_start.S
 ramstage-y += cbmem.c
 ramstage-y += cpu.c
-ramstage-y += cpu_common.c
 ramstage-y += ebda.c
 ramstage-y += exception.c
 ramstage-y += idt.S
 ramstage-y += gdt.c
 ramstage-$(CONFIG_IOAPIC) += ioapic.c
-ramstage-y += memcpy.c
 ramstage-y += memlayout.ld
 ramstage-y += memmove.c
-ramstage-y += memset.c
 ramstage-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 ramstage-$(CONFIG_GENERATE_MP_TABLE) += mpspec.c
 ramstage-$(CONFIG_GENERATE_PIRQ_TABLE) += pirq_routing.c

[Arthur Heymans (Code Review)]

Hello Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36002

to look at the new patch set (#2).

Change subject: arch/x86/Makefile.inc: Use the 'all' target to add common sources
......................................................................

arch/x86/Makefile.inc: Use the 'all' target to add common sources

Change-Id: Ibbd418656c32f56be2b00481068e8499421b147c
Signed-off-by: Arthur Heymans 
---
M src/arch/x86/Makefile.inc
1 file changed, 14 insertions(+), 26 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/36002/2

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36002 )

Change subject: arch/x86/Makefile.inc: Use the 'all' target to add common sources
......................................................................

Patch Set 2:

Decompressor stage is missing

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36002 )

Change subject: arch/x86/Makefile.inc: Use the 'all' target to add common sources
......................................................................

Patch Set 2:
> Patch Set 2:
>
> Decompressor stage is missing

what do you mean? Decompressor is not added to the 'all' target. Do you think it should?

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36002 )

Change subject: arch/x86/Makefile.inc: Use the 'all' target to add common sources
......................................................................

Patch Set 2:
> Patch Set 2:
> > Patch Set 2:
> >
> > Decompressor stage is missing
>
> what do you mean? Decompressor is not added to the 'all' target. Do you think it should?

IMHO yes, as you write *all* except SMM

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36002 )

Change subject: arch/x86/Makefile.inc: Use the 'all' target to add common sources
......................................................................

Patch Set 2:
> Patch Set 2:
> > Patch Set 2:
> > > Patch Set 2:
> > >
> > > Decompressor stage is missing
> >
> > what do you mean? Decompressor is not added to the 'all' target. Do you think it should?
>
> IMHO yes, as you write *all* except SMM

The decompressor class is only a stub to load a compressed bootblock to sram, so it shouldn't matter for x86. The special 'all' class currently deals with classes that are more or less traditional 'stages' so places where you expect things like console, timestamps, gpio's, ... . I'm not sufficiently familiar with the decompressor class to know if it would be useful to add it to the all class as it would likely reduce the amount of files that could use 'all'.

Would 'all_stages' cover the current semantics better?

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36002 )

Change subject: arch/x86/Makefile.inc: Use the 'all' target to add common sources
......................................................................

Patch Set 2: Code-Review+2
> Would 'all_stages' cover the current semantics better?

How about `stages`?

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36002 )

Change subject: arch/x86/Makefile.inc: Use the 'all' target to add common sources
......................................................................

arch/x86/Makefile.inc: Use the 'all' target to add common sources

Change-Id: Ibbd418656c32f56be2b00481068e8499421b147c
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36002
Tested-by: build bot (Jenkins) 
Reviewed-by: Nico Huber 
---
M src/arch/x86/Makefile.inc
1 file changed, 14 insertions(+), 26 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc
index 612424d..8d00174 100644
--- a/src/arch/x86/Makefile.inc
+++ b/src/arch/x86/Makefile.inc
@@ -47,12 +47,6 @@
 pci$(stripped_vgabios_dgpu_id).rom-file := $(call strip_quotes,$(CONFIG_VGA_BIOS_DGPU_FILE))
 pci$(stripped_vgabios_dgpu_id).rom-type := optionrom

-verstage-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
-bootblock-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
-romstage-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
-ramstage-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
-postcar-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
-
 ###############################################################################
 # common support for early assembly includes
 ###############################################################################
@@ -95,17 +89,27 @@
 endef

###############################################################################
+# all (bootblock,verstage,romstage,postcar,ramstage)
+###############################################################################
+
+ifeq ($(CONFIG_ARCH_X86),y)
+
+all-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
+all-y += boot.c
+all-y += memcpy.c
+all-y += memset.c
+all-y += cpu_common.c
+
+endif
+
+###############################################################################
 # bootblock
 ###############################################################################

ifeq ($(CONFIG_ARCH_BOOTBLOCK_X86_32)$(CONFIG_ARCH_BOOTBLOCK_X86_64),y)

-bootblock-y += boot.c
-bootblock-y += cpu_common.c
 bootblock-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c
 bootblock-$(CONFIG_IDT_IN_EVERY_STAGE) += idt.S
-bootblock-y += memcpy.c
-bootblock-y += memset.c
 bootblock-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c
 bootblock-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c

@@ -183,14 +187,10 @@

ifeq ($(CONFIG_ARCH_VERSTAGE_X86_32)$(CONFIG_ARCH_VERSTAGE_X86_64),y)

-verstage-y += boot.c
 verstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += gdt_init.S
 verstage-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c
 verstage-$(CONFIG_IDT_IN_EVERY_STAGE) += idt.S

-verstage-y += cpu_common.c
-verstage-y += memset.c
-verstage-y += memcpy.c
 verstage-y += memmove.c
 verstage-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 # If verstage is a separate stage it means there's no need
@@ -219,19 +219,15 @@
 ifeq ($(CONFIG_ARCH_ROMSTAGE_X86_32)$(CONFIG_ARCH_ROMSTAGE_X86_64),y)

romstage-$(CONFIG_HAVE_ACPI_RESUME) += acpi_s3.c
-romstage-y += boot.c
 # gdt_init.S is included by entry32.inc when romstage is the first C
 # environment.
 romstage-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += gdt_init.S
 romstage-y += cbmem.c
 romstage-y += cbfs_and_run.c
-romstage-y += cpu_common.c
 romstage-$(CONFIG_EARLY_EBDA_INIT) += ebda.c
 romstage-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c
 romstage-$(CONFIG_IDT_IN_EVERY_STAGE) += idt.S
-romstage-y += memcpy.c
 romstage-y += memmove.c
-romstage-y += memset.c
 romstage-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 romstage-y += postcar_loader.c
 romstage-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c
@@ -261,18 +257,14 @@
 postcar-generic-ccopts += -D__POSTCAR__

postcar-$(CONFIG_HAVE_ACPI_RESUME) += acpi_s3.c
-postcar-y += boot.c
 postcar-y += gdt_init.S
 postcar-y += cbfs_and_run.c
 postcar-y += cbmem.c
-postcar-y += cpu_common.c
 postcar-$(CONFIG_EARLY_EBDA_INIT) += ebda.c
 postcar-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c
 postcar-$(CONFIG_IDT_IN_EVERY_STAGE) += idt.S
 postcar-y += exit_car.S
-postcar-y += memcpy.c
 postcar-y += memmove.c
-postcar-y += memset.c
 postcar-y += memlayout.ld
 postcar-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 postcar-y += postcar.c
@@ -306,20 +298,16 @@
 ramstage-$(CONFIG_HAVE_ACPI_TABLES) += acpi_pld.c
 ramstage-$(CONFIG_HAVE_ACPI_RESUME) += acpi_s3.c
 ramstage-$(CONFIG_ACPI_BERT) += acpi_bert_storage.c
-ramstage-y += boot.c
 ramstage-y += c_start.S
 ramstage-y += cbmem.c
 ramstage-y += cpu.c
-ramstage-y += cpu_common.c
 ramstage-y += ebda.c
 ramstage-y += exception.c
 ramstage-y += idt.S
 ramstage-y += gdt.c
 ramstage-$(CONFIG_IOAPIC) += ioapic.c
-ramstage-y += memcpy.c
 ramstage-y += memlayout.ld
 ramstage-y += memmove.c
-ramstage-y += memset.c
 ramstage-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 ramstage-$(CONFIG_GENERATE_MP_TABLE) += mpspec.c
 ramstage-$(CONFIG_GENERATE_PIRQ_TABLE) += pirq_routing.c