From 7c6db4e050601f359081fde418ca6dc4fc2d0011 Mon Sep 17 00:00:00 2001 From: Alexey Starikovskiy Date: Thu, 25 Sep 2008 21:00:31 +0400 Subject: [PATCH 1/4] ACPI: EC: do transaction from interrupt context It is easier and faster to do transaction directly from interrupt context rather than waking control thread. Also, cleaner GPE storm avoidance is implemented. References: http://bugzilla.kernel.org/show_bug.cgi?id=9998 http://bugzilla.kernel.org/show_bug.cgi?id=10724 http://bugzilla.kernel.org/show_bug.cgi?id=10919 http://bugzilla.kernel.org/show_bug.cgi?id=11309 http://bugzilla.kernel.org/show_bug.cgi?id=11549 Signed-off-by: Alexey Starikovskiy Tested-by: Sitsofe Wheeler Signed-off-by: Len Brown --- drivers/acpi/ec.c | 321 ++++++++++++++++++++++------------------------ 1 file changed, 155 insertions(+), 166 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 13593f9f2197..7f0d81c0ec99 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1,7 +1,7 @@ /* - * ec.c - ACPI Embedded Controller Driver (v2.0) + * ec.c - ACPI Embedded Controller Driver (v2.1) * - * Copyright (C) 2006, 2007 Alexey Starikovskiy + * Copyright (C) 2006-2008 Alexey Starikovskiy * Copyright (C) 2006 Denis Sadykov * Copyright (C) 2004 Luming Yu * Copyright (C) 2001, 2002 Andy Grover @@ -26,7 +26,7 @@ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ */ -/* Uncomment next line to get verbose print outs*/ +/* Uncomment next line to get verbose printout */ /* #define DEBUG */ #include @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -65,22 +66,21 @@ enum ec_command { ACPI_EC_COMMAND_QUERY = 0x84, }; -/* EC events */ -enum ec_event { - ACPI_EC_EVENT_OBF_1 = 1, /* Output buffer full */ - ACPI_EC_EVENT_IBF_0, /* Input buffer empty */ -}; - #define ACPI_EC_DELAY 500 /* Wait 500ms max. during EC ops */ #define ACPI_EC_UDELAY_GLK 1000 /* Wait 1ms max. to get global lock */ #define ACPI_EC_UDELAY 100 /* Wait 100us before polling EC again */ +#define ACPI_EC_STORM_THRESHOLD 20 /* number of false interrupts + per one transaction */ + enum { - EC_FLAGS_WAIT_GPE = 0, /* Don't check status until GPE arrives */ EC_FLAGS_QUERY_PENDING, /* Query is pending */ - EC_FLAGS_GPE_MODE, /* Expect GPE to be sent for status change */ + EC_FLAGS_GPE_MODE, /* Expect GPE to be sent + * for status change */ EC_FLAGS_NO_GPE, /* Don't use GPE mode */ - EC_FLAGS_RESCHEDULE_POLL /* Re-schedule poll */ + EC_FLAGS_GPE_STORM, /* GPE storm detected */ + EC_FLAGS_HANDLERS_INSTALLED /* Handlers for GPE and + * OpReg are installed */ }; /* If we find an EC via the ECDT, we need to keep a ptr to its context */ @@ -95,6 +95,14 @@ struct acpi_ec_query_handler { u8 query_bit; }; +struct transaction_data { + const u8 *wdata; + u8 *rdata; + unsigned short irq_count; + u8 wlen; + u8 rlen; +}; + static struct acpi_ec { acpi_handle handle; unsigned long gpe; @@ -105,9 +113,8 @@ static struct acpi_ec { struct mutex lock; wait_queue_head_t wait; struct list_head list; - struct delayed_work work; - atomic_t irq_count; - u8 handlers_installed; + struct transaction_data *t; + spinlock_t t_lock; } *boot_ec, *first_ec; /* @@ -150,7 +157,7 @@ static inline u8 acpi_ec_read_data(struct acpi_ec *ec) { u8 x = inb(ec->data_addr); pr_debug(PREFIX "---> data = 0x%2.2x\n", x); - return inb(ec->data_addr); + return x; } static inline void acpi_ec_write_cmd(struct acpi_ec *ec, u8 command) @@ -165,68 +172,79 @@ static inline void acpi_ec_write_data(struct acpi_ec *ec, u8 data) outb(data, ec->data_addr); } -static inline int acpi_ec_check_status(struct acpi_ec *ec, enum ec_event event) +static int ec_transaction_done(struct acpi_ec *ec) { - if (test_bit(EC_FLAGS_WAIT_GPE, &ec->flags)) - return 0; - if (event == ACPI_EC_EVENT_OBF_1) { - if (acpi_ec_read_status(ec) & ACPI_EC_FLAG_OBF) - return 1; - } else if (event == ACPI_EC_EVENT_IBF_0) { - if (!(acpi_ec_read_status(ec) & ACPI_EC_FLAG_IBF)) - return 1; - } + unsigned long flags; + int ret = 0; + spin_lock_irqsave(&ec->t_lock, flags); + if (!ec->t || (!ec->t->wlen && !ec->t->rlen)) + ret = 1; + spin_unlock_irqrestore(&ec->t_lock, flags); + return ret; +} +static void gpe_transaction(struct acpi_ec *ec, u8 status) +{ + unsigned long flags; + spin_lock_irqsave(&ec->t_lock, flags); + if (!ec->t) + goto unlock; + if (ec->t->wlen > 0) { + if ((status & ACPI_EC_FLAG_IBF) == 0) { + acpi_ec_write_data(ec, *(ec->t->wdata++)); + --ec->t->wlen; + } else + /* false interrupt, state didn't change */ + ++ec->t->irq_count; + + } else if (ec->t->rlen > 0) { + if ((status & ACPI_EC_FLAG_OBF) == 1) { + *(ec->t->rdata++) = acpi_ec_read_data(ec); + --ec->t->rlen; + } else + /* false interrupt, state didn't change */ + ++ec->t->irq_count; + } +unlock: + spin_unlock_irqrestore(&ec->t_lock, flags); +} + +static int acpi_ec_wait(struct acpi_ec *ec) +{ + if (wait_event_timeout(ec->wait, ec_transaction_done(ec), + msecs_to_jiffies(ACPI_EC_DELAY))) + return 0; + /* missing GPEs, switch back to poll mode */ + if (printk_ratelimit()) + pr_info(PREFIX "missing confirmations, " + "switch off interrupt mode.\n"); + set_bit(EC_FLAGS_NO_GPE, &ec->flags); + clear_bit(EC_FLAGS_GPE_MODE, &ec->flags); + return 1; +} + +static void acpi_ec_gpe_query(void *ec_cxt); + +static int ec_check_sci(struct acpi_ec *ec, u8 state) +{ + if (state & ACPI_EC_FLAG_SCI) { + if (!test_and_set_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) + return acpi_os_execute(OSL_EC_BURST_HANDLER, + acpi_ec_gpe_query, ec); + } return 0; } -static void ec_schedule_ec_poll(struct acpi_ec *ec) +static int ec_poll(struct acpi_ec *ec) { - if (test_bit(EC_FLAGS_RESCHEDULE_POLL, &ec->flags)) - schedule_delayed_work(&ec->work, - msecs_to_jiffies(ACPI_EC_DELAY)); -} - -static void ec_switch_to_poll_mode(struct acpi_ec *ec) -{ - set_bit(EC_FLAGS_NO_GPE, &ec->flags); - clear_bit(EC_FLAGS_GPE_MODE, &ec->flags); - acpi_disable_gpe(NULL, ec->gpe, ACPI_NOT_ISR); - set_bit(EC_FLAGS_RESCHEDULE_POLL, &ec->flags); -} - -static int acpi_ec_wait(struct acpi_ec *ec, enum ec_event event, int force_poll) -{ - atomic_set(&ec->irq_count, 0); - if (likely(test_bit(EC_FLAGS_GPE_MODE, &ec->flags)) && - likely(!force_poll)) { - if (wait_event_timeout(ec->wait, acpi_ec_check_status(ec, event), - msecs_to_jiffies(ACPI_EC_DELAY))) - return 0; - clear_bit(EC_FLAGS_WAIT_GPE, &ec->flags); - if (acpi_ec_check_status(ec, event)) { - /* missing GPEs, switch back to poll mode */ - if (printk_ratelimit()) - pr_info(PREFIX "missing confirmations, " - "switch off interrupt mode.\n"); - ec_switch_to_poll_mode(ec); - ec_schedule_ec_poll(ec); - return 0; - } - } else { - unsigned long delay = jiffies + msecs_to_jiffies(ACPI_EC_DELAY); - clear_bit(EC_FLAGS_WAIT_GPE, &ec->flags); - while (time_before(jiffies, delay)) { - if (acpi_ec_check_status(ec, event)) - return 0; - msleep(1); - } - if (acpi_ec_check_status(ec,event)) + unsigned long delay = jiffies + msecs_to_jiffies(ACPI_EC_DELAY); + msleep(1); + while (time_before(jiffies, delay)) { + gpe_transaction(ec, acpi_ec_read_status(ec)); + msleep(1); + if (ec_transaction_done(ec)) return 0; } - pr_err(PREFIX "acpi_ec_wait timeout, status = 0x%2.2x, event = %s\n", - acpi_ec_read_status(ec), - (event == ACPI_EC_EVENT_OBF_1) ? "\"b0=1\"" : "\"b1=0\""); return -ETIME; } @@ -235,45 +253,51 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, u8 command, u8 * rdata, unsigned rdata_len, int force_poll) { - int result = 0; - set_bit(EC_FLAGS_WAIT_GPE, &ec->flags); + unsigned long tmp; + struct transaction_data t = {.wdata = wdata, .rdata = rdata, + .wlen = wdata_len, .rlen = rdata_len, + .irq_count = 0}; + int ret = 0; pr_debug(PREFIX "transaction start\n"); + /* disable GPE during transaction if storm is detected */ + if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) { + clear_bit(EC_FLAGS_GPE_MODE, &ec->flags); + acpi_disable_gpe(NULL, ec->gpe, ACPI_NOT_ISR); + } + /* start transaction */ + spin_lock_irqsave(&ec->t_lock, tmp); + /* following two actions should be kept atomic */ + ec->t = &t; acpi_ec_write_cmd(ec, command); - for (; wdata_len > 0; --wdata_len) { - result = acpi_ec_wait(ec, ACPI_EC_EVENT_IBF_0, force_poll); - if (result) { - pr_err(PREFIX - "write_cmd timeout, command = %d\n", command); - goto end; - } - set_bit(EC_FLAGS_WAIT_GPE, &ec->flags); - acpi_ec_write_data(ec, *(wdata++)); - } - - if (!rdata_len) { - result = acpi_ec_wait(ec, ACPI_EC_EVENT_IBF_0, force_poll); - if (result) { - pr_err(PREFIX - "finish-write timeout, command = %d\n", command); - goto end; - } - } else if (command == ACPI_EC_COMMAND_QUERY) + if (command == ACPI_EC_COMMAND_QUERY) clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags); - - for (; rdata_len > 0; --rdata_len) { - result = acpi_ec_wait(ec, ACPI_EC_EVENT_OBF_1, force_poll); - if (result) { - pr_err(PREFIX "read timeout, command = %d\n", command); - goto end; - } - /* Don't expect GPE after last read */ - if (rdata_len > 1) - set_bit(EC_FLAGS_WAIT_GPE, &ec->flags); - *(rdata++) = acpi_ec_read_data(ec); - } - end: + spin_unlock_irqrestore(&ec->t_lock, tmp); + /* if we selected poll mode or failed in GPE-mode do a poll loop */ + if (force_poll || + !test_bit(EC_FLAGS_GPE_MODE, &ec->flags) || + acpi_ec_wait(ec)) + ret = ec_poll(ec); pr_debug(PREFIX "transaction end\n"); - return result; + spin_lock_irqsave(&ec->t_lock, tmp); + ec->t = NULL; + spin_unlock_irqrestore(&ec->t_lock, tmp); + if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) { + /* check if we received SCI during transaction */ + ec_check_sci(ec, acpi_ec_read_status(ec)); + /* it is safe to enable GPE outside of transaction */ + acpi_enable_gpe(NULL, ec->gpe, ACPI_NOT_ISR); + } else if (test_bit(EC_FLAGS_GPE_MODE, &ec->flags) && + t.irq_count > ACPI_EC_STORM_THRESHOLD) { + pr_debug(PREFIX "GPE storm detected\n"); + set_bit(EC_FLAGS_GPE_STORM, &ec->flags); + } + return ret; +} + +static int ec_check_ibf0(struct acpi_ec *ec) +{ + u8 status = acpi_ec_read_status(ec); + return (status & ACPI_EC_FLAG_IBF) == 0; } static int acpi_ec_transaction(struct acpi_ec *ec, u8 command, @@ -283,40 +307,34 @@ static int acpi_ec_transaction(struct acpi_ec *ec, u8 command, { int status; u32 glk; - if (!ec || (wdata_len && !wdata) || (rdata_len && !rdata)) return -EINVAL; - if (rdata) memset(rdata, 0, rdata_len); - mutex_lock(&ec->lock); if (ec->global_lock) { status = acpi_acquire_global_lock(ACPI_EC_UDELAY_GLK, &glk); if (ACPI_FAILURE(status)) { - mutex_unlock(&ec->lock); - return -ENODEV; + status = -ENODEV; + goto unlock; } } - - status = acpi_ec_wait(ec, ACPI_EC_EVENT_IBF_0, 0); - if (status) { + if (!wait_event_timeout(ec->wait, ec_check_ibf0(ec), + msecs_to_jiffies(ACPI_EC_DELAY))) { pr_err(PREFIX "input buffer is not empty, " "aborting transaction\n"); + status = -ETIME; goto end; } - status = acpi_ec_transaction_unlocked(ec, command, wdata, wdata_len, rdata, rdata_len, force_poll); - - end: - +end: if (ec->global_lock) acpi_release_global_lock(glk); +unlock: mutex_unlock(&ec->lock); - return status; } @@ -332,7 +350,9 @@ int acpi_ec_burst_enable(struct acpi_ec *ec) int acpi_ec_burst_disable(struct acpi_ec *ec) { - return acpi_ec_transaction(ec, ACPI_EC_BURST_DISABLE, NULL, 0, NULL, 0, 0); + return (acpi_ec_read_status(ec) & ACPI_EC_FLAG_BURST) ? + acpi_ec_transaction(ec, ACPI_EC_BURST_DISABLE, + NULL, 0, NULL, 0, 0) : 0; } static int acpi_ec_read(struct acpi_ec *ec, u8 address, u8 * data) @@ -513,46 +533,26 @@ static void acpi_ec_gpe_query(void *ec_cxt) static u32 acpi_ec_gpe_handler(void *data) { - acpi_status status = AE_OK; struct acpi_ec *ec = data; - u8 state = acpi_ec_read_status(ec); + u8 status; pr_debug(PREFIX "~~~> interrupt\n"); - atomic_inc(&ec->irq_count); - if (atomic_read(&ec->irq_count) > 5) { - pr_err(PREFIX "GPE storm detected, disabling EC GPE\n"); - ec_switch_to_poll_mode(ec); - goto end; - } - clear_bit(EC_FLAGS_WAIT_GPE, &ec->flags); - if (test_bit(EC_FLAGS_GPE_MODE, &ec->flags)) + status = acpi_ec_read_status(ec); + + gpe_transaction(ec, status); + if (ec_transaction_done(ec) && (status & ACPI_EC_FLAG_IBF) == 0) wake_up(&ec->wait); - if (state & ACPI_EC_FLAG_SCI) { - if (!test_and_set_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) - status = acpi_os_execute(OSL_EC_BURST_HANDLER, - acpi_ec_gpe_query, ec); - } else if (!test_bit(EC_FLAGS_GPE_MODE, &ec->flags) && - !test_bit(EC_FLAGS_NO_GPE, &ec->flags) && - in_interrupt()) { + ec_check_sci(ec, status); + if (!test_bit(EC_FLAGS_GPE_MODE, &ec->flags) && + !test_bit(EC_FLAGS_NO_GPE, &ec->flags)) { /* this is non-query, must be confirmation */ if (printk_ratelimit()) pr_info(PREFIX "non-query interrupt received," " switching to interrupt mode\n"); set_bit(EC_FLAGS_GPE_MODE, &ec->flags); - clear_bit(EC_FLAGS_RESCHEDULE_POLL, &ec->flags); } -end: - ec_schedule_ec_poll(ec); - return ACPI_SUCCESS(status) ? - ACPI_INTERRUPT_HANDLED : ACPI_INTERRUPT_NOT_HANDLED; -} - -static void do_ec_poll(struct work_struct *work) -{ - struct acpi_ec *ec = container_of(work, struct acpi_ec, work.work); - atomic_set(&ec->irq_count, 0); - (void)acpi_ec_gpe_handler(ec); + return ACPI_INTERRUPT_HANDLED; } /* -------------------------------------------------------------------------- @@ -696,8 +696,7 @@ static struct acpi_ec *make_acpi_ec(void) mutex_init(&ec->lock); init_waitqueue_head(&ec->wait); INIT_LIST_HEAD(&ec->list); - INIT_DELAYED_WORK_DEFERRABLE(&ec->work, do_ec_poll); - atomic_set(&ec->irq_count, 0); + spin_lock_init(&ec->t_lock); return ec; } @@ -736,22 +735,15 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval) return AE_CTRL_TERMINATE; } -static void ec_poll_stop(struct acpi_ec *ec) -{ - clear_bit(EC_FLAGS_RESCHEDULE_POLL, &ec->flags); - cancel_delayed_work(&ec->work); -} - static void ec_remove_handlers(struct acpi_ec *ec) { - ec_poll_stop(ec); if (ACPI_FAILURE(acpi_remove_address_space_handler(ec->handle, ACPI_ADR_SPACE_EC, &acpi_ec_space_handler))) pr_err(PREFIX "failed to remove space handler\n"); if (ACPI_FAILURE(acpi_remove_gpe_handler(NULL, ec->gpe, &acpi_ec_gpe_handler))) pr_err(PREFIX "failed to remove gpe handler\n"); - ec->handlers_installed = 0; + clear_bit(EC_FLAGS_HANDLERS_INSTALLED, &ec->flags); } static int acpi_ec_add(struct acpi_device *device) @@ -846,17 +838,15 @@ ec_parse_io_ports(struct acpi_resource *resource, void *context) static int ec_install_handlers(struct acpi_ec *ec) { acpi_status status; - if (ec->handlers_installed) + if (test_bit(EC_FLAGS_HANDLERS_INSTALLED, &ec->flags)) return 0; status = acpi_install_gpe_handler(NULL, ec->gpe, - ACPI_GPE_EDGE_TRIGGERED, - &acpi_ec_gpe_handler, ec); + ACPI_GPE_EDGE_TRIGGERED, + &acpi_ec_gpe_handler, ec); if (ACPI_FAILURE(status)) return -ENODEV; - acpi_set_gpe_type(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); acpi_enable_gpe(NULL, ec->gpe, ACPI_NOT_ISR); - status = acpi_install_address_space_handler(ec->handle, ACPI_ADR_SPACE_EC, &acpi_ec_space_handler, @@ -866,7 +856,7 @@ static int ec_install_handlers(struct acpi_ec *ec) return -ENODEV; } - ec->handlers_installed = 1; + set_bit(EC_FLAGS_HANDLERS_INSTALLED, &ec->flags); return 0; } @@ -887,7 +877,6 @@ static int acpi_ec_start(struct acpi_device *device) /* EC is fully operational, allow queries */ clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags); - ec_schedule_ec_poll(ec); return ret; } @@ -906,7 +895,7 @@ static int acpi_ec_stop(struct acpi_device *device, int type) int __init acpi_boot_ec_enable(void) { - if (!boot_ec || boot_ec->handlers_installed) + if (!boot_ec || test_bit(EC_FLAGS_HANDLERS_INSTALLED, &boot_ec->flags)) return 0; if (!ec_install_handlers(boot_ec)) { first_ec = boot_ec; From 8463200a00fe2aea938b40173198a0983f2929ef Mon Sep 17 00:00:00 2001 From: Alexey Starikovskiy Date: Fri, 26 Sep 2008 00:54:28 +0400 Subject: [PATCH 2/4] ACPI: EC: Rename some variables No functional changes. Signed-off-by: Alexey Starikovskiy Acked-by: Rafael J. Wysocki Signed-off-by: Len Brown --- drivers/acpi/ec.c | 118 +++++++++++++++++++++++++--------------------- 1 file changed, 63 insertions(+), 55 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 7f0d81c0ec99..453ba1e534ef 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -95,10 +95,11 @@ struct acpi_ec_query_handler { u8 query_bit; }; -struct transaction_data { +struct transaction { const u8 *wdata; u8 *rdata; unsigned short irq_count; + u8 command; u8 wlen; u8 rlen; }; @@ -113,8 +114,8 @@ static struct acpi_ec { struct mutex lock; wait_queue_head_t wait; struct list_head list; - struct transaction_data *t; - spinlock_t t_lock; + struct transaction *curr; + spinlock_t curr_lock; } *boot_ec, *first_ec; /* @@ -176,37 +177,37 @@ static int ec_transaction_done(struct acpi_ec *ec) { unsigned long flags; int ret = 0; - spin_lock_irqsave(&ec->t_lock, flags); - if (!ec->t || (!ec->t->wlen && !ec->t->rlen)) + spin_lock_irqsave(&ec->curr_lock, flags); + if (!ec->curr || (!ec->curr->wlen && !ec->curr->rlen)) ret = 1; - spin_unlock_irqrestore(&ec->t_lock, flags); + spin_unlock_irqrestore(&ec->curr_lock, flags); return ret; } static void gpe_transaction(struct acpi_ec *ec, u8 status) { unsigned long flags; - spin_lock_irqsave(&ec->t_lock, flags); - if (!ec->t) + spin_lock_irqsave(&ec->curr_lock, flags); + if (!ec->curr) goto unlock; - if (ec->t->wlen > 0) { + if (ec->curr->wlen > 0) { if ((status & ACPI_EC_FLAG_IBF) == 0) { - acpi_ec_write_data(ec, *(ec->t->wdata++)); - --ec->t->wlen; + acpi_ec_write_data(ec, *(ec->curr->wdata++)); + --ec->curr->wlen; } else /* false interrupt, state didn't change */ - ++ec->t->irq_count; + ++ec->curr->irq_count; - } else if (ec->t->rlen > 0) { + } else if (ec->curr->rlen > 0) { if ((status & ACPI_EC_FLAG_OBF) == 1) { - *(ec->t->rdata++) = acpi_ec_read_data(ec); - --ec->t->rlen; + *(ec->curr->rdata++) = acpi_ec_read_data(ec); + --ec->curr->rlen; } else /* false interrupt, state didn't change */ - ++ec->t->irq_count; + ++ec->curr->irq_count; } unlock: - spin_unlock_irqrestore(&ec->t_lock, flags); + spin_unlock_irqrestore(&ec->curr_lock, flags); } static int acpi_ec_wait(struct acpi_ec *ec) @@ -248,15 +249,11 @@ static int ec_poll(struct acpi_ec *ec) return -ETIME; } -static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, u8 command, - const u8 * wdata, unsigned wdata_len, - u8 * rdata, unsigned rdata_len, +static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, + struct transaction *t, int force_poll) { unsigned long tmp; - struct transaction_data t = {.wdata = wdata, .rdata = rdata, - .wlen = wdata_len, .rlen = rdata_len, - .irq_count = 0}; int ret = 0; pr_debug(PREFIX "transaction start\n"); /* disable GPE during transaction if storm is detected */ @@ -265,29 +262,30 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, u8 command, acpi_disable_gpe(NULL, ec->gpe, ACPI_NOT_ISR); } /* start transaction */ - spin_lock_irqsave(&ec->t_lock, tmp); + spin_lock_irqsave(&ec->curr_lock, tmp); /* following two actions should be kept atomic */ - ec->t = &t; - acpi_ec_write_cmd(ec, command); - if (command == ACPI_EC_COMMAND_QUERY) + t->irq_count = 0; + ec->curr = t; + acpi_ec_write_cmd(ec, ec->curr->command); + if (ec->curr->command == ACPI_EC_COMMAND_QUERY) clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags); - spin_unlock_irqrestore(&ec->t_lock, tmp); + spin_unlock_irqrestore(&ec->curr_lock, tmp); /* if we selected poll mode or failed in GPE-mode do a poll loop */ if (force_poll || !test_bit(EC_FLAGS_GPE_MODE, &ec->flags) || acpi_ec_wait(ec)) ret = ec_poll(ec); pr_debug(PREFIX "transaction end\n"); - spin_lock_irqsave(&ec->t_lock, tmp); - ec->t = NULL; - spin_unlock_irqrestore(&ec->t_lock, tmp); + spin_lock_irqsave(&ec->curr_lock, tmp); + ec->curr = NULL; + spin_unlock_irqrestore(&ec->curr_lock, tmp); if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) { /* check if we received SCI during transaction */ ec_check_sci(ec, acpi_ec_read_status(ec)); /* it is safe to enable GPE outside of transaction */ acpi_enable_gpe(NULL, ec->gpe, ACPI_NOT_ISR); } else if (test_bit(EC_FLAGS_GPE_MODE, &ec->flags) && - t.irq_count > ACPI_EC_STORM_THRESHOLD) { + t->irq_count > ACPI_EC_STORM_THRESHOLD) { pr_debug(PREFIX "GPE storm detected\n"); set_bit(EC_FLAGS_GPE_STORM, &ec->flags); } @@ -300,17 +298,15 @@ static int ec_check_ibf0(struct acpi_ec *ec) return (status & ACPI_EC_FLAG_IBF) == 0; } -static int acpi_ec_transaction(struct acpi_ec *ec, u8 command, - const u8 * wdata, unsigned wdata_len, - u8 * rdata, unsigned rdata_len, +static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t, int force_poll) { int status; u32 glk; - if (!ec || (wdata_len && !wdata) || (rdata_len && !rdata)) + if (!ec || (!t) || (t->wlen && !t->wdata) || (t->rlen && !t->rdata)) return -EINVAL; - if (rdata) - memset(rdata, 0, rdata_len); + if (t->rdata) + memset(t->rdata, 0, t->rlen); mutex_lock(&ec->lock); if (ec->global_lock) { status = acpi_acquire_global_lock(ACPI_EC_UDELAY_GLK, &glk); @@ -326,10 +322,7 @@ static int acpi_ec_transaction(struct acpi_ec *ec, u8 command, status = -ETIME; goto end; } - status = acpi_ec_transaction_unlocked(ec, command, - wdata, wdata_len, - rdata, rdata_len, - force_poll); + status = acpi_ec_transaction_unlocked(ec, t, force_poll); end: if (ec->global_lock) acpi_release_global_lock(glk); @@ -345,23 +338,32 @@ unlock: int acpi_ec_burst_enable(struct acpi_ec *ec) { u8 d; - return acpi_ec_transaction(ec, ACPI_EC_BURST_ENABLE, NULL, 0, &d, 1, 0); + struct transaction t = {.command = ACPI_EC_BURST_ENABLE, + .wdata = NULL, .rdata = &d, + .wlen = 0, .rlen = 1}; + + return acpi_ec_transaction(ec, &t, 0); } int acpi_ec_burst_disable(struct acpi_ec *ec) { + struct transaction t = {.command = ACPI_EC_BURST_DISABLE, + .wdata = NULL, .rdata = NULL, + .wlen = 0, .rlen = 0}; + return (acpi_ec_read_status(ec) & ACPI_EC_FLAG_BURST) ? - acpi_ec_transaction(ec, ACPI_EC_BURST_DISABLE, - NULL, 0, NULL, 0, 0) : 0; + acpi_ec_transaction(ec, &t, 0) : 0; } static int acpi_ec_read(struct acpi_ec *ec, u8 address, u8 * data) { int result; u8 d; + struct transaction t = {.command = ACPI_EC_COMMAND_READ, + .wdata = &address, .rdata = &d, + .wlen = 1, .rlen = 1}; - result = acpi_ec_transaction(ec, ACPI_EC_COMMAND_READ, - &address, 1, &d, 1, 0); + result = acpi_ec_transaction(ec, &t, 0); *data = d; return result; } @@ -369,8 +371,11 @@ static int acpi_ec_read(struct acpi_ec *ec, u8 address, u8 * data) static int acpi_ec_write(struct acpi_ec *ec, u8 address, u8 data) { u8 wdata[2] = { address, data }; - return acpi_ec_transaction(ec, ACPI_EC_COMMAND_WRITE, - wdata, 2, NULL, 0, 0); + struct transaction t = {.command = ACPI_EC_COMMAND_WRITE, + .wdata = wdata, .rdata = NULL, + .wlen = 2, .rlen = 0}; + + return acpi_ec_transaction(ec, &t, 0); } /* @@ -432,12 +437,13 @@ int ec_transaction(u8 command, u8 * rdata, unsigned rdata_len, int force_poll) { + struct transaction t = {.command = command, + .wdata = wdata, .rdata = rdata, + .wlen = wdata_len, .rlen = rdata_len}; if (!first_ec) return -ENODEV; - return acpi_ec_transaction(first_ec, command, wdata, - wdata_len, rdata, rdata_len, - force_poll); + return acpi_ec_transaction(first_ec, &t, force_poll); } EXPORT_SYMBOL(ec_transaction); @@ -446,7 +452,9 @@ static int acpi_ec_query(struct acpi_ec *ec, u8 * data) { int result; u8 d; - + struct transaction t = {.command = ACPI_EC_COMMAND_QUERY, + .wdata = NULL, .rdata = &d, + .wlen = 0, .rlen = 1}; if (!ec || !data) return -EINVAL; @@ -456,7 +464,7 @@ static int acpi_ec_query(struct acpi_ec *ec, u8 * data) * bit to be cleared (and thus clearing the interrupt source). */ - result = acpi_ec_transaction(ec, ACPI_EC_COMMAND_QUERY, NULL, 0, &d, 1, 0); + result = acpi_ec_transaction(ec, &t, 0); if (result) return result; @@ -696,7 +704,7 @@ static struct acpi_ec *make_acpi_ec(void) mutex_init(&ec->lock); init_waitqueue_head(&ec->wait); INIT_LIST_HEAD(&ec->list); - spin_lock_init(&ec->t_lock); + spin_lock_init(&ec->curr_lock); return ec; } From 455c8793d2c49eaecad038c8de83dade9fc3759f Mon Sep 17 00:00:00 2001 From: Zhao Yakui Date: Mon, 6 Oct 2008 10:31:36 +0800 Subject: [PATCH 3/4] ACPI: Enable EC device immediately after ACPI full initialization when there is no ECDT table and no _INI object for EC device, it will be enabled before scanning ACPI device. But it is too late after the following the commit is merged. >commit 7752d5cfe3d11ca0bb9c673ec38bd78ba6578f8e > Author: Robert Hancock > Date: Fri Feb 15 01:27:20 2008 -0800 >x86: validate against acpi motherboard resources After the above commit is merged, OS will check whether MCFG area is reserved in ACPI motherboard resources by calling the function of acpi_get_devices when there exists MCFG table. In the acpi_get_devices the _STA object will be evaluated to check the status of the ACPI device. On some broken BIOS the MYEC object of EC device is initialized as one, which indicates that EC operation region is already accessible before enabling EC device.So on these broken BIOS the EC operation region will be accessed in course of evaluating the _STA object before enabling EC device, which causes that OS will print the following warning messages: >ACPI Error (evregion-0315): No handler for Region [EC__] (ffff88007f8145e8) [EmbeddedControl] [20080609] >ACPI Error (exfldio-0290): Region EmbeddedControl(3) has no handler [20080321] >ACPI Error (psparse-0530): Method parse/execution failed [\_SB_.PCI0.SBRG. EC__.BAT1._STA] (Node ffff81013fc17a00), AE_NOT_EXIST >ACPI Error (uteval-0233): Method execution failed [\_SB_.PCI0.SBRG.EC__.BAT1. _STA] (Node ffff81013fc17a00), AE_NOT_EXIST Although the above warning message is harmless, it looks confusing. So it is necessary to enable EC device as early as possible.Maybe it is appropriate to enable it immediately after ACPI full initialization. http://bugzilla.kernel.org/show_bug.cgi?id=11255 http://bugzilla.kernel.org/show_bug.cgi?id=11374 http://bugzilla.kernel.org/show_bug.cgi?id=11660 Signed-off-by: Zhao Yakui Acked-by: Alexey Starikovskiy Signed-off-by: Len Brown --- drivers/acpi/bus.c | 6 ++++++ drivers/acpi/scan.c | 4 ---- include/acpi/acpi_drivers.h | 1 + 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index ccae305ee55d..0e0bbc6209e5 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -749,6 +749,12 @@ static int __init acpi_bus_init(void) goto error1; } + /* + * Maybe EC region is required at bus_scan/acpi_get_devices. So it + * is necessary to enable it as early as possible. + */ + acpi_boot_ec_enable(); + printk(KERN_INFO PREFIX "Interpreter enabled\n"); /* Initialize sleep structures */ diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index f6f52c1a2aba..2ae218f5ea59 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1545,7 +1545,6 @@ static int acpi_bus_scan_fixed(struct acpi_device *root) return result; } -int __init acpi_boot_ec_enable(void); static int __init acpi_scan_init(void) { @@ -1579,9 +1578,6 @@ static int __init acpi_scan_init(void) */ result = acpi_bus_scan_fixed(acpi_root); - /* EC region might be needed at bus_scan, so enable it now */ - acpi_boot_ec_enable(); - if (!result) result = acpi_bus_scan(acpi_root, &ops); diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h index e5f38e5ce86f..ab926a70b39f 100644 --- a/include/acpi/acpi_drivers.h +++ b/include/acpi/acpi_drivers.h @@ -100,6 +100,7 @@ int acpi_power_transition(struct acpi_device *device, int state); -------------------------------------------------------------------------- */ #ifdef CONFIG_ACPI_EC int acpi_ec_ecdt_probe(void); +int acpi_boot_ec_enable(void); #endif /* -------------------------------------------------------------------------- From c0ff17720ec5f42205b3d2ca03a18da0a8272976 Mon Sep 17 00:00:00 2001 From: Alexey Starikovskiy Date: Thu, 16 Oct 2008 02:02:33 +0400 Subject: [PATCH 4/4] ACPI: EC: Check for IBF=0 periodically if not in GPE mode Signed-off-by: Alexey Starikovskiy Tested-by: Alan Jenkins Signed-off-by: Len Brown --- drivers/acpi/ec.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 453ba1e534ef..444cd9ed12c7 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -298,6 +298,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) { @@ -315,8 +327,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;