From patchwork Mon Dec 14 20:25:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 343597 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CD191C2BB48 for ; Mon, 14 Dec 2020 20:34:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9A082222BB for ; Mon, 14 Dec 2020 20:34:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2503394AbgLNUeo (ORCPT ); Mon, 14 Dec 2020 15:34:44 -0500 Received: from cloudserver094114.home.pl ([79.96.170.134]:58924 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2503081AbgLNUeh (ORCPT ); Mon, 14 Dec 2020 15:34:37 -0500 Received: from 89-77-60-66.dynamic.chello.pl (89.77.60.66) (HELO kreacher.localnet) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.530) id 7070c570a071b69d; Mon, 14 Dec 2020 21:33:41 +0100 From: "Rafael J. Wysocki" To: Linux ACPI Cc: Hans De Goede , LKML , "Rafael J. Wysocki" , Mika Westerberg Subject: [RFT][PATCH v1 1/3] ACPI: scan: Evaluate _DEP before adding the device Date: Mon, 14 Dec 2020 21:25:23 +0100 Message-ID: <1841307.UyYFXhZDfv@kreacher> In-Reply-To: <1646930.v2jOOB1UEN@kreacher> References: <1646930.v2jOOB1UEN@kreacher> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org From: Rafael J. Wysocki Evaluate _DEP before calling acpi_add_single_object() from acpi_bus_check_add() and do that only for ACPI_BUS_TYPE_DEVICE objects. While at it, rename acpi_device_dep_initialize() to acpi_scan_check_dep(), fix up a memory allocation statement in that function, consistently treat memory allocation failures in there as intermittent errors and make some related janitorial changes in it. This change will help to avoid calling acpi_add_single_object() if there are unmet _DEP dependencies in the future, as that may cause some control methods, potentially depending on the presence of operation regions supplied by other devices, to be evaluated prematurely. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/scan.c | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) Index: linux-pm/drivers/acpi/scan.c =================================================================== --- linux-pm.orig/drivers/acpi/scan.c +++ linux-pm/drivers/acpi/scan.c @@ -1842,32 +1842,30 @@ static void acpi_scan_init_hotplug(struc } } -static void acpi_device_dep_initialize(struct acpi_device *adev) +static u32 acpi_scan_check_dep(acpi_handle handle) { - struct acpi_dep_data *dep; struct acpi_handle_list dep_devices; acpi_status status; + u32 count; int i; - adev->dep_unmet = 0; + if (!acpi_has_method(handle, "_DEP")) + return 0; - if (!acpi_has_method(adev->handle, "_DEP")) - return; - - status = acpi_evaluate_reference(adev->handle, "_DEP", NULL, - &dep_devices); + status = acpi_evaluate_reference(handle, "_DEP", NULL, &dep_devices); if (ACPI_FAILURE(status)) { - dev_dbg(&adev->dev, "Failed to evaluate _DEP.\n"); - return; + acpi_handle_debug(handle, "Failed to evaluate _DEP.\n"); + return 0; } - for (i = 0; i < dep_devices.count; i++) { + for (count = 0, i = 0; i < dep_devices.count; i++) { struct acpi_device_info *info; - int skip; + struct acpi_dep_data *dep; + bool skip; status = acpi_get_object_info(dep_devices.handles[i], &info); if (ACPI_FAILURE(status)) { - dev_dbg(&adev->dev, "Error reading _DEP device info\n"); + acpi_handle_debug(handle, "Error reading _DEP device info\n"); continue; } @@ -1877,26 +1875,30 @@ static void acpi_device_dep_initialize(s if (skip) continue; - dep = kzalloc(sizeof(struct acpi_dep_data), GFP_KERNEL); + dep = kzalloc(sizeof(*dep), GFP_KERNEL); if (!dep) - return; + continue; + + count++; dep->supplier = dep_devices.handles[i]; - dep->consumer = adev->handle; - adev->dep_unmet++; + dep->consumer = handle; mutex_lock(&acpi_dep_list_lock); list_add_tail(&dep->node , &acpi_dep_list); mutex_unlock(&acpi_dep_list_lock); } + + return count; } static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used, void *not_used, void **return_value) { struct acpi_device *device = NULL; - int type; + u32 dep_count = 0; unsigned long long sta; + int type; int result; acpi_bus_get_device(handle, &device); @@ -1912,12 +1914,16 @@ static acpi_status acpi_bus_check_add(ac return AE_OK; } + if (type == ACPI_BUS_TYPE_DEVICE) + dep_count = acpi_scan_check_dep(handle); + acpi_add_single_object(&device, handle, type, sta); if (!device) return AE_CTRL_DEPTH; + device->dep_unmet = dep_count; + acpi_scan_init_hotplug(device); - acpi_device_dep_initialize(device); out: if (!*return_value) From patchwork Mon Dec 14 20:27:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 343596 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E835EC4361B for ; Mon, 14 Dec 2020 20:36:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id ABCFB22286 for ; Mon, 14 Dec 2020 20:36:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2502881AbgLNUf7 (ORCPT ); Mon, 14 Dec 2020 15:35:59 -0500 Received: from cloudserver094114.home.pl ([79.96.170.134]:57844 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2503355AbgLNUeh (ORCPT ); Mon, 14 Dec 2020 15:34:37 -0500 Received: from 89-77-60-66.dynamic.chello.pl (89.77.60.66) (HELO kreacher.localnet) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.530) id 4438d705b772e000; Mon, 14 Dec 2020 21:33:40 +0100 From: "Rafael J. Wysocki" To: Linux ACPI Cc: Hans De Goede , LKML , "Rafael J. Wysocki" , Mika Westerberg Subject: [RFT][PATCH v1 2/3] ACPI: scan: Defer enumeration of devices with _DEP lists Date: Mon, 14 Dec 2020 21:27:27 +0100 Message-ID: <2066680.U3vERgk28e@kreacher> In-Reply-To: <1646930.v2jOOB1UEN@kreacher> References: <1646930.v2jOOB1UEN@kreacher> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org From: Rafael J. Wysocki In some cases ACPI control methods used during device enumeration (such as _HID or _STA) may rely on Operation Region handlers supplied by the drivers of other devices [1]: An example of this is the Acer Switch 10E SW3-016 model. The _HID method of the ACPI node for the UART attached Bluetooth, reads GPIOs to detect the installed wifi chip and update the _HID for the Bluetooth's ACPI node accordingly. The current ACPI scan code calls _HID before the GPIO controller's OpRegions are available, leading to the wrong _HID being used and Bluetooth not working. In principle, in those cases there should be a _DEP control method under the device object with OpRegion enumeration dependencies, so deferring the enumeration of devices with _DEP returning a non-empty list of suppliers of OpRegions depended on by the given device (modulo some known exceptions that don't really supply any OpRegions and are listed by _DEP for other reasons irrelevant for Linux) should at least address the first-order dependencies by allowing the OpRegion suppliers to be enumerated before their consumers. Implement the above idea by modifying acpi_bus_scan() to enumerate devices in the given scope of the ACPI namespace in two passes, where the first pass covers the devices without "significant" lists of dependencies coming from _DEP only and the second pass covers all of the devices that were not enumerated in the first pass. Take _DEP into account only for device objects with _HID, mostly in order to avoid deferring the creation of ACPI device objects that represent PCI devices and must be present during the enumeration of the PCI bus (which takes place during the processing of the ACPI device object that represents the host bridge), so that they can be properly associated with the corresponding PCI devices. Link: https://lore.kernel.org/linux-acpi/20201121203040.146252-1-hdegoede@redhat.com/ # [1] Reported-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/scan.c | 103 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 78 insertions(+), 25 deletions(-) Index: linux-pm/drivers/acpi/scan.c =================================================================== --- linux-pm.orig/drivers/acpi/scan.c +++ linux-pm/drivers/acpi/scan.c @@ -1635,8 +1635,6 @@ void acpi_init_device_object(struct acpi device_initialize(&device->dev); dev_set_uevent_suppress(&device->dev, true); acpi_init_coherency(device); - /* Assume there are unmet deps until acpi_device_dep_initialize() runs */ - device->dep_unmet = 1; } void acpi_device_add_finalize(struct acpi_device *device) @@ -1849,7 +1847,13 @@ static u32 acpi_scan_check_dep(acpi_hand u32 count; int i; - if (!acpi_has_method(handle, "_DEP")) + /* + * Check for _HID here to avoid deferring the enumeration of: + * 1. PCI devices. + * 2. ACPI nodes describing USB ports. + * Still, checking for _HID catches more then just these cases ... + */ + if (!acpi_has_method(handle, "_DEP") || !acpi_has_method(handle, "_HID")) return 0; status = acpi_evaluate_reference(handle, "_DEP", NULL, &dep_devices); @@ -1892,11 +1896,24 @@ static u32 acpi_scan_check_dep(acpi_hand return count; } -static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used, - void *not_used, void **return_value) +static void acpi_scan_dep_init(struct acpi_device *adev) +{ + struct acpi_dep_data *dep; + + mutex_lock(&acpi_dep_list_lock); + + list_for_each_entry(dep, &acpi_dep_list, node) { + if (dep->consumer == adev->handle) + adev->dep_unmet++; + } + + mutex_unlock(&acpi_dep_list_lock); +} + +static acpi_status acpi_bus_check_add(acpi_handle handle, bool check_dep, + struct acpi_device **adev_p) { struct acpi_device *device = NULL; - u32 dep_count = 0; unsigned long long sta; int type; int result; @@ -1914,24 +1931,40 @@ static acpi_status acpi_bus_check_add(ac return AE_OK; } - if (type == ACPI_BUS_TYPE_DEVICE) - dep_count = acpi_scan_check_dep(handle); + if (type == ACPI_BUS_TYPE_DEVICE && check_dep) { + u32 count = acpi_scan_check_dep(handle); + /* Bail out if the number of recorded dependencies is not 0. */ + if (count > 0) + return AE_CTRL_DEPTH; + } acpi_add_single_object(&device, handle, type, sta); if (!device) return AE_CTRL_DEPTH; - device->dep_unmet = dep_count; - acpi_scan_init_hotplug(device); + if (!check_dep) + acpi_scan_dep_init(device); - out: - if (!*return_value) - *return_value = device; +out: + if (!*adev_p) + *adev_p = device; return AE_OK; } +static acpi_status acpi_bus_check_add_1(acpi_handle handle, u32 lvl_not_used, + void *not_used, void **ret_p) +{ + return acpi_bus_check_add(handle, true, (struct acpi_device **)ret_p); +} + +static acpi_status acpi_bus_check_add_2(acpi_handle handle, u32 lvl_not_used, + void *not_used, void **ret_p) +{ + return acpi_bus_check_add(handle, false, (struct acpi_device **)ret_p); +} + static void acpi_default_enumeration(struct acpi_device *device) { /* @@ -1999,12 +2032,16 @@ static int acpi_scan_attach_handler(stru return ret; } -static void acpi_bus_attach(struct acpi_device *device) +static void acpi_bus_attach(struct acpi_device *device, bool first_pass) { struct acpi_device *child; + bool skip = !first_pass && device->flags.visited; acpi_handle ejd; int ret; + if (skip) + goto ok; + if (ACPI_SUCCESS(acpi_bus_get_ejd(device->handle, &ejd))) register_dock_dependent_device(device, ejd); @@ -2051,9 +2088,9 @@ static void acpi_bus_attach(struct acpi_ ok: list_for_each_entry(child, &device->children, node) - acpi_bus_attach(child); + acpi_bus_attach(child, first_pass); - if (device->handler && device->handler->hotplug.notify_online) + if (!skip && device->handler && device->handler->hotplug.notify_online) device->handler->hotplug.notify_online(device); } @@ -2071,7 +2108,8 @@ void acpi_walk_dep_device_list(acpi_hand adev->dep_unmet--; if (!adev->dep_unmet) - acpi_bus_attach(adev); + acpi_bus_attach(adev, true); + list_del(&dep->node); kfree(dep); } @@ -2096,17 +2134,32 @@ EXPORT_SYMBOL_GPL(acpi_walk_dep_device_l */ int acpi_bus_scan(acpi_handle handle) { - void *device = NULL; + struct acpi_device *device = NULL; - if (ACPI_SUCCESS(acpi_bus_check_add(handle, 0, NULL, &device))) + /* Pass 1: Avoid enumerating devices with missing dependencies. */ + + if (ACPI_SUCCESS(acpi_bus_check_add(handle, true, &device))) acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, - acpi_bus_check_add, NULL, NULL, &device); + acpi_bus_check_add_1, NULL, NULL, + (void **)&device); - if (device) { - acpi_bus_attach(device); - return 0; - } - return -ENODEV; + if (!device) + return -ENODEV; + + acpi_bus_attach(device, true); + + /* Pass 2: Enumerate all of the remaining devices. */ + + device = NULL; + + if (ACPI_SUCCESS(acpi_bus_check_add(handle, false, &device))) + acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, + acpi_bus_check_add_2, NULL, NULL, + (void **)&device); + + acpi_bus_attach(device, false); + + return 0; } EXPORT_SYMBOL(acpi_bus_scan); From patchwork Mon Dec 14 20:32:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 344051 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 60041C4361B for ; Mon, 14 Dec 2020 20:37:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 26A85222BB for ; Mon, 14 Dec 2020 20:37:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2503371AbgLNUef (ORCPT ); Mon, 14 Dec 2020 15:34:35 -0500 Received: from cloudserver094114.home.pl ([79.96.170.134]:61792 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2503353AbgLNUeb (ORCPT ); Mon, 14 Dec 2020 15:34:31 -0500 Received: from 89-77-60-66.dynamic.chello.pl (89.77.60.66) (HELO kreacher.localnet) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.530) id 9c0cdc76c89b6df9; Mon, 14 Dec 2020 21:33:39 +0100 From: "Rafael J. Wysocki" To: Linux ACPI Cc: Hans De Goede , LKML , "Rafael J. Wysocki" , Mika Westerberg Subject: [RFT][PATCH v1 3/3] ACPI: scan: Avoid unnecessary second pass in acpi_bus_scan() Date: Mon, 14 Dec 2020 21:32:44 +0100 Message-ID: <1954514.xqII67ylRR@kreacher> In-Reply-To: <1646930.v2jOOB1UEN@kreacher> References: <1646930.v2jOOB1UEN@kreacher> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org From: Rafael J. Wysocki If there are no devices whose enumeration has been deferred after the first pass in acpi_bus_scan(), the second pass is not necssary, so avoid it with the help of a new static variable. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/scan.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) Index: linux-pm/drivers/acpi/scan.c =================================================================== --- linux-pm.orig/drivers/acpi/scan.c +++ linux-pm/drivers/acpi/scan.c @@ -1910,6 +1910,8 @@ static void acpi_scan_dep_init(struct ac mutex_unlock(&acpi_dep_list_lock); } +static bool acpi_bus_scan_second_pass; + static acpi_status acpi_bus_check_add(acpi_handle handle, bool check_dep, struct acpi_device **adev_p) { @@ -1934,8 +1936,10 @@ static acpi_status acpi_bus_check_add(ac if (type == ACPI_BUS_TYPE_DEVICE && check_dep) { u32 count = acpi_scan_check_dep(handle); /* Bail out if the number of recorded dependencies is not 0. */ - if (count > 0) + if (count > 0) { + acpi_bus_scan_second_pass = true; return AE_CTRL_DEPTH; + } } acpi_add_single_object(&device, handle, type, sta); @@ -2136,6 +2140,8 @@ int acpi_bus_scan(acpi_handle handle) { struct acpi_device *device = NULL; + acpi_bus_scan_second_pass = false; + /* Pass 1: Avoid enumerating devices with missing dependencies. */ if (ACPI_SUCCESS(acpi_bus_check_add(handle, true, &device))) @@ -2148,6 +2154,9 @@ int acpi_bus_scan(acpi_handle handle) acpi_bus_attach(device, true); + if (!acpi_bus_scan_second_pass) + return 0; + /* Pass 2: Enumerate all of the remaining devices. */ device = NULL;