Re: [PATCH v3 2/4] ACPI: scan: Stop calling acpi_bus_init_power() early

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Non-power-manageable devices are incorrectly left in `ACPI_STATE_D0` instead of inheriting their parent's power state, breaking the non-recursive power state hierarchy.
--

commit 1b9c3462d28f35e1a6f47e065a29b14b3971db3a
Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: ACPI: scan: Stop calling acpi_bus_init_power() early

This commit stops calling acpi_bus_init_power() from
acpi_bus_get_power_flags() to prevent devices with missing dependencies
from being put into D0 prematurely. It introduces a separate invocation
during PCI device initialization and uses ACPI power state tracking to
decide when to initialize a device's power state.

Link: https://lore.kernel.org/linux-acpi/20260820-acpi-power-resource-ref-fix-v2-1-29818173ea13@linux.spacemit.com/

> diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
> index 4269735aadde5..76104f715efa1 100644
> --- a/drivers/acpi/device_pm.c
> +++ b/drivers/acpi/device_pm.c
[ ... ]
> @@ -340,9 +346,36 @@ int acpi_bus_init_power(struct acpi_device *device)
>  		state = ACPI_STATE_D0;
>  	}
>  	device->power.state = state;
> +
> +	acpi_handle_debug(device->handle, "Initial power state: %s\n",
> +			  acpi_power_state_string(state));
> +
>  	return 0;
>  }
>  
> +int acpi_bus_init_power(struct acpi_device *device)
> +{
> +	int result;
> +
> +	if (device->power.state != ACPI_STATE_UNKNOWN)
> +		return 0;

[Severity: High]
Does this check prevent non-PM devices from ever executing
acpi_device_init_power()?

In acpi_add_single_object(), struct acpi_device is zero-initialized, which
defaults device->power.state to 0 (ACPI_STATE_D0). If a non-PM device skips
the ACPI_STATE_UNKNOWN assignment in acpi_bus_get_power_flags(), this early
return will silently freeze its state at D0 instead of letting it inherit
its parent's power state.

> +
> +	/*
> +	 * The ACPI device power state can be only initialized once.  If this
> +	 * fails, ACPI power management will not be used for the device going
> +	 * forward.
> +	 */
> +	result = acpi_device_init_power(device);
[ ... ]
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index f48715ed827c7..f219a16e018a4 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
[ ... ]
> @@ -1145,8 +1146,7 @@ static void acpi_bus_get_power_flags(struct acpi_device *device)
>  			device->power.states[ACPI_STATE_D3_COLD].flags.valid = 1;
>  	}
>  
> -	if (acpi_bus_init_power(device))
> -		device->flags.power_manageable = 0;
> +	device->power.state = ACPI_STATE_UNKNOWN;

[Severity: High]
Will non-power-manageable devices be incorrectly left in ACPI_STATE_D0
because of this new assignment's placement?

Earlier in acpi_bus_get_power_flags(), the code returns early for non-PM
devices:

	/* Presence of _PS0|_PR0 indicates 'power manageable' */
	if (!acpi_has_method(device->handle, "_PS0") &&
	    !acpi_has_method(device->handle, "_PR0"))
		return;

Because this early return happens before the assignment to
ACPI_STATE_UNKNOWN, non-PM devices bypass it and retain their
zero-initialized D0 state.

Can this break the non-recursive ACPI power state hierarchy check?

If intermediate non-PM devices are left in D0, a PM-capable child device
might be incorrectly allowed to power up to D0 even if its grandparent is
physically powered off (D3_COLD). This could lead to bus errors or system
crashes when the child attempts to access powered-off hardware.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/6044499.DvuYhMxLoT@rafael.j.wysocki?part=2