Re: [PATCH v2] ACPI: scan: Defer device power initialization
"Rafael J. Wysocki (Intel)" <[email protected]>
| Newsgroups | org.infradead.lists.linux-riscv,dev.linux.lists.spacemit,org.kernel.vger.linux-acpi,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAJZ5v0hxNALP_GRBk+QpfrpSAwJxbhjcvdYWkh8aakTppZnHSQ@mail.gmail.com> |
On Thu, Aug 20, 2026 at 7:56 AM Peixin Xie <[email protected]> wrote: > > acpi_bus_get_power_flags() initializes the device power state while the > ACPI device object is being created, before checking whether the device > is ready for enumeration. > > If enumeration is deferred, acpi_bus_attach() clears the initialized and > power_manageable flags. When the dependency is later satisfied, > acpi_bus_init_power() is called again and takes additional references to > the power resources used by the device. These references prevent the > resources from being turned off when the device enters D3. > > Remove the early acpi_bus_init_power() call and leave regular ACPI device > objects uninitialized until acpi_bus_attach() runs after the device is > ready for enumeration. > > Power resource objects are initialized through acpi_add_power_resource() > and do not require the generic initialization in acpi_bus_attach(), so > mark them as initialized there. > > This also avoids powering up devices before their dependencies are > available. > > Signed-off-by: Peixin Xie <[email protected]> > --- > This issue was reproduced on a SpacemiT K3 RISC-V Pico-ITX. 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.314611] ACPI Debug: "I2P2 _STA" > [ 0.318260] ACPI: \_SB_.I2P2: ACPI: PM: Power resource is on > [ 0.323998] ACPI: \_SB_.I2P2: New power resource > [ 0.382108] ACPI Debug: "I2P2 _STA" > [ 0.478964] ACPI Debug: "I2P2 _ON" > [ 0.482498] ACPI: \_SB_.I2P2: ACPI: PM: Power resource turned on > [ 0.488597] ACPI Debug: "I2C2, PS0" > [ 0.863170] ACPI: \_SB_.I2P2: ACPI: PM: Power resource already on > [ 0.873686] ACPI Debug: "I2C2, PS0" > [ 2.416055] ACPI Debug: "I2C2, PS3" > [ 2.423397] ACPI: \_SB_.I2P2: ACPI: PM: Power resource still in use > > After the change, device power state initialization is deferred until its > dependency is met. Since no reference is acquired before then, the power > resource left on by firmware is turned off as unused after the namespace > scan. Once the dependency is met, the resource is turned on once for the > device and is turned off normally when the device later enters D3: > > [ 0.314628] ACPI Debug: "I2P2 _STA" > [ 0.318277] ACPI: \_SB_.I2P2: ACPI: PM: Power resource is on > [ 0.324016] ACPI: \_SB_.I2P2: New power resource > [ 0.382118] ACPI Debug: "I2P2 _STA" > [ 0.496116] ACPI: \_SB_.I2P2: ACPI: PM: Turning OFF > [ 0.501081] ACPI Debug: "I2P2 _OFF" > [ 0.504705] ACPI: \_SB_.I2P2: ACPI: PM: Power resource turned off > [ 1.415899] ACPI Debug: "I2P2 _ON" > [ 1.418866] ACPI: \_SB_.I2P2: ACPI: PM: Power resource turned on > [ 1.424947] ACPI Debug: "I2C2, PS0" > [ 2.647655] ACPI Debug: "I2C2, PS3" > [ 2.654856] ACPI Debug: "I2P2 _OFF" > [ 2.654866] ACPI: \_SB_.I2P2: ACPI: PM: Power resource turned off > --- > Changes in v2: > - Replace the D3cold transition added in v1 with deferred device power > initialization, as suggested by Rafael. > - Mark power resource objects as initialized in their dedicated creation > path. > - Link to v1: https://patch.msgid.link/20260811-acpi-power-resource-ref-fix-v1-1-828f1c383831@linux.spacemit.com > --- > drivers/acpi/power.c | 1 + > drivers/acpi/scan.c | 4 ---- > 2 files changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c > index d4131c184be8..4f1479103bfe 100644 > --- a/drivers/acpi/power.c > +++ b/drivers/acpi/power.c > @@ -955,6 +955,7 @@ struct acpi_device *acpi_add_power_resource(acpi_handle handle) > INIT_LIST_HEAD(&resource->dependents); > device->power.state = ACPI_STATE_UNKNOWN; > device->flags.match_driver = true; > + device->flags.initialized = true; > > /* Evaluate the object to get the system level and resource order. */ > status = acpi_evaluate_object(handle, NULL, NULL, &buffer); > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index 9a7ac2eb9ce0..489352ba80e1 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -1141,9 +1141,6 @@ static void acpi_bus_get_power_flags(struct acpi_device *device) > if (!list_empty(&device->power.states[ACPI_STATE_D3_HOT].resources)) > device->power.states[ACPI_STATE_D3_COLD].flags.valid = 1; > } > - > - if (acpi_bus_init_power(device)) > - device->flags.power_manageable = 0; > } > > static void acpi_bus_get_flags(struct acpi_device *device) > @@ -1822,7 +1819,6 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle, > acpi_init_properties(device); > acpi_bus_get_flags(device); > device->flags.match_driver = false; > - device->flags.initialized = true; > device->flags.enumeration_by_parent = > acpi_device_enumeration_by_parent(device); > acpi_device_clear_enumerated(device); > > --- Applied as 7.3-rc material, thanks! _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv