Here is an explanation from one of the ACPI experts: > In most cases, it cannot be cleared until after the method is executed. > The method usually talks to the hardware that is holding the input > signal low to tell it to "let go". If you try to clear before the method > is executed, it won't work because the GPE will still be held low. > -----Original Message----- > From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi- > owner@xxxxxxxxxxxxxxx] On Behalf Of William Morrow > Sent: Friday, August 18, 2006 11:15 AM > To: Moore, Robert > Cc: akpm@xxxxxxxx; Brown, Len; linux-acpi@xxxxxxxxxxxxxxx; > jordan.crouse@xxxxxxx; Yu, Luming > Subject: Re: [patch 12/14] ACPI: Clear GPE before disabling it > > Moore, Robert wrote: > > >Here is the required software behavior, from the ACPI spec version 3.0A, > >page 138, section 5.6.2.2: > > > >When OSPM receives a general-purpose event (the event is from a GPEx_BLK > >STS bit), OSPM does the following: > >1. Disables the interrupt source (GPEx_BLK EN bit). > >2. If an edge event, clears the status bit. > >3. Performs one of the following: > >* Dispatches to an ACPI-aware device driver. > >* Queues the matching control method for execution. > >* Manages a wake event using device _PRW objects. > >4. If a level event, clears the status bit. > >5. Enables the interrupt source. > > > > > I have to admit, I dont see how step 4 is supposed to be occurring. > If any unclearable interrupt source is active here, then the storm will > ensue. > The background execution of a method will never happen to clear it. > > I have to plead ignorance when it comes to the full understanding of > the acpi system. On re-reading your email, I think I may have missed > your point on clearing the events in other cases. If you are advocating > that the fix should be conditioned to only clear the bit in the level case > then I have to agree. Like I said, I frequently seem to miss the point. > > morrow > > >This is essentially what is implemented today in ACPICA. (It appears > >that 1 and 2 are transposed in the code, this may or may not be a > >problem.) > > > >Note that edge GPEs are cleared before dispatch, but level GPEs are > >cleared after dispatch. > > > >Bob > > > > > > > > > >>-----Original Message----- > >>From: William Morrow [mailto:William.Morrow@xxxxxxx] > >>Sent: Thursday, August 17, 2006 9:39 AM > >>To: Moore, Robert > >>Cc: akpm@xxxxxxxx; Brown, Len; linux-acpi@xxxxxxxxxxxxxxx; > >>jordan.crouse@xxxxxxx; Yu, Luming > >>Subject: Re: [patch 12/14] ACPI: Clear GPE before disabling it > >> > >>Moore, Robert wrote: > >> > >> > >> > >>>Worse, the GPE is already cleared in the edge case, at the start of > >>> > >>> > >the > > > > > >>>GPE dispatch function: > >>> > >>> /* > >>> * If edge-triggered, clear the GPE status bit now. Note that > >>> * level-triggered events are cleared after the GPE is serviced. > >>> */ > >>> if ((GpeEventInfo->Flags & ACPI_GPE_XRUPT_TYPE_MASK) == > >>> ACPI_GPE_EDGE_TRIGGERED) > >>> { > >>> Status = AcpiHwClearGpe (GpeEventInfo); > >>> if (ACPI_FAILURE (Status)) > >>> { > >>> ACPI_EXCEPTION ((AE_INFO, Status, > >>> "Unable to clear GPE[%2X]", GpeNumber)); > >>> return_UINT32 (ACPI_INTERRUPT_NOT_HANDLED); > >>> } > >>> } > >>> > >>> > >>> > >>> > >>> > >>Sorry for the late reply, but I have not looked at this for some time. > >>I had to re-examine this issue. > >> > >>The interrupt type is tagged as level triggered in this case. > >> > >>The dispatch handler is ACPI_GPE_DISPATCH_METHOD and so the > >>method is queued, but the interrupt is not cleared (it is disabled > >>instead). > >>In this hw, that is all that happens. It is disabled but not cleared. > >>The original comment would lead me to think that the prevailing > >> > >> > >thought > > > > > >>was that disabling the gpe implied clearing the interrupt status. > >> > >> > >That is > > > > > >>not the case in this hw environment, and the result is the interrupt > >>storm. > >> > >>Although this handler is artfully done, the architecture of this > >>handler is unusual in that most interrupt handlers field status > >>and clear the offending event at once, then process the new input. > >>This handler is coded in a way that seems to indicate that interrupt > >>status is possible when events are not enabled, but is not willing to > >>clear them - again indicating that the hw design is required to block > >>interrupts if the event has been disabled, but the interrupt status is > >>not clear. If that is true - then this code has no effect, or is > >> > >> > >another > > > > > >>case which can result in an interrupt storm. > >> > >> /* Check if there is anything active at all in this register */ > >> > >> enabled_status_byte = (u8) (status_reg & enable_reg); > >> if (!enabled_status_byte) { > >> /* No active GPEs in this register, move on */ > >> continue; > >> } > >> > >>Additionally, any error return which prevents the interrupt > >>source from being cleared will result in an interrupt storm. > >>This occurs several times. > >> > >>The comment: > >> > >>/* > >> * Execute the method associated with the GPE > >> * NOTE: Level-triggered GPEs are cleared after the method completes. > >> */ > >> Status = AcpiOsExecute (OSL_GPE_HANDLER, > >> AcpiEvAsynchExecuteGpeMethod, GpeEventInfo); > >> > >>is alarming, since it documents that the interrupt is active and > >>expects the OS to be able to schedule with pending interrupts. > >> > >>My personal preference is that the code be modified to work in > >>the more traditional way, but since that would be a large change > >>and this produced the desired result - I opted for the minimum > >>coding distance change. > >> > >> > >>If there are any other materials you need to evaluate this change, let > >>me know. > >>I hope this addresses your point. I am sort of renowned for being a > >>little askew > >>when trying to explain myself. > >> > >>Thanks for your attention! > >>morrow > >> > >> > >> > >>> > >>> > >>> > >>>>-----Original Message----- > >>>>From: Moore, Robert > >>>>Sent: Wednesday, August 16, 2006 2:50 PM > >>>>To: 'akpm@xxxxxxxx'; Brown, Len > >>>>Cc: linux-acpi@xxxxxxxxxxxxxxx; william.morrow@xxxxxxx; > >>>>jordan.crouse@xxxxxxx; Yu, Luming > >>>>Subject: RE: [patch 12/14] ACPI: Clear GPE before disabling it > >>>> > >>>>How does this patch relate to level-triggered GPEs, where we have > >>>> > >>>> > >the > > > > > >>>>following comment in the code just after the patch: > >>>> > >>>> /* > >>>> * Execute the method associated with the GPE > >>>> * NOTE: Level-triggered GPEs are cleared after the method > >>>>completes. > >>>> */ > >>>> Status = AcpiOsExecute (OSL_GPE_HANDLER, > >>>> AcpiEvAsynchExecuteGpeMethod, GpeEventInfo); > >>>> > >>>> > >>>> > >>>> > >>>> > >>>>>-----Original Message----- > >>>>>From: akpm@xxxxxxxx [mailto:akpm@xxxxxxxx] > >>>>>Sent: Monday, August 14, 2006 10:38 PM > >>>>>To: Brown, Len > >>>>>Cc: linux-acpi@xxxxxxxxxxxxxxx; akpm@xxxxxxxx; > >>>>> > >>>>> > >>>>> > >>>>> > >>>william.morrow@xxxxxxx; > >>> > >>> > >>> > >>> > >>>>>jordan.crouse@xxxxxxx; Yu, Luming; Moore, Robert > >>>>>Subject: [patch 12/14] ACPI: Clear GPE before disabling it > >>>>> > >>>>>From: William Morrow <william.morrow@xxxxxxx> > >>>>> > >>>>>On some BIOSen, the GPE bit will remain set even if it is disabled, > >>>>>resulting in a interrupt storm. This patch clears the bit before > >>>>>disabling > >>>>>it. > >>>>> > >>>>>Signed-off-by: William Morrow <william.morrow@xxxxxxx> > >>>>>Signed-off-by: Jordan Crouse <jordan.crouse@xxxxxxx> > >>>>>Cc: "Yu, Luming" <luming.yu@xxxxxxxxx> > >>>>>Cc: "Brown, Len" <len.brown@xxxxxxxxx> > >>>>>Cc: "Moore, Robert" <robert.moore@xxxxxxxxx> > >>>>>Signed-off-by: Andrew Morton <akpm@xxxxxxxx> > >>>>>--- > >>>>> > >>>>>drivers/acpi/events/evgpe.c | 14 +++++++++++++- > >>>>>1 file changed, 13 insertions(+), 1 deletion(-) > >>>>> > >>>>>diff -puN > >>>>> > >>>>> > >>>>> > >>>>> > >>>drivers/acpi/events/evgpe.c~acpi-clear-gpe-before-disabling-it > >>> > >>> > >>> > >>> > >>>>>drivers/acpi/events/evgpe.c > >>>>>--- > >>>>> > >>>>> > >a/drivers/acpi/events/evgpe.c~acpi-clear-gpe-before-disabling-it > > > > > >>>>>+++ a/drivers/acpi/events/evgpe.c > >>>>>@@ -677,10 +677,22 @@ acpi_ev_gpe_dispatch(struct acpi_gpe_eve > >>>>> case ACPI_GPE_DISPATCH_METHOD: > >>>>> > >>>>> /* > >>>>>- * Disable GPE, so it doesn't keep firing before the > >>>>> > >>>>> > >>>>> > >>>>> > >>>method > >>> > >>> > >>> > >>> > >>>>>has a > >>>>>+ * Clear GPE, so it doesn't keep firing before the > >>>>> > >>>>> > >>>>> > >>>>> > >>>method has > >>> > >>> > >>> > >>> > >>>>>a > >>>>> * chance to run. > >>>>> */ > >>>>>+ status = acpi_hw_clear_gpe(gpe_event_info); > >>>>>+ if (ACPI_FAILURE(status)) { > >>>>>+ ACPI_EXCEPTION((AE_INFO, status, > >>>>>+ "Unable to clear GPE[%2X]", > >>>>>+ gpe_number)); > >>>>>+ return_UINT32(ACPI_INTERRUPT_NOT_HANDLED); > >>>>>+ } > >>>>>+ /* > >>>>>+ * Disable GPE, so it doesn't keep happen again. > >>>>>+ */ > >>>>>+ > >>>>> status = acpi_ev_disable_gpe(gpe_event_info); > >>>>>+ > >>>>> if (ACPI_FAILURE(status)) { > >>>>> ACPI_EXCEPTION((AE_INFO, status, > >>>>> "Unable to disable GPE[%2X]", > >>>>>_ > >>>>> > >>>>> > >>>>> > >>>>> > >>> > >>> > >>> > >>> > > > > > > > > > > > > - > 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 - 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
