Hi,
This series addresses some enumeration ordering issues by using information
from _DEP to defer the enumeration of devices that are likely to depend on
operation region (OpRegion) handlers supplied by the drivers of other
devices.
This allows the OpRegion suppliers to be probed and start working before the
devices depending on them are enumerated.
Please see the patch changelogs for details.
Hans, please test this series on the system with OpRegion dependencies in
control methods used for device enumeration.
Thanks!
From: Rafael J. Wysocki <[email protected]>
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 <[email protected]>
---
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)
Hi,
On 12/14/20 9:23 PM, Rafael J. Wysocki wrote:
> Hi,
>
> This series addresses some enumeration ordering issues by using information
> from _DEP to defer the enumeration of devices that are likely to depend on
> operation region (OpRegion) handlers supplied by the drivers of other
> devices.
>
> This allows the OpRegion suppliers to be probed and start working before the
> devices depending on them are enumerated.
>
> Please see the patch changelogs for details.
>
> Hans, please test this series on the system with OpRegion dependencies in
> control methods used for device enumeration.
Thank you for you work on this!
I started with reviewing the series so that I would know what to
expect during testing. All 3 patches look good to me:
Reviewed-by: Hans de Goede <[email protected]>
As is the series does not fix the Bluetooth HID misreporting on
the system with OpRegion dependencies in its BT ACPI node's _HID,
but ...
This is expected because that _HID has a _DEP on the GPO1 GPIO
controller, which in turn has a _DEP pointing to the
"Intel Baytrail Mailbox Device" ("INT33BD")
We (Linux) don't do anything with the INT33BD device, so it can
simply be added to the acpi_ignore_dep_ids list. With this done the
3 patches from this RFT fix the BT _HID issue. IOW everything works
as expected / as we want.
While at it I also booted the kernel with these patches on a
Lenovo Yoga X1 Carbon 8th gen. I'm not seeing any adverse side
effects there, so please add my tested-by to the entire series:
Tested-by: Hans de Goede <[email protected]>
I will send out a patch adding "INT33BD" to the
acpi_ignore_dep_ids list (to be applied on top of this series)
right away.
Regards,
Hans
Hi Rafael,
On Mon, Dec 14, 2020 at 09:23:47PM +0100, Rafael J. Wysocki wrote:
> Hi,
>
> This series addresses some enumeration ordering issues by using information
> from _DEP to defer the enumeration of devices that are likely to depend on
> operation region (OpRegion) handlers supplied by the drivers of other
> devices.
>
> This allows the OpRegion suppliers to be probed and start working before the
> devices depending on them are enumerated.
For the whole series,
Reviewed-by: Mika Westerberg <[email protected]>