linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch 12/14] ACPI: Clear GPE before disabling it
@ 2006-08-15  5:37 akpm
  0 siblings, 0 replies; 11+ messages in thread
From: akpm @ 2006-08-15  5:37 UTC (permalink / raw)
  To: len.brown
  Cc: linux-acpi, akpm, william.morrow, jordan.crouse, luming.yu,
	robert.moore

From: William Morrow <william.morrow@amd.com>

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@amd.com>
Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
Cc: "Yu, Luming" <luming.yu@intel.com>
Cc: "Brown, Len" <len.brown@intel.com>
Cc: "Moore, Robert" <robert.moore@intel.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 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]",
_

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [patch 12/14] ACPI: Clear GPE before disabling it
@ 2006-08-16 21:50 Moore, Robert
  0 siblings, 0 replies; 11+ messages in thread
From: Moore, Robert @ 2006-08-16 21:50 UTC (permalink / raw)
  To: akpm, Brown, Len; +Cc: linux-acpi, william.morrow, jordan.crouse, Yu, Luming

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@osdl.org [mailto:akpm@osdl.org]
> Sent: Monday, August 14, 2006 10:38 PM
> To: Brown, Len
> Cc: linux-acpi@vger.kernel.org; akpm@osdl.org; william.morrow@amd.com;
> jordan.crouse@amd.com; Yu, Luming; Moore, Robert
> Subject: [patch 12/14] ACPI: Clear GPE before disabling it
> 
> From: William Morrow <william.morrow@amd.com>
> 
> 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@amd.com>
> Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
> Cc: "Yu, Luming" <luming.yu@intel.com>
> Cc: "Brown, Len" <len.brown@intel.com>
> Cc: "Moore, Robert" <robert.moore@intel.com>
> Signed-off-by: Andrew Morton <akpm@osdl.org>
> ---
> 
>  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]",
> _

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [patch 12/14] ACPI: Clear GPE before disabling it
@ 2006-08-16 21:53 Moore, Robert
  2006-08-17 16:39 ` William Morrow
  0 siblings, 1 reply; 11+ messages in thread
From: Moore, Robert @ 2006-08-16 21:53 UTC (permalink / raw)
  To: Moore, Robert, akpm, Brown, Len
  Cc: linux-acpi, william.morrow, jordan.crouse, Yu, Luming

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);
        }
    }




> -----Original Message-----
> From: Moore, Robert
> Sent: Wednesday, August 16, 2006 2:50 PM
> To: 'akpm@osdl.org'; Brown, Len
> Cc: linux-acpi@vger.kernel.org; william.morrow@amd.com;
> jordan.crouse@amd.com; 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@osdl.org [mailto:akpm@osdl.org]
> > Sent: Monday, August 14, 2006 10:38 PM
> > To: Brown, Len
> > Cc: linux-acpi@vger.kernel.org; akpm@osdl.org;
william.morrow@amd.com;
> > jordan.crouse@amd.com; Yu, Luming; Moore, Robert
> > Subject: [patch 12/14] ACPI: Clear GPE before disabling it
> >
> > From: William Morrow <william.morrow@amd.com>
> >
> > 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@amd.com>
> > Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
> > Cc: "Yu, Luming" <luming.yu@intel.com>
> > Cc: "Brown, Len" <len.brown@intel.com>
> > Cc: "Moore, Robert" <robert.moore@intel.com>
> > Signed-off-by: Andrew Morton <akpm@osdl.org>
> > ---
> >
> >  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]",
> > _

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 12/14] ACPI: Clear GPE before disabling it
  2006-08-16 21:53 [patch 12/14] ACPI: Clear GPE before disabling it Moore, Robert
@ 2006-08-17 16:39 ` William Morrow
  0 siblings, 0 replies; 11+ messages in thread
From: William Morrow @ 2006-08-17 16:39 UTC (permalink / raw)
  To: Moore, Robert; +Cc: akpm, Brown, Len, linux-acpi, jordan.crouse, Yu, Luming

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@osdl.org'; Brown, Len
>>Cc: linux-acpi@vger.kernel.org; william.morrow@amd.com;
>>jordan.crouse@amd.com; 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@osdl.org [mailto:akpm@osdl.org]
>>>Sent: Monday, August 14, 2006 10:38 PM
>>>To: Brown, Len
>>>Cc: linux-acpi@vger.kernel.org; akpm@osdl.org;
>>>      
>>>
>william.morrow@amd.com;
>  
>
>>>jordan.crouse@amd.com; Yu, Luming; Moore, Robert
>>>Subject: [patch 12/14] ACPI: Clear GPE before disabling it
>>>
>>>From: William Morrow <william.morrow@amd.com>
>>>
>>>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@amd.com>
>>>Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
>>>Cc: "Yu, Luming" <luming.yu@intel.com>
>>>Cc: "Brown, Len" <len.brown@intel.com>
>>>Cc: "Moore, Robert" <robert.moore@intel.com>
>>>Signed-off-by: Andrew Morton <akpm@osdl.org>
>>>---
>>>
>>> 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]",
>>>_
>>>      
>>>
>
>
>  
>




^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [patch 12/14] ACPI: Clear GPE before disabling it
@ 2006-08-17 23:36 Moore, Robert
  2006-08-18 18:14 ` William Morrow
  0 siblings, 1 reply; 11+ messages in thread
From: Moore, Robert @ 2006-08-17 23:36 UTC (permalink / raw)
  To: William Morrow; +Cc: akpm, Brown, Len, linux-acpi, jordan.crouse, Yu, Luming

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.

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@amd.com]
> Sent: Thursday, August 17, 2006 9:39 AM
> To: Moore, Robert
> Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
> jordan.crouse@amd.com; 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@osdl.org'; Brown, Len
> >>Cc: linux-acpi@vger.kernel.org; william.morrow@amd.com;
> >>jordan.crouse@amd.com; 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@osdl.org [mailto:akpm@osdl.org]
> >>>Sent: Monday, August 14, 2006 10:38 PM
> >>>To: Brown, Len
> >>>Cc: linux-acpi@vger.kernel.org; akpm@osdl.org;
> >>>
> >>>
> >william.morrow@amd.com;
> >
> >
> >>>jordan.crouse@amd.com; Yu, Luming; Moore, Robert
> >>>Subject: [patch 12/14] ACPI: Clear GPE before disabling it
> >>>
> >>>From: William Morrow <william.morrow@amd.com>
> >>>
> >>>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@amd.com>
> >>>Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
> >>>Cc: "Yu, Luming" <luming.yu@intel.com>
> >>>Cc: "Brown, Len" <len.brown@intel.com>
> >>>Cc: "Moore, Robert" <robert.moore@intel.com>
> >>>Signed-off-by: Andrew Morton <akpm@osdl.org>
> >>>---
> >>>
> >>> 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]",
> >>>_
> >>>
> >>>
> >
> >
> >
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 12/14] ACPI: Clear GPE before disabling it
  2006-08-17 23:36 Moore, Robert
@ 2006-08-18 18:14 ` William Morrow
  0 siblings, 0 replies; 11+ messages in thread
From: William Morrow @ 2006-08-18 18:14 UTC (permalink / raw)
  To: Moore, Robert; +Cc: akpm, Brown, Len, linux-acpi, jordan.crouse, Yu, Luming

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@amd.com]
>>Sent: Thursday, August 17, 2006 9:39 AM
>>To: Moore, Robert
>>Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
>>jordan.crouse@amd.com; 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@osdl.org'; Brown, Len
>>>>Cc: linux-acpi@vger.kernel.org; william.morrow@amd.com;
>>>>jordan.crouse@amd.com; 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@osdl.org [mailto:akpm@osdl.org]
>>>>>Sent: Monday, August 14, 2006 10:38 PM
>>>>>To: Brown, Len
>>>>>Cc: linux-acpi@vger.kernel.org; akpm@osdl.org;
>>>>>
>>>>>
>>>>>          
>>>>>
>>>william.morrow@amd.com;
>>>
>>>
>>>      
>>>
>>>>>jordan.crouse@amd.com; Yu, Luming; Moore, Robert
>>>>>Subject: [patch 12/14] ACPI: Clear GPE before disabling it
>>>>>
>>>>>From: William Morrow <william.morrow@amd.com>
>>>>>
>>>>>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@amd.com>
>>>>>Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
>>>>>Cc: "Yu, Luming" <luming.yu@intel.com>
>>>>>Cc: "Brown, Len" <len.brown@intel.com>
>>>>>Cc: "Moore, Robert" <robert.moore@intel.com>
>>>>>Signed-off-by: Andrew Morton <akpm@osdl.org>
>>>>>---
>>>>>
>>>>>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]",
>>>>>_
>>>>>
>>>>>
>>>>>          
>>>>>
>>>
>>>
>>>      
>>>
>
>
>  
>




