[PATCH v3 0/4] ACPI: scan: Adjust power management initialization and PCI devices handling

[PATCH v3 0/4] ACPI: scan: Adjust power management initialization and PCI devices handling

[Rafael J. Wysocki]

Hi All,

This is an update of

https://lore.kernel.org/linux-acpi/5144065.31r3eYUQgx@rafael.j.wysocki/

that adds 1 patch, 3 two patches, and addresses review comments in patch [2/4].

The new patch drops in v3 is a cleanup relocating the acpi_bus_init_power()
declaration to an internal header file in the core ACPI support code (patch
[3/4]).

One of the patches dropped from the v2 has been folded into patch [2/4]
and the other two have become cleanups on top of this series that can be
done later.

Overall, the series aims at addressing a few issues present in the core ACPI
device enumeration code, mostly related to the acpi_bus_attach() function.

The first one is that, on some systems, ACPI power management is initialized
twice for devices that have missing dependencies to start with.  An attempt
to address this issue had been made during the 7.3 merge window, but it had
to be reverted [1].  The approach used here is roughly the same as in the
reverted commit, but it takes PCI devices (which are a special case) into
account.

In addition to the above, ACPI power management may be initialized prematurely
for device objects whose parents are not ready for enumeration.

The other issues are that flags are unnecessarily cleared by acpi_bus_attach()
for devices with missing dependencies and PCI devices are handled by
acpi_bus_attach() like any other devices which is a mistake.

Please see the changelogs of individual patches for details.

Thanks!


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

[PATCH v3 1/4] ACPI: PM: Drop parent state update from acpi_device_get_power()

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The parent state update in acpi_device_get_power(), that occurs when the
child device turns out to be in D0 and the parent's power state is still
unknown, is highly questionable because it may cause the reference
counters of the power resources used by the parent in D0 (if any) to
underflow when the parent goes into a low-power state later.  Moreover,
there is no reason to do it on reads from the real_power_state sysfs
attribute of the child.

