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