* acpi-test tree on eeepc: EC error message on second resume @ 2008-10-11 16:57 Alan Jenkins 2008-10-11 17:09 ` Rafael J. Wysocki 0 siblings, 1 reply; 18+ messages in thread From: Alan Jenkins @ 2008-10-11 16:57 UTC (permalink / raw) To: Alexey Starikovskiy; +Cc: linux acpi, linux-kernel I've just run an acpi-test kernel on my EeePC and noticed a new issue. It seems to be caused (or revealed) by the EC interrupt transaction patch. On the second suspend/resume cycle, I see a kernel error message. [ 78.747707] ACPI: Waking up from system sleep state S3 [ 79.330001] ACPI: EC: input buffer not empty, aborting transaction [ 79.423327] ACPI: EC: non-query interrupt received, switching to interrupt mode I still don't see any issues in the code. I'll try getting a DEBUG trace to see the EC interrupts. Any other suggestions? Thanks Alan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-11 16:57 acpi-test tree on eeepc: EC error message on second resume Alan Jenkins @ 2008-10-11 17:09 ` Rafael J. Wysocki 2008-10-11 17:12 ` Alan Jenkins 0 siblings, 1 reply; 18+ messages in thread From: Rafael J. Wysocki @ 2008-10-11 17:09 UTC (permalink / raw) To: Alan Jenkins; +Cc: Alexey Starikovskiy, linux acpi, linux-kernel On Saturday, 11 of October 2008, Alan Jenkins wrote: > I've just run an acpi-test kernel on my EeePC and noticed a new issue. > It seems to be caused (or revealed) by the EC interrupt transaction patch. > > On the second suspend/resume cycle, I see a kernel error message. > > [ 78.747707] ACPI: Waking up from system sleep state S3 > [ 79.330001] ACPI: EC: input buffer not empty, aborting transaction > [ 79.423327] ACPI: EC: non-query interrupt received, switching to > interrupt mode > > I still don't see any issues in the code. I'll try getting a DEBUG > trace to see the EC interrupts. Any other suggestions? Not really, but is this reproducible? I mean, does it happen always on the second resume and does it happen on every next resume after the first one? Thanks, Rafael ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-11 17:09 ` Rafael J. Wysocki @ 2008-10-11 17:12 ` Alan Jenkins 2008-10-11 17:53 ` Rafael J. Wysocki 0 siblings, 1 reply; 18+ messages in thread From: Alan Jenkins @ 2008-10-11 17:12 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Alexey Starikovskiy, linux acpi, linux-kernel Rafael J. Wysocki wrote: > On Saturday, 11 of October 2008, Alan Jenkins wrote: > >> I've just run an acpi-test kernel on my EeePC and noticed a new issue. >> It seems to be caused (or revealed) by the EC interrupt transaction patch. >> >> On the second suspend/resume cycle, I see a kernel error message. >> >> [ 78.747707] ACPI: Waking up from system sleep state S3 >> [ 79.330001] ACPI: EC: input buffer not empty, aborting transaction >> [ 79.423327] ACPI: EC: non-query interrupt received, switching to >> interrupt mode >> >> I still don't see any issues in the code. I'll try getting a DEBUG >> trace to see the EC interrupts. Any other suggestions? >> > > Not really, but is this reproducible? I mean, does it happen always on the > second resume and does it happen on every next resume after the first one? > > Thanks, > Rafael > Ah. No, I spoke to soon. It happened on the second resume the first two times I tried it. But this third time with DEBUG enabled, it happened on the first suspend/resume. And it doesn't happen on all subsequent resumes either. I've had one suspend/resume without the error, just after a suspend/resume with the error. So it's not deterministic, but it is easy to reproduce. Alan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-11 17:12 ` Alan Jenkins @ 2008-10-11 17:53 ` Rafael J. Wysocki 2008-10-11 18:15 ` Alan Jenkins 0 siblings, 1 reply; 18+ messages in thread From: Rafael J. Wysocki @ 2008-10-11 17:53 UTC (permalink / raw) To: Alan Jenkins; +Cc: Alexey Starikovskiy, linux acpi, linux-kernel On Saturday, 11 of October 2008, Alan Jenkins wrote: > Rafael J. Wysocki wrote: > > On Saturday, 11 of October 2008, Alan Jenkins wrote: > > > >> I've just run an acpi-test kernel on my EeePC and noticed a new issue. > >> It seems to be caused (or revealed) by the EC interrupt transaction patch. > >> > >> On the second suspend/resume cycle, I see a kernel error message. > >> > >> [ 78.747707] ACPI: Waking up from system sleep state S3 > >> [ 79.330001] ACPI: EC: input buffer not empty, aborting transaction > >> [ 79.423327] ACPI: EC: non-query interrupt received, switching to > >> interrupt mode > >> > >> I still don't see any issues in the code. I'll try getting a DEBUG > >> trace to see the EC interrupts. Any other suggestions? > >> > > > > Not really, but is this reproducible? I mean, does it happen always on the > > second resume and does it happen on every next resume after the first one? > > > > Thanks, > > Rafael > > > > Ah. No, I spoke to soon. It happened on the second resume the first > two times I tried it. But this third time with DEBUG enabled, it > happened on the first suspend/resume. > > And it doesn't happen on all subsequent resumes either. I've had one > suspend/resume without the error, just after a suspend/resume with the > error. > > So it's not deterministic, but it is easy to reproduce. I can't reproduce this on any hardware available to me, so far. Is this related to any other problem, like things not working etc.? Rafael ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-11 17:53 ` Rafael J. Wysocki @ 2008-10-11 18:15 ` Alan Jenkins 2008-10-11 18:39 ` Alexey Starikovskiy 0 siblings, 1 reply; 18+ messages in thread From: Alan Jenkins @ 2008-10-11 18:15 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Alexey Starikovskiy, linux acpi, linux-kernel Rafael J. Wysocki wrote: > On Saturday, 11 of October 2008, Alan Jenkins wrote: > >> Rafael J. Wysocki wrote: >> >>> On Saturday, 11 of October 2008, Alan Jenkins wrote: >>> >>> >>>> I've just run an acpi-test kernel on my EeePC and noticed a new issue. >>>> It seems to be caused (or revealed) by the EC interrupt transaction patch. >>>> >>>> On the second suspend/resume cycle, I see a kernel error message. >>>> >>>> [ 78.747707] ACPI: Waking up from system sleep state S3 >>>> [ 79.330001] ACPI: EC: input buffer not empty, aborting transaction >>>> [ 79.423327] ACPI: EC: non-query interrupt received, switching to >>>> interrupt mode >>>> >>>> I still don't see any issues in the code. I'll try getting a DEBUG >>>> trace to see the EC interrupts. Any other suggestions? >>>> >>>> >>> Not really, but is this reproducible? I mean, does it happen always on the >>> second resume and does it happen on every next resume after the first one? >>> >>> Thanks, >>> Rafael >>> >>> >> Ah. No, I spoke to soon. It happened on the second resume the first >> two times I tried it. But this third time with DEBUG enabled, it >> happened on the first suspend/resume. >> >> And it doesn't happen on all subsequent resumes either. I've had one >> suspend/resume without the error, just after a suspend/resume with the >> error. >> >> So it's not deterministic, but it is easy to reproduce. >> > > I can't reproduce this on any hardware available to me, so far. > > Is this related to any other problem, like things not working etc.? > Nope, just an error message. Though I do worry that "random EC transaction aborts during resume" could hit something important somewhere, sometime. I think I found the problem. The "input buffer empty" wait depends on "interrupt mode" to work properly, and we don't immediately enable the interrupt on resume. The wait should have a polling fallback anyway, to be consistent with the other transaction waits. Alan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-11 18:15 ` Alan Jenkins @ 2008-10-11 18:39 ` Alexey Starikovskiy 2008-10-11 19:30 ` Rafael J. Wysocki 0 siblings, 1 reply; 18+ messages in thread From: Alexey Starikovskiy @ 2008-10-11 18:39 UTC (permalink / raw) To: Alan Jenkins; +Cc: Rafael J. Wysocki, linux acpi, linux-kernel [-- Attachment #1: Type: text/plain, Size: 369 bytes --] Alan Jenkins wrote: > I think I found the problem. The "input buffer empty" wait depends on > "interrupt mode" to work properly, and we don't immediately enable the > interrupt on resume. The wait should have a polling fallback anyway, to > be consistent with the other transaction waits. > > Alan Yep, I think something like attached patch may help: Thanks, Alex. [-- Attachment #2: 2.patch --] [-- Type: text/x-diff, Size: 743 bytes --] ACPI: EC: Check for IBF=0 once again after timeout From: Alexey Starikovskiy <astarikovskiy@suse.de> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de> --- drivers/acpi/ec.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 69f5f78..f2902c1 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -318,7 +318,8 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t, } } if (!wait_event_timeout(ec->wait, ec_check_ibf0(ec), - msecs_to_jiffies(ACPI_EC_DELAY))) { + msecs_to_jiffies(ACPI_EC_DELAY)) && + !ec_check_ibf0(ec)) { pr_err(PREFIX "input buffer is not empty, " "aborting transaction\n"); status = -ETIME; ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-11 18:39 ` Alexey Starikovskiy @ 2008-10-11 19:30 ` Rafael J. Wysocki 2008-10-11 19:31 ` Alexey Starikovskiy 0 siblings, 1 reply; 18+ messages in thread From: Rafael J. Wysocki @ 2008-10-11 19:30 UTC (permalink / raw) To: Alexey Starikovskiy; +Cc: Alan Jenkins, linux acpi, linux-kernel On Saturday, 11 of October 2008, Alexey Starikovskiy wrote: > Alan Jenkins wrote: > > I think I found the problem. The "input buffer empty" wait depends on > > "interrupt mode" to work properly, and we don't immediately enable the > > interrupt on resume. The wait should have a polling fallback anyway, to > > be consistent with the other transaction waits. > > > > Alan > Yep, I think something like attached patch may help: [Can you please append patches instead of or apart from attaching them? That would make it easier to comment them.] if (!wait_event_timeout(ec->wait, ec_check_ibf0(ec), - msecs_to_jiffies(ACPI_EC_DELAY))) { + msecs_to_jiffies(ACPI_EC_DELAY)) && + !ec_check_ibf0(ec)) { Shouldn't this go under the spinlock? Surely it can race with the GPE handler. pr_err(PREFIX "input buffer is not empty, " "aborting transaction\n"); Thanks, Rafael ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-11 19:30 ` Rafael J. Wysocki @ 2008-10-11 19:31 ` Alexey Starikovskiy 2008-10-11 19:40 ` Rafael J. Wysocki 0 siblings, 1 reply; 18+ messages in thread From: Alexey Starikovskiy @ 2008-10-11 19:31 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Alexey Starikovskiy, Alan Jenkins, linux acpi, linux-kernel Rafael J. Wysocki wrote: > On Saturday, 11 of October 2008, Alexey Starikovskiy wrote: > >> Alan Jenkins wrote: >> >>> I think I found the problem. The "input buffer empty" wait depends on >>> "interrupt mode" to work properly, and we don't immediately enable the >>> interrupt on resume. The wait should have a polling fallback anyway, to >>> be consistent with the other transaction waits. >>> >>> Alan >>> >> Yep, I think something like attached patch may help: >> > > [Can you please append patches instead of or apart from attaching them? > That would make it easier to comment them.] > > Ok. > if (!wait_event_timeout(ec->wait, ec_check_ibf0(ec), > - msecs_to_jiffies(ACPI_EC_DELAY))) { > + msecs_to_jiffies(ACPI_EC_DELAY)) && > + !ec_check_ibf0(ec)) { > > Shouldn't this go under the spinlock? Surely it can race with the GPE handler. > > No, we discussed this before -- we are outside of the transaction, thus no GPE activity could interfere with ec_check_ibf0. Regards, Alex. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-11 19:31 ` Alexey Starikovskiy @ 2008-10-11 19:40 ` Rafael J. Wysocki 2008-10-11 19:38 ` Alexey Starikovskiy 0 siblings, 1 reply; 18+ messages in thread From: Rafael J. Wysocki @ 2008-10-11 19:40 UTC (permalink / raw) To: Alexey Starikovskiy Cc: Alexey Starikovskiy, Alan Jenkins, linux acpi, linux-kernel On Saturday, 11 of October 2008, Alexey Starikovskiy wrote: > Rafael J. Wysocki wrote: > > On Saturday, 11 of October 2008, Alexey Starikovskiy wrote: > > > >> Alan Jenkins wrote: > >> > >>> I think I found the problem. The "input buffer empty" wait depends on > >>> "interrupt mode" to work properly, and we don't immediately enable the > >>> interrupt on resume. The wait should have a polling fallback anyway, to > >>> be consistent with the other transaction waits. > >>> > >>> Alan > >>> > >> Yep, I think something like attached patch may help: > >> > > > > [Can you please append patches instead of or apart from attaching them? > > That would make it easier to comment them.] > > > > > Ok. > > if (!wait_event_timeout(ec->wait, ec_check_ibf0(ec), > > - msecs_to_jiffies(ACPI_EC_DELAY))) { > > + msecs_to_jiffies(ACPI_EC_DELAY)) && > > + !ec_check_ibf0(ec)) { > > > > Shouldn't this go under the spinlock? Surely it can race with the GPE handler. > > > > > No, we discussed this before -- we are outside of the transaction, thus > no GPE > activity could interfere with ec_check_ibf0. Ok, this is in the process context and we don't really expect to get an interrupt at this point, but what happens if the EC generates an event that's not related to any transiaction. Is that guaranteed to never happen? Thanks, Rafael ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-11 19:40 ` Rafael J. Wysocki @ 2008-10-11 19:38 ` Alexey Starikovskiy 2008-10-11 20:54 ` Rafael J. Wysocki 0 siblings, 1 reply; 18+ messages in thread From: Alexey Starikovskiy @ 2008-10-11 19:38 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Alexey Starikovskiy, Alan Jenkins, linux acpi, linux-kernel Rafael J. Wysocki wrote: >> No, we discussed this before -- we are outside of the transaction, thus >> no GPE >> activity could interfere with ec_check_ibf0. > > Ok, this is in the process context and we don't really expect to get an > interrupt at this point, but what happens if the EC generates an event that's > not related to any transiaction. Is that guaranteed to never happen? Interrupt handler in this case can't cause a change to status register, thus our read of it will not be affected by interrupt. > > Thanks, > Rafael ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-11 19:38 ` Alexey Starikovskiy @ 2008-10-11 20:54 ` Rafael J. Wysocki 2008-10-12 9:13 ` Alan Jenkins 0 siblings, 1 reply; 18+ messages in thread From: Rafael J. Wysocki @ 2008-10-11 20:54 UTC (permalink / raw) To: Alexey Starikovskiy Cc: Alexey Starikovskiy, Alan Jenkins, linux acpi, linux-kernel On Saturday, 11 of October 2008, Alexey Starikovskiy wrote: > Rafael J. Wysocki wrote: > >> No, we discussed this before -- we are outside of the transaction, thus > >> no GPE > >> activity could interfere with ec_check_ibf0. > > > > Ok, this is in the process context and we don't really expect to get an > > interrupt at this point, but what happens if the EC generates an event that's > > not related to any transiaction. Is that guaranteed to never happen? > Interrupt handler in this case can't cause a change to status register, thus our > read of it will not be affected by interrupt. Ok, thanks. Alan, does the patch work for you? Rafael ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-11 20:54 ` Rafael J. Wysocki @ 2008-10-12 9:13 ` Alan Jenkins 2008-10-12 19:23 ` Alexey Starikovskiy 0 siblings, 1 reply; 18+ messages in thread From: Alan Jenkins @ 2008-10-12 9:13 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Alexey Starikovskiy, Alexey Starikovskiy, linux acpi, linux-kernel Rafael J. Wysocki wrote: > On Saturday, 11 of October 2008, Alexey Starikovskiy wrote: > >> Rafael J. Wysocki wrote: >> >>>> No, we discussed this before -- we are outside of the transaction, thus >>>> no GPE >>>> activity could interfere with ec_check_ibf0. >>>> >>> Ok, this is in the process context and we don't really expect to get an >>> interrupt at this point, but what happens if the EC generates an event that's >>> not related to any transiaction. Is that guaranteed to never happen? >>> >> Interrupt handler in this case can't cause a change to status register, thus our >> read of it will not be affected by interrupt. >> > > Ok, thanks. > > Alan, does the patch work for you? > > Rafael > Yes. Two reboot cycles, three suspend/resume cycles each, and no error message. I hope we have a better fix in mind though :-P. The patch doesn't solve the unnecessary 500ms delay when this thing happens. Thanks Alan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-12 9:13 ` Alan Jenkins @ 2008-10-12 19:23 ` Alexey Starikovskiy 2008-10-13 5:56 ` Zhao Yakui 2008-10-13 8:22 ` Alan Jenkins 0 siblings, 2 replies; 18+ messages in thread From: Alexey Starikovskiy @ 2008-10-12 19:23 UTC (permalink / raw) To: Alan Jenkins Cc: Rafael J. Wysocki, Alexey Starikovskiy, linux acpi, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1038 bytes --] Alan Jenkins wrote: > Rafael J. Wysocki wrote: >> On Saturday, 11 of October 2008, Alexey Starikovskiy wrote: >> >>> Rafael J. Wysocki wrote: >>> >>>>> No, we discussed this before -- we are outside of the transaction, thus >>>>> no GPE >>>>> activity could interfere with ec_check_ibf0. >>>>> >>>> Ok, this is in the process context and we don't really expect to get an >>>> interrupt at this point, but what happens if the EC generates an event that's >>>> not related to any transiaction. Is that guaranteed to never happen? >>>> >>> Interrupt handler in this case can't cause a change to status register, thus our >>> read of it will not be affected by interrupt. >>> >> Ok, thanks. >> >> Alan, does the patch work for you? >> >> Rafael >> > > Yes. Two reboot cycles, three suspend/resume cycles each, and no error > message. > > I hope we have a better fix in mind though :-P. The patch doesn't solve > the unnecessary 500ms delay when this thing happens. Something like this? Regards, Alex. [-- Attachment #2: 2.patch --] [-- Type: text/x-diff, Size: 1225 bytes --] ACPI: EC: Check for IBF=0 once again after timeout From: Alexey Starikovskiy <astarikovskiy@suse.de> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de> --- drivers/acpi/ec.c | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 69f5f78..5e7c9c5 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -300,6 +300,17 @@ static int ec_check_ibf0(struct acpi_ec *ec) return (status & ACPI_EC_FLAG_IBF) == 0; } +static int ec_wait_ibf0(struct acpi_ec *ec) +{ + unsigned long delay = jiffies + msecs_to_jiffies(ACPI_EC_DELAY); + while (time_before(jiffies, delay)) { + if (wait_event_timeout(ec->wait, ec_check_ibf0(ec), + msecs_to_jiffies(1))) + return 0; + } + return -ETIME; +} + static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t, int force_poll) { @@ -317,8 +328,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t, goto unlock; } } - if (!wait_event_timeout(ec->wait, ec_check_ibf0(ec), - msecs_to_jiffies(ACPI_EC_DELAY))) { + if (ec_wait_ibf0(ec)) { pr_err(PREFIX "input buffer is not empty, " "aborting transaction\n"); status = -ETIME; ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-12 19:23 ` Alexey Starikovskiy @ 2008-10-13 5:56 ` Zhao Yakui 2008-10-13 8:22 ` Alan Jenkins 1 sibling, 0 replies; 18+ messages in thread From: Zhao Yakui @ 2008-10-13 5:56 UTC (permalink / raw) To: Alexey Starikovskiy Cc: Alan Jenkins, Rafael J. Wysocki, Alexey Starikovskiy, linux acpi, linux-kernel On Sun, 2008-10-12 at 23:23 +0400, Alexey Starikovskiy wrote: > +static int ec_wait_ibf0(struct acpi_ec *ec) > +{ > + unsigned long delay = jiffies + > msecs_to_jiffies(ACPI_EC_DELAY); > + while (time_before(jiffies, delay)) { > + if (wait_event_timeout(ec->wait, ec_check_ibf0(ec), > + msecs_to_jiffies(1))) > + return 0; Please add the bogus timeout check. best regards. Yakui > + } > + return -ETIME; > +} > + ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-12 19:23 ` Alexey Starikovskiy 2008-10-13 5:56 ` Zhao Yakui @ 2008-10-13 8:22 ` Alan Jenkins 2008-10-13 16:39 ` Alan Jenkins 1 sibling, 1 reply; 18+ messages in thread From: Alan Jenkins @ 2008-10-13 8:22 UTC (permalink / raw) To: Alexey Starikovskiy Cc: Rafael J. Wysocki, Alexey Starikovskiy, linux acpi, linux-kernel Alexis Starikovskiy wrote: > Alan Jenkins wrote: >> Rafael J. Wysocki wrote: >>> On Saturday, 11 of October 2008, Alexey Starikovskiy wrote: >>> >>>> Rafael J. Wysocki wrote: >>>> >>>>>> No, we discussed this before -- we are outside of the >>>>>> transaction, thus no GPE >>>>>> activity could interfere with ec_check_ibf0. >>>>>> >>>>> Ok, this is in the process context and we don't really expect to >>>>> get an >>>>> interrupt at this point, but what happens if the EC generates an >>>>> event that's >>>>> not related to any transiaction. Is that guaranteed to never happen? >>>>> >>>> Interrupt handler in this case can't cause a change to status >>>> register, thus our read of it will not be affected by interrupt. >>>> >>> Ok, thanks. >>> >>> Alan, does the patch work for you? >>> >>> Rafael >>> >> >> Yes. Two reboot cycles, three suspend/resume cycles each, and no error >> message. >> >> I hope we have a better fix in mind though :-P. The patch doesn't solve >> the unnecessary 500ms delay when this thing happens. > > Something like this? > > Regards, > Alex. You sent it as an attachment again :-). That should work, odd as it looks. We don't need to worry about the GPE workaround because that's only active _inside_ the transaction. I don't know what Zhao thinks is missing. Sorry I can't test right now. I tried to install 3D support on my laptop for showing-off purposes, and somehow broke X. Thanks Alan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: acpi-test tree on eeepc: EC error message on second resume 2008-10-13 8:22 ` Alan Jenkins @ 2008-10-13 16:39 ` Alan Jenkins 2008-10-15 22:02 ` [PATCH] ACPI: EC: Check for IBF=0 periodically if not in GPE mode Alexey Starikovskiy 0 siblings, 1 reply; 18+ messages in thread From: Alan Jenkins @ 2008-10-13 16:39 UTC (permalink / raw) To: Alexey Starikovskiy Cc: Rafael J. Wysocki, Alexey Starikovskiy, linux acpi, linux-kernel Alan Jenkins wrote: > Alexis Starikovskiy wrote: > >> Alan Jenkins wrote: >> >>> Rafael J. Wysocki wrote: >>> >>>> On Saturday, 11 of October 2008, Alexey Starikovskiy wrote: >>>> >>>> >>>>> Rafael J. Wysocki wrote: >>>>> >>>>> >>>>>>> No, we discussed this before -- we are outside of the >>>>>>> transaction, thus no GPE >>>>>>> activity could interfere with ec_check_ibf0. >>>>>>> >>>>>>> >>>>>> Ok, this is in the process context and we don't really expect to >>>>>> get an >>>>>> interrupt at this point, but what happens if the EC generates an >>>>>> event that's >>>>>> not related to any transiaction. Is that guaranteed to never happen? >>>>>> >>>>>> >>>>> Interrupt handler in this case can't cause a change to status >>>>> register, thus our read of it will not be affected by interrupt. >>>>> >>>>> >>>> Ok, thanks. >>>> >>>> Alan, does the patch work for you? >>>> >>>> Rafael >>>> >>>> >>> Yes. Two reboot cycles, three suspend/resume cycles each, and no error >>> message. >>> >>> I hope we have a better fix in mind though :-P. The patch doesn't solve >>> the unnecessary 500ms delay when this thing happens. >>> >> Something like this? >> >> Regards, >> Alex. >> > > You sent it as an attachment again :-). > > That should work, odd as it looks. We don't need to worry about the GPE > workaround because that's only active _inside_ the transaction. I don't > know what Zhao thinks is missing. > > Sorry I can't test right now. I tried to install 3D support on my > laptop for showing-off purposes, and somehow broke X. > Drama over, I've now tested it. No error messages, and the printk timings show that it has stopped hanging for half a second. Thanks Alan ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ACPI: EC: Check for IBF=0 periodically if not in GPE mode 2008-10-13 16:39 ` Alan Jenkins @ 2008-10-15 22:02 ` Alexey Starikovskiy 2008-10-16 22:14 ` Len Brown 0 siblings, 1 reply; 18+ messages in thread From: Alexey Starikovskiy @ 2008-10-15 22:02 UTC (permalink / raw) To: LenBrown; +Cc: Linux-acpi Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de> --- drivers/acpi/ec.c | 15 +++++++++++++-- 1 files changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 69f5f78..f9c0782 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -300,6 +300,18 @@ static int ec_check_ibf0(struct acpi_ec *ec) return (status & ACPI_EC_FLAG_IBF) == 0; } +static int ec_wait_ibf0(struct acpi_ec *ec) +{ + unsigned long delay = jiffies + msecs_to_jiffies(ACPI_EC_DELAY); + /* interrupt wait manually if GPE mode is not active */ + unsigned long timeout = test_bit(EC_FLAGS_GPE_MODE, &ec->flags) ? + msecs_to_jiffies(ACPI_EC_DELAY) : msecs_to_jiffies(1); + while (time_before(jiffies, delay)) + if (wait_event_timeout(ec->wait, ec_check_ibf0(ec), timeout)) + return 0; + return -ETIME; +} + static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t, int force_poll) { @@ -317,8 +329,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t, goto unlock; } } - if (!wait_event_timeout(ec->wait, ec_check_ibf0(ec), - msecs_to_jiffies(ACPI_EC_DELAY))) { + if (ec_wait_ibf0(ec)) { pr_err(PREFIX "input buffer is not empty, " "aborting transaction\n"); status = -ETIME; ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] ACPI: EC: Check for IBF=0 periodically if not in GPE mode 2008-10-15 22:02 ` [PATCH] ACPI: EC: Check for IBF=0 periodically if not in GPE mode Alexey Starikovskiy @ 2008-10-16 22:14 ` Len Brown 0 siblings, 0 replies; 18+ messages in thread From: Len Brown @ 2008-10-16 22:14 UTC (permalink / raw) To: Alexey Starikovskiy; +Cc: Linux-acpi applied to acpi tree, ec branch. thanks, -len On Thu, 16 Oct 2008, Alexey Starikovskiy wrote: > Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de> > --- > > drivers/acpi/ec.c | 15 +++++++++++++-- > 1 files changed, 13 insertions(+), 2 deletions(-) > > > diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c > index 69f5f78..f9c0782 100644 > --- a/drivers/acpi/ec.c > +++ b/drivers/acpi/ec.c > @@ -300,6 +300,18 @@ static int ec_check_ibf0(struct acpi_ec *ec) > return (status & ACPI_EC_FLAG_IBF) == 0; > } > > +static int ec_wait_ibf0(struct acpi_ec *ec) > +{ > + unsigned long delay = jiffies + msecs_to_jiffies(ACPI_EC_DELAY); > + /* interrupt wait manually if GPE mode is not active */ > + unsigned long timeout = test_bit(EC_FLAGS_GPE_MODE, &ec->flags) ? > + msecs_to_jiffies(ACPI_EC_DELAY) : msecs_to_jiffies(1); > + while (time_before(jiffies, delay)) > + if (wait_event_timeout(ec->wait, ec_check_ibf0(ec), timeout)) > + return 0; > + return -ETIME; > +} > + > static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t, > int force_poll) > { > @@ -317,8 +329,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t, > goto unlock; > } > } > - if (!wait_event_timeout(ec->wait, ec_check_ibf0(ec), > - msecs_to_jiffies(ACPI_EC_DELAY))) { > + if (ec_wait_ibf0(ec)) { > pr_err(PREFIX "input buffer is not empty, " > "aborting transaction\n"); > status = -ETIME; > > -- > 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] 18+ messages in thread
end of thread, other threads:[~2008-10-16 22:15 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-10-11 16:57 acpi-test tree on eeepc: EC error message on second resume Alan Jenkins 2008-10-11 17:09 ` Rafael J. Wysocki 2008-10-11 17:12 ` Alan Jenkins 2008-10-11 17:53 ` Rafael J. Wysocki 2008-10-11 18:15 ` Alan Jenkins 2008-10-11 18:39 ` Alexey Starikovskiy 2008-10-11 19:30 ` Rafael J. Wysocki 2008-10-11 19:31 ` Alexey Starikovskiy 2008-10-11 19:40 ` Rafael J. Wysocki 2008-10-11 19:38 ` Alexey Starikovskiy 2008-10-11 20:54 ` Rafael J. Wysocki 2008-10-12 9:13 ` Alan Jenkins 2008-10-12 19:23 ` Alexey Starikovskiy 2008-10-13 5:56 ` Zhao Yakui 2008-10-13 8:22 ` Alan Jenkins 2008-10-13 16:39 ` Alan Jenkins 2008-10-15 22:02 ` [PATCH] ACPI: EC: Check for IBF=0 periodically if not in GPE mode Alexey Starikovskiy 2008-10-16 22:14 ` Len Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox