Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41987 )
Change subject: [WIP] sb,soc/intel: Consolidate set_acpi_mode() ......................................................................
[WIP] sb,soc/intel: Consolidate set_acpi_mode()
Change-Id: I50bdce6a8e9043b209c5558b5ab613f945b4b787 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/x86/smm/smi_trigger.c M src/include/cpu/x86/smm.h M src/soc/intel/apollolake/pmc.c M src/soc/intel/broadwell/lpc.c M src/soc/intel/cannonlake/pmc.c M src/soc/intel/common/block/include/intelblocks/pmc.h M src/soc/intel/common/block/pmc/pmc.c M src/soc/intel/denverton_ns/pmc.c M src/soc/intel/icelake/pmc.c M src/soc/intel/jasperlake/pmc.c M src/soc/intel/skylake/pmc.c M src/soc/intel/tigerlake/pmc.c M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/i82801gx/lpc.c M src/southbridge/intel/i82801ix/lpc.c M src/southbridge/intel/i82801jx/lpc.c M src/southbridge/intel/ibexpeak/lpc.c M src/southbridge/intel/lynxpoint/lpc.c 18 files changed, 26 insertions(+), 84 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/41987/1
diff --git a/src/cpu/x86/smm/smi_trigger.c b/src/cpu/x86/smm/smi_trigger.c index f0a968d..586d3ce 100644 --- a/src/cpu/x86/smm/smi_trigger.c +++ b/src/cpu/x86/smm/smi_trigger.c @@ -4,6 +4,15 @@ #include <console/console.h> #include <cpu/x86/smm.h>
+void set_acpi_mode_on_exit(void) +{ + if (acpi_disable_sci_on_exit()) { + apm_control(APM_CNT_ACPI_DISABLE); + } else { + apm_control(APM_CNT_ACPI_ENABLE); + } +} + int apm_control(u8 cmd) { if (!CONFIG(HAVE_SMI_HANDLER)) diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h index 4fdf58f..d71414f 100644 --- a/src/include/cpu/x86/smm.h +++ b/src/include/cpu/x86/smm.h @@ -31,6 +31,7 @@
/* Send cmd to APM_CNT with HAVE_SMI_HANDLER checking. */ int apm_control(u8 cmd); +void set_acpi_mode_on_exit(void);
void io_trap_handler(int smif); int southbridge_io_trap_handler(int smif); diff --git a/src/soc/intel/apollolake/pmc.c b/src/soc/intel/apollolake/pmc.c index 2eb8f11..0a4dcfb 100644 --- a/src/soc/intel/apollolake/pmc.c +++ b/src/soc/intel/apollolake/pmc.c @@ -96,7 +96,7 @@
/* Set up GPE configuration */ pmc_gpe_init(); - pmc_set_acpi_mode(); + set_acpi_mode_on_exit();
if (cfg != NULL) set_slp_s3_assertion_width(cfg->slp_s3_assertion_width_usecs); diff --git a/src/soc/intel/broadwell/lpc.c b/src/soc/intel/broadwell/lpc.c index b841291..1dbd4dd 100644 --- a/src/soc/intel/broadwell/lpc.c +++ b/src/soc/intel/broadwell/lpc.c @@ -407,13 +407,6 @@ pch_iobp_update(0xCE00C000, ~1UL, 0x00000000); }
-static void pch_set_acpi_mode(void) -{ - if (!acpi_is_wakeup_s3()) { - apm_control(APM_CNT_ACPI_DISABLE); - } -} - static void lpc_init(struct device *dev) { /* Legacy initialization */ @@ -433,7 +426,7 @@ pch_pm_init(dev); pch_cg_init(dev);
- pch_set_acpi_mode(); + set_acpi_mode_on_exit(); }
static void pch_lpc_add_mmio_resources(struct device *dev) diff --git a/src/soc/intel/cannonlake/pmc.c b/src/soc/intel/cannonlake/pmc.c index e1b56e9..c2971bc 100644 --- a/src/soc/intel/cannonlake/pmc.c +++ b/src/soc/intel/cannonlake/pmc.c @@ -112,7 +112,7 @@ { /* * PMC initialization happens earlier for this SoC because FSP-Silicon - * init hides PMC from PCI bus. However, pmc_set_acpi_mode, which + * init hides PMC from PCI bus. However, set_acpi_mode_on_exit, which * disables ACPI mode doesn't need to happen that early and can be * delayed till typical BS_DEV_INIT. This ensures that ACPI mode * disabling happens the same way for all SoCs and hence the ordering of @@ -127,7 +127,7 @@ * hidden and hence the PMC driver never gets enumerated and so init is * not called for it. */ - pmc_set_acpi_mode(); + set_acpi_mode_on_exit(); }
BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_EXIT, soc_acpi_mode_init, NULL); diff --git a/src/soc/intel/common/block/include/intelblocks/pmc.h b/src/soc/intel/common/block/include/intelblocks/pmc.h index 75e2127..7625a63 100644 --- a/src/soc/intel/common/block/include/intelblocks/pmc.h +++ b/src/soc/intel/common/block/include/intelblocks/pmc.h @@ -49,7 +49,7 @@ int pmc_soc_get_resources(struct pmc_resource_config *cfg);
/* API to set ACPI mode */ -void pmc_set_acpi_mode(void); +void set_acpi_mode_on_exit(void);
/* * Returns a reference to the PMC MUX device for the given port number. diff --git a/src/soc/intel/common/block/pmc/pmc.c b/src/soc/intel/common/block/pmc/pmc.c index 7d25493..7979bfd 100644 --- a/src/soc/intel/common/block/pmc/pmc.c +++ b/src/soc/intel/common/block/pmc/pmc.c @@ -91,13 +91,6 @@ pch_pmc_add_io_resources(dev, config); }
-void pmc_set_acpi_mode(void) -{ - if (acpi_disable_sci_on_exit()) { - apm_control(APM_CNT_ACPI_DISABLE); - } -} - static struct device_operations device_ops = { .read_resources = pch_pmc_read_resources, .set_resources = pci_dev_set_resources, diff --git a/src/soc/intel/denverton_ns/pmc.c b/src/soc/intel/denverton_ns/pmc.c index 3d84a0b..5a79764 100644 --- a/src/soc/intel/denverton_ns/pmc.c +++ b/src/soc/intel/denverton_ns/pmc.c @@ -22,13 +22,6 @@
static void pch_power_options(struct device *dev) { /* TODO */ }
-static void pch_set_acpi_mode(void) -{ - if (acpi_disable_sci_on_exit()) { - apm_control(APM_CNT_ACPI_DISABLE); - } -} - static void pmc_init(struct device *dev) { printk(BIOS_DEBUG, "pch: pmc_init\n"); @@ -46,7 +39,7 @@ pch_power_options(dev);
/* Configure ACPI mode. */ - pch_set_acpi_mode(); + set_acpi_mode_on_exit(); }
static void pci_pmc_read_resources(struct device *dev) diff --git a/src/soc/intel/icelake/pmc.c b/src/soc/intel/icelake/pmc.c index 5bd4438..d654c32 100644 --- a/src/soc/intel/icelake/pmc.c +++ b/src/soc/intel/icelake/pmc.c @@ -78,7 +78,7 @@ pmc_set_power_failure_state(true); pmc_gpe_init();
- pmc_set_acpi_mode(); + set_acpi_mode_on_exit();
config_deep_s3(config->deep_s3_enable_ac, config->deep_s3_enable_dc); config_deep_s5(config->deep_s5_enable_ac, config->deep_s5_enable_dc); diff --git a/src/soc/intel/jasperlake/pmc.c b/src/soc/intel/jasperlake/pmc.c index 5bd4438..d654c32 100644 --- a/src/soc/intel/jasperlake/pmc.c +++ b/src/soc/intel/jasperlake/pmc.c @@ -78,7 +78,7 @@ pmc_set_power_failure_state(true); pmc_gpe_init();
- pmc_set_acpi_mode(); + set_acpi_mode_on_exit();
config_deep_s3(config->deep_s3_enable_ac, config->deep_s3_enable_dc); config_deep_s5(config->deep_s5_enable_ac, config->deep_s5_enable_dc); diff --git a/src/soc/intel/skylake/pmc.c b/src/soc/intel/skylake/pmc.c index 14e9d4c..92a67d0 100644 --- a/src/soc/intel/skylake/pmc.c +++ b/src/soc/intel/skylake/pmc.c @@ -140,7 +140,7 @@ /* Note that certain bits may be cleared from running script as * certain bit fields are write 1 to clear. */ reg_script_run_on_dev(dev, pch_pmc_misc_init_script); - pmc_set_acpi_mode(); + set_acpi_mode_on_exit();
config_deep_s3(config->deep_s3_enable_ac, config->deep_s3_enable_dc); config_deep_s5(config->deep_s5_enable_ac, config->deep_s5_enable_dc); diff --git a/src/soc/intel/tigerlake/pmc.c b/src/soc/intel/tigerlake/pmc.c index 84a18e3..9c3c6a4 100644 --- a/src/soc/intel/tigerlake/pmc.c +++ b/src/soc/intel/tigerlake/pmc.c @@ -78,7 +78,7 @@ pmc_set_power_failure_state(true); pmc_gpe_init();
- pmc_set_acpi_mode(); + set_acpi_mode_on_exit();
config_deep_s3(config->deep_s3_enable_ac, config->deep_s3_enable_dc); config_deep_s5(config->deep_s5_enable_ac, config->deep_s5_enable_dc); diff --git a/src/southbridge/intel/bd82x6x/lpc.c b/src/southbridge/intel/bd82x6x/lpc.c index c3e79a4..7bfde3f 100644 --- a/src/southbridge/intel/bd82x6x/lpc.c +++ b/src/southbridge/intel/bd82x6x/lpc.c @@ -406,13 +406,6 @@ RCBA32_OR(0x3564, 0x3); }
-static void pch_set_acpi_mode(void) -{ - if (acpi_disable_sci_on_exit()) { - apm_control(APM_CNT_ACPI_DISABLE); - } -} - static void pch_disable_smm_only_flashing(struct device *dev) { u8 reg8; @@ -578,7 +571,7 @@
pch_disable_smm_only_flashing(dev);
- pch_set_acpi_mode(); + set_acpi_mode_on_exit();
pch_fixups(dev);
diff --git a/src/southbridge/intel/i82801gx/lpc.c b/src/southbridge/intel/i82801gx/lpc.c index 42e856d..4231cc7 100644 --- a/src/southbridge/intel/i82801gx/lpc.c +++ b/src/southbridge/intel/i82801gx/lpc.c @@ -316,15 +316,6 @@ RCBA32(CG) = reg32; }
-static void i82801gx_set_acpi_mode(struct device *dev) -{ - if (acpi_disable_sci_on_exit()) { - apm_control(APM_CNT_ACPI_DISABLE); - } else { - apm_control(APM_CNT_ACPI_ENABLE); - } -} - #define SPIBASE 0x3020 static void i82801gx_spi_init(void) { @@ -389,7 +380,7 @@ /* Interrupt 9 should be level triggered (SCI) */ i8259_configure_irq_trigger(9, 1);
- i82801gx_set_acpi_mode(dev); + set_acpi_mode_on_exit();
i82801gx_spi_init();
diff --git a/src/southbridge/intel/i82801ix/lpc.c b/src/southbridge/intel/i82801ix/lpc.c index eeff94a..5e30a0e 100644 --- a/src/southbridge/intel/i82801ix/lpc.c +++ b/src/southbridge/intel/i82801ix/lpc.c @@ -350,15 +350,6 @@ RCBA32(0x38c0) |= 7; }
-static void i82801ix_set_acpi_mode(struct device *dev) -{ - if (acpi_disable_sci_on_exit()) { - apm_control(APM_CNT_ACPI_DISABLE); - } else { - apm_control(APM_CNT_ACPI_ENABLE); - } -} - static void lpc_init(struct device *dev) { printk(BIOS_DEBUG, "i82801ix: %s\n", __func__); @@ -399,7 +390,7 @@ /* Interrupt 9 should be level triggered (SCI) */ i8259_configure_irq_trigger(9, 1);
- i82801ix_set_acpi_mode(dev); + set_acpi_mode_on_exit();
/* Don't allow evil boot loaders, kernels, or * userspace applications to deceive us: diff --git a/src/southbridge/intel/i82801jx/lpc.c b/src/southbridge/intel/i82801jx/lpc.c index 5bb69e3..7fd0954 100644 --- a/src/southbridge/intel/i82801jx/lpc.c +++ b/src/southbridge/intel/i82801jx/lpc.c @@ -355,15 +355,6 @@ RCBA32(0x38c0) |= 7; }
-static void i82801jx_set_acpi_mode(struct device *dev) -{ - if (acpi_disable_sci_on_exit()) { - apm_control(APM_CNT_ACPI_DISABLE); - } else { - apm_control(APM_CNT_ACPI_ENABLE); - } -} - static void lpc_init(struct device *dev) { printk(BIOS_DEBUG, "i82801jx: %s\n", __func__); @@ -404,7 +395,7 @@ /* Interrupt 9 should be level triggered (SCI) */ i8259_configure_irq_trigger(9, 1);
- i82801jx_set_acpi_mode(dev); + set_acpi_mode_on_exit(); }
unsigned long acpi_fill_madt(unsigned long current) diff --git a/src/southbridge/intel/ibexpeak/lpc.c b/src/southbridge/intel/ibexpeak/lpc.c index 1a05eff..9a115f7 100644 --- a/src/southbridge/intel/ibexpeak/lpc.c +++ b/src/southbridge/intel/ibexpeak/lpc.c @@ -410,13 +410,6 @@ RCBA32_OR(0x3564, 0x3); }
-static void pch_set_acpi_mode(void) -{ - if (acpi_disable_sci_on_exit()) { - apm_control(APM_CNT_ACPI_DISABLE); - } -} - static void pch_disable_smm_only_flashing(struct device *dev) { u8 reg8; @@ -481,7 +474,7 @@
pch_disable_smm_only_flashing(dev);
- pch_set_acpi_mode(); + set_acpi_mode_on_exit();
pch_fixups(dev); } diff --git a/src/southbridge/intel/lynxpoint/lpc.c b/src/southbridge/intel/lynxpoint/lpc.c index 2d5b6f6..d3a066e 100644 --- a/src/southbridge/intel/lynxpoint/lpc.c +++ b/src/southbridge/intel/lynxpoint/lpc.c @@ -471,12 +471,6 @@ pch_iobp_update(0xCE00C000, ~1UL, 0x00000000); // bit0=0 in BWG 1.4.0 }
-static void pch_set_acpi_mode(void) -{ - if (acpi_disable_sci_on_exit()) - apm_control(APM_CNT_ACPI_DISABLE); -} - static void pch_disable_smm_only_flashing(struct device *dev) { u8 reg8; @@ -547,7 +541,7 @@
pch_disable_smm_only_flashing(dev);
- pch_set_acpi_mode(); + set_acpi_mode_on_exit();
pch_fixups(dev); }
Hello build bot (Jenkins), David Guckian, Vanessa Eusebio, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41987
to look at the new patch set (#2).
Change subject: [WIP] sb,soc/intel: Consolidate set_acpi_mode() ......................................................................
[WIP] sb,soc/intel: Consolidate set_acpi_mode()
Change-Id: I50bdce6a8e9043b209c5558b5ab613f945b4b787 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/x86/smm/smi_trigger.c M src/include/cpu/x86/smm.h M src/soc/intel/apollolake/pmc.c M src/soc/intel/broadwell/lpc.c M src/soc/intel/cannonlake/pmc.c M src/soc/intel/common/block/include/intelblocks/pmc.h M src/soc/intel/common/block/pmc/pmc.c M src/soc/intel/denverton_ns/pmc.c M src/soc/intel/icelake/pmc.c M src/soc/intel/jasperlake/pmc.c M src/soc/intel/skylake/pmc.c M src/soc/intel/tigerlake/pmc.c M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/i82801gx/lpc.c M src/southbridge/intel/i82801ix/lpc.c M src/southbridge/intel/i82801jx/lpc.c M src/southbridge/intel/ibexpeak/lpc.c M src/southbridge/intel/lynxpoint/lpc.c 18 files changed, 27 insertions(+), 86 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/41987/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41987 )
Change subject: [WIP] sb,soc/intel: Consolidate set_acpi_mode() ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41987/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41987/2//COMMIT_MSG@7 PS2, Line 7: [WIP] sb,soc/intel: Consolidate set_acpi_mode() Would be good to note that some chipsets only perform one of the two branches.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41987 )
Change subject: [WIP] sb,soc/intel: Consolidate set_acpi_mode() ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41987/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41987/2//COMMIT_MSG@7 PS2, Line 7: [WIP] sb,soc/intel: Consolidate set_acpi_mode()
Would be good to note that some chipsets only perform one of the two branches.
Currently pretty much nothing builds with HAVE_SMI_HANDLER=n. This calls for some discussion on what we actually need to do here.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41987 )
Change subject: [WIP] sb,soc/intel: Consolidate set_acpi_mode() ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41987/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41987/2//COMMIT_MSG@7 PS2, Line 7: [WIP] sb,soc/intel: Consolidate set_acpi_mode()
Currently pretty much nothing builds with HAVE_SMI_HANDLER=n. […]
Well, currently `acpi_disable_sci_on_exit()` is just CONFIG(HAVE_SMI_HANDLER), but comments suggest that it should also evaluate `!acpi_is_wakeup_s3()`. ICH platforms do this:
if (acpi_disable_sci_on_exit()) { apm_control(APM_CNT_ACPI_DISABLE); } else { apm_control(APM_CNT_ACPI_ENABLE); }
Whereas PCH platforms onwards do this:
if (acpi_disable_sci_on_exit()) { apm_control(APM_CNT_ACPI_DISABLE); }
That will be changed if factoring code out, so it would be good to at least mention this change in the commit message.
Hello build bot (Jenkins), David Guckian, Vanessa Eusebio, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41987
to look at the new patch set (#3).
Change subject: [WIP] sb,soc/intel: Consolidate set_acpi_mode() ......................................................................
[WIP] sb,soc/intel: Consolidate set_acpi_mode()
Change-Id: I50bdce6a8e9043b209c5558b5ab613f945b4b787 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/x86/smm/smi_trigger.c M src/include/cpu/x86/smm.h M src/soc/intel/apollolake/pmc.c M src/soc/intel/broadwell/lpc.c M src/soc/intel/cannonlake/pmc.c M src/soc/intel/common/block/include/intelblocks/pmc.h M src/soc/intel/common/block/pmc/pmc.c M src/soc/intel/denverton_ns/pmc.c M src/soc/intel/icelake/pmc.c M src/soc/intel/jasperlake/pmc.c M src/soc/intel/skylake/pmc.c M src/soc/intel/tigerlake/pmc.c M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/i82801gx/lpc.c M src/southbridge/intel/i82801ix/lpc.c M src/southbridge/intel/i82801jx/lpc.c M src/southbridge/intel/ibexpeak/lpc.c M src/southbridge/intel/lynxpoint/lpc.c 18 files changed, 33 insertions(+), 96 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/41987/3
Hello build bot (Jenkins), David Guckian, Vanessa Eusebio, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41987
to look at the new patch set (#4).
Change subject: [WIP] sb,soc/intel: Consolidate set_acpi_mode() ......................................................................
[WIP] sb,soc/intel: Consolidate set_acpi_mode()
Change-Id: I50bdce6a8e9043b209c5558b5ab613f945b4b787 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/x86/smm/smi_trigger.c M src/include/cpu/x86/smm.h M src/soc/intel/apollolake/pmc.c M src/soc/intel/broadwell/lpc.c M src/soc/intel/cannonlake/pmc.c M src/soc/intel/common/block/include/intelblocks/pmc.h M src/soc/intel/common/block/pmc/pmc.c M src/soc/intel/denverton_ns/pmc.c M src/soc/intel/icelake/pmc.c M src/soc/intel/jasperlake/pmc.c M src/soc/intel/skylake/pmc.c M src/soc/intel/tigerlake/pmc.c M src/southbridge/intel/bd82x6x/lpc.c M src/southbridge/intel/i82801gx/lpc.c M src/southbridge/intel/i82801ix/lpc.c M src/southbridge/intel/i82801jx/lpc.c M src/southbridge/intel/ibexpeak/lpc.c M src/southbridge/intel/lynxpoint/lpc.c 18 files changed, 34 insertions(+), 96 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/41987/4
Kyösti Mälkki has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/41987 )
Change subject: sb,soc/intel: Consolidate set_acpi_mode() ......................................................................
Abandoned
-
Angel Pons (Code Review) -
Kyösti Mälkki (Code Review)