[PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[Mathias Nyman]

The relationship between a USB4 Host Interface providing USB3 tunnels,
and tunneled USB3 devices is not represented in device hierarchy.

This caused issues with power managment as devices may suspend and
resume in incorrect order.
A device link between the USB4 Host Interface and the USB3 xHCI was
originally added to solve this, preventing the USB4 Host Interface from
suspending if the USB3 xHCI Host was still active.
This unfortunately also prevents USB4 Host Interface from suspending even
if the USB3 xHCI Host is only serving native non-tunneled USB devices.

Improve the current powermanagement situation by creating device links
directly from tunneled USB3 devices to the USB4 Host Interface they depend
on instead of a device link between the hosts.
This way USB4 host may suspend when the last tunneled device is
disconnected.

Intel xHCI hosts are capable of reporting if connected USB3 devices are
tunneled via vendor specific capabilities.
Use this until a standard way is available.

Mathias Nyman (4):
  xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
  usb: Add tunneled parameter to usb device structure
  usb: acpi: add device link between tunneled USB3 device and USB4 Host
    Interface
  thunderbolt: Don't create device link from USB4 Host Interface to USB3
    xHC host

 drivers/thunderbolt/acpi.c       | 40 ++++++------------------
 drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci-ext-caps.h |  5 +++
 drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
 drivers/usb/host/xhci.c          | 12 ++++++++
 drivers/usb/host/xhci.h          |  1 +
 include/linux/usb.h              |  2 ++
 7 files changed, 111 insertions(+), 30 deletions(-)

-- 
2.25.1

[PATCH 1/4] xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts

[Mathias Nyman]

Knowledge about tunneled devices is useful in order to correctly describe
the relationship between tunneled USB3 device and USB4 Host Interface,
ensuring proper suspend and resume order, and to be able to power down
Thunderbolt if there is no need for tunneling.

Intel hosts share if a USB3 connection is native or tunneled via vendor
specific "SPR eSS PORT" registers.

These vendor registers are available if host supports a vendor specific
SPR shadow extended capability with ID 206. Registers are per USB3 port
and 0x20 apart.

Knowing the tunneling status of the device connected to roothub is enough
as all its children will have the same status.

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-ext-caps.h |  5 +++++
 drivers/usb/host/xhci-hub.c      | 29 +++++++++++++++++++++++++++++
 drivers/usb/host/xhci.c          | 11 +++++++++++
 drivers/usb/host/xhci.h          |  1 +
 4 files changed, 46 insertions(+)

diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
index 96eb36a58738..67ecf7320c62 100644
--- a/drivers/usb/host/xhci-ext-caps.h
+++ b/drivers/usb/host/xhci-ext-caps.h
@@ -42,6 +42,7 @@
 #define XHCI_EXT_CAPS_DEBUG	10
 /* Vendor caps */
 #define XHCI_EXT_CAPS_VENDOR_INTEL	192
+#define XHCI_EXT_CAPS_INTEL_SPR_SHADOW	206
 /* USB Legacy Support Capability - section 7.1.1 */
 #define XHCI_HC_BIOS_OWNED	(1 << 16)
 #define XHCI_HC_OS_OWNED	(1 << 24)
@@ -64,6 +65,10 @@
 #define XHCI_HLC               (1 << 19)
 #define XHCI_BLC               (1 << 20)
 
+/* Intel SPR shadow capability */
+#define XHCI_INTEL_SPR_ESS_PORT_OFFSET  0x8ac4	/* SuperSpeed port control */
+#define XHCI_INTEL_SPR_TUNEN	BIT(4)		/* Tunnel mode enabled */
+
 /* command register values to disable interrupts and halt the HC */
 /* start/stop HC execution - do not write unless HC is halted*/
 #define XHCI_CMD_RUN		(1 << 0)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 61f083de6e19..a8043c15a390 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -752,6 +752,35 @@ static int xhci_exit_test_mode(struct xhci_hcd *xhci)
 	return xhci_reset(xhci, XHCI_RESET_SHORT_USEC);
 }
 
