Skip to contentSkip to conversation

thunderbolt: Add support for receiver lane margining

PATCH In review Mika Westerberg 1 participant 7 messages Aug 30, 2022 – Sep 5, 2022

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover thunderbolt: Add support for receiver lane margining

Hi all,

This series adds support for receiver lane margining. This is standard USB4 port feature that can be used in manufacturing to check electrical robustness and quality of given USB4 port. This is a separate Kconfig option (CONFIG_USB4_DEBUGFS_MARGINING) that can be enabled by the kernels used in manufacturing floor (normal distro kernels do not need this to be enabled).

This exposes a new debugfs directory "margining" under each connected USB4 port that can be used to run the margining test. This supports both hardware and software lane margining although I have only tested the former as the current Intel hardware only supports that.

Previous version of the series can be found here:

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

Changes from the previous version:

* Pass bitmask of CL states into tb_port_clx_supported(). This is a

    new patch 3/5.
  * Use bitmask of CL states also in tb_port_is_clx_enabled() and drop
    tb_port_are_clx_enabled().

Mika Westerberg (5):
thunderbolt: Move tb_xdomain_parent() to tb.h
thunderbolt: Move port CL state functions into correct place in switch.c
thunderbolt: Pass CL state bitmask to tb_port_clx_supported()
thunderbolt: Add helper to check if CL states are enabled on port
thunderbolt: Add support for receiver lane margining

 drivers/thunderbolt/Kconfig   |  10 +
 drivers/thunderbolt/debugfs.c | 836 ++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/sb_regs.h |  58 +++
 drivers/thunderbolt/switch.c  | 235 +++++-----
 drivers/thunderbolt/tb.h      |  25 +-
 drivers/thunderbolt/tb_regs.h |   2 +
 drivers/thunderbolt/usb4.c    | 120 +++++
 drivers/thunderbolt/xdomain.c |   9 +-
 8 files changed, 1182 insertions(+), 113 deletions(-)
Signature · 1 line
2.35.1
#1 lore raw

PATCH v2 1/5 thunderbolt: Move tb_xdomain_parent() to tb.h

We are going to need this for lane margining support so make it available outside of xdomain.c.

---
 drivers/thunderbolt/tb.h      | 5 +++++
 drivers/thunderbolt/xdomain.c | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)
drivers/thunderbolt/tb.h +5 -0
index 5db76de40cc1..8291cabd2e92 100644
@@ -1132,6 +1132,11 @@ void tb_xdomain_remove(struct tb_xdomain *xd);
 struct tb_xdomain *tb_xdomain_find_by_link_depth(struct tb *tb, u8 link,
 						 u8 depth);
 
+static inline struct tb_switch *tb_xdomain_parent(struct tb_xdomain *xd)
+{
+	return tb_to_switch(xd->dev.parent);
+}
+
 int tb_retimer_scan(struct tb_port *port, bool add);
 void tb_retimer_remove_all(struct tb_port *port);
 
drivers/thunderbolt/xdomain.c +0 -5
index c31c0d94d8b3..e4bbc4cb5f97 100644
@@ -1131,11 +1131,6 @@ static int populate_properties(struct tb_xdomain *xd,
 	return 0;
 }
 
-static inline struct tb_switch *tb_xdomain_parent(struct tb_xdomain *xd)
-{
-	return tb_to_switch(xd->dev.parent);
-}
-
 static int tb_xdomain_update_link_attributes(struct tb_xdomain *xd)
 {
 	bool change = false;
Signature · 1 line
2.35.1
#2 lore raw

PATCH v2 2/5 thunderbolt: Move port CL state functions into correct place in switch.c

In reply to Mika Westerberg

They should be close to other functions dealing with USB4 ports. No functional impact.

---
 drivers/thunderbolt/switch.c | 212 +++++++++++++++++------------------
 1 file changed, 106 insertions(+), 106 deletions(-)
drivers/thunderbolt/switch.c +106 -106
index 244f8cd38b25..deefc92c7c60 100644
@@ -1229,6 +1229,112 @@ int tb_port_update_credits(struct tb_port *port)
 	return tb_port_do_update_credits(port->dual_link_port);
 }
 
