ACPI: scan: Drop power resource references for deferred devices

[PATCH] ACPI: scan: Drop power resource references for deferred devices

Posted by Peixin Xie 1 month, 2 weeks ago

acpi_bus_get_power_flags() initializes the device power state and takes
references to any power resources required by that state.

If the device is not ready for enumeration, acpi_bus_attach() parks it
and clears power_manageable without dropping those references. When the
dependency is later satisfied, power initialization takes the references
again. The additional references prevent the resources from being
turned off when the device enters D3.

Transition the device to D3cold before clearing power_manageable to drop
the references acquired during initial power state initialization.

This is particularly easy to trigger on RISC-V, where dependencies on
interrupt controllers are derived automatically from interrupt resources.

Signed-off-by: Peixin Xie <peixin.xie@linux.spacemit.com>
---
This issue was reproduced on a SpacemiT K3 RISC-V platform. The affected
device uses a power resource through _PR0 and has an automatically derived
dependency on its interrupt controller.

Before the change, the initial power initialization acquires a power
resource reference. The device is then deferred, but that reference is not
dropped. When the dependency becomes available, power initialization
acquires another reference. Consequently, entering D3 only drops the
reference count from 2 to 1 and _OFF is not evaluated:

  [    0.403217] ACPI: \_SB_.USBD.PDPR: ACPI: PM: Power resource is on
  [    0.409393] ACPI: \_SB_.USBD.PDPR: New power resource
  [    0.419165] ACPI Debug:  "ACPI USBD: entering PDPR._ON"
  [    0.424495] ACPI: \_SB_.USBD.PDPR: ACPI: PM: Power resource turned on
  [    0.431036] ACPI Debug:  "ACPI USBD: entering _PS0"
  [    0.790826] ACPI: \_SB_.USBD.PDPR: ACPI: PM: Power resource already on
  [    0.801777] ACPI Debug:  "ACPI USBD: entering _PS0"
  [    0.995285] ACPI Debug:  "ACPI USBD: entering _PS3"
  [    1.005550] ACPI: \_SB_.USBD.PDPR: ACPI: PM: Power resource still in use

After the change, the reference acquired before deferred enumeration is
dropped, and _OFF is evaluated both when the device is deferred and when
it later enters D3:

  [    0.340111] ACPI Debug:  "ACPI USBD: entering PDPR._ON"
  [    0.345459] ACPI Debug:  "ACPI USBD: entering _PS0"
  [    0.350532] ACPI Debug:  "ACPI USBD: entering PDPR._OFF"
  [    0.742054] ACPI Debug:  "ACPI USBD: entering PDPR._ON"
  [    0.747282] ACPI Debug:  "ACPI USBD: entering _PS0"
  [    0.758664] ACPI Debug:  "ACPI USBD: entering _PS3"
  [    0.768959] ACPI Debug:  "ACPI USBD: entering PDPR._OFF"
---
 drivers/acpi/scan.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 9a7ac2eb9ce0..7a8404719fa1 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2350,6 +2350,11 @@ static int acpi_bus_attach(struct acpi_device *device, void *first_pass)
 	acpi_bus_get_status(device);
 	/* Skip devices that are not ready for enumeration (e.g. not present) */
 	if (!acpi_dev_ready_for_enumeration(device)) {
+		/*
+		 * Drop power resource references acquired during initial
+		 * power state initialization before parking the device.
+		 */
+		acpi_power_transition(device, ACPI_STATE_D3_COLD);
 		device->flags.initialized = false;
 		acpi_device_clear_enumerated(device);
 		device->flags.power_manageable = 0;

---
base-commit: d58772d8520c7ef247c4b95c9bd76d3a25da9ff5
change-id: 20260811-acpi-power-resource-ref-fix-2fa4798f590b

Best regards,
--  
Peixin Xie <peixin.xie@linux.spacemit.com>

Re: [PATCH] ACPI: scan: Drop power resource references for deferred devices

Posted by Rafael J. Wysocki (Intel) 1 month, 2 weeks ago

On Tue, Aug 11, 2026 at 2:37 PM Peixin Xie
<peixin.xie@linux.spacemit.com> wrote:
>
> acpi_bus_get_power_flags() initializes the device power state and takes
> references to any power resources required by that state.
>
> If the device is not ready for enumeration, acpi_bus_attach() parks it
> and clears power_manageable without dropping those references. When the
> dependency is later satisfied, power initialization takes the references
> again. The additional references prevent the resources from being
> turned off when the device enters D3.
>
> Transition the device to D3cold before clearing power_manageable to drop
> the references acquired during initial power state initialization.
>
> This is particularly easy to trigger on RISC-V, where dependencies on
> interrupt controllers are derived automatically from interrupt resources.
>
> Signed-off-by: Peixin Xie <peixin.xie@linux.spacemit.com>
> ---
> This issue was reproduced on a SpacemiT K3 RISC-V platform. The affected
> device uses a power resource through _PR0 and has an automatically derived
> dependency on its interrupt controller.
>
> Before the change, the initial power initialization acquires a power
> resource reference. The device is then deferred, but that reference is not
> dropped. When the dependency becomes available, power initialization
> acquires another reference. Consequently, entering D3 only drops the
> reference count from 2 to 1 and _OFF is not evaluated:
>
>   [    0.403217] ACPI: \_SB_.USBD.PDPR: ACPI: PM: Power resource is on
>   [    0.409393] ACPI: \_SB_.USBD.PDPR: New power resource
>   [    0.419165] ACPI Debug:  "ACPI USBD: entering PDPR._ON"
>   [    0.424495] ACPI: \_SB_.USBD.PDPR: ACPI: PM: Power resource turned on
>   [    0.431036] ACPI Debug:  "ACPI USBD: entering _PS0"
>   [    0.790826] ACPI: \_SB_.USBD.PDPR: ACPI: PM: Power resource already on
>   [    0.801777] ACPI Debug:  "ACPI USBD: entering _PS0"
>   [    0.995285] ACPI Debug:  "ACPI USBD: entering _PS3"
>   [    1.005550] ACPI: \_SB_.USBD.PDPR: ACPI: PM: Power resource still in use
>
> After the change, the reference acquired before deferred enumeration is
> dropped, and _OFF is evaluated both when the device is deferred and when
> it later enters D3:
>
>   [    0.340111] ACPI Debug:  "ACPI USBD: entering PDPR._ON"
>   [    0.345459] ACPI Debug:  "ACPI USBD: entering _PS0"
>   [    0.350532] ACPI Debug:  "ACPI USBD: entering PDPR._OFF"
>   [    0.742054] ACPI Debug:  "ACPI USBD: entering PDPR._ON"
>   [    0.747282] ACPI Debug:  "ACPI USBD: entering _PS0"
>   [    0.758664] ACPI Debug:  "ACPI USBD: entering _PS3"
>   [    0.768959] ACPI Debug:  "ACPI USBD: entering PDPR._OFF"
> ---
>  drivers/acpi/scan.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 9a7ac2eb9ce0..7a8404719fa1 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2350,6 +2350,11 @@ static int acpi_bus_attach(struct acpi_device *device, void *first_pass)
>         acpi_bus_get_status(device);
>         /* Skip devices that are not ready for enumeration (e.g. not present) */
>         if (!acpi_dev_ready_for_enumeration(device)) {
> +               /*
> +                * Drop power resource references acquired during initial
> +                * power state initialization before parking the device.
> +                */
> +               acpi_power_transition(device, ACPI_STATE_D3_COLD);
>                 device->flags.initialized = false;
>                 acpi_device_clear_enumerated(device);
>                 device->flags.power_manageable = 0;
>
> ---

Sashiko has concerns regarding this change:

https://sashiko.dev/#/patchset/20260811-acpi-power-resource-ref-fix-v1-1-828f1c383831%40linux.spacemit.com

Moreover, it is kind of pointless to power up a device and then power
it down almost immediately later.

Have you considered dropping the acpi_bus_init_power() call from
acpi_bus_get_power_flags() and doing it only once when the device gets
ready for enumeration?

Since there should not be any drivers binding directly to struct
acpi_device in the tree now (and struc acpi_device is going to be
dropped entirely in 7.3 if all goes well), something like that should
work.

Re: [PATCH] ACPI: scan: Drop power resource references for deferred devices

Posted by Peixin Xie 1 month, 1 week ago

On Wed, Aug 12, 2026 at 03:15:27PM +0200, Rafael J. Wysocki wrote:
> Have you considered dropping the acpi_bus_init_power() call from
> acpi_bus_get_power_flags() and doing it only once when the device gets
> ready for enumeration?

Thank you for the suggestion. I tested this approach on the SpacemiT K3
platform and it works as expected.

In addition to removing the acpi_bus_init_power() call, I set
device->flags.initialized to false at the end of acpi_bus_get_power_flags()
to mark the device power state as not yet initialized. This causes the
existing path in acpi_bus_attach() to call acpi_bus_init_power() once the
device is ready for enumeration.

The power resource references are now balanced and runtime suspend and
resume continue to work correctly. I will send a v2 in next couple days.

Re: [PATCH] ACPI: scan: Drop power resource references for deferred devices

Posted by Rafael J. Wysocki (Intel) 1 month, 1 week ago

On Tue, Aug 18, 2026 at 5:41 AM Peixin Xie
<peixin.xie@linux.spacemit.com> wrote:
>
> On Wed, Aug 12, 2026 at 03:15:27PM +0200, Rafael J. Wysocki wrote:
> > Have you considered dropping the acpi_bus_init_power() call from
> > acpi_bus_get_power_flags() and doing it only once when the device gets
> > ready for enumeration?
>
> Thank you for the suggestion. I tested this approach on the SpacemiT K3
> platform and it works as expected.
>
> In addition to removing the acpi_bus_init_power() call, I set
> device->flags.initialized to false at the end of acpi_bus_get_power_flags()
> to mark the device power state as not yet initialized.

Instead of doing this, IMV it would be better to remove the setting of
device->flags.initialized to 1 in acpi_init_device_object() and set it
to 1 in acpi_add_power_resource().

> This causes the existing path in acpi_bus_attach() to call acpi_bus_init_power() once the
> device is ready for enumeration.
>
> The power resource references are now balanced and runtime suspend and
> resume continue to work correctly. I will send a v2 in next couple days.

Please do, but please take the comment above into consideration.

Thanks!