+/**
+ * xhci_port_is_tunneled() - Check if USB3 connection is tunneled over USB4
+ * @xhci: xhci host controller
+ * @port: USB3 port to be checked.
+ *
+ * Some hosts can detect if a USB3 connection is native USB3 or tunneled over
+ * USB4. Intel hosts expose this via vendor specific extended capability 206
+ * eSS PORT registers TUNEN (tunnel enabled) bit.
+ *
+ * A USB3 device must be connected to the port to detect the tunnel.
+ *
+ * Return: true if USB3 connection is tunneled over USB4
+ */
+bool xhci_port_is_tunneled(struct xhci_hcd *xhci, struct xhci_port *port)
+{
+	void __iomem *base;
+	u32 offset;
+
+	base = &xhci->cap_regs->hc_capbase;
+	offset = xhci_find_next_ext_cap(base, 0, XHCI_EXT_CAPS_INTEL_SPR_SHADOW);
+
+	if (offset && offset <= XHCI_INTEL_SPR_ESS_PORT_OFFSET) {
+		offset = XHCI_INTEL_SPR_ESS_PORT_OFFSET + port->hcd_portnum * 0x20;
+		return !!(readl(base + offset) & XHCI_INTEL_SPR_TUNEN);
+	}
+
+	return false;
+}
+
 void xhci_set_link_state(struct xhci_hcd *xhci, struct xhci_port *port,
 			 u32 link_state)
 {
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 37eb37b0affa..a85173098de1 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4513,6 +4513,17 @@ static int xhci_update_device(struct usb_hcd *hcd, struct usb_device *udev)
 	struct xhci_port *port;
 	u32 capability;
 
+	/* Check if USB3 device at root port is tunneled over USB4 */
+	if (hcd->speed >= HCD_USB3 && !udev->parent->parent) {
+		port = xhci->usb3_rhub.ports[udev->portnum - 1];
+
+		if (xhci_port_is_tunneled(xhci, port))
+			dev_dbg(&udev->dev, "tunneled over USB4 link\n");
+		else
+			dev_dbg(&udev->dev, "native USB 3.x link\n");
+		return 0;
+	}
+
 	if (hcd->speed >= HCD_USB3 || !udev->lpm_capable || !xhci->hw_lpm_support)
 		return 0;
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 78d014c4d884..d9f706d80ac6 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1926,6 +1926,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16 wIndex,
 int xhci_hub_status_data(struct usb_hcd *hcd, char *buf);
 int xhci_find_raw_port_number(struct usb_hcd *hcd, int port1);
 struct xhci_hub *xhci_get_rhub(struct usb_hcd *hcd);
+bool xhci_port_is_tunneled(struct xhci_hcd *xhci, struct xhci_port *port);
 
 void xhci_hc_died(struct xhci_hcd *xhci);
 
-- 
2.25.1

[PATCH 2/4] usb: Add tunneled parameter to usb device structure

[Mathias Nyman]

Add 'tunneled' parameter to usb device structure to describe if a device
connection is tunneled over USB4 or connected directly using native
USB2/USB3.

Tunneled devices depend on USB4 NHI host to maintain the tunnel.
Knowledge about tunneled devices is important to ensure correct
suspend and resume order between USB4 hosts and tunneled devices.
i.e. make sure tunnel is up before the USB device using it resumes.

USB hosts such as xHCI have vendor specific ways to detect tunneled
connections. This 'tunneled' parameter can be set by USB3 host driver
during hcd->driver->update_device(hcd, udev) callback

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci.c | 3 ++-
 include/linux/usb.h     | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a85173098de1..e11251e3e4fc 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4517,7 +4517,8 @@ static int xhci_update_device(struct usb_hcd *hcd, struct usb_device *udev)
 	if (hcd->speed >= HCD_USB3 && !udev->parent->parent) {
 		port = xhci->usb3_rhub.ports[udev->portnum - 1];
 
-		if (xhci_port_is_tunneled(xhci, port))
+		udev->tunneled = xhci_port_is_tunneled(xhci, port);
+		if (udev->tunneled)
 			dev_dbg(&udev->dev, "tunneled over USB4 link\n");
 		else
 			dev_dbg(&udev->dev, "native USB 3.x link\n");
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 1913a13833f2..a4c5c09ea2bd 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -605,6 +605,7 @@ struct usb3_lpm_parameters {
  *	WUSB devices are not, until we authorize them from user space.
  *	FIXME -- complete doc
  * @authenticated: Crypto authentication passed
+ * @tunneled: Connected to root port and tunneled over USB4
  * @lpm_capable: device supports LPM
  * @lpm_devinit_allow: Allow USB3 device initiated LPM, exit latency is in range
  * @usb2_hw_lpm_capable: device can perform USB2 hardware LPM
@@ -685,6 +686,7 @@ struct usb_device {
 	unsigned have_langid:1;
 	unsigned authorized:1;
 	unsigned authenticated:1;
+	unsigned tunneled:1;
 	unsigned lpm_capable:1;
 	unsigned lpm_devinit_allow:1;
 	unsigned usb2_hw_lpm_capable:1;
-- 
2.25.1

[PATCH 3/4] usb: acpi: add device link between tunneled USB3 device and USB4 Host Interface

[Mathias Nyman]

Describe the power management relationship between a tunneled USB3 device
and the tunnel providing USB4 host with a device link as the relationship
between them is not evident from normal device hierarchy.

Tunneling capable ports have an ACPI _DSD object pointing to the USB4
Host Interface that is used to establish USB3 3.x tunnels

Set the link directly between tunneled USB3 devices and USB4 Host
Interface to ensure that the USB4 host can runtime suspend if no tunneled
USB 3.x devices exist.
Current Thunderbolt code sets a link between USB4 Host Interface and USB3
xHCI host which prevents USB4 Host Interface from runtime suspending even
if the USB3 host is only serving native USB devices.

See commit b2be2b05cf3b ("thunderbolt: Create device links from ACPI
description") for details.

As the device link is only set for USB3 devices that are already tunneled
we know that USB4 Host Interface exists and is bound to its driver.

Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/core/usb-acpi.c | 52 +++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index 7f8a912d4fe2..316bcc168e76 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -142,6 +142,53 @@ int usb_acpi_set_power_state(struct usb_device *hdev, int index, bool enable)
 }
 EXPORT_SYMBOL_GPL(usb_acpi_set_power_state);
 
+/**
+ * usb_acpi_add_usb4_devlink - add device link to USB4 Host Interface for tunneled USB3 devices
+ *
+ * @udev: Tunneled USB3 device connected to a roothub.
+ *
+ * Adds a device link between a tunneled USB3 device and the USB4 Host Interface
+ * device to ensure correct runtime PM suspend and resume order. This function
+ * should only be called for tunneled USB3 devices.
+ * The USB4 Host Interface this tunneled device depends on is found from the roothub
+ * port ACPI device specific data _DSD entry.
+ *
+ * Return: negative error code on failure, 0 otherwise
+ */
+static int usb_acpi_add_usb4_devlink(struct usb_device *udev)
+{
+	const struct device_link *link;
+	struct usb_port *port_dev;
+	struct usb_hub *hub;
+
+	if (!udev->parent || udev->parent->parent)
+		return 0;
+
+	hub = usb_hub_to_struct_hub(udev->parent);
+	port_dev = hub->ports[udev->portnum - 1];
+
+	struct fwnode_handle *nhi_fwnode __free(fwnode_handle) =
+		fwnode_find_reference(dev_fwnode(&port_dev->dev), "usb4-host-interface", 0);
+
+	if (IS_ERR(nhi_fwnode))
+		return 0;
+
+	link = device_link_add(&port_dev->child->dev, nhi_fwnode->dev,
+			       DL_FLAG_AUTOREMOVE_CONSUMER |
+			       DL_FLAG_RPM_ACTIVE |
+			       DL_FLAG_PM_RUNTIME);
+	if (!link) {
+		dev_err(&port_dev->dev, "Failed to created device link from %s to %s\n",
+			dev_name(&port_dev->child->dev), dev_name(nhi_fwnode->dev));
+		return -EINVAL;
+	}
+
+	dev_dbg(&port_dev->dev, "Created device link from %s to %s\n",
+		dev_name(&port_dev->child->dev), dev_name(nhi_fwnode->dev));
+
+	return 0;
+}
+
 /*
  * Private to usb-acpi, all the core needs to know is that
  * port_dev->location is non-zero when it has been set by the firmware.
@@ -262,6 +309,11 @@ usb_acpi_find_companion_for_device(struct usb_device *udev)
 	if (!hub)
 		return NULL;
 
+
+	/* Tunneled devices depend on USB4 Host Interface, set device link to it */
+	if (udev->tunneled && !udev->parent->parent)
+		usb_acpi_add_usb4_devlink(udev);
+
 	/*
 	 * This is an embedded USB device connected to a port and such
 	 * devices share port's ACPI companion.
-- 
2.25.1

[PATCH 4/4] thunderbolt: Don't create device link from USB4 Host Interface to USB3 xHC host

[Mathias Nyman]

USB core will create device links between tunneled USB3 devices and
USB4 Host Interface during USB device creation.

Those device links are removed with the tunneled USB3 devices, allowing
USB4 Host Interface to runtime suspend if USB3 tunnels are not used.

So remove device link creation between USB4 Host Interface and USB3 xHC
during NHI probe

Reported-by: Rajaram Regupathy <rajaram.regupathy@intel.com>
Reported-by: Saranya Gopal <saranya.gopal@intel.com>
Tested-by: Saranya Gopal <saranya.gopal@intel.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/thunderbolt/acpi.c | 40 ++++++++++----------------------------
 1 file changed, 10 insertions(+), 30 deletions(-)

diff --git a/drivers/thunderbolt/acpi.c b/drivers/thunderbolt/acpi.c
index c9b6bb46111c..d2a0054217da 100644
--- a/drivers/thunderbolt/acpi.c
+++ b/drivers/thunderbolt/acpi.c
@@ -32,40 +32,20 @@ static acpi_status tb_acpi_add_link(acpi_handle handle, u32 level, void *data,
 		goto out_put;
 
 	/*
-	 * Try to find physical device walking upwards to the hierarcy.
-	 * We need to do this because the xHCI driver might not yet be
-	 * bound so the USB3 SuperSpeed ports are not yet created.
+	 * Ignore USB3 ports here as USB core will set up device links between
+	 * tunneled USB3 devices and NHI host during USB device creation.
+	 * USB3 ports might not even have a physical device yet if xHCI driver
+	 * isn't bound yet.
 	 */
-	do {
-		dev = acpi_get_first_physical_node(adev);
-		if (dev)
-			break;
-
-		adev = acpi_dev_parent(adev);
-	} while (adev);
-
-	/*
-	 * Check that the device is PCIe. This is because USB3
-	 * SuperSpeed ports have this property and they are not power
-	 * managed with the xHCI and the SuperSpeed hub so we create the
-	 * link from xHCI instead.
-	 */
-	while (dev && !dev_is_pci(dev))
-		dev = dev->parent;
-
-	if (!dev)
+	dev = acpi_get_first_physical_node(adev);
+	if (!dev || !dev_is_pci(dev))
 		goto out_put;
 
-	/*
-	 * Check that this actually matches the type of device we
-	 * expect. It should either be xHCI or PCIe root/downstream
-	 * port.
-	 */
+	/* Check that this matches a PCIe root/downstream port. */
 	pdev = to_pci_dev(dev);
-	if (pdev->class == PCI_CLASS_SERIAL_USB_XHCI ||
-	    (pci_is_pcie(pdev) &&
-		(pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
-		 pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM))) {
+	if (pci_is_pcie(pdev) &&
+	    (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ||
+	     pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM)) {
 		const struct device_link *link;
 
 		/*
-- 
2.25.1

Re: [PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[Mika Westerberg]

+CC Mario from AMD side to check that we are good and don't break
anything accidentally.

On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
> The relationship between a USB4 Host Interface providing USB3 tunnels,
> and tunneled USB3 devices is not represented in device hierarchy.
> 
> This caused issues with power managment as devices may suspend and
> resume in incorrect order.
> A device link between the USB4 Host Interface and the USB3 xHCI was
> originally added to solve this, preventing the USB4 Host Interface from
> suspending if the USB3 xHCI Host was still active.
> This unfortunately also prevents USB4 Host Interface from suspending even
> if the USB3 xHCI Host is only serving native non-tunneled USB devices.
> 
> Improve the current powermanagement situation by creating device links
> directly from tunneled USB3 devices to the USB4 Host Interface they depend
> on instead of a device link between the hosts.
> This way USB4 host may suspend when the last tunneled device is
> disconnected.
> 
> Intel xHCI hosts are capable of reporting if connected USB3 devices are
> tunneled via vendor specific capabilities.
> Use this until a standard way is available.
> 
> Mathias Nyman (4):
>   xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
>   usb: Add tunneled parameter to usb device structure
>   usb: acpi: add device link between tunneled USB3 device and USB4 Host
>     Interface
>   thunderbolt: Don't create device link from USB4 Host Interface to USB3
>     xHC host
> 
>  drivers/thunderbolt/acpi.c       | 40 ++++++------------------
>  drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
>  drivers/usb/host/xhci-ext-caps.h |  5 +++
>  drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
>  drivers/usb/host/xhci.c          | 12 ++++++++
>  drivers/usb/host/xhci.h          |  1 +
>  include/linux/usb.h              |  2 ++
>  7 files changed, 111 insertions(+), 30 deletions(-)
> 
> -- 
> 2.25.1

Re: [PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[Mario Limonciello]

[-- Attachment #1: Type: text/plain, Size: 3173 bytes --]

On 6/20/2024 01:41, Mika Westerberg wrote:
> +CC Mario from AMD side to check that we are good and don't break
> anything accidentally.
> 
> On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
>> The relationship between a USB4 Host Interface providing USB3 tunnels,
>> and tunneled USB3 devices is not represented in device hierarchy.
>>
>> This caused issues with power managment as devices may suspend and
>> resume in incorrect order.
>> A device link between the USB4 Host Interface and the USB3 xHCI was
>> originally added to solve this, preventing the USB4 Host Interface from
>> suspending if the USB3 xHCI Host was still active.
>> This unfortunately also prevents USB4 Host Interface from suspending even
>> if the USB3 xHCI Host is only serving native non-tunneled USB devices.
>>
>> Improve the current powermanagement situation by creating device links
>> directly from tunneled USB3 devices to the USB4 Host Interface they depend
>> on instead of a device link between the hosts.
>> This way USB4 host may suspend when the last tunneled device is
>> disconnected.
>>
>> Intel xHCI hosts are capable of reporting if connected USB3 devices are
>> tunneled via vendor specific capabilities.
>> Use this until a standard way is available.
>>
>> Mathias Nyman (4):
>>    xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
>>    usb: Add tunneled parameter to usb device structure
>>    usb: acpi: add device link between tunneled USB3 device and USB4 Host
>>      Interface
>>    thunderbolt: Don't create device link from USB4 Host Interface to USB3
>>      xHC host
>>
>>   drivers/thunderbolt/acpi.c       | 40 ++++++------------------
>>   drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
>>   drivers/usb/host/xhci-ext-caps.h |  5 +++
>>   drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
>>   drivers/usb/host/xhci.c          | 12 ++++++++
>>   drivers/usb/host/xhci.h          |  1 +
>>   include/linux/usb.h              |  2 ++
>>   7 files changed, 111 insertions(+), 30 deletions(-)
>>
>> -- 
>> 2.25.1

Hi Mika,

Thanks for looping me in.  Unfortunately with this is appears the XHCI 
controller link never gets created.  I've not checked functional impact 
from this, but I'd guess there "should" be some functional problems too.

I grabbed a tree output on an AMD Phoenix system using:

# tree -l /sys/bus/pci/drivers/thunderbolt/ -L 5

I'll attach the before/after patch.  Also here are the relevant devices 
from lspci -nn output:

00:03.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Family 19h 
USB4/Thunderbolt PCIe tunnel [1022:14ef]
00:04.1 PCI bridge [0604]: Advanced Micro Devices, Inc. [AMD] Family 19h 
USB4/Thunderbolt PCIe tunnel [1022:14ef]
.
.
.
c4:00.3 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:15c0]
c4:00.4 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Device 
[1022:15c1]
c4:00.5 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Pink 
Sardine USB4/Thunderbolt NHI controller #1 [1022:1668]
c4:00.6 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Pink 
Sardine USB4/Thunderbolt NHI controller #2 [1022:1669]

[-- Attachment #2: tree_after.txt --]
[-- Type: text/plain, Size: 22003 bytes --]

/sys/bus/pci/drivers/thunderbolt/
├── 0000:c4:00.5 -> ../../../../devices/pci0000:00/0000:00:08.3/0000:c4:00.5
│   ├── ari_enabled
│   ├── broken_parity_status
│   ├── class
│   ├── config
│   ├── consistent_dma_mask_bits
│   ├── consumer:pci:0000:00:03.1 -> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1
│   │   ├── auto_remove_on
│   │   ├── consumer -> ../../../pci0000:00/0000:00:03.1
│   │   │   ├── 0000:00:03.1:pcie001
│   │   │   │   ├── driver -> ../../../../bus/pci_express/drivers/pcie_pme
│   │   │   │   ├── power
│   │   │   │   ├── subsystem -> ../../../../bus/pci_express
│   │   │   │   └── uevent
│   │   │   ├── 0000:00:03.1:pcie004
│   │   │   │   ├── driver -> ../../../../bus/pci_express/drivers/pciehp
│   │   │   │   ├── power
│   │   │   │   ├── subsystem -> ../../../../bus/pci_express  [recursive, not followed]
│   │   │   │   └── uevent
│   │   │   ├── aer_dev_correctable
│   │   │   ├── aer_dev_fatal
│   │   │   ├── aer_dev_nonfatal
│   │   │   ├── aer_rootport_total_err_cor
│   │   │   ├── aer_rootport_total_err_fatal
│   │   │   ├── aer_rootport_total_err_nonfatal
│   │   │   ├── ari_enabled
│   │   │   ├── broken_parity_status
│   │   │   ├── class
│   │   │   ├── config
│   │   │   ├── consistent_dma_mask_bits
│   │   │   ├── current_link_speed
│   │   │   ├── current_link_width
│   │   │   ├── d3cold_allowed
│   │   │   ├── device
│   │   │   ├── dma_mask_bits
│   │   │   ├── driver -> ../../../bus/pci/drivers/pcieport
│   │   │   │   ├── 0000:00:01.3 -> ../../../../devices/pci0000:00/0000:00:01.3
│   │   │   │   ├── 0000:00:02.1 -> ../../../../devices/pci0000:00/0000:00:02.1
│   │   │   │   ├── 0000:00:02.4 -> ../../../../devices/pci0000:00/0000:00:02.4
│   │   │   │   ├── 0000:00:03.1 -> ../../../../devices/pci0000:00/0000:00:03.1  [recursive, not followed]
│   │   │   │   ├── 0000:00:04.1 -> ../../../../devices/pci0000:00/0000:00:04.1
│   │   │   │   ├── 0000:00:08.1 -> ../../../../devices/pci0000:00/0000:00:08.1
│   │   │   │   ├── 0000:00:08.2 -> ../../../../devices/pci0000:00/0000:00:08.2
│   │   │   │   ├── 0000:00:08.3 -> ../../../../devices/pci0000:00/0000:00:08.3
│   │   │   │   ├── bind
│   │   │   │   ├── new_id
│   │   │   │   ├── remove_id
│   │   │   │   ├── uevent
│   │   │   │   └── unbind
│   │   │   ├── driver_override
│   │   │   ├── enable
│   │   │   ├── firmware_node -> ../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:13
│   │   │   │   ├── adr
│   │   │   │   ├── device:14
│   │   │   │   ├── LNXPOWER:04
│   │   │   │   ├── path
│   │   │   │   ├── physical_node -> ../../../../pci0000:00/0000:00:03.1  [recursive, not followed]
│   │   │   │   ├── power
│   │   │   │   ├── power_resources_D0
│   │   │   │   ├── power_resources_D3hot
│   │   │   │   ├── power_state
│   │   │   │   ├── real_power_state
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../../bus/acpi
│   │   │   │   ├── uevent
│   │   │   │   └── wakeup
│   │   │   ├── iommu -> ../0000:00:00.2/iommu/ivhd0
│   │   │   │   ├── amd-iommu
│   │   │   │   ├── device -> ../../../0000:00:00.2
│   │   │   │   ├── devices
│   │   │   │   ├── power
│   │   │   │   ├── subsystem -> ../../../../../class/iommu
│   │   │   │   └── uevent
│   │   │   ├── iommu_group -> ../../../kernel/iommu_groups/5
│   │   │   │   ├── devices
│   │   │   │   ├── reserved_regions
│   │   │   │   └── type
│   │   │   ├── irq
│   │   │   ├── link
│   │   │   ├── local_cpulist
│   │   │   ├── local_cpus
│   │   │   ├── max_link_speed
│   │   │   ├── max_link_width
│   │   │   ├── modalias
│   │   │   ├── msi_bus
│   │   │   ├── msi_irqs
│   │   │   │   └── 41
│   │   │   ├── numa_node
│   │   │   ├── pci_bus
│   │   │   │   └── 0000:04
│   │   │   ├── power
│   │   │   │   ├── async
│   │   │   │   ├── autosuspend_delay_ms
│   │   │   │   ├── control
│   │   │   │   ├── runtime_active_kids
│   │   │   │   ├── runtime_active_time
│   │   │   │   ├── runtime_enabled
│   │   │   │   ├── runtime_status
│   │   │   │   ├── runtime_suspended_time
│   │   │   │   ├── runtime_usage
│   │   │   │   ├── wakeup
│   │   │   │   ├── wakeup_abort_count
│   │   │   │   ├── wakeup_active
│   │   │   │   ├── wakeup_active_count
│   │   │   │   ├── wakeup_count
│   │   │   │   ├── wakeup_expire_count
│   │   │   │   ├── wakeup_last_time_ms
│   │   │   │   ├── wakeup_max_time_ms
│   │   │   │   └── wakeup_total_time_ms
│   │   │   ├── power_state
│   │   │   ├── remove
│   │   │   ├── rescan
│   │   │   ├── reset
│   │   │   ├── reset_method
│   │   │   ├── resource
│   │   │   ├── revision
│   │   │   ├── secondary_bus_number
│   │   │   ├── subordinate_bus_number
│   │   │   ├── subsystem -> ../../../bus/pci
│   │   │   │   ├── devices
│   │   │   │   ├── drivers
│   │   │   │   ├── drivers_autoprobe
│   │   │   │   ├── drivers_probe
│   │   │   │   ├── rescan
│   │   │   │   ├── resource_alignment
│   │   │   │   ├── slots
│   │   │   │   └── uevent
│   │   │   ├── subsystem_device
│   │   │   ├── subsystem_vendor
│   │   │   ├── supplier:pci:0000:c4:00.5 -> ../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1  [recursive, not followed]
│   │   │   ├── uevent
│   │   │   ├── vendor
│   │   │   └── wakeup
│   │   │       └── wakeup6
│   │   ├── runtime_pm
│   │   ├── status
│   │   ├── subsystem -> ../../../../class/devlink
│   │   │   ├── pci:0000:c2:00.0--pci:0000:c2:00.1 -> ../../devices/virtual/devlink/pci:0000:c2:00.0--pci:0000:c2:00.1
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── consumer -> ../../../pci0000:00/0000:00:08.1/0000:c2:00.1
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── supplier -> ../../../pci0000:00/0000:00:08.1/0000:c2:00.0
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   ├── pci:0000:c4:00.5--pci:0000:00:03.1 -> ../../devices/virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1  [recursive, not followed]
│   │   │   ├── pci:0000:c4:00.6--pci:0000:00:04.1 -> ../../devices/virtual/devlink/pci:0000:c4:00.6--pci:0000:00:04.1
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── consumer -> ../../../pci0000:00/0000:00:04.1  [recursive, not followed]
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── supplier -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.6
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   ├── platform:PNP0C14:00--wmi:05901221-D566-11D1-B2F0-00A0C9062910 -> ../../devices/virtual/devlink/platform:PNP0C14:00--wmi:05901221-D566-11D1-B2F0-00A0C9062910
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   └── platform:PNP0C14:00--wmi:ABBC0F6A-8EA1-11D1-00A0-C90629100000 -> ../../devices/virtual/devlink/platform:PNP0C14:00--wmi:ABBC0F6A-8EA1-11D1-00A0-C90629100000
│   │   │       ├── auto_remove_on
│   │   │       ├── runtime_pm
│   │   │       ├── status
│   │   │       ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │       ├── sync_state_only
│   │   │       └── uevent
│   │   ├── supplier -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   ├── sync_state_only
│   │   └── uevent
│   ├── current_link_speed
│   ├── current_link_width
│   ├── d3cold_allowed
│   ├── device
│   ├── dma_mask_bits
│   ├── domain0
│   │   ├── 0-0
│   │   │   ├── authorized
│   │   │   ├── generation
│   │   │   ├── power
│   │   │   │   ├── async
│   │   │   │   ├── autosuspend_delay_ms
│   │   │   │   ├── control
│   │   │   │   ├── runtime_active_kids
│   │   │   │   ├── runtime_active_time
│   │   │   │   ├── runtime_enabled
│   │   │   │   ├── runtime_status
│   │   │   │   ├── runtime_suspended_time
│   │   │   │   ├── runtime_usage
│   │   │   │   ├── wakeup
│   │   │   │   ├── wakeup_abort_count
│   │   │   │   ├── wakeup_active
│   │   │   │   ├── wakeup_active_count
│   │   │   │   ├── wakeup_count
│   │   │   │   ├── wakeup_expire_count
│   │   │   │   ├── wakeup_last_time_ms
│   │   │   │   ├── wakeup_max_time_ms
│   │   │   │   └── wakeup_total_time_ms
│   │   │   ├── subsystem -> ../../../../../../bus/thunderbolt
│   │   │   │   ├── devices
│   │   │   │   ├── drivers
│   │   │   │   ├── drivers_autoprobe
│   │   │   │   ├── drivers_probe
│   │   │   │   └── uevent
│   │   │   ├── uevent
│   │   │   ├── unique_id
│   │   │   ├── usb4_port2
│   │   │   │   ├── link
│   │   │   │   ├── power
│   │   │   │   └── uevent
│   │   │   └── wakeup
│   │   │       └── wakeup47
│   │   ├── deauthorization
│   │   ├── iommu_dma_protection
│   │   ├── power
│   │   │   ├── async
│   │   │   ├── runtime_active_kids
│   │   │   ├── runtime_enabled
│   │   │   ├── runtime_status
│   │   │   ├── runtime_usage
│   │   │   ├── wakeup
│   │   │   ├── wakeup_abort_count
│   │   │   ├── wakeup_active
│   │   │   ├── wakeup_active_count
│   │   │   ├── wakeup_count
│   │   │   ├── wakeup_expire_count
│   │   │   ├── wakeup_last_time_ms
│   │   │   ├── wakeup_max_time_ms
│   │   │   └── wakeup_total_time_ms
│   │   ├── security
│   │   ├── subsystem -> ../../../../../bus/thunderbolt  [recursive, not followed]
│   │   ├── uevent
│   │   └── wakeup
│   │       └── wakeup48
│   │           ├── active_count
│   │           ├── active_time_ms
│   │           ├── device -> ../../../domain0  [recursive, not followed]
│   │           ├── event_count
│   │           ├── expire_count
│   │           ├── last_change_ms
│   │           ├── max_time_ms
│   │           ├── name
│   │           ├── prevent_suspend_time_ms
│   │           ├── subsystem -> ../../../../../../../class/wakeup
│   │           ├── total_time_ms
│   │           ├── uevent
│   │           └── wakeup_count
│   ├── driver -> ../../../../bus/pci/drivers/thunderbolt  [recursive, not followed]
│   ├── driver_override
│   ├── enable
│   ├── firmware_node -> ../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:36/device:3b
│   │   ├── adr
│   │   ├── path
│   │   ├── physical_node -> ../../../../../pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   ├── power
│   │   │   ├── async
│   │   │   ├── autosuspend_delay_ms
│   │   │   ├── control
│   │   │   ├── runtime_active_kids
│   │   │   ├── runtime_active_time
│   │   │   ├── runtime_enabled
│   │   │   ├── runtime_status
│   │   │   ├── runtime_suspended_time
│   │   │   └── runtime_usage
│   │   ├── power_resources_D0
│   │   │   └── LNXPOWER:0f -> ../../LNXPOWER:0f
│   │   │       ├── hid
│   │   │       ├── modalias
│   │   │       ├── path
│   │   │       ├── power
│   │   │       ├── resource_in_use
│   │   │       ├── status
│   │   │       ├── subsystem -> ../../../../../../bus/acpi  [recursive, not followed]
│   │   │       └── uevent
│   │   ├── power_resources_D3hot
│   │   │   └── LNXPOWER:0f -> ../../LNXPOWER:0f  [recursive, not followed]
│   │   ├── power_state
│   │   ├── real_power_state
│   │   ├── status
│   │   ├── subsystem -> ../../../../../../bus/acpi  [recursive, not followed]
│   │   ├── uevent
│   │   └── wakeup
│   │       └── wakeup29
│   │           ├── active_count
│   │           ├── active_time_ms
│   │           ├── device -> ../../../device:3b  [recursive, not followed]
│   │           ├── event_count
│   │           ├── expire_count
│   │           ├── last_change_ms
│   │           ├── max_time_ms
│   │           ├── name
│   │           ├── prevent_suspend_time_ms
│   │           ├── subsystem -> ../../../../../../../../class/wakeup  [recursive, not followed]
│   │           ├── total_time_ms
│   │           ├── uevent
│   │           └── wakeup_count
│   ├── iommu -> ../../0000:00:00.2/iommu/ivhd0  [recursive, not followed]
│   ├── iommu_group -> ../../../../kernel/iommu_groups/29
│   │   ├── devices
│   │   │   └── 0000:c4:00.5 -> ../../../../devices/pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   ├── reserved_regions
│   │   └── type
│   ├── irq
│   ├── link
│   │   ├── l0s_aspm
│   │   └── l1_aspm
│   ├── local_cpulist
│   ├── local_cpus
│   ├── max_link_speed
│   ├── max_link_width
│   ├── modalias
│   ├── msi_bus
│   ├── msi_irqs
│   │   ├── 50
│   │   ├── 51
│   │   ├── 53
│   │   ├── 54
│   │   ├── 55
│   │   ├── 56
│   │   ├── 57
│   │   ├── 58
│   │   ├── 59
│   │   ├── 61
│   │   ├── 62
│   │   ├── 64
│   │   ├── 65
│   │   ├── 66
│   │   ├── 67
│   │   └── 68
│   ├── numa_node
│   ├── pools
│   ├── power
│   │   ├── async
│   │   ├── autosuspend_delay_ms
│   │   ├── control
│   │   ├── runtime_active_kids
│   │   ├── runtime_active_time
│   │   ├── runtime_enabled
│   │   ├── runtime_status
│   │   ├── runtime_suspended_time
│   │   ├── runtime_usage
│   │   ├── wakeup
│   │   ├── wakeup_abort_count
│   │   ├── wakeup_active
│   │   ├── wakeup_active_count
│   │   ├── wakeup_count
│   │   ├── wakeup_expire_count
│   │   ├── wakeup_last_time_ms
│   │   ├── wakeup_max_time_ms
│   │   └── wakeup_total_time_ms
│   ├── power_state
│   ├── remove
│   ├── rescan
│   ├── reset
│   ├── reset_method
│   ├── resource
│   ├── resource0
│   ├── revision
│   ├── subsystem -> ../../../../bus/pci  [recursive, not followed]
│   ├── subsystem_device
│   ├── subsystem_vendor
│   ├── uevent
│   ├── vendor
│   └── wakeup
│       └── wakeup49
│           ├── active_count
│           ├── active_time_ms
│           ├── device -> ../../../0000:c4:00.5  [recursive, not followed]
│           ├── event_count
│           ├── expire_count
│           ├── last_change_ms
│           ├── max_time_ms
│           ├── name
│           ├── prevent_suspend_time_ms
│           ├── subsystem -> ../../../../../../class/wakeup  [recursive, not followed]
│           ├── total_time_ms
│           ├── uevent
│           └── wakeup_count
├── 0000:c4:00.6 -> ../../../../devices/pci0000:00/0000:00:08.3/0000:c4:00.6  [recursive, not followed]
├── bind
├── module -> ../../../../module/thunderbolt
│   ├── coresize
│   ├── drivers
│   │   └── pci:thunderbolt -> ../../../bus/pci/drivers/thunderbolt  [recursive, not followed]
│   ├── holders
│   ├── initsize
│   ├── initstate
│   ├── notes
│   ├── parameters
│   │   ├── asym_threshold
│   │   ├── bw_alloc_mode
│   │   ├── clx
│   │   ├── dma_credits
│   │   ├── host_reset
│   │   ├── start_icm
│   │   └── xdomain
│   ├── refcnt
│   ├── sections
│   │   ├── __bpf_raw_tp_map
│   │   ├── __bug_table
│   │   ├── __dyndbg
│   │   ├── _ftrace_events
│   │   ├── __jump_table
│   │   ├── __kcrctab_gpl
│   │   ├── __ksymtab_gpl
│   │   ├── __ksymtab_strings
│   │   ├── __mcount_loc
│   │   ├── __param
│   │   ├── __patchable_function_entries
│   │   ├── __tracepoints
│   │   ├── __tracepoints_ptrs
│   │   └── __tracepoints_strings
│   ├── srcversion
│   ├── taint
│   └── uevent
├── new_id
├── remove_id
├── uevent
└── unbind

116 directories, 307 files

[-- Attachment #3: tree_before.txt --]
[-- Type: text/plain, Size: 22503 bytes --]

/sys/bus/pci/drivers/thunderbolt/
├── 0000:c4:00.5 -> ../../../../devices/pci0000:00/0000:00:08.3/0000:c4:00.5
│   ├── ari_enabled
│   ├── broken_parity_status
│   ├── class
│   ├── config
│   ├── consistent_dma_mask_bits
│   ├── consumer:pci:0000:00:03.1 -> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1
│   │   ├── auto_remove_on
│   │   ├── consumer -> ../../../pci0000:00/0000:00:03.1
│   │   │   ├── 0000:00:03.1:pcie001
│   │   │   │   ├── driver -> ../../../../bus/pci_express/drivers/pcie_pme
│   │   │   │   ├── power
│   │   │   │   ├── subsystem -> ../../../../bus/pci_express
│   │   │   │   └── uevent
│   │   │   ├── 0000:00:03.1:pcie004
│   │   │   │   ├── driver -> ../../../../bus/pci_express/drivers/pciehp
│   │   │   │   ├── power
│   │   │   │   ├── subsystem -> ../../../../bus/pci_express  [recursive, not followed]
│   │   │   │   └── uevent
│   │   │   ├── aer_dev_correctable
│   │   │   ├── aer_dev_fatal
│   │   │   ├── aer_dev_nonfatal
│   │   │   ├── aer_rootport_total_err_cor
│   │   │   ├── aer_rootport_total_err_fatal
│   │   │   ├── aer_rootport_total_err_nonfatal
│   │   │   ├── ari_enabled
│   │   │   ├── broken_parity_status
│   │   │   ├── class
│   │   │   ├── config
│   │   │   ├── consistent_dma_mask_bits
│   │   │   ├── current_link_speed
│   │   │   ├── current_link_width
│   │   │   ├── d3cold_allowed
│   │   │   ├── device
│   │   │   ├── dma_mask_bits
│   │   │   ├── driver -> ../../../bus/pci/drivers/pcieport
│   │   │   │   ├── 0000:00:01.3 -> ../../../../devices/pci0000:00/0000:00:01.3
│   │   │   │   ├── 0000:00:02.1 -> ../../../../devices/pci0000:00/0000:00:02.1
│   │   │   │   ├── 0000:00:02.4 -> ../../../../devices/pci0000:00/0000:00:02.4
│   │   │   │   ├── 0000:00:03.1 -> ../../../../devices/pci0000:00/0000:00:03.1  [recursive, not followed]
│   │   │   │   ├── 0000:00:04.1 -> ../../../../devices/pci0000:00/0000:00:04.1
│   │   │   │   ├── 0000:00:08.1 -> ../../../../devices/pci0000:00/0000:00:08.1
│   │   │   │   ├── 0000:00:08.2 -> ../../../../devices/pci0000:00/0000:00:08.2
│   │   │   │   ├── 0000:00:08.3 -> ../../../../devices/pci0000:00/0000:00:08.3
│   │   │   │   ├── bind
│   │   │   │   ├── new_id
│   │   │   │   ├── remove_id
│   │   │   │   ├── uevent
│   │   │   │   └── unbind
│   │   │   ├── driver_override
│   │   │   ├── enable
│   │   │   ├── firmware_node -> ../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:13
│   │   │   │   ├── adr
│   │   │   │   ├── device:14
│   │   │   │   ├── LNXPOWER:04
│   │   │   │   ├── path
│   │   │   │   ├── physical_node -> ../../../../pci0000:00/0000:00:03.1  [recursive, not followed]
│   │   │   │   ├── power
│   │   │   │   ├── power_resources_D0
│   │   │   │   ├── power_resources_D3hot
│   │   │   │   ├── power_state
│   │   │   │   ├── real_power_state
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../../bus/acpi
│   │   │   │   ├── uevent
│   │   │   │   └── wakeup
│   │   │   ├── iommu -> ../0000:00:00.2/iommu/ivhd0
│   │   │   │   ├── amd-iommu
│   │   │   │   ├── device -> ../../../0000:00:00.2
│   │   │   │   ├── devices
│   │   │   │   ├── power
│   │   │   │   ├── subsystem -> ../../../../../class/iommu
│   │   │   │   └── uevent
│   │   │   ├── iommu_group -> ../../../kernel/iommu_groups/5
│   │   │   │   ├── devices
│   │   │   │   ├── reserved_regions
│   │   │   │   └── type
│   │   │   ├── irq
│   │   │   ├── link
│   │   │   ├── local_cpulist
│   │   │   ├── local_cpus
│   │   │   ├── max_link_speed
│   │   │   ├── max_link_width
│   │   │   ├── modalias
│   │   │   ├── msi_bus
│   │   │   ├── msi_irqs
│   │   │   │   └── 41
│   │   │   ├── numa_node
│   │   │   ├── pci_bus
│   │   │   │   └── 0000:04
│   │   │   ├── power
│   │   │   │   ├── autosuspend_delay_ms
│   │   │   │   ├── control
│   │   │   │   ├── runtime_active_time
│   │   │   │   ├── runtime_status
│   │   │   │   ├── runtime_suspended_time
│   │   │   │   ├── wakeup
│   │   │   │   ├── wakeup_abort_count
│   │   │   │   ├── wakeup_active
│   │   │   │   ├── wakeup_active_count
│   │   │   │   ├── wakeup_count
│   │   │   │   ├── wakeup_expire_count
│   │   │   │   ├── wakeup_last_time_ms
│   │   │   │   ├── wakeup_max_time_ms
│   │   │   │   └── wakeup_total_time_ms
│   │   │   ├── power_state
│   │   │   ├── remove
│   │   │   ├── rescan
│   │   │   ├── reset
│   │   │   ├── reset_method
│   │   │   ├── resource
│   │   │   ├── revision
│   │   │   ├── secondary_bus_number
│   │   │   ├── subordinate_bus_number
│   │   │   ├── subsystem -> ../../../bus/pci
│   │   │   │   ├── devices
│   │   │   │   ├── drivers
│   │   │   │   ├── drivers_autoprobe
│   │   │   │   ├── drivers_probe
│   │   │   │   ├── rescan
│   │   │   │   ├── resource_alignment
│   │   │   │   ├── slots
│   │   │   │   └── uevent
│   │   │   ├── subsystem_device
│   │   │   ├── subsystem_vendor
│   │   │   ├── supplier:pci:0000:c4:00.5 -> ../../virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1  [recursive, not followed]
│   │   │   ├── uevent
│   │   │   ├── vendor
│   │   │   └── wakeup
│   │   │       └── wakeup6
│   │   ├── runtime_pm
│   │   ├── status
│   │   ├── subsystem -> ../../../../class/devlink
│   │   │   ├── pci:0000:c2:00.0--pci:0000:c2:00.1 -> ../../devices/virtual/devlink/pci:0000:c2:00.0--pci:0000:c2:00.1
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── consumer -> ../../../pci0000:00/0000:00:08.1/0000:c2:00.1
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── supplier -> ../../../pci0000:00/0000:00:08.1/0000:c2:00.0
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   ├── pci:0000:c4:00.5--pci:0000:00:03.1 -> ../../devices/virtual/devlink/pci:0000:c4:00.5--pci:0000:00:03.1  [recursive, not followed]
│   │   │   ├── pci:0000:c4:00.5--pci:0000:c4:00.3 -> ../../devices/virtual/devlink/pci:0000:c4:00.5--pci:0000:c4:00.3
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── consumer -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.3
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── supplier -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   ├── pci:0000:c4:00.6--pci:0000:00:04.1 -> ../../devices/virtual/devlink/pci:0000:c4:00.6--pci:0000:00:04.1
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── consumer -> ../../../pci0000:00/0000:00:04.1  [recursive, not followed]
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── supplier -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.6
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   ├── pci:0000:c4:00.6--pci:0000:c4:00.4 -> ../../devices/virtual/devlink/pci:0000:c4:00.6--pci:0000:c4:00.4
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── consumer -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.4
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── supplier -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.6  [recursive, not followed]
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   ├── platform:PNP0C14:00--wmi:05901221-D566-11D1-B2F0-00A0C9062910 -> ../../devices/virtual/devlink/platform:PNP0C14:00--wmi:05901221-D566-11D1-B2F0-00A0C9062910
│   │   │   │   ├── auto_remove_on
│   │   │   │   ├── runtime_pm
│   │   │   │   ├── status
│   │   │   │   ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │   │   ├── sync_state_only
│   │   │   │   └── uevent
│   │   │   └── platform:PNP0C14:00--wmi:ABBC0F6A-8EA1-11D1-00A0-C90629100000 -> ../../devices/virtual/devlink/platform:PNP0C14:00--wmi:ABBC0F6A-8EA1-11D1-00A0-C90629100000
│   │   │       ├── auto_remove_on
│   │   │       ├── runtime_pm
│   │   │       ├── status
│   │   │       ├── subsystem -> ../../../../class/devlink  [recursive, not followed]
│   │   │       ├── sync_state_only
│   │   │       └── uevent
│   │   ├── supplier -> ../../../pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   ├── sync_state_only
│   │   └── uevent
│   ├── consumer:pci:0000:c4:00.3 -> ../../../virtual/devlink/pci:0000:c4:00.5--pci:0000:c4:00.3  [recursive, not followed]
│   ├── current_link_speed
│   ├── current_link_width
│   ├── d3cold_allowed
│   ├── device
│   ├── dma_mask_bits
│   ├── domain0
│   │   ├── 0-0
│   │   │   ├── authorized
│   │   │   ├── generation
│   │   │   ├── power
│   │   │   │   ├── autosuspend_delay_ms
│   │   │   │   ├── control
│   │   │   │   ├── runtime_active_time
│   │   │   │   ├── runtime_status
│   │   │   │   ├── runtime_suspended_time
│   │   │   │   ├── wakeup
│   │   │   │   ├── wakeup_abort_count
│   │   │   │   ├── wakeup_active
│   │   │   │   ├── wakeup_active_count
│   │   │   │   ├── wakeup_count
│   │   │   │   ├── wakeup_expire_count
│   │   │   │   ├── wakeup_last_time_ms
│   │   │   │   ├── wakeup_max_time_ms
│   │   │   │   └── wakeup_total_time_ms
│   │   │   ├── subsystem -> ../../../../../../bus/thunderbolt
│   │   │   │   ├── devices
│   │   │   │   ├── drivers
│   │   │   │   ├── drivers_autoprobe
│   │   │   │   ├── drivers_probe
│   │   │   │   └── uevent
│   │   │   ├── uevent
│   │   │   ├── unique_id
│   │   │   ├── usb4_port2
│   │   │   │   ├── link
│   │   │   │   ├── power
│   │   │   │   └── uevent
│   │   │   └── wakeup
│   │   │       └── wakeup50
│   │   ├── deauthorization
│   │   ├── iommu_dma_protection
│   │   ├── power
│   │   │   ├── wakeup
│   │   │   ├── wakeup_abort_count
│   │   │   ├── wakeup_active
│   │   │   ├── wakeup_active_count
│   │   │   ├── wakeup_count
│   │   │   ├── wakeup_expire_count
│   │   │   ├── wakeup_last_time_ms
│   │   │   ├── wakeup_max_time_ms
│   │   │   └── wakeup_total_time_ms
│   │   ├── security
│   │   ├── subsystem -> ../../../../../bus/thunderbolt  [recursive, not followed]
│   │   ├── uevent
│   │   └── wakeup
│   │       └── wakeup51
│   │           ├── active_count
│   │           ├── active_time_ms
│   │           ├── device -> ../../../domain0  [recursive, not followed]
│   │           ├── event_count
│   │           ├── expire_count
│   │           ├── last_change_ms
│   │           ├── max_time_ms
│   │           ├── name
│   │           ├── prevent_suspend_time_ms
│   │           ├── subsystem -> ../../../../../../../class/wakeup
│   │           ├── total_time_ms
│   │           ├── uevent
│   │           └── wakeup_count
│   ├── driver -> ../../../../bus/pci/drivers/thunderbolt  [recursive, not followed]
│   ├── driver_override
│   ├── enable
│   ├── firmware_node -> ../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:36/device:3b
│   │   ├── adr
│   │   ├── path
│   │   ├── physical_node -> ../../../../../pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   ├── power
│   │   │   ├── autosuspend_delay_ms
│   │   │   ├── control
│   │   │   ├── runtime_active_time
│   │   │   ├── runtime_status
│   │   │   └── runtime_suspended_time
│   │   ├── power_resources_D0
│   │   │   └── LNXPOWER:0f -> ../../LNXPOWER:0f
│   │   │       ├── hid
│   │   │       ├── modalias
│   │   │       ├── path
│   │   │       ├── power
│   │   │       ├── resource_in_use
│   │   │       ├── status
│   │   │       ├── subsystem -> ../../../../../../bus/acpi  [recursive, not followed]
│   │   │       └── uevent
│   │   ├── power_resources_D3hot
│   │   │   └── LNXPOWER:0f -> ../../LNXPOWER:0f  [recursive, not followed]
│   │   ├── power_state
│   │   ├── real_power_state
│   │   ├── status
│   │   ├── subsystem -> ../../../../../../bus/acpi  [recursive, not followed]
│   │   ├── uevent
│   │   └── wakeup
│   │       └── wakeup29
│   │           ├── active_count
│   │           ├── active_time_ms
│   │           ├── device -> ../../../device:3b  [recursive, not followed]
│   │           ├── event_count
│   │           ├── expire_count
│   │           ├── last_change_ms
│   │           ├── max_time_ms
│   │           ├── name
│   │           ├── prevent_suspend_time_ms
│   │           ├── subsystem -> ../../../../../../../../class/wakeup  [recursive, not followed]
│   │           ├── total_time_ms
│   │           ├── uevent
│   │           └── wakeup_count
│   ├── iommu -> ../../0000:00:00.2/iommu/ivhd0  [recursive, not followed]
│   ├── iommu_group -> ../../../../kernel/iommu_groups/29
│   │   ├── devices
│   │   │   └── 0000:c4:00.5 -> ../../../../devices/pci0000:00/0000:00:08.3/0000:c4:00.5  [recursive, not followed]
│   │   ├── reserved_regions
│   │   └── type
│   ├── irq
│   ├── link
│   │   ├── l0s_aspm
│   │   └── l1_aspm
│   ├── local_cpulist
│   ├── local_cpus
│   ├── max_link_speed
│   ├── max_link_width
│   ├── modalias
│   ├── msi_bus
│   ├── msi_irqs
│   │   ├── 57
│   │   ├── 59
│   │   ├── 60
│   │   ├── 61
│   │   ├── 62
│   │   ├── 63
│   │   ├── 64
│   │   ├── 65
│   │   ├── 66
│   │   ├── 67
│   │   ├── 68
│   │   ├── 69
│   │   ├── 70
│   │   ├── 71
│   │   ├── 72
│   │   └── 73
│   ├── numa_node
│   ├── pools
│   ├── power
│   │   ├── autosuspend_delay_ms
│   │   ├── control
│   │   ├── runtime_active_time
│   │   ├── runtime_status
│   │   ├── runtime_suspended_time
│   │   ├── wakeup
│   │   ├── wakeup_abort_count
│   │   ├── wakeup_active
│   │   ├── wakeup_active_count
│   │   ├── wakeup_count
│   │   ├── wakeup_expire_count
│   │   ├── wakeup_last_time_ms
│   │   ├── wakeup_max_time_ms
│   │   └── wakeup_total_time_ms
│   ├── power_state
│   ├── remove
│   ├── rescan
│   ├── reset
│   ├── reset_method
│   ├── resource
│   ├── resource0
│   ├── revision
│   ├── subsystem -> ../../../../bus/pci  [recursive, not followed]
│   ├── subsystem_device
│   ├── subsystem_vendor
│   ├── uevent
│   ├── vendor
│   └── wakeup
│       └── wakeup52
│           ├── active_count
│           ├── active_time_ms
│           ├── device -> ../../../0000:c4:00.5  [recursive, not followed]
│           ├── event_count
│           ├── expire_count
│           ├── last_change_ms
│           ├── max_time_ms
│           ├── name
│           ├── prevent_suspend_time_ms
│           ├── subsystem -> ../../../../../../class/wakeup  [recursive, not followed]
│           ├── total_time_ms
│           ├── uevent
│           └── wakeup_count
├── 0000:c4:00.6 -> ../../../../devices/pci0000:00/0000:00:08.3/0000:c4:00.6  [recursive, not followed]
├── bind
├── module -> ../../../../module/thunderbolt
│   ├── coresize
│   ├── drivers
│   │   └── pci:thunderbolt -> ../../../bus/pci/drivers/thunderbolt  [recursive, not followed]
│   ├── holders
│   ├── initsize
│   ├── initstate
│   ├── notes
│   ├── parameters
│   │   ├── asym_threshold
│   │   ├── bw_alloc_mode
│   │   ├── clx
│   │   ├── dma_credits
│   │   ├── host_reset
│   │   ├── start_icm
│   │   └── xdomain
│   ├── refcnt
│   ├── sections
│   │   ├── __bpf_raw_tp_map
│   │   ├── __bug_table
│   │   ├── __dyndbg
│   │   ├── _ftrace_events
│   │   ├── __jump_table
│   │   ├── __ksymtab_gpl
│   │   ├── __ksymtab_strings
│   │   ├── __mcount_loc
│   │   ├── __param
│   │   ├── __patchable_function_entries
│   │   ├── __tracepoints
│   │   ├── __tracepoints_ptrs
│   │   └── __tracepoints_strings
│   ├── taint
│   └── uevent
├── new_id
├── remove_id
├── uevent
└── unbind

125 directories, 294 files

Re: [PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[Mika Westerberg]

Hi Mario,

On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
> On 6/20/2024 01:41, Mika Westerberg wrote:
> > +CC Mario from AMD side to check that we are good and don't break
> > anything accidentally.
> > 
> > On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
> > > The relationship between a USB4 Host Interface providing USB3 tunnels,
> > > and tunneled USB3 devices is not represented in device hierarchy.
> > > 
> > > This caused issues with power managment as devices may suspend and
> > > resume in incorrect order.
> > > A device link between the USB4 Host Interface and the USB3 xHCI was
> > > originally added to solve this, preventing the USB4 Host Interface from
> > > suspending if the USB3 xHCI Host was still active.
> > > This unfortunately also prevents USB4 Host Interface from suspending even
> > > if the USB3 xHCI Host is only serving native non-tunneled USB devices.
> > > 
> > > Improve the current powermanagement situation by creating device links
> > > directly from tunneled USB3 devices to the USB4 Host Interface they depend
> > > on instead of a device link between the hosts.
> > > This way USB4 host may suspend when the last tunneled device is
> > > disconnected.
> > > 
> > > Intel xHCI hosts are capable of reporting if connected USB3 devices are
> > > tunneled via vendor specific capabilities.
> > > Use this until a standard way is available.
> > > 
> > > Mathias Nyman (4):
> > >    xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
> > >    usb: Add tunneled parameter to usb device structure
> > >    usb: acpi: add device link between tunneled USB3 device and USB4 Host
> > >      Interface
> > >    thunderbolt: Don't create device link from USB4 Host Interface to USB3
> > >      xHC host
> > > 
> > >   drivers/thunderbolt/acpi.c       | 40 ++++++------------------
> > >   drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
> > >   drivers/usb/host/xhci-ext-caps.h |  5 +++
> > >   drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
> > >   drivers/usb/host/xhci.c          | 12 ++++++++
> > >   drivers/usb/host/xhci.h          |  1 +
> > >   include/linux/usb.h              |  2 ++
> > >   7 files changed, 111 insertions(+), 30 deletions(-)
> > > 
> > > -- 
> > > 2.25.1
> 
> Hi Mika,
> 
> Thanks for looping me in.  Unfortunately with this is appears the XHCI
> controller link never gets created.  I've not checked functional impact from
> this, but I'd guess there "should" be some functional problems too.

Thanks for checking!

I think the code that sets up the device link based on ->tunneled should
do that always if the hardware cannot tell if this is native or tunneled
link to keep the existing functionality and only do the "optimization"
if the hardware is capable of identifying that.

Perhaps it can be a callback provided by the xHCI controller driver
(->is_tunneled()) that then defaults to true if the
"usb4-host-interface" property is there and in case of Intel hardware
also checks the proprietary register?

Re: [PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[Mario Limonciello]

On 6/21/2024 01:19, Mika Westerberg wrote:
> Hi Mario,
> 
> On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
>> On 6/20/2024 01:41, Mika Westerberg wrote:
>>> +CC Mario from AMD side to check that we are good and don't break
>>> anything accidentally.
>>>
>>> On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
>>>> The relationship between a USB4 Host Interface providing USB3 tunnels,
>>>> and tunneled USB3 devices is not represented in device hierarchy.
>>>>
>>>> This caused issues with power managment as devices may suspend and
>>>> resume in incorrect order.
>>>> A device link between the USB4 Host Interface and the USB3 xHCI was
>>>> originally added to solve this, preventing the USB4 Host Interface from
>>>> suspending if the USB3 xHCI Host was still active.
>>>> This unfortunately also prevents USB4 Host Interface from suspending even
>>>> if the USB3 xHCI Host is only serving native non-tunneled USB devices.
>>>>
>>>> Improve the current powermanagement situation by creating device links
>>>> directly from tunneled USB3 devices to the USB4 Host Interface they depend
>>>> on instead of a device link between the hosts.
>>>> This way USB4 host may suspend when the last tunneled device is
>>>> disconnected.
>>>>
>>>> Intel xHCI hosts are capable of reporting if connected USB3 devices are
>>>> tunneled via vendor specific capabilities.
>>>> Use this until a standard way is available.
>>>>
>>>> Mathias Nyman (4):
>>>>     xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
>>>>     usb: Add tunneled parameter to usb device structure
>>>>     usb: acpi: add device link between tunneled USB3 device and USB4 Host
>>>>       Interface
>>>>     thunderbolt: Don't create device link from USB4 Host Interface to USB3
>>>>       xHC host
>>>>
>>>>    drivers/thunderbolt/acpi.c       | 40 ++++++------------------
>>>>    drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
>>>>    drivers/usb/host/xhci-ext-caps.h |  5 +++
>>>>    drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
>>>>    drivers/usb/host/xhci.c          | 12 ++++++++
>>>>    drivers/usb/host/xhci.h          |  1 +
>>>>    include/linux/usb.h              |  2 ++
>>>>    7 files changed, 111 insertions(+), 30 deletions(-)
>>>>
>>>> -- 
>>>> 2.25.1
>>
>> Hi Mika,
>>
>> Thanks for looping me in.  Unfortunately with this is appears the XHCI
>> controller link never gets created.  I've not checked functional impact from
>> this, but I'd guess there "should" be some functional problems too.
> 
> Thanks for checking!
> 
> I think the code that sets up the device link based on ->tunneled should
> do that always if the hardware cannot tell if this is native or tunneled
> link to keep the existing functionality and only do the "optimization"
> if the hardware is capable of identifying that.
> 
> Perhaps it can be a callback provided by the xHCI controller driver
> (->is_tunneled()) that then defaults to true if the
> "usb4-host-interface" property is there and in case of Intel hardware
> also checks the proprietary register?

So I think the problem is you will have an ordering dependency between 
the two drivers for when the link gets created.

Like if thunderbolt.ko loads you would end up with links to PCIe root 
port for tunneling as well as XHCI controller.

Then xhci loads and you end up also adding links to individual ports.
Would you remove the link to the controller?

And if the order is the other way around you end up with a larger state 
machine.

How about PCIe core provides a helper to know whether or not a PCIe 
device will support the proprietary register?

Then thunderbolt.ko can use this to know whether or not to make the XHCI 
link and xhci.ko can use this to know whether or not to make the port links.

Hopefully that means no ordering dependencies.

Re: [PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[Mika Westerberg]

On Fri, Jun 21, 2024 at 11:30:05AM -0500, Mario Limonciello wrote:
> On 6/21/2024 01:19, Mika Westerberg wrote:
> > Hi Mario,
> > 
> > On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
> > > On 6/20/2024 01:41, Mika Westerberg wrote:
> > > > +CC Mario from AMD side to check that we are good and don't break
> > > > anything accidentally.
> > > > 
> > > > On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
> > > > > The relationship between a USB4 Host Interface providing USB3 tunnels,
> > > > > and tunneled USB3 devices is not represented in device hierarchy.
> > > > > 
> > > > > This caused issues with power managment as devices may suspend and
> > > > > resume in incorrect order.
> > > > > A device link between the USB4 Host Interface and the USB3 xHCI was
> > > > > originally added to solve this, preventing the USB4 Host Interface from
> > > > > suspending if the USB3 xHCI Host was still active.
> > > > > This unfortunately also prevents USB4 Host Interface from suspending even
> > > > > if the USB3 xHCI Host is only serving native non-tunneled USB devices.
> > > > > 
> > > > > Improve the current powermanagement situation by creating device links
> > > > > directly from tunneled USB3 devices to the USB4 Host Interface they depend
> > > > > on instead of a device link between the hosts.
> > > > > This way USB4 host may suspend when the last tunneled device is
> > > > > disconnected.
> > > > > 
> > > > > Intel xHCI hosts are capable of reporting if connected USB3 devices are
> > > > > tunneled via vendor specific capabilities.
> > > > > Use this until a standard way is available.
> > > > > 
> > > > > Mathias Nyman (4):
> > > > >     xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
> > > > >     usb: Add tunneled parameter to usb device structure
> > > > >     usb: acpi: add device link between tunneled USB3 device and USB4 Host
> > > > >       Interface
> > > > >     thunderbolt: Don't create device link from USB4 Host Interface to USB3
> > > > >       xHC host
> > > > > 
> > > > >    drivers/thunderbolt/acpi.c       | 40 ++++++------------------
> > > > >    drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
> > > > >    drivers/usb/host/xhci-ext-caps.h |  5 +++
> > > > >    drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
> > > > >    drivers/usb/host/xhci.c          | 12 ++++++++
> > > > >    drivers/usb/host/xhci.h          |  1 +
> > > > >    include/linux/usb.h              |  2 ++
> > > > >    7 files changed, 111 insertions(+), 30 deletions(-)
> > > > > 
> > > > > -- 
> > > > > 2.25.1
> > > 
> > > Hi Mika,
> > > 
> > > Thanks for looping me in.  Unfortunately with this is appears the XHCI
> > > controller link never gets created.  I've not checked functional impact from
> > > this, but I'd guess there "should" be some functional problems too.
> > 
> > Thanks for checking!
> > 
> > I think the code that sets up the device link based on ->tunneled should
> > do that always if the hardware cannot tell if this is native or tunneled
> > link to keep the existing functionality and only do the "optimization"
> > if the hardware is capable of identifying that.
> > 
> > Perhaps it can be a callback provided by the xHCI controller driver
> > (->is_tunneled()) that then defaults to true if the
> > "usb4-host-interface" property is there and in case of Intel hardware
> > also checks the proprietary register?
> 
> So I think the problem is you will have an ordering dependency between the
> two drivers for when the link gets created.
> 
> Like if thunderbolt.ko loads you would end up with links to PCIe root port
> for tunneling as well as XHCI controller.

With this patch we only create links to PCIe Root/Downstream ports from
Thunderbolt side and the USB core will deal with the USB ones.

> Then xhci loads and you end up also adding links to individual ports.
> Would you remove the link to the controller?

See above.

> And if the order is the other way around you end up with a larger state
> machine.
> 
> How about PCIe core provides a helper to know whether or not a PCIe device
> will support the proprietary register?

I think the xHCI can be non-PCIe device too (Apple silicon for
instance). The links here are created dynamically and only if there is
need (and support from the hardware) so we can let the USB4 controller
enter D3hot if there is no USB 3.x tunnel needed.

Re: [PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[Mario Limonciello]

>>
>> So I think the problem is you will have an ordering dependency between the
>> two drivers for when the link gets created.
>>
>> Like if thunderbolt.ko loads you would end up with links to PCIe root port
>> for tunneling as well as XHCI controller.
> 
> With this patch we only create links to PCIe Root/Downstream ports from
> Thunderbolt side and the USB core will deal with the USB ones.
> 
>> Then xhci loads and you end up also adding links to individual ports.
>> Would you remove the link to the controller?
> 
> See above.
> 
>> And if the order is the other way around you end up with a larger state
>> machine.
>>
>> How about PCIe core provides a helper to know whether or not a PCIe device
>> will support the proprietary register?
> 
> I think the xHCI can be non-PCIe device too (Apple silicon for
> instance). The links here are created dynamically and only if there is
> need (and support from the hardware) so we can let the USB4 controller
> enter D3hot if there is no USB 3.x tunnel needed.

When I replied I was under the presumption that the next version the 
link creation code for XHCI controller would stay in thunderbolt.ko and 
the XHCI port would be in xhci.ko.  But if you move both non-Intel and 
Intel cases to xhci.ko this should be totally fine.  If you can CC me on 
the next version of the series I'll get that tested for AMD case.

Thanks!

Re: [PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[Mika Westerberg]

On Mon, Jun 24, 2024 at 01:41:09PM -0500, Mario Limonciello wrote:
> > > 
> > > So I think the problem is you will have an ordering dependency between the
> > > two drivers for when the link gets created.
> > > 
> > > Like if thunderbolt.ko loads you would end up with links to PCIe root port
> > > for tunneling as well as XHCI controller.
> > 
> > With this patch we only create links to PCIe Root/Downstream ports from
> > Thunderbolt side and the USB core will deal with the USB ones.
> > 
> > > Then xhci loads and you end up also adding links to individual ports.
> > > Would you remove the link to the controller?
> > 
> > See above.
> > 
> > > And if the order is the other way around you end up with a larger state
> > > machine.
> > > 
> > > How about PCIe core provides a helper to know whether or not a PCIe device
> > > will support the proprietary register?
> > 
> > I think the xHCI can be non-PCIe device too (Apple silicon for
> > instance). The links here are created dynamically and only if there is
> > need (and support from the hardware) so we can let the USB4 controller
> > enter D3hot if there is no USB 3.x tunnel needed.
> 
> When I replied I was under the presumption that the next version the link
> creation code for XHCI controller would stay in thunderbolt.ko and the XHCI
> port would be in xhci.ko.  But if you move both non-Intel and Intel cases to
> xhci.ko this should be totally fine.  If you can CC me on the next version
> of the series I'll get that tested for AMD case.

Sure, we will.

Re: [PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[Mathias Nyman]

On 24.6.2024 7.59, Mika Westerberg wrote:
> On Fri, Jun 21, 2024 at 11:30:05AM -0500, Mario Limonciello wrote:
>> On 6/21/2024 01:19, Mika Westerberg wrote:
>>> Hi Mario,
>>>
>>> On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
>>>> On 6/20/2024 01:41, Mika Westerberg wrote:
>>>>> +CC Mario from AMD side to check that we are good and don't break
>>>>> anything accidentally.
>>>>>
>>>>> On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
>>>>>> The relationship between a USB4 Host Interface providing USB3 tunnels,
>>>>>> and tunneled USB3 devices is not represented in device hierarchy.
>>>>>>
>>>>>> This caused issues with power managment as devices may suspend and
>>>>>> resume in incorrect order.
>>>>>> A device link between the USB4 Host Interface and the USB3 xHCI was
>>>>>> originally added to solve this, preventing the USB4 Host Interface from
>>>>>> suspending if the USB3 xHCI Host was still active.
>>>>>> This unfortunately also prevents USB4 Host Interface from suspending even
>>>>>> if the USB3 xHCI Host is only serving native non-tunneled USB devices.
>>>>>>
>>>>>> Improve the current powermanagement situation by creating device links
>>>>>> directly from tunneled USB3 devices to the USB4 Host Interface they depend
>>>>>> on instead of a device link between the hosts.
>>>>>> This way USB4 host may suspend when the last tunneled device is
>>>>>> disconnected.
>>>>>>
>>>>>> Intel xHCI hosts are capable of reporting if connected USB3 devices are
>>>>>> tunneled via vendor specific capabilities.
>>>>>> Use this until a standard way is available.
>>>>>>
>>>>>> Mathias Nyman (4):
>>>>>>      xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
>>>>>>      usb: Add tunneled parameter to usb device structure
>>>>>>      usb: acpi: add device link between tunneled USB3 device and USB4 Host
>>>>>>        Interface
>>>>>>      thunderbolt: Don't create device link from USB4 Host Interface to USB3
>>>>>>        xHC host
>>>>>>
>>>>>>     drivers/thunderbolt/acpi.c       | 40 ++++++------------------
>>>>>>     drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
>>>>>>     drivers/usb/host/xhci-ext-caps.h |  5 +++
>>>>>>     drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
>>>>>>     drivers/usb/host/xhci.c          | 12 ++++++++
>>>>>>     drivers/usb/host/xhci.h          |  1 +
>>>>>>     include/linux/usb.h              |  2 ++
>>>>>>     7 files changed, 111 insertions(+), 30 deletions(-)
>>>>>>
>>>>>> -- 
>>>>>> 2.25.1
>>>>
>>>> Hi Mika,
>>>>
>>>> Thanks for looping me in.  Unfortunately with this is appears the XHCI
>>>> controller link never gets created.  I've not checked functional impact from
>>>> this, but I'd guess there "should" be some functional problems too.
>>>
>>> Thanks for checking!
>>>
>>> I think the code that sets up the device link based on ->tunneled should
>>> do that always if the hardware cannot tell if this is native or tunneled
>>> link to keep the existing functionality and only do the "optimization"
>>> if the hardware is capable of identifying that.
>>>
>>> Perhaps it can be a callback provided by the xHCI controller driver
>>> (->is_tunneled()) that then defaults to true if the
>>> "usb4-host-interface" property is there and in case of Intel hardware
>>> also checks the proprietary register?

How about changing the boolean udev->tunneled into a 3 value enum with
"link_unknown", "link_native", and "link_tunneled" options.

"link_unknown" would be default, xhci driver only changes this to "link_tunneled" or
"link_native" if the host can detect tunnels.

device link to USB4 host would be created for USB3 devices that have
"usb4-host-interface" property and udev->tunneled != native.

Thanks
Mathias

Re: [PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[Mika Westerberg]

On Tue, Jun 25, 2024 at 05:37:27PM +0300, Mathias Nyman wrote:
> On 24.6.2024 7.59, Mika Westerberg wrote:
> > On Fri, Jun 21, 2024 at 11:30:05AM -0500, Mario Limonciello wrote:
> > > On 6/21/2024 01:19, Mika Westerberg wrote:
> > > > Hi Mario,
> > > > 
> > > > On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
> > > > > On 6/20/2024 01:41, Mika Westerberg wrote:
> > > > > > +CC Mario from AMD side to check that we are good and don't break
> > > > > > anything accidentally.
> > > > > > 
> > > > > > On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
> > > > > > > The relationship between a USB4 Host Interface providing USB3 tunnels,
> > > > > > > and tunneled USB3 devices is not represented in device hierarchy.
> > > > > > > 
> > > > > > > This caused issues with power managment as devices may suspend and
> > > > > > > resume in incorrect order.
> > > > > > > A device link between the USB4 Host Interface and the USB3 xHCI was
> > > > > > > originally added to solve this, preventing the USB4 Host Interface from
> > > > > > > suspending if the USB3 xHCI Host was still active.
> > > > > > > This unfortunately also prevents USB4 Host Interface from suspending even
> > > > > > > if the USB3 xHCI Host is only serving native non-tunneled USB devices.
> > > > > > > 
> > > > > > > Improve the current powermanagement situation by creating device links
> > > > > > > directly from tunneled USB3 devices to the USB4 Host Interface they depend
> > > > > > > on instead of a device link between the hosts.
> > > > > > > This way USB4 host may suspend when the last tunneled device is
> > > > > > > disconnected.
> > > > > > > 
> > > > > > > Intel xHCI hosts are capable of reporting if connected USB3 devices are
> > > > > > > tunneled via vendor specific capabilities.
> > > > > > > Use this until a standard way is available.
> > > > > > > 
> > > > > > > Mathias Nyman (4):
> > > > > > >      xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
> > > > > > >      usb: Add tunneled parameter to usb device structure
> > > > > > >      usb: acpi: add device link between tunneled USB3 device and USB4 Host
> > > > > > >        Interface
> > > > > > >      thunderbolt: Don't create device link from USB4 Host Interface to USB3
> > > > > > >        xHC host
> > > > > > > 
> > > > > > >     drivers/thunderbolt/acpi.c       | 40 ++++++------------------
> > > > > > >     drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
> > > > > > >     drivers/usb/host/xhci-ext-caps.h |  5 +++
> > > > > > >     drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
> > > > > > >     drivers/usb/host/xhci.c          | 12 ++++++++
> > > > > > >     drivers/usb/host/xhci.h          |  1 +
> > > > > > >     include/linux/usb.h              |  2 ++
> > > > > > >     7 files changed, 111 insertions(+), 30 deletions(-)
> > > > > > > 
> > > > > > > -- 
> > > > > > > 2.25.1
> > > > > 
> > > > > Hi Mika,
> > > > > 
> > > > > Thanks for looping me in.  Unfortunately with this is appears the XHCI
> > > > > controller link never gets created.  I've not checked functional impact from
> > > > > this, but I'd guess there "should" be some functional problems too.
> > > > 
> > > > Thanks for checking!
> > > > 
> > > > I think the code that sets up the device link based on ->tunneled should
> > > > do that always if the hardware cannot tell if this is native or tunneled
> > > > link to keep the existing functionality and only do the "optimization"
> > > > if the hardware is capable of identifying that.
> > > > 
> > > > Perhaps it can be a callback provided by the xHCI controller driver
> > > > (->is_tunneled()) that then defaults to true if the
> > > > "usb4-host-interface" property is there and in case of Intel hardware
> > > > also checks the proprietary register?
> 
> How about changing the boolean udev->tunneled into a 3 value enum with
> "link_unknown", "link_native", and "link_tunneled" options.
> 
> "link_unknown" would be default, xhci driver only changes this to "link_tunneled" or
> "link_native" if the host can detect tunnels.
> 
> device link to USB4 host would be created for USB3 devices that have
> "usb4-host-interface" property and udev->tunneled != native.

Sounds good to me :)

Re: [PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[Mario Limonciello]

On 6/25/2024 09:45, Mika Westerberg wrote:
> On Tue, Jun 25, 2024 at 05:37:27PM +0300, Mathias Nyman wrote:
>> On 24.6.2024 7.59, Mika Westerberg wrote:
>>> On Fri, Jun 21, 2024 at 11:30:05AM -0500, Mario Limonciello wrote:
>>>> On 6/21/2024 01:19, Mika Westerberg wrote:
>>>>> Hi Mario,
>>>>>
>>>>> On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
>>>>>> On 6/20/2024 01:41, Mika Westerberg wrote:
>>>>>>> +CC Mario from AMD side to check that we are good and don't break
>>>>>>> anything accidentally.
>>>>>>>
>>>>>>> On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
>>>>>>>> The relationship between a USB4 Host Interface providing USB3 tunnels,
>>>>>>>> and tunneled USB3 devices is not represented in device hierarchy.
>>>>>>>>
>>>>>>>> This caused issues with power managment as devices may suspend and
>>>>>>>> resume in incorrect order.
>>>>>>>> A device link between the USB4 Host Interface and the USB3 xHCI was
>>>>>>>> originally added to solve this, preventing the USB4 Host Interface from
>>>>>>>> suspending if the USB3 xHCI Host was still active.
>>>>>>>> This unfortunately also prevents USB4 Host Interface from suspending even
>>>>>>>> if the USB3 xHCI Host is only serving native non-tunneled USB devices.
>>>>>>>>
>>>>>>>> Improve the current powermanagement situation by creating device links
>>>>>>>> directly from tunneled USB3 devices to the USB4 Host Interface they depend
>>>>>>>> on instead of a device link between the hosts.
>>>>>>>> This way USB4 host may suspend when the last tunneled device is
>>>>>>>> disconnected.
>>>>>>>>
>>>>>>>> Intel xHCI hosts are capable of reporting if connected USB3 devices are
>>>>>>>> tunneled via vendor specific capabilities.
>>>>>>>> Use this until a standard way is available.
>>>>>>>>
>>>>>>>> Mathias Nyman (4):
>>>>>>>>       xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
>>>>>>>>       usb: Add tunneled parameter to usb device structure
>>>>>>>>       usb: acpi: add device link between tunneled USB3 device and USB4 Host
>>>>>>>>         Interface
>>>>>>>>       thunderbolt: Don't create device link from USB4 Host Interface to USB3
>>>>>>>>         xHC host
>>>>>>>>
>>>>>>>>      drivers/thunderbolt/acpi.c       | 40 ++++++------------------
>>>>>>>>      drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
>>>>>>>>      drivers/usb/host/xhci-ext-caps.h |  5 +++
>>>>>>>>      drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
>>>>>>>>      drivers/usb/host/xhci.c          | 12 ++++++++
>>>>>>>>      drivers/usb/host/xhci.h          |  1 +
>>>>>>>>      include/linux/usb.h              |  2 ++
>>>>>>>>      7 files changed, 111 insertions(+), 30 deletions(-)
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> 2.25.1
>>>>>>
>>>>>> Hi Mika,
>>>>>>
>>>>>> Thanks for looping me in.  Unfortunately with this is appears the XHCI
>>>>>> controller link never gets created.  I've not checked functional impact from
>>>>>> this, but I'd guess there "should" be some functional problems too.
>>>>>
>>>>> Thanks for checking!
>>>>>
>>>>> I think the code that sets up the device link based on ->tunneled should
>>>>> do that always if the hardware cannot tell if this is native or tunneled
>>>>> link to keep the existing functionality and only do the "optimization"
>>>>> if the hardware is capable of identifying that.
>>>>>
>>>>> Perhaps it can be a callback provided by the xHCI controller driver
>>>>> (->is_tunneled()) that then defaults to true if the
>>>>> "usb4-host-interface" property is there and in case of Intel hardware
>>>>> also checks the proprietary register?
>>
>> How about changing the boolean udev->tunneled into a 3 value enum with
>> "link_unknown", "link_native", and "link_tunneled" options.
>>
>> "link_unknown" would be default, xhci driver only changes this to "link_tunneled" or
>> "link_native" if the host can detect tunnels.
>>
>> device link to USB4 host would be created for USB3 devices that have
>> "usb4-host-interface" property and udev->tunneled != native.
> 
> Sounds good to me :)

I think it's a good idea as well.  Could you leave a dynamic debugging 
statement with the result?

I think this could be helpful for rooting out unexpected BIOS problems 
with this.