From 6fd13452c1a2e6dfe5b9a4c84c1144383cc55472 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 10 Nov 2021 13:16:40 +0200 Subject: [PATCH 01/15] ACPI: processor: Replace kernel.h with the necessary inclusions When kernel.h is used in the headers it adds a lot into dependency hell, especially when there are circular dependencies are involved. Replace kernel.h inclusion with the list of what is really being used. Signed-off-by: Andy Shevchenko Signed-off-by: Rafael J. Wysocki --- include/acpi/acpi_numa.h | 1 - include/acpi/processor.h | 7 ++++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/include/acpi/acpi_numa.h b/include/acpi/acpi_numa.h index 68e4d80c1b3266..b5f594754a9e4f 100644 --- a/include/acpi/acpi_numa.h +++ b/include/acpi/acpi_numa.h @@ -3,7 +3,6 @@ #define __ACPI_NUMA_H #ifdef CONFIG_ACPI_NUMA -#include #include /* Proximity bitmap length */ diff --git a/include/acpi/processor.h b/include/acpi/processor.h index 683e124ad517d0..1940273719285e 100644 --- a/include/acpi/processor.h +++ b/include/acpi/processor.h @@ -2,11 +2,16 @@ #ifndef __ACPI_PROCESSOR_H #define __ACPI_PROCESSOR_H -#include #include #include #include +#include +#include +#include #include +#include +#include + #include #define ACPI_PROCESSOR_CLASS "processor" From 0e6078c3c6737df7d0bd0c890fbadf24a27fffbb Mon Sep 17 00:00:00 2001 From: Guo Zhengkui Date: Tue, 9 Nov 2021 15:50:51 +0800 Subject: [PATCH 02/15] ACPI: processor idle: Use swap() instead of open coding it Address the following coccicheck warning: ./drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c:914:40-41: WARNING opportunity for swap(). by using swap() for the swapping of variable values and drop the tmp variable that is not needed any more. Signed-off-by: Guo Zhengkui [ rjw: Subject and changelog rewrite ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/processor_idle.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 76ef1bcc884809..4b906bb527e8cc 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -20,6 +20,7 @@ #include #include #include +#include #include /* @@ -400,13 +401,10 @@ static int acpi_cst_latency_cmp(const void *a, const void *b) static void acpi_cst_latency_swap(void *a, void *b, int n) { struct acpi_processor_cx *x = a, *y = b; - u32 tmp; if (!(x->valid && y->valid)) return; - tmp = x->latency; - x->latency = y->latency; - y->latency = tmp; + swap(x->latency, y->latency); } static int acpi_processor_power_verify(struct acpi_processor *pr) From 4a9af6cac050dce2e895ec3205c4615383ad9112 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 23 Nov 2021 19:36:51 +0100 Subject: [PATCH 03/15] ACPI: EC: Rework flushing of EC work while suspended to idle The flushing of pending work in the EC driver uses drain_workqueue() to flush the event handling work that can requeue itself via advance_transaction(), but this is problematic, because that work may also be requeued from the query workqueue. Namely, if an EC transaction is carried out during the execution of a query handler, it involves calling advance_transaction() which may queue up the event handling work again. This causes the kernel to complain about attempts to add a work item to the EC event workqueue while it is being drained and worst-case it may cause a valid event to be skipped. To avoid this problem, introduce two new counters, events_in_progress and queries_in_progress, incremented when a work item is queued on the event workqueue or the query workqueue, respectively, and decremented at the end of the corresponding work function, and make acpi_ec_dispatch_gpe() the workqueues in a loop until the both of these counters are zero (or system wakeup is pending) instead of calling acpi_ec_flush_work(). At the same time, change __acpi_ec_flush_work() to call flush_workqueue() instead of drain_workqueue() to flush the event workqueue. While at it, use the observation that the work item queued in acpi_ec_query() cannot be pending at that time, because it is used only once, to simplify the code in there. Additionally, clean up a comment in acpi_ec_query() and adjust white space in acpi_ec_event_processor(). Fixes: f0ac20c3f613 ("ACPI: EC: Fix flushing of pending work") Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 57 +++++++++++++++++++++++++++++++---------- drivers/acpi/internal.h | 2 ++ 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index a6366d3f0c7863..b9c44e6c5e4009 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -166,6 +166,7 @@ struct acpi_ec_query { struct transaction transaction; struct work_struct work; struct acpi_ec_query_handler *handler; + struct acpi_ec *ec; }; static int acpi_ec_query(struct acpi_ec *ec, u8 *data); @@ -452,6 +453,7 @@ static void acpi_ec_submit_query(struct acpi_ec *ec) ec_dbg_evt("Command(%s) submitted/blocked", acpi_ec_cmd_string(ACPI_EC_COMMAND_QUERY)); ec->nr_pending_queries++; + ec->events_in_progress++; queue_work(ec_wq, &ec->work); } } @@ -518,7 +520,7 @@ static void acpi_ec_enable_event(struct acpi_ec *ec) #ifdef CONFIG_PM_SLEEP static void __acpi_ec_flush_work(void) { - drain_workqueue(ec_wq); /* flush ec->work */ + flush_workqueue(ec_wq); /* flush ec->work */ flush_workqueue(ec_query_wq); /* flush queries */ } @@ -1103,7 +1105,7 @@ void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit) } EXPORT_SYMBOL_GPL(acpi_ec_remove_query_handler); -static struct acpi_ec_query *acpi_ec_create_query(u8 *pval) +static struct acpi_ec_query *acpi_ec_create_query(struct acpi_ec *ec, u8 *pval) { struct acpi_ec_query *q; struct transaction *t; @@ -1111,11 +1113,13 @@ static struct acpi_ec_query *acpi_ec_create_query(u8 *pval) q = kzalloc(sizeof (struct acpi_ec_query), GFP_KERNEL); if (!q) return NULL; + INIT_WORK(&q->work, acpi_ec_event_processor); t = &q->transaction; t->command = ACPI_EC_COMMAND_QUERY; t->rdata = pval; t->rlen = 1; + q->ec = ec; return q; } @@ -1132,13 +1136,21 @@ static void acpi_ec_event_processor(struct work_struct *work) { struct acpi_ec_query *q = container_of(work, struct acpi_ec_query, work); struct acpi_ec_query_handler *handler = q->handler; + struct acpi_ec *ec = q->ec; ec_dbg_evt("Query(0x%02x) started", handler->query_bit); + if (handler->func) handler->func(handler->data); else if (handler->handle) acpi_evaluate_object(handler->handle, NULL, NULL, NULL); + ec_dbg_evt("Query(0x%02x) stopped", handler->query_bit); + + spin_lock_irq(&ec->lock); + ec->queries_in_progress--; + spin_unlock_irq(&ec->lock); + acpi_ec_delete_query(q); } @@ -1148,7 +1160,7 @@ static int acpi_ec_query(struct acpi_ec *ec, u8 *data) int result; struct acpi_ec_query *q; - q = acpi_ec_create_query(&value); + q = acpi_ec_create_query(ec, &value); if (!q) return -ENOMEM; @@ -1170,19 +1182,20 @@ static int acpi_ec_query(struct acpi_ec *ec, u8 *data) } /* - * It is reported that _Qxx are evaluated in a parallel way on - * Windows: + * It is reported that _Qxx are evaluated in a parallel way on Windows: * https://bugzilla.kernel.org/show_bug.cgi?id=94411 * - * Put this log entry before schedule_work() in order to make - * it appearing before any other log entries occurred during the - * work queue execution. + * Put this log entry before queue_work() to make it appear in the log + * before any other messages emitted during workqueue handling. */ ec_dbg_evt("Query(0x%02x) scheduled", value); - if (!queue_work(ec_query_wq, &q->work)) { - ec_dbg_evt("Query(0x%02x) overlapped", value); - result = -EBUSY; - } + + spin_lock_irq(&ec->lock); + + ec->queries_in_progress++; + queue_work(ec_query_wq, &q->work); + + spin_unlock_irq(&ec->lock); err_exit: if (result) @@ -1240,6 +1253,10 @@ static void acpi_ec_event_handler(struct work_struct *work) ec_dbg_evt("Event stopped"); acpi_ec_check_event(ec); + + spin_lock_irqsave(&ec->lock, flags); + ec->events_in_progress--; + spin_unlock_irqrestore(&ec->lock, flags); } static void acpi_ec_handle_interrupt(struct acpi_ec *ec) @@ -2021,6 +2038,7 @@ void acpi_ec_set_gpe_wake_mask(u8 action) bool acpi_ec_dispatch_gpe(void) { + bool work_in_progress; u32 ret; if (!first_ec) @@ -2041,8 +2059,19 @@ bool acpi_ec_dispatch_gpe(void) if (ret == ACPI_INTERRUPT_HANDLED) pm_pr_dbg("ACPI EC GPE dispatched\n"); - /* Flush the event and query workqueues. */ - acpi_ec_flush_work(); + /* Drain EC work. */ + do { + acpi_ec_flush_work(); + + pm_pr_dbg("ACPI EC work flushed\n"); + + spin_lock_irq(&first_ec->lock); + + work_in_progress = first_ec->events_in_progress + + first_ec->queries_in_progress > 0; + + spin_unlock_irq(&first_ec->lock); + } while (work_in_progress && !pm_wakeup_pending()); return false; } diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index d91b560e886747..54b2be94d23dcb 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -183,6 +183,8 @@ struct acpi_ec { struct work_struct work; unsigned long timestamp; unsigned long nr_pending_queries; + unsigned int events_in_progress; + unsigned int queries_in_progress; bool busy_polling; unsigned int polling_guard; }; From ca8283dcd933a2ca776fade77fb4527321521463 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 23 Nov 2021 19:37:39 +0100 Subject: [PATCH 04/15] ACPI: EC: Call advance_transaction() from acpi_ec_dispatch_gpe() Calling acpi_dispatch_gpe() from acpi_ec_dispatch_gpe() is generally problematic, because it may cause the spurious interrupt handling in advance_transaction() to trigger in theory. However, instead of calling acpi_dispatch_gpe() to dispatch the EC GPE, acpi_ec_dispatch_gpe() can call advance_transaction() directly on first_ec and it can pass 'false' as its second argument to indicate calling it from process context. Moreover, if advance_transaction() is modified to return a bool value indicating whether or not the EC work needs to be flushed, it can be used to avoid unnecessary EC work flushing in acpi_ec_dispatch_gpe(), so change the code accordingly. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index b9c44e6c5e4009..bfc29cdfa5f067 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -170,7 +170,7 @@ struct acpi_ec_query { }; static int acpi_ec_query(struct acpi_ec *ec, u8 *data); -static void advance_transaction(struct acpi_ec *ec, bool interrupt); +static bool advance_transaction(struct acpi_ec *ec, bool interrupt); static void acpi_ec_event_handler(struct work_struct *work); static void acpi_ec_event_processor(struct work_struct *work); @@ -444,18 +444,25 @@ static bool acpi_ec_submit_flushable_request(struct acpi_ec *ec) return true; } -static void acpi_ec_submit_query(struct acpi_ec *ec) +static bool acpi_ec_submit_query(struct acpi_ec *ec) { acpi_ec_mask_events(ec); if (!acpi_ec_event_enabled(ec)) - return; + return false; + if (!test_and_set_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) { ec_dbg_evt("Command(%s) submitted/blocked", acpi_ec_cmd_string(ACPI_EC_COMMAND_QUERY)); ec->nr_pending_queries++; ec->events_in_progress++; - queue_work(ec_wq, &ec->work); + return queue_work(ec_wq, &ec->work); } + + /* + * The event handling work has not been completed yet, so it needs to be + * flushed. + */ + return true; } static void acpi_ec_complete_query(struct acpi_ec *ec) @@ -628,10 +635,11 @@ static void acpi_ec_spurious_interrupt(struct acpi_ec *ec, struct transaction *t acpi_ec_mask_events(ec); } -static void advance_transaction(struct acpi_ec *ec, bool interrupt) +static bool advance_transaction(struct acpi_ec *ec, bool interrupt) { struct transaction *t = ec->curr; bool wakeup = false; + bool ret = false; u8 status; ec_dbg_stm("%s (%d)", interrupt ? "IRQ" : "TASK", smp_processor_id()); @@ -698,10 +706,12 @@ static void advance_transaction(struct acpi_ec *ec, bool interrupt) out: if (status & ACPI_EC_FLAG_SCI) - acpi_ec_submit_query(ec); + ret = acpi_ec_submit_query(ec); if (wakeup && interrupt) wake_up(&ec->wait); + + return ret; } static void start_transaction(struct acpi_ec *ec) @@ -2038,8 +2048,7 @@ void acpi_ec_set_gpe_wake_mask(u8 action) bool acpi_ec_dispatch_gpe(void) { - bool work_in_progress; - u32 ret; + bool work_in_progress = false; if (!first_ec) return acpi_any_gpe_status_set(U32_MAX); @@ -2055,9 +2064,17 @@ bool acpi_ec_dispatch_gpe(void) * Dispatch the EC GPE in-band, but do not report wakeup in any case * to allow the caller to process events properly after that. */ - ret = acpi_dispatch_gpe(NULL, first_ec->gpe); - if (ret == ACPI_INTERRUPT_HANDLED) - pm_pr_dbg("ACPI EC GPE dispatched\n"); + spin_lock_irq(&first_ec->lock); + + if (acpi_ec_gpe_status_set(first_ec)) + work_in_progress = advance_transaction(first_ec, false); + + spin_unlock_irq(&first_ec->lock); + + if (!work_in_progress) + return false; + + pm_pr_dbg("ACPI EC GPE dispatched\n"); /* Drain EC work. */ do { From 1f2350443dd21028d3de6907f98ff1be75a2c9bc Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 23 Nov 2021 19:38:21 +0100 Subject: [PATCH 05/15] ACPI: EC: Pass one argument to acpi_ec_query() Notice that the second argument to acpi_ec_query() is redundant, because in the only case when it is not NULL, the value passed through it is only checked against 0 and it can only be 0 when acpi_ec_query() returns an error code, but its return value is checked along with the value passed through its second argument. Accordingly, modify acpi_ec_query() to take only one argument and while at it, change its handling of the case when acpi_ec_transaction() returns an error so as to return that error value to the caller right away. No expected functional impact. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index bfc29cdfa5f067..4b2573d85ac663 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -169,7 +169,7 @@ struct acpi_ec_query { struct acpi_ec *ec; }; -static int acpi_ec_query(struct acpi_ec *ec, u8 *data); +static int acpi_ec_query(struct acpi_ec *ec); static bool advance_transaction(struct acpi_ec *ec, bool interrupt); static void acpi_ec_event_handler(struct work_struct *work); static void acpi_ec_event_processor(struct work_struct *work); @@ -496,12 +496,10 @@ static inline void __acpi_ec_disable_event(struct acpi_ec *ec) */ static void acpi_ec_clear(struct acpi_ec *ec) { - int i, status; - u8 value = 0; + int i; for (i = 0; i < ACPI_EC_CLEAR_MAX; i++) { - status = acpi_ec_query(ec, &value); - if (status || !value) + if (acpi_ec_query(ec)) break; } if (unlikely(i == ACPI_EC_CLEAR_MAX)) @@ -1164,11 +1162,11 @@ static void acpi_ec_event_processor(struct work_struct *work) acpi_ec_delete_query(q); } -static int acpi_ec_query(struct acpi_ec *ec, u8 *data) +static int acpi_ec_query(struct acpi_ec *ec) { + struct acpi_ec_query *q; u8 value = 0; int result; - struct acpi_ec_query *q; q = acpi_ec_create_query(ec, &value); if (!q) @@ -1180,11 +1178,14 @@ 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, &q->transaction); - if (!value) - result = -ENODATA; if (result) goto err_exit; + if (!value) { + result = -ENODATA; + goto err_exit; + } + q->handler = acpi_ec_get_query_handler_by_value(ec, value); if (!q->handler) { result = -ENODATA; @@ -1210,8 +1211,7 @@ static int acpi_ec_query(struct acpi_ec *ec, u8 *data) err_exit: if (result) acpi_ec_delete_query(q); - if (data) - *data = value; + return result; } @@ -1243,7 +1243,9 @@ static void acpi_ec_event_handler(struct work_struct *work) spin_lock_irqsave(&ec->lock, flags); while (ec->nr_pending_queries) { spin_unlock_irqrestore(&ec->lock, flags); - (void)acpi_ec_query(ec, NULL); + + acpi_ec_query(ec); + spin_lock_irqsave(&ec->lock, flags); ec->nr_pending_queries--; /* From 98d364509d77e2d441ef6d8bdf13e1a4258eac6c Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 23 Nov 2021 19:39:05 +0100 Subject: [PATCH 06/15] ACPI: EC: Fold acpi_ec_check_event() into acpi_ec_event_handler() Because acpi_ec_event_handler() is the only caller of acpi_ec_check_event() and the separation of these two functions makes it harder to follow the code flow, fold the latter into the former (and simplify that code while at it). No expected functional impact. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 4b2573d85ac663..07506afa1fdab2 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1215,24 +1215,6 @@ static int acpi_ec_query(struct acpi_ec *ec) return result; } -static void acpi_ec_check_event(struct acpi_ec *ec) -{ - unsigned long flags; - - if (ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT) { - if (ec_guard(ec)) { - spin_lock_irqsave(&ec->lock, flags); - /* - * Take care of the SCI_EVT unless no one else is - * taking care of it. - */ - if (!ec->curr) - advance_transaction(ec, false); - spin_unlock_irqrestore(&ec->lock, flags); - } - } -} - static void acpi_ec_event_handler(struct work_struct *work) { unsigned long flags; @@ -1264,7 +1246,15 @@ static void acpi_ec_event_handler(struct work_struct *work) ec_dbg_evt("Event stopped"); - acpi_ec_check_event(ec); + if (ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT && ec_guard(ec)) { + spin_lock_irqsave(&ec->lock, flags); + + /* Take care of SCI_EVT unless someone else is doing that. */ + if (!ec->curr) + advance_transaction(ec, false); + + spin_unlock_irqrestore(&ec->lock, flags); + } spin_lock_irqsave(&ec->lock, flags); ec->events_in_progress--; From 388fb77dcf9793b9882e0f0651019f416ef50900 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 23 Nov 2021 19:40:04 +0100 Subject: [PATCH 07/15] ACPI: EC: Rearrange the loop in acpi_ec_event_handler() It is not necessary to check ec->nr_pending_queries against 0 in the while () loop in acpi_ec_event_handler(), because that loop terminates when ec->nr_pending_queries is 0 and the code depending on that can be run after the loop has ended. Modify the code accordingly and while at it rewrite the comment regarding that code to make it clearer. No intentional functional impact. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 07506afa1fdab2..7582ef59b81f5d 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1230,18 +1230,17 @@ static void acpi_ec_event_handler(struct work_struct *work) spin_lock_irqsave(&ec->lock, flags); ec->nr_pending_queries--; - /* - * Before exit, make sure that this work item can be - * scheduled again. There might be QR_EC failures, leaving - * EC_FLAGS_QUERY_PENDING uncleared and preventing this work - * item from being scheduled again. - */ - if (!ec->nr_pending_queries) { - if (ec_event_clearing == ACPI_EC_EVT_TIMING_STATUS || - ec_event_clearing == ACPI_EC_EVT_TIMING_QUERY) - acpi_ec_complete_query(ec); - } } + + /* + * Before exit, make sure that the it will be possible to queue up the + * event handling work again regardless of whether or not the query + * queued up above is processed successfully. + */ + if (ec_event_clearing == ACPI_EC_EVT_TIMING_STATUS || + ec_event_clearing == ACPI_EC_EVT_TIMING_QUERY) + acpi_ec_complete_query(ec); + spin_unlock_irqrestore(&ec->lock, flags); ec_dbg_evt("Event stopped"); From a105acd7e38436ed6bc0714f3120aaaad56cbe3d Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 23 Nov 2021 19:40:50 +0100 Subject: [PATCH 08/15] ACPI: EC: Simplify locking in acpi_ec_event_handler() Because acpi_ec_event_handler() is a work function, it always runs in process context with interrupts enabled, so it can use spin_lock_irq() and spin_unlock_irq() for the locking. Make it do so and adjust white space around those calls. No expected functional impact. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 7582ef59b81f5d..6fbd8521b90151 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1217,18 +1217,18 @@ static int acpi_ec_query(struct acpi_ec *ec) static void acpi_ec_event_handler(struct work_struct *work) { - unsigned long flags; struct acpi_ec *ec = container_of(work, struct acpi_ec, work); ec_dbg_evt("Event started"); - spin_lock_irqsave(&ec->lock, flags); + spin_lock_irq(&ec->lock); + while (ec->nr_pending_queries) { - spin_unlock_irqrestore(&ec->lock, flags); + spin_unlock_irq(&ec->lock); acpi_ec_query(ec); - spin_lock_irqsave(&ec->lock, flags); + spin_lock_irq(&ec->lock); ec->nr_pending_queries--; } @@ -1241,23 +1241,23 @@ static void acpi_ec_event_handler(struct work_struct *work) ec_event_clearing == ACPI_EC_EVT_TIMING_QUERY) acpi_ec_complete_query(ec); - spin_unlock_irqrestore(&ec->lock, flags); + spin_unlock_irq(&ec->lock); ec_dbg_evt("Event stopped"); if (ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT && ec_guard(ec)) { - spin_lock_irqsave(&ec->lock, flags); + spin_lock_irq(&ec->lock); /* Take care of SCI_EVT unless someone else is doing that. */ if (!ec->curr) advance_transaction(ec, false); - spin_unlock_irqrestore(&ec->lock, flags); + spin_unlock_irq(&ec->lock); } - spin_lock_irqsave(&ec->lock, flags); + spin_lock_irq(&ec->lock); ec->events_in_progress--; - spin_unlock_irqrestore(&ec->lock, flags); + spin_unlock_irq(&ec->lock); } static void acpi_ec_handle_interrupt(struct acpi_ec *ec) From eafe7509ab8c8a200a169bc378fc9a56164acc66 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 23 Nov 2021 19:42:02 +0100 Subject: [PATCH 09/15] ACPI: EC: Rename three functions Rename acpi_ec_submit_query() to acpi_ec_submit_event(), acpi_ec_query() to acpi_ec_submit_query(), and acpi_ec_complete_query() to acpi_ec_close_event() to make the names reflect what the functions do. No expected functional impact. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 6fbd8521b90151..473b1d8617b583 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -169,7 +169,7 @@ struct acpi_ec_query { struct acpi_ec *ec; }; -static int acpi_ec_query(struct acpi_ec *ec); +static int acpi_ec_submit_query(struct acpi_ec *ec); static bool advance_transaction(struct acpi_ec *ec, bool interrupt); static void acpi_ec_event_handler(struct work_struct *work); static void acpi_ec_event_processor(struct work_struct *work); @@ -444,7 +444,7 @@ static bool acpi_ec_submit_flushable_request(struct acpi_ec *ec) return true; } -static bool acpi_ec_submit_query(struct acpi_ec *ec) +static bool acpi_ec_submit_event(struct acpi_ec *ec) { acpi_ec_mask_events(ec); if (!acpi_ec_event_enabled(ec)) @@ -465,7 +465,7 @@ static bool acpi_ec_submit_query(struct acpi_ec *ec) return true; } -static void acpi_ec_complete_query(struct acpi_ec *ec) +static void acpi_ec_close_event(struct acpi_ec *ec) { if (test_and_clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) ec_dbg_evt("Command(%s) unblocked", @@ -499,7 +499,7 @@ static void acpi_ec_clear(struct acpi_ec *ec) int i; for (i = 0; i < ACPI_EC_CLEAR_MAX; i++) { - if (acpi_ec_query(ec)) + if (acpi_ec_submit_query(ec)) break; } if (unlikely(i == ACPI_EC_CLEAR_MAX)) @@ -613,10 +613,10 @@ static inline void ec_transaction_transition(struct acpi_ec *ec, unsigned long f if (ec->curr->command == ACPI_EC_COMMAND_QUERY) { if (ec_event_clearing == ACPI_EC_EVT_TIMING_STATUS && flag == ACPI_EC_COMMAND_POLL) - acpi_ec_complete_query(ec); + acpi_ec_close_event(ec); if (ec_event_clearing == ACPI_EC_EVT_TIMING_QUERY && flag == ACPI_EC_COMMAND_COMPLETE) - acpi_ec_complete_query(ec); + acpi_ec_close_event(ec); if (ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT && flag == ACPI_EC_COMMAND_COMPLETE) set_bit(EC_FLAGS_QUERY_GUARDING, &ec->flags); @@ -668,7 +668,7 @@ static bool advance_transaction(struct acpi_ec *ec, bool interrupt) (!ec->nr_pending_queries || test_bit(EC_FLAGS_QUERY_GUARDING, &ec->flags))) { clear_bit(EC_FLAGS_QUERY_GUARDING, &ec->flags); - acpi_ec_complete_query(ec); + acpi_ec_close_event(ec); } if (!t) goto out; @@ -704,7 +704,7 @@ static bool advance_transaction(struct acpi_ec *ec, bool interrupt) out: if (status & ACPI_EC_FLAG_SCI) - ret = acpi_ec_submit_query(ec); + ret = acpi_ec_submit_event(ec); if (wakeup && interrupt) wake_up(&ec->wait); @@ -1162,7 +1162,7 @@ static void acpi_ec_event_processor(struct work_struct *work) acpi_ec_delete_query(q); } -static int acpi_ec_query(struct acpi_ec *ec) +static int acpi_ec_submit_query(struct acpi_ec *ec) { struct acpi_ec_query *q; u8 value = 0; @@ -1226,7 +1226,7 @@ static void acpi_ec_event_handler(struct work_struct *work) while (ec->nr_pending_queries) { spin_unlock_irq(&ec->lock); - acpi_ec_query(ec); + acpi_ec_submit_query(ec); spin_lock_irq(&ec->lock); ec->nr_pending_queries--; @@ -1239,7 +1239,7 @@ static void acpi_ec_event_handler(struct work_struct *work) */ if (ec_event_clearing == ACPI_EC_EVT_TIMING_STATUS || ec_event_clearing == ACPI_EC_EVT_TIMING_QUERY) - acpi_ec_complete_query(ec); + acpi_ec_close_event(ec); spin_unlock_irq(&ec->lock); From c793570d8725e44b64dbe466eb8ecda34c5eb8ac Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 23 Nov 2021 19:43:05 +0100 Subject: [PATCH 10/15] ACPI: EC: Avoid queuing unnecessary work in acpi_ec_submit_event() Notice that it is not necessary to queue up the event work again if the while () loop in acpi_ec_event_handler() is still running which is the case if nr_pending_queries is greater than 0 at the beginning of acpi_ec_submit_event() and modify the code to avoid doing that. While at it, rename nr_pending_queries in struct acpi_ec to events_to_process which actually matches the role of that field and change its data type to unsigned int which is sufficient. No expected functional impact. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 17 +++++++++++++---- drivers/acpi/internal.h | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 473b1d8617b583..d578f41410efc1 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -453,7 +453,16 @@ static bool acpi_ec_submit_event(struct acpi_ec *ec) if (!test_and_set_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) { ec_dbg_evt("Command(%s) submitted/blocked", acpi_ec_cmd_string(ACPI_EC_COMMAND_QUERY)); - ec->nr_pending_queries++; + /* + * If events_to_process is greqter than 0 at this point, the + * while () loop in acpi_ec_event_handler() is still running + * and incrementing events_to_process will cause it to invoke + * acpi_ec_submit_query() once more, so it is not necessary to + * queue up the event work to start the same loop again. + */ + if (ec->events_to_process++ > 0) + return true; + ec->events_in_progress++; return queue_work(ec_wq, &ec->work); } @@ -665,7 +674,7 @@ static bool advance_transaction(struct acpi_ec *ec, bool interrupt) */ if (!t || !(t->flags & ACPI_EC_COMMAND_POLL)) { if (ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT && - (!ec->nr_pending_queries || + (!ec->events_to_process || test_bit(EC_FLAGS_QUERY_GUARDING, &ec->flags))) { clear_bit(EC_FLAGS_QUERY_GUARDING, &ec->flags); acpi_ec_close_event(ec); @@ -1223,13 +1232,13 @@ static void acpi_ec_event_handler(struct work_struct *work) spin_lock_irq(&ec->lock); - while (ec->nr_pending_queries) { + while (ec->events_to_process) { spin_unlock_irq(&ec->lock); acpi_ec_submit_query(ec); spin_lock_irq(&ec->lock); - ec->nr_pending_queries--; + ec->events_to_process--; } /* diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 54b2be94d23dcb..de546be3bc6a6b 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -182,7 +182,7 @@ struct acpi_ec { spinlock_t lock; struct work_struct work; unsigned long timestamp; - unsigned long nr_pending_queries; + unsigned int events_to_process; unsigned int events_in_progress; unsigned int queries_in_progress; bool busy_polling; From c33676aa48249b007d55198dc8348cd117e3d8cc Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 23 Nov 2021 19:44:46 +0100 Subject: [PATCH 11/15] ACPI: EC: Make the event work state machine visible The EC driver uses a relatively simple state machine for the event work handling, but it is not really straightforward to figure out. The states are as follows: "Ready": The event handling work can be submitted. In this state, the EC_FLAGS_QUERY_PENDING flag is clear. "In progress": The event handling work is pending or is being processed. It cannot be submitted again. In ths state, the EC_FLAGS_QUERY_PENDING flag is set and both the events_to_process count is nonzero and the EC_FLAGS_QUERY_GUARDING flag is clear. "Complete": The event handling work has been completed, but it still cannot be submitted again. In ths state, the EC_FLAGS_QUERY_PENDING flag is set and the events_to_process count is zero or the EC_FLAGS_QUERY_GUARDING flag is set. The state changes from "Ready" to "In progress" when new event is detected by advance_transaction() and acpi_ec_submit_event() is called by it. Next, the state can change from "In progress" directly to "Ready" in the following situations: * ec_event_clearing is ACPI_EC_EVT_TIMING_STATUS and the state of an ACPI_EC_COMMAND_QUERY transaction becomes ACPI_EC_COMMAND_POLL. * ec_event_clearing is ACPI_EC_EVT_TIMING_QUERY and the state of an ACPI_EC_COMMAND_QUERY transaction becomes ACPI_EC_COMMAND_COMPLETE. * ec_event_clearing is either ACPI_EC_EVT_TIMING_STATUS or ACPI_EC_EVT_TIMING_QUERY and there are no more events to process (ie. ec->events_to_process becomes 0). If ec_event_clearing is ACPI_EC_EVT_TIMING_EVENT, however, the state must change from "In progress" to "Complete" before it can change to "Ready". The changes from "In progress" to "Complete" in that case occur in the following situations: * The state of an ACPI_EC_COMMAND_QUERY transaction becomes ACPI_EC_COMMAND_COMPLETE. * There are no more events to process (ie. ec->events_to_process becomes 0). Finally, the state changes from "Complete" to "Ready" when advance_transaction() is invoked when the state is "Complete" and the state of the current transaction is not ACPI_EC_COMMAND_POLL. To make this state machine visible in the code, add a new event_state field to struct acpi_ec and modify the code to use it istead the EC_FLAGS_QUERY_PENDING and EC_FLAGS_QUERY_GUARDING flags. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 75 ++++++++++++++++++++++++----------------- drivers/acpi/internal.h | 8 +++++ 2 files changed, 52 insertions(+), 31 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index d578f41410efc1..7d7bff4d2100cc 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -92,8 +92,6 @@ enum ec_command { enum { EC_FLAGS_QUERY_ENABLED, /* Query is enabled */ - EC_FLAGS_QUERY_PENDING, /* Query is pending */ - EC_FLAGS_QUERY_GUARDING, /* Guard for SCI_EVT check */ EC_FLAGS_EVENT_HANDLER_INSTALLED, /* Event handler installed */ EC_FLAGS_EC_HANDLER_INSTALLED, /* OpReg handler installed */ EC_FLAGS_QUERY_METHODS_INSTALLED, /* _Qxx handlers installed */ @@ -450,9 +448,11 @@ static bool acpi_ec_submit_event(struct acpi_ec *ec) if (!acpi_ec_event_enabled(ec)) return false; - if (!test_and_set_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) { + if (ec->event_state == EC_EVENT_READY) { ec_dbg_evt("Command(%s) submitted/blocked", acpi_ec_cmd_string(ACPI_EC_COMMAND_QUERY)); + + ec->event_state = EC_EVENT_IN_PROGRESS; /* * If events_to_process is greqter than 0 at this point, the * while () loop in acpi_ec_event_handler() is still running @@ -474,11 +474,19 @@ static bool acpi_ec_submit_event(struct acpi_ec *ec) return true; } +static void acpi_ec_complete_event(struct acpi_ec *ec) +{ + if (ec->event_state == EC_EVENT_IN_PROGRESS) + ec->event_state = EC_EVENT_COMPLETE; +} + static void acpi_ec_close_event(struct acpi_ec *ec) { - if (test_and_clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) + if (ec->event_state != EC_EVENT_READY) ec_dbg_evt("Command(%s) unblocked", acpi_ec_cmd_string(ACPI_EC_COMMAND_QUERY)); + + ec->event_state = EC_EVENT_READY; acpi_ec_unmask_events(ec); } @@ -565,8 +573,8 @@ void acpi_ec_flush_work(void) static bool acpi_ec_guard_event(struct acpi_ec *ec) { - bool guarded = true; unsigned long flags; + bool guarded; spin_lock_irqsave(&ec->lock, flags); /* @@ -575,19 +583,15 @@ static bool acpi_ec_guard_event(struct acpi_ec *ec) * evaluating _Qxx, so we need to re-check SCI_EVT after waiting an * acceptable period. * - * The guarding period begins when EC_FLAGS_QUERY_PENDING is - * flagged, which means SCI_EVT check has just been performed. - * But if the current transaction is ACPI_EC_COMMAND_QUERY, the - * guarding should have already been performed (via - * EC_FLAGS_QUERY_GUARDING) and should not be applied so that the - * ACPI_EC_COMMAND_QUERY transaction can be transitioned into - * ACPI_EC_COMMAND_POLL state immediately. + * The guarding period is applicable if the event state is not + * EC_EVENT_READY, but otherwise if the current transaction is of the + * ACPI_EC_COMMAND_QUERY type, the guarding should have elapsed already + * and it should not be applied to let the transaction transition into + * the ACPI_EC_COMMAND_POLL state immediately. */ - if (ec_event_clearing == ACPI_EC_EVT_TIMING_STATUS || - ec_event_clearing == ACPI_EC_EVT_TIMING_QUERY || - !test_bit(EC_FLAGS_QUERY_PENDING, &ec->flags) || - (ec->curr && ec->curr->command == ACPI_EC_COMMAND_QUERY)) - guarded = false; + guarded = ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT && + ec->event_state != EC_EVENT_READY && + (!ec->curr || ec->curr->command != ACPI_EC_COMMAND_QUERY); spin_unlock_irqrestore(&ec->lock, flags); return guarded; } @@ -619,16 +623,26 @@ static int ec_transaction_completed(struct acpi_ec *ec) static inline void ec_transaction_transition(struct acpi_ec *ec, unsigned long flag) { ec->curr->flags |= flag; - if (ec->curr->command == ACPI_EC_COMMAND_QUERY) { - if (ec_event_clearing == ACPI_EC_EVT_TIMING_STATUS && - flag == ACPI_EC_COMMAND_POLL) + + if (ec->curr->command != ACPI_EC_COMMAND_QUERY) + return; + + switch (ec_event_clearing) { + case ACPI_EC_EVT_TIMING_STATUS: + if (flag == ACPI_EC_COMMAND_POLL) acpi_ec_close_event(ec); - if (ec_event_clearing == ACPI_EC_EVT_TIMING_QUERY && - flag == ACPI_EC_COMMAND_COMPLETE) + + return; + + case ACPI_EC_EVT_TIMING_QUERY: + if (flag == ACPI_EC_COMMAND_COMPLETE) acpi_ec_close_event(ec); - if (ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT && - flag == ACPI_EC_COMMAND_COMPLETE) - set_bit(EC_FLAGS_QUERY_GUARDING, &ec->flags); + + return; + + case ACPI_EC_EVT_TIMING_EVENT: + if (flag == ACPI_EC_COMMAND_COMPLETE) + acpi_ec_complete_event(ec); } } @@ -674,11 +688,9 @@ static bool advance_transaction(struct acpi_ec *ec, bool interrupt) */ if (!t || !(t->flags & ACPI_EC_COMMAND_POLL)) { if (ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT && - (!ec->events_to_process || - test_bit(EC_FLAGS_QUERY_GUARDING, &ec->flags))) { - clear_bit(EC_FLAGS_QUERY_GUARDING, &ec->flags); + ec->event_state == EC_EVENT_COMPLETE) acpi_ec_close_event(ec); - } + if (!t) goto out; } @@ -1246,8 +1258,9 @@ static void acpi_ec_event_handler(struct work_struct *work) * event handling work again regardless of whether or not the query * queued up above is processed successfully. */ - if (ec_event_clearing == ACPI_EC_EVT_TIMING_STATUS || - ec_event_clearing == ACPI_EC_EVT_TIMING_QUERY) + if (ec_event_clearing == ACPI_EC_EVT_TIMING_EVENT) + acpi_ec_complete_event(ec); + else acpi_ec_close_event(ec); spin_unlock_irq(&ec->lock); diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index de546be3bc6a6b..1db3a2f81763fe 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -166,6 +166,13 @@ static inline void acpi_early_processor_osc(void) {} /* -------------------------------------------------------------------------- Embedded Controller -------------------------------------------------------------------------- */ + +enum acpi_ec_event_state { + EC_EVENT_READY = 0, /* Event work can be submitted */ + EC_EVENT_IN_PROGRESS, /* Event work is pending or being processed */ + EC_EVENT_COMPLETE, /* Event work processing has completed */ +}; + struct acpi_ec { acpi_handle handle; int gpe; @@ -182,6 +189,7 @@ struct acpi_ec { spinlock_t lock; struct work_struct work; unsigned long timestamp; + enum acpi_ec_event_state event_state; unsigned int events_to_process; unsigned int events_in_progress; unsigned int queries_in_progress; From befd9b5b0c621af33a363596c65a8fc0176e2795 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 23 Nov 2021 19:46:09 +0100 Subject: [PATCH 12/15] ACPI: EC: Relocate acpi_ec_create_query() and drop acpi_ec_delete_query() Move acpi_ec_create_query() after acpi_ec_event_processor(), drop the no longer needed forward declaration of the latter, and eliminate acpi_ec_delete_query() which isn't really necessary. No intentional functional impact. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 54 ++++++++++++++++++++--------------------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 7d7bff4d2100cc..0077d2c85df870 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -170,7 +170,6 @@ struct acpi_ec_query { static int acpi_ec_submit_query(struct acpi_ec *ec); static bool advance_transaction(struct acpi_ec *ec, bool interrupt); static void acpi_ec_event_handler(struct work_struct *work); -static void acpi_ec_event_processor(struct work_struct *work); struct acpi_ec *first_ec; EXPORT_SYMBOL(first_ec); @@ -1134,33 +1133,6 @@ void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit) } EXPORT_SYMBOL_GPL(acpi_ec_remove_query_handler); -static struct acpi_ec_query *acpi_ec_create_query(struct acpi_ec *ec, u8 *pval) -{ - struct acpi_ec_query *q; - struct transaction *t; - - q = kzalloc(sizeof (struct acpi_ec_query), GFP_KERNEL); - if (!q) - return NULL; - - INIT_WORK(&q->work, acpi_ec_event_processor); - t = &q->transaction; - t->command = ACPI_EC_COMMAND_QUERY; - t->rdata = pval; - t->rlen = 1; - q->ec = ec; - return q; -} - -static void acpi_ec_delete_query(struct acpi_ec_query *q) -{ - if (q) { - if (q->handler) - acpi_ec_put_query_handler(q->handler); - kfree(q); - } -} - static void acpi_ec_event_processor(struct work_struct *work) { struct acpi_ec_query *q = container_of(work, struct acpi_ec_query, work); @@ -1180,7 +1152,26 @@ static void acpi_ec_event_processor(struct work_struct *work) ec->queries_in_progress--; spin_unlock_irq(&ec->lock); - acpi_ec_delete_query(q); + acpi_ec_put_query_handler(handler); + kfree(q); +} + +static struct acpi_ec_query *acpi_ec_create_query(struct acpi_ec *ec, u8 *pval) +{ + struct acpi_ec_query *q; + struct transaction *t; + + q = kzalloc(sizeof (struct acpi_ec_query), GFP_KERNEL); + if (!q) + return NULL; + + INIT_WORK(&q->work, acpi_ec_event_processor); + t = &q->transaction; + t->command = ACPI_EC_COMMAND_QUERY; + t->rdata = pval; + t->rlen = 1; + q->ec = ec; + return q; } static int acpi_ec_submit_query(struct acpi_ec *ec) @@ -1229,9 +1220,10 @@ static int acpi_ec_submit_query(struct acpi_ec *ec) spin_unlock_irq(&ec->lock); + return 0; + err_exit: - if (result) - acpi_ec_delete_query(q); + kfree(q); return result; } From b66f86849414807745b5c2129e2de5f27a788c9f Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 25 Nov 2021 11:36:16 +0100 Subject: [PATCH 13/15] ACPI: EC: Mark the ec_sys write_support param as module_param_hw() Using write_support=1 with the ec_sys module changes the mode of the "io" debugfs file to 0600. This will cause any attempts to access it under a kernel in lockdown mode to return -EPERM, which makes the entire ec_sys module unusable. Use the special module_param_hw() macro for module parameters which may not be used while in lockdown mode, to avoid this. Signed-off-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec_sys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/acpi/ec_sys.c b/drivers/acpi/ec_sys.c index fd39c14493ab87..c074a0fae059fa 100644 --- a/drivers/acpi/ec_sys.c +++ b/drivers/acpi/ec_sys.c @@ -19,7 +19,7 @@ MODULE_DESCRIPTION("ACPI EC sysfs access driver"); MODULE_LICENSE("GPL"); static bool write_support; -module_param(write_support, bool, 0644); +module_param_hw(write_support, bool, other, 0644); MODULE_PARM_DESC(write_support, "Dangerous, reboot and removal of battery may " "be needed."); From 87ebbb8c612b1214f227ebb8f25442c6d163e802 Mon Sep 17 00:00:00 2001 From: "Kirill A. Shutemov" Date: Mon, 6 Dec 2021 15:29:51 +0300 Subject: [PATCH 14/15] ACPI: processor: idle: Only flush cache on entering C3 According to ACPI 6.4, Section 8.2, CPU cache flushing required on entering the C3 power state. Avoid flushing the cache on entering other C-states. Signed-off-by: Kirill A. Shutemov [ rjw: Changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/processor_idle.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 4b906bb527e8cc..d94794bc269e55 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -565,7 +565,8 @@ static int acpi_idle_play_dead(struct cpuidle_device *dev, int index) { struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu); - ACPI_FLUSH_CPU_CACHE(); + if (cx->type == ACPI_STATE_C3) + ACPI_FLUSH_CPU_CACHE(); while (1) { From 8120832d8f82aa7316c578fbccf11e385a5b3601 Mon Sep 17 00:00:00 2001 From: Manfred Spraul Date: Wed, 22 Dec 2021 15:09:31 +0100 Subject: [PATCH 15/15] ACPI: processor: thermal: avoid cpufreq_get_policy() cpu_has_cpufreq() stores a 'struct cpufreq_policy' on the stack. Unfortunately, with debugging options enabled, the structure can be larger than 1024 bytes, which causes a compiler warning/error. (actually observed: 1184 bytes). Therefore: Switch to cpufreq_cpu_get(). Signed-off-by: Manfred Spraul Signed-off-by: Rafael J. Wysocki --- drivers/acpi/processor_thermal.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c index a3d34e3f9f94be..d8b2dfcd59b5ff 100644 --- a/drivers/acpi/processor_thermal.c +++ b/drivers/acpi/processor_thermal.c @@ -53,10 +53,17 @@ static int phys_package_first_cpu(int cpu) static int cpu_has_cpufreq(unsigned int cpu) { - struct cpufreq_policy policy; - if (!acpi_processor_cpufreq_init || cpufreq_get_policy(&policy, cpu)) + struct cpufreq_policy *policy; + + if (!acpi_processor_cpufreq_init) return 0; - return 1; + + policy = cpufreq_cpu_get(cpu); + if (policy) { + cpufreq_cpu_put(policy); + return 1; + } + return 0; } static int cpufreq_get_max_state(unsigned int cpu)