That check had been added by commit 8f7412a792bc ("ACPI / PM: Infer
parent power state from child if unknown, v2") before starting to
handle the "missing _PSC and no power resources" case directly in
acpi_bus_init_power(), as of commit b3785492268f ("ACPI / PM: Do not
power manage devices in unknown initial states").  It is not necessary
any more and commit b3785492268f should have removed it.

Drop it now.

Fixes: b3785492268f ("ACPI / PM: Do not power manage devices in unknown initial states")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v2 -> v3: No changes

New patch in v2.

---
 drivers/acpi/device_pm.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index aa55ecfc2923..4269735aadde 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -75,15 +75,14 @@ static int acpi_dev_pm_explicit_get(struct acpi_device *device, int *state)
 int acpi_device_get_power(struct acpi_device *device, int *state)
 {
 	int result = ACPI_STATE_UNKNOWN;
-	struct acpi_device *parent;
 	int error;
 
 	if (!device || !state)
 		return -EINVAL;
 
-	parent = acpi_dev_parent(device);
-
 	if (!device->flags.power_manageable) {
+		struct acpi_device *parent = acpi_dev_parent(device);
+
 		/* TBD: Non-recursive algorithm for walking up hierarchy. */
 		*state = parent ? parent->power.state : ACPI_STATE_D0;
 		goto out;
@@ -119,16 +118,6 @@ int acpi_device_get_power(struct acpi_device *device, int *state)
 			result = psc > ACPI_STATE_D2 ? ACPI_STATE_D3_HOT : psc;
 	}
 
-	/*
-	 * If we were unsure about the device parent's power state up to this
-	 * point, the fact that the device is in D0 implies that the parent has
-	 * to be in D0 too, except if ignore_parent is set.
-	 */
-	if (!device->power.flags.ignore_parent && parent &&
-	    parent->power.state == ACPI_STATE_UNKNOWN &&
-	    result == ACPI_STATE_D0)
-		parent->power.state = ACPI_STATE_D0;
-
 	*state = result;
 
  out:
-- 
2.51.0

Re: [PATCH v3 1/4] ACPI: PM: Drop parent state update from acpi_device_get_power()

[sashiko-bot]

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> The parent state update in acpi_device_get_power(), that occurs when the
> child device turns out to be in D0 and the parent's power state is still
> unknown, is highly questionable because it may cause the reference
> counters of the power resources used by the parent in D0 (if any) to
> underflow when the parent goes into a low-power state later.  Moreover,
> there is no reason to do it on reads from the real_power_state sysfs
> attribute of the child.
> 
> That check had been added by commit 8f7412a792bc ("ACPI / PM: Infer
> parent power state from child if unknown, v2") before starting to
> handle the "missing _PSC and no power resources" case directly in
> acpi_bus_init_power(), as of commit b3785492268f ("ACPI / PM: Do not
> power manage devices in unknown initial states").  It is not necessary
> any more and commit b3785492268f should have removed it.
> 
> Drop it now.
> 
> Fixes: b3785492268f ("ACPI / PM: Do not power manage devices in unknown initial states")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

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

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

There is a problem, introduced by commit 9d9bcae47fd5 ("ACPI: delay
enumeration of devices with a _DEP pointing to an INT3472 device")
inadvertently, that devices with missing dependencies may be put
into power state D0 prematurely [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()
runs again for the device and now it calls acpi_bus_init_power() that
takes 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.

Another problem, related to the previous one, is that ACPI power state
initialization may be carried out for devices whose parents are not
ready for enumeration which may lead to initialization ordering issues.

To address both, 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
acpi_bus_attach() is called for the latter.

To that end, notice that acpi_power_up_if_adr_present() is used for
powering-up PCI devices in D3cold before walking the bus in order to
discover them and the initial ACPI power state of those devices needs
to be known for this purpose, so add an acpi_bus_init_power() invocation
to that function.  [The debug statement printed by it duplicates the
debug statements printed during the acpi_bus_init_power() execution, so
drop it.]

Moreover, since the ACPI companions of PCI devices are associated with
the corresponding PCI devices found on the bus before acpi_bus_attach()
is called for them, it is not necessary or even useful to skip them in
acpi_bus_attach() due to an ACPI status mismatch, so avoid doing that
and complain if the ACPI status does not match the observed situation.

Also use the ACPI power state tracking to decide whether or not
the device's power state needs to be initialized in acpi_bus_attach()
instead of using the "initialized" flag of the ACPI device object
for this purpose, which is fragile and inconvenient, and clear the
power_manageable flag on failure in acpi_bus_init_power() (additionally,
poison the device ACPI power state as "invalid" if the initialization of
it fails).  That allows the clearing of the power_manageable flag for
devices with unmet dependencies to be dropped.

While at it, add a debug message pringing statement to
acpi_bus_init_power() to facilitate diagnostics.

Fixes: 9d9bcae47fd5 ("ACPI: delay enumeration of devices with a _DEP pointing to an INT3472 device")
Link: https://lore.kernel.org/linux-acpi/20260820-acpi-power-resource-ref-fix-v2-1-29818173ea13@linux.spacemit.com/ [1]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v2 -> v3:
   * Address Sashiko review comments:
     https://lore.kernel.org/linux-pci/20260902195849.174031F000E9@smtp.kernel.org/
   * Fold patch [6/6] from v2 in for completeness
   * Add recovery for parents with invalid ACPI power states to
     acpi_bus_init_power()
   * Set device->flags.initialized in acpi_bus_attach() to avoid clearing it
     permanently after the first attach/detach cycle of a device

v1 -> v2:
   * Address Sashiko review comments:
     https://lore.kernel.org/linux-pci/20260831201426.92F091F000E9@smtp.kernel.org/

---
 drivers/acpi/device_pm.c | 62 ++++++++++++++++++++++++++++++++--------
 drivers/acpi/scan.c      | 40 +++++++++++++++++---------
 2 files changed, 76 insertions(+), 26 deletions(-)

diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 4269735aadde..76104f715efa 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -23,6 +23,8 @@
 #include "fan.h"
 #include "internal.h"
 
+#define ACPI_D_STATE_INVALID	ACPI_D_STATE_COUNT
+
 /**
  * acpi_power_state_string - String representation of ACPI device power state.
  * @state: ACPI device power state to return the string representation of.
@@ -293,24 +295,28 @@ int acpi_bus_set_power(acpi_handle handle, int state)
 }
 EXPORT_SYMBOL(acpi_bus_set_power);
 
-int acpi_bus_init_power(struct acpi_device *device)
+static int acpi_device_init_power(struct acpi_device *device)
 {
 	int state;
 	int result;
 
-	if (!device)
-		return -EINVAL;
-
-	device->power.state = ACPI_STATE_UNKNOWN;
-	if (!acpi_device_is_present(device)) {
-		device->flags.initialized = false;
-		return -ENXIO;
-	}
-
 	result = acpi_device_get_power(device, &state);
 	if (result)
 		return result;
 
+	/*
+	 * If the current power state of the device is D0 and it has a parent
+	 * whose power state is not ignored, and the parent's power state
+	 * initialization has failed, the parent's power state can be updated to
+	 * D0 for consistency.
+	 */
+	if (!device->power.flags.ignore_parent && state == ACPI_STATE_D0) {
+		struct acpi_device *parent = acpi_dev_parent(device);
+
+		if (parent && parent->power.state == ACPI_D_STATE_INVALID)
+			parent->power.state = ACPI_STATE_D0;
+	}
+
 	if (state < ACPI_STATE_D3_COLD && device->power.flags.power_resources) {
 		/* Reference count the power resources. */
 		result = acpi_power_on_resources(device, state);
@@ -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;
+
+	/*
+	 * 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);
+	if (result) {
+		device->flags.power_manageable = 0;
+		device->power.state = ACPI_D_STATE_INVALID;
+		acpi_handle_info(device->handle,
+				 "Initial power state undetermined, ACPI PM disabled\n");
+	}
+
+	return result;
+}
+
 /**
  * acpi_device_fix_up_power - Force device with missing _PSC into D0.
  * @device: Device object whose power state is to be fixed up.
@@ -464,8 +497,13 @@ static int acpi_power_up_if_adr_present(struct acpi_device *adev, void *not_used
 	if (!(adev->flags.power_manageable && adev->pnp.type.bus_address))
 		return 0;
 
-	acpi_handle_debug(adev->handle, "Power state: %s\n",
-			  acpi_power_state_string(adev->power.state));
+	/*
+	 * This is done during the PCI root initialization which occurs before
+	 * acpi_bus_attach() is called for the device, so the ACPI power state
+	 * of the device needs to be initialized here.
+	 */
+	if (acpi_bus_init_power(adev))
+		return 0;
 
 	if (adev->power.state == ACPI_STATE_D3_COLD)
 		return acpi_device_set_power(adev, ACPI_STATE_D0);
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index f48715ed827c..f219a16e018a 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>
@@ -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;
 }
 
 static void acpi_bus_get_flags(struct acpi_device *device)
@@ -2342,38 +2342,50 @@ static int acpi_scan_attach_handler(struct acpi_device *device)
 static int acpi_bus_attach(struct acpi_device *device, void *first_pass)
 {
 	bool skip = !first_pass && device->flags.visited;
+	struct pci_dev *pci;
 	acpi_handle ejd;
 	int ret;
 
 	if (skip)
 		goto ok;
 
+	device->flags.initialized = true;
+
 	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)) {
-		device->flags.initialized = false;
+	/*
+	 * If the given ACPI device object has been already associated with a
+	 * PCI device found on the bus, its status is effectively "present and
+	 * enabled", and dependencies are not tracked for PCI devices, so it is
+	 * not necessary or even useful to check the device's readiness in that
+	 * case.
+	 */
+	pci = acpi_dev_get_pci_dev(device);
+	if (pci) {
+		acpi_handle_debug(device->handle, "PCI companion %s found\n",
+				  pci_name(pci));
+
+		if (!acpi_device_is_present(device))
+			pci_info(pci, FW_BUG "ACPI status differs from reality\n");
+
+		pci_dev_put(pci);
+	} else if (!acpi_dev_ready_for_enumeration(device)) {
+		/* The device is not ready (e.g. not present), so skip it. */
 		acpi_device_clear_enumerated(device);
-		device->flags.power_manageable = 0;
 		return 0;
 	}
+
 	if (device->handler)
 		goto ok;
 
 	acpi_ec_register_opregions(device);
 
-	if (!device->flags.initialized) {
-		device->flags.power_manageable =
-			device->power.states[ACPI_STATE_D0].flags.valid;
-		if (acpi_bus_init_power(device))
-			device->flags.power_manageable = 0;
+	acpi_bus_init_power(device);
 
-		device->flags.initialized = true;
-	} else if (device->flags.visited) {
+	if (device->flags.visited)
 		goto ok;
-	}
 
 	ret = acpi_scan_attach_handler(device);
 	if (ret < 0)
-- 
2.51.0

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

[sashiko-bot]

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

[PATCH v3 3/4] ACPI: PM: Move acpi_bus_init_power() declaration to internal header file

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Since acpi_bus_init_power() is only used internally in the core ACPI
device enumeration and power management code, it need not be visible
outside, so move its declaration to an internal header file.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

New patch in v3.

---
 drivers/acpi/internal.h | 1 +
 include/acpi/acpi_bus.h | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 40f875b265a9..011da4ed6a80 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -157,6 +157,7 @@ void acpi_turn_off_unused_power_resources(void);
                               Device Power Management
    -------------------------------------------------------------------------- */
 int acpi_device_get_power(struct acpi_device *device, int *state);
+int acpi_bus_init_power(struct acpi_device *device);
 int acpi_wakeup_device_init(void);
 
 /* --------------------------------------------------------------------------
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 1a45e0d521d8..6b3f8b230eec 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -618,7 +618,6 @@ int acpi_bus_get_status(struct acpi_device *device);
 int acpi_bus_set_power(acpi_handle handle, int state);
 const char *acpi_power_state_string(int state);
 int acpi_device_set_power(struct acpi_device *device, int state);
-int acpi_bus_init_power(struct acpi_device *device);
 int acpi_device_fix_up_power(struct acpi_device *device);
 void acpi_device_fix_up_power_extended(struct acpi_device *adev);
 void acpi_device_fix_up_power_children(struct acpi_device *adev);
-- 
2.51.0

Re: [PATCH v3 3/4] ACPI: PM: Move acpi_bus_init_power() declaration to internal header file

[sashiko-bot]

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> Since acpi_bus_init_power() is only used internally in the core ACPI
> device enumeration and power management code, it need not be visible
> outside, so move its declaration to an internal header file.
> 
> No functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

[PATCH v3 4/4] ACPI: scan: Combine two conditionals in acpi_bus_attach()

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

There are two conditionals in acpi_bus_attach() that can be combined,
which slightly reduces the overhead and makes the code a bit easier
to follow, so do that.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

v2 -> v3:
   * Add tag from Andy

v1 -> v2:
   * Reverse checks to avoid multiple negations (Andy)
   * Rebase on top of the new [2/6]

Link to the v1:
https://lore.kernel.org/linux-pci/2021470.taCxCBeP46@rafael.j.wysocki/

---
 drivers/acpi/scan.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index f219a16e018a..1b8d833131a0 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2391,13 +2391,8 @@ static int acpi_bus_attach(struct acpi_device *device, void *first_pass)
 	if (ret < 0)
 		return 0;
 
-	if (ret > 0 && !device->flags.enumeration_by_parent) {
-		acpi_device_set_enumerated(device);
-		goto ok;
-	}
-
-	if (device->pnp.type.platform_id || device->pnp.type.backlight ||
-	    device->flags.enumeration_by_parent)
+	if (device->flags.enumeration_by_parent ||
+	    (!ret && (device->pnp.type.platform_id || device->pnp.type.backlight)))
 		acpi_default_enumeration(device);
 	else
 		acpi_device_set_enumerated(device);
-- 
2.51.0

Re: [PATCH v3 4/4] ACPI: scan: Combine two conditionals in acpi_bus_attach()

[sashiko-bot]

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> There are two conditionals in acpi_bus_attach() that can be combined,
> which slightly reduces the overhead and makes the code a bit easier
> to follow, so do that.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [PATCH v3 0/4] ACPI: scan: Adjust power management initialization and PCI devices handling

[Peixin Xie]

Hi Rafael,

On Thu, 03 Sep 2026 18:57:09 +0200, Rafael J. Wysocki wrote:
> Hi All,
> 
> This is an update of
> 
> https://lore.kernel.org/linux-acpi/5144065.31r3eYUQgx@rafael.j.wysocki/
> 
> that adds 1 patch, 3 two patches, and addresses review comments in patch [2/4].
> 
> The new patch drops in v3 is a cleanup relocating the acpi_bus_init_power()
> declaration to an internal header file in the core ACPI support code (patch
> [3/4]).
> 
> One of the patches dropped from the v2 has been folded into patch [2/4]
> and the other two have become cleanups on top of this series that can be
> done later.
> 
> Overall, the series aims at addressing a few issues present in the core ACPI
> device enumeration code, mostly related to the acpi_bus_attach() function.
> 
> The first one is that, on some systems, ACPI power management is initialized
> twice for devices that have missing dependencies to start with.  An attempt
> to address this issue had been made during the 7.3 merge window, but it had
> to be reverted [1].  The approach used here is roughly the same as in the
> reverted commit, but it takes PCI devices (which are a special case) into
> account.
> 
> In addition to the above, ACPI power management may be initialized prematurely
> for device objects whose parents are not ready for enumeration.
> 
> The other issues are that flags are unnecessarily cleared by acpi_bus_attach()
> for devices with missing dependencies and PCI devices are handled by
> acpi_bus_attach() like any other devices which is a mistake.
> 
> Please see the changelogs of individual patches for details.
> 
> Thanks!
> 
> 
> Link: https://lore.kernel.org/linux-acpi/20260820-acpi-power-resource-ref-fix-v2-1-29818173ea13@linux.spacemit.com/ [1]

I tested the complete v3 series on the SpacemiT K3 RISC-V Pico-ITX platform
where the original duplicate ACPI PowerResource reference issue was
observed.

The power resource left on by firmware is now turned off as unused after
the namespace scan. After the device dependency is satisfied, the power
resource is turned on once for the device and is turned off normally
during runtime suspend. Runtime resume also works correctly.

Tested-by: Peixin Xie <peixin.xie@linux.spacemit.com>

-- 
Best Regards,
Peixin Xie

Re: [PATCH v3 0/4] ACPI: scan: Adjust power management initialization and PCI devices handling

[Rafael J. Wysocki (Intel)]

On Fri, Sep 4, 2026 at 11:41 AM Peixin Xie
<peixin.xie@linux.spacemit.com> wrote:
>
> Hi Rafael,
>
> On Thu, 03 Sep 2026 18:57:09 +0200, Rafael J. Wysocki wrote:
> > Hi All,
> >
> > This is an update of
> >
> > https://lore.kernel.org/linux-acpi/5144065.31r3eYUQgx@rafael.j.wysocki/
> >
> > that adds 1 patch, 3 two patches, and addresses review comments in patch [2/4].
> >
> > The new patch drops in v3 is a cleanup relocating the acpi_bus_init_power()
> > declaration to an internal header file in the core ACPI support code (patch
> > [3/4]).
> >
> > One of the patches dropped from the v2 has been folded into patch [2/4]
> > and the other two have become cleanups on top of this series that can be
> > done later.
> >
> > Overall, the series aims at addressing a few issues present in the core ACPI
> > device enumeration code, mostly related to the acpi_bus_attach() function.
> >
> > The first one is that, on some systems, ACPI power management is initialized
> > twice for devices that have missing dependencies to start with.  An attempt
> > to address this issue had been made during the 7.3 merge window, but it had
> > to be reverted [1].  The approach used here is roughly the same as in the
> > reverted commit, but it takes PCI devices (which are a special case) into
> > account.
> >
> > In addition to the above, ACPI power management may be initialized prematurely
> > for device objects whose parents are not ready for enumeration.
> >
> > The other issues are that flags are unnecessarily cleared by acpi_bus_attach()
> > for devices with missing dependencies and PCI devices are handled by
> > acpi_bus_attach() like any other devices which is a mistake.
> >
> > Please see the changelogs of individual patches for details.
> >
> > Thanks!
> >
> >
> > Link: https://lore.kernel.org/linux-acpi/20260820-acpi-power-resource-ref-fix-v2-1-29818173ea13@linux.spacemit.com/ [1]
>
> I tested the complete v3 series on the SpacemiT K3 RISC-V Pico-ITX platform
> where the original duplicate ACPI PowerResource reference issue was
> observed.
>
> The power resource left on by firmware is now turned off as unused after
> the namespace scan. After the device dependency is satisfied, the power
> resource is turned on once for the device and is turned off normally
> during runtime suspend. Runtime resume also works correctly.
>
> Tested-by: Peixin Xie <peixin.xie@linux.spacemit.com>

Thank you!