linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* GPE handling
@ 2007-11-09  7:50 Li, Shaohua
  2007-11-09  9:45 ` Alexey Starikovskiy
  0 siblings, 1 reply; 8+ messages in thread
From: Li, Shaohua @ 2007-11-09  7:50 UTC (permalink / raw)
  To: Moore, Robert, Lin, Ming M; +Cc: ACPI Devel Maling List

Bob & Ming,
I'm working on adding wakeup GPE support at system runtime, this
capability can help us
Identify which device invokes a wakeup event at runtime, this is
required for runtime device
Power management.

Below is the ASL code. For example, _L0c, USB3 will send a wakeup GPE
and invoke
a notify. In the notify handler, OS will clear USB3's PCI PME status to
avoid wakeup
event flood. But in current code, acpi_ev_asynch_execute_gpe_method will
start an asynchronous
execution of notify and return soon. Just before the return,
acpi_ev_asynch_execute_gpe_method
will reenable GPE 0C. That means GPE is enabled before OS execute
notification handler and USB3's
PCI PME status is cleared, and cause GPE flood. Ideally, I think we
should delay GPE enable
of _L0c till notification handler is finished or simply the method _L0c
is really finished.
What do you think?

Thanks,
Shaohua


        Method (_L0C, 0, NotSerialized)
        {
            Notify (\_SB.PCI0.USB3, 0x02)
        }

        Method (_L0D, 0, NotSerialized)
        {
            Notify (\_SB.PCI0.USB7, 0x02)
        }

        Method (_L0E, 0, NotSerialized)
        {
            Notify (\_SB.PCI0.USB4, 0x02)
        }

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

* Re: GPE handling
  2007-11-09  7:50 GPE handling Li, Shaohua
@ 2007-11-09  9:45 ` Alexey Starikovskiy
  2007-11-12  0:42   ` Shaohua Li
  0 siblings, 1 reply; 8+ messages in thread
From: Alexey Starikovskiy @ 2007-11-09  9:45 UTC (permalink / raw)
  To: Li, Shaohua; +Cc: Moore, Robert, Lin, Ming M, ACPI Devel Maling List

Shaohua,
There is cond_resched() before the exit from deferred execution routine,
specifically to let notify thread a chance to execute.
Thus by the time deferred execution is exited, notify should be already 
done,
and we could safely enable _Lxx again.
Do you think it is not sufficient?

Regards,
Alex.

Li, Shaohua wrote:
> Bob & Ming,
> I'm working on adding wakeup GPE support at system runtime, this
> capability can help us
> Identify which device invokes a wakeup event at runtime, this is
> required for runtime device
> Power management.
>
> Below is the ASL code. For example, _L0c, USB3 will send a wakeup GPE
> and invoke
> a notify. In the notify handler, OS will clear USB3's PCI PME status to
> avoid wakeup
> event flood. But in current code, acpi_ev_asynch_execute_gpe_method will
> start an asynchronous
> execution of notify and return soon. Just before the return,
> acpi_ev_asynch_execute_gpe_method
> will reenable GPE 0C. That means GPE is enabled before OS execute
> notification handler and USB3's
> PCI PME status is cleared, and cause GPE flood. Ideally, I think we
> should delay GPE enable
> of _L0c till notification handler is finished or simply the method _L0c
> is really finished.
> What do you think?
>
> Thanks,
> Shaohua
>
>
>         Method (_L0C, 0, NotSerialized)
>         {
>             Notify (\_SB.PCI0.USB3, 0x02)
>         }
>
>         Method (_L0D, 0, NotSerialized)
>         {
>             Notify (\_SB.PCI0.USB7, 0x02)
>         }
>
>         Method (_L0E, 0, NotSerialized)
>         {
>             Notify (\_SB.PCI0.USB4, 0x02)
>         }
> -
> 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] 8+ messages in thread

* Re: GPE handling
  2007-11-09  9:45 ` Alexey Starikovskiy
@ 2007-11-12  0:42   ` Shaohua Li
  2007-11-13 10:05     ` [PATCH] ACPI: Defer enabling of level GPE until all pending notifies done Alexey Starikovskiy
  0 siblings, 1 reply; 8+ messages in thread
From: Shaohua Li @ 2007-11-12  0:42 UTC (permalink / raw)
  To: Alexey Starikovskiy; +Cc: Moore, Robert, Lin, Ming M, ACPI Devel Maling List

