Re: [PATCH v1 1/7] ACPI: scan: Stop calling acpi_bus_init_power() early

On Mon, Aug 31, 2026 at 06:24:45PM +0200, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <[email protected]>
> 
> There is a problem, introduced by commit 9d9bcae47fd5 ("ACPI: delay
> enumeration of devices with a _DEP pointing to an INT3472 device")
> inadvertently, that causes devices with missing dependencies to be
> put into power state D0 prematurely on some systems [1].
> 
> Namely, acpi_bus_init_power() called by acpi_bus_get_power_flags()
> during the early initialization of ACPI device objects, may discover
> that all of the power resources needed by the given device to be in
> power state D0 are initially on, so it will reference count those
> power resources and transition the device into D0.  Later, if
> acpi_bus_attach() running for that device notices that it has missing
> dependencies, the enumeration of it will be deferred and its
> power_manageable flag will be cleared, even though it is still in D0
> at that point.
> 
> After the dependencies in question have been met, acpi_bus_attach()
> will run again for the device and now it will call
> acpi_bus_init_power() that will take additional references to the
> power resources used by the device in D0.  These additional
> references prevent the power resources from being turned off when
> the device goes into D3hot/D3cold.
> 
> To address this, stop calling acpi_bus_init_power() from
> acpi_bus_get_power_flags(), but also take the initialization of
> PCI devices into account, which needs to be done because they
> are initialized and bound to their ACPI companions before
> calling acpi_bus_attach() for the latter.
> 
> To that end, notice that each PCI device discovered on the bus
> is put into power state D0 via pci_power_up() which involves
> invoking acpi_device_set_power().  The initial ACPI power state of
> the device needs to be known at that point to carry out the power
> transition of it properly, so modify acpi_device_set_power() to
> call acpi_bus_init_power() upfront if the device's ACPI power
> state is still unknown.
> 
> Also use the ACPI power state tracking to decide whether or not
> acpi_bus_init_power() needs to be called by acpi_bus_attach()
> instead of using the "initialized" flag of the ACPI device object
> for this purpose, which is fragile and inconvenient, and stop
> clearing the power_manageable flag for devices with unmet
> dependencies, which is not necessary.
> 
> While at it, add a debug message pringing statement to
> acpi_bus_init_power() to facilitate diagnostics.

...

> +	if (device->power.state == ACPI_STATE_UNKNOWN &&
> +	    acpi_bus_init_power(device)) {
> +		device->flags.power_manageable = 0;
> +		return -ENODEV;
> +	}

In this form it might be harder to catch the side effect of the conditional.
I would split it into two:


	if (device->power.state == ACPI_STATE_UNKNOWN) {
		int result;

		result = acpi_bus_init_power(device);
		if (result) {
			device->flags.power_manageable = 0;
			return result; // shouldn't we instead of return -ENODEV?
		}
	}

...

> +	if (device->flags.power_manageable &&
> +	    device->power.state == ACPI_STATE_UNKNOWN &&
> +	    acpi_bus_init_power(device))
> +		device->flags.power_manageable = 0;

In the similar way. And it might be even worth to have a helper to deduplicate
this check and setting?

static inline int ...()
{
	int result;

	if (device->power.state != ACPI_STATE_UNKNOWN)
		return 0;

	result = acpi_bus_init_power(device);
	if (result)
		device->flags.power_manageable = 0;

	return result;
}

-- 
With Best Regards,
Andy Shevchenko