Re: [PATCH] ACPI / EC: Clear stale EC events on Samsung systems

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Juan Manuel Cabo wrote:

that's really nasty EC firmware!
Yes!
And this bug has been unsolved for about two years.

20 is enough?
the query index is length of a byte.

According to our humble tests, 8 is the maximum number of
accumulated events. For instance, if the one plugs or unplugs the PSU
16 times, (or battery % changes 16 times) during S3 sleep, then the
EC returns no more than 8 events when polled by this patch or by
my userspace util.
And having reached 8 events, it won't produce its GPE until queried.

that surely indicts their EC firmware only queued 8 events,

the query index is length of a byte.
There is no query index, unless you refer to something else (I'm sorry if its
something that escapes me).

oh, sorry, I'm referring internal EC firmware code
for Q event queuing, not ACPI SPEC, ;-)
for machine you tested, 8 is the queue size,
but for some unknown also nasty EC firmwares(let's suppose it exists),
it may queue more Q events.
and I saw several firmwares queued 32 events by default,
then, let's say, they be used for some samsung products,
and also they also forgot to deal with sleep/resume state,
then, we'll also leave stale Q event there.

Thanks!

     For us, a query is just: send 0x84 through EC CMD port, and read status
from CMD port and event type from EC DATA port. This is done with
the usual ec.c functions that would handle a query after a GPE interrupt,
but using them instead to poll (not GPE initiated) at awake. The EC would
then return status without 0x20 mask and 'event type'==0 when no more left.

--
Juan Manuel Cabo<juanmanuel.cabo@xxxxxxxxx>



   enum {
       EC_FLAGS_QUERY_PENDING,        /* Query is pending */
@@ -116,6 +118,7 @@ EXPORT_SYMBOL(first_ec);
   static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */
   static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated */
   static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan */
+static int EC_FLAGS_CLEAR_ON_RESUME; /* EC should be polled on boot/resume */

seems name is implicit, what about EC_FLAGS_QEVENT_CLR_ON_RESUME?
seems too long :-)

   /* --------------------------------------------------------------------------
                                Transaction Management
@@ -440,6 +443,26 @@ acpi_handle ec_get_handle(void)

   EXPORT_SYMBOL(ec_get_handle);

+static int acpi_ec_query_unlocked(struct acpi_ec *ec, u8 *data);
+
+/* 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_unlocked(ec,&value);
+        if (status || !value)
+            break;
+    }
+
+    if (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);
+}
+
   void acpi_ec_block_transactions(void)
   {
       struct acpi_ec *ec = first_ec;
@@ -463,6 +486,10 @@ void acpi_ec_unblock_transactions(void)
       mutex_lock(&ec->mutex);
       /* Allow transactions to be carried out again */
       clear_bit(EC_FLAGS_BLOCKED,&ec->flags);
+
+    if (EC_FLAGS_CLEAR_ON_RESUME)
+        acpi_ec_clear(ec);
+
       mutex_unlock(&ec->mutex);
   }

@@ -821,6 +848,13 @@ static int acpi_ec_add(struct acpi_device *device)

       /* EC is fully operational, allow queries */
       clear_bit(EC_FLAGS_QUERY_PENDING,&ec->flags);
+
+    /* Some hardware may need the EC to be cleared before use */

description is implicit, should specify what we clear is Q event, not EC.

Thanks!
Li Guang

+    if (EC_FLAGS_CLEAR_ON_RESUME) {
+        mutex_lock(&ec->mutex);
+        acpi_ec_clear(ec);
+        mutex_unlock(&ec->mutex);
+    }
       return ret;
   }

@@ -922,6 +956,30 @@ static int ec_enlarge_storm_threshold(const struct dmi_system_id *id)
       return 0;
   }

+/*
+ * 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 EC 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;
+    return 0;
+}
+
   static struct dmi_system_id ec_dmi_table[] __initdata = {
       {
       ec_skip_dsdt_scan, "Compal JFL92", {
@@ -965,6 +1023,9 @@ static struct dmi_system_id ec_dmi_table[] __initdata = {
       ec_validate_ecdt, "ASUS hardware", {
       DMI_MATCH(DMI_SYS_VENDOR, "ASUSTek Computer Inc."),
       DMI_MATCH(DMI_PRODUCT_NAME, "L4R"),}, NULL},
+    {
+    ec_clear_on_resume, "Samsung hardware", {
+    DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD.")}, NULL},
       {},
   };



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux