On Sunday 07 February 2010, Rafael J. Wysocki wrote: > 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. > > 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. For easier reference, I reworked the Matthew's patches to implement the above idea. The patches follow, comments welcome. Rafael -- 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
