[PATCH v2 0/2] Don't make noise about disconnected USB4 devices

[PATCH v2 0/2] Don't make noise about disconnected USB4 devices

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

When a USB4 or TBT3 dock is disconnected a lot of warnings and errors
are emitted related to the PCIe tunnels and XHCI controllers in th
dock.

The messages are loud, but it's mostly because the functions that
emit the messages don't check whether the device is actually alive.
The PCIe hotplug services mark the device as perm dead, so that
can be used to hide some of the messsages.

In the XHCI driver the device is marked as dying already, so that
can also be used to hide messages.

v2:
 * Drop USB patches, these were merged in USB tree already
 * Use pci_dev_is_disconnected() instead

Mario Limonciello (2):
  PCI: Don't show errors on inaccessible PCI devices
  PCI: Fix runtime PM usage count underflow

 drivers/pci/pci-driver.c | 3 ++-
 drivers/pci/pci.c        | 5 +++++
 2 files changed, 7 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH v2 1/2] PCI: Don't show errors on inaccessible PCI devices

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

When a USB4 dock is unplugged the PCIe bridge it's connected to will
remove issue a "Link Down" and "Card not detected event". The PCI core
will treat this as a surprise hotplug event and unconfigure all downstream
devices. This involves setting the device error state to
`pci_channel_io_perm_failure` which pci_dev_is_disconnected() will check.

As the device is already gone and the PCI core is cleaning up there isn't
really any reason to show error messages to the user about failing to
change power states. Detect the device is marked disconnected and skip the
messaging.

Suggested-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2:
 * Use pci_dev_is_disconnected()
v1: https://lore.kernel.org/linux-usb/20250609020223.269407-1-superm1@kernel.org/T/#mf95c947990d016fbfccfd11afe60b8ae08aafa0b
---
 drivers/pci/pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e9448d55113bd..3dd44d1ad829b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1374,6 +1374,11 @@ int pci_power_up(struct pci_dev *dev)
 		return -EIO;
 	}
 
+	if (pci_dev_is_disconnected(dev)) {
+		dev->current_state = PCI_D3cold;
+		return -EIO;
+	}
+
 	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
 	if (PCI_POSSIBLE_ERROR(pmcsr)) {
 		pci_err(dev, "Unable to change power state from %s to D0, device inaccessible\n",
-- 
2.43.0

Re: [PATCH v2 1/2] PCI: Don't show errors on inaccessible PCI devices

[Lukas Wunner]

On Mon, Jun 16, 2025 at 02:26:56PM -0500, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> When a USB4 dock is unplugged the PCIe bridge it's connected to will
> remove issue a "Link Down" and "Card not detected event". The PCI core
> will treat this as a surprise hotplug event and unconfigure all downstream
> devices. This involves setting the device error state to
> `pci_channel_io_perm_failure` which pci_dev_is_disconnected() will check.
> 
> As the device is already gone and the PCI core is cleaning up there isn't
> really any reason to show error messages to the user about failing to
> change power states. Detect the device is marked disconnected and skip the
> messaging.

Code change looks reasonable to me, but should be cc'ed to Rafael.

I find the commit message a bit confusing.  I guess your point is
that it doesn't make sense to runtime resume disconnected devices
to D0 and report the (expected) failure to do so.  That's one sentence.
And the subject could be something like:

    PCI/PM: Skip resuming to D0 if disconnected

Thanks,

Lukas

Re: [PATCH v2 1/2] PCI: Don't show errors on inaccessible PCI devices

[Mario Limonciello]

On 6/19/2025 1:14 AM, Lukas Wunner wrote:
> On Mon, Jun 16, 2025 at 02:26:56PM -0500, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> When a USB4 dock is unplugged the PCIe bridge it's connected to will
>> remove issue a "Link Down" and "Card not detected event". The PCI core
>> will treat this as a surprise hotplug event and unconfigure all downstream
>> devices. This involves setting the device error state to
>> `pci_channel_io_perm_failure` which pci_dev_is_disconnected() will check.
>>
>> As the device is already gone and the PCI core is cleaning up there isn't
>> really any reason to show error messages to the user about failing to
>> change power states. Detect the device is marked disconnected and skip the
>> messaging.
> 
> Code change looks reasonable to me, but should be cc'ed to Rafael.
> 
> I find the commit message a bit confusing.  I guess your point is
> that it doesn't make sense to runtime resume disconnected devices
> to D0 and report the (expected) failure to do so.  That's one sentence.
> And the subject could be something like:

Thanks I'll replace the second paragraph with your proposed sentence and 
update the commit subject for v3.

> 
>      PCI/PM: Skip resuming to D0 if disconnected
> 
> Thanks,
> 
> Lukas

[PATCH v2 2/2] PCI: Fix runtime PM usage count underflow

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

When a USB4 dock is unplugged the PCIe bridge it's connected to will
remove issue a "Link Down" and "Card not detected event". The PCI core
will treat this as a surprise hotplug event and unconfigure all downstream
devices.

When PCI core gets to the point that the device is removed using
pci_device_remove() the runtime count has already been decremented and
so calling pm_runtime_put_sync() will cause an underflow.

Detect the device ishas been disconnected and skip the call for this
cleanup path.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v2:
 * Use pci_dev_is_disconnected()
v1: https://lore.kernel.org/linux-usb/20250609020223.269407-1-superm1@kernel.org/T/#mf95c947990d016fbfccfd11afe60b8ae08aafa0b
---
 drivers/pci/pci-driver.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 67db34fd10ee7..0d4c67829958b 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -479,7 +479,8 @@ static void pci_device_remove(struct device *dev)
 	pci_iov_remove(pci_dev);
 
 	/* Undo the runtime PM settings in local_pci_probe() */
-	pm_runtime_put_sync(dev);
+	if (!pci_dev_is_disconnected(pci_dev))
+		pm_runtime_put_sync(dev);
 
 	/*
 	 * If the device is still on, set the power state as "unknown",
-- 
2.43.0

Re: [PATCH v2 2/2] PCI: Fix runtime PM usage count underflow

[Lukas Wunner]

On Mon, Jun 16, 2025 at 02:26:57PM -0500, Mario Limonciello wrote:
> When a USB4 dock is unplugged the PCIe bridge it's connected to will
> remove issue a "Link Down" and "Card not detected event". The PCI core
> will treat this as a surprise hotplug event and unconfigure all downstream
> devices.
> 
> When PCI core gets to the point that the device is removed using
> pci_device_remove() the runtime count has already been decremented and
> so calling pm_runtime_put_sync() will cause an underflow.

Unfortunately this v2 does not address the review comment I provided
earlier:

   "Where has it been decremented?  I think this needs to be identified
    and a Fixes tag added."

   https://lore.kernel.org/r/aEb60TkaaLZ3kKIT@wunner.de/

Thanks,

Lukas