+static int __tb_port_pm_secondary_set(struct tb_port *port, bool secondary)
+{
+	u32 phy;
+	int ret;
+
+	ret = tb_port_read(port, &phy, TB_CFG_PORT,
+			   port->cap_phy + LANE_ADP_CS_1, 1);
+	if (ret)
+		return ret;
+
+	if (secondary)
+		phy |= LANE_ADP_CS_1_PMS;
+	else
+		phy &= ~LANE_ADP_CS_1_PMS;
+
+	return tb_port_write(port, &phy, TB_CFG_PORT,
+			     port->cap_phy + LANE_ADP_CS_1, 1);
+}
+
+static int tb_port_pm_secondary_enable(struct tb_port *port)
+{
+	return __tb_port_pm_secondary_set(port, true);
+}
+
+static int tb_port_pm_secondary_disable(struct tb_port *port)
+{
+	return __tb_port_pm_secondary_set(port, false);
+}
+
+/* Called for USB4 or Titan Ridge routers only */
+static bool tb_port_clx_supported(struct tb_port *port, enum tb_clx clx)
+{
+	u32 mask, val;
+	bool ret;
+
+	/* Don't enable CLx in case of two single-lane links */
+	if (!port->bonded && port->dual_link_port)
+		return false;
+
+	/* Don't enable CLx in case of inter-domain link */
+	if (port->xdomain)
+		return false;
+
+	if (tb_switch_is_usb4(port->sw)) {
+		if (!usb4_port_clx_supported(port))
+			return false;
+	} else if (!tb_lc_is_clx_supported(port)) {
+		return false;
+	}
+
+	switch (clx) {
+	case TB_CL1:
+		/* CL0s and CL1 are enabled and supported together */
+		mask = LANE_ADP_CS_0_CL0S_SUPPORT | LANE_ADP_CS_0_CL1_SUPPORT;
+		break;
+
+	/* For now we support only CL0s and CL1. Not CL2 */
+	case TB_CL2:
+	default:
+		return false;
+	}
+
+	ret = tb_port_read(port, &val, TB_CFG_PORT,
+			   port->cap_phy + LANE_ADP_CS_0, 1);
+	if (ret)
+		return false;
+
+	return !!(val & mask);
+}
+
+static int __tb_port_clx_set(struct tb_port *port, enum tb_clx clx, bool enable)
+{
+	u32 phy, mask;
+	int ret;
+
+	/* CL0s and CL1 are enabled and supported together */
+	if (clx == TB_CL1)
+		mask = LANE_ADP_CS_1_CL0S_ENABLE | LANE_ADP_CS_1_CL1_ENABLE;
+	else
+		/* For now we support only CL0s and CL1. Not CL2 */
+		return -EOPNOTSUPP;
+
+	ret = tb_port_read(port, &phy, TB_CFG_PORT,
+			   port->cap_phy + LANE_ADP_CS_1, 1);
+	if (ret)
+		return ret;
+
+	if (enable)
+		phy |= mask;
+	else
+		phy &= ~mask;
+
+	return tb_port_write(port, &phy, TB_CFG_PORT,
+			     port->cap_phy + LANE_ADP_CS_1, 1);
+}
+
+static int tb_port_clx_disable(struct tb_port *port, enum tb_clx clx)
+{
+	return __tb_port_clx_set(port, clx, false);
+}
+
+static int tb_port_clx_enable(struct tb_port *port, enum tb_clx clx)
+{
+	return __tb_port_clx_set(port, clx, true);
+}
+
 static int tb_port_start_lane_initialization(struct tb_port *port)
 {
 	int ret;
@@ -3361,35 +3467,6 @@ struct tb_port *tb_switch_find_port(struct tb_switch *sw,
 	return NULL;
 }
 
-static int __tb_port_pm_secondary_set(struct tb_port *port, bool secondary)
-{
-	u32 phy;
-	int ret;
-
-	ret = tb_port_read(port, &phy, TB_CFG_PORT,
-			   port->cap_phy + LANE_ADP_CS_1, 1);
-	if (ret)
-		return ret;
-
-	if (secondary)
-		phy |= LANE_ADP_CS_1_PMS;
-	else
-		phy &= ~LANE_ADP_CS_1_PMS;
-
-	return tb_port_write(port, &phy, TB_CFG_PORT,
-			     port->cap_phy + LANE_ADP_CS_1, 1);
-}
-
-static int tb_port_pm_secondary_enable(struct tb_port *port)
-{
-	return __tb_port_pm_secondary_set(port, true);
-}
-
-static int tb_port_pm_secondary_disable(struct tb_port *port)
-{
-	return __tb_port_pm_secondary_set(port, false);
-}
-
 static int tb_switch_pm_secondary_resolve(struct tb_switch *sw)
 {
 	struct tb_switch *parent = tb_switch_parent(sw);
@@ -3408,83 +3485,6 @@ static int tb_switch_pm_secondary_resolve(struct tb_switch *sw)
 	return tb_port_pm_secondary_disable(down);
 }
 
-/* Called for USB4 or Titan Ridge routers only */
-static bool tb_port_clx_supported(struct tb_port *port, enum tb_clx clx)
-{
-	u32 mask, val;
-	bool ret;
-
-	/* Don't enable CLx in case of two single-lane links */
-	if (!port->bonded && port->dual_link_port)
-		return false;
-
-	/* Don't enable CLx in case of inter-domain link */
-	if (port->xdomain)
-		return false;
-
-	if (tb_switch_is_usb4(port->sw)) {
-		if (!usb4_port_clx_supported(port))
-			return false;
-	} else if (!tb_lc_is_clx_supported(port)) {
-		return false;
-	}
-
-	switch (clx) {
-	case TB_CL1:
-		/* CL0s and CL1 are enabled and supported together */
-		mask = LANE_ADP_CS_0_CL0S_SUPPORT | LANE_ADP_CS_0_CL1_SUPPORT;
-		break;
-
-	/* For now we support only CL0s and CL1. Not CL2 */
-	case TB_CL2:
-	default:
-		return false;
-	}
-
-	ret = tb_port_read(port, &val, TB_CFG_PORT,
-			   port->cap_phy + LANE_ADP_CS_0, 1);
-	if (ret)
-		return false;
-
-	return !!(val & mask);
-}
-
-static int __tb_port_clx_set(struct tb_port *port, enum tb_clx clx, bool enable)
-{
-	u32 phy, mask;
-	int ret;
-
-	/* CL0s and CL1 are enabled and supported together */
-	if (clx == TB_CL1)
-		mask = LANE_ADP_CS_1_CL0S_ENABLE | LANE_ADP_CS_1_CL1_ENABLE;
-	else
-		/* For now we support only CL0s and CL1. Not CL2 */
-		return -EOPNOTSUPP;
-
-	ret = tb_port_read(port, &phy, TB_CFG_PORT,
-			   port->cap_phy + LANE_ADP_CS_1, 1);
-	if (ret)
-		return ret;
-
-	if (enable)
-		phy |= mask;
-	else
-		phy &= ~mask;
-
-	return tb_port_write(port, &phy, TB_CFG_PORT,
-			     port->cap_phy + LANE_ADP_CS_1, 1);
-}
-
-static int tb_port_clx_disable(struct tb_port *port, enum tb_clx clx)
-{
-	return __tb_port_clx_set(port, clx, false);
-}
-
-static int tb_port_clx_enable(struct tb_port *port, enum tb_clx clx)
-{
-	return __tb_port_clx_set(port, clx, true);
-}
-
 static int __tb_switch_enable_clx(struct tb_switch *sw, enum tb_clx clx)
 {
 	struct tb_switch *parent = tb_switch_parent(sw);
Signature · 1 line
2.35.1
#3 lore raw

PATCH v2 3/5 thunderbolt: Pass CL state bitmask to tb_port_clx_supported()

In reply to Mika Westerberg

Instead of testing just a single CL state we can pass a bitmask of states to check. This makes it simpler for callers of the function.

We also add a check for CL2 even though not fully supported by the driver yet.

---
 drivers/thunderbolt/switch.c  | 17 ++++++-----------
 drivers/thunderbolt/tb.h      |  4 ++--
 drivers/thunderbolt/tb_regs.h |  1 +
 3 files changed, 9 insertions(+), 13 deletions(-)
drivers/thunderbolt/switch.c +6 -11
index deefc92c7c60..0671f5361b5b 100644
@@ -1259,9 +1259,9 @@ static int tb_port_pm_secondary_disable(struct tb_port *port)
 }
 
 /* Called for USB4 or Titan Ridge routers only */
-static bool tb_port_clx_supported(struct tb_port *port, enum tb_clx clx)
+static bool tb_port_clx_supported(struct tb_port *port, unsigned int clx_mask)
 {
-	u32 mask, val;
+	u32 val, mask = 0;
 	bool ret;
 
 	/* Don't enable CLx in case of two single-lane links */
@@ -1279,17 +1279,12 @@ static bool tb_port_clx_supported(struct tb_port *port, enum tb_clx clx)
 		return false;
 	}
 
-	switch (clx) {
-	case TB_CL1:
+	if (clx_mask & TB_CL1) {
 		/* CL0s and CL1 are enabled and supported together */
-		mask = LANE_ADP_CS_0_CL0S_SUPPORT | LANE_ADP_CS_0_CL1_SUPPORT;
-		break;
-
-	/* For now we support only CL0s and CL1. Not CL2 */
-	case TB_CL2:
-	default:
-		return false;
+		mask |= LANE_ADP_CS_0_CL0S_SUPPORT | LANE_ADP_CS_0_CL1_SUPPORT;
 	}
+	if (clx_mask & TB_CL2)
+		mask |= LANE_ADP_CS_0_CL2_SUPPORT;
 
 	ret = tb_port_read(port, &val, TB_CFG_PORT,
 			   port->cap_phy + LANE_ADP_CS_0, 1);
drivers/thunderbolt/tb.h +2 -2
index 8291cabd2e92..8555ad9d7234 100644
@@ -113,8 +113,8 @@ struct tb_switch_tmu {
 enum tb_clx {
 	TB_CLX_DISABLE,
 	/* CL0s and CL1 are enabled and supported together */
-	TB_CL1,
-	TB_CL2,
+	TB_CL1 = BIT(0),
+	TB_CL2 = BIT(1),
 };
 
 /**
drivers/thunderbolt/tb_regs.h +1 -0
index 166054110388..a45f295533cd 100644
@@ -324,6 +324,7 @@ struct tb_regs_port_header {
 #define LANE_ADP_CS_0_SUPPORTED_WIDTH_DUAL	0x2
 #define LANE_ADP_CS_0_CL0S_SUPPORT		BIT(26)
 #define LANE_ADP_CS_0_CL1_SUPPORT		BIT(27)
+#define LANE_ADP_CS_0_CL2_SUPPORT		BIT(28)
 #define LANE_ADP_CS_1				0x01
 #define LANE_ADP_CS_1_TARGET_SPEED_MASK		GENMASK(3, 0)
 #define LANE_ADP_CS_1_TARGET_SPEED_GEN3		0xc
Signature · 1 line
2.35.1
#4 lore raw

PATCH v2 4/5 thunderbolt: Add helper to check if CL states are enabled on port

In reply to Mika Westerberg

We will need this when enabling lane margining support.

---
 drivers/thunderbolt/switch.c  | 28 ++++++++++++++++++++++++++++
 drivers/thunderbolt/tb.h      |  1 +
 drivers/thunderbolt/tb_regs.h |  1 +
 3 files changed, 30 insertions(+)
drivers/thunderbolt/switch.c +28 -0
index 0671f5361b5b..bd815e2cc6ec 100644
@@ -1330,6 +1330,34 @@ static int tb_port_clx_enable(struct tb_port *port, enum tb_clx clx)
 	return __tb_port_clx_set(port, clx, true);
 }
 
+/**
+ * tb_port_is_clx_enabled() - Is given CL state enabled
+ * @port: USB4 port to check
+ * @clx_mask: Mask of CL states to check
+ *
+ * Returns true if any of the given CL states is enabled for @port.
+ */
+bool tb_port_is_clx_enabled(struct tb_port *port, unsigned int clx_mask)
+{
+	u32 val, mask = 0;
+	int ret;
+
+	if (!tb_port_clx_supported(port, clx_mask))
+		return false;
+
+	if (clx_mask & TB_CL1)
+		mask |= LANE_ADP_CS_1_CL0S_ENABLE | LANE_ADP_CS_1_CL1_ENABLE;
+	if (clx_mask & TB_CL2)
+		mask |= LANE_ADP_CS_1_CL2_ENABLE;
+
+	ret = tb_port_read(port, &val, TB_CFG_PORT,
+			   port->cap_phy + LANE_ADP_CS_1, 1);
+	if (ret)
+		return false;
+
+	return !!(val & mask);
+}
+
 static int tb_port_start_lane_initialization(struct tb_port *port)
 {
 	int ret;
drivers/thunderbolt/tb.h +1 -0
index 8555ad9d7234..ded1f1d6c12e 100644
@@ -1035,6 +1035,7 @@ void tb_port_lane_bonding_disable(struct tb_port *port);
 int tb_port_wait_for_link_width(struct tb_port *port, int width,
 				int timeout_msec);
 int tb_port_update_credits(struct tb_port *port);
+bool tb_port_is_clx_enabled(struct tb_port *port, enum tb_clx clx);
 
 int tb_switch_find_vse_cap(struct tb_switch *sw, enum tb_switch_vse_cap vsec);
 int tb_switch_find_cap(struct tb_switch *sw, enum tb_switch_cap cap);
drivers/thunderbolt/tb_regs.h +1 -0
index a45f295533cd..0fe1daa21423 100644
@@ -334,6 +334,7 @@ struct tb_regs_port_header {
 #define LANE_ADP_CS_1_TARGET_WIDTH_DUAL		0x3
 #define LANE_ADP_CS_1_CL0S_ENABLE		BIT(10)
 #define LANE_ADP_CS_1_CL1_ENABLE		BIT(11)
+#define LANE_ADP_CS_1_CL2_ENABLE		BIT(12)
 #define LANE_ADP_CS_1_LD			BIT(14)
 #define LANE_ADP_CS_1_LB			BIT(15)
 #define LANE_ADP_CS_1_CURRENT_SPEED_MASK	GENMASK(19, 16)
Signature · 1 line
2.35.1
#5 lore raw

PATCH v2 5/5 thunderbolt: Add support for receiver lane margining

In reply to Mika Westerberg

USB4 spec defines standard set of registers to be used for receiver lane margining. This is useful for I/O interface quality and electrical robustness validation during manufacturing. Expose receiver lane margining through new debugfs directory "margining" that is added under each connected USB4 port. Users can then run the margining by writing to the exposed attributes under that directory.

---
 drivers/thunderbolt/Kconfig   |  10 +
 drivers/thunderbolt/debugfs.c | 836 ++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/sb_regs.h |  58 +++
 drivers/thunderbolt/tb.h      |  15 +
 drivers/thunderbolt/usb4.c    | 120 +++++
 drivers/thunderbolt/xdomain.c |   4 +
 6 files changed, 1043 insertions(+)
drivers/thunderbolt/Kconfig +10 -0
index e76a6c173637..d76ba4faca0d 100644
@@ -27,6 +27,16 @@ config USB4_DEBUGFS_WRITE
 	  Only enable this if you know what you are doing! Never enable
 	  this for production systems or distro kernels.
 
+config USB4_DEBUGFS_MARGINING
+	bool "Expose receiver lane margining operations under USB4 ports (DANGEROUS)"
+	depends on DEBUG_FS
+	depends on USB4_DEBUGFS_WRITE
+	help
+	  Enables hardware and software based receiver lane margining support
+	  under each USB4 port. Used for electrical quality and robustness
+	  validation during manufacturing. Should not be enabled by distro
+	  kernels.
+
 config USB4_KUNIT_TEST
 	bool "KUnit tests" if !KUNIT_ALL_TESTS
 	depends on (USB4=m || KUNIT=y)
drivers/thunderbolt/debugfs.c +836 -0

863 lines not on this page · raw message

drivers/thunderbolt/sb_regs.h +58 -0

68 lines not on this page · raw message

drivers/thunderbolt/tb.h +15 -0
index ded1f1d6c12e..87aefac03d8a 100644
@@ -279,12 +279,16 @@ struct tb_port {
  * @can_offline: Does the port have necessary platform support to moved
  *		 it into offline mode and back
  * @offline: The port is currently in offline mode
+ * @margining: Pointer to margining structure if enabled
  */
 struct usb4_port {
 	struct device dev;
 	struct tb_port *port;
 	bool can_offline;
 	bool offline;
+#ifdef CONFIG_USB4_DEBUGFS_MARGINING
+	struct tb_margining *margining;
+#endif
 };
 
 /**
@@ -1188,6 +1192,13 @@ int usb4_port_router_offline(struct tb_port *port);
 int usb4_port_router_online(struct tb_port *port);
 int usb4_port_enumerate_retimers(struct tb_port *port);
 bool usb4_port_clx_supported(struct tb_port *port);
+int usb4_port_margining_caps(struct tb_port *port, u32 *caps);
+int usb4_port_hw_margin(struct tb_port *port, unsigned int lanes,
+			unsigned int ber_level, bool timing, bool right_high,
+			u32 *results);
+int usb4_port_sw_margin(struct tb_port *port, unsigned int lanes, bool timing,
+			bool right_high, u32 counter);
+int usb4_port_sw_margin_errors(struct tb_port *port, u32 *errors);
 
 int usb4_port_retimer_set_inbound_sbtx(struct tb_port *port, u8 index);
 int usb4_port_retimer_read(struct tb_port *port, u8 index, u8 reg, void *buf,
@@ -1270,6 +1281,8 @@ void tb_debugfs_init(void);
 void tb_debugfs_exit(void);
 void tb_switch_debugfs_init(struct tb_switch *sw);
 void tb_switch_debugfs_remove(struct tb_switch *sw);
+void tb_xdomain_debugfs_init(struct tb_xdomain *xd);
+void tb_xdomain_debugfs_remove(struct tb_xdomain *xd);
 void tb_service_debugfs_init(struct tb_service *svc);
 void tb_service_debugfs_remove(struct tb_service *svc);
 #else
@@ -1277,6 +1290,8 @@ static inline void tb_debugfs_init(void) { }
 static inline void tb_debugfs_exit(void) { }
 static inline void tb_switch_debugfs_init(struct tb_switch *sw) { }
 static inline void tb_switch_debugfs_remove(struct tb_switch *sw) { }
+static inline void tb_xdomain_debugfs_init(struct tb_xdomain *xd) { }
+static inline void tb_xdomain_debugfs_remove(struct tb_xdomain *xd) { }
 static inline void tb_service_debugfs_init(struct tb_service *svc) { }
 static inline void tb_service_debugfs_remove(struct tb_service *svc) { }
 #endif
drivers/thunderbolt/usb4.c +120 -0

126 lines not on this page · raw message

drivers/thunderbolt/xdomain.c +4 -0
index e4bbc4cb5f97..dcee91f25075 100644
@@ -1435,6 +1435,8 @@ static int tb_xdomain_get_properties(struct tb_xdomain *xd)
 		if (xd->vendor_name && xd->device_name)
 			dev_info(&xd->dev, "%s %s\n", xd->vendor_name,
 				 xd->device_name);
+
+		tb_xdomain_debugfs_init(xd);
 	} else {
 		kobject_uevent(&xd->dev.kobj, KOBJ_CHANGE);
 	}
@@ -1935,6 +1937,8 @@ static int unregister_service(struct device *dev, void *data)
  */
 void tb_xdomain_remove(struct tb_xdomain *xd)
 {
+	tb_xdomain_debugfs_remove(xd);
+
 	stop_handshake(xd);
 
 	device_for_each_child_reverse(&xd->dev, xd, unregister_service);
Signature · 1 line
2.35.1
#6 lore raw
In reply to Mika Westerberg
Quoted 5 lines · Mika Westerberg

thunderbolt: Move tb_xdomain_parent() to tb.h
thunderbolt: Move port CL state functions into correct place in switch.c
thunderbolt: Pass CL state bitmask to tb_port_clx_supported()
thunderbolt: Add helper to check if CL states are enabled on port
thunderbolt: Add support for receiver lane margining

All applied to thunderbolt.git/next.