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@xxxxxxxxxx> wrote:
> On Tue, Apr 11, 2017 at 11:36 AM, Mika Westerberg
> <mika.westerberg@xxxxxxxxxxxxxxx> wrote:
>> On Tue, Apr 11, 2017 at 12:23:42AM +0200, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>>>
>>> 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@xxxxxxxxx>
>>
>> Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
>
> 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