From 6d27975851b134be8d2a170437210c9719e524aa Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 10 May 2021 19:53:18 +0200 Subject: ACPI: scan: Rearrange dep_unmet initialization The dep_unmet field in struct acpi_device is used to store the number of unresolved _DEP dependencies (that is, operation region dependencies for which there are no drivers present) for the ACPI device object represented by it. That field is initialized to 1 for all ACPI device objects in acpi_add_single_object(), via acpi_init_device_object(), so as to avoid evaluating _STA prematurely for battery device objects in acpi_scan_init_status(), and it is "fixed up" in acpi_bus_check_add() after the acpi_add_single_object() called by it has returned. This is not particularly straightforward and causes dep_unmet to remain 1 for device objects without dependencies created by invoking acpi_add_single_object() directly, outside acpi_bus_check_add(). For this reason, rearrange acpi_add_single_object() to initialize dep_unmet completely before calling acpi_scan_init_status(), which requires passing one extra bool argument to it, and update all of its callers accordingly. Signed-off-by: Rafael J. Wysocki Reviewed-by: Hans de Goede --- drivers/acpi/scan.c | 60 +++++++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 32 deletions(-) (limited to 'drivers/acpi/scan.c') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 453eff8ec8c3..a2df7bcf4d07 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1671,8 +1671,20 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle, device_initialize(&device->dev); dev_set_uevent_suppress(&device->dev, true); acpi_init_coherency(device); - /* Assume there are unmet deps to start with. */ - device->dep_unmet = 1; +} + +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); } void acpi_device_add_finalize(struct acpi_device *device) @@ -1688,7 +1700,7 @@ static void acpi_scan_init_status(struct acpi_device *adev) } static int acpi_add_single_object(struct acpi_device **child, - acpi_handle handle, int type) + acpi_handle handle, int type, bool dep_init) { struct acpi_device *device; int result; @@ -1703,8 +1715,12 @@ static int acpi_add_single_object(struct acpi_device **child, * acpi_bus_get_status() and use its quirk handling. Note that * this must be done before the get power-/wakeup_dev-flags calls. */ - if (type == ACPI_BUS_TYPE_DEVICE || type == ACPI_BUS_TYPE_PROCESSOR) + if (type == ACPI_BUS_TYPE_DEVICE || type == ACPI_BUS_TYPE_PROCESSOR) { + if (dep_init) + acpi_scan_dep_init(device); + acpi_scan_init_status(device); + } acpi_bus_get_power_flags(device); acpi_bus_get_wakeup_device_flags(device); @@ -1886,22 +1902,6 @@ static u32 acpi_scan_check_dep(acpi_handle handle, bool check_dep) return count; } -static void acpi_scan_dep_init(struct acpi_device *adev) -{ - struct acpi_dep_data *dep; - - adev->dep_unmet = 0; - - 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 bool acpi_bus_scan_second_pass; static acpi_status acpi_bus_check_add(acpi_handle handle, bool check_dep, @@ -1949,19 +1949,15 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, bool check_dep, return AE_OK; } - acpi_add_single_object(&device, handle, type); - if (!device) - return AE_CTRL_DEPTH; - - acpi_scan_init_hotplug(device); /* * If check_dep is true at this point, the device has no dependencies, * or the creation of the device object would have been postponed above. */ - if (check_dep) - device->dep_unmet = 0; - else - acpi_scan_dep_init(device); + acpi_add_single_object(&device, handle, type, !check_dep); + if (!device) + return AE_CTRL_DEPTH; + + acpi_scan_init_hotplug(device); out: if (!*adev_p) @@ -2223,7 +2219,7 @@ int acpi_bus_register_early_device(int type) struct acpi_device *device = NULL; int result; - result = acpi_add_single_object(&device, NULL, type); + result = acpi_add_single_object(&device, NULL, type, false); if (result) return result; @@ -2243,7 +2239,7 @@ static int acpi_bus_scan_fixed(void) struct acpi_device *device = NULL; result = acpi_add_single_object(&device, NULL, - ACPI_BUS_TYPE_POWER_BUTTON); + ACPI_BUS_TYPE_POWER_BUTTON, false); if (result) return result; @@ -2259,7 +2255,7 @@ static int acpi_bus_scan_fixed(void) struct acpi_device *device = NULL; result = acpi_add_single_object(&device, NULL, - ACPI_BUS_TYPE_SLEEP_BUTTON); + ACPI_BUS_TYPE_SLEEP_BUTTON, false); if (result) return result; -- cgit v1.2.3 From a9e10e58730432e5de840eb3ddd55c75f29341b3 Mon Sep 17 00:00:00 2001 From: Daniel Scally Date: Thu, 3 Jun 2021 23:40:02 +0100 Subject: ACPI: scan: Extend acpi_walk_dep_device_list() The acpi_walk_dep_device_list() function is not as generic as its name implies, serving only to decrement the dependency count for each dependent device of the input. Extend it to accept a callback which can be applied to all the dependencies in acpi_dep_list. Replace all existing calls to the function with calls to a wrapper, passing a callback that applies the same dependency reduction. Reviewed-by: Andy Shevchenko Acked-by: Maximilian Luz # for platform/surface parts Signed-off-by: Daniel Scally [ rjw: Changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 2 +- drivers/acpi/pmic/intel_pmic_chtdc_ti.c | 2 +- drivers/acpi/scan.c | 69 +++++++++++++++++++------- drivers/gpio/gpiolib-acpi.c | 10 ++-- drivers/i2c/i2c-core-acpi.c | 8 +-- drivers/platform/surface/aggregator/core.c | 6 +-- drivers/platform/surface/surface3_power.c | 22 ++++---- drivers/platform/surface/surface_acpi_notify.c | 7 +-- include/acpi/acpi_bus.h | 7 +++ include/linux/acpi.h | 4 +- 10 files changed, 90 insertions(+), 47 deletions(-) (limited to 'drivers/acpi/scan.c') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 13565629ce0a..3f7680a007a3 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1627,7 +1627,7 @@ static int acpi_ec_add(struct acpi_device *device) WARN(!ret, "Could not request EC cmd io port 0x%lx", ec->command_addr); /* Reprobe devices depending on the EC */ - acpi_walk_dep_device_list(ec->handle); + acpi_dev_clear_dependencies(device); acpi_handle_debug(ec->handle, "enumerated.\n"); return 0; diff --git a/drivers/acpi/pmic/intel_pmic_chtdc_ti.c b/drivers/acpi/pmic/intel_pmic_chtdc_ti.c index a5101b07611a..fef7831d0d63 100644 --- a/drivers/acpi/pmic/intel_pmic_chtdc_ti.c +++ b/drivers/acpi/pmic/intel_pmic_chtdc_ti.c @@ -117,7 +117,7 @@ static int chtdc_ti_pmic_opregion_probe(struct platform_device *pdev) return err; /* Re-enumerate devices depending on PMIC */ - acpi_walk_dep_device_list(ACPI_HANDLE(pdev->dev.parent)); + acpi_dev_clear_dependencies(ACPI_COMPANION(pdev->dev.parent)); return 0; } diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index a2df7bcf4d07..2277add6da2f 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -47,12 +47,6 @@ static DEFINE_MUTEX(acpi_hp_context_lock); */ static u64 spcr_uart_addr; -struct acpi_dep_data { - struct list_head node; - acpi_handle supplier; - acpi_handle consumer; -}; - void acpi_scan_lock_acquire(void) { mutex_lock(&acpi_scan_lock); @@ -2107,30 +2101,69 @@ static void acpi_bus_attach(struct acpi_device *device, bool first_pass) device->handler->hotplug.notify_online(device); } -void acpi_walk_dep_device_list(acpi_handle handle) +static int acpi_scan_clear_dep(struct acpi_dep_data *dep, void *data) { - struct acpi_dep_data *dep, *tmp; struct acpi_device *adev; + acpi_bus_get_device(dep->consumer, &adev); + + if (adev) { + adev->dep_unmet--; + if (!adev->dep_unmet) + acpi_bus_attach(adev, true); + } + + list_del(&dep->node); + kfree(dep); + + return 0; +} + +/** + * acpi_walk_dep_device_list - Apply a callback to every entry in acpi_dep_list + * @handle: The ACPI handle of the supplier device + * @callback: Pointer to the callback function to apply + * @data: Pointer to some data to pass to the callback + * + * The return value of the callback determines this function's behaviour. If 0 + * is returned we continue to iterate over acpi_dep_list. If a positive value + * is returned then the loop is broken but this function returns 0. If a + * negative value is returned by the callback then the loop is broken and that + * value is returned as the final error. + */ +int acpi_walk_dep_device_list(acpi_handle handle, + int (*callback)(struct acpi_dep_data *, void *), + void *data) +{ + struct acpi_dep_data *dep, *tmp; + int ret; + mutex_lock(&acpi_dep_list_lock); list_for_each_entry_safe(dep, tmp, &acpi_dep_list, node) { if (dep->supplier == handle) { - acpi_bus_get_device(dep->consumer, &adev); - - if (adev) { - adev->dep_unmet--; - if (!adev->dep_unmet) - acpi_bus_attach(adev, true); - } - - list_del(&dep->node); - kfree(dep); + ret = callback(dep, data); + if (ret) + break; } } mutex_unlock(&acpi_dep_list_lock); + + return ret > 0 ? 0 : ret; } EXPORT_SYMBOL_GPL(acpi_walk_dep_device_list); +/** + * acpi_dev_clear_dependencies - Inform consumers that the device is now active + * @supplier: Pointer to the supplier &struct acpi_device + * + * Clear dependencies on the given device. + */ +void acpi_dev_clear_dependencies(struct acpi_device *supplier) +{ + acpi_walk_dep_device_list(supplier->handle, acpi_scan_clear_dep, NULL); +} +EXPORT_SYMBOL_GPL(acpi_dev_clear_dependencies); + /** * acpi_bus_scan - Add ACPI device node objects in a given namespace scope. * @handle: Root of the namespace scope to scan. diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index 3ef22a3c104d..5b4111e4be3f 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -1233,14 +1233,14 @@ static void acpi_gpiochip_scan_gpios(struct acpi_gpio_chip *achip) void acpi_gpiochip_add(struct gpio_chip *chip) { struct acpi_gpio_chip *acpi_gpio; - acpi_handle handle; + struct acpi_device *adev; acpi_status status; if (!chip || !chip->parent) return; - handle = ACPI_HANDLE(chip->parent); - if (!handle) + adev = ACPI_COMPANION(chip->parent); + if (!adev) return; acpi_gpio = kzalloc(sizeof(*acpi_gpio), GFP_KERNEL); @@ -1254,7 +1254,7 @@ void acpi_gpiochip_add(struct gpio_chip *chip) INIT_LIST_HEAD(&acpi_gpio->events); INIT_LIST_HEAD(&acpi_gpio->deferred_req_irqs_list_entry); - status = acpi_attach_data(handle, acpi_gpio_chip_dh, acpi_gpio); + status = acpi_attach_data(adev->handle, acpi_gpio_chip_dh, acpi_gpio); if (ACPI_FAILURE(status)) { dev_err(chip->parent, "Failed to attach ACPI GPIO chip\n"); kfree(acpi_gpio); @@ -1263,7 +1263,7 @@ void acpi_gpiochip_add(struct gpio_chip *chip) acpi_gpiochip_request_regions(acpi_gpio); acpi_gpiochip_scan_gpios(acpi_gpio); - acpi_walk_dep_device_list(handle); + acpi_dev_clear_dependencies(adev); } void acpi_gpiochip_remove(struct gpio_chip *chip) diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c index 8ceaa88dd78f..6f0aa0ed3241 100644 --- a/drivers/i2c/i2c-core-acpi.c +++ b/drivers/i2c/i2c-core-acpi.c @@ -259,8 +259,8 @@ static acpi_status i2c_acpi_add_device(acpi_handle handle, u32 level, */ void i2c_acpi_register_devices(struct i2c_adapter *adap) { + struct acpi_device *adev; acpi_status status; - acpi_handle handle; if (!has_acpi_companion(&adap->dev)) return; @@ -275,11 +275,11 @@ void i2c_acpi_register_devices(struct i2c_adapter *adap) if (!adap->dev.parent) return; - handle = ACPI_HANDLE(adap->dev.parent); - if (!handle) + adev = ACPI_COMPANION(adap->dev.parent); + if (!adev) return; - acpi_walk_dep_device_list(handle); + acpi_dev_clear_dependencies(adev); } static const struct acpi_device_id i2c_acpi_force_400khz_device_ids[] = { diff --git a/drivers/platform/surface/aggregator/core.c b/drivers/platform/surface/aggregator/core.c index 8dc2c267bcd6..517f774a6e60 100644 --- a/drivers/platform/surface/aggregator/core.c +++ b/drivers/platform/surface/aggregator/core.c @@ -621,8 +621,8 @@ static const struct acpi_gpio_mapping ssam_acpi_gpios[] = { static int ssam_serial_hub_probe(struct serdev_device *serdev) { + struct acpi_device *ssh = ACPI_COMPANION(&serdev->dev); struct ssam_controller *ctrl; - acpi_handle *ssh = ACPI_HANDLE(&serdev->dev); acpi_status astatus; int status; @@ -652,7 +652,7 @@ static int ssam_serial_hub_probe(struct serdev_device *serdev) if (status) goto err_devopen; - astatus = ssam_serdev_setup_via_acpi(ssh, serdev); + astatus = ssam_serdev_setup_via_acpi(ssh->handle, serdev); if (ACPI_FAILURE(astatus)) { status = -ENXIO; goto err_devinit; @@ -706,7 +706,7 @@ static int ssam_serial_hub_probe(struct serdev_device *serdev) * For now let's thus default power/wakeup to false. */ device_set_wakeup_capable(&serdev->dev, true); - acpi_walk_dep_device_list(ssh); + acpi_dev_clear_dependencies(ssh); return 0; diff --git a/drivers/platform/surface/surface3_power.c b/drivers/platform/surface/surface3_power.c index cc4f9cba6856..dea82aa1abd4 100644 --- a/drivers/platform/surface/surface3_power.c +++ b/drivers/platform/surface/surface3_power.c @@ -446,12 +446,12 @@ mshw0011_space_handler(u32 function, acpi_physical_address command, static int mshw0011_install_space_handler(struct i2c_client *client) { - acpi_handle handle; + struct acpi_device *adev; struct mshw0011_handler_data *data; acpi_status status; - handle = ACPI_HANDLE(&client->dev); - if (!handle) + adev = ACPI_COMPANION(&client->dev); + if (!adev) return -ENODEV; data = kzalloc(sizeof(struct mshw0011_handler_data), @@ -460,25 +460,25 @@ static int mshw0011_install_space_handler(struct i2c_client *client) return -ENOMEM; data->client = client; - status = acpi_bus_attach_private_data(handle, (void *)data); + status = acpi_bus_attach_private_data(adev->handle, (void *)data); if (ACPI_FAILURE(status)) { kfree(data); return -ENOMEM; } - status = acpi_install_address_space_handler(handle, - ACPI_ADR_SPACE_GSBUS, - &mshw0011_space_handler, - NULL, - data); + status = acpi_install_address_space_handler(adev->handle, + ACPI_ADR_SPACE_GSBUS, + &mshw0011_space_handler, + NULL, + data); if (ACPI_FAILURE(status)) { dev_err(&client->dev, "Error installing i2c space handler\n"); - acpi_bus_detach_private_data(handle); + acpi_bus_detach_private_data(adev->handle); kfree(data); return -ENOMEM; } - acpi_walk_dep_device_list(handle); + acpi_dev_clear_dependencies(adev); return 0; } diff --git a/drivers/platform/surface/surface_acpi_notify.c b/drivers/platform/surface/surface_acpi_notify.c index ef9c1f8e8336..8339988d95c1 100644 --- a/drivers/platform/surface/surface_acpi_notify.c +++ b/drivers/platform/surface/surface_acpi_notify.c @@ -798,7 +798,7 @@ static int san_consumer_links_setup(struct platform_device *pdev) static int san_probe(struct platform_device *pdev) { - acpi_handle san = ACPI_HANDLE(&pdev->dev); + struct acpi_device *san = ACPI_COMPANION(&pdev->dev); struct ssam_controller *ctrl; struct san_data *data; acpi_status astatus; @@ -821,7 +821,8 @@ static int san_probe(struct platform_device *pdev) platform_set_drvdata(pdev, data); - astatus = acpi_install_address_space_handler(san, ACPI_ADR_SPACE_GSBUS, + astatus = acpi_install_address_space_handler(san->handle, + ACPI_ADR_SPACE_GSBUS, &san_opreg_handler, NULL, &data->info); if (ACPI_FAILURE(astatus)) @@ -835,7 +836,7 @@ static int san_probe(struct platform_device *pdev) if (status) goto err_install_dev; - acpi_walk_dep_device_list(san); + acpi_dev_clear_dependencies(san); return 0; err_install_dev: diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 3a82faac5767..0b2c4f170f4d 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -280,6 +280,12 @@ struct acpi_device_power { struct acpi_device_power_state states[ACPI_D_STATE_COUNT]; /* Power states (D0-D3Cold) */ }; +struct acpi_dep_data { + struct list_head node; + acpi_handle supplier; + acpi_handle consumer; +}; + /* Performance Management */ struct acpi_device_perf_flags { @@ -685,6 +691,7 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev) bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2); +void acpi_dev_clear_dependencies(struct acpi_device *supplier); struct acpi_device * acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv); struct acpi_device * diff --git a/include/linux/acpi.h b/include/linux/acpi.h index c60745f657e9..170b9bebdb2b 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -666,7 +666,9 @@ extern bool acpi_driver_match_device(struct device *dev, const struct device_driver *drv); int acpi_device_uevent_modalias(struct device *, struct kobj_uevent_env *); int acpi_device_modalias(struct device *, char *, int); -void acpi_walk_dep_device_list(acpi_handle handle); +int acpi_walk_dep_device_list(acpi_handle handle, + int (*callback)(struct acpi_dep_data *, void *), + void *data); struct platform_device *acpi_create_platform_device(struct acpi_device *, struct property_entry *); -- cgit v1.2.3 From b83e2b306736cb0d108df791fd4ee39f6d52184f Mon Sep 17 00:00:00 2001 From: Daniel Scally Date: Thu, 3 Jun 2021 23:40:03 +0100 Subject: ACPI: scan: Add function to fetch dependent of ACPI device In some ACPI tables we encounter, devices use the _DEP method to assert a dependence on other ACPI devices as opposed to the OpRegions that the specification intends. We need to be able to find those devices "from" the dependee, so add a callback and a wrapper to walk over the acpi_dep_list and return the dependent ACPI device. Reviewed-by: Andy Shevchenko Signed-off-by: Daniel Scally Signed-off-by: Rafael J. Wysocki --- drivers/acpi/scan.c | 35 +++++++++++++++++++++++++++++++++++ include/acpi/acpi_bus.h | 1 + 2 files changed, 36 insertions(+) (limited to 'drivers/acpi/scan.c') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 2277add6da2f..708c1d84e7bf 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2101,6 +2101,20 @@ static void acpi_bus_attach(struct acpi_device *device, bool first_pass) device->handler->hotplug.notify_online(device); } +static int acpi_dev_get_first_consumer_dev_cb(struct acpi_dep_data *dep, void *data) +{ + struct acpi_device *adev; + + adev = acpi_bus_get_acpi_device(dep->consumer); + if (!adev) + /* If we don't find an adev then we want to continue parsing */ + return 0; + + *(struct acpi_device **)data = adev; + + return 1; +} + static int acpi_scan_clear_dep(struct acpi_dep_data *dep, void *data) { struct acpi_device *adev; @@ -2164,6 +2178,27 @@ void acpi_dev_clear_dependencies(struct acpi_device *supplier) } EXPORT_SYMBOL_GPL(acpi_dev_clear_dependencies); +/** + * acpi_dev_get_first_consumer_dev - Return ACPI device dependent on @supplier + * @supplier: Pointer to the dependee device + * + * Returns the first &struct acpi_device which declares itself dependent on + * @supplier via the _DEP buffer, parsed from the acpi_dep_list. + * + * The caller is responsible for putting the reference to adev when it is no + * longer needed. + */ +struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier) +{ + struct acpi_device *adev = NULL; + + acpi_walk_dep_device_list(supplier->handle, + acpi_dev_get_first_consumer_dev_cb, &adev); + + return adev; +} +EXPORT_SYMBOL_GPL(acpi_dev_get_first_consumer_dev); + /** * acpi_bus_scan - Add ACPI device node objects in a given namespace scope. * @handle: Root of the namespace scope to scan. diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 0b2c4f170f4d..4bed30e61c5b 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -692,6 +692,7 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev) bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2); void acpi_dev_clear_dependencies(struct acpi_device *supplier); +struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier); struct acpi_device * acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv); struct acpi_device * -- cgit v1.2.3 From 23db673d7e5194c8fbbb8c307e23960767305c09 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Wed, 9 Jun 2021 18:33:12 +0100 Subject: ACPI: scan: initialize local variable to avoid garbage being returned In the unlikely event that there are no callback calls made in acpi_walk_dep_device_list(), local variable ret will be returned as an uninitialized value. Clean up static analysis warnings by ensuring ret is initialized. Addresses-Coverity: ("Uninitialized scalar variable") Fixes: a9e10e587304 ("ACPI: scan: Extend acpi_walk_dep_device_list()") Signed-off-by: Colin Ian King Reviewed-by: Daniel Scally [ rjw: Subject and changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/scan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/acpi/scan.c') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 708c1d84e7bf..f64119a17382 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2150,7 +2150,7 @@ int acpi_walk_dep_device_list(acpi_handle handle, void *data) { struct acpi_dep_data *dep, *tmp; - int ret; + int ret = 0; mutex_lock(&acpi_dep_list_lock); list_for_each_entry_safe(dep, tmp, &acpi_dep_list, node) { -- cgit v1.2.3 From 2d0795148a5a7dd33999daf600eb6fdeffabd6ba Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 16 Jun 2021 16:06:52 +0200 Subject: ACPI: scan: Define acpi_bus_put_acpi_device() as static inline Since acpi_bus_put_acpi_device() is a synonym for acpi_dev_put(), define it as static inline in analogy with the latter. No functional impact. Signed-off-by: Rafael J. Wysocki Reviewed-by: Andy Shevchenko --- drivers/acpi/scan.c | 5 ----- include/acpi/acpi_bus.h | 9 +++++++-- 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'drivers/acpi/scan.c') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index f64119a17382..75b6bbc717b0 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -606,11 +606,6 @@ struct acpi_device *acpi_bus_get_acpi_device(acpi_handle handle) return handle_to_device(handle, get_acpi_device); } -void acpi_bus_put_acpi_device(struct acpi_device *adev) -{ - acpi_dev_put(adev); -} - static struct acpi_device_bus_id *acpi_device_bus_id_match(const char *dev_id) { struct acpi_device_bus_id *acpi_device_bus_id; diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 4bed30e61c5b..d8e7235b4cf0 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -504,8 +504,6 @@ extern int unregister_acpi_notifier(struct notifier_block *); */ int acpi_bus_get_device(acpi_handle handle, struct acpi_device **device); -struct acpi_device *acpi_bus_get_acpi_device(acpi_handle handle); -void acpi_bus_put_acpi_device(struct acpi_device *adev); acpi_status acpi_bus_get_status_handle(acpi_handle handle, unsigned long long *sta); int acpi_bus_get_status(struct acpi_device *device); @@ -726,6 +724,13 @@ static inline void acpi_dev_put(struct acpi_device *adev) { put_device(&adev->dev); } + +struct acpi_device *acpi_bus_get_acpi_device(acpi_handle handle); + +static inline void acpi_bus_put_acpi_device(struct acpi_device *adev) +{ + acpi_dev_put(adev); +} #else /* CONFIG_ACPI */ static inline int register_acpi_bus_type(void *bus) { return 0; } -- cgit v1.2.3 From ad4d451e14e58792e9b7c8a4bfc3276f0128e94a Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 16 Jun 2021 16:21:51 +0200 Subject: ACPI: scan: Rearrange acpi_dev_get_first_consumer_dev_cb() Make acpi_dev_get_first_consumer_dev_cb() a bit more straightforward and rewrite the comment in it. No functional impact. Signed-off-by: Rafael J. Wysocki Reviewed-by: Hans de Goede --- drivers/acpi/scan.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'drivers/acpi/scan.c') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 75b6bbc717b0..524d85dc540c 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2101,13 +2101,12 @@ static int acpi_dev_get_first_consumer_dev_cb(struct acpi_dep_data *dep, void *d struct acpi_device *adev; adev = acpi_bus_get_acpi_device(dep->consumer); - if (!adev) - /* If we don't find an adev then we want to continue parsing */ - return 0; - - *(struct acpi_device **)data = adev; - - return 1; + if (adev) { + *(struct acpi_device **)data = adev; + return 1; + } + /* Continue parsing if the device object is not present. */ + return 0; } static int acpi_scan_clear_dep(struct acpi_dep_data *dep, void *data) -- cgit v1.2.3 From aff0dbd03d3b750e2331f7cb93e01fe25ed27086 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 16 Jun 2021 16:22:50 +0200 Subject: ACPI: scan: Make acpi_walk_dep_device_list() Because acpi_walk_dep_device_list() is only called by the code in the file in which it is defined, make it static, drop the export of it and drop its header from acpi.h. Signed-off-by: Rafael J. Wysocki Reviewed-by: Hans de Goede --- drivers/acpi/scan.c | 7 +++---- include/linux/acpi.h | 3 --- 2 files changed, 3 insertions(+), 7 deletions(-) (limited to 'drivers/acpi/scan.c') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 524d85dc540c..b7f9b7ac0d04 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2139,9 +2139,9 @@ static int acpi_scan_clear_dep(struct acpi_dep_data *dep, void *data) * negative value is returned by the callback then the loop is broken and that * value is returned as the final error. */ -int acpi_walk_dep_device_list(acpi_handle handle, - int (*callback)(struct acpi_dep_data *, void *), - void *data) +static int acpi_walk_dep_device_list(acpi_handle handle, + int (*callback)(struct acpi_dep_data *, void *), + void *data) { struct acpi_dep_data *dep, *tmp; int ret = 0; @@ -2158,7 +2158,6 @@ int acpi_walk_dep_device_list(acpi_handle handle, return ret > 0 ? 0 : ret; } -EXPORT_SYMBOL_GPL(acpi_walk_dep_device_list); /** * acpi_dev_clear_dependencies - Inform consumers that the device is now active diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 170b9bebdb2b..0a6d2845fcaf 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -666,9 +666,6 @@ extern bool acpi_driver_match_device(struct device *dev, const struct device_driver *drv); int acpi_device_uevent_modalias(struct device *, struct kobj_uevent_env *); int acpi_device_modalias(struct device *, char *, int); -int acpi_walk_dep_device_list(acpi_handle handle, - int (*callback)(struct acpi_dep_data *, void *), - void *data); struct platform_device *acpi_create_platform_device(struct acpi_device *, struct property_entry *); -- cgit v1.2.3 From dc612486c91983a113adefedac030575ea7a4c4a Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 16 Jun 2021 16:23:44 +0200 Subject: ACPI: scan: Fix device object rescan in acpi_scan_clear_dep() In general, acpi_bus_attach() can only be run safely under acpi_scan_lock, but that lock cannot be acquired under acpi_dep_list_lock, so make acpi_scan_clear_dep() schedule deferred execution of acpi_bus_attach() under acpi_scan_lock instead of calling it directly. This also fixes a possible race between acpi_scan_clear_dep() and device removal that might cause a device object that went away to be accessed, because acpi_scan_clear_dep() is changed to acquire a reference on the consumer device object. Signed-off-by: Rafael J. Wysocki Reviewed-by: Hans de Goede --- drivers/acpi/scan.c | 50 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 5 deletions(-) (limited to 'drivers/acpi/scan.c') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index b7f9b7ac0d04..b0bdd9b90e44 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2109,16 +2109,56 @@ static int acpi_dev_get_first_consumer_dev_cb(struct acpi_dep_data *dep, void *d return 0; } -static int acpi_scan_clear_dep(struct acpi_dep_data *dep, void *data) -{ +struct acpi_scan_clear_dep_work { + struct work_struct work; struct acpi_device *adev; +}; + +static void acpi_scan_clear_dep_fn(struct work_struct *work) +{ + struct acpi_scan_clear_dep_work *cdw; + + cdw = container_of(work, struct acpi_scan_clear_dep_work, work); + + acpi_scan_lock_acquire(); + acpi_bus_attach(cdw->adev, true); + acpi_scan_lock_release(); + + acpi_dev_put(cdw->adev); + kfree(cdw); +} + +static bool acpi_scan_clear_dep_queue(struct acpi_device *adev) +{ + struct acpi_scan_clear_dep_work *cdw; - acpi_bus_get_device(dep->consumer, &adev); + if (adev->dep_unmet) + return false; + + cdw = kmalloc(sizeof(*cdw), GFP_KERNEL); + if (!cdw) + return false; + + cdw->adev = adev; + INIT_WORK(&cdw->work, acpi_scan_clear_dep_fn); + /* + * Since the work function may block on the lock until the entire + * initial enumeration of devices is complete, put it into the unbound + * workqueue. + */ + queue_work(system_unbound_wq, &cdw->work); + + return true; +} + +static int acpi_scan_clear_dep(struct acpi_dep_data *dep, void *data) +{ + struct acpi_device *adev = acpi_bus_get_acpi_device(dep->consumer); if (adev) { adev->dep_unmet--; - if (!adev->dep_unmet) - acpi_bus_attach(adev, true); + if (!acpi_scan_clear_dep_queue(adev)) + acpi_dev_put(adev); } list_del(&dep->node); -- cgit v1.2.3 From c6a493a1b603ed216ce69d1faac3f0ddc6a2f8eb Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 16 Jun 2021 16:24:30 +0200 Subject: ACPI: scan: Reorganize acpi_device_add() Move the invocation of acpi_attach_data() in acpi_device_add() into a separate function. No intentional functional impact. Signed-off-by: Rafael J. Wysocki Reviewed-by: Hans de Goede --- drivers/acpi/scan.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) (limited to 'drivers/acpi/scan.c') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index b0bdd9b90e44..c62ce287fdb9 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -633,23 +633,32 @@ static int acpi_device_set_name(struct acpi_device *device, return 0; } +static int acpi_tie_acpi_dev(struct acpi_device *adev) +{ + acpi_handle handle = adev->handle; + acpi_status status; + + if (!handle) + return 0; + + status = acpi_attach_data(handle, acpi_scan_drop_device, adev); + if (ACPI_FAILURE(status)) { + acpi_handle_err(handle, "Unable to attach device data\n"); + return -ENODEV; + } + + return 0; +} + int acpi_device_add(struct acpi_device *device, void (*release)(struct device *)) { struct acpi_device_bus_id *acpi_device_bus_id; int result; - if (device->handle) { - acpi_status status; - - status = acpi_attach_data(device->handle, acpi_scan_drop_device, - device); - if (ACPI_FAILURE(status)) { - acpi_handle_err(device->handle, - "Unable to attach device data\n"); - return -ENODEV; - } - } + result = acpi_tie_acpi_dev(device); + if (result) + return result; /* * Linkage -- cgit v1.2.3 From 5f4ce26078fde9cd406c008ba35e31bbb26a23a1 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 17 Jun 2021 15:57:07 +0200 Subject: ACPI: scan: Fix race related to dropping dependencies If acpi_add_single_object() runs concurrently with respect to acpi_scan_clear_dep() which deletes a dependencies list entry where the device being added is the consumer, the device's dep_unmet counter may not be updated to reflect that change. Namely, if the dependencies list entry is deleted right after calling acpi_scan_dep_init() and before calling acpi_device_add(), acpi_scan_clear_dep() will not find the device object corresponding to the consumer device ACPI handle and it will not update its dep_unmet counter to reflect the deletion of the list entry. Consequently, the dep_unmet counter of the device will never become zero going forward which may prevent it from being completely enumerated. To address this problem, modify acpi_add_single_object() to run acpi_tie_acpi_dev(), to attach the ACPI device object created by it to the corresponding ACPI namespace node, under acpi_dep_list_lock along with acpi_scan_dep_init() whenever the latter is called. Signed-off-by: Rafael J. Wysocki Reviewed-by: Hans de Goede --- drivers/acpi/scan.c | 45 ++++++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 13 deletions(-) (limited to 'drivers/acpi/scan.c') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index c62ce287fdb9..1c6205661000 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -650,16 +650,12 @@ static int acpi_tie_acpi_dev(struct acpi_device *adev) return 0; } -int acpi_device_add(struct acpi_device *device, - void (*release)(struct device *)) +static int __acpi_device_add(struct acpi_device *device, + void (*release)(struct device *)) { struct acpi_device_bus_id *acpi_device_bus_id; int result; - result = acpi_tie_acpi_dev(device); - if (result) - return result; - /* * Linkage * ------- @@ -748,6 +744,17 @@ err_unlock: return result; } +int acpi_device_add(struct acpi_device *adev, void (*release)(struct device *)) +{ + int ret; + + ret = acpi_tie_acpi_dev(adev); + if (ret) + return ret; + + return __acpi_device_add(adev, release); +} + /* -------------------------------------------------------------------------- Device Enumeration -------------------------------------------------------------------------- */ @@ -1675,14 +1682,10 @@ 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); } void acpi_device_add_finalize(struct acpi_device *device) @@ -1701,6 +1704,7 @@ static int acpi_add_single_object(struct acpi_device **child, acpi_handle handle, int type, bool dep_init) { struct acpi_device *device; + bool release_dep_lock = false; int result; device = kzalloc(sizeof(struct acpi_device), GFP_KERNEL); @@ -1714,16 +1718,31 @@ static int acpi_add_single_object(struct acpi_device **child, * this must be done before the get power-/wakeup_dev-flags calls. */ if (type == ACPI_BUS_TYPE_DEVICE || type == ACPI_BUS_TYPE_PROCESSOR) { - if (dep_init) + if (dep_init) { + mutex_lock(&acpi_dep_list_lock); + /* + * Hold the lock until the acpi_tie_acpi_dev() call + * below to prevent concurrent acpi_scan_clear_dep() + * from deleting a dependency list entry without + * updating dep_unmet for the device. + */ + release_dep_lock = true; acpi_scan_dep_init(device); - + } acpi_scan_init_status(device); } acpi_bus_get_power_flags(device); acpi_bus_get_wakeup_device_flags(device); - result = acpi_device_add(device, acpi_device_release); + result = acpi_tie_acpi_dev(device); + + if (release_dep_lock) + mutex_unlock(&acpi_dep_list_lock); + + if (!result) + result = __acpi_device_add(device, acpi_device_release); + if (result) { acpi_device_release(&device->dev); return result; -- cgit v1.2.3 From 8d287e8292ea126d55beb29f2b3f07dfad5b6bc0 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 16 Jun 2021 16:05:50 +0200 Subject: ACPI: scan: Simplify acpi_table_events_fn() Notice that the table field of struct acpi_table_events_work is never read and its event field is always equal to ACPI_TABLE_EVENT_LOAD, so both of them are redundant. Accordingly, drop struct acpi_table_events_work and use struct work_struct directly instead of it, simplify acpi_scan_table_handler() and rename it to acpi_scan_table_notify(). Moreover, make acpi_bus_table_handler() check the event code against ACPI_TABLE_EVENT_LOAD before calling acpi_scan_table_notify(), so it is not necessary to do that check in the latter. No intentional functional impact. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/bus.c | 3 ++- drivers/acpi/internal.h | 2 +- drivers/acpi/scan.c | 38 ++++++++++---------------------------- 3 files changed, 13 insertions(+), 30 deletions(-) (limited to 'drivers/acpi/scan.c') diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index be7da23fad76..a3a56c00bd08 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -1206,7 +1206,8 @@ void __init acpi_subsystem_init(void) static acpi_status acpi_bus_table_handler(u32 event, void *table, void *context) { - acpi_scan_table_handler(event, table, context); + if (event == ACPI_TABLE_EVENT_LOAD) + acpi_scan_table_notify(); return acpi_sysfs_table_handler(event, table, context); } diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index f973bbe90e5e..47947d612b43 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -88,7 +88,7 @@ void acpi_device_hotplug(struct acpi_device *adev, u32 src); bool acpi_scan_is_offline(struct acpi_device *adev, bool uevent); acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context); -void acpi_scan_table_handler(u32 event, void *table, void *context); +void acpi_scan_table_notify(void); /* -------------------------------------------------------------------------- Device Node Initialization / Removal diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 1c6205661000..f8eaddb83484 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2533,46 +2533,28 @@ int __init __acpi_probe_device_table(struct acpi_probe_entry *ap_head, int nr) return count; } -struct acpi_table_events_work { - struct work_struct work; - void *table; - u32 event; -}; - static void acpi_table_events_fn(struct work_struct *work) { - struct acpi_table_events_work *tew; - - tew = container_of(work, struct acpi_table_events_work, work); - - if (tew->event == ACPI_TABLE_EVENT_LOAD) { - acpi_scan_lock_acquire(); - acpi_bus_scan(ACPI_ROOT_OBJECT); - acpi_scan_lock_release(); - } + acpi_scan_lock_acquire(); + acpi_bus_scan(ACPI_ROOT_OBJECT); + acpi_scan_lock_release(); - kfree(tew); + kfree(work); } -void acpi_scan_table_handler(u32 event, void *table, void *context) +void acpi_scan_table_notify(void) { - struct acpi_table_events_work *tew; + struct work_struct *work; if (!acpi_scan_initialized) return; - if (event != ACPI_TABLE_EVENT_LOAD) + work = kmalloc(sizeof(*work), GFP_KERNEL); + if (!work) return; - tew = kmalloc(sizeof(*tew), GFP_KERNEL); - if (!tew) - return; - - INIT_WORK(&tew->work, acpi_table_events_fn); - tew->table = table; - tew->event = event; - - schedule_work(&tew->work); + INIT_WORK(work, acpi_table_events_fn); + schedule_work(work); } int acpi_reconfig_notifier_register(struct notifier_block *nb) -- cgit v1.2.3