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

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

 



Kieran Clancy wrote:
On Thu, Feb 27, 2014 at 12:29 PM, Li Guang<lig.fnst@xxxxxxxxxxxxxx>  wrote:
+#define ACPI_EC_CLEAR_MAX      20      /* Maximum number of events to
query
+                                        * when trying to clear the EC */


20 is enough?
the query index is length of a byte.
On my machine, 8 seems to be enough, so 20 seems to be a conservative
maximum. Just reading your other email, maybe we should set this to
32? or 40? 100?

If it's not enough, hopefully anyone seeing bugs will notice the
warning "maximum of X stale EC events cleared".

Here's what happens if I plug/replug the AC lots of times (more than
8) during suspend:

[ 8807.019800] ACPI : EC: --->  status = 0x29
[ 8807.019804] ACPI : EC: --->  data = 0x66
[ 8807.020790] ACPI : EC: --->  status = 0x29
[ 8807.020793] ACPI : EC: --->  data = 0x66
[ 8807.021793] ACPI : EC: --->  status = 0x29
[ 8807.021798] ACPI : EC: --->  data = 0x66
[ 8807.022831] ACPI : EC: --->  status = 0x29
[ 8807.022834] ACPI : EC: --->  data = 0x66
[ 8807.023788] ACPI : EC: --->  status = 0x29
[ 8807.023792] ACPI : EC: --->  data = 0x66
[ 8807.024787] ACPI : EC: --->  status = 0x29
[ 8807.024791] ACPI : EC: --->  data = 0x66
[ 8807.025787] ACPI : EC: --->  status = 0x29
[ 8807.025790] ACPI : EC: --->  data = 0x66
[ 8807.026787] ACPI : EC: --->  status = 0x29
[ 8807.026790] ACPI : EC: --->  data = 0x66
[ 8807.027786] ACPI : EC: --->  status = 0x09
[ 8807.027790] ACPI : EC: --->  data = 0x00
[ 8807.027792] ACPI : EC: 8 stale EC events cleared

Note that most of these have SCI_EVT set, but the OS is not notified
according to ACPI specs (seemingly because these events happened
during sleep).

The _Q66 method in my DSDT, is:

                         P8XH (Zero, 0x66)
                         If (LEqual (B1EX, One))
                         {
                             Notify (BAT1, 0x80)
                         }

So, basically, this is supposed to notify that the battery (BAT1 =
PNP0C0A) has changed state, but they are stale events so we don't run
the handlers.

+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 :-)
In my mind this is referring to the function name (acpi_ec_)clear.
Perhaps we could just make the connection more explicit in the
comment:

/* needs acpi_ec_clear() on boot/resume */

Not sure if this is better?

+       /* Some hardware may need the EC to be cleared before use */
description is implicit, should specify what we clear is Q event, not EC.
Are Q events the only thing we can get from the EC data port? I've
read the relevant parts of the ACPI spec and I can't say I am 100%
sure.

I guess you want to clear Q events here,
EC usually has ACPI space to be read by cmd 80.

Thanks!



--
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