>-----Original Message----- >From: Rafael J. Wysocki [mailto:rjw@xxxxxxx] >Sent: Saturday, February 06, 2010 3:32 PM >To: Moore, Robert >Cc: Matthew Garrett; linux-acpi@xxxxxxxxxxxxxxx; Len Brown >Subject: Re: Recent GPE patches - some questions. > >Hi, > >On Wednesday 03 February 2010, Moore, Robert wrote: >> Matthew, >> >> Thanks for your response to my questions. >> >> I've been thinking about these interfaces: >> >> acpi_ref_runtime_gpe >> acpi_ref_wakeup_gpe >> acpi_unref_runtime_gpe >> acpi_unref_wakeup_gpe >> >> While I understand the need for the reference counting mechanism, it is a >> good idea to support the shared GPEs, I think it may be simpler and >cleaner >> to simply not directly expose this mechanism to GPE users. > >Well, it already is exposed to the users and in a way that doesn't look >very >clean to me. Namely, to enable a wakeup GPE for run time, the caller needs >to >set its type to ACPI_GPE_TYPE_WAKE_RUN before calling acpi_enable_gpe(), >while with the Matthew's interface the only thing the caller would need to >do >would be calling acpi_ref_runtime_gpe(). > >> I'm suggesting that we add the reference counting mechanism to the >existing >> AcpiEnableGpe and AcpiDisableGpe interfaces and update their >descriptions. >> We already hide the differences between wake, run, and wake-run GPEs >behind >> these interfaces. > >Not really, as I said above. > >Moreover, we need two reference counters, because there are cases when a >GPE should be enabled for wakeup and not enabled for run time and vice >versa. > >> Adding the reference count semantic to these interfaces changes their >> behavior in a fairly simple way: >> >> Support for shared GPEs: >> AcpiEnableGpe: For a given GPE, it is actually enabled only on the first >call. >> AcpiDisableGpe: For a given GPE, it is actually disabled only on the last >call. > >I suppose you mean acpi_enable_gpe() and acpi_disable_gpe(). > >That wouldn't work, because sometimes we need to actually hardware-disable >GPEs and hardware-enable them regardless of the refcount mechanism, like >for example in the EC suspend and resume routines. Yes, this makes sense, even if the EC GPE is shared. > >That said, if you are afraid that the new interface may be cumbersome for >the >callers, I think we can introduce just two callbacks, > >acpi_get_gpe() >acpi_put_gpe() > >taking 3 arguments each, where the two first arguments are like for the >Matthew's callbacks and the third argument is a mask of two bits: > >ACPI_GPE_TYPE_WAKE >ACPI_GPE_TYPE_RUNTIME > >that will tell the callback whether to use the wakeup or runtime counter >for >reference counting (if called with ACPI_GPE_TYPE_WAKE_RUN, both >reference counters will be modified at the same time). > >Please tell me what you think. > >Rafael Good, I like having the two interfaces better than the original four. However, I'm having some trouble with the naming. I think I would like to keep acpi_enable_gpe and acpi_disable_gpe as the high-level enable/disable interfaces that may or may not actually touch the hardware depending on reference count, run vs. wake, etc. Thus, enable_gpe and disable_gpe remain similar to today (with addition of new reference count mechanisms) and would correspond to put_gpe and get_gpe above. I think this also has the advantage of reducing changes to existing drivers across all operating systems supported by ACPICA. Then, we have the special case where a driver must absolutely (H/W) enable/disable a GPE, regardless of sharing. Probably only the EC driver needs this. I would like to see new interfaces for these. Actually, one new interface would probably suffice. Something like: acpi_set_gpe (ENABLE | DISABLE) Bob -- 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
