| Message ID | 6207934.MhkbZ0Pkbq@rafael.j.wysocki |
|---|---|
| State | New |
| Headers | show |
| Series | ACPI: scan: Adjust power management initialization and PCI devices handling | expand |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Bypassing `acpi_bus_get_status()` for PCI companions leaves `device->status` stale, which can cause `acpi_device_is_present()` to incorrectly return false. -- commit 7f00077fea1f2ada305a470b1a050f31934232c4 Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com> ACPI: scan: Take PCI device enumeration into account directly This commit skips evaluating ACPI device status during hierarchy walks for devices already associated with a PCI companion, assuming they are present and functional because the PCI devices are already enumerated. > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index f4718b0207e0e..34f7f477a25ad 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c [ ... ] > @@ -2353,10 +2355,26 @@ static int attach_subtree(struct acpi_device *device, void *not_used) > if (ACPI_SUCCESS(acpi_bus_get_ejd(device->handle, &ejd))) > register_dock_dependent_device(device, ejd); > > - acpi_bus_get_status(device); > - /* Skip devices that are not ready for enumeration (e.g. not present) */ > - if (!acpi_dev_ready_for_enumeration(device)) > - return 0; > + /* > + * If the given ACPI device object has been already associated with a > + * PCI device found on the bus, its status is effectively "present > + * and functional". > + */ > + pci = acpi_dev_get_pci_dev(device); > + if (pci) { > + acpi_handle_debug(device->handle, "PCI companion %s found\n", > + pci_name(pci)); > + > + pci_dev_put(pci); > + } else { [Severity: High] Bypassing acpi_bus_get_status() for PCI companions leaves device->status stale. Could this cause acpi_device_is_present() to incorrectly return false? If the initial evaluation of the device status returned 0 (e.g., due to unmet dependencies at boot or firmware quirks), device->status.present will remain 0 permanently because it is not explicitly updated here to reflect the "present and functional" assumption. This stale status might prevent ACPI power management features, such as acpi_device_fix_up_power(), and other subsystems relying on acpi_device_is_present() from properly operating on the device. > + acpi_bus_get_status(device); > + /* > + * Skip devices that are not ready for enumeration (e.g. not > + * present). > + */ > + if (!acpi_dev_ready_for_enumeration(device)) > + return 0; > + } > > acpi_ec_register_opregions(device);
On Wed, Sep 02, 2026 at 09:36:54PM +0200, Rafael J. Wysocki wrote: > The ACPI companions of PCI devices are associated with the corresponding > PCI devices before being processed by acpi_scan_attach() and by the time > they are passed to attach_subtree(), the PCI devices associated with > them have been already enumerated and initialized. > > Accordingly, it is not necessary or even useful to check their status in > attach_subtree(), so do not do that. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index f4718b0207e0..34f7f477a25a 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -20,6 +20,7 @@ #include <linux/kthread.h> #include <linux/dmi.h> #include <linux/dma-map-ops.h> +#include <linux/pci.h> #include <linux/platform_data/x86/apple.h> #include <linux/pgtable.h> #include <linux/crc32.h> @@ -2342,6 +2343,7 @@ static int acpi_scan_attach_handler(struct acpi_device *device) static int attach_subtree(struct acpi_device *device, void *not_used) { + struct pci_dev *pci; acpi_handle ejd; bool skip; int ret; @@ -2353,10 +2355,26 @@ static int attach_subtree(struct acpi_device *device, void *not_used) if (ACPI_SUCCESS(acpi_bus_get_ejd(device->handle, &ejd))) register_dock_dependent_device(device, ejd); - acpi_bus_get_status(device); - /* Skip devices that are not ready for enumeration (e.g. not present) */ - if (!acpi_dev_ready_for_enumeration(device)) - return 0; + /* + * If the given ACPI device object has been already associated with a + * PCI device found on the bus, its status is effectively "present + * and functional". + */ + pci = acpi_dev_get_pci_dev(device); + if (pci) { + acpi_handle_debug(device->handle, "PCI companion %s found\n", + pci_name(pci)); + + pci_dev_put(pci); + } else { + acpi_bus_get_status(device); + /* + * Skip devices that are not ready for enumeration (e.g. not + * present). + */ + if (!acpi_dev_ready_for_enumeration(device)) + return 0; + } acpi_ec_register_opregions(device);