On Fri, 2007-11-09 at 12:45 +0300, Alexey Starikovskiy wrote:
> Shaohua,
> There is cond_resched() before the exit from deferred execution routine,
> specifically to let notify thread a chance to execute.
> Thus by the time deferred execution is exited, notify should be already 
> done,
> and we could safely enable _Lxx again.
> Do you think it is not sufficient?
Certainly not. A resched() can help to give a chance for other threads
to execute, but it's very unreliable. If it works, just because of luck.
We should add a wait there to guarantee.

Thanks,
Shaohua

> 
> Li, Shaohua wrote:
> > Bob & Ming,
> > I'm working on adding wakeup GPE support at system runtime, this
> > capability can help us
> > Identify which device invokes a wakeup event at runtime, this is
> > required for runtime device
> > Power management.
> >
> > Below is the ASL code. For example, _L0c, USB3 will send a wakeup GPE
> > and invoke
> > a notify. In the notify handler, OS will clear USB3's PCI PME status to
> > avoid wakeup
> > event flood. But in current code, acpi_ev_asynch_execute_gpe_method will
> > start an asynchronous
> > execution of notify and return soon. Just before the return,
> > acpi_ev_asynch_execute_gpe_method
> > will reenable GPE 0C. That means GPE is enabled before OS execute
> > notification handler and USB3's
> > PCI PME status is cleared, and cause GPE flood. Ideally, I think we
> > should delay GPE enable
> > of _L0c till notification handler is finished or simply the method _L0c
> > is really finished.
> > What do you think?
> >
> > Thanks,
> > Shaohua
> >
> >
> >         Method (_L0C, 0, NotSerialized)
> >         {
> >             Notify (\_SB.PCI0.USB3, 0x02)
> >         }
> >
> >         Method (_L0D, 0, NotSerialized)
> >         {
> >             Notify (\_SB.PCI0.USB7, 0x02)
> >         }
> >
> >         Method (_L0E, 0, NotSerialized)
> >         {
> >             Notify (\_SB.PCI0.USB4, 0x02)
> >         }
> > -
> > 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] 8+ messages in thread

* [PATCH] ACPI: Defer enabling of level GPE until all pending notifies done
  2007-11-12  0:42   ` Shaohua Li
@ 2007-11-13 10:05     ` Alexey Starikovskiy
  2007-11-15  5:44       ` [PATCH] ACPI: Defer enabling of level GPE until all pending notifiesdone Shaohua Li
                         ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Alexey Starikovskiy @ 2007-11-13 10:05 UTC (permalink / raw)
  To: LenBrown; +Cc: Linux-acpi

Level GPE should not be enabled until all work caused by it is done,
e.g. all Notify() methods are completed.
This could be accomplished by appending enable_gpe function to the end
of notify queue.

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/events/evgpe.c |   17 +++++++++++++----
 drivers/acpi/osl.c          |   42 ++++++++----------------------------------
 2 files changed, 21 insertions(+), 38 deletions(-)

diff --git a/drivers/acpi/events/evgpe.c b/drivers/acpi/events/evgpe.c
index e22f4a9..b4509f9 100644
--- a/drivers/acpi/events/evgpe.c
+++ b/drivers/acpi/events/evgpe.c
@@ -501,6 +501,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info * gpe_xrupt_list)
  *              an interrupt handler.
  *
  ******************************************************************************/
+static void acpi_ev_asynch_enable_gpe(void *context);
 
 static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context)
 {
@@ -576,22 +577,30 @@ static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context)
 					 method_node)));
 		}
 	}
+	/* Defer enabling of GPE until all notify handlers are done */
+	acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_ev_asynch_enable_gpe,
+				gpe_event_info);
+	return_VOID;
+}
 
-	if ((local_gpe_event_info.flags & ACPI_GPE_XRUPT_TYPE_MASK) ==
+static void acpi_ev_asynch_enable_gpe(void *context)
+{
+	struct acpi_gpe_event_info *gpe_event_info = context;
+	acpi_status status;
+	if ((gpe_event_info->flags & ACPI_GPE_XRUPT_TYPE_MASK) ==
 	    ACPI_GPE_LEVEL_TRIGGERED) {
 		/*
 		 * GPE is level-triggered, we clear the GPE status bit after
 		 * handling the event.
 		 */
-		status = acpi_hw_clear_gpe(&local_gpe_event_info);
+		status = acpi_hw_clear_gpe(gpe_event_info);
 		if (ACPI_FAILURE(status)) {
 			return_VOID;
 		}
 	}
 
 	/* Enable this GPE */
-
-	(void)acpi_hw_write_gpe_enable_reg(&local_gpe_event_info);
+	(void)acpi_hw_write_gpe_enable_reg(gpe_event_info);
 	return_VOID;
 }
 
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index aabc6ca..6816ac6 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -625,25 +625,6 @@ static void acpi_os_execute_deferred(struct work_struct *work)
 	dpc->function(dpc->context);
 	kfree(dpc);
 
-	/* Yield cpu to notify thread */
-	cond_resched();
-
-	return;
-}
-
-static void acpi_os_execute_notify(struct work_struct *work)
-{
-	struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
-
-	if (!dpc) {
-		printk(KERN_ERR PREFIX "Invalid (NULL) context\n");
-		return;
-	}
-
-	dpc->function(dpc->context);
-
-	kfree(dpc);
-
 	return;
 }
 
@@ -667,7 +648,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
 {
 	acpi_status status = AE_OK;
 	struct acpi_os_dpc *dpc;
-
+	struct workqueue_struct *queue;
 	ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
 			  "Scheduling function [%p(%p)] for deferred execution.\n",
 			  function, context));
@@ -691,20 +672,13 @@ acpi_status acpi_os_execute(acpi_execute_type type,
 	dpc->function = function;
 	dpc->context = context;
 
-	if (type == OSL_NOTIFY_HANDLER) {
-		INIT_WORK(&dpc->work, acpi_os_execute_notify);
-		if (!queue_work(kacpi_notify_wq, &dpc->work)) {
-			status = AE_ERROR;
-			kfree(dpc);
-		}
-	} else {
-		INIT_WORK(&dpc->work, acpi_os_execute_deferred);
-		if (!queue_work(kacpid_wq, &dpc->work)) {
-			ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
-				  "Call to queue_work() failed.\n"));
-			status = AE_ERROR;
-			kfree(dpc);
-		}
+	INIT_WORK(&dpc->work, acpi_os_execute_deferred);
+	queue = (type == OSL_NOTIFY_HANDLER) ? kacpi_notify_wq : kacpid_wq;
+	if (!queue_work(queue, &dpc->work)) {
+		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
+			  "Call to queue_work() failed.\n"));
+		status = AE_ERROR;
+		kfree(dpc);
 	}
 	return_ACPI_STATUS(status);
 }


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

* Re: [PATCH] ACPI: Defer enabling of level GPE until all pending notifiesdone
  2007-11-13 10:05     ` [PATCH] ACPI: Defer enabling of level GPE until all pending notifies done Alexey Starikovskiy
