Skip to contentSkip to conversation

thunderbolt: Add device links for Apple T2

PATCH In review Mika Westerberg 2 participants 7 messages Aug 31 – Sep 8

Subsystems (2)
USB Thunderbolt driver
lore
Share & tools
lore

Mika Westerberg

author
#0 lore raw

v10 cover thunderbolt: Add device links for Apple T2

Hi all,

Original cover letter:

This series adds support for Device linking on Titan ridge, and Icelake, which are used exclusively on Apple T2 machines.

I went and did some changes so wanted to check that these still work on the Apple T2 systems. I dropped Tested-by from Alexander Fischer because these are not anymore the same patches. I would appreciate if you could re-test these and let me know if they still work or not.

The changes I did:

- Make ->add_links a NHI operation instead.
- Move the check to nhi.c instead after we have selected software CM.
- Split out add_link() helper that can be used both integrated and discrete case.
- Link only the matching PCIe root ports to the NHI that actually tunnels them. The previous version (v9) linked all four for each NHI so there are duplicate links. Each NHI tunnels just up to 2 PCIe root ports in Ice Lake.

v9 of the patch series can be found here:

https://lore.kernel.org/linux-usb/[email protected]/

Atharva Tiwari (3):
thunderbolt: Move tb_apple_add_links() to pci.c
thunderbolt: Add device links for Apple machines with Titan Ridge
thunderbolt: Add device links for Apple systems with Ice Lake

 drivers/thunderbolt/nhi.c |  34 +++++++++---
 drivers/thunderbolt/nhi.h |   2 +
 drivers/thunderbolt/pci.c | 113 ++++++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/tb.c  |  78 --------------------------
 4 files changed, 140 insertions(+), 87 deletions(-)
Signature · 1 line
2.50.1
author
#1 lore raw

PATCH v10 1/3 thunderbolt: Move tb_apple_add_links() to pci.c

From: Atharva Tiwari <[email protected]>

Due to historical reasons the software connection manager in tb.c directly sets up the device link between the tunneled PCIe ports and the NHI. Now as we are starting to support non-PCIe host interfaces as well we are trying to keep the connection manager implementation agnostic of the underlying host interface. For this reason move the device link creation into pci.c and expose it through a new NHI operation that the host interface code calls when it sets up the connection manager.

Suggested-by Mika Westerberg <[email protected]>
Signed-off-by Atharva Tiwari <[email protected]>
Signed-off-by Mika Westerberg <[email protected]>
---
 drivers/thunderbolt/nhi.c | 34 ++++++++++++-----
 drivers/thunderbolt/nhi.h |  2 +
 drivers/thunderbolt/pci.c | 76 ++++++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/tb.c  | 78 ---------------------------------------
 4 files changed, 103 insertions(+), 87 deletions(-)
drivers/thunderbolt/nhi.c +25 -9
index d4d1efa2afa0..5827c498c628 100644
@@ -1208,24 +1208,40 @@ void nhi_reset_interface(struct tb_nhi *nhi)
 
 static struct tb *nhi_select_cm(struct tb_nhi *nhi)
 {
+	bool linked = false;
 	struct tb *tb;
 
 	/*
 	 * USB4 case is simple. If we got control of any of the
 	 * capabilities, we use software CM.
 	 */
-	if (tb_acpi_is_native())
-		return tb_probe(nhi);
+	if (!tb_acpi_is_native()) {
+		/*
+		 * Either firmware based CM is running (we did not get
+		 * control from the firmware) or this is pre-USB4 PC so
+		 * try first firmware CM and then fallback to software CM.
+		 */
+		tb = icm_probe(nhi);
+		if (tb)
+			return tb;
+	}
 
-	/*
-	 * Either firmware based CM is running (we did not get control
-	 * from the firmware) or this is pre-USB4 PC so try first
-	 * firmware CM and then fallback to software CM.
-	 */
-	tb = icm_probe(nhi);
+	tb = tb_probe(nhi);
 	if (!tb)
-		tb = tb_probe(nhi);
+		return NULL;
 
+	if (nhi->ops->add_links)
+		linked = nhi->ops->add_links(nhi);
+	if (!linked)
+		linked = tb_acpi_add_links(nhi);
+	/*
+	 * Device links are needed to make sure we establish tunnels
+	 * before protocol stacks are resumed so complain here if we
+	 * found them missing.
+	 */
+	if (!linked)
+		dev_warn(nhi->dev,
+			 "device links to tunneled native ports are missing!\n");
 	return tb;
 }
