[PATCH] ACPI / scan: Set the visited flag for all enumerated devices

[PATCH] ACPI / scan: Set the visited flag for all enumerated devices

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Commit 10c7e20b2ff3 (ACPI / scan: fix enumeration (visited) flags for
bus rescans) attempted to fix a problem with ACPI-based enumerateion
of I2C/SPI devices, but it forgot to ensure that the visited flag
will be set for all of the other enumerated devices, so fix that.

Fixes: 10c7e20b2ff3 (ACPI / scan: fix enumeration (visited) flags for bus rescans)
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1857,15 +1857,20 @@ static void acpi_bus_attach(struct acpi_
 		return;
 
 	device->flags.match_driver = true;
-	if (!ret) {
-		ret = device_attach(&device->dev);
-		if (ret < 0)
-			return;
-
-		if (!ret && device->pnp.type.platform_id)
-			acpi_default_enumeration(device);
+	if (ret > 0) {
+		acpi_device_set_enumerated(device);
+		goto ok;
 	}
 
+	ret = device_attach(&device->dev);
+	if (ret < 0)
+		return;
+
+	if (ret > 0 || !device->pnp.type.platform_id)
+		acpi_device_set_enumerated(device);
+	else
+		acpi_default_enumeration(device);
+
  ok:
 	list_for_each_entry(child, &device->children, node)
 		acpi_bus_attach(child);

Re: [PATCH] ACPI / scan: Set the visited flag for all enumerated devices

[Mika Westerberg]

On Tue, Apr 11, 2017 at 12:23:42AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Commit 10c7e20b2ff3 (ACPI / scan: fix enumeration (visited) flags for
> bus rescans) attempted to fix a problem with ACPI-based enumerateion
> of I2C/SPI devices, but it forgot to ensure that the visited flag
> will be set for all of the other enumerated devices, so fix that.
> 
> Fixes: 10c7e20b2ff3 (ACPI / scan: fix enumeration (visited) flags for bus rescans)
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH] ACPI / scan: Set the visited flag for all enumerated devices

[Rafael J. Wysocki]

On Tue, Apr 11, 2017 at 11:36 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Tue, Apr 11, 2017 at 12:23:42AM +0200, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> Commit 10c7e20b2ff3 (ACPI / scan: fix enumeration (visited) flags for
>> bus rescans) attempted to fix a problem with ACPI-based enumerateion
>> of I2C/SPI devices, but it forgot to ensure that the visited flag
>> will be set for all of the other enumerated devices, so fix that.
>>
>> Fixes: 10c7e20b2ff3 (ACPI / scan: fix enumeration (visited) flags for bus rescans)
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thanks for the review, but I'm actually unsure about one thing.

Namely, we may want to call acpi_default_enumeration(device) even if
an ACPI driver has been bound to the device.

The reason why is because this runs before driver modules are loaded
(and quite likely before ACPI drivers are registered too) and
acpi_default_enumeration(device) is going to be invoked even if there
is a matching ACPI driver, but it is registered later.  That driver
can still bind to the device object going forward, of course, so there
can be an ACPI device object with both a companion "physical" device
and an ACPI driver bound at the same time anyway.

However, if acpi_bus_scan() is called for the same device as a result
of a bus/device check notification, for example, the
acpi_default_enumeration(device) will be skipped for device objects
with ACPI drivers bound (as of the current version of the patch) which
is quite inconsistent IMO.

Thanks,
Rafael

Re: [PATCH] ACPI / scan: Set the visited flag for all enumerated devices

[Rafael J. Wysocki]

On Tue, Apr 11, 2017 at 4:15 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Tue, Apr 11, 2017 at 11:36 AM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
>> On Tue, Apr 11, 2017 at 12:23:42AM +0200, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>> Commit 10c7e20b2ff3 (ACPI / scan: fix enumeration (visited) flags for
>>> bus rescans) attempted to fix a problem with ACPI-based enumerateion
>>> of I2C/SPI devices, but it forgot to ensure that the visited flag
>>> will be set for all of the other enumerated devices, so fix that.
>>>
>>> Fixes: 10c7e20b2ff3 (ACPI / scan: fix enumeration (visited) flags for bus rescans)
>>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> Thanks for the review, but I'm actually unsure about one thing.
>
> Namely, we may want to call acpi_default_enumeration(device) even if
> an ACPI driver has been bound to the device.
>
> The reason why is because this runs before driver modules are loaded
> (and quite likely before ACPI drivers are registered too) and
> acpi_default_enumeration(device) is going to be invoked even if there
> is a matching ACPI driver, but it is registered later.  That driver
> can still bind to the device object going forward, of course, so there
> can be an ACPI device object with both a companion "physical" device
> and an ACPI driver bound at the same time anyway.
>
> However, if acpi_bus_scan() is called for the same device as a result
> of a bus/device check notification, for example, the
> acpi_default_enumeration(device) will be skipped for device objects
> with ACPI drivers bound (as of the current version of the patch) which
> is quite inconsistent IMO.

Well, the unpatched code does not call
acpi_default_enumeration(device) on device objects with ACPI drivers
bound, so I guess that may be a change on top of the current patch.

I'll go ahead with the patch as is, sorry for the noise.

Thanks,
Rafael