@ 2007-11-15  5:44       ` Shaohua Li
  2007-12-07  2:55       ` [PATCH] ACPI: Defer enabling of level GPE until all pending notifies done Len Brown
  2008-02-02  9:45       ` Len Brown
  2 siblings, 0 replies; 8+ messages in thread
From: Shaohua Li @ 2007-11-15  5:44 UTC (permalink / raw)
  To: Alexey Starikovskiy; +Cc: LenBrown, Linux-acpi

On Tue, 2007-11-13 at 18:05 +0800, Alexey Starikovskiy wrote:
> Level GPE should not be enabled until all work caused by it is done,
> e.g. all Notify() methods are completed.
> This could be accomplished by appending enable_gpe function to the end
> of notify queue.
Patch looks good, but the method is a little tricky, I'd suggest adding
some comments in the code to explain it.

Thanks,
Shaohua

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

* Re: [PATCH] ACPI: Defer enabling of level GPE until all pending notifies done
  2007-11-13 10:05     ` [PATCH] ACPI: Defer enabling of level GPE until all pending notifies done Alexey Starikovskiy
  2007-11-15  5:44       ` [PATCH] ACPI: Defer enabling of level GPE until all pending notifiesdone Shaohua Li
@ 2007-12-07  2:55       ` Len Brown
  2008-02-02  9:45       ` Len Brown
  2 siblings, 0 replies; 8+ messages in thread
From: Len Brown @ 2007-12-07  2:55 UTC (permalink / raw)
  To: Alexey Starikovskiy; +Cc: Linux-acpi

applied to acpi test branch.

thanks,
-len

On Tuesday 13 November 2007 05:05, Alexey Starikovskiy wrote:
> Level GPE should not be enabled until all work caused by it is done,
> e.g. all Notify() methods are completed.
> This could be accomplished by appending enable_gpe function to the end
> of notify queue.
> 
> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> ---
> 
>  drivers/acpi/events/evgpe.c |   17 +++++++++++++----
>  drivers/acpi/osl.c          |   42 ++++++++----------------------------------
>  2 files changed, 21 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/acpi/events/evgpe.c b/drivers/acpi/events/evgpe.c
> index e22f4a9..b4509f9 100644
> --- a/drivers/acpi/events/evgpe.c
> +++ b/drivers/acpi/events/evgpe.c
> @@ -501,6 +501,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info * gpe_xrupt_list)
>   *              an interrupt handler.
>   *
>   ******************************************************************************/
> +static void acpi_ev_asynch_enable_gpe(void *context);
>  
>  static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context)
>  {
> @@ -576,22 +577,30 @@ static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context)
>  					 method_node)));
>  		}
>  	}
> +	/* Defer enabling of GPE until all notify handlers are done */
> +	acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_ev_asynch_enable_gpe,
> +				gpe_event_info);
> +	return_VOID;
> +}
>  
> -	if ((local_gpe_event_info.flags & ACPI_GPE_XRUPT_TYPE_MASK) ==
> +static void acpi_ev_asynch_enable_gpe(void *context)
> +{
> +	struct acpi_gpe_event_info *gpe_event_info = context;
> +	acpi_status status;
> +	if ((gpe_event_info->flags & ACPI_GPE_XRUPT_TYPE_MASK) ==
>  	    ACPI_GPE_LEVEL_TRIGGERED) {
>  		/*
>  		 * GPE is level-triggered, we clear the GPE status bit after
>  		 * handling the event.
>  		 */
> -		status = acpi_hw_clear_gpe(&local_gpe_event_info);
> +		status = acpi_hw_clear_gpe(gpe_event_info);
>  		if (ACPI_FAILURE(status)) {
>  			return_VOID;
>  		}
>  	}
>  
>  	/* Enable this GPE */
> -
> -	(void)acpi_hw_write_gpe_enable_reg(&local_gpe_event_info);
> +	(void)acpi_hw_write_gpe_enable_reg(gpe_event_info);
>  	return_VOID;
>  }
>  
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index aabc6ca..6816ac6 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -625,25 +625,6 @@ static void acpi_os_execute_deferred(struct work_struct *work)
>  	dpc->function(dpc->context);
>  	kfree(dpc);
>  
> -	/* Yield cpu to notify thread */
> -	cond_resched();
> -
> -	return;
> -}
> -
> -static void acpi_os_execute_notify(struct work_struct *work)
> -{
> -	struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
> -
> -	if (!dpc) {
> -		printk(KERN_ERR PREFIX "Invalid (NULL) context\n");
> -		return;
> -	}
> -
> -	dpc->function(dpc->context);
> -
> -	kfree(dpc);
> -
>  	return;
>  }
>  
> @@ -667,7 +648,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
>  {
>  	acpi_status status = AE_OK;
>  	struct acpi_os_dpc *dpc;
> -
> +	struct workqueue_struct *queue;
>  	ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
>  			  "Scheduling function [%p(%p)] for deferred execution.\n",
>  			  function, context));
> @@ -691,20 +672,13 @@ acpi_status acpi_os_execute(acpi_execute_type type,
>  	dpc->function = function;
>  	dpc->context = context;
>  
> -	if (type == OSL_NOTIFY_HANDLER) {
> -		INIT_WORK(&dpc->work, acpi_os_execute_notify);
> -		if (!queue_work(kacpi_notify_wq, &dpc->work)) {
> -			status = AE_ERROR;
> -			kfree(dpc);
> -		}
> -	} else {
> -		INIT_WORK(&dpc->work, acpi_os_execute_deferred);
> -		if (!queue_work(kacpid_wq, &dpc->work)) {
> -			ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
> -				  "Call to queue_work() failed.\n"));
> -			status = AE_ERROR;
> -			kfree(dpc);
> -		}
> +	INIT_WORK(&dpc->work, acpi_os_execute_deferred);
> +	queue = (type == OSL_NOTIFY_HANDLER) ? kacpi_notify_wq : kacpid_wq;
> +	if (!queue_work(queue, &dpc->work)) {
> +		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
> +			  "Call to queue_work() failed.\n"));
> +		status = AE_ERROR;
> +		kfree(dpc);
>  	}
>  	return_ACPI_STATUS(status);
>  }
> 
> -
> 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] 8+ messages in thread

* Re: [PATCH] ACPI: Defer enabling of level GPE until all pending notifies done
  2007-11-13 10:05     ` [PATCH] ACPI: Defer enabling of level GPE until all pending notifies done Alexey Starikovskiy
  2007-11-15  5:44       ` [PATCH] ACPI: Defer enabling of level GPE until all pending notifiesdone Shaohua Li
  2007-12-07  2:55       ` [PATCH] ACPI: Defer enabling of level GPE until all pending notifies done Len Brown