^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [patch 12/14] ACPI: Clear GPE before disabling it
@ 2006-08-18 19:58 Moore, Robert
  2006-08-18 22:01 ` William Morrow
  0 siblings, 1 reply; 11+ messages in thread
From: Moore, Robert @ 2006-08-18 19:58 UTC (permalink / raw)
  To: William Morrow; +Cc: akpm, Brown, Len, linux-acpi, jordan.crouse, Yu, Luming

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@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of William Morrow
> Sent: Friday, August 18, 2006 11:15 AM
> To: Moore, Robert
> Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
> jordan.crouse@amd.com; 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@amd.com]
> >>Sent: Thursday, August 17, 2006 9:39 AM
> >>To: Moore, Robert
> >>Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
> >>jordan.crouse@amd.com; 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@osdl.org'; Brown, Len
> >>>>Cc: linux-acpi@vger.kernel.org; william.morrow@amd.com;
> >>>>jordan.crouse@amd.com; 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@osdl.org [mailto:akpm@osdl.org]
> >>>>>Sent: Monday, August 14, 2006 10:38 PM
> >>>>>To: Brown, Len
> >>>>>Cc: linux-acpi@vger.kernel.org; akpm@osdl.org;
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>william.morrow@amd.com;
> >>>
> >>>
> >>>
> >>>
> >>>>>jordan.crouse@amd.com; Yu, Luming; Moore, Robert
> >>>>>Subject: [patch 12/14] ACPI: Clear GPE before disabling it
> >>>>>
> >>>>>From: William Morrow <william.morrow@amd.com>
> >>>>>
> >>>>>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@amd.com>
> >>>>>Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
> >>>>>Cc: "Yu, Luming" <luming.yu@intel.com>
> >>>>>Cc: "Brown, Len" <len.brown@intel.com>
> >>>>>Cc: "Moore, Robert" <robert.moore@intel.com>
> >>>>>Signed-off-by: Andrew Morton <akpm@osdl.org>
> >>>>>---
> >>>>>
> >>>>>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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 12/14] ACPI: Clear GPE before disabling it
  2006-08-18 19:58 Moore, Robert
@ 2006-08-18 22:01 ` William Morrow
  0 siblings, 0 replies; 11+ messages in thread
From: William Morrow @ 2006-08-18 22:01 UTC (permalink / raw)
  To: Moore, Robert; +Cc: akpm, Brown, Len, linux-acpi, jordan.crouse, Yu, Luming

[-- Attachment #1: Type: text/plain, Size: 11945 bytes --]

Moore, Robert wrote:

>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.
>>    
>>
I still don't see how the background method can execute with an interrupt
pending.   I suggest that the handler should disable the pending event,
schedule the background method to deal with the hardware, and clear the
event status.  The event/interrupt wont be refired because it is disabled.
The background task will have to deal with the hardware to remove the event
assertion.  This would simplify the suggested fix.  To that end, I have
created another patch which is attached.

I notice that in all cases, it should be that the interrupt status should be
cleared or a storm may be possible.  I suggest moving the acpi_hw_clear_gpe
call in the dispatch handler to the end of the acpi_ev_gpe_dispatch routine.
This gives the code more strength (is cleared in any case) and provides
good bracketing - although it is not clear why the status should be cleared
later rather than sooner.  It also sort of resembles the spec.

This does alter the hw status presented in the ctrl when the background
hander runs.  But since that is asynchronous, if that data is important
it should be captured in the isr and forwarded via the handler parameter
interface.

morrow

>
>  
>
>>-----Original Message-----
>>From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
>>owner@vger.kernel.org] On Behalf Of William Morrow
>>Sent: Friday, August 18, 2006 11:15 AM
>>To: Moore, Robert
>>Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
>>jordan.crouse@amd.com; 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@amd.com]
>>>>Sent: Thursday, August 17, 2006 9:39 AM
>>>>To: Moore, Robert
>>>>Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
>>>>jordan.crouse@amd.com; 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@osdl.org'; Brown, Len
>>>>>>Cc: linux-acpi@vger.kernel.org; william.morrow@amd.com;
>>>>>>jordan.crouse@amd.com; 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@osdl.org [mailto:akpm@osdl.org]
>>>>>>>Sent: Monday, August 14, 2006 10:38 PM
>>>>>>>To: Brown, Len
>>>>>>>Cc: linux-acpi@vger.kernel.org; akpm@osdl.org;
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>>>
>>>>>william.morrow@amd.com;
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>          
>>>>>
>>>>>>>jordan.crouse@amd.com; Yu, Luming; Moore, Robert
>>>>>>>Subject: [patch 12/14] ACPI: Clear GPE before disabling it
>>>>>>>
>>>>>>>From: William Morrow <william.morrow@amd.com>
>>>>>>>
>>>>>>>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@amd.com>
>>>>>>>Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
>>>>>>>Cc: "Yu, Luming" <luming.yu@intel.com>
>>>>>>>Cc: "Brown, Len" <len.brown@intel.com>
>>>>>>>Cc: "Moore, Robert" <robert.moore@intel.com>
>>>>>>>Signed-off-by: Andrew Morton <akpm@osdl.org>
>>>>>>>---
>>>>>>>
>>>>>>>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@vger.kernel.org
>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>    
>>
>
>
>  
>


