2021-04-01 21:05:29

by Evan Green

[permalink] [raw]
Subject: [PATCH] firmware: google: Enable s0ix logging by default

Many moons ago, support was added to the SMI handlers to log s0ix entry
and exit. Early iterations of firmware on Apollo Lake correctly returned
"unsupported" for this new command they did not recognize, but
unfortunately also contained a quirk where this command would cause them
to power down rather than resume from s0ix.

Fixes for this quirk were pushed out long ago, so all APL devices still
in the field should have updated firmware. As such, we no longer need to
have the s0ix_logging_enable be opt-in, where every new platform has to
add this to their kernel commandline parameters. Change it to be on by
default.

In theory we could remove the parameter altogether: updated versions of
Chrome OS containing a kernel with this change would also be coupled
with firmware that behaves properly with these commands. Eventually we
should probably do that. For now, convert this to an opt-out parameter
so there's an emergency valve for people who are deliberately running
old firmware, or as an escape hatch in case of unforeseen regressions.

Signed-off-by: Evan Green <[email protected]>
---

drivers/firmware/google/gsmi.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/google/gsmi.c b/drivers/firmware/google/gsmi.c
index 3d77f26c1e8c93..bb6e77ee3898c1 100644
--- a/drivers/firmware/google/gsmi.c
+++ b/drivers/firmware/google/gsmi.c
@@ -136,12 +136,16 @@ MODULE_PARM_DESC(spincount,
"The number of loop iterations to use when using the spin handshake.");

/*
- * Platforms might not support S0ix logging in their GSMI handlers. In order to
- * avoid any side-effects of generating an SMI for S0ix logging, use the S0ix
- * related GSMI commands only for those platforms that explicitly enable this
- * option.
+ * Some older platforms with Apollo Lake chipsets do not support S0ix logging
+ * in their GSMI handlers, and behaved poorly when resuming via power button
+ * press if the logging was attempted. Updated firmware with proper behavior
+ * has long since shipped, removing the need for this opt-in parameter. It
+ * now exists as an opt-out parameter for folks defiantly running old
+ * firmware, or unforeseen circumstances. After the change from opt-in to
+ * opt-out has baked sufficiently, this parameter should probably be removed
+ * entirely.
*/
-static bool s0ix_logging_enable;
+static bool s0ix_logging_enable = true;
module_param(s0ix_logging_enable, bool, 0600);

static struct gsmi_buf *gsmi_buf_alloc(void)
--
2.29.2