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. Thank you for your advice, Kieran. -- 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