[-- Attachment #2: acpi3.patch --]
[-- Type: text/plain, Size: 1152 bytes --]

--- drivers/acpi/events/evgpe.c.orig	2006-08-03 18:52:32.000000000 -0600
+++ drivers/acpi/events/evgpe.c	2006-08-18 15:17:03.000000000 -0600
@@ -637,18 +637,6 @@
 								handler->
 								context);
 
-		/* It is now safe to clear level-triggered events. */
-
-		if ((gpe_event_info->flags & ACPI_GPE_XRUPT_TYPE_MASK) ==
-		    ACPI_GPE_LEVEL_TRIGGERED) {
-			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);
-			}
-		}
 		break;
 
 	case ACPI_GPE_DISPATCH_METHOD:
@@ -701,6 +689,21 @@
 		break;
 	}
 
+	/* It is now safe to clear level-triggered events. */
+	if ((gpe_event_info->flags & ACPI_GPE_XRUPT_TYPE_MASK) ==
+	    ACPI_GPE_LEVEL_TRIGGERED) {
+		/*
+		 * Clear GPE, so interrupt doesn't storm
+		 */
+		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);
+		}
+	}
+
 	return_UINT32(ACPI_INTERRUPT_HANDLED);
 }
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [patch 12/14] ACPI: Clear GPE before disabling it
@ 2006-08-18 22:13 Moore, Robert
  0 siblings, 0 replies; 11+ messages in thread
From: Moore, Robert @ 2006-08-18 22:13 UTC (permalink / raw)
  To: William Morrow; +Cc: akpm, Brown, Len, linux-acpi, jordan.crouse, Yu, Luming

I believe that this is the reason for disabling the GPE in the handler,
so that it will not generate additional interrupts (SCIs)

    case ACPI_GPE_DISPATCH_METHOD:

        /*
         * Disable GPE, so it doesn't keep firing before the method has
a
         * chance to run.
         */
        Status = AcpiEvDisableGpe (GpeEventInfo);



