Change in coreboot[master]: soc/intel/icelake: Make use of "all-y" to avoid exclusive stage files...

[Subrata Banik (Code Review)]

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36546 )

Change subject: soc/intel/icelake: Make use of "all-y" to avoid exclusive stage files in Makefile.inc
......................................................................

soc/intel/icelake: Make use of "all-y" to avoid exclusive stage files in Makefile.inc

This patch makes use of "all-y" in order to replace all common stage (bootblock, verstage,
romstage, postcar, ramstage) files inclusion in Makefile.inc

Change-Id: I11001d0d381ec9c1df41bc331da845f51e666a44
Signed-off-by: Subrata Banik 
---
M src/soc/intel/icelake/Makefile.inc
1 file changed, 7 insertions(+), 26 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/36546/1

diff --git a/src/soc/intel/icelake/Makefile.inc b/src/soc/intel/icelake/Makefile.inc
index 80dcdc1..a4ebd20 100644
--- a/src/soc/intel/icelake/Makefile.inc
+++ b/src/soc/intel/icelake/Makefile.inc
@@ -8,29 +8,26 @@
 subdirs-y += ../../../cpu/x86/smm
 subdirs-y += ../../../cpu/x86/tsc

+# all (bootblock, verstage, romstage, postcar, ramstage)
+all-y += gspi.c
+all-y += i2c.c
+all-y += pmutil.c
+all-y += spi.c
+all-y += uart.c
+
 bootblock-y += bootblock/bootblock.c
 bootblock-y += bootblock/cpu.c
 bootblock-y += bootblock/pch.c
-bootblock-y += pmutil.c
 bootblock-y += bootblock/report_platform.c
 bootblock-y += espi.c
 bootblock-y += gpio.c
-bootblock-y += gspi.c
-bootblock-y += i2c.c
 bootblock-y += memmap.c
-bootblock-y += spi.c
 bootblock-y += p2sb.c
-bootblock-y += uart.c

romstage-y += espi.c
 romstage-y += gpio.c
-romstage-y += gspi.c
-romstage-y += i2c.c
 romstage-y += memmap.c
-romstage-y += pmutil.c
 romstage-y += reset.c
-romstage-y += spi.c
-romstage-y += uart.c

ramstage-y += acpi.c
 ramstage-y += chip.c
@@ -41,18 +38,13 @@
 ramstage-y += fsp_params.c
 ramstage-y += gpio.c
 ramstage-y += graphics.c
-ramstage-y += gspi.c
-ramstage-y += i2c.c
 ramstage-y += lockdown.c
 ramstage-y += memmap.c
 ramstage-y += p2sb.c
 ramstage-y += pmc.c
-ramstage-y += pmutil.c
 ramstage-y += reset.c
 ramstage-y += smmrelocate.c
-ramstage-y += spi.c
 ramstage-y += systemagent.c
-ramstage-y += uart.c
 ramstage-y += sd.c

smm-y += gpio.c
@@ -63,17 +55,6 @@
 smm-y += uart.c

postcar-y += memmap.c
-postcar-y += pmutil.c
-postcar-y += i2c.c
-postcar-y += gspi.c
-postcar-y += spi.c
-postcar-y += uart.c
-
-verstage-y += gspi.c
-verstage-y += i2c.c
-verstage-y += pmutil.c
-verstage-y += spi.c
-verstage-y += uart.c

CPPFLAGS_common += -I$(src)/soc/intel/icelake
 CPPFLAGS_common += -I$(src)/soc/intel/icelake/include

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36546 )

Change subject: soc/intel/icelake: Make use of "all-y" to avoid exclusive stage files in Makefile.inc
......................................................................

Patch Set 1: Code-Review+2

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36546 )

Change subject: soc/intel/icelake: Make use of "all-y" to avoid exclusive stage files in Makefile.inc
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36546/1/src/soc/intel/icelake/Makef... 
File src/soc/intel/icelake/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/36546/1/src/soc/intel/icelake/Makef... 
PS1, Line 22: espi
Is it bad to add some more files like espi.c, gpio.c memmap.c p2sb.c to all-? I understand they are not currently added to verstage and maybe postcar. But, doesn't the linker anyways get rid of unused blocks anyways?

Or are they pulling in other dependencies and causing issues to build?

[Arthur Heymans (Code Review)]

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

Change subject: soc/intel/icelake: Make use of "all-y" to avoid exclusive stage files in Makefile.inc
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36546/1/src/soc/intel/icelake/Makef... 
File src/soc/intel/icelake/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/36546/1/src/soc/intel/icelake/Makef... 
PS1, Line 22: espi
> Is it bad to add some more files like espi.c, gpio.c memmap.c p2sb.c to all-? I understand they are not currently added to verstage and maybe postcar. But, doesn't the linker anyways get rid of unused blocks anyways?
>
> Or are they pulling in other dependencies and causing issues to build?

The linker deals with them properly. IMO for memmap it's bad as that is only meaningful starting from romstage but for other things you listed it would make sense.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36546 )

Change subject: soc/intel/icelake: Make use of "all-y" to avoid exclusive stage files in Makefile.inc
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36546/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/36546/1//COMMIT_MSG@7 
PS1, Line 7: to avoid exclusive stage files in Makefile.inc
Minor: I would remove this last part to make the commit summary shorter

Change in coreboot[master]: soc/intel/icelake: Make use of "all-y"

[Subrata Banik (Code Review)]

Hello Patrick Rudolph, Angel Pons, Arthur Heymans, build bot (Jenkins), Patrick Georgi, Martin Roth,

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

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

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

Change subject: soc/intel/icelake: Make use of "all-y"
......................................................................

soc/intel/icelake: Make use of "all-y"

This patch makes use of "all-y" in order to replace all common stage (bootblock, verstage,
romstage, postcar, ramstage) files inclusion in Makefile.inc

Change-Id: I11001d0d381ec9c1df41bc331da845f51e666a44
Signed-off-by: Subrata Banik 
---
M src/soc/intel/icelake/Makefile.inc
1 file changed, 7 insertions(+), 26 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/36546/2

Change in coreboot[master]: soc/intel/icelake: Make use of "all-y"

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36546 )

Change subject: soc/intel/icelake: Make use of "all-y"
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/36546/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/36546/1//COMMIT_MSG@7 
PS1, Line 7: to avoid exclusive stage files in Makefile.inc
> Minor: I would remove this last part to make the commit summary shorter

Done

https://review.coreboot.org/c/coreboot/+/36546/1/src/soc/intel/icelake/Makef... 
File src/soc/intel/icelake/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/36546/1/src/soc/intel/icelake/Makef... 
PS1, Line 22: espi
> > Is it bad to add some more files like espi.c, gpio.c memmap.c p2sb. […]

also shouldn't we try to limit the files inclusion in stages unless required ?

Change in coreboot[master]: soc/intel/icelake: Make use of "all-y"

[Subrata Banik (Code Review)]

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36546 )

Change subject: soc/intel/icelake: Make use of "all-y"
......................................................................

soc/intel/icelake: Make use of "all-y"

This patch makes use of "all-y" in order to replace all common stage (bootblock, verstage,
romstage, postcar, ramstage) files inclusion in Makefile.inc

Change-Id: I11001d0d381ec9c1df41bc331da845f51e666a44
Signed-off-by: Subrata Banik 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36546
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/icelake/Makefile.inc
1 file changed, 7 insertions(+), 26 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/src/soc/intel/icelake/Makefile.inc b/src/soc/intel/icelake/Makefile.inc
index 80dcdc1..a4ebd20 100644
--- a/src/soc/intel/icelake/Makefile.inc
+++ b/src/soc/intel/icelake/Makefile.inc
@@ -8,29 +8,26 @@
 subdirs-y += ../../../cpu/x86/smm
 subdirs-y += ../../../cpu/x86/tsc

+# all (bootblock, verstage, romstage, postcar, ramstage)
+all-y += gspi.c
+all-y += i2c.c
+all-y += pmutil.c
+all-y += spi.c
+all-y += uart.c
+
 bootblock-y += bootblock/bootblock.c
 bootblock-y += bootblock/cpu.c
 bootblock-y += bootblock/pch.c
-bootblock-y += pmutil.c
 bootblock-y += bootblock/report_platform.c
 bootblock-y += espi.c
 bootblock-y += gpio.c
-bootblock-y += gspi.c
-bootblock-y += i2c.c
 bootblock-y += memmap.c
-bootblock-y += spi.c
 bootblock-y += p2sb.c
-bootblock-y += uart.c

romstage-y += espi.c
 romstage-y += gpio.c
-romstage-y += gspi.c
-romstage-y += i2c.c
 romstage-y += memmap.c
-romstage-y += pmutil.c
 romstage-y += reset.c
-romstage-y += spi.c
-romstage-y += uart.c

ramstage-y += acpi.c
 ramstage-y += chip.c
@@ -41,18 +38,13 @@
 ramstage-y += fsp_params.c
 ramstage-y += gpio.c
 ramstage-y += graphics.c
-ramstage-y += gspi.c
-ramstage-y += i2c.c
 ramstage-y += lockdown.c
 ramstage-y += memmap.c
 ramstage-y += p2sb.c
 ramstage-y += pmc.c
-ramstage-y += pmutil.c
 ramstage-y += reset.c
 ramstage-y += smmrelocate.c
-ramstage-y += spi.c
 ramstage-y += systemagent.c
-ramstage-y += uart.c
 ramstage-y += sd.c

smm-y += gpio.c
@@ -63,17 +55,6 @@
 smm-y += uart.c

postcar-y += memmap.c
-postcar-y += pmutil.c
-postcar-y += i2c.c
-postcar-y += gspi.c
-postcar-y += spi.c
-postcar-y += uart.c
-
-verstage-y += gspi.c
-verstage-y += i2c.c
-verstage-y += pmutil.c
-verstage-y += spi.c
-verstage-y += uart.c

CPPFLAGS_common += -I$(src)/soc/intel/icelake
 CPPFLAGS_common += -I$(src)/soc/intel/icelake/include