| Message ID | 99f23db65bbe89ee856018629654584a96734c84.1501141963.git.lv.zheng@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
There is a known issue in EC event handling. This patchset tries to handle noirq stage EC event polling to fix this known issue. In the very early version, the EC event polling mechanism is implemented by a kernel thread to poll EC events. Now the mechanism is implemented by a timer ticked in noirq stage to poll EC GPEs. In this newest timer version, timer running period is shortened and thus is safer for s2idle mode. After adding such a mechanism, we can try to handle EC events earlier after resume, this may be able to solve some driver order issues. Lv Zheng (4): ACPI / EC: Cleanup EC GPE mask flag ACPI / EC: Add IRQ polling support for noirq stages ACPI / EC: Add support to handle EC events earlier ACPI / EC: Enable noirq stage GPE polling drivers/acpi/ec.c | 157 ++++++++++++++++++++++++++++++++++++++++++++---- drivers/acpi/internal.h | 1 + 2 files changed, 145 insertions(+), 13 deletions(-)
EC events are special, required to be handled during suspend/resume. But there is a problem preventing EC events from being detected during noirq stages. This patchset fixes this issue by polling EC IRQs timely during suspend/resume noirq stages. With this issue fixed, we should be able to handler EC events earlier during resume, and have great opportunities to fix some driver order issues caused by the deferred detected EC events. v3 of this patch series only contains patch description updates. Lv Zheng (4): ACPI / EC: Cleanup EC GPE mask flag ACPI / EC: Add IRQ polling support for noirq stages ACPI / EC: Add support to handle EC events earlier ACPI / EC: Enable noirq stage GPE polling drivers/acpi/ec.c | 157 ++++++++++++++++++++++++++++++++++++++++++++---- drivers/acpi/internal.h | 1 + 2 files changed, 145 insertions(+), 13 deletions(-)
EC events are special, required to be handled during suspend/resume. But
there are special logics in Linux causing several issues related to the EC
event handling:
1. During noirq stage, Linux cannot detect EC events which are target
driven.
2. When EC event handling is enabled later than the other drivers, order
problem could be observed.
When fixing these problems, care should be taken to not to trigger
regressions to the following problem which has alredy been fixed using
different approach):
3. When EC event handling is enabled before the end of the noirq stage,
EC event handling may stuck.
This patchset fixes these issues.
v4 of this patch series re-orders the fixes so that the fix of the problem
2 could be independent against the fix of the problem 1, this is done by
refining the fix of the problem 2, making it immune to the problem 3.
Lv Zheng (3):
ACPI / EC: Fix possible driver order issue by moving EC event handling
earlier
ACPI / EC: Add event detection support for noirq stages
ACPI / EC: Enable noirq stage event detection
drivers/acpi/ec.c | 116 +++++++++++++++++++++++++++++++++++++++++++++---
drivers/acpi/internal.h | 1 +
2 files changed, 111 insertions(+), 6 deletions(-)
Hi, Rafael I'm now working on v5 of this series. Which 1. splits root causes in a more detailed way, 2. clarifies root causes in patch description with real bugs, and 3. is safer according to the known EC FW behaviors. So you can discard v3/v4 from the patchwork site. And I'll post v5 when everything is cleared to me. Thanks and best regards Lv > From: Zheng, Lv > Subject: [PATCH v4 0/3] ACPI / EC: Fix EC event handling issues > > EC events are special, required to be handled during suspend/resume. But > there are special logics in Linux causing several issues related to the EC > event handling: > 1. During noirq stage, Linux cannot detect EC events which are target > driven. > 2. When EC event handling is enabled later than the other drivers, order > problem could be observed. > When fixing these problems, care should be taken to not to trigger > regressions to the following problem which has alredy been fixed using > different approach): > 3. When EC event handling is enabled before the end of the noirq stage, > EC event handling may stuck. > This patchset fixes these issues. > > v4 of this patch series re-orders the fixes so that the fix of the problem > 2 could be independent against the fix of the problem 1, this is done by > refining the fix of the problem 2, making it immune to the problem 3. > > Lv Zheng (3): > ACPI / EC: Fix possible driver order issue by moving EC event handling > earlier > ACPI / EC: Add event detection support for noirq stages > ACPI / EC: Enable noirq stage event detection > > drivers/acpi/ec.c | 116 +++++++++++++++++++++++++++++++++++++++++++++--- > drivers/acpi/internal.h | 1 + > 2 files changed, 111 insertions(+), 6 deletions(-) > > -- > 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
If EC events occurred during BIOS S3-exit and early OS S3-exit steps can
be detected by OS earlier, then there can be less driver order issues
between acpi_ec_resume() and some other drivers' .resume() hook (e.x.
acpi_button_resume()).
However there are known facts that EC FW does drop EC events during S3,
and it takes time for EC FW to initialize (maximum 1.4 observed) while
Windows acts normally, so detecting EC event earlier might just be a
workaround for other drivers (they should be aware of this order issue and
deal with it themselves). As such, this patchset is marked as an RFC.
If Linux EC driver started to detect events during early OS S3-exit, it
need to timely poll EC events during noirq stages as in this stage there is
no EC event triggering source.
This patchset implements earlier EC event handling for Linux.
Lv Zheng (3):
ACPI / EC: Fix possible driver order issue by moving EC event handling
earlier
ACPI / EC: Add event detection support for noirq stages
ACPI / EC: Enable noirq stage event detection
drivers/acpi/ec.c | 128 +++++++++++++++++++++++++++++++++++++++++++-----
drivers/acpi/internal.h | 1 +
2 files changed, 118 insertions(+), 11 deletions(-)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index ddb01e9..54879c7 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -112,8 +112,7 @@ enum { EC_FLAGS_EVT_HANDLER_INSTALLED, /* _Qxx handlers installed */ EC_FLAGS_STARTED, /* Driver is started */ EC_FLAGS_STOPPED, /* Driver is stopped */ - EC_FLAGS_COMMAND_STORM, /* GPE storms occurred to the - * current command processing */ + EC_FLAGS_GPE_MASKED, /* GPE masked */ }; #define ACPI_EC_COMMAND_POLL 0x01 /* Available for command byte */ @@ -421,19 +420,19 @@ static void acpi_ec_complete_request(struct acpi_ec *ec) wake_up(&ec->wait); } -static void acpi_ec_set_storm(struct acpi_ec *ec, u8 flag) +static void acpi_ec_mask_gpe(struct acpi_ec *ec) { - if (!test_bit(flag, &ec->flags)) { + if (!test_bit(EC_FLAGS_GPE_MASKED, &ec->flags)) { acpi_ec_disable_gpe(ec, false); ec_dbg_drv("Polling enabled"); - set_bit(flag, &ec->flags); + set_bit(EC_FLAGS_GPE_MASKED, &ec->flags); } } -static void acpi_ec_clear_storm(struct acpi_ec *ec, u8 flag) +static void acpi_ec_unmask_gpe(struct acpi_ec *ec) { - if (test_bit(flag, &ec->flags)) { - clear_bit(flag, &ec->flags); + if (test_bit(EC_FLAGS_GPE_MASKED, &ec->flags)) { + clear_bit(EC_FLAGS_GPE_MASKED, &ec->flags); acpi_ec_enable_gpe(ec, false); ec_dbg_drv("Polling disabled"); } @@ -460,7 +459,7 @@ static bool acpi_ec_submit_flushable_request(struct acpi_ec *ec) static void acpi_ec_submit_query(struct acpi_ec *ec) { - acpi_ec_set_storm(ec, EC_FLAGS_COMMAND_STORM); + acpi_ec_mask_gpe(ec); if (!acpi_ec_event_enabled(ec)) return; if (!test_and_set_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) { @@ -476,7 +475,7 @@ static void acpi_ec_complete_query(struct acpi_ec *ec) if (test_and_clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) ec_dbg_evt("Command(%s) unblocked", acpi_ec_cmd_string(ACPI_EC_COMMAND_QUERY)); - acpi_ec_clear_storm(ec, EC_FLAGS_COMMAND_STORM); + acpi_ec_unmask_gpe(ec); } static inline void __acpi_ec_enable_event(struct acpi_ec *ec) @@ -688,7 +687,7 @@ static void advance_transaction(struct acpi_ec *ec) ++t->irq_count; /* Allow triggering on 0 threshold */ if (t->irq_count == ec_storm_threshold) - acpi_ec_set_storm(ec, EC_FLAGS_COMMAND_STORM); + acpi_ec_mask_gpe(ec); } } out: @@ -786,7 +785,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, spin_lock_irqsave(&ec->lock, tmp); if (t->irq_count == ec_storm_threshold) - acpi_ec_clear_storm(ec, EC_FLAGS_COMMAND_STORM); + acpi_ec_unmask_gpe(ec); ec_dbg_req("Command(%s) stopped", acpi_ec_cmd_string(t->command)); ec->curr = NULL; /* Disable GPE for command processing (IBF=0/OBF=1) */
EC_FLAGS_COMMAND_STORM is actually used to mask GPE during IRQ processing. This patch cleans it up using more readable flag/function names. Signed-off-by: Lv Zheng <lv.zheng@intel.com> --- drivers/acpi/ec.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)