drivers/thunderbolt/nhi.h +2 -0
index f72d6b274501..b2e2e2c413b2 100644
@@ -43,6 +43,7 @@ extern const struct dev_pm_ops nhi_pm_ops;
 /**
  * struct tb_nhi_ops - NHI specific optional operations
  * @init: NHI specific initialization
+ * @add_links: Setup device links for tunneling native protocols
  * @suspend_noirq: NHI specific suspend_noirq hook
  * @resume_noirq: NHI specific resume_noirq hook
  * @runtime_suspend: NHI specific runtime_suspend hook

@@ -58,6 +59,7 @@ extern const struct dev_pm_ops nhi_pm_ops;
  */
 struct tb_nhi_ops {
 	int (*init)(struct tb_nhi *nhi);
+	bool (*add_links)(struct tb_nhi *nhi);
 	int (*suspend_noirq)(struct tb_nhi *nhi, bool wakeup);
 	int (*resume_noirq)(struct tb_nhi *nhi);
 	int (*runtime_suspend)(struct tb_nhi *nhi);
drivers/thunderbolt/pci.c +76 -0

94 lines not on this page · raw message

drivers/thunderbolt/tb.c +0 -78

95 lines not on this page · raw message

Signature · 1 line
2.50.1
author
#2 lore raw

PATCH v10 2/3 thunderbolt: Add device links for Apple machines with Titan Ridge

In reply to Mika Westerberg

From: Atharva Tiwari <[email protected]>

Add device links for Apple machines with the Titan Ridge thunderbolt controller.

Signed-off-by Atharva Tiwari <[email protected]>
Signed-off-by Mika Westerberg <[email protected]>
---
 drivers/thunderbolt/pci.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/thunderbolt/pci.c +2 -0
index c9c50004a6ac..996a8ab3cfb7 100644
@@ -168,6 +168,8 @@ static bool nhi_pci_add_links(struct tb_nhi *nhi)
 	case PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C:
 	case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_NHI:
 	case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI:
+	case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_NHI:
+	case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_NHI:
 		break;
 	default:
 		return false;
Signature · 1 line
2.50.1
author
#3 lore raw

PATCH v10 3/3 thunderbolt: Add device links for Apple systems with Ice Lake

In reply to Mika Westerberg

From: Atharva Tiwari <[email protected]>

Ice Lake Thunderbolt NHI that are on some Macs. The NHI and its associated PCIe Root Ports all sit directly on the Root Complex with no upstream port. Identify the tunneled PCIe Root Ports by their PCI IDs and create device links back to the NHI so that PCIe tunnels can be re-established after sleep.

Co-developed-by Andre Eikmeyer <[email protected]>
Signed-off-by Andre Eikmeyer <[email protected]>
Signed-off-by Atharva Tiwari <[email protected]>
Signed-off-by Mika Westerberg <[email protected]>
---
 drivers/thunderbolt/pci.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
drivers/thunderbolt/pci.c +35 -0
index 996a8ab3cfb7..e40d4d6af071 100644
@@ -521,6 +521,40 @@ static int icl_nhi_resume(struct tb_nhi *nhi)
 	return 0;
 }
 
+static bool icl_nhi_add_links(struct tb_nhi *nhi)
+{
+	struct pci_dev *nhi_pdev = to_pci_dev(nhi->dev);
+	struct pci_bus *bus = nhi_pdev->bus;
+	struct pci_dev *pdev;
+	bool ret;
+
+	if (!x86_apple_machine)
+		return false;
+
+	/*
+	 * On integrated controllers the tunneled PCIe root ports are
+	 * directly under the host bridge.
+	 */
+	if (!pci_is_root_bus(bus))
+		return false;
+
+	ret = false;
+	for_each_pci_bridge(pdev, bus) {
+		switch (nhi_pdev->device) {
+		case PCI_DEVICE_ID_INTEL_ICL_NHI0:
+			if (pdev->device == 0x8a1d || pdev->device == 0x8a1f)
+				ret |= add_link(nhi, pdev);
+			break;
+		case PCI_DEVICE_ID_INTEL_ICL_NHI1:
+			if (pdev->device == 0x8a21 || pdev->device == 0x8a23)
+				ret |= add_link(nhi, pdev);
+			break;
+		}
+	}
+
+	return ret;
+}
+
 static void icl_nhi_shutdown(struct tb_nhi *nhi)
 {
 	nhi_pci_release_irq(nhi);

@@ -530,6 +564,7 @@ static void icl_nhi_shutdown(struct tb_nhi *nhi)
 
 static const struct tb_nhi_ops icl_nhi_ops = {
 	.init = icl_nhi_resume,
+	.add_links = icl_nhi_add_links,
 	.suspend_noirq = icl_nhi_suspend_noirq,
 	.resume_noirq = icl_nhi_resume,
 	.runtime_suspend = icl_nhi_suspend,
Signature · 1 line
2.50.1
author
#4 lore raw
In reply to Mika Westerberg

Hi,

Quoted 23 lines · Mika Westerberg

Hi all,

Original cover letter:

This series adds support for Device linking on Titan ridge, and Icelake, which are used exclusively on Apple T2 machines.

I went and did some changes so wanted to check that these still work on the Apple T2 systems. I dropped Tested-by from Alexander Fischer because these are not anymore the same patches. I would appreciate if you could re-test these and let me know if they still work or not.

The changes I did:

- Make ->add_links a NHI operation instead.
- Move the check to nhi.c instead after we have selected software CM.
- Split out add_link() helper that can be used both integrated and discrete case.
- Link only the matching PCIe root ports to the NHI that actually tunnels them. The previous version (v9) linked all four for each NHI so there are duplicate links. Each NHI tunnels just up to 2 PCIe root ports in Ice Lake.

Quoted 8 lines

v9 of the patch series can be found here:

https://lore.kernel.org/linux-usb/[email protected]/

Atharva Tiwari (3):
thunderbolt: Move tb_apple_add_links() to pci.c
thunderbolt: Add device links for Apple machines with Titan Ridge
thunderbolt: Add device links for Apple systems with Ice Lake

Appled these to thunderbolt.git/next. I did not get any reports whether this still works on the Apple T2 system but I guess silence means "no issues" ;-) Let me know if that's not the case.

#5 lore raw

v10 cover thunderbolt: Add device links for Apple T2

Quoted 3 lines

Appled these to thunderbolt.git/next. I did not get any reports whether this still works on the Apple T2 system but I guess silence means "no issues" ;-) Let me know if that's not the case.

Sorry for the silence, but yes the code is working perfectly fine.

author
#6 lore raw
Quoted 5 lines · Atharva Tiwari
Quoted 3 lines

Appled these to thunderbolt.git/next. I did not get any reports whether this still works on the Apple T2 system but I guess silence means "no issues" ;-) Let me know if that's not the case.

Sorry for the silence, but yes the code is working perfectly fine.

Okay thanks for letting me know!