@ 2008-02-02  9:45       ` Len Brown
  2008-02-02 11:03         ` Alexey Starikovskiy
  2 siblings, 1 reply; 8+ messages in thread
From: Len Brown @ 2008-02-02  9:45 UTC (permalink / raw)
  To: Alexey Starikovskiy, Zhang Rui; +Cc: Linux-acpi

Alexey, Rui,
this now conflicts with theacpi_os_execute() changes
proposed in the battery hotplug series.

recommendations?

thanks,
-Len


On Tuesday 13 November 2007 05:05, Alexey Starikovskiy wrote:
> Level GPE should not be enabled until all work caused by it is done,
> e.g. all Notify() methods are completed.
> This could be accomplished by appending enable_gpe function to the end
> of notify queue.
> 
> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> ---
> 
>  drivers/acpi/events/evgpe.c |   17 +++++++++++++----
>  drivers/acpi/osl.c          |   42 ++++++++----------------------------------
>  2 files changed, 21 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/acpi/events/evgpe.c b/drivers/acpi/events/evgpe.c
> index e22f4a9..b4509f9 100644
> --- a/drivers/acpi/events/evgpe.c
> +++ b/drivers/acpi/events/evgpe.c
> @@ -501,6 +501,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info * gpe_xrupt_list)
>   *              an interrupt handler.
>   *
>   ******************************************************************************/
> +static void acpi_ev_asynch_enable_gpe(void *context);
>  
>  static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context)
>  {
> @@ -576,22 +577,30 @@ static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context)
>  					 method_node)));
>  		}
>  	}
> +	/* Defer enabling of GPE until all notify handlers are done */
> +	acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_ev_asynch_enable_gpe,
> +				gpe_event_info);
> +	return_VOID;
> +}
>  
> -	if ((local_gpe_event_info.flags & ACPI_GPE_XRUPT_TYPE_MASK) ==
> +static void acpi_ev_asynch_enable_gpe(void *context)
> +{
> +	struct acpi_gpe_event_info *gpe_event_info = context;
> +	acpi_status status;
> +	if ((gpe_event_info->flags & ACPI_GPE_XRUPT_TYPE_MASK) ==
>  	    ACPI_GPE_LEVEL_TRIGGERED) {
>  		/*
>  		 * GPE is level-triggered, we clear the GPE status bit after
>  		 * handling the event.
>  		 */
> -		status = acpi_hw_clear_gpe(&local_gpe_event_info);
> +		status = acpi_hw_clear_gpe(gpe_event_info);
>  		if (ACPI_FAILURE(status)) {
>  			return_VOID;
>  		}
>  	}
>  
>  	/* Enable this GPE */
> -
> -	(void)acpi_hw_write_gpe_enable_reg(&local_gpe_event_info);
> +	(void)acpi_hw_write_gpe_enable_reg(gpe_event_info);
>  	return_VOID;
>  }
>  
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index aabc6ca..6816ac6 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -625,25 +625,6 @@ static void acpi_os_execute_deferred(struct work_struct *work)
>  	dpc->function(dpc->context);
>  	kfree(dpc);
>  
> -	/* Yield cpu to notify thread */
> -	cond_resched();
> -
> -	return;
> -}
> -
> -static void acpi_os_execute_notify(struct work_struct *work)
> -{
> -	struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
> -
> -	if (!dpc) {
> -		printk(KERN_ERR PREFIX "Invalid (NULL) context\n");
> -		return;
> -	}
> -
> -	dpc->function(dpc->context);
> -
> -	kfree(dpc);
> -
>  	return;
>  }
>  
> @@ -667,7 +648,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
>  {
>  	acpi_status status = AE_OK;
>  	struct acpi_os_dpc *dpc;
> -
> +	struct workqueue_struct *queue;
>  	ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
>  			  "Scheduling function [%p(%p)] for deferred execution.\n",
>  			  function, context));
> @@ -691,20 +672,13 @@ acpi_status acpi_os_execute(acpi_execute_type type,
>  	dpc->function = function;
>  	dpc->context = context;
>  
> -	if (type == OSL_NOTIFY_HANDLER) {
> -		INIT_WORK(&dpc->work, acpi_os_execute_notify);
> -		if (!queue_work(kacpi_notify_wq, &dpc->work)) {
> -			status = AE_ERROR;
> -			kfree(dpc);
> -		}
> -	} else {
> -		INIT_WORK(&dpc->work, acpi_os_execute_deferred);
> -		if (!queue_work(kacpid_wq, &dpc->work)) {
> -			ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
> -				  "Call to queue_work() failed.\n"));
> -			status = AE_ERROR;
> -			kfree(dpc);
> -		}
> +	INIT_WORK(&dpc->work, acpi_os_execute_deferred);
> +	queue = (type == OSL_NOTIFY_HANDLER) ? kacpi_notify_wq : kacpid_wq;
> +	if (!queue_work(queue, &dpc->work)) {
> +		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
> +			  "Call to queue_work() failed.\n"));
> +		status = AE_ERROR;
> +		kfree(dpc);
>  	}
>  	return_ACPI_STATUS(status);
>  }
> 
> -
> 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] 8+ messages in thread

