[PATCH] PCI: Stop setting cached power state to "unknown" on unbind

[PATCH] PCI: Stop setting cached power state to "unknown" on unbind

[Lukas Wunner]

When a PCI device is unbound from its driver, pci_device_remove() sets the
cached power state in pci_dev->current_state to PCI_UNKNOWN.  This was
introduced by commit 2449e06a5696 ("PCI: reset pci device state to unknown
state for resume") to invalidate the cached power state in case the system
is subsequently put to sleep.

For bound devices, the cached power state is set to PCI_UNKNOWN in
pci_pm_suspend_noirq(), immediately before entering system sleep.

Extend to unbound devices for consistency.

This obviates the need to change the cached power state on unbind, so stop
doing so.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/pci-driver.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 7c2d9d5..774892d 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -482,13 +482,6 @@ static void pci_device_remove(struct device *dev)
 	pm_runtime_put_sync(dev);
 
 	/*
-	 * If the device is still on, set the power state as "unknown",
-	 * since it might change by the next time we load the driver.
-	 */
-	if (pci_dev->current_state == PCI_D0)
-		pci_dev->current_state = PCI_UNKNOWN;
-
-	/*
 	 * We would love to complain here if pci_dev->is_enabled is set, that
 	 * the driver should have called pci_disable_device(), but the
 	 * unfortunate fact is there are too many odd BIOS and bridge setups
@@ -862,7 +855,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
 
 	if (!pm) {
 		pci_save_state(pci_dev);
-		goto Fixup;
+		goto set_unknown;
 	}
 
 	if (pm->suspend_noirq) {
@@ -914,6 +907,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
 		goto Fixup;
 	}
 
+set_unknown:
 	pci_pm_set_unknown_state(pci_dev);
 
 	/*
-- 
2.51.0

Re: [PATCH] PCI: Stop setting cached power state to "unknown" on unbind

[Mario Limonciello]

On 4/17/26 03:51, Lukas Wunner wrote:
> When a PCI device is unbound from its driver, pci_device_remove() sets the
> cached power state in pci_dev->current_state to PCI_UNKNOWN.  This was
> introduced by commit 2449e06a5696 ("PCI: reset pci device state to unknown
> state for resume") to invalidate the cached power state in case the system
> is subsequently put to sleep.
> 
> For bound devices, the cached power state is set to PCI_UNKNOWN in
> pci_pm_suspend_noirq(), immediately before entering system sleep.
> 
> Extend to unbound devices for consistency.
> 
> This obviates the need to change the cached power state on unbind, so stop
> doing so.
> 
> Signed-off-by: Lukas Wunner <lukas@wunner.de>

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>

FWIW I also had review-prompts /kreview check for corner cases and it 
didn't find any.

> ---
>   drivers/pci/pci-driver.c | 10 ++--------
>   1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 7c2d9d5..774892d 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -482,13 +482,6 @@ static void pci_device_remove(struct device *dev)
>   	pm_runtime_put_sync(dev);
>   
>   	/*
> -	 * If the device is still on, set the power state as "unknown",
> -	 * since it might change by the next time we load the driver.
> -	 */
> -	if (pci_dev->current_state == PCI_D0)
> -		pci_dev->current_state = PCI_UNKNOWN;
> -
> -	/*
>   	 * We would love to complain here if pci_dev->is_enabled is set, that
>   	 * the driver should have called pci_disable_device(), but the
>   	 * unfortunate fact is there are too many odd BIOS and bridge setups
> @@ -862,7 +855,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
>   
>   	if (!pm) {
>   		pci_save_state(pci_dev);
> -		goto Fixup;
> +		goto set_unknown;
>   	}
>   
>   	if (pm->suspend_noirq) {
> @@ -914,6 +907,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
>   		goto Fixup;
>   	}
>   
> +set_unknown:
>   	pci_pm_set_unknown_state(pci_dev);
>   
>   	/*