| Message ID | 00b4445d67d2e56fef8c319c2651f655d7e6d8e9.1502432974.git.lv.zheng@intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
On Friday, August 11, 2017 8:36:28 AM CEST Lv Zheng wrote: > 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> > Tested-by: Tomislav Ivek <tomislav.ivek@gmail.com> Applied, thanks! I'm not sure about the rest of the series, though, but let me comment on the specific patches. Thanks, Rafael -- 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
Hi, > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net] > Subject: Re: [PATCH v3 1/4] ACPI / EC: Cleanup EC GPE mask flag > > On Friday, August 11, 2017 8:36:28 AM CEST Lv Zheng wrote: > > 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> > > Tested-by: Tomislav Ivek <tomislav.ivek@gmail.com> > > Applied, thanks! Thanks! > > I'm not sure about the rest of the series, though, but let me comment on the > specific patches. Though it is a long bug fix story, it's actually simple and can be summarized with 1 line: The patchset polls EC IRQs during noirq stage to fix EC event handling issues. I've submitted this solution several years ago using an IRQ polling kernel thread. You commented me to use a timer instead, here you are. During these years, I tried different solutions than the "polling IRQ in noirq stage". But they didn't work perfectly. The noirq stage makes the EC event handling issue fixes mutual exclusive. Fixing one issue can trigger regression for the other. And bug reports prove that we must handle EC events during noirq stage. So finally I picked the IRQ polling solution back, and refreshed it to follow your comment (using a timer instead of a kthread). If you apply this patch and enable EC debugging log with "dyndbg='file ec.c +p'". You should be able to see some event handling logs in noirq stages. Without this solution applied, EC event handling log is silent during noirq stages. That could be the only difference. Thanks and best regards Lv -- 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
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 62068a5..d338f40 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 */ @@ -425,19 +424,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"); } @@ -464,7 +463,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)) { @@ -480,7 +479,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) @@ -700,7 +699,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: @@ -798,7 +797,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) */