* Re: [PATCH] ACPI: Defer enabling of level GPE until all pending notifies done
  2008-02-02  9:45       ` Len Brown
@ 2008-02-02 11:03         ` Alexey Starikovskiy
  0 siblings, 0 replies; 8+ messages in thread
From: Alexey Starikovskiy @ 2008-02-02 11:03 UTC (permalink / raw)
  To: Len Brown; +Cc: Zhang Rui, Linux-acpi

Len Brown wrote:
> Alexey, Rui,
> this now conflicts with theacpi_os_execute() changes
> proposed in the battery hotplug series.
Great! We already sold this patch to FreeBSD, but don't use it ourselves?
> 
> recommendations?
There are no fundamental conflicts, these patches just happen to change 
same function. I could adjust any of two patches to not conflict with the other.

Regards,
Alex.
> 
> thanks,
> -Len
> 
> 
> On Tuesday 13 November 2007 05:05, Alexey Starikovskiy wrote:
>> Level GPE should not be enabled until all work caused by it is done,
>> e.g. all Notify() methods are completed.
>> This could be accomplished by appending enable_gpe function to the end
>> of notify queue.
>>
>> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
>> ---
>>
>>  drivers/acpi/events/evgpe.c |   17 +++++++++++++----
>>  drivers/acpi/osl.c          |   42 ++++++++----------------------------------
>>  2 files changed, 21 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/acpi/events/evgpe.c b/drivers/acpi/events/evgpe.c
>> index e22f4a9..b4509f9 100644
>> --- a/drivers/acpi/events/evgpe.c
>> +++ b/drivers/acpi/events/evgpe.c
>> @@ -501,6 +501,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info * gpe_xrupt_list)
>>   *              an interrupt handler.
>>   *
>>   ******************************************************************************/
>> +static void acpi_ev_asynch_enable_gpe(void *context);
>>  
>>  static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context)
>>  {
>> @@ -576,22 +577,30 @@ static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context)
>>  					 method_node)));
>>  		}
>>  	}
>> +	/* Defer enabling of GPE until all notify handlers are done */
>> +	acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_ev_asynch_enable_gpe,
>> +				gpe_event_info);
>> +	return_VOID;
>> +}
>>  
>> -	if ((local_gpe_event_info.flags & ACPI_GPE_XRUPT_TYPE_MASK) ==
>> +static void acpi_ev_asynch_enable_gpe(void *context)
>> +{
>> +	struct acpi_gpe_event_info *gpe_event_info = context;
>> +	acpi_status status;
>> +	if ((gpe_event_info->flags & ACPI_GPE_XRUPT_TYPE_MASK) ==
>>  	    ACPI_GPE_LEVEL_TRIGGERED) {
>>  		/*
>>  		 * GPE is level-triggered, we clear the GPE status bit after
>>  		 * handling the event.
>>  		 */
>> -		status = acpi_hw_clear_gpe(&local_gpe_event_info);
>> +		status = acpi_hw_clear_gpe(gpe_event_info);
>>  		if (ACPI_FAILURE(status)) {
>>  			return_VOID;
>>  		}
>>  	}
>>  
>>  	/* Enable this GPE */
>> -
>> -	(void)acpi_hw_write_gpe_enable_reg(&local_gpe_event_info);
>> +	(void)acpi_hw_write_gpe_enable_reg(gpe_event_info);
>>  	return_VOID;
>>  }
>>  
>> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
>> index aabc6ca..6816ac6 100644
>> --- a/drivers/acpi/osl.c
>> +++ b/drivers/acpi/osl.c
>> @@ -625,25 +625,6 @@ static void acpi_os_execute_deferred(struct work_struct *work)
>>  	dpc->function(dpc->context);
>>  	kfree(dpc);
>>  
>> -	/* Yield cpu to notify thread */
>> -	cond_resched();
>> -
>> -	return;
>> -}
>> -
>> -static void acpi_os_execute_notify(struct work_struct *work)
>> -{
>> -	struct acpi_os_dpc *dpc = container_of(work, struct acpi_os_dpc, work);
>> -
>> -	if (!dpc) {
>> -		printk(KERN_ERR PREFIX "Invalid (NULL) context\n");
>> -		return;
>> -	}
>> -
>> -	dpc->function(dpc->context);
>> -
>> -	kfree(dpc);
>> -
>>  	return;
>>  }
>>  
>> @@ -667,7 +648,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
>>  {
>>  	acpi_status status = AE_OK;
>>  	struct acpi_os_dpc *dpc;
>> -
>> +	struct workqueue_struct *queue;
>>  	ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
>>  			  "Scheduling function [%p(%p)] for deferred execution.\n",
>>  			  function, context));
>> @@ -691,20 +672,13 @@ acpi_status acpi_os_execute(acpi_execute_type type,
>>  	dpc->function = function;
>>  	dpc->context = context;
>>  
>> -	if (type == OSL_NOTIFY_HANDLER) {
>> -		INIT_WORK(&dpc->work, acpi_os_execute_notify);
>> -		if (!queue_work(kacpi_notify_wq, &dpc->work)) {
>> -			status = AE_ERROR;
>> -			kfree(dpc);
>> -		}
>> -	} else {
>> -		INIT_WORK(&dpc->work, acpi_os_execute_deferred);
>> -		if (!queue_work(kacpid_wq, &dpc->work)) {
>> -			ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
>> -				  "Call to queue_work() failed.\n"));
>> -			status = AE_ERROR;
>> -			kfree(dpc);
>> -		}
>> +	INIT_WORK(&dpc->work, acpi_os_execute_deferred);
>> +	queue = (type == OSL_NOTIFY_HANDLER) ? kacpi_notify_wq : kacpid_wq;
>> +	if (!queue_work(queue, &dpc->work)) {
>> +		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
>> +			  "Call to queue_work() failed.\n"));
>> +		status = AE_ERROR;
>> +		kfree(dpc);
>>  	}
>>  	return_ACPI_STATUS(status);
>>  }
>>
>> -
>> 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] 8+ messages in thread

end of thread, other threads:[~2008-02-02 11:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-09  7:50 GPE handling Li, Shaohua
2007-11-09  9:45 ` Alexey Starikovskiy
2007-11-12  0:42   ` Shaohua Li
2007-11-13 10:05     ` [PATCH] ACPI: Defer enabling of level GPE until all pending notifies done Alexey Starikovskiy
2007-11-15  5:44       ` [PATCH] ACPI: Defer enabling of level GPE until all pending notifiesdone Shaohua Li
2007-12-07  2:55       ` [PATCH] ACPI: Defer enabling of level GPE until all pending notifies done Len Brown
2008-02-02  9:45       ` Len Brown
2008-02-02 11:03         ` Alexey Starikovskiy

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).