On Friday, February 1, 2019 7:13:41 AM CET Zhang Rui wrote:
> On some Samsung hardware, it is necessary to clear events accumulated by
> the EC during sleep. These ECs stop reporting GPEs until they are manually
> polled, if too many events are accumulated.
> Thus the CLEAR_ON_RESUME quirk is introduced to send EC query commands
> unconditionally after resume to clear all the EC query events on those
> platforms.
>
> Later, commit 4c237371f290 ("ACPI / EC: Remove old CLEAR_ON_RESUME quirk")
> removes the CLEAR_ON_RESUME quirk because we thought the new EC IRQ
> polling logic should handle this case.
>
> Now it has been proved that the EC IRQ Polling logic does not fix the
> issue actually because we got regression report on these Samsung
> platforms after removing the quirk.
>
> Thus revert commit 4c237371f290 ("ACPI / EC: Remove old CLEAR_ON_RESUME
> quirk") to introduce back the Samsung quirk in this patch.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=44161
> Tested-by: Ortwin Glück <odi@xxxxxx>
> Tested-by: Francisco Cribari <cribari@xxxxxxxxx>
> Tested-by: Balazs Varga <balazs4web@xxxxxxxxx>
> Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx>
> ---
> drivers/acpi/ec.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index 9d66a47..49e16f0 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -194,6 +194,7 @@ static struct workqueue_struct *ec_query_wq;
> static int EC_FLAGS_QUERY_HANDSHAKE; /* Needs QR_EC issued when SCI_EVT set */
> static int EC_FLAGS_CORRECT_ECDT; /* Needs ECDT port address correction */
> static int EC_FLAGS_IGNORE_DSDT_GPE; /* Needs ECDT GPE as correction setting */
> +static int EC_FLAGS_CLEAR_ON_RESUME; /* Needs acpi_ec_clear() on boot/resume */
>
> /* --------------------------------------------------------------------------
> * Logging/Debugging
> @@ -499,6 +500,26 @@ static inline void __acpi_ec_disable_event(struct acpi_ec *ec)
> ec_log_drv("event blocked");
> }
>
> +/*
> + * Process _Q events that might have accumulated in the EC.
> + * Run with locked ec mutex.
> + */
> +static void acpi_ec_clear(struct acpi_ec *ec)
> +{
> + int i, status;
> + u8 value = 0;
> +
> + for (i = 0; i < ACPI_EC_CLEAR_MAX; i++) {
> + status = acpi_ec_query(ec, &value);
> + if (status || !value)
> + break;
> + }
> + if (unlikely(i == ACPI_EC_CLEAR_MAX))
> + pr_warn("Warning: Maximum of %d stale EC events cleared\n", i);
> + else
> + pr_info("%d stale EC events cleared\n", i);
> +}
> +
> static void acpi_ec_enable_event(struct acpi_ec *ec)
> {
> unsigned long flags;
> @@ -507,6 +528,10 @@ static void acpi_ec_enable_event(struct acpi_ec *ec)
> if (acpi_ec_started(ec))
> __acpi_ec_enable_event(ec);
> spin_unlock_irqrestore(&ec->lock, flags);
> +
> + /* Drain additional events if hardware requires that */
> + if (EC_FLAGS_CLEAR_ON_RESUME)
> + acpi_ec_clear(ec);
> }
>
> #ifdef CONFIG_PM_SLEEP
> @@ -1821,6 +1846,31 @@ static int ec_flag_query_handshake(const struct dmi_system_id *id)
> #endif
>
> /*
> + * On some hardware it is necessary to clear events accumulated by the EC during
> + * sleep. These ECs stop reporting GPEs until they are manually polled, if too
> + * many events are accumulated. (e.g. Samsung Series 5/9 notebooks)
> + *
> + * https://bugzilla.kernel.org/show_bug.cgi?id=44161
> + *
> + * Ideally, the EC should also be instructed NOT to accumulate events during
> + * sleep (which Windows seems to do somehow), but the interface to control this
> + * behaviour is not known at this time.
> + *
> + * Models known to be affected are Samsung 530Uxx/535Uxx/540Uxx/550Pxx/900Xxx,
> + * however it is very likely that other Samsung models are affected.
> + *
> + * On systems which don't accumulate _Q events during sleep, this extra check
> + * should be harmless.
> + */
> +static int ec_clear_on_resume(const struct dmi_system_id *id)
> +{
> + pr_debug("Detected system needing EC poll on resume.\n");
> + EC_FLAGS_CLEAR_ON_RESUME = 1;
> + ec_event_clearing = ACPI_EC_EVT_TIMING_STATUS;
> + return 0;
> +}
> +
> +/*
> * Some ECDTs contain wrong register addresses.
> * MSI MS-171F
> * https://bugzilla.kernel.org/show_bug.cgi?id=12461
> @@ -1869,6 +1919,9 @@ static const struct dmi_system_id ec_dmi_table[] __initconst = {
> ec_honor_ecdt_gpe, "ASUS X580VD", {
> DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> DMI_MATCH(DMI_PRODUCT_NAME, "X580VD"),}, NULL},
> + {
> + ec_clear_on_resume, "Samsung hardware", {
> + DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD.")}, NULL},
> {},
> };
>
>
Applied, thanks!
