Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32067
Change subject: nb/intel/sandybridge: Move boot_count_increment() ......................................................................
nb/intel/sandybridge: Move boot_count_increment()
Move boot_count_increment() to romstage.c, drop preprocessor code and only increase counter once on regular boot.
Change-Id: I6aa52b75edf19953405b70284c7e7db30f607cd6 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/northbridge/intel/sandybridge/early_init.c M src/northbridge/intel/sandybridge/romstage.c 2 files changed, 5 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/32067/1
diff --git a/src/northbridge/intel/sandybridge/early_init.c b/src/northbridge/intel/sandybridge/early_init.c index b923065..34aec38 100644 --- a/src/northbridge/intel/sandybridge/early_init.c +++ b/src/northbridge/intel/sandybridge/early_init.c @@ -20,7 +20,6 @@ #include <arch/io.h> #include <device/pci_ops.h> #include <device/pci_def.h> -#include <elog.h> #include <pc80/mc146818rtc.h> #include <romstage_handoff.h> #include "sandybridge.h" @@ -45,22 +44,7 @@ pci_write_config8(PCI_DEV(0, 0x00, 0), PAM5, 0x33); pci_write_config8(PCI_DEV(0, 0x00, 0), PAM6, 0x33);
-#if CONFIG(ELOG_BOOT_COUNT) - /* Increment Boot Counter for non-S3 resume */ - if ((inw(DEFAULT_PMBASE + PM1_STS) & WAK_STS) && - ((inl(DEFAULT_PMBASE + PM1_CNT) >> 10) & 7) != SLP_TYP_S3) - boot_count_increment(); -#endif - - printk(BIOS_DEBUG, " done.\n"); - -#if CONFIG(ELOG_BOOT_COUNT) - /* Increment Boot Counter except when resuming from S3 */ - if ((inw(DEFAULT_PMBASE + PM1_STS) & WAK_STS) && - ((inl(DEFAULT_PMBASE + PM1_CNT) >> 10) & 7) == SLP_TYP_S3) - return; - boot_count_increment(); -#endif + printk(BIOS_DEBUG, " done\n"); }
static void sandybridge_setup_graphics(void) diff --git a/src/northbridge/intel/sandybridge/romstage.c b/src/northbridge/intel/sandybridge/romstage.c index 8ddc156..96d98ea 100644 --- a/src/northbridge/intel/sandybridge/romstage.c +++ b/src/northbridge/intel/sandybridge/romstage.c @@ -29,6 +29,7 @@ #include <northbridge/intel/sandybridge/chip.h> #include <southbridge/intel/bd82x6x/pch.h> #include <southbridge/intel/common/pmclib.h> +#include <elog.h>
static void early_pch_reset_pmcon(void) { @@ -80,6 +81,9 @@
s3resume = southbridge_detect_s3_resume();
+ if (CONFIG(ELOG_BOOT_COUNT) && !s3resume) + boot_count_increment(); + post_code(0x38);
mainboard_early_init(s3resume);
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32067
to look at the new patch set (#4).
Change subject: nb/intel/sandybridge: Move boot_count_increment() ......................................................................
nb/intel/sandybridge: Move boot_count_increment()
Move boot_count_increment() to romstage.c, drop preprocessor code and only increase counter once on regular boot.
Tested on Lenovo T520 (Intel Sandy Bridge). Still boots to OS, no errors visible in dmesg.
Change-Id: I6aa52b75edf19953405b70284c7e7db30f607cd6 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/northbridge/intel/sandybridge/early_init.c M src/northbridge/intel/sandybridge/romstage.c 2 files changed, 5 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/32067/4
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32067 )
Change subject: nb/intel/sandybridge: Move boot_count_increment() ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/32067/4/src/northbridge/intel/sandybridge/ea... File src/northbridge/intel/sandybridge/early_init.c:
https://review.coreboot.org/#/c/32067/4/src/northbridge/intel/sandybridge/ea... PS4, Line 48: : : : : : : : : : : : : : : : Was it incremented twice on non-S3_resume path?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32067 )
Change subject: nb/intel/sandybridge: Move boot_count_increment() ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/32067/4/src/northbridge/intel/sandybridge/ea... File src/northbridge/intel/sandybridge/early_init.c:
https://review.coreboot.org/#/c/32067/4/src/northbridge/intel/sandybridge/ea... PS4, Line 48: : : : : : : : : : : : : : : :
Was it incremented twice on non-S3_resume path?
it looks like that.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32067 )
Change subject: nb/intel/sandybridge: Move boot_count_increment() ......................................................................
Patch Set 5: Code-Review+1
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32067 )
Change subject: nb/intel/sandybridge: Move boot_count_increment() ......................................................................
Patch Set 5: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32067 )
Change subject: nb/intel/sandybridge: Move boot_count_increment() ......................................................................
nb/intel/sandybridge: Move boot_count_increment()
Move boot_count_increment() to romstage.c, drop preprocessor code and only increase counter once on regular boot.
Tested on Lenovo T520 (Intel Sandy Bridge). Still boots to OS, no errors visible in dmesg.
Change-Id: I6aa52b75edf19953405b70284c7e7db30f607cd6 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32067 Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Reviewed-by: Patrick Georgi pgeorgi@google.com Reviewed-by: Arthur Heymans arthur@aheymans.xyz Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/northbridge/intel/sandybridge/early_init.c M src/northbridge/intel/sandybridge/romstage.c 2 files changed, 5 insertions(+), 17 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved HAOUAS Elyes: Looks good to me, but someone else must approve Arthur Heymans: Looks good to me, but someone else must approve
diff --git a/src/northbridge/intel/sandybridge/early_init.c b/src/northbridge/intel/sandybridge/early_init.c index b923065..34aec38 100644 --- a/src/northbridge/intel/sandybridge/early_init.c +++ b/src/northbridge/intel/sandybridge/early_init.c @@ -20,7 +20,6 @@ #include <arch/io.h> #include <device/pci_ops.h> #include <device/pci_def.h> -#include <elog.h> #include <pc80/mc146818rtc.h> #include <romstage_handoff.h> #include "sandybridge.h" @@ -45,22 +44,7 @@ pci_write_config8(PCI_DEV(0, 0x00, 0), PAM5, 0x33); pci_write_config8(PCI_DEV(0, 0x00, 0), PAM6, 0x33);
-#if CONFIG(ELOG_BOOT_COUNT) - /* Increment Boot Counter for non-S3 resume */ - if ((inw(DEFAULT_PMBASE + PM1_STS) & WAK_STS) && - ((inl(DEFAULT_PMBASE + PM1_CNT) >> 10) & 7) != SLP_TYP_S3) - boot_count_increment(); -#endif - - printk(BIOS_DEBUG, " done.\n"); - -#if CONFIG(ELOG_BOOT_COUNT) - /* Increment Boot Counter except when resuming from S3 */ - if ((inw(DEFAULT_PMBASE + PM1_STS) & WAK_STS) && - ((inl(DEFAULT_PMBASE + PM1_CNT) >> 10) & 7) == SLP_TYP_S3) - return; - boot_count_increment(); -#endif + printk(BIOS_DEBUG, " done\n"); }
static void sandybridge_setup_graphics(void) diff --git a/src/northbridge/intel/sandybridge/romstage.c b/src/northbridge/intel/sandybridge/romstage.c index 43316a2..064d042 100644 --- a/src/northbridge/intel/sandybridge/romstage.c +++ b/src/northbridge/intel/sandybridge/romstage.c @@ -30,6 +30,7 @@ #include <northbridge/intel/sandybridge/chip.h> #include <southbridge/intel/bd82x6x/pch.h> #include <southbridge/intel/common/pmclib.h> +#include <elog.h>
static void early_pch_reset_pmcon(void) { @@ -79,6 +80,9 @@
s3resume = southbridge_detect_s3_resume();
+ if (CONFIG(ELOG_BOOT_COUNT) && !s3resume) + boot_count_increment(); + post_code(0x38);
mainboard_early_init(s3resume);
-
Arthur Heymans (Code Review) -
HAOUAS Elyes (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)