Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32316
Change subject: sb/intel/ibexpeak: Use SOUTHBRIDGE_INTEL_COMMON_PMCLIB ......................................................................
sb/intel/ibexpeak: Use SOUTHBRIDGE_INTEL_COMMON_PMCLIB
Use common code to detect ACPI S3. Untested.
Change-Id: I766a3f4206e6605e6b7bb9a1ff0b2deb6721eb77 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/mainboard/lenovo/x201/romstage.c M src/mainboard/packardbell/ms2290/romstage.c M src/southbridge/intel/ibexpeak/Kconfig 3 files changed, 13 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/32316/1
diff --git a/src/mainboard/lenovo/x201/romstage.c b/src/mainboard/lenovo/x201/romstage.c index 7e895b4..e65622c 100644 --- a/src/mainboard/lenovo/x201/romstage.c +++ b/src/mainboard/lenovo/x201/romstage.c @@ -34,6 +34,7 @@ #include "dock.h" #include <southbridge/intel/ibexpeak/pch.h> #include <southbridge/intel/common/gpio.h> +#include <southbridge/intel/common/pmclib.h> #include <northbridge/intel/nehalem/nehalem.h>
#include <northbridge/intel/nehalem/raminit.h> @@ -206,24 +207,14 @@ /* Halt if there was a built in self test failure */ report_bist_failure(bist);
- /* Read PM1_CNT */ - reg32 = inl(DEFAULT_PMBASE + 0x04); - printk(BIOS_DEBUG, "PM1_CNT: %08x\n", reg32); - if (((reg32 >> 10) & 7) == 5) { - u8 reg8; - reg8 = pci_read_config8(PCI_DEV(0, 0x1f, 0), 0xa2); + /* Check for S3 resume. */ + s3resume = southbridge_detect_s3_resume(); + if (s3resume) { + u8 reg8 = pci_read_config8(PCI_DEV(0, 0x1f, 0), 0xa2); printk(BIOS_DEBUG, "a2: %02x\n", reg8); if (!(reg8 & 0x20)) { - outl(reg32 & ~(7 << 10), DEFAULT_PMBASE + 0x04); printk(BIOS_DEBUG, "Bad resume from S3 detected.\n"); - } else { - if (acpi_s3_resume_allowed()) { - printk(BIOS_DEBUG, "Resume from S3 detected.\n"); - s3resume = 1; - } else { - printk(BIOS_DEBUG, - "Resume from S3 detected, but disabled.\n"); - } + s3resume = 0; } }
@@ -263,13 +254,5 @@
intel_early_me_status();
- if (s3resume) { - /* Clear SLP_TYPE. This will break stage2 but - * we care for that when we get there. - */ - reg32 = inl(DEFAULT_PMBASE + 0x04); - outl(reg32 & ~(7 << 10), DEFAULT_PMBASE + 0x04); - } - romstage_handoff_init(s3resume); } diff --git a/src/mainboard/packardbell/ms2290/romstage.c b/src/mainboard/packardbell/ms2290/romstage.c index 8442dfa..854c835 100644 --- a/src/mainboard/packardbell/ms2290/romstage.c +++ b/src/mainboard/packardbell/ms2290/romstage.c @@ -34,6 +34,7 @@ #include "arch/early_variables.h" #include <southbridge/intel/ibexpeak/pch.h> #include <northbridge/intel/nehalem/nehalem.h> +#include <southbridge/intel/common/pmclib.h>
#include <northbridge/intel/nehalem/raminit.h> #include <southbridge/intel/ibexpeak/me.h> @@ -206,24 +207,14 @@ /* Halt if there was a built in self test failure */ report_bist_failure(bist);
- /* Read PM1_CNT */ - reg32 = inl(DEFAULT_PMBASE + 0x04); - printk(BIOS_DEBUG, "PM1_CNT: %08x\n", reg32); - if (((reg32 >> 10) & 7) == 5) { - u8 reg8; - reg8 = pci_read_config8(PCI_DEV(0, 0x1f, 0), 0xa2); + /* Check for S3 resume. */ + s3resume = southbridge_detect_s3_resume(); + if (s3resume) { + u8 reg8 = pci_read_config8(PCI_DEV(0, 0x1f, 0), 0xa2); printk(BIOS_DEBUG, "a2: %02x\n", reg8); if (!(reg8 & 0x20)) { - outl(reg32 & ~(7 << 10), DEFAULT_PMBASE + 0x04); printk(BIOS_DEBUG, "Bad resume from S3 detected.\n"); - } else { - if (acpi_s3_resume_allowed()) { - printk(BIOS_DEBUG, "Resume from S3 detected.\n"); - s3resume = 1; - } else { - printk(BIOS_DEBUG, - "Resume from S3 detected, but disabled.\n"); - } + s3resume = 0; } }
@@ -255,13 +246,5 @@
intel_early_me_status();
- if (s3resume) { - /* Clear SLP_TYPE. This will break stage2 but - * we care for that when we get there. - */ - reg32 = inl(DEFAULT_PMBASE + 0x04); - outl(reg32 & ~(7 << 10), DEFAULT_PMBASE + 0x04); - } - romstage_handoff_init(s3resume); } diff --git a/src/southbridge/intel/ibexpeak/Kconfig b/src/southbridge/intel/ibexpeak/Kconfig index bb6e22c..59b012e 100644 --- a/src/southbridge/intel/ibexpeak/Kconfig +++ b/src/southbridge/intel/ibexpeak/Kconfig @@ -32,6 +32,7 @@ select SOUTHBRIDGE_INTEL_COMMON_RCBA_PIRQ select SOUTHBRIDGE_INTEL_COMMON_SMBUS select SOUTHBRIDGE_INTEL_COMMON_SPI + select SOUTHBRIDGE_INTEL_COMMON_PMCLIB select HAVE_USBDEBUG_OPTIONS select COMMON_FADT select ACPI_SATA_GENERATOR
Hello Kyösti Mälkki, Alexander Couzens, Patrick Rudolph, Arthur Heymans, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32316
to look at the new patch set (#2).
Change subject: sb/intel/ibexpeak: Use SOUTHBRIDGE_INTEL_COMMON_PMCLIB ......................................................................
sb/intel/ibexpeak: Use SOUTHBRIDGE_INTEL_COMMON_PMCLIB
Use common code to detect ACPI S3. Untested.
Change-Id: I766a3f4206e6605e6b7bb9a1ff0b2deb6721eb77 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/mainboard/lenovo/x201/romstage.c M src/mainboard/packardbell/ms2290/romstage.c M src/southbridge/intel/ibexpeak/Kconfig 3 files changed, 15 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/32316/2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32316 )
Change subject: sb/intel/ibexpeak: Use SOUTHBRIDGE_INTEL_COMMON_PMCLIB ......................................................................
Patch Set 2: Code-Review-1
(1 comment)
https://review.coreboot.org/#/c/32316/2/src/mainboard/lenovo/x201/romstage.c File src/mainboard/lenovo/x201/romstage.c:
https://review.coreboot.org/#/c/32316/2/src/mainboard/lenovo/x201/romstage.c... PS2, Line 266: : : : : : : raminit needs this, which is why it was cleared so late.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32316 )
Change subject: sb/intel/ibexpeak: Use SOUTHBRIDGE_INTEL_COMMON_PMCLIB ......................................................................
Patch Set 2: Code-Review-2
(1 comment)
https://review.coreboot.org/#/c/32316/2/src/mainboard/lenovo/x201/romstage.c File src/mainboard/lenovo/x201/romstage.c:
https://review.coreboot.org/#/c/32316/2/src/mainboard/lenovo/x201/romstage.c... PS2, Line 266: : : : : : :
raminit needs this, which is why it was cleared so late.
SLP_TYPE really needs to be cleared after intel_early_me_status is run. If done before it hangs during the raminit on a non-trivial place. So I guess the common code code cannot use at all...
Probably worth it to document it here, for future reference.
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/32316?usp=email )
Change subject: sb/intel/ibexpeak: Use SOUTHBRIDGE_INTEL_COMMON_PMCLIB ......................................................................
Abandoned
-
Arthur Heymans (Code Review) -
Patrick Rudolph (Code Review)
-
Stefan Reinauer (Code Review)