> -----Original Message-----
> From: William Morrow [mailto:William.Morrow@amd.com]
> Sent: Friday, August 18, 2006 3:01 PM
> To: Moore, Robert
> Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
> jordan.crouse@amd.com; Yu, Luming
> Subject: Re: [patch 12/14] ACPI: Clear GPE before disabling it
> 
> Moore, Robert wrote:
> 
> >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.
> >>
> >>
> I still don't see how the background method can execute with an
interrupt
> pending.   I suggest that the handler should disable the pending
event,
> schedule the background method to deal with the hardware, and clear
the
> event status.  The event/interrupt wont be refired because it is
disabled.
> The background task will have to deal with the hardware to remove the
> event
> assertion.  This would simplify the suggested fix.  To that end, I
have
> created another patch which is attached.
> 
> I notice that in all cases, it should be that the interrupt status
should
> be
> cleared or a storm may be possible.  I suggest moving the
> acpi_hw_clear_gpe
> call in the dispatch handler to the end of the acpi_ev_gpe_dispatch
> routine.
> This gives the code more strength (is cleared in any case) and
provides
> good bracketing - although it is not clear why the status should be
> cleared
> later rather than sooner.  It also sort of resembles the spec.
> 
> This does alter the hw status presented in the ctrl when the
background
> hander runs.  But since that is asynchronous, if that data is
important
> it should be captured in the isr and forwarded via the handler
parameter
> interface.
> 
> morrow
> 
> >
> >
> >
> >>-----Original Message-----
> >>From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> >>owner@vger.kernel.org] On Behalf Of William Morrow
> >>Sent: Friday, August 18, 2006 11:15 AM
> >>To: Moore, Robert
> >>Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
> >>jordan.crouse@amd.com; 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@amd.com]
> >>>>Sent: Thursday, August 17, 2006 9:39 AM
> >>>>To: Moore, Robert
> >>>>Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
> >>>>jordan.crouse@amd.com; 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@osdl.org'; Brown, Len
> >>>>>>Cc: linux-acpi@vger.kernel.org; william.morrow@amd.com;
> >>>>>>jordan.crouse@amd.com; 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@osdl.org [mailto:akpm@osdl.org]
> >>>>>>>Sent: Monday, August 14, 2006 10:38 PM
> >>>>>>>To: Brown, Len
> >>>>>>>Cc: linux-acpi@vger.kernel.org; akpm@osdl.org;
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>william.morrow@amd.com;
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>>>jordan.crouse@amd.com; Yu, Luming; Moore, Robert
> >>>>>>>Subject: [patch 12/14] ACPI: Clear GPE before disabling it
> >>>>>>>
> >>>>>>>From: William Morrow <william.morrow@amd.com>
> >>>>>>>
> >>>>>>>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@amd.com>
> >>>>>>>Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
> >>>>>>>Cc: "Yu, Luming" <luming.yu@intel.com>
> >>>>>>>Cc: "Brown, Len" <len.brown@intel.com>
> >>>>>>>Cc: "Moore, Robert" <robert.moore@intel.com>
> >>>>>>>Signed-off-by: Andrew Morton <akpm@osdl.org>
> >>>>>>>---
> >>>>>>>
> >>>>>>>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@vger.kernel.org
> >>More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> >>
> >
> >
> >
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [patch 12/14] ACPI: Clear GPE before disabling it
@ 2006-08-22 21:10 Moore, Robert
  2006-08-23 14:10 ` William Morrow
  0 siblings, 1 reply; 11+ messages in thread
From: Moore, Robert @ 2006-08-22 21:10 UTC (permalink / raw)
  To: Moore, Robert, William Morrow
  Cc: akpm, Brown, Len, linux-acpi, jordan.crouse, Yu, Luming

Is this a "wake" or "runtime" GPE?

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Moore, Robert
> Sent: Friday, August 18, 2006 3:14 PM
> To: William Morrow
> Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
> jordan.crouse@amd.com; Yu, Luming
> Subject: RE: [patch 12/14] ACPI: Clear GPE before disabling it
> 
> I believe that this is the reason for disabling the GPE in the
handler,
> so that it will not generate additional interrupts (SCIs)
> 
>     case ACPI_GPE_DISPATCH_METHOD:
> 
>         /*
>          * Disable GPE, so it doesn't keep firing before the method
has
> a
>          * chance to run.
>          */
>         Status = AcpiEvDisableGpe (GpeEventInfo);
> 
> 
> 
> > -----Original Message-----
> > From: William Morrow [mailto:William.Morrow@amd.com]
> > Sent: Friday, August 18, 2006 3:01 PM
> > To: Moore, Robert
> > Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
> > jordan.crouse@amd.com; Yu, Luming
> > Subject: Re: [patch 12/14] ACPI: Clear GPE before disabling it
> >
> > Moore, Robert wrote:
> >
> > >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.
> > >>
> > >>
> > I still don't see how the background method can execute with an
> interrupt
> > pending.   I suggest that the handler should disable the pending
> event,
> > schedule the background method to deal with the hardware, and clear
> the
> > event status.  The event/interrupt wont be refired because it is
> disabled.
> > The background task will have to deal with the hardware to remove
the
> > event
> > assertion.  This would simplify the suggested fix.  To that end, I
> have
> > created another patch which is attached.
> >
> > I notice that in all cases, it should be that the interrupt status
> should
> > be
> > cleared or a storm may be possible.  I suggest moving the
> > acpi_hw_clear_gpe
> > call in the dispatch handler to the end of the acpi_ev_gpe_dispatch
> > routine.
> > This gives the code more strength (is cleared in any case) and
> provides
> > good bracketing - although it is not clear why the status should be
> > cleared
> > later rather than sooner.  It also sort of resembles the spec.
> >
> > This does alter the hw status presented in the ctrl when the
> background
> > hander runs.  But since that is asynchronous, if that data is
> important
> > it should be captured in the isr and forwarded via the handler
> parameter
> > interface.
> >
> > morrow
> >
> > >
> > >
> > >
> > >>-----Original Message-----
> > >>From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> > >>owner@vger.kernel.org] On Behalf Of William Morrow
> > >>Sent: Friday, August 18, 2006 11:15 AM
> > >>To: Moore, Robert
> > >>Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
> > >>jordan.crouse@amd.com; 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@amd.com]
> > >>>>Sent: Thursday, August 17, 2006 9:39 AM
> > >>>>To: Moore, Robert
> > >>>>Cc: akpm@osdl.org; Brown, Len; linux-acpi@vger.kernel.org;
> > >>>>jordan.crouse@amd.com; 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@osdl.org'; Brown, Len
> > >>>>>>Cc: linux-acpi@vger.kernel.org; william.morrow@amd.com;
> > >>>>>>jordan.crouse@amd.com; 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@osdl.org [mailto:akpm@osdl.org]
> > >>>>>>>Sent: Monday, August 14, 2006 10:38 PM
> > >>>>>>>To: Brown, Len
> > >>>>>>>Cc: linux-acpi@vger.kernel.org; akpm@osdl.org;
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>william.morrow@amd.com;
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>>>jordan.crouse@amd.com; Yu, Luming; Moore, Robert
> > >>>>>>>Subject: [patch 12/14] ACPI: Clear GPE before disabling it
> > >>>>>>>
> > >>>>>>>From: William Morrow <william.morrow@amd.com>
> > >>>>>>>
> > >>>>>>>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@amd.com>
> > >>>>>>>Signed-off-by: Jordan Crouse <jordan.crouse@amd.com>
> > >>>>>>>Cc: "Yu, Luming" <luming.yu@intel.com>
> > >>>>>>>Cc: "Brown, Len" <len.brown@intel.com>
> > >>>>>>>Cc: "Moore, Robert" <robert.moore@intel.com>
> > >>>>>>>Signed-off-by: Andrew Morton <akpm@osdl.org>
> > >>>>>>>---
> > >>>>>>>
> > >>>>>>>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@vger.kernel.org
> > >>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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [patch 12/14] ACPI: Clear GPE before disabling it
  2006-08-22 21:10 Moore, Robert
@ 2006-08-23 14:10 ` William Morrow
  0 siblings, 0 replies; 11+ messages in thread
From: William Morrow @ 2006-08-23 14:10 UTC (permalink / raw)
  To: Moore, Robert; +Cc: akpm, Brown, Len, linux-acpi, jordan.crouse, Yu, Luming

Moore, Robert wrote:

>Is this a "wake" or "runtime" GPE?
>
>  
>
Hi,  I will be gone for a few weeks.  Please feel free to
implement (or not implement) whatever you feel is an
appropriate correction.  I will review the results when
I return.  Meanwhile, thanks for your attention.

morrow




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2006-08-23 14:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-16 21:53 [patch 12/14] ACPI: Clear GPE before disabling it Moore, Robert
2006-08-17 16:39 ` William Morrow
  -- strict thread matches above, loose matches on Subject: below --
2006-08-22 21:10 Moore, Robert
2006-08-23 14:10 ` William Morrow
2006-08-18 22:13 Moore, Robert
2006-08-18 19:58 Moore, Robert
2006-08-18 22:01 ` William Morrow
2006-08-17 23:36 Moore, Robert
2006-08-18 18:14 ` William Morrow
2006-08-16 21:50 Moore, Robert
2006-08-15  5:37 akpm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).