Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43093 )
Change subject: haswell: relocate `romstage_common` to northrbridge ......................................................................
haswell: relocate `romstage_common` to northrbridge
Other platforms do this as well. It will ease refactoring on follow-ups.
Change-Id: I643982a58c6f5370c78acef93740f27df001a06d Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/cpu/intel/haswell/Makefile.inc M src/cpu/intel/haswell/haswell.h M src/northbridge/intel/haswell/Makefile.inc M src/northbridge/intel/haswell/haswell.h R src/northbridge/intel/haswell/romstage.c 5 files changed, 12 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/43093/1
diff --git a/src/cpu/intel/haswell/Makefile.inc b/src/cpu/intel/haswell/Makefile.inc index aebeed4..b93b911 100644 --- a/src/cpu/intel/haswell/Makefile.inc +++ b/src/cpu/intel/haswell/Makefile.inc @@ -1,5 +1,4 @@ ramstage-y += haswell_init.c -romstage-y += romstage.c romstage-y += ../car/romstage.c
ramstage-y += acpi.c diff --git a/src/cpu/intel/haswell/haswell.h b/src/cpu/intel/haswell/haswell.h index 7906b83..b336e4c 100644 --- a/src/cpu/intel/haswell/haswell.h +++ b/src/cpu/intel/haswell/haswell.h @@ -118,16 +118,6 @@ # error "CONFIG_IED_REGION_SIZE is not a power of 2" #endif
-struct pei_data; -struct rcba_config_instruction; -struct romstage_params { - struct pei_data *pei_data; - const void *gpio_map; - const struct rcba_config_instruction *rcba_config; - void (*copy_spd)(struct pei_data *); -}; -void romstage_common(const struct romstage_params *params); - /* Lock MSRs */ void intel_cpu_haswell_finalize_smm(void);
diff --git a/src/northbridge/intel/haswell/Makefile.inc b/src/northbridge/intel/haswell/Makefile.inc index 8ef3079..b2fd530 100644 --- a/src/northbridge/intel/haswell/Makefile.inc +++ b/src/northbridge/intel/haswell/Makefile.inc @@ -14,6 +14,7 @@
romstage-y += memmap.c romstage-y += raminit.c +romstage-y += romstage.c romstage-y += early_init.c romstage-y += report_platform.c
diff --git a/src/northbridge/intel/haswell/haswell.h b/src/northbridge/intel/haswell/haswell.h index c393049..b98d880 100644 --- a/src/northbridge/intel/haswell/haswell.h +++ b/src/northbridge/intel/haswell/haswell.h @@ -189,6 +189,16 @@
void intel_northbridge_haswell_finalize_smm(void);
+struct pei_data; +struct rcba_config_instruction; +struct romstage_params { + struct pei_data *pei_data; + const void *gpio_map; + const struct rcba_config_instruction *rcba_config; + void (*copy_spd)(struct pei_data *peid); +}; +void romstage_common(const struct romstage_params *params); + void haswell_early_initialization(void); void haswell_late_initialization(void); void set_translation_table(int start, int end, u64 base, int inc); diff --git a/src/cpu/intel/haswell/romstage.c b/src/northbridge/intel/haswell/romstage.c similarity index 97% rename from src/cpu/intel/haswell/romstage.c rename to src/northbridge/intel/haswell/romstage.c index 7886de0..579eca7 100644 --- a/src/cpu/intel/haswell/romstage.c +++ b/src/northbridge/intel/haswell/romstage.c @@ -7,11 +7,11 @@ #include <cbmem.h> #include <commonlib/helpers.h> #include <romstage_handoff.h> +#include <cpu/intel/haswell/haswell.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> #include <southbridge/intel/lynxpoint/me.h> -#include "haswell.h"
void romstage_common(const struct romstage_params *params) {
Tristan Corrick has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43093 )
Change subject: haswell: relocate `romstage_common` to northrbridge ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
I agree that the northbridge directory is a more appropriate location, and that is reason enough for the change. However, it's not clear to me from reading this commit how the change helps with refactoring later.
https://review.coreboot.org/c/coreboot/+/43093/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43093/1//COMMIT_MSG@7 PS1, Line 7: northrbridge typo: northbridge
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Tristan Corrick, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43093
to look at the new patch set (#2).
Change subject: haswell: relocate `romstage_common` to northbridge ......................................................................
haswell: relocate `romstage_common` to northbridge
Other platforms do this as well. It will ease refactoring on follow-ups.
Change-Id: I643982a58c6f5370c78acef93740f27df001a06d Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/cpu/intel/haswell/Makefile.inc M src/cpu/intel/haswell/haswell.h M src/northbridge/intel/haswell/Makefile.inc M src/northbridge/intel/haswell/haswell.h R src/northbridge/intel/haswell/romstage.c 5 files changed, 12 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/43093/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43093 )
Change subject: haswell: relocate `romstage_common` to northbridge ......................................................................
Patch Set 2:
(1 comment)
Patch Set 1: Code-Review+2
(1 comment)
I agree that the northbridge directory is a more appropriate location, and that is reason enough for the change. However, it's not clear to me from reading this commit how the change helps with refactoring later.
It's for similarity with sandybridge and because it brings the files I touch around the same place
https://review.coreboot.org/c/coreboot/+/43093/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43093/1//COMMIT_MSG@7 PS1, Line 7: northrbridge
typo: northbridge
Done
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43093 )
Change subject: haswell: relocate `romstage_common` to northbridge ......................................................................
haswell: relocate `romstage_common` to northbridge
Other platforms do this as well. It will ease refactoring on follow-ups.
Change-Id: I643982a58c6f5370c78acef93740f27df001a06d Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/43093 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tristan Corrick tristan@corrick.kiwi --- M src/cpu/intel/haswell/Makefile.inc M src/cpu/intel/haswell/haswell.h M src/northbridge/intel/haswell/Makefile.inc M src/northbridge/intel/haswell/haswell.h R src/northbridge/intel/haswell/romstage.c 5 files changed, 12 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Tristan Corrick: Looks good to me, approved
diff --git a/src/cpu/intel/haswell/Makefile.inc b/src/cpu/intel/haswell/Makefile.inc index aebeed4..b93b911 100644 --- a/src/cpu/intel/haswell/Makefile.inc +++ b/src/cpu/intel/haswell/Makefile.inc @@ -1,5 +1,4 @@ ramstage-y += haswell_init.c -romstage-y += romstage.c romstage-y += ../car/romstage.c
ramstage-y += acpi.c diff --git a/src/cpu/intel/haswell/haswell.h b/src/cpu/intel/haswell/haswell.h index 7906b83..b336e4c 100644 --- a/src/cpu/intel/haswell/haswell.h +++ b/src/cpu/intel/haswell/haswell.h @@ -118,16 +118,6 @@ # error "CONFIG_IED_REGION_SIZE is not a power of 2" #endif
-struct pei_data; -struct rcba_config_instruction; -struct romstage_params { - struct pei_data *pei_data; - const void *gpio_map; - const struct rcba_config_instruction *rcba_config; - void (*copy_spd)(struct pei_data *); -}; -void romstage_common(const struct romstage_params *params); - /* Lock MSRs */ void intel_cpu_haswell_finalize_smm(void);
diff --git a/src/northbridge/intel/haswell/Makefile.inc b/src/northbridge/intel/haswell/Makefile.inc index 8ef3079..b2fd530 100644 --- a/src/northbridge/intel/haswell/Makefile.inc +++ b/src/northbridge/intel/haswell/Makefile.inc @@ -14,6 +14,7 @@
romstage-y += memmap.c romstage-y += raminit.c +romstage-y += romstage.c romstage-y += early_init.c romstage-y += report_platform.c
diff --git a/src/northbridge/intel/haswell/haswell.h b/src/northbridge/intel/haswell/haswell.h index c393049..b98d880 100644 --- a/src/northbridge/intel/haswell/haswell.h +++ b/src/northbridge/intel/haswell/haswell.h @@ -189,6 +189,16 @@
void intel_northbridge_haswell_finalize_smm(void);
+struct pei_data; +struct rcba_config_instruction; +struct romstage_params { + struct pei_data *pei_data; + const void *gpio_map; + const struct rcba_config_instruction *rcba_config; + void (*copy_spd)(struct pei_data *peid); +}; +void romstage_common(const struct romstage_params *params); + void haswell_early_initialization(void); void haswell_late_initialization(void); void set_translation_table(int start, int end, u64 base, int inc); diff --git a/src/cpu/intel/haswell/romstage.c b/src/northbridge/intel/haswell/romstage.c similarity index 97% rename from src/cpu/intel/haswell/romstage.c rename to src/northbridge/intel/haswell/romstage.c index 7886de0..579eca7 100644 --- a/src/cpu/intel/haswell/romstage.c +++ b/src/northbridge/intel/haswell/romstage.c @@ -7,11 +7,11 @@ #include <cbmem.h> #include <commonlib/helpers.h> #include <romstage_handoff.h> +#include <cpu/intel/haswell/haswell.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> #include <southbridge/intel/lynxpoint/me.h> -#include "haswell.h"
void romstage_common(const struct romstage_params *params) {
-
Angel Pons (Code Review) -
Tristan Corrick (Code Review)