<p>Furquan Shaikh has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/22033">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">soc/intel/common/block/pmc: Move pmc_disable_all_gpe to romstage<br><br>Instead of disabling all GPEs during PMC init in bootblock, this<br>change moves it to pmc_fill_power_state which allows romstage to<br>correctly fill up GPE_EN registers in chipset_power_state. This is<br>essential for correctly identifying the wake source.<br><br>Disabling all GPEs was added recently in change 74145f76<br>(intel/common/pmc: Disable all GPEs during pmc_init) because keeping<br>GPEs enabled in coreboot while enabling SMI could lead to<br>side-effects as explained in the change. Moving pmc_disable_all_gpe to<br>pmc_fill_power_state should be safe as that happens before SMI is<br>enabled in coreboot.<br><br>TEST=Verified that GPE-based wake source is correctly<br>identified. Also, no issues observed while resuming from S3.<br><br>Change-Id: I8e992ad09ffdefba62de11fa572e783715776bf1<br>Signed-off-by: Furquan Shaikh <furquan@chromium.org><br>---<br>M src/soc/intel/common/block/include/intelblocks/pmclib.h<br>M src/soc/intel/common/block/pmc/pmclib.c<br>2 files changed, 5 insertions(+), 3 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/22033/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/src/soc/intel/common/block/include/intelblocks/pmclib.h b/src/soc/intel/common/block/include/intelblocks/pmclib.h<br>index 9dbac24..89c3cdc 100644<br>--- a/src/soc/intel/common/block/include/intelblocks/pmclib.h<br>+++ b/src/soc/intel/common/block/include/intelblocks/pmclib.h<br>@@ -149,7 +149,9 @@<br> <br> /*<br>  * Reads and prints ACPI specific PM registers which are common across<br>- * chipsets. Returns the previous sleep state which is one of ACPI_SX values.<br>+ * chipsets. Returns the previous sleep state which is one of ACPI_SX<br>+ * values. Additionally, it also disables all GPEs after GPE_EN<br>+ * registers are read.<br>  */<br> int pmc_fill_power_state(struct chipset_power_state *ps);<br> <br>diff --git a/src/soc/intel/common/block/pmc/pmclib.c b/src/soc/intel/common/block/pmc/pmclib.c<br>index b8ec17d..e78eb79 100644<br>--- a/src/soc/intel/common/block/pmc/pmclib.c<br>+++ b/src/soc/intel/common/block/pmc/pmclib.c<br>@@ -410,6 +410,8 @@<br>       ps->prev_sleep_state = pmc_prev_sleep_state(ps);<br>   printk(BIOS_DEBUG, "prev_sleep_state %d\n", ps->prev_sleep_state);<br> <br>+   pmc_disable_all_gpe();<br>+<br>     return ps->prev_sleep_state;<br> }<br> <br>@@ -558,6 +560,4 @@<br> <br>        /* Set the routes in the GPIO communities as well. */<br>         gpio_route_gpe(dw0, dw1, dw2);<br>-<br>-    pmc_disable_all_gpe();<br> }<br></pre><p>To view, visit <a href="https://review.coreboot.org/22033">change 22033</a>. To unsubscribe, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/22033"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I8e992ad09ffdefba62de11fa572e783715776bf1 </div>
<div style="display:none"> Gerrit-Change-Number: 22033 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Furquan Shaikh <furquan@google.com> </div>