From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexey Starikovskiy Subject: Re: [PATCH 3/5] ACPI: EC: wait for last write gpe Date: Tue, 11 Nov 2008 23:13:36 +0300 Message-ID: <4919E770.4050607@gmail.com> References: <20081109155847.13635.15244.stgit@thinkpad> <20081109160050.13635.46234.stgit@thinkpad> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from ug-out-1314.google.com ([66.249.92.174]:4259 "EHLO ug-out-1314.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751353AbYKKUNj (ORCPT ); Tue, 11 Nov 2008 15:13:39 -0500 Received: by ug-out-1314.google.com with SMTP id 39so844632ugf.37 for ; Tue, 11 Nov 2008 12:13:38 -0800 (PST) In-Reply-To: Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Len Brown Cc: Alexey Starikovskiy , Linux-acpi@vger.kernel.org Len Brown wrote: > applied to acpi-test. > > we struggled with ec burst mode for a long time before enabling it > finally in 2005. have we had any problems with it since then > that might be addressed by this race? > > hard to say, I was thinking that with the fast transaction, we may fall into this window, but it does not help in any of the current bug reports. regards, Alex. > thanks, > -Len > > > On Sun, 9 Nov 2008, Alexey Starikovskiy wrote: > > >> There is a possibility that EC might break if next command is >> issued within 1 us after write or burst-disable command. >> This "possibility" was in EC driver for 3.5 years, after >> 451566f45a2e6cd10ba56e7220a9dd84ba3ef550. >> >> References: http://marc.info/?l=linux-acpi&m=122616284402886&w=4 >> Cc: Zhao Yakui >> Signed-off-by: Alexey Starikovskiy >> --- >> >> drivers/acpi/ec.c | 21 +++++++++++++-------- >> 1 files changed, 13 insertions(+), 8 deletions(-) >> >> >> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c >> index e5dbe21..d6007ac 100644 >> --- a/drivers/acpi/ec.c >> +++ b/drivers/acpi/ec.c >> @@ -102,6 +102,7 @@ struct transaction { >> u8 command; >> u8 wlen; >> u8 rlen; >> + bool done; >> }; >> >> static struct acpi_ec { >> @@ -178,7 +179,7 @@ static int ec_transaction_done(struct acpi_ec *ec) >> unsigned long flags; >> int ret = 0; >> spin_lock_irqsave(&ec->curr_lock, flags); >> - if (!ec->curr || (!ec->curr->wlen && !ec->curr->rlen)) >> + if (!ec->curr || ec->curr->done) >> ret = 1; >> spin_unlock_irqrestore(&ec->curr_lock, flags); >> return ret; >> @@ -195,17 +196,20 @@ static void gpe_transaction(struct acpi_ec *ec, u8 status) >> acpi_ec_write_data(ec, *(ec->curr->wdata++)); >> --ec->curr->wlen; >> } else >> - /* false interrupt, state didn't change */ >> - ++ec->curr->irq_count; >> - >> + goto err; >> } else if (ec->curr->rlen > 0) { >> if ((status & ACPI_EC_FLAG_OBF) == 1) { >> *(ec->curr->rdata++) = acpi_ec_read_data(ec); >> - --ec->curr->rlen; >> + if (--ec->curr->rlen == 0) >> + ec->curr->done = true; >> } else >> - /* false interrupt, state didn't change */ >> - ++ec->curr->irq_count; >> - } >> + goto err; >> + } else if (ec->curr->wlen == 0 && (status & ACPI_EC_FLAG_IBF) == 0) >> + ec->curr->done = true; >> + goto unlock; >> +err: >> + /* false interrupt, state didn't change */ >> + ++ec->curr->irq_count; >> unlock: >> spin_unlock_irqrestore(&ec->curr_lock, flags); >> } >> @@ -265,6 +269,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, >> spin_lock_irqsave(&ec->curr_lock, tmp); >> /* following two actions should be kept atomic */ >> t->irq_count = 0; >> + t->done = false; >> ec->curr = t; >> acpi_ec_write_cmd(ec, ec->curr->command); >> if (ec->curr->command == ACPI_EC_COMMAND_QUERY) >> >> -- >> 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 >