Messages in this thread | | | Date | Thu, 27 Feb 2014 12:47:28 +0800 | From | Li Guang <> | Subject | Re: [PATCH] ACPI / EC: Clear stale EC events on Samsung systems |
| |
Kieran Clancy wrote: > On Thu, Feb 27, 2014 at 12:29 PM, Li Guang<lig.fnst@cn.fujitsu.com> 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!
| |