Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45047 )
Change subject: sb/intel/lynxpoint: Do not determine PCH type at runtime ......................................................................
sb/intel/lynxpoint: Do not determine PCH type at runtime
Both PCH types are very different, and mixing the code for both together isn't useful. First of all, inline `pch_is_lp` to return a constant.
Subsequent commits will further split the southbridge code.
Change-Id: Iba904acf64096478d1b76ffd05a076f0203502f8 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/lynxpoint/early_pch.c M src/southbridge/intel/lynxpoint/pch.c M src/southbridge/intel/lynxpoint/pch.h 3 files changed, 5 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/45047/1
diff --git a/src/southbridge/intel/lynxpoint/early_pch.c b/src/southbridge/intel/lynxpoint/early_pch.c index 85f9f33..956d1d2 100644 --- a/src/southbridge/intel/lynxpoint/early_pch.c +++ b/src/southbridge/intel/lynxpoint/early_pch.c @@ -35,11 +35,6 @@ return PCH_TYPE_DESKTOP; }
-int pch_is_lp(void) -{ - return get_pch_platform_type() == PCH_TYPE_ULT; -} - static void pch_enable_bars(void) { pci_write_config32(PCH_LPC_DEV, RCBA, (uintptr_t)DEFAULT_RCBA | 1); diff --git a/src/southbridge/intel/lynxpoint/pch.c b/src/southbridge/intel/lynxpoint/pch.c index c08f0da..adc011b 100644 --- a/src/southbridge/intel/lynxpoint/pch.c +++ b/src/southbridge/intel/lynxpoint/pch.c @@ -57,11 +57,6 @@ return PCH_TYPE_DESKTOP; }
-int pch_is_lp(void) -{ - return get_pch_platform_type() == PCH_TYPE_ULT; -} - u16 get_pmbase(void) { static u16 pmbase; diff --git a/src/southbridge/intel/lynxpoint/pch.h b/src/southbridge/intel/lynxpoint/pch.h index 2c86ff0..1ecad62 100644 --- a/src/southbridge/intel/lynxpoint/pch.h +++ b/src/southbridge/intel/lynxpoint/pch.h @@ -69,6 +69,11 @@
#ifndef __ACPI__
+static inline int pch_is_lp(void) +{ + return CONFIG(INTEL_LYNXPOINT_LP); +} + /* PCH platform types, safe for MRC consumption */ enum pch_platform_type { PCH_TYPE_MOBILE = 0, @@ -84,7 +89,6 @@ enum pch_platform_type get_pch_platform_type(void); int pch_silicon_revision(void); int pch_silicon_id(void); -int pch_is_lp(void); u16 get_pmbase(void); u16 get_gpiobase(void);
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45047 )
Change subject: sb/intel/lynxpoint: Do not determine PCH type at runtime ......................................................................
Patch Set 3: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/45047/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45047/3//COMMIT_MSG@7 PS3, Line 7: Do not determine PCH type at runtime Maybe motivate this by stating that determining this at buildtime allows the compiler to optimize out unused code?
https://review.coreboot.org/c/coreboot/+/45047/3/src/southbridge/intel/lynxp... File src/southbridge/intel/lynxpoint/early_pch.c:
https://review.coreboot.org/c/coreboot/+/45047/3/src/southbridge/intel/lynxp... PS3, Line 40: get_pch_platform_type() == PCH_TYPE_ULT Outside the scope of this CL: would asserting (get_pch_platform_type() == CONFIG(INTEL_LYNXPOINT_LP)) somewhere be useful?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45047 )
Change subject: sb/intel/lynxpoint: Do not determine PCH type at runtime ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45047/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45047/3//COMMIT_MSG@7 PS3, Line 7: Do not determine PCH type at runtime
Maybe motivate this by stating that determining this at buildtime allows the compiler to optimize ou […]
Will do it right now.
https://review.coreboot.org/c/coreboot/+/45047/3/src/southbridge/intel/lynxp... File src/southbridge/intel/lynxpoint/early_pch.c:
https://review.coreboot.org/c/coreboot/+/45047/3/src/southbridge/intel/lynxp... PS3, Line 40: get_pch_platform_type() == PCH_TYPE_ULT
Outside the scope of this CL: would asserting (get_pch_platform_type() == CONFIG(INTEL_LYNXPOINT_LP) […]
The only sane thing we could do is die()
Hello build bot (Jenkins), Arthur Heymans, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45047
to look at the new patch set (#4).
Change subject: sb/intel/lynxpoint: Do not determine PCH type at runtime ......................................................................
sb/intel/lynxpoint: Do not determine PCH type at runtime
Both PCH types are very different, and mixing the code for both together isn't useful. First of all, inline `pch_is_lp` to return a constant. This allows the compiler to optimize out unused code, which results in smaller executables. For the Asrock B85M Pro4, it's about 2.5 KiB less.
Subsequent commits will further split the southbridge code.
Change-Id: Iba904acf64096478d1b76ffd05a076f0203502f8 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/lynxpoint/early_pch.c M src/southbridge/intel/lynxpoint/pch.c M src/southbridge/intel/lynxpoint/pch.h 3 files changed, 5 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/45047/4
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45047 )
Change subject: sb/intel/lynxpoint: Do not determine PCH type at runtime ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45047/3/src/southbridge/intel/lynxp... File src/southbridge/intel/lynxpoint/early_pch.c:
https://review.coreboot.org/c/coreboot/+/45047/3/src/southbridge/intel/lynxp... PS3, Line 40: get_pch_platform_type() == PCH_TYPE_ULT
The only sane thing we could do is die()
Sure.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45047 )
Change subject: sb/intel/lynxpoint: Do not determine PCH type at runtime ......................................................................
sb/intel/lynxpoint: Do not determine PCH type at runtime
Both PCH types are very different, and mixing the code for both together isn't useful. First of all, inline `pch_is_lp` to return a constant. This allows the compiler to optimize out unused code, which results in smaller executables. For the Asrock B85M Pro4, it's about 2.5 KiB less.
Subsequent commits will further split the southbridge code.
Change-Id: Iba904acf64096478d1b76ffd05a076f0203502f8 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/45047 Reviewed-by: Arthur Heymans arthur@aheymans.xyz Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/southbridge/intel/lynxpoint/early_pch.c M src/southbridge/intel/lynxpoint/pch.c M src/southbridge/intel/lynxpoint/pch.h 3 files changed, 5 insertions(+), 11 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved
diff --git a/src/southbridge/intel/lynxpoint/early_pch.c b/src/southbridge/intel/lynxpoint/early_pch.c index 85f9f33..956d1d2 100644 --- a/src/southbridge/intel/lynxpoint/early_pch.c +++ b/src/southbridge/intel/lynxpoint/early_pch.c @@ -35,11 +35,6 @@ return PCH_TYPE_DESKTOP; }
-int pch_is_lp(void) -{ - return get_pch_platform_type() == PCH_TYPE_ULT; -} - static void pch_enable_bars(void) { pci_write_config32(PCH_LPC_DEV, RCBA, (uintptr_t)DEFAULT_RCBA | 1); diff --git a/src/southbridge/intel/lynxpoint/pch.c b/src/southbridge/intel/lynxpoint/pch.c index c08f0da..adc011b 100644 --- a/src/southbridge/intel/lynxpoint/pch.c +++ b/src/southbridge/intel/lynxpoint/pch.c @@ -57,11 +57,6 @@ return PCH_TYPE_DESKTOP; }
-int pch_is_lp(void) -{ - return get_pch_platform_type() == PCH_TYPE_ULT; -} - u16 get_pmbase(void) { static u16 pmbase; diff --git a/src/southbridge/intel/lynxpoint/pch.h b/src/southbridge/intel/lynxpoint/pch.h index 2c86ff0..1ecad62 100644 --- a/src/southbridge/intel/lynxpoint/pch.h +++ b/src/southbridge/intel/lynxpoint/pch.h @@ -69,6 +69,11 @@
#ifndef __ACPI__
+static inline int pch_is_lp(void) +{ + return CONFIG(INTEL_LYNXPOINT_LP); +} + /* PCH platform types, safe for MRC consumption */ enum pch_platform_type { PCH_TYPE_MOBILE = 0, @@ -84,7 +89,6 @@ enum pch_platform_type get_pch_platform_type(void); int pch_silicon_revision(void); int pch_silicon_id(void); -int pch_is_lp(void); u16 get_pmbase(void); u16 get_gpiobase(void);
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Patrick Georgi (Code Review)