From df45db6177f8dde380d44149cca46ad800a00575 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Wed, 3 Aug 2016 09:07:58 +0800 Subject: ACPI / EC: Add PM operations for suspend/resume noirq stage It is reported that on some platforms, resume speed is not fast. The cause is: in noirq stage, EC driver is working in polling mode, and each state machine advancement requires a context switch. The context switch is not necessary to the EC driver's polling mode. This patch implements PM hooks to automatically switch the driver to/from the busy polling mode to eliminate the overhead caused by the context switch. This finally contributes to the tuning result: acpi_pm_finish() execution time is improved from 192ms to 6ms. Signed-off-by: Lv Zheng Reported-and-tested-by: Todd E Brandt [ rjw: Subject ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ drivers/acpi/internal.h | 2 ++ 2 files changed, 55 insertions(+) (limited to 'drivers') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index e7bd57cc550a..6f6c7d1eaf8c 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1619,6 +1619,58 @@ error: return ret; } +#ifdef CONFIG_PM_SLEEP +static void acpi_ec_enter_noirq(struct acpi_ec *ec) +{ + unsigned long flags; + + if (ec == first_ec) { + spin_lock_irqsave(&ec->lock, flags); + ec->saved_busy_polling = ec_busy_polling; + ec->saved_polling_guard = ec_polling_guard; + ec_busy_polling = true; + ec_polling_guard = 0; + ec_log_drv("interrupt blocked"); + spin_unlock_irqrestore(&ec->lock, flags); + } +} + +static void acpi_ec_leave_noirq(struct acpi_ec *ec) +{ + unsigned long flags; + + if (ec == first_ec) { + spin_lock_irqsave(&ec->lock, flags); + ec_busy_polling = ec->saved_busy_polling; + ec_polling_guard = ec->saved_polling_guard; + ec_log_drv("interrupt unblocked"); + spin_unlock_irqrestore(&ec->lock, flags); + } +} + +static int acpi_ec_suspend_noirq(struct device *dev) +{ + struct acpi_ec *ec = + acpi_driver_data(to_acpi_device(dev)); + + acpi_ec_enter_noirq(ec); + return 0; +} + +static int acpi_ec_resume_noirq(struct device *dev) +{ + struct acpi_ec *ec = + acpi_driver_data(to_acpi_device(dev)); + + acpi_ec_leave_noirq(ec); + return 0; +} +#endif + +static const struct dev_pm_ops acpi_ec_pm = { + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(acpi_ec_suspend_noirq, acpi_ec_resume_noirq) +}; + static int param_set_event_clearing(const char *val, struct kernel_param *kp) { int result = 0; @@ -1664,6 +1716,7 @@ static struct acpi_driver acpi_ec_driver = { .add = acpi_ec_add, .remove = acpi_ec_remove, }, + .drv.pm = &acpi_ec_pm, }; static inline int acpi_ec_query_init(void) diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 940218ff0193..6996121ee003 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -174,6 +174,8 @@ struct acpi_ec { struct work_struct work; unsigned long timestamp; unsigned long nr_pending_queries; + bool saved_busy_polling; + unsigned int saved_polling_guard; }; extern struct acpi_ec *first_ec; -- cgit v1.2.3 From 750f628be68e8b8e1624d8abd003b9f1fc758ed6 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Wed, 3 Aug 2016 16:01:24 +0800 Subject: ACPI / EC: Add EC_FLAGS_QUERY_ENABLED to reveal a hidden logic There is a hidden logic in the EC driver: 1. During boot, EC_FLAGS_QUERY_PENDING is responsible for blocking event handling; 2. During suspend, EC_FLAGS_STARTED is responsible for blocking event handling. This patch uses a new EC_FLAGS_QUERY_ENABLED flag to make this hidden logic explicit and have code cleaned up. No functional change. Signed-off-by: Lv Zheng Tested-by: Todd E Brandt Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 103 +++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 71 insertions(+), 32 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 6f6c7d1eaf8c..4ab34d7dd943 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -104,6 +104,7 @@ enum ec_command { #define ACPI_EC_MAX_QUERIES 16 /* Maximum number of parallel queries */ 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_GPE_HANDLER_INSTALLED, /* GPE handler installed */ @@ -239,6 +240,22 @@ static bool acpi_ec_started(struct acpi_ec *ec) !test_bit(EC_FLAGS_STOPPED, &ec->flags); } +static bool acpi_ec_event_enabled(struct acpi_ec *ec) +{ + /* + * There is an OSPM early stage logic. During the early stages + * (boot/resume), OSPMs shouldn't enable the event handling, only + * the EC transactions are allowed to be performed. + */ + if (!test_bit(EC_FLAGS_QUERY_ENABLED, &ec->flags)) + return false; + /* + * The EC event handling is automatically disabled as soon as the + * EC driver is stopped. + */ + return test_bit(EC_FLAGS_STARTED, &ec->flags); +} + static bool acpi_ec_flushed(struct acpi_ec *ec) { return ec->reference_count == 1; @@ -429,7 +446,8 @@ static bool acpi_ec_submit_flushable_request(struct acpi_ec *ec) static void acpi_ec_submit_query(struct acpi_ec *ec) { - if (!test_and_set_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) { + if (acpi_ec_event_enabled(ec) && + !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++; @@ -446,6 +464,52 @@ static void acpi_ec_complete_query(struct acpi_ec *ec) } } +static inline void __acpi_ec_enable_event(struct acpi_ec *ec) +{ + if (!test_and_set_bit(EC_FLAGS_QUERY_ENABLED, &ec->flags)) + ec_log_drv("event unblocked"); +} + +static inline void __acpi_ec_disable_event(struct acpi_ec *ec) +{ + if (test_and_clear_bit(EC_FLAGS_QUERY_ENABLED, &ec->flags)) + ec_log_drv("event blocked"); +} + +/* + * Process _Q events that might have accumulated in the EC. + * Run with locked ec mutex. + */ +static void acpi_ec_clear(struct acpi_ec *ec) +{ + int i, status; + u8 value = 0; + + for (i = 0; i < ACPI_EC_CLEAR_MAX; i++) { + status = acpi_ec_query(ec, &value); + if (status || !value) + break; + } + if (unlikely(i == ACPI_EC_CLEAR_MAX)) + pr_warn("Warning: Maximum of %d stale EC events cleared\n", i); + else + pr_info("%d stale EC events cleared\n", i); +} + +static void acpi_ec_enable_event(struct acpi_ec *ec) +{ + unsigned long flags; + + spin_lock_irqsave(&ec->lock, flags); + if (acpi_ec_started(ec)) + __acpi_ec_enable_event(ec); + spin_unlock_irqrestore(&ec->lock, flags); + + /* Drain additional events if hardware requires that */ + if (EC_FLAGS_CLEAR_ON_RESUME) + acpi_ec_clear(ec); +} + static bool acpi_ec_guard_event(struct acpi_ec *ec) { bool guarded = true; @@ -832,27 +896,6 @@ acpi_handle ec_get_handle(void) } EXPORT_SYMBOL(ec_get_handle); -/* - * Process _Q events that might have accumulated in the EC. - * Run with locked ec mutex. - */ -static void acpi_ec_clear(struct acpi_ec *ec) -{ - int i, status; - u8 value = 0; - - for (i = 0; i < ACPI_EC_CLEAR_MAX; i++) { - status = acpi_ec_query(ec, &value); - if (status || !value) - break; - } - - if (unlikely(i == ACPI_EC_CLEAR_MAX)) - pr_warn("Warning: Maximum of %d stale EC events cleared\n", i); - else - pr_info("%d stale EC events cleared\n", i); -} - static void acpi_ec_start(struct acpi_ec *ec, bool resuming) { unsigned long flags; @@ -864,7 +907,8 @@ static void acpi_ec_start(struct acpi_ec *ec, bool resuming) if (!resuming) { acpi_ec_submit_request(ec); ec_dbg_ref(ec, "Increase driver"); - } + } else + __acpi_ec_enable_event(ec); ec_log_drv("EC started"); } spin_unlock_irqrestore(&ec->lock, flags); @@ -896,7 +940,8 @@ static void acpi_ec_stop(struct acpi_ec *ec, bool suspending) if (!suspending) { acpi_ec_complete_request(ec); ec_dbg_ref(ec, "Decrease driver"); - } + } else + __acpi_ec_disable_event(ec); clear_bit(EC_FLAGS_STARTED, &ec->flags); clear_bit(EC_FLAGS_STOPPED, &ec->flags); ec_log_drv("EC stopped"); @@ -927,8 +972,7 @@ void acpi_ec_unblock_transactions(void) /* Allow transactions to be carried out again */ acpi_ec_start(ec, true); - if (EC_FLAGS_CLEAR_ON_RESUME) - acpi_ec_clear(ec); + acpi_ec_enable_event(ec); } void acpi_ec_unblock_transactions_early(void) @@ -1234,7 +1278,6 @@ static struct acpi_ec *make_acpi_ec(void) if (!ec) return NULL; - ec->flags = 1 << EC_FLAGS_QUERY_PENDING; mutex_init(&ec->mutex); init_waitqueue_head(&ec->wait); INIT_LIST_HEAD(&ec->list); @@ -1421,11 +1464,7 @@ static int acpi_ec_add(struct acpi_device *device) acpi_walk_dep_device_list(ec->handle); /* EC is fully operational, allow queries */ - clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags); - - /* Clear stale _Q events if hardware might require that */ - if (EC_FLAGS_CLEAR_ON_RESUME) - acpi_ec_clear(ec); + acpi_ec_enable_event(ec); return ret; } -- cgit v1.2.3 From e923e8e79e18fd6be9162f1be6b99a002e9df2cb Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Wed, 3 Aug 2016 16:01:30 +0800 Subject: ACPI / EC: Fix an issue that SCI_EVT cannot be detected after event is enabled After enabling the EC event handling, Linux is still in the noirq stage, if there is no triggering source (EC transaction, GPE STS status), advance_transaction() will not be invoked and SCI_EVT cannot be detected. This patch adds one more triggering source after enabling the EC event handling to poll the pending SCI_EVT. Known issues: 1. Still no SCI_EVT triggering source There could still be no SCI_EVT triggering source after handling the first SCI_EVT (polled by this patch if any). Because after handling the first SCI_EVT, Linux could still be in noirq stage and there could still be no further triggering source in this stage. Then the second SCI_EVT indicated during this stage still cannot be detected by the EC driver. With this improvement applied, it is then possible to move acpi_ec_enable_event() out of the noirq stage to fix this issue (if the first SCI_EVT is handled out of the noirq stage, the follow-up SCI_EVTs should be able to trigger IRQs). Signed-off-by: Lv Zheng Tested-by: Todd E Brandt Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 4ab34d7dd943..79305bed4164 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -468,6 +468,8 @@ static inline void __acpi_ec_enable_event(struct acpi_ec *ec) { if (!test_and_set_bit(EC_FLAGS_QUERY_ENABLED, &ec->flags)) ec_log_drv("event unblocked"); + if (!test_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) + advance_transaction(ec); } static inline void __acpi_ec_disable_event(struct acpi_ec *ec) -- cgit v1.2.3 From c2b46d679b30c5c0d7eb47a21085943242bdd8dc Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Wed, 3 Aug 2016 16:01:36 +0800 Subject: ACPI / EC: Add PM operations to improve event handling for resume process This patch makes 2 changes: 1. Restore old behavior Originally, EC driver stops handling both events and transactions in acpi_ec_block_transactions(), and restarts to handle transactions in acpi_ec_unblock_transactions_early(), restarts to handle both events and transactions in acpi_ec_unblock_transactions(). While currently, EC driver still stops handling both events and transactions in acpi_ec_block_transactions(), but restarts to handle both events and transactions in acpi_ec_unblock_transactions_early(). This patch tries to restore the old behavior by dropping __acpi_ec_enable_event() from acpi_unblock_transactions_early(). 2. Improve old behavior However this still cannot fix the real issue as both of the acpi_ec_unblock_xxx() functions are invoked in the noirq stage. Since the EC driver actually doesn't implement the event handling in the polling mode, re-enabling the event handling too early in the noirq stage could result in the problem that if there is no triggering source causing advance_transaction() to be invoked, pending SCI_EVT cannot be detected by the EC driver and _Qxx cannot be triggered. It actually makes sense to restart the event handling in any point during resuming after the noirq stage. Just like the boot stage where the event handling is enabled in .add(), this patch further moves acpi_ec_enable_event() to .resume(). After doing that, the following 2 functions can be combined: acpi_ec_unblock_transactions_early()/acpi_ec_unblock_transactions(). The differences of the event handling availability between the old behavior (this patch isn't applied) and the new behavior (this patch is applied) are as follows: !Applied Applied before suspend Y Y suspend before EC Y Y suspend after EC Y Y suspend_late Y Y suspend_noirq Y (actually N) Y (actually N) resume_noirq Y (actually N) Y (actually N) resume_late Y (actually N) Y (actually N) resume before EC Y (actually N) Y (actually N) resume after EC Y (actually N) Y after resume Y (actually N) Y Where "actually N" means if there is no triggering source, the EC driver is actually not able to notice the pending SCI_EVT occurred in the noirq stage. So we can clearly see that this patch has improved the situation. Signed-off-by: Lv Zheng Tested-by: Todd E Brandt Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 26 +++++++++++--------------- drivers/acpi/internal.h | 1 - drivers/acpi/sleep.c | 4 ++-- 3 files changed, 13 insertions(+), 18 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 79305bed4164..8d5444defd7e 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -909,8 +909,7 @@ static void acpi_ec_start(struct acpi_ec *ec, bool resuming) if (!resuming) { acpi_ec_submit_request(ec); ec_dbg_ref(ec, "Increase driver"); - } else - __acpi_ec_enable_event(ec); + } ec_log_drv("EC started"); } spin_unlock_irqrestore(&ec->lock, flags); @@ -965,19 +964,6 @@ void acpi_ec_block_transactions(void) } void acpi_ec_unblock_transactions(void) -{ - struct acpi_ec *ec = first_ec; - - if (!ec) - return; - - /* Allow transactions to be carried out again */ - acpi_ec_start(ec, true); - - acpi_ec_enable_event(ec); -} - -void acpi_ec_unblock_transactions_early(void) { /* * Allow transactions to happen again (this function is called from @@ -1706,10 +1692,20 @@ static int acpi_ec_resume_noirq(struct device *dev) acpi_ec_leave_noirq(ec); return 0; } + +static int acpi_ec_resume(struct device *dev) +{ + struct acpi_ec *ec = + acpi_driver_data(to_acpi_device(dev)); + + acpi_ec_enable_event(ec); + return 0; +} #endif static const struct dev_pm_ops acpi_ec_pm = { SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(acpi_ec_suspend_noirq, acpi_ec_resume_noirq) + SET_SYSTEM_SLEEP_PM_OPS(NULL, acpi_ec_resume) }; static int param_set_event_clearing(const char *val, struct kernel_param *kp) diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 6996121ee003..29f206318d3d 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -189,7 +189,6 @@ int acpi_ec_ecdt_probe(void); int acpi_ec_dsdt_probe(void); void acpi_ec_block_transactions(void); void acpi_ec_unblock_transactions(void); -void acpi_ec_unblock_transactions_early(void); int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle, acpi_ec_query_func func, void *data); diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 2b38c1bb0446..bb1e0d21f828 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -586,7 +586,7 @@ static int acpi_suspend_enter(suspend_state_t pm_state) */ acpi_disable_all_gpes(); /* Allow EC transactions to happen. */ - acpi_ec_unblock_transactions_early(); + acpi_ec_unblock_transactions(); suspend_nvs_restore(); @@ -784,7 +784,7 @@ static void acpi_hibernation_leave(void) /* Restore the NVS memory area */ suspend_nvs_restore(); /* Allow EC transactions to happen. */ - acpi_ec_unblock_transactions_early(); + acpi_ec_unblock_transactions(); } static void acpi_pm_thaw(void) -- cgit v1.2.3 From 39a2a2aa3e9e5538984e9130c92a6c889ad86435 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Wed, 3 Aug 2016 16:01:43 +0800 Subject: ACPI / EC: Add PM operations to improve event handling for suspend process In the original EC driver, though the event handling is not explicitly stopped, the EC driver is actually not able to handle events during the noirq stage as the EC driver is not prepared to handle the EC events in the polling mode. So if there is no advance_transaction() triggered, the EC driver couldn't notice the EC events. However, do we actually need to handle EC events during suspend/resume stage? EC events are mostly useless for the suspend/resume period (key strokes and battery/thermal updates, etc.,), and the useful ones (lid close, power/sleep button press) should have already been delivered to the OSPM to trigger the power saving operations. Thus this patch implements acpi_ec_disable_event() to be a reverse call of acpi_ec_enable_event(), with which, the EC driver is able to stop handling the EC events in a position before entering the noirq stage. Since there are actually 2 choices for us: 1. implement event handling in polling mode; 2. stop event handling before entering noirq stage. And this patch only implements the second choice using .suspend() callback. Thus this is experimental (first choice is better? or different hook position is better?). This patch finally keeps the old behavior by default and prepares a boot parameter to enable this feature. The differences of the event handling availability between the old behavior (this patch is not applied) and the new behavior (this patch is applied) are as follows: !FreezeEvents FreezeEvents before suspend Y Y suspend before EC Y Y suspend after EC Y N suspend_late Y N suspend_noirq Y (actually N) N resume_noirq Y (actually N) N resume_late Y (actually N) N resume before EC Y (actually N) N resume after EC Y Y after resume Y Y Where "actually N" means if there is no EC transactions, the EC driver is actually not able to notice the pending events. We can see that FreezeEvents is the only approach now can actually flush the EC event handling with both query commands and _Qxx evaluations flushed, other modes can only flush the EC event handling with only query commands flushed, _Qxx evaluations occurred after stopping the EC driver may end up failure due to the failure of the EC transaction carried out in the _Qxx control methods. We also can see that this feature should be able to trigger some platform notifications later than resuming other drivers. Signed-off-by: Lv Zheng Tested-by: Todd E Brandt Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 8d5444defd7e..2bec709eb4e5 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -146,6 +146,10 @@ static unsigned int ec_storm_threshold __read_mostly = 8; module_param(ec_storm_threshold, uint, 0644); MODULE_PARM_DESC(ec_storm_threshold, "Maxim false GPE numbers not considered as GPE storm"); +static bool ec_freeze_events __read_mostly = false; +module_param(ec_freeze_events, bool, 0644); +MODULE_PARM_DESC(ec_freeze_events, "Disabling event handling during suspend/resume"); + struct acpi_ec_query_handler { struct list_head node; acpi_ec_query_func func; @@ -250,10 +254,18 @@ static bool acpi_ec_event_enabled(struct acpi_ec *ec) if (!test_bit(EC_FLAGS_QUERY_ENABLED, &ec->flags)) return false; /* - * The EC event handling is automatically disabled as soon as the - * EC driver is stopped. + * However, disabling the event handling is experimental for late + * stage (suspend), and is controlled by the boot parameter of + * "ec_freeze_events": + * 1. true: The EC event handling is disabled before entering + * the noirq stage. + * 2. false: The EC event handling is automatically disabled as + * soon as the EC driver is stopped. */ - return test_bit(EC_FLAGS_STARTED, &ec->flags); + if (ec_freeze_events) + return acpi_ec_started(ec); + else + return test_bit(EC_FLAGS_STARTED, &ec->flags); } static bool acpi_ec_flushed(struct acpi_ec *ec) @@ -512,6 +524,38 @@ static void acpi_ec_enable_event(struct acpi_ec *ec) acpi_ec_clear(ec); } +static bool acpi_ec_query_flushed(struct acpi_ec *ec) +{ + bool flushed; + unsigned long flags; + + spin_lock_irqsave(&ec->lock, flags); + flushed = !ec->nr_pending_queries; + spin_unlock_irqrestore(&ec->lock, flags); + return flushed; +} + +static void __acpi_ec_flush_event(struct acpi_ec *ec) +{ + /* + * When ec_freeze_events is true, we need to flush events in + * the proper position before entering the noirq stage. + */ + wait_event(ec->wait, acpi_ec_query_flushed(ec)); + if (ec_query_wq) + flush_workqueue(ec_query_wq); +} + +static void acpi_ec_disable_event(struct acpi_ec *ec) +{ + unsigned long flags; + + spin_lock_irqsave(&ec->lock, flags); + __acpi_ec_disable_event(ec); + spin_unlock_irqrestore(&ec->lock, flags); + __acpi_ec_flush_event(ec); +} + static bool acpi_ec_guard_event(struct acpi_ec *ec) { bool guarded = true; @@ -941,7 +985,7 @@ static void acpi_ec_stop(struct acpi_ec *ec, bool suspending) if (!suspending) { acpi_ec_complete_request(ec); ec_dbg_ref(ec, "Decrease driver"); - } else + } else if (!ec_freeze_events) __acpi_ec_disable_event(ec); clear_bit(EC_FLAGS_STARTED, &ec->flags); clear_bit(EC_FLAGS_STOPPED, &ec->flags); @@ -1693,6 +1737,16 @@ static int acpi_ec_resume_noirq(struct device *dev) return 0; } +static int acpi_ec_suspend(struct device *dev) +{ + struct acpi_ec *ec = + acpi_driver_data(to_acpi_device(dev)); + + if (ec_freeze_events) + acpi_ec_disable_event(ec); + return 0; +} + static int acpi_ec_resume(struct device *dev) { struct acpi_ec *ec = @@ -1705,7 +1759,7 @@ static int acpi_ec_resume(struct device *dev) static const struct dev_pm_ops acpi_ec_pm = { SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(acpi_ec_suspend_noirq, acpi_ec_resume_noirq) - SET_SYSTEM_SLEEP_PM_OPS(NULL, acpi_ec_resume) + SET_SYSTEM_SLEEP_PM_OPS(acpi_ec_suspend, acpi_ec_resume) }; static int param_set_event_clearing(const char *val, struct kernel_param *kp) -- cgit v1.2.3 From d30283057ecdf8c543ae757ae34db3d7fd2d7732 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Wed, 3 Aug 2016 16:01:50 +0800 Subject: ACPI / EC: Enable event freeze mode to improve event handling for suspend process This patch enables the event freeze mode, flushing the EC event handling in .suspend() callback. This feature is experimental, if it is bisected out to be the cause of the real issues, please report the issues to the kernel bugzilla for further root causing and improvement. This mode eliminates useless _Qxx handling during the power saving operations, thus can help to tune the power saving operations faster. Tests show that this mode can efficiently block flooding _Qxx during the suspend process and tune the speed of the suspend faster. Signed-off-by: Lv Zheng Tested-by: Todd E Brandt Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 2bec709eb4e5..1925589ecf66 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -146,7 +146,7 @@ static unsigned int ec_storm_threshold __read_mostly = 8; module_param(ec_storm_threshold, uint, 0644); MODULE_PARM_DESC(ec_storm_threshold, "Maxim false GPE numbers not considered as GPE storm"); -static bool ec_freeze_events __read_mostly = false; +static bool ec_freeze_events __read_mostly = true; module_param(ec_freeze_events, bool, 0644); MODULE_PARM_DESC(ec_freeze_events, "Disabling event handling during suspend/resume"); -- cgit v1.2.3 From 72c77b7ea9ce781f4987840984a462e4456ba98e Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Wed, 7 Sep 2016 16:50:08 +0800 Subject: ACPI / EC: Cleanup first_ec/boot_ec code In order to support full ECDT (driving the ECDT EC after probing the namespace EC), we need to change our EC device alloc/free algorithm, ensure not to free old boot EC before qualifying new boot EC. This patch achieves this by cleaning up first_ec/boot_ec logic: 1. first_ec: used to perform transactions, so it is assigned in new acpi_ec_setup() function. 2. boot_ec: used to track early EC device, so it is assigned in new acpi_config_boot_ec() function which explictly tells the driver to save the EC device as early EC device. Link: https://bugzilla.kernel.org/show_bug.cgi?id=115021 Reported-and-tested-by: Luya Tshimbalanga Tested-by: Jonh Henderson Signed-off-by: Lv Zheng Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 96 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 33 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 1925589ecf66..9eab651a4502 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -184,6 +184,7 @@ static void acpi_ec_event_processor(struct work_struct *work); struct acpi_ec *boot_ec, *first_ec; EXPORT_SYMBOL(first_ec); +static bool boot_ec_is_ecdt = false; static struct workqueue_struct *ec_query_wq; static int EC_FLAGS_CLEAR_ON_RESUME; /* Needs acpi_ec_clear() on boot/resume */ @@ -1304,7 +1305,16 @@ acpi_ec_space_handler(u32 function, acpi_physical_address address, static acpi_status ec_parse_io_ports(struct acpi_resource *resource, void *context); -static struct acpi_ec *make_acpi_ec(void) +static void acpi_ec_free(struct acpi_ec *ec) +{ + if (first_ec == ec) + first_ec = NULL; + if (boot_ec == ec) + boot_ec = NULL; + kfree(ec); +} + +static struct acpi_ec *acpi_ec_alloc(void) { struct acpi_ec *ec = kzalloc(sizeof(struct acpi_ec), GFP_KERNEL); @@ -1365,6 +1375,11 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval) return AE_CTRL_TERMINATE; } +/* + * Note: This function returns an error code only when the address space + * handler is not installed, which means "not able to handle + * transactions". + */ static int ec_install_handlers(struct acpi_ec *ec) { acpi_status status; @@ -1440,21 +1455,49 @@ static void ec_remove_handlers(struct acpi_ec *ec) } } -static struct acpi_ec *acpi_ec_alloc(void) +static int acpi_ec_setup(struct acpi_ec *ec) { - struct acpi_ec *ec; + int ret; - /* Check for boot EC */ - if (boot_ec) { - ec = boot_ec; - boot_ec = NULL; - ec_remove_handlers(ec); - if (first_ec == ec) - first_ec = NULL; - } else { - ec = make_acpi_ec(); + ret = ec_install_handlers(ec); + if (ret) + return ret; + + /* First EC capable of handling transactions */ + if (!first_ec) { + first_ec = ec; + acpi_handle_info(first_ec->handle, "Used as first EC\n"); } - return ec; + + acpi_handle_info(ec->handle, + "GPE=0x%lx, EC_CMD/EC_SC=0x%lx, EC_DATA=0x%lx\n", + ec->gpe, ec->command_addr, ec->data_addr); + return ret; +} + +static int acpi_config_boot_ec(struct acpi_ec *ec, bool is_ecdt) +{ + int ret; + + if (boot_ec) + ec_remove_handlers(boot_ec); + + /* Unset old boot EC */ + if (boot_ec != ec) + acpi_ec_free(boot_ec); + + ret = acpi_ec_setup(ec); + if (ret) + return ret; + + /* Set new boot EC */ + if (!boot_ec) { + boot_ec = ec; + boot_ec_is_ecdt = is_ecdt; + acpi_handle_info(boot_ec->handle, "Used as boot %s EC\n", + is_ecdt ? "ECDT" : "DSDT"); + } + return ret; } static int acpi_ec_add(struct acpi_device *device) @@ -1470,7 +1513,7 @@ static int acpi_ec_add(struct acpi_device *device) return -ENOMEM; if (ec_parse_device(device->handle, 0, ec, NULL) != AE_CTRL_TERMINATE) { - kfree(ec); + acpi_ec_free(ec); return -EINVAL; } @@ -1478,8 +1521,6 @@ static int acpi_ec_add(struct acpi_device *device) acpi_walk_namespace(ACPI_TYPE_METHOD, ec->handle, 1, acpi_ec_register_query_methods, NULL, ec, NULL); - if (!first_ec) - first_ec = ec; device->driver_data = ec; ret = !!request_region(ec->data_addr, 1, "EC data"); @@ -1487,10 +1528,7 @@ static int acpi_ec_add(struct acpi_device *device) ret = !!request_region(ec->command_addr, 1, "EC cmd"); WARN(!ret, "Could not request EC cmd io port 0x%lx", ec->command_addr); - pr_info("GPE = 0x%lx, I/O: command/status = 0x%lx, data = 0x%lx\n", - ec->gpe, ec->command_addr, ec->data_addr); - - ret = ec_install_handlers(ec); + ret = acpi_config_boot_ec(ec, false); /* Reprobe devices depending on the EC */ acpi_walk_dep_device_list(ec->handle); @@ -1513,9 +1551,7 @@ static int acpi_ec_remove(struct acpi_device *device) release_region(ec->data_addr, 1); release_region(ec->command_addr, 1); device->driver_data = NULL; - if (ec == first_ec) - first_ec = NULL; - kfree(ec); + acpi_ec_free(ec); return 0; } @@ -1567,13 +1603,10 @@ int __init acpi_ec_dsdt_probe(void) ret = -ENODEV; goto error; } - ret = ec_install_handlers(ec); - + ret = acpi_config_boot_ec(ec, false); error: if (ret) - kfree(ec); - else - first_ec = boot_ec = ec; + acpi_ec_free(ec); return ret; } @@ -1671,7 +1704,6 @@ int __init acpi_ec_ecdt_probe(void) goto error; } - pr_info("EC description table is found, configuring boot EC\n"); if (EC_FLAGS_CORRECT_ECDT) { ec->command_addr = ecdt_ptr->data.address; ec->data_addr = ecdt_ptr->control.address; @@ -1681,12 +1713,10 @@ int __init acpi_ec_ecdt_probe(void) } ec->gpe = ecdt_ptr->gpe; ec->handle = ACPI_ROOT_OBJECT; - ret = ec_install_handlers(ec); + ret = acpi_config_boot_ec(ec, true); error: if (ret) - kfree(ec); - else - first_ec = boot_ec = ec; + acpi_ec_free(ec); return ret; } -- cgit v1.2.3 From 46922d2a3aff5122253d97e64500801c08f4f2c0 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Wed, 7 Sep 2016 16:50:14 +0800 Subject: ACPI / EC: Fix a memory leakage issue in acpi_ec_add() When the handler installation failed, there was no code to free the allocated EC device. This patch fixes this memory leakage issue. Link: https://bugzilla.kernel.org/show_bug.cgi?id=115021 Reported-and-tested-by: Luya Tshimbalanga Tested-by: Jonh Henderson Signed-off-by: Lv Zheng Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 9eab651a4502..50895fff6964 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1513,14 +1513,18 @@ static int acpi_ec_add(struct acpi_device *device) return -ENOMEM; if (ec_parse_device(device->handle, 0, ec, NULL) != AE_CTRL_TERMINATE) { - acpi_ec_free(ec); - return -EINVAL; + ret = -EINVAL; + goto err_alloc; } /* Find and register all query methods */ acpi_walk_namespace(ACPI_TYPE_METHOD, ec->handle, 1, acpi_ec_register_query_methods, NULL, ec, NULL); + ret = acpi_config_boot_ec(ec, false); + if (ret) + goto err_query; + device->driver_data = ec; ret = !!request_region(ec->data_addr, 1, "EC data"); @@ -1528,13 +1532,17 @@ static int acpi_ec_add(struct acpi_device *device) ret = !!request_region(ec->command_addr, 1, "EC cmd"); WARN(!ret, "Could not request EC cmd io port 0x%lx", ec->command_addr); - ret = acpi_config_boot_ec(ec, false); - /* Reprobe devices depending on the EC */ acpi_walk_dep_device_list(ec->handle); /* EC is fully operational, allow queries */ acpi_ec_enable_event(ec); + return 0; + +err_query: + acpi_ec_remove_query_handlers(ec, true, 0); +err_alloc: + acpi_ec_free(ec); return ret; } -- cgit v1.2.3 From 2a5708409e4e05446eb1a89ecb48641d6fd5d5a9 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Wed, 7 Sep 2016 16:50:21 +0800 Subject: ACPI / EC: Fix a gap that ECDT EC cannot handle EC events It is possible to register _Qxx from namespace and use the ECDT EC to perform event handling. The reported bug reveals that Windows is using ECDT in this way in case the namespace EC is not present. This patch facilitates Linux to support ECDT in this way. Link: https://bugzilla.kernel.org/show_bug.cgi?id=115021 Reported-and-tested-by: Luya Tshimbalanga Tested-by: Jonh Henderson Reviewed-by: Peter Wu Signed-off-by: Lv Zheng Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 119 ++++++++++++++++++++++++++++++++++++++---------- drivers/acpi/internal.h | 1 + drivers/acpi/scan.c | 1 + 3 files changed, 98 insertions(+), 23 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 50895fff6964..2ae9194cc630 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -109,6 +109,7 @@ enum { EC_FLAGS_QUERY_GUARDING, /* Guard for SCI_EVT check */ EC_FLAGS_GPE_HANDLER_INSTALLED, /* GPE handler installed */ EC_FLAGS_EC_HANDLER_INSTALLED, /* OpReg handler installed */ + EC_FLAGS_EVT_HANDLER_INSTALLED, /* _Qxx handlers installed */ EC_FLAGS_STARTED, /* Driver is started */ EC_FLAGS_STOPPED, /* Driver is stopped */ EC_FLAGS_COMMAND_STORM, /* GPE storms occurred to the @@ -1380,7 +1381,7 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval) * handler is not installed, which means "not able to handle * transactions". */ -static int ec_install_handlers(struct acpi_ec *ec) +static int ec_install_handlers(struct acpi_ec *ec, bool handle_events) { acpi_status status; @@ -1409,6 +1410,16 @@ static int ec_install_handlers(struct acpi_ec *ec) set_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags); } + if (!handle_events) + return 0; + + if (!test_bit(EC_FLAGS_EVT_HANDLER_INSTALLED, &ec->flags)) { + /* Find and register all query methods */ + acpi_walk_namespace(ACPI_TYPE_METHOD, ec->handle, 1, + acpi_ec_register_query_methods, + NULL, ec, NULL); + set_bit(EC_FLAGS_EVT_HANDLER_INSTALLED, &ec->flags); + } if (!test_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags)) { status = acpi_install_gpe_raw_handler(NULL, ec->gpe, ACPI_GPE_EDGE_TRIGGERED, @@ -1419,6 +1430,9 @@ static int ec_install_handlers(struct acpi_ec *ec) if (test_bit(EC_FLAGS_STARTED, &ec->flags) && ec->reference_count >= 1) acpi_ec_enable_gpe(ec, true); + + /* EC is fully operational, allow queries */ + acpi_ec_enable_event(ec); } } @@ -1453,13 +1467,17 @@ static void ec_remove_handlers(struct acpi_ec *ec) pr_err("failed to remove gpe handler\n"); clear_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags); } + if (test_bit(EC_FLAGS_EVT_HANDLER_INSTALLED, &ec->flags)) { + acpi_ec_remove_query_handlers(ec, true, 0); + clear_bit(EC_FLAGS_EVT_HANDLER_INSTALLED, &ec->flags); + } } -static int acpi_ec_setup(struct acpi_ec *ec) +static int acpi_ec_setup(struct acpi_ec *ec, bool handle_events) { int ret; - ret = ec_install_handlers(ec); + ret = ec_install_handlers(ec, handle_events); if (ret) return ret; @@ -1475,18 +1493,33 @@ static int acpi_ec_setup(struct acpi_ec *ec) return ret; } -static int acpi_config_boot_ec(struct acpi_ec *ec, bool is_ecdt) +static int acpi_config_boot_ec(struct acpi_ec *ec, acpi_handle handle, + bool handle_events, bool is_ecdt) { int ret; - if (boot_ec) + /* + * Changing the ACPI handle results in a re-configuration of the + * boot EC. And if it happens after the namespace initialization, + * it causes _REG evaluations. + */ + if (boot_ec && boot_ec->handle != handle) ec_remove_handlers(boot_ec); /* Unset old boot EC */ if (boot_ec != ec) acpi_ec_free(boot_ec); - ret = acpi_ec_setup(ec); + /* + * ECDT device creation is split into acpi_ec_ecdt_probe() and + * acpi_ec_ecdt_start(). This function takes care of completing the + * ECDT parsing logic as the handle update should be performed + * between the installation/uninstallation of the handlers. + */ + if (ec->handle != handle) + ec->handle = handle; + + ret = acpi_ec_setup(ec, handle_events); if (ret) return ret; @@ -1494,9 +1527,12 @@ static int acpi_config_boot_ec(struct acpi_ec *ec, bool is_ecdt) if (!boot_ec) { boot_ec = ec; boot_ec_is_ecdt = is_ecdt; - acpi_handle_info(boot_ec->handle, "Used as boot %s EC\n", - is_ecdt ? "ECDT" : "DSDT"); } + + acpi_handle_info(boot_ec->handle, + "Used as boot %s EC to handle transactions%s\n", + is_ecdt ? "ECDT" : "DSDT", + handle_events ? " and events" : ""); return ret; } @@ -1517,11 +1553,7 @@ static int acpi_ec_add(struct acpi_device *device) goto err_alloc; } - /* Find and register all query methods */ - acpi_walk_namespace(ACPI_TYPE_METHOD, ec->handle, 1, - acpi_ec_register_query_methods, NULL, ec, NULL); - - ret = acpi_config_boot_ec(ec, false); + ret = acpi_config_boot_ec(ec, device->handle, true, false); if (ret) goto err_query; @@ -1534,9 +1566,6 @@ static int acpi_ec_add(struct acpi_device *device) /* Reprobe devices depending on the EC */ acpi_walk_dep_device_list(ec->handle); - - /* EC is fully operational, allow queries */ - acpi_ec_enable_event(ec); return 0; err_query: @@ -1555,7 +1584,6 @@ static int acpi_ec_remove(struct acpi_device *device) ec = acpi_driver_data(device); ec_remove_handlers(ec); - acpi_ec_remove_query_handlers(ec, true, 0); release_region(ec->data_addr, 1); release_region(ec->command_addr, 1); device->driver_data = NULL; @@ -1601,9 +1629,8 @@ int __init acpi_ec_dsdt_probe(void) if (!ec) return -ENOMEM; /* - * Finding EC from DSDT if there is no ECDT EC available. When this - * function is invoked, ACPI tables have been fully loaded, we can - * walk namespace now. + * At this point, the namespace is initialized, so start to find + * the namespace objects. */ status = acpi_get_devices(ec_device_ids[0].id, ec_parse_device, ec, NULL); @@ -1611,13 +1638,55 @@ int __init acpi_ec_dsdt_probe(void) ret = -ENODEV; goto error; } - ret = acpi_config_boot_ec(ec, false); + /* + * When the DSDT EC is available, always re-configure boot EC to + * have _REG evaluated. _REG can only be evaluated after the + * namespace initialization. + * At this point, the GPE is not fully initialized, so do not to + * handle the events. + */ + ret = acpi_config_boot_ec(ec, ec->handle, false, false); error: if (ret) acpi_ec_free(ec); return ret; } +/* + * If the DSDT EC is not functioning, we still need to prepare a fully + * functioning ECDT EC first in order to handle the events. + * https://bugzilla.kernel.org/show_bug.cgi?id=115021 + */ +int __init acpi_ec_ecdt_start(void) +{ + struct acpi_table_ecdt *ecdt_ptr; + acpi_status status; + acpi_handle handle; + + if (!boot_ec) + return -ENODEV; + /* + * The DSDT EC should have already been started in + * acpi_ec_add(). + */ + if (!boot_ec_is_ecdt) + return -ENODEV; + + status = acpi_get_table(ACPI_SIG_ECDT, 1, + (struct acpi_table_header **)&ecdt_ptr); + if (ACPI_FAILURE(status)) + return -ENODEV; + + /* + * At this point, the namespace and the GPE is initialized, so + * start to find the namespace objects and handle the events. + */ + status = acpi_get_handle(NULL, ecdt_ptr->id, &handle); + if (ACPI_FAILURE(status)) + return -ENODEV; + return acpi_config_boot_ec(boot_ec, handle, true, true); +} + #if 0 /* * Some EC firmware variations refuses to respond QR_EC when SCI_EVT is not @@ -1720,8 +1789,12 @@ int __init acpi_ec_ecdt_probe(void) ec->data_addr = ecdt_ptr->data.address; } ec->gpe = ecdt_ptr->gpe; - ec->handle = ACPI_ROOT_OBJECT; - ret = acpi_config_boot_ec(ec, true); + + /* + * At this point, the namespace is not initialized, so do not find + * the namespace objects, or handle the events. + */ + ret = acpi_config_boot_ec(ec, ACPI_ROOT_OBJECT, false, true); error: if (ret) acpi_ec_free(ec); diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 29f206318d3d..73bee2cbe41f 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -187,6 +187,7 @@ typedef int (*acpi_ec_query_func) (void *data); int acpi_ec_init(void); int acpi_ec_ecdt_probe(void); int acpi_ec_dsdt_probe(void); +int acpi_ec_ecdt_start(void); void acpi_ec_block_transactions(void); void acpi_ec_unblock_transactions(void); int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index ad9fc84a8601..763c0da506bf 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2044,6 +2044,7 @@ int __init acpi_scan_init(void) } acpi_update_all_gpes(); + acpi_ec_ecdt_start(); acpi_scan_initialized = true; -- cgit v1.2.3 From 97cb159fd91d00f8d7d1adeb075503dc0d946bff Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Wed, 7 Sep 2016 16:50:27 +0800 Subject: ACPI / EC: Fix issues related to boot_ec There are issues related to the boot_ec: 1. If acpi_ec_remove() is invoked, boot_ec will also be freed, this is not expected as the boot_ec could be enumerated via ECDT. 2. Address space handler installation/unstallation lead to unexpected _REG evaluations. This patch adds acpi_is_boot_ec() check to be used to fix the above issues. However, since acpi_ec_remove() actually won't be invoked, this patch doesn't handle the reference counting of "struct acpi_ec", it only ensures the correctness of the boot_ec destruction during the boot. Link: https://bugzilla.kernel.org/show_bug.cgi?id=153511 Reported-and-tested-by: Jonh Henderson Signed-off-by: Lv Zheng Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 63 ++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 49 insertions(+), 14 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 2ae9194cc630..680531062160 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1536,6 +1536,37 @@ static int acpi_config_boot_ec(struct acpi_ec *ec, acpi_handle handle, return ret; } +static bool acpi_ec_ecdt_get_handle(acpi_handle *phandle) +{ + struct acpi_table_ecdt *ecdt_ptr; + acpi_status status; + acpi_handle handle; + + status = acpi_get_table(ACPI_SIG_ECDT, 1, + (struct acpi_table_header **)&ecdt_ptr); + if (ACPI_FAILURE(status)) + return false; + + status = acpi_get_handle(NULL, ecdt_ptr->id, &handle); + if (ACPI_FAILURE(status)) + return false; + + *phandle = handle; + return true; +} + +static bool acpi_is_boot_ec(struct acpi_ec *ec) +{ + if (!boot_ec) + return false; + if (ec->handle == boot_ec->handle && + ec->gpe == boot_ec->gpe && + ec->command_addr == boot_ec->command_addr && + ec->data_addr == boot_ec->data_addr) + return true; + return false; +} + static int acpi_ec_add(struct acpi_device *device) { struct acpi_ec *ec = NULL; @@ -1553,7 +1584,14 @@ static int acpi_ec_add(struct acpi_device *device) goto err_alloc; } - ret = acpi_config_boot_ec(ec, device->handle, true, false); + if (acpi_is_boot_ec(ec)) { + boot_ec_is_ecdt = false; + acpi_handle_debug(ec->handle, "duplicated.\n"); + acpi_ec_free(ec); + ec = boot_ec; + ret = acpi_config_boot_ec(ec, ec->handle, true, false); + } else + ret = acpi_ec_setup(ec, true); if (ret) goto err_query; @@ -1566,12 +1604,15 @@ static int acpi_ec_add(struct acpi_device *device) /* Reprobe devices depending on the EC */ acpi_walk_dep_device_list(ec->handle); + acpi_handle_debug(ec->handle, "enumerated.\n"); return 0; err_query: - acpi_ec_remove_query_handlers(ec, true, 0); + if (ec != boot_ec) + acpi_ec_remove_query_handlers(ec, true, 0); err_alloc: - acpi_ec_free(ec); + if (ec != boot_ec) + acpi_ec_free(ec); return ret; } @@ -1583,11 +1624,13 @@ static int acpi_ec_remove(struct acpi_device *device) return -EINVAL; ec = acpi_driver_data(device); - ec_remove_handlers(ec); release_region(ec->data_addr, 1); release_region(ec->command_addr, 1); device->driver_data = NULL; - acpi_ec_free(ec); + if (ec != boot_ec) { + ec_remove_handlers(ec); + acpi_ec_free(ec); + } return 0; } @@ -1659,8 +1702,6 @@ error: */ int __init acpi_ec_ecdt_start(void) { - struct acpi_table_ecdt *ecdt_ptr; - acpi_status status; acpi_handle handle; if (!boot_ec) @@ -1672,17 +1713,11 @@ int __init acpi_ec_ecdt_start(void) if (!boot_ec_is_ecdt) return -ENODEV; - status = acpi_get_table(ACPI_SIG_ECDT, 1, - (struct acpi_table_header **)&ecdt_ptr); - if (ACPI_FAILURE(status)) - return -ENODEV; - /* * At this point, the namespace and the GPE is initialized, so * start to find the namespace objects and handle the events. */ - status = acpi_get_handle(NULL, ecdt_ptr->id, &handle); - if (ACPI_FAILURE(status)) + if (!acpi_ec_ecdt_get_handle(&handle)) return -ENODEV; return acpi_config_boot_ec(boot_ec, handle, true, true); } -- cgit v1.2.3 From 058dfc7670086edda8d34f0dbe93c596db5d4a6b Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Tue, 20 Sep 2016 15:30:51 +0300 Subject: ACPI / watchdog: Add support for WDAT hardware watchdog Starting from Intel Skylake the iTCO watchdog timer registers were moved to reside in the same register space with SMBus host controller. Not all needed registers are available though and we need to unhide P2SB (Primary to Sideband) device briefly to be able to read status of required NO_REBOOT bit. The i2c-i801.c SMBus driver used to handle this and creation of the iTCO watchdog platform device. Windows, on the other hand, does not use the iTCO watchdog hardware directly even if it is available. Instead it relies on ACPI Watchdog Action Table (WDAT) table to describe the watchdog hardware to the OS. This table contains necessary information about the the hardware and also set of actions which are executed by a driver as needed. This patch implements a new watchdog driver that takes advantage of the ACPI WDAT table. We split the functionality into two parts: first part enumerates the WDAT table and if found, populates resources and creates platform device for the actual driver. The second part is the driver itself. The reason for the split is that this way we can make the driver itself to be a module and loaded automatically if the WDAT table is found. Otherwise the module is not loaded. Signed-off-by: Mika Westerberg Reviewed-by: Guenter Roeck Signed-off-by: Rafael J. Wysocki --- drivers/acpi/Kconfig | 3 + drivers/acpi/Makefile | 1 + drivers/acpi/acpi_watchdog.c | 123 ++++++++++ drivers/acpi/internal.h | 10 + drivers/acpi/scan.c | 1 + drivers/watchdog/Kconfig | 13 ++ drivers/watchdog/Makefile | 1 + drivers/watchdog/wdat_wdt.c | 525 +++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 677 insertions(+) create mode 100644 drivers/acpi/acpi_watchdog.c create mode 100644 drivers/watchdog/wdat_wdt.c (limited to 'drivers') diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 445ce28475b3..dcfa7e9e92f5 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -462,6 +462,9 @@ source "drivers/acpi/nfit/Kconfig" source "drivers/acpi/apei/Kconfig" source "drivers/acpi/dptf/Kconfig" +config ACPI_WATCHDOG + bool + config ACPI_EXTLOG tristate "Extended Error Log support" depends on X86_MCE && X86_LOCAL_APIC diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 5ae9d85c5159..3a1fa8f03749 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -56,6 +56,7 @@ acpi-$(CONFIG_ACPI_NUMA) += numa.o acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o acpi-y += acpi_lpat.o acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o +acpi-$(CONFIG_ACPI_WATCHDOG) += acpi_watchdog.o # These are (potentially) separate modules diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c new file mode 100644 index 000000000000..13caebd679f5 --- /dev/null +++ b/drivers/acpi/acpi_watchdog.c @@ -0,0 +1,123 @@ +/* + * ACPI watchdog table parsing support. + * + * Copyright (C) 2016, Intel Corporation + * Author: Mika Westerberg + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#define pr_fmt(fmt) "ACPI: watchdog: " fmt + +#include +#include +#include + +#include "internal.h" + +/** + * Returns true if this system should prefer ACPI based watchdog instead of + * the native one (which are typically the same hardware). + */ +bool acpi_has_watchdog(void) +{ + struct acpi_table_header hdr; + + if (acpi_disabled) + return false; + + return ACPI_SUCCESS(acpi_get_table_header(ACPI_SIG_WDAT, 0, &hdr)); +} +EXPORT_SYMBOL_GPL(acpi_has_watchdog); + +void __init acpi_watchdog_init(void) +{ + const struct acpi_wdat_entry *entries; + const struct acpi_table_wdat *wdat; + struct list_head resource_list; + struct resource_entry *rentry; + struct platform_device *pdev; + struct resource *resources; + size_t nresources = 0; + acpi_status status; + int i; + + status = acpi_get_table(ACPI_SIG_WDAT, 0, + (struct acpi_table_header **)&wdat); + if (ACPI_FAILURE(status)) { + /* It is fine if there is no WDAT */ + return; + } + + /* Watchdog disabled by BIOS */ + if (!(wdat->flags & ACPI_WDAT_ENABLED)) + return; + + /* Skip legacy PCI WDT devices */ + if (wdat->pci_segment != 0xff || wdat->pci_bus != 0xff || + wdat->pci_device != 0xff || wdat->pci_function != 0xff) + return; + + INIT_LIST_HEAD(&resource_list); + + entries = (struct acpi_wdat_entry *)(wdat + 1); + for (i = 0; i < wdat->entries; i++) { + const struct acpi_generic_address *gas; + struct resource_entry *rentry; + struct resource res; + bool found; + + gas = &entries[i].register_region; + + res.start = gas->address; + if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) { + res.flags = IORESOURCE_MEM; + res.end = res.start + ALIGN(gas->access_width, 4); + } else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) { + res.flags = IORESOURCE_IO; + res.end = res.start + gas->access_width; + } else { + pr_warn("Unsupported address space: %u\n", + gas->space_id); + goto fail_free_resource_list; + } + + found = false; + resource_list_for_each_entry(rentry, &resource_list) { + if (resource_contains(rentry->res, &res)) { + found = true; + break; + } + } + + if (!found) { + rentry = resource_list_create_entry(NULL, 0); + if (!rentry) + goto fail_free_resource_list; + + *rentry->res = res; + resource_list_add_tail(rentry, &resource_list); + nresources++; + } + } + + resources = kcalloc(nresources, sizeof(*resources), GFP_KERNEL); + if (!resources) + goto fail_free_resource_list; + + i = 0; + resource_list_for_each_entry(rentry, &resource_list) + resources[i++] = *rentry->res; + + pdev = platform_device_register_simple("wdat_wdt", PLATFORM_DEVID_NONE, + resources, nresources); + if (IS_ERR(pdev)) + pr_err("Failed to create platform device\n"); + + kfree(resources); + +fail_free_resource_list: + resource_list_free(&resource_list); +} diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 940218ff0193..fb9a7ad96756 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -225,4 +225,14 @@ static inline void suspend_nvs_restore(void) {} void acpi_init_properties(struct acpi_device *adev); void acpi_free_properties(struct acpi_device *adev); +/*-------------------------------------------------------------------------- + Watchdog + -------------------------------------------------------------------------- */ + +#ifdef CONFIG_ACPI_WATCHDOG +void acpi_watchdog_init(void); +#else +static inline void acpi_watchdog_init(void) {} +#endif + #endif /* _ACPI_INTERNAL_H_ */ diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index e878fc799af7..3b85d87021da 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2002,6 +2002,7 @@ int __init acpi_scan_init(void) acpi_pnp_init(); acpi_int340x_thermal_init(); acpi_amba_init(); + acpi_watchdog_init(); acpi_scan_add_handler(&generic_device_handler); diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 1bffe006ca9a..50dbaa805658 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -152,6 +152,19 @@ config TANGOX_WATCHDOG This driver can be built as a module. The module name is tangox_wdt. +config WDAT_WDT + tristate "ACPI Watchdog Action Table (WDAT)" + depends on ACPI + select ACPI_WATCHDOG + help + This driver adds support for systems with ACPI Watchdog Action + Table (WDAT) table. Servers typically have this but it can be + found on some desktop machines as well. This driver will take + over the native iTCO watchdog driver found on many Intel CPUs. + + To compile this driver as module, choose M here: the module will + be called wdat_wdt. + config WM831X_WATCHDOG tristate "WM831x watchdog" depends on MFD_WM831X diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index c22ad3ea3539..cba00430151b 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -202,6 +202,7 @@ obj-$(CONFIG_DA9062_WATCHDOG) += da9062_wdt.o obj-$(CONFIG_DA9063_WATCHDOG) += da9063_wdt.o obj-$(CONFIG_GPIO_WATCHDOG) += gpio_wdt.o obj-$(CONFIG_TANGOX_WATCHDOG) += tangox_wdt.o +obj-$(CONFIG_WDAT_WDT) += wdat_wdt.o obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c new file mode 100644 index 000000000000..6b6464596674 --- /dev/null +++ b/drivers/watchdog/wdat_wdt.c @@ -0,0 +1,525 @@ +/* + * ACPI Hardware Watchdog (WDAT) driver. + * + * Copyright (C) 2016, Intel Corporation + * Author: Mika Westerberg + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include + +#define MAX_WDAT_ACTIONS ACPI_WDAT_ACTION_RESERVED + +/** + * struct wdat_instruction - Single ACPI WDAT instruction + * @entry: Copy of the ACPI table instruction + * @reg: Register the instruction is accessing + * @node: Next instruction in action sequence + */ +struct wdat_instruction { + struct acpi_wdat_entry entry; + void __iomem *reg; + struct list_head node; +}; + +/** + * struct wdat_wdt - ACPI WDAT watchdog device + * @pdev: Parent platform device + * @wdd: Watchdog core device + * @period: How long is one watchdog period in ms + * @stopped_in_sleep: Is this watchdog stopped by the firmware in S1-S5 + * @stopped: Was the watchdog stopped by the driver in suspend + * @actions: An array of instruction lists indexed by an action number from + * the WDAT table. There can be %NULL entries for not implemented + * actions. + */ +struct wdat_wdt { + struct platform_device *pdev; + struct watchdog_device wdd; + unsigned int period; + bool stopped_in_sleep; + bool stopped; + struct list_head *instructions[MAX_WDAT_ACTIONS]; +}; + +#define to_wdat_wdt(wdd) container_of(wdd, struct wdat_wdt, wdd) + +static bool nowayout = WATCHDOG_NOWAYOUT; +module_param(nowayout, bool, 0); +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); + +static int wdat_wdt_read(struct wdat_wdt *wdat, + const struct wdat_instruction *instr, u32 *value) +{ + const struct acpi_generic_address *gas = &instr->entry.register_region; + + switch (gas->access_width) { + case 1: + *value = ioread8(instr->reg); + break; + case 2: + *value = ioread16(instr->reg); + break; + case 3: + *value = ioread32(instr->reg); + break; + default: + return -EINVAL; + } + + dev_dbg(&wdat->pdev->dev, "Read %#x from 0x%08llx\n", *value, + gas->address); + + return 0; +} + +static int wdat_wdt_write(struct wdat_wdt *wdat, + const struct wdat_instruction *instr, u32 value) +{ + const struct acpi_generic_address *gas = &instr->entry.register_region; + + switch (gas->access_width) { + case 1: + iowrite8((u8)value, instr->reg); + break; + case 2: + iowrite16((u16)value, instr->reg); + break; + case 3: + iowrite32(value, instr->reg); + break; + default: + return -EINVAL; + } + + dev_dbg(&wdat->pdev->dev, "Wrote %#x to 0x%08llx\n", value, + gas->address); + + return 0; +} + +static int wdat_wdt_run_action(struct wdat_wdt *wdat, unsigned int action, + u32 param, u32 *retval) +{ + struct wdat_instruction *instr; + + if (action >= ARRAY_SIZE(wdat->instructions)) + return -EINVAL; + + if (!wdat->instructions[action]) + return -EOPNOTSUPP; + + dev_dbg(&wdat->pdev->dev, "Running action %#x\n", action); + + /* Run each instruction sequentially */ + list_for_each_entry(instr, wdat->instructions[action], node) { + const struct acpi_wdat_entry *entry = &instr->entry; + const struct acpi_generic_address *gas; + u32 flags, value, mask, x, y; + bool preserve; + int ret; + + gas = &entry->register_region; + + preserve = entry->instruction & ACPI_WDAT_PRESERVE_REGISTER; + flags = entry->instruction & ~ACPI_WDAT_PRESERVE_REGISTER; + value = entry->value; + mask = entry->mask; + + switch (flags) { + case ACPI_WDAT_READ_VALUE: + ret = wdat_wdt_read(wdat, instr, &x); + if (ret) + return ret; + x >>= gas->bit_offset; + x &= mask; + if (retval) + *retval = x == value; + break; + + case ACPI_WDAT_READ_COUNTDOWN: + ret = wdat_wdt_read(wdat, instr, &x); + if (ret) + return ret; + x >>= gas->bit_offset; + x &= mask; + if (retval) + *retval = x; + break; + + case ACPI_WDAT_WRITE_VALUE: + x = value & mask; + x <<= gas->bit_offset; + if (preserve) { + ret = wdat_wdt_read(wdat, instr, &y); + if (ret) + return ret; + y = y & ~(mask << gas->bit_offset); + x |= y; + } + ret = wdat_wdt_write(wdat, instr, x); + if (ret) + return ret; + break; + + case ACPI_WDAT_WRITE_COUNTDOWN: + x = param; + x &= mask; + x <<= gas->bit_offset; + if (preserve) { + ret = wdat_wdt_read(wdat, instr, &y); + if (ret) + return ret; + y = y & ~(mask << gas->bit_offset); + x |= y; + } + ret = wdat_wdt_write(wdat, instr, x); + if (ret) + return ret; + break; + + default: + dev_err(&wdat->pdev->dev, "Unknown instruction: %u\n", + flags); + return -EINVAL; + } + } + + return 0; +} + +static int wdat_wdt_enable_reboot(struct wdat_wdt *wdat) +{ + int ret; + + /* + * WDAT specification says that the watchdog is required to reboot + * the system when it fires. However, it also states that it is + * recommeded to make it configurable through hardware register. We + * enable reboot now if it is configrable, just in case. + */ + ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_REBOOT, 0, 0); + if (ret && ret != -EOPNOTSUPP) { + dev_err(&wdat->pdev->dev, + "Failed to enable reboot when watchdog triggers\n"); + return ret; + } + + return 0; +} + +static void wdat_wdt_boot_status(struct wdat_wdt *wdat) +{ + u32 boot_status = 0; + int ret; + + ret = wdat_wdt_run_action(wdat, ACPI_WDAT_GET_STATUS, 0, &boot_status); + if (ret && ret != -EOPNOTSUPP) { + dev_err(&wdat->pdev->dev, "Failed to read boot status\n"); + return; + } + + if (boot_status) + wdat->wdd.bootstatus = WDIOF_CARDRESET; + + /* Clear the boot status in case BIOS did not do it */ + ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_STATUS, 0, 0); + if (ret && ret != -EOPNOTSUPP) + dev_err(&wdat->pdev->dev, "Failed to clear boot status\n"); +} + +static void wdat_wdt_set_running(struct wdat_wdt *wdat) +{ + u32 running = 0; + int ret; + + ret = wdat_wdt_run_action(wdat, ACPI_WDAT_GET_RUNNING_STATE, 0, + &running); + if (ret && ret != -EOPNOTSUPP) + dev_err(&wdat->pdev->dev, "Failed to read running state\n"); + + if (running) + set_bit(WDOG_HW_RUNNING, &wdat->wdd.status); +} + +static int wdat_wdt_start(struct watchdog_device *wdd) +{ + return wdat_wdt_run_action(to_wdat_wdt(wdd), + ACPI_WDAT_SET_RUNNING_STATE, 0, NULL); +} + +static int wdat_wdt_stop(struct watchdog_device *wdd) +{ + return wdat_wdt_run_action(to_wdat_wdt(wdd), + ACPI_WDAT_SET_STOPPED_STATE, 0, NULL); +} + +static int wdat_wdt_ping(struct watchdog_device *wdd) +{ + return wdat_wdt_run_action(to_wdat_wdt(wdd), ACPI_WDAT_RESET, 0, NULL); +} + +static int wdat_wdt_set_timeout(struct watchdog_device *wdd, + unsigned int timeout) +{ + struct wdat_wdt *wdat = to_wdat_wdt(wdd); + unsigned int periods; + int ret; + + periods = timeout * 1000 / wdat->period; + ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_COUNTDOWN, periods, NULL); + if (!ret) + wdd->timeout = timeout; + return ret; +} + +static unsigned int wdat_wdt_get_timeleft(struct watchdog_device *wdd) +{ + struct wdat_wdt *wdat = to_wdat_wdt(wdd); + u32 periods = 0; + + wdat_wdt_run_action(wdat, ACPI_WDAT_GET_COUNTDOWN, 0, &periods); + return periods * wdat->period / 1000; +} + +static const struct watchdog_info wdat_wdt_info = { + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE, + .firmware_version = 0, + .identity = "wdat_wdt", +}; + +static const struct watchdog_ops wdat_wdt_ops = { + .owner = THIS_MODULE, + .start = wdat_wdt_start, + .stop = wdat_wdt_stop, + .ping = wdat_wdt_ping, + .set_timeout = wdat_wdt_set_timeout, + .get_timeleft = wdat_wdt_get_timeleft, +}; + +static int wdat_wdt_probe(struct platform_device *pdev) +{ + const struct acpi_wdat_entry *entries; + const struct acpi_table_wdat *tbl; + struct wdat_wdt *wdat; + struct resource *res; + void __iomem **regs; + acpi_status status; + int i, ret; + + status = acpi_get_table(ACPI_SIG_WDAT, 0, + (struct acpi_table_header **)&tbl); + if (ACPI_FAILURE(status)) + return -ENODEV; + + wdat = devm_kzalloc(&pdev->dev, sizeof(*wdat), GFP_KERNEL); + if (!wdat) + return -ENOMEM; + + regs = devm_kcalloc(&pdev->dev, pdev->num_resources, sizeof(*regs), + GFP_KERNEL); + if (!regs) + return -ENOMEM; + + /* WDAT specification wants to have >= 1ms period */ + if (tbl->timer_period < 1) + return -EINVAL; + if (tbl->min_count > tbl->max_count) + return -EINVAL; + + wdat->period = tbl->timer_period; + wdat->wdd.min_hw_heartbeat_ms = wdat->period * tbl->min_count; + wdat->wdd.max_hw_heartbeat_ms = wdat->period * tbl->max_count; + wdat->stopped_in_sleep = tbl->flags & ACPI_WDAT_STOPPED; + wdat->wdd.info = &wdat_wdt_info; + wdat->wdd.ops = &wdat_wdt_ops; + wdat->pdev = pdev; + + /* Request and map all resources */ + for (i = 0; i < pdev->num_resources; i++) { + void __iomem *reg; + + res = &pdev->resource[i]; + if (resource_type(res) == IORESOURCE_MEM) { + reg = devm_ioremap_resource(&pdev->dev, res); + } else if (resource_type(res) == IORESOURCE_IO) { + reg = devm_ioport_map(&pdev->dev, res->start, 1); + } else { + dev_err(&pdev->dev, "Unsupported resource\n"); + return -EINVAL; + } + + if (!reg) + return -ENOMEM; + + regs[i] = reg; + } + + entries = (struct acpi_wdat_entry *)(tbl + 1); + for (i = 0; i < tbl->entries; i++) { + const struct acpi_generic_address *gas; + struct wdat_instruction *instr; + struct list_head *instructions; + unsigned int action; + struct resource r; + int j; + + action = entries[i].action; + if (action >= MAX_WDAT_ACTIONS) { + dev_dbg(&pdev->dev, "Skipping unknown action: %u\n", + action); + continue; + } + + instr = devm_kzalloc(&pdev->dev, sizeof(*instr), GFP_KERNEL); + if (!instr) + return -ENOMEM; + + INIT_LIST_HEAD(&instr->node); + instr->entry = entries[i]; + + gas = &entries[i].register_region; + + memset(&r, 0, sizeof(r)); + r.start = gas->address; + r.end = r.start + gas->access_width; + if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) { + r.flags = IORESOURCE_MEM; + } else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) { + r.flags = IORESOURCE_IO; + } else { + dev_dbg(&pdev->dev, "Unsupported address space: %d\n", + gas->space_id); + continue; + } + + /* Find the matching resource */ + for (j = 0; j < pdev->num_resources; j++) { + res = &pdev->resource[j]; + if (resource_contains(res, &r)) { + instr->reg = regs[j] + r.start - res->start; + break; + } + } + + if (!instr->reg) { + dev_err(&pdev->dev, "I/O resource not found\n"); + return -EINVAL; + } + + instructions = wdat->instructions[action]; + if (!instructions) { + instructions = devm_kzalloc(&pdev->dev, + sizeof(*instructions), GFP_KERNEL); + if (!instructions) + return -ENOMEM; + + INIT_LIST_HEAD(instructions); + wdat->instructions[action] = instructions; + } + + list_add_tail(&instr->node, instructions); + } + + wdat_wdt_boot_status(wdat); + wdat_wdt_set_running(wdat); + + ret = wdat_wdt_enable_reboot(wdat); + if (ret) + return ret; + + platform_set_drvdata(pdev, wdat); + + watchdog_set_nowayout(&wdat->wdd, nowayout); + return devm_watchdog_register_device(&pdev->dev, &wdat->wdd); +} + +#ifdef CONFIG_PM_SLEEP +static int wdat_wdt_suspend_noirq(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct wdat_wdt *wdat = platform_get_drvdata(pdev); + int ret; + + if (!watchdog_active(&wdat->wdd)) + return 0; + + /* + * We need to stop the watchdog if firmare is not doing it or if we + * are going suspend to idle (where firmware is not involved). If + * firmware is stopping the watchdog we kick it here one more time + * to give it some time. + */ + wdat->stopped = false; + if (acpi_target_system_state() == ACPI_STATE_S0 || + !wdat->stopped_in_sleep) { + ret = wdat_wdt_stop(&wdat->wdd); + if (!ret) + wdat->stopped = true; + } else { + ret = wdat_wdt_ping(&wdat->wdd); + } + + return ret; +} + +static int wdat_wdt_resume_noirq(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct wdat_wdt *wdat = platform_get_drvdata(pdev); + int ret; + + if (!watchdog_active(&wdat->wdd)) + return 0; + + if (!wdat->stopped) { + /* + * Looks like the boot firmware reinitializes the watchdog + * before it hands off to the OS on resume from sleep so we + * stop and reprogram the watchdog here. + */ + ret = wdat_wdt_stop(&wdat->wdd); + if (ret) + return ret; + + ret = wdat_wdt_set_timeout(&wdat->wdd, wdat->wdd.timeout); + if (ret) + return ret; + + ret = wdat_wdt_enable_reboot(wdat); + if (ret) + return ret; + } + + return wdat_wdt_start(&wdat->wdd); +} +#endif + +static const struct dev_pm_ops wdat_wdt_pm_ops = { + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(wdat_wdt_suspend_noirq, + wdat_wdt_resume_noirq) +}; + +static struct platform_driver wdat_wdt_driver = { + .probe = wdat_wdt_probe, + .driver = { + .name = "wdat_wdt", + .pm = &wdat_wdt_pm_ops, + }, +}; + +module_platform_driver(wdat_wdt_driver); + +MODULE_AUTHOR("Mika Westerberg "); +MODULE_DESCRIPTION("ACPI Hardware Watchdog (WDAT) driver"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:wdat_wdt"); -- cgit v1.2.3 From 3413f702fae096184ad8bd9be9d7ecbc7c182bd7 Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Tue, 20 Sep 2016 15:30:52 +0300 Subject: mfd: lpc_ich: Do not create iTCO watchdog when WDAT table exists ACPI WDAT table is the preferred way to use hardware watchdog over the native iTCO_wdt. Windows only uses this table for its hardware watchdog implementation so we should be relatively safe to trust it has been validated by OEMs Prevent iTCO watchdog creation if we detect that there is ACPI WDAT table. Signed-off-by: Mika Westerberg Reviewed-by: Guenter Roeck Acked-by: Lee Jones Signed-off-by: Rafael J. Wysocki --- drivers/mfd/lpc_ich.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers') diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c index bd3aa4578346..c8dee47b45d9 100644 --- a/drivers/mfd/lpc_ich.c +++ b/drivers/mfd/lpc_ich.c @@ -984,6 +984,10 @@ static int lpc_ich_init_wdt(struct pci_dev *dev) int ret; struct resource *res; + /* If we have ACPI based watchdog use that instead */ + if (acpi_has_watchdog()) + return -ENODEV; + /* Setup power management base register */ pci_read_config_dword(dev, priv->abase, &base_addr_cfg); base_addr = base_addr_cfg & 0x0000ff80; -- cgit v1.2.3 From 1f6dbb022bd477239bfbb52ef868f694b5f1d36a Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Tue, 20 Sep 2016 15:30:53 +0300 Subject: i2c: i801: Do not create iTCO watchdog when WDAT table exists ACPI WDAT table is the preferred way to use hardware watchdog over the native iTCO_wdt. Windows only uses this table for its hardware watchdog implementation so we should be relatively safe to trust it has been validated by OEMs Prevent iTCO watchdog creation if we detect that there is ACPI WDAT table. Signed-off-by: Mika Westerberg Reviewed-by: Guenter Roeck Acked-by: Wolfram Sang Signed-off-by: Rafael J. Wysocki --- drivers/i2c/busses/i2c-i801.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 5ef9b733d153..26298af73232 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -1486,7 +1486,9 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id) priv->features |= FEATURE_IRQ; priv->features |= FEATURE_SMBUS_PEC; priv->features |= FEATURE_BLOCK_BUFFER; - priv->features |= FEATURE_TCO; + /* If we have ACPI based watchdog use that instead */ + if (!acpi_has_watchdog()) + priv->features |= FEATURE_TCO; priv->features |= FEATURE_HOST_NOTIFY; break; -- cgit v1.2.3 From bba6529ea039925ed422696be5d52677bd16643c Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Tue, 20 Sep 2016 15:30:54 +0300 Subject: platform/x86: intel_pmc_ipc: Do not create iTCO watchdog when WDAT table exists ACPI WDAT table is the preferred way to use hardware watchdog over the native iTCO_wdt. Windows only uses this table for its hardware watchdog implementation so we should be relatively safe to trust it has been validated by OEMs. Prevent iTCO watchdog creation if we detect that there is an ACPI WDAT table. Signed-off-by: Mika Westerberg Reviewed-by: Guenter Roeck Signed-off-by: Rafael J. Wysocki --- drivers/platform/x86/intel_pmc_ipc.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c index b86e1bcaa055..a511d518206b 100644 --- a/drivers/platform/x86/intel_pmc_ipc.c +++ b/drivers/platform/x86/intel_pmc_ipc.c @@ -651,11 +651,15 @@ static int ipc_create_pmc_devices(void) { int ret; - ret = ipc_create_tco_device(); - if (ret) { - dev_err(ipcdev.dev, "Failed to add tco platform device\n"); - return ret; + /* If we have ACPI based watchdog use that instead */ + if (!acpi_has_watchdog()) { + ret = ipc_create_tco_device(); + if (ret) { + dev_err(ipcdev.dev, "Failed to add tco platform device\n"); + return ret; + } } + ret = ipc_create_punit_device(); if (ret) { dev_err(ipcdev.dev, "Failed to add punit platform device\n"); -- cgit v1.2.3 From 356ed043517dbceb646a9353831abde91062a395 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Wed, 28 Sep 2016 23:15:54 +0200 Subject: watchdog: wdat_wdt: fix return value check in wdat_wdt_probe() In case of error, the function devm_ioremap_resource() returns ERR_PTR() and never returns NULL. The NULL test in the return value check should be replaced with IS_ERR(). Signed-off-by: Wei Yongjun Acked-by: Mika Westerberg Reviewed-by: Guenter Roeck Signed-off-by: Rafael J. Wysocki --- drivers/watchdog/wdat_wdt.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c index 6b6464596674..4594723c27ba 100644 --- a/drivers/watchdog/wdat_wdt.c +++ b/drivers/watchdog/wdat_wdt.c @@ -351,16 +351,17 @@ static int wdat_wdt_probe(struct platform_device *pdev) res = &pdev->resource[i]; if (resource_type(res) == IORESOURCE_MEM) { reg = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(reg)) + return PTR_ERR(reg); } else if (resource_type(res) == IORESOURCE_IO) { reg = devm_ioport_map(&pdev->dev, res->start, 1); + if (!reg) + return -ENOMEM; } else { dev_err(&pdev->dev, "Unsupported resource\n"); return -EINVAL; } - if (!reg) - return -ENOMEM; - regs[i] = reg; } -- cgit v1.2.3 From cda3b9178510297ac968b7e129e20caaaa66c581 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Wed, 28 Sep 2016 23:17:11 +0200 Subject: watchdog: wdat_wdt: Fix warning for using 0 as NULL Fixes the following sparse warnings: drivers/watchdog/wdat_wdt.c:210:66: warning: Using plain integer as NULL pointer drivers/watchdog/wdat_wdt.c:235:66: warning: Using plain integer as NULL pointer Signed-off-by: Wei Yongjun Acked-by: Mika Westerberg Reviewed-by: Guenter Roeck Signed-off-by: Rafael J. Wysocki --- drivers/watchdog/wdat_wdt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c index 4594723c27ba..e473e3b23720 100644 --- a/drivers/watchdog/wdat_wdt.c +++ b/drivers/watchdog/wdat_wdt.c @@ -207,7 +207,7 @@ static int wdat_wdt_enable_reboot(struct wdat_wdt *wdat) * recommeded to make it configurable through hardware register. We * enable reboot now if it is configrable, just in case. */ - ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_REBOOT, 0, 0); + ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_REBOOT, 0, NULL); if (ret && ret != -EOPNOTSUPP) { dev_err(&wdat->pdev->dev, "Failed to enable reboot when watchdog triggers\n"); @@ -232,7 +232,7 @@ static void wdat_wdt_boot_status(struct wdat_wdt *wdat) wdat->wdd.bootstatus = WDIOF_CARDRESET; /* Clear the boot status in case BIOS did not do it */ - ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_STATUS, 0, 0); + ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_STATUS, 0, NULL); if (ret && ret != -EOPNOTSUPP) dev_err(&wdat->pdev->dev, "Failed to clear boot status\n"); } -- cgit v1.2.3