| Message ID | 1749017.AgSQtRUy3j@aspire.rjw.lan (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
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> -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);