All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] thunderbolt: CLx support for USB4 and Titan Ridge
@ 2021-11-25 14:38 Gil Fine
  2021-11-25 14:38 ` [PATCH 1/7] thunderbolt: Add TMU unidirectional mode Gil Fine
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Gil Fine @ 2021-11-25 14:38 UTC (permalink / raw)
  To: andreas.noever, michael.jamet, mika.westerberg, YehezkelShB
  Cc: gil.fine, linux-usb, lukas

In this series of patches we add support for link low power states (CLx).
Low Power states are used to reduce transmitter and receiver power
when a Lane is idle. First we enable it for USB4 devices, then for Titan Ridge
device. No other thunderbolt devices support CLx.

Gil Fine (7):
  thunderbolt: Add TMU unidirectional mode
  thunderbolt: Add CL0s support for USB4
  thunderbolt: Move usb4_switch_wait_for_bit() to switch.c
  thunderbolt: Enable TMU for Titan Ridge device
  thunderbolt: Rename Intel VSC capability
  thunderbolt: Enable CLx for Titan Ridge device
  thunderbolt: Add kernel param for CLx disabling

 drivers/thunderbolt/lc.c      |  27 ++
 drivers/thunderbolt/switch.c  | 527 +++++++++++++++++++++++++++++++++-
 drivers/thunderbolt/tb.c      |  23 +-
 drivers/thunderbolt/tb.h      |  62 +++-
 drivers/thunderbolt/tb_regs.h |  45 ++-
 drivers/thunderbolt/tmu.c     | 324 ++++++++++++++++++---
 drivers/thunderbolt/usb4.c    |  52 ++--
 7 files changed, 980 insertions(+), 80 deletions(-)

-- 
2.17.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/7] thunderbolt: Add TMU unidirectional mode
  2021-11-25 14:38 [PATCH 0/7] thunderbolt: CLx support for USB4 and Titan Ridge Gil Fine
@ 2021-11-25 14:38 ` Gil Fine
  2021-11-26 11:58   ` Yehezkel Bernat
  2021-11-29  8:19   ` Mika Westerberg
  2021-11-25 14:38 ` [PATCH 2/7] thunderbolt: Add CL0s support for USB4 Gil Fine
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 12+ messages in thread
From: Gil Fine @ 2021-11-25 14:38 UTC (permalink / raw)
  To: andreas.noever, michael.jamet, mika.westerberg, YehezkelShB
  Cc: gil.fine, linux-usb, lukas

Up until Titan Ridge (Thunderbolt 3) device, routers only supported
bidirectional mode. In this patch, we add to TMU, a unidirectional mode.
Unidirectional mode shall be used for enabling of low power state of the link.
The Clx enabling is implemented in the next patch of this series.

Signed-off-by: Gil Fine <gil.fine@intel.com>
---
 drivers/thunderbolt/tb.c      |   9 +-
 drivers/thunderbolt/tb.h      |  24 ++--
 drivers/thunderbolt/tb_regs.h |   2 +
 drivers/thunderbolt/tmu.c     | 244 ++++++++++++++++++++++++++++------
 4 files changed, 231 insertions(+), 48 deletions(-)

diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index a231191b06c6..533fe48e85be 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -221,7 +221,7 @@ static int tb_enable_tmu(struct tb_switch *sw)
 	int ret;
 
 	/* If it is already enabled in correct mode, don't touch it */
-	if (tb_switch_tmu_is_enabled(sw))
+	if (tb_switch_tmu_hifi_is_enabled(sw, sw->tmu.unidirect_request))
 		return 0;
 
 	ret = tb_switch_tmu_disable(sw);
@@ -669,6 +669,7 @@ static void tb_scan_port(struct tb_port *port)
 	tb_switch_lane_bonding_enable(sw);
 	/* Set the link configured */
 	tb_switch_configure_link(sw);
+	tb_switch_tmu_configure(sw, TB_SWITCH_TMU_RATE_HIFI, false);
 
 	if (tb_enable_tmu(sw))
 		tb_sw_warn(sw, "failed to enable TMU\n");
@@ -1375,6 +1376,7 @@ static int tb_start(struct tb *tb)
 		return ret;
 	}
 
+	tb_switch_tmu_configure(tb->root_switch, TB_SWITCH_TMU_RATE_HIFI, false);
 	/* Enable TMU if it is off */
 	tb_switch_tmu_enable(tb->root_switch);
 	/* Full scan to discover devices added before the driver was loaded. */
@@ -1418,6 +1420,11 @@ static void tb_restore_children(struct tb_switch *sw)
 	if (sw->is_unplugged)
 		return;
 
+	/*
+	 * tb_switch_tmu_configure() was already called when the switch was
+	 * added before entering SX/RPM, so no need to call it again
+	 * before enabling TMU.
+	 */
 	if (tb_enable_tmu(sw))
 		tb_sw_warn(sw, "failed to restore TMU configuration\n");
 
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 3fae40670b72..0205361ff89a 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -89,15 +89,24 @@ enum tb_switch_tmu_rate {
  * @cap: Offset to the TMU capability (%0 if not found)
  * @has_ucap: Does the switch support uni-directional mode
  * @rate: TMU refresh rate related to upstream switch. In case of root
- *	  switch this holds the domain rate.
+ *	  switch this holds the domain rate. Reflects the HW setting.
  * @unidirectional: Is the TMU in uni-directional or bi-directional mode
- *		    related to upstream switch. Don't case for root switch.
+ *		    related to upstream switch. Don't care for root switch.
+ *		    Reflects the HW setting.
+ * @rate_request: TMU new refresh rate related to upstream switch that is
+ *		  requested to be set. In case of root switch, this holds
+ *		  the new domain rate that is requested to be set.
+ * @unidirect_request: Is the new TMU mode: uni-directional or bi-directional
+ *		       that is requested to be set. Related to upstream switch.
+ *		       Don't care for root switch.
  */
 struct tb_switch_tmu {
 	int cap;
 	bool has_ucap;
 	enum tb_switch_tmu_rate rate;
 	bool unidirectional;
+	bool unidirect_request;
+	enum tb_switch_tmu_rate rate_request;
 };
 
 /**
@@ -891,13 +900,10 @@ int tb_switch_tmu_init(struct tb_switch *sw);
 int tb_switch_tmu_post_time(struct tb_switch *sw);
 int tb_switch_tmu_disable(struct tb_switch *sw);
 int tb_switch_tmu_enable(struct tb_switch *sw);
-
-static inline bool tb_switch_tmu_is_enabled(const struct tb_switch *sw)
-{
-	return sw->tmu.rate == TB_SWITCH_TMU_RATE_HIFI &&
-	       !sw->tmu.unidirectional;
-}
-
+bool tb_switch_tmu_hifi_is_enabled(struct tb_switch *sw, bool unidirect);
+void tb_switch_tmu_configure(struct tb_switch *sw,
+			     enum tb_switch_tmu_rate rate,
+			     bool unidirectional);
 int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged);
 int tb_port_add_nfc_credits(struct tb_port *port, int credits);
 int tb_port_clear_counter(struct tb_port *port, int counter);
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index d8ab6c820451..38bc15680d06 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -305,6 +305,8 @@ struct tb_regs_port_header {
 /* TMU adapter registers */
 #define TMU_ADP_CS_3				0x03
 #define TMU_ADP_CS_3_UDM			BIT(29)
+#define TMU_ADP_CS_6				0x06
+#define TMU_ADP_CS_6_DTS			BIT(1)
 
 /* Lane adapter registers */
 #define LANE_ADP_CS_0				0x00
diff --git a/drivers/thunderbolt/tmu.c b/drivers/thunderbolt/tmu.c
index 039c42a06000..1374813bfc5c 100644
--- a/drivers/thunderbolt/tmu.c
+++ b/drivers/thunderbolt/tmu.c
@@ -115,6 +115,11 @@ static inline int tb_port_tmu_unidirectional_disable(struct tb_port *port)
 	return tb_port_tmu_set_unidirectional(port, false);
 }
 
+static inline int tb_port_tmu_unidirectional_enable(struct tb_port *port)
+{
+	return tb_port_tmu_set_unidirectional(port, true);
+}
+
 static bool tb_port_tmu_is_unidirectional(struct tb_port *port)
 {
 	int ret;
@@ -128,6 +133,23 @@ static bool tb_port_tmu_is_unidirectional(struct tb_port *port)
 	return val & TMU_ADP_CS_3_UDM;
 }
 
+static int tb_port_tmu_time_sync(struct tb_port *port, bool time_sync)
+{
+	u32 val = time_sync ? TMU_ADP_CS_6_DTS : 0;
+
+	return tb_port_tmu_write(port, TMU_ADP_CS_6, TMU_ADP_CS_6_DTS, val);
+}
+
+static int tb_port_tmu_time_sync_disable(struct tb_port *port)
+{
+	return tb_port_tmu_time_sync(port, true);
+}
+
+static int tb_port_tmu_time_sync_enable(struct tb_port *port)
+{
+	return tb_port_tmu_time_sync(port, false);
+}
+
 static int tb_switch_tmu_set_time_disruption(struct tb_switch *sw, bool set)
 {
 	int ret;
@@ -297,6 +319,9 @@ int tb_switch_tmu_post_time(struct tb_switch *sw)
  */
 int tb_switch_tmu_disable(struct tb_switch *sw)
 {
+	struct tb_switch *sw_conf, *parent = tb_switch_parent(sw);
+	bool unidirect = tb_switch_tmu_hifi_is_enabled(sw, true);
+	struct tb_port *up, *down;
 	int ret;
 
 	if (!tb_switch_is_usb4(sw))
@@ -306,21 +331,28 @@ int tb_switch_tmu_disable(struct tb_switch *sw)
 	if (sw->tmu.rate == TB_SWITCH_TMU_RATE_OFF)
 		return 0;
 
-	if (sw->tmu.unidirectional) {
-		struct tb_switch *parent = tb_switch_parent(sw);
-		struct tb_port *up, *down;
+	up = tb_upstream_port(sw);
+	down = tb_port_at(tb_route(sw), parent);
 
-		up = tb_upstream_port(sw);
-		down = tb_port_at(tb_route(sw), parent);
+	if (tb_route(sw)) {
+		sw_conf = unidirect ? parent : sw;
+		tb_switch_tmu_rate_write(sw_conf, TB_SWITCH_TMU_RATE_OFF);
 
-		/* The switch may be unplugged so ignore any errors */
-		tb_port_tmu_unidirectional_disable(up);
-		ret = tb_port_tmu_unidirectional_disable(down);
+		tb_port_tmu_time_sync_disable(up);
+		ret = tb_port_tmu_time_sync_disable(down);
 		if (ret)
 			return ret;
-	}
 
-	tb_switch_tmu_rate_write(sw, TB_SWITCH_TMU_RATE_OFF);
+		if (unidirect) {
+			/* The switch may be unplugged so ignore any errors */
+			tb_port_tmu_unidirectional_disable(up);
+			ret = tb_port_tmu_unidirectional_disable(down);
+			if (ret)
+				return ret;
+		}
+	} else {
+		tb_switch_tmu_rate_write(sw, TB_SWITCH_TMU_RATE_OFF);
+	}
 
 	sw->tmu.unidirectional = false;
 	sw->tmu.rate = TB_SWITCH_TMU_RATE_OFF;
@@ -329,55 +361,191 @@ int tb_switch_tmu_disable(struct tb_switch *sw)
 	return 0;
 }
 
-/**
- * tb_switch_tmu_enable() - Enable TMU on a switch
- * @sw: Switch whose TMU to enable
- *
- * Enables TMU of a switch to be in bi-directional, HiFi mode. In this mode
- * all tunneling should work.
+/*
+ * This function is called when the previous TMU mode was
+ * TB_SWITCH_TMU_RATE_OFF
  */
-int tb_switch_tmu_enable(struct tb_switch *sw)
+static int __tb_switch_tmu_enable_bidir(struct tb_switch *sw)
 {
+	struct tb_switch *parent = tb_switch_parent(sw);
+	struct tb_port *up, *down;
 	int ret;
 
-	if (!tb_switch_is_usb4(sw))
-		return 0;
+	up = tb_upstream_port(sw);
+	down = tb_port_at(tb_route(sw), parent);
 
-	if (tb_switch_tmu_is_enabled(sw))
-		return 0;
+	ret = tb_port_tmu_unidirectional_enable(up);
+	if (ret)
+		return ret;
 
-	ret = tb_switch_tmu_set_time_disruption(sw, true);
+	ret = tb_port_tmu_unidirectional_enable(down);
+	if (ret)
+		goto out;
+
+	ret = tb_switch_tmu_rate_write(parent, TB_SWITCH_TMU_RATE_HIFI);
+	if (ret)
+		goto out;
+
+	ret = tb_port_tmu_time_sync_enable(up);
+	if (ret)
+		goto out;
+
+	ret = tb_port_tmu_time_sync_enable(down);
+	if (ret)
+		goto out;
+
+	return 0;
+out:
+	/*
+	 * In case of any failure in one of the steps, get back to the
+	 * TMU configurations in OFF mode. In case of additional failures in
+	 * the functions below, ignore them since we already report a failure.
+	 */
+	tb_port_tmu_time_sync_disable(up);
+	tb_switch_tmu_rate_write(parent, TB_SWITCH_TMU_RATE_OFF);
+	tb_port_tmu_unidirectional_disable(down);
+	tb_port_tmu_unidirectional_disable(up);
+	return ret;
+}
+
+/*
+ * This function is called when the previous TMU mode was
+ * TB_SWITCH_TMU_RATE_OFF
+ */
+static int __tb_switch_tmu_enable_uni(struct tb_switch *sw)
+{
+	struct tb_switch *parent = tb_switch_parent(sw);
+	struct tb_port *up, *down;
+	int ret;
+
+	up = tb_upstream_port(sw);
+	down = tb_port_at(tb_route(sw), parent);
+	ret = tb_switch_tmu_rate_write(parent, TB_SWITCH_TMU_RATE_HIFI);
 	if (ret)
 		return ret;
 
-	/* Change mode to bi-directional */
-	if (tb_route(sw) && sw->tmu.unidirectional) {
-		struct tb_switch *parent = tb_switch_parent(sw);
-		struct tb_port *up, *down;
+	ret = tb_port_tmu_unidirectional_enable(up);
+	if (ret)
+		goto out;
+
+	ret = tb_port_tmu_time_sync_enable(up);
+	if (ret)
+		goto out;
 
-		up = tb_upstream_port(sw);
-		down = tb_port_at(tb_route(sw), parent);
+	ret = tb_port_tmu_unidirectional_enable(down);
+	if (ret)
+		goto out;
 
-		ret = tb_port_tmu_unidirectional_disable(down);
-		if (ret)
-			return ret;
+	ret = tb_port_tmu_time_sync_enable(down);
+	if (ret)
+		goto out;
 
-		ret = tb_switch_tmu_rate_write(sw, TB_SWITCH_TMU_RATE_HIFI);
-		if (ret)
-			return ret;
+	return 0;
+out:
+	/*
+	 * In case of any failure in one of the steps, get back to the
+	 * TMU configurations in OFF mode. In case of additional failures in
+	 * the functions below, ignore them since we already report a failure.
+	 */
+	tb_port_tmu_unidirectional_disable(down);
+	tb_port_tmu_time_sync_disable(up);
+	tb_port_tmu_unidirectional_disable(up);
+	tb_switch_tmu_rate_write(parent, TB_SWITCH_TMU_RATE_OFF);
+	return ret;
+}
 
-		ret = tb_port_tmu_unidirectional_disable(up);
-		if (ret)
-			return ret;
+static int tb_switch_tmu_hifi_enable(struct tb_switch *sw)
+{
+	bool unidirect = sw->tmu.unidirect_request;
+	int ret;
+
+	if (unidirect && !sw->tmu.has_ucap)
+		return -EOPNOTSUPP;
+
+	if (!tb_switch_is_usb4(sw))
+		return 0;
+
+	if (tb_switch_tmu_hifi_is_enabled(sw, sw->tmu.unidirect_request))
+		return 0;
+
+	ret = tb_switch_tmu_set_time_disruption(sw, true);
+	if (ret)
+		return ret;
+
+	if (tb_route(sw)) {
+		/* The used mode changes are from OFF to HiFi-Uni/HiFi-BiDir */
+		if (sw->tmu.rate == TB_SWITCH_TMU_RATE_OFF) {
+			ret = unidirect ? __tb_switch_tmu_enable_uni(sw)
+					 : __tb_switch_tmu_enable_bidir(sw);
+			if (ret)
+				return ret;
+		}
+		sw->tmu.unidirectional = unidirect;
 	} else {
+		/*
+		 * Host-router port configurations are written as
+		 * configurations for down-stream port of the parent of the
+		 * child node - see above.
+		 * Here only the host's router rate configuration is written
+		 */
 		ret = tb_switch_tmu_rate_write(sw, TB_SWITCH_TMU_RATE_HIFI);
 		if (ret)
 			return ret;
 	}
 
-	sw->tmu.unidirectional = false;
 	sw->tmu.rate = TB_SWITCH_TMU_RATE_HIFI;
 	tb_sw_dbg(sw, "TMU: mode set to: %s\n", tb_switch_tmu_mode_name(sw));
 
 	return tb_switch_tmu_set_time_disruption(sw, false);
 }
+
+/**
+ * tb_switch_tmu_enable() - Enable TMU on a switch
+ * @sw: Switch whose TMU to enable
+ *
+ * Enables TMU of a switch to be in unidirectional or bidirectional HiFi mode.
+ * Calling tb_switch_tmu_configure() is required before calling this function,
+ * to select the mode HiFi and directionality (unidirectional/bidirectional).
+ * In both modes all tunneling should work. Unidirectional mode is required for
+ * CLx (Link Low-Power) to work. LowRes mode is not used currently.
+ */
+int tb_switch_tmu_enable(struct tb_switch *sw)
+{
+	if (sw->tmu.rate_request == TB_SWITCH_TMU_RATE_NORMAL)
+		return -EOPNOTSUPP;
+
+	return tb_switch_tmu_hifi_enable(sw);
+}
+
+/**
+ * tb_switch_tmu_configure() - Configure the TMU rate and directionality
+ * @sw: Switch whose mode to change
+ * @rate: Rate to configure Off/LowRes/HiFi
+ * @unidirectional: Unidirectionality selection: Unidirectional or Bidirectional
+ *
+ * Selects the rate of the TMU (Off, LowRes, HiFi), and Directionality
+ * (Unidirectional or Bidirectional)
+ * Shall be called before tb_switch_tmu_enable()
+ */
+void tb_switch_tmu_configure(struct tb_switch *sw,
+			     enum tb_switch_tmu_rate rate, bool unidirectional)
+{
+	sw->tmu.unidirect_request = unidirectional;
+	sw->tmu.rate_request = rate;
+}
+
+/**
+ * tb_switch_tmu_hifi_is_enabled() - Checks if the specified TMU mode
+ *				     bidir/uni enabled correctly
+ * @sw: Switch whose TMU mode to check
+ * @unidirect: Select bidirectional or unidirectional mode to check
+ *
+ * Read TMU directionality and rate from HW, and return true,
+ * if matches to bidirectional/unidirectional HiFi mode settings.
+ * Otherwise returns false.
+ */
+bool tb_switch_tmu_hifi_is_enabled(struct tb_switch *sw, bool unidirect)
+{
+	return sw->tmu.rate == TB_SWITCH_TMU_RATE_HIFI &&
+			       sw->tmu.unidirectional == unidirect;
+}
-- 
2.17.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/7] thunderbolt: Add CL0s support for USB4
  2021-11-25 14:38 [PATCH 0/7] thunderbolt: CLx support for USB4 and Titan Ridge Gil Fine
  2021-11-25 14:38 ` [PATCH 1/7] thunderbolt: Add TMU unidirectional mode Gil Fine
@ 2021-11-25 14:38 ` Gil Fine
  2021-11-29  8:38   ` Mika Westerberg
  2021-11-25 14:38 ` [PATCH 3/7] thunderbolt: Move usb4_switch_wait_for_bit() to switch.c Gil Fine
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: Gil Fine @ 2021-11-25 14:38 UTC (permalink / raw)
  To: andreas.noever, michael.jamet, mika.westerberg, YehezkelShB
  Cc: gil.fine, linux-usb, lukas

In this patch we add enabling of Low Power state (CLx) of the link.
Low Power states are used to reduce transmitter and receiver power
when a Lane is idle.
For now we only enable CL0s state. We enable it, if both sides of the link
support it and only for the first hop router.
(i.e. the first device that connected to the host router).

Signed-off-by: Gil Fine <gil.fine@intel.com>
---
 drivers/thunderbolt/switch.c  | 291 ++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/tb.c      |   9 +-
 drivers/thunderbolt/tb.h      |  29 ++++
 drivers/thunderbolt/tb_regs.h |   6 +
 drivers/thunderbolt/usb4.c    |  20 +++
 5 files changed, 354 insertions(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 13f9230104d7..05aaa34bbfd9 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -3223,3 +3223,294 @@ 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);
+	struct tb_port *up, *down;
+	int ret;
+
+	if (!tb_route(sw))
+		return 0;
+
+	up = tb_upstream_port(sw);
+	down = tb_port_at(tb_route(sw), parent);
+	ret = tb_port_pm_secondary_enable(up);
+	if (ret)
+		return ret;
+
+	ret = tb_port_pm_secondary_disable(down);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+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 (!usb4_port_clx_supported(port))
+		return false;
+
+	switch (clx) {
+	case TB_CL0S:
+		/* CL0s support requires also CL1 support */
+		mask = LANE_ADP_CS_0_CL0S_SUPPORT | LANE_ADP_CS_0_CL1_SUPPORT;
+		break;
+
+	/* For now we support only CL0s. Not CL1, CL2. */
+	case TB_CL1:
+	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 inline bool tb_port_cl0s_supported(struct tb_port *port)
+{
+	return tb_port_clx_supported(port, TB_CL0S);
+}
+
+static int __tb_port_cl0s_set(struct tb_port *port, bool enable)
+{
+	u32 phy, mask;
+	int ret;
+
+	/* To enable CL0s, also required to enable CL1 */
+	mask = LANE_ADP_CS_1_CL0S_ENABLE | LANE_ADP_CS_1_CL1_ENABLE;
+	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_cl0s_disable(struct tb_port *port)
+{
+	return __tb_port_cl0s_set(port, false);
+}
+
+static int tb_port_cl0s_enable(struct tb_port *port)
+{
+	return __tb_port_cl0s_set(port, true);
+}
+
+static int tb_switch_enable_cl0s(struct tb_switch *sw)
+{
+	struct tb_switch *parent = tb_switch_parent(sw);
+	bool up_cl0s_support, down_cl0s_support;
+	struct tb_port *up, *down;
+	int ret;
+
+	if (!tb_switch_is_usb4(sw))
+		return 0;
+
+	/*
+	 * Enable CLx for host router's downstream port as part of the
+	 * downstream router enabling procedure.
+	 */
+	if (!tb_route(sw))
+		return 0;
+
+	/* Enable CLx only for first hop router (depth = 1) */
+	if (tb_route(parent))
+		return 0;
+
+	if (tb_switch_pm_secondary_resolve(sw))
+		return -EINVAL;
+
+	up = tb_upstream_port(sw);
+	down = tb_port_at(tb_route(sw), parent);
+
+	up_cl0s_support = tb_port_cl0s_supported(up);
+	down_cl0s_support = tb_port_cl0s_supported(down);
+
+	tb_port_dbg(up, "CL0s %ssupported\n",
+		    up_cl0s_support ? "" : "not ");
+	tb_port_dbg(down, "CL0s %ssupported\n",
+		    down_cl0s_support ? "" : "not ");
+
+	if (!up_cl0s_support || !down_cl0s_support)
+		return -EOPNOTSUPP;
+
+	ret = tb_port_cl0s_enable(up);
+	if (ret)
+		return ret;
+
+	ret = tb_port_cl0s_enable(down);
+	if (ret)
+		goto out;
+
+	sw->clx = TB_CL0S;
+	tb_sw_dbg(sw, "enabled CL0s on upstream port\n");
+	return 0;
+out:
+	tb_port_cl0s_disable(up);
+	return ret;
+}
+
+/**
+ * tb_switch_enable_clx() - Enable CLx on upstream port of specified router
+ * @sw: The switch to enable CLx for
+ * @clx: The CLx state to enable
+ *
+ * Enable CLx state only for first hop router. This is because of the HW
+ * limitation on Intel platforms.
+ * CLx is enabled only if both sides of the link support CLx, and if
+ * both sides of the link are not configured as two single lane links
+ * and only if the link is not inter-domain link.
+ * The conditions are descibed in CM Guide 1.0 section 8.1.
+ *
+ * Return: Returns 0 on success or an error code on failure.
+ */
+int tb_switch_enable_clx(struct tb_switch *sw, enum tb_clx clx)
+{
+	struct tb_switch *root_sw = sw->tb->root_switch;
+
+	/* CLx is not enabled and validated on USB4 platforms before ADL */
+	if (root_sw->generation < 4 ||
+	    tb_switch_is_tiger_lake(root_sw))
+		return 0;
+
+	switch (clx) {
+	case TB_CL0S:
+		return tb_switch_enable_cl0s(sw);
+
+	case TB_CL1:
+	case TB_CL2:
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int tb_switch_disable_cl0s(struct tb_switch *sw)
+{
+	struct tb_switch *parent = tb_switch_parent(sw);
+	struct tb_port *up, *down;
+	int ret;
+
+	if (!tb_switch_is_usb4(sw))
+		return 0;
+
+	/*
+	 * Disable CLx for host router's downstream port as part of the
+	 * downstream router enabling procedure.
+	 */
+	if (!tb_route(sw))
+		return 0;
+
+	/* Disable CLx only for first hop router (depth = 1) */
+	if (tb_route(parent))
+		return 0;
+
+	up = tb_upstream_port(sw);
+	down = tb_port_at(tb_route(sw), parent);
+	ret = tb_port_cl0s_disable(up);
+	if (ret)
+		return ret;
+
+	ret = tb_port_cl0s_disable(down);
+	if (ret)
+		return ret;
+
+	sw->clx = TB_CLX_DISABLE;
+	tb_sw_dbg(sw, "disabled CL0s on upstream port\n");
+	return 0;
+}
+
+/**
+ * tb_switch_disable_clx() - Disable CLx on upstream port of specified router
+ * @sw: The switch to disable CLx for
+ * @clx: The CLx state to disable
+ *
+ * Return: Returns 0 on success or an error code on failure.
+ */
+int tb_switch_disable_clx(struct tb_switch *sw, enum tb_clx clx)
+{
+	switch (clx) {
+	case TB_CL0S:
+		return tb_switch_disable_cl0s(sw);
+
+	case TB_CL1:
+	case TB_CL2:
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+/**
+ * tb_switch_is_clx_enabled() - Checks if the CLx is enabled
+ * @sw: Router to check the CLx state for
+ *
+ * Checks if the CLx is enabled on the router upstream link.
+ * Not applicable for a host router.
+ */
+bool tb_switch_is_clx_enabled(struct tb_switch *sw)
+{
+	return sw->clx != TB_CLX_DISABLE;
+}
+
+/**
+ * tb_switch_is_cl0s_enabled() - Checks if the CL0s is enabled
+ * @sw: Router to check the CLx state for
+ *
+ * Checks if the CL0s is enabled on the router upstream link.
+ * Not applicable for a host router.
+ */
+bool tb_switch_is_cl0s_enabled(struct tb_switch *sw)
+{
+	return sw->clx == TB_CL0S;
+}
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 533fe48e85be..f241d42c1c6e 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -669,7 +669,11 @@ static void tb_scan_port(struct tb_port *port)
 	tb_switch_lane_bonding_enable(sw);
 	/* Set the link configured */
 	tb_switch_configure_link(sw);
-	tb_switch_tmu_configure(sw, TB_SWITCH_TMU_RATE_HIFI, false);
+	if (tb_switch_enable_clx(sw, TB_CL0S))
+		tb_sw_warn(sw, "failed to enable CLx on upstream port\n");
+
+	tb_switch_tmu_configure(sw, TB_SWITCH_TMU_RATE_HIFI,
+				tb_switch_is_clx_enabled(sw) ? true : false);
 
 	if (tb_enable_tmu(sw))
 		tb_sw_warn(sw, "failed to enable TMU\n");
@@ -1420,6 +1424,9 @@ static void tb_restore_children(struct tb_switch *sw)
 	if (sw->is_unplugged)
 		return;
 
+	if (tb_switch_enable_clx(sw, TB_CL0S))
+		tb_sw_warn(sw, "failed to re-enable CLx on upstream port\n");
+
 	/*
 	 * tb_switch_tmu_configure() was already called when the switch was
 	 * added before entering SX/RPM, so no need to call it again
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 0205361ff89a..e7792b090bc7 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -109,6 +109,13 @@ struct tb_switch_tmu {
 	enum tb_switch_tmu_rate rate_request;
 };
 
+enum tb_clx {
+	TB_CLX_DISABLE,
+	TB_CL0S,
+	TB_CL1,
+	TB_CL2,
+};
+
 /**
  * struct tb_switch - a thunderbolt switch
  * @dev: Device for the switch
@@ -157,6 +164,7 @@ struct tb_switch_tmu {
  * @min_dp_main_credits: Router preferred minimum number of buffers for DP MAIN
  * @max_pcie_credits: Router preferred number of buffers for PCIe
  * @max_dma_credits: Router preferred number of buffers for DMA/P2P
+ * @clx: CLx state on the upstream link of the switch
  *
  * When the switch is being added or removed to the domain (other
  * switches) you need to have domain lock held.
@@ -205,6 +213,7 @@ struct tb_switch {
 	unsigned int min_dp_main_credits;
 	unsigned int max_pcie_credits;
 	unsigned int max_dma_credits;
+	enum tb_clx clx;
 };
 
 /**
@@ -862,6 +871,20 @@ static inline bool tb_switch_is_titan_ridge(const struct tb_switch *sw)
 	return false;
 }
 
+static inline bool tb_switch_is_tiger_lake(const struct tb_switch *sw)
+{
+	if (sw->config.vendor_id == PCI_VENDOR_ID_INTEL) {
+		switch (sw->config.device_id) {
+		case PCI_DEVICE_ID_INTEL_TGL_NHI0:
+		case PCI_DEVICE_ID_INTEL_TGL_NHI1:
+		case PCI_DEVICE_ID_INTEL_TGL_H_NHI0:
+		case PCI_DEVICE_ID_INTEL_TGL_H_NHI1:
+			return true;
+		}
+	}
+	return false;
+}
+
 /**
  * tb_switch_is_usb4() - Is the switch USB4 compliant
  * @sw: Switch to check
@@ -904,6 +927,11 @@ bool tb_switch_tmu_hifi_is_enabled(struct tb_switch *sw, bool unidirect);
 void tb_switch_tmu_configure(struct tb_switch *sw,
 			     enum tb_switch_tmu_rate rate,
 			     bool unidirectional);
+
+int tb_switch_enable_clx(struct tb_switch *sw, enum tb_clx clx);
+int tb_switch_disable_clx(struct tb_switch *sw, enum tb_clx clx);
+bool tb_switch_is_clx_enabled(struct tb_switch *sw);
+
 int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged);
 int tb_port_add_nfc_credits(struct tb_port *port, int credits);
 int tb_port_clear_counter(struct tb_port *port, int counter);
@@ -1126,6 +1154,7 @@ static inline struct usb4_port *tb_to_usb4_port_device(struct device *dev)
 struct usb4_port *usb4_port_device_add(struct tb_port *port);
 void usb4_port_device_remove(struct usb4_port *usb4);
 int usb4_port_device_resume(struct usb4_port *usb4);
+bool usb4_port_clx_supported(struct tb_port *port);
 
 /* Keep link controller awake during update */
 #define QUIRK_FORCE_POWER_LINK_CONTROLLER		BIT(0)
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 38bc15680d06..8b432d3e3c6f 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -312,11 +312,15 @@ struct tb_regs_port_header {
 #define LANE_ADP_CS_0				0x00
 #define LANE_ADP_CS_0_SUPPORTED_WIDTH_MASK	GENMASK(25, 20)
 #define LANE_ADP_CS_0_SUPPORTED_WIDTH_SHIFT	20
+#define LANE_ADP_CS_0_CL0S_SUPPORT		BIT(26)
+#define LANE_ADP_CS_0_CL1_SUPPORT		BIT(27)
 #define LANE_ADP_CS_1				0x01
 #define LANE_ADP_CS_1_TARGET_WIDTH_MASK		GENMASK(9, 4)
 #define LANE_ADP_CS_1_TARGET_WIDTH_SHIFT	4
 #define LANE_ADP_CS_1_TARGET_WIDTH_SINGLE	0x1
 #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_LD			BIT(14)
 #define LANE_ADP_CS_1_LB			BIT(15)
 #define LANE_ADP_CS_1_CURRENT_SPEED_MASK	GENMASK(19, 16)
@@ -325,6 +329,7 @@ struct tb_regs_port_header {
 #define LANE_ADP_CS_1_CURRENT_SPEED_GEN3	0x4
 #define LANE_ADP_CS_1_CURRENT_WIDTH_MASK	GENMASK(25, 20)
 #define LANE_ADP_CS_1_CURRENT_WIDTH_SHIFT	20
+#define LANE_ADP_CS_1_PMS			BIT(30)
 
 /* USB4 port registers */
 #define PORT_CS_1				0x01
@@ -340,6 +345,7 @@ struct tb_regs_port_header {
 #define PORT_CS_18				0x12
 #define PORT_CS_18_BE				BIT(8)
 #define PORT_CS_18_TCM				BIT(9)
+#define PORT_CS_18_CPS				BIT(10)
 #define PORT_CS_18_WOU4S			BIT(18)
 #define PORT_CS_19				0x13
 #define PORT_CS_19_PC				BIT(3)
diff --git a/drivers/thunderbolt/usb4.c b/drivers/thunderbolt/usb4.c
index ceddbe7e9f93..704820c9a042 100644
--- a/drivers/thunderbolt/usb4.c
+++ b/drivers/thunderbolt/usb4.c
@@ -2029,3 +2029,23 @@ int usb4_usb3_port_release_bandwidth(struct tb_port *port, int *upstream_bw,
 	usb4_usb3_port_clear_cm_request(port);
 	return ret;
 }
+
+/**
+ * usb4_port_clx_supported() - Check if CLx is supported by the link
+ * @port: Port to check for CLx support for
+ *
+ * PORT_CS_18_CPS bit reflects if the link supports CLx including
+ * active cables (if connected on the link).
+ */
+bool usb4_port_clx_supported(struct tb_port *port)
+{
+	int ret;
+	u32 val;
+
+	ret = tb_port_read(port, &val, TB_CFG_PORT,
+			   port->cap_usb4 + PORT_CS_18, 1);
+	if (ret)
+		return false;
+
+	return !!(val & PORT_CS_18_CPS);
+}
-- 
2.17.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/7] thunderbolt: Move usb4_switch_wait_for_bit() to switch.c
  2021-11-25 14:38 [PATCH 0/7] thunderbolt: CLx support for USB4 and Titan Ridge Gil Fine
  2021-11-25 14:38 ` [PATCH 1/7] thunderbolt: Add TMU unidirectional mode Gil Fine
  2021-11-25 14:38 ` [PATCH 2/7] thunderbolt: Add CL0s support for USB4 Gil Fine
@ 2021-11-25 14:38 ` Gil Fine
  2021-11-25 14:38 ` [PATCH 4/7] thunderbolt: Enable TMU for Titan Ridge device Gil Fine
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Gil Fine @ 2021-11-25 14:38 UTC (permalink / raw)
  To: andreas.noever, michael.jamet, mika.westerberg, YehezkelShB
  Cc: gil.fine, linux-usb, lukas

Currently usb4_switch_wait_for_bit() used only in usb4.c
Moving to switch.c to call it from other files.
Also change the prefix to "tb_" to follow to the naming convention.

Signed-off-by: Gil Fine <gil.fine@intel.com>
---
 drivers/thunderbolt/switch.c | 36 +++++++++++++++++++++++++++++++++++-
 drivers/thunderbolt/tb.h     |  2 ++
 drivers/thunderbolt/usb4.c   | 32 +++++---------------------------
 3 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 05aaa34bbfd9..754ff79e48ab 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -643,7 +643,7 @@ int tb_port_add_nfc_credits(struct tb_port *port, int credits)
  * @port: Port whose counters to clear
  * @counter: Counter index to clear
  *
- * Return: Returns 0 on success or an error code on failure.
+ * Return:  success or an error code on failure.
  */
 int tb_port_clear_counter(struct tb_port *port, int counter)
 {
@@ -3224,6 +3224,40 @@ struct tb_port *tb_switch_find_port(struct tb_switch *sw,
 	return NULL;
 }
 
+/**
+ * tb_switch_wait_for_bit() - Wait for specified value of bits in offset
+ * @sw: Switch to read the offset value from
+ * @offset: Offset in the router config space to read from
+ * @bit: Bit mask in the offset to wait for
+ * @value: Value of the bits to wait for
+ * @timeout_msec: Timeout in ms how long to wait
+ *
+ * Wait till the specified bits in specified offset reach specified value.
+ * Returns %0 in case of success, %-ETIMEDOUT if the @value was not reached
+ * within the given timeout or a negative errno in case of failure.
+ */
+int tb_switch_wait_for_bit(struct tb_switch *sw, u32 offset, u32 bit,
+			   u32 value, int timeout_msec)
+{
+	ktime_t timeout = ktime_add_ms(ktime_get(), timeout_msec);
+
+	do {
+		u32 val;
+		int ret;
+
+		ret = tb_sw_read(sw, &val, TB_CFG_SWITCH, offset, 1);
+		if (ret)
+			return ret;
+
+		if ((val & bit) == value)
+			return 0;
+
+		usleep_range(50, 100);
+	} while (ktime_before(ktime_get(), timeout));
+
+	return -ETIMEDOUT;
+}
+
 static int __tb_port_pm_secondary_set(struct tb_port *port, bool secondary)
 {
 	u32 phy;
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index e7792b090bc7..3601c2b5e4bc 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -1155,6 +1155,8 @@ struct usb4_port *usb4_port_device_add(struct tb_port *port);
 void usb4_port_device_remove(struct usb4_port *usb4);
 int usb4_port_device_resume(struct usb4_port *usb4);
 bool usb4_port_clx_supported(struct tb_port *port);
+int tb_switch_wait_for_bit(struct tb_switch *sw, u32 offset, u32 bit,
+			   u32 value, int timeout_msec);
 
 /* Keep link controller awake during update */
 #define QUIRK_FORCE_POWER_LINK_CONTROLLER		BIT(0)
diff --git a/drivers/thunderbolt/usb4.c b/drivers/thunderbolt/usb4.c
index 704820c9a042..64613cafcbc3 100644
--- a/drivers/thunderbolt/usb4.c
+++ b/drivers/thunderbolt/usb4.c
@@ -50,28 +50,6 @@ enum usb4_ba_index {
 #define USB4_BA_VALUE_MASK		GENMASK(31, 16)
 #define USB4_BA_VALUE_SHIFT		16
 
-static int usb4_switch_wait_for_bit(struct tb_switch *sw, u32 offset, u32 bit,
-				    u32 value, int timeout_msec)
-{
-	ktime_t timeout = ktime_add_ms(ktime_get(), timeout_msec);
-
-	do {
-		u32 val;
-		int ret;
-
-		ret = tb_sw_read(sw, &val, TB_CFG_SWITCH, offset, 1);
-		if (ret)
-			return ret;
-
-		if ((val & bit) == value)
-			return 0;
-
-		usleep_range(50, 100);
-	} while (ktime_before(ktime_get(), timeout));
-
-	return -ETIMEDOUT;
-}
-
 static int usb4_native_switch_op(struct tb_switch *sw, u16 opcode,
 				 u32 *metadata, u8 *status,
 				 const void *tx_data, size_t tx_dwords,
@@ -97,7 +75,7 @@ static int usb4_native_switch_op(struct tb_switch *sw, u16 opcode,
 	if (ret)
 		return ret;
 
-	ret = usb4_switch_wait_for_bit(sw, ROUTER_CS_26, ROUTER_CS_26_OV, 0, 500);
+	ret = tb_switch_wait_for_bit(sw, ROUTER_CS_26, ROUTER_CS_26_OV, 0, 500);
 	if (ret)
 		return ret;
 
@@ -303,8 +281,8 @@ int usb4_switch_setup(struct tb_switch *sw)
 	if (ret)
 		return ret;
 
-	return usb4_switch_wait_for_bit(sw, ROUTER_CS_6, ROUTER_CS_6_CR,
-					ROUTER_CS_6_CR, 50);
+	return tb_switch_wait_for_bit(sw, ROUTER_CS_6, ROUTER_CS_6_CR,
+				      ROUTER_CS_6_CR, 50);
 }
 
 /**
@@ -480,8 +458,8 @@ int usb4_switch_set_sleep(struct tb_switch *sw)
 	if (ret)
 		return ret;
 
-	return usb4_switch_wait_for_bit(sw, ROUTER_CS_6, ROUTER_CS_6_SLPR,
-					ROUTER_CS_6_SLPR, 500);
+	return tb_switch_wait_for_bit(sw, ROUTER_CS_6, ROUTER_CS_6_SLPR,
+				      ROUTER_CS_6_SLPR, 500);
 }
 
 /**
-- 
2.17.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/7] thunderbolt: Enable TMU for Titan Ridge device
  2021-11-25 14:38 [PATCH 0/7] thunderbolt: CLx support for USB4 and Titan Ridge Gil Fine
                   ` (2 preceding siblings ...)
  2021-11-25 14:38 ` [PATCH 3/7] thunderbolt: Move usb4_switch_wait_for_bit() to switch.c Gil Fine
@ 2021-11-25 14:38 ` Gil Fine
  2021-11-25 14:38 ` [PATCH 5/7] thunderbolt: Rename Intel VSC capability Gil Fine
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Gil Fine @ 2021-11-25 14:38 UTC (permalink / raw)
  To: andreas.noever, michael.jamet, mika.westerberg, YehezkelShB
  Cc: gil.fine, linux-usb, lukas

In this patch we enable TMU for Titan Ridge device.
This is used once needed to enable CLx.

Signed-off-by: Gil Fine <gil.fine@intel.com>
---
 drivers/thunderbolt/switch.c  |  4 ++++
 drivers/thunderbolt/tb.h      |  2 ++
 drivers/thunderbolt/tb_regs.h |  4 ++++
 drivers/thunderbolt/tmu.c     | 18 +++++++++++++-----
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 754ff79e48ab..6ff2866a4c2b 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -2194,6 +2194,10 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent,
 	if (ret > 0)
 		sw->cap_plug_events = ret;
 
+	ret = tb_switch_find_vse_cap(sw, TB_VSE_CAP_TIME2);
+	if (ret > 0)
+		sw->cap_vsec_tmu = ret;
+
 	ret = tb_switch_find_vse_cap(sw, TB_VSE_CAP_LINK_CONTROLLER);
 	if (ret > 0)
 		sw->cap_lc = ret;
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 3601c2b5e4bc..0fcc279277df 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -138,6 +138,7 @@ enum tb_clx {
  * @link_usb4: Upstream link is USB4
  * @generation: Switch Thunderbolt generation
  * @cap_plug_events: Offset to the plug events capability (%0 if not found)
+ * @cap_vsec_tmu: Offset to the TMU vendor specific capability (%0 if not found)
  * @cap_lc: Offset to the link controller capability (%0 if not found)
  * @is_unplugged: The switch is going away
  * @drom: DROM of the switch (%NULL if not found)
@@ -189,6 +190,7 @@ struct tb_switch {
 	bool link_usb4;
 	unsigned int generation;
 	int cap_plug_events;
+	int cap_vsec_tmu;
 	int cap_lc;
 	bool is_unplugged;
 	u8 *drom;
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 8b432d3e3c6f..2ceed608573f 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -446,6 +446,10 @@ struct tb_regs_hop {
 	u32 unknown3:3; /* set to zero */
 } __packed;
 
+/* TMU Thunderbolt 3 registers */
+#define TB_TIME_VSEC_3_CS_26		0x1a
+#define TB_TIME_VSEC_3_CS_26_TD		BIT(22)
+
 /* Common link controller registers */
 #define TB_LC_DESC			0x02
 #define TB_LC_DESC_NLC_MASK		GENMASK(3, 0)
diff --git a/drivers/thunderbolt/tmu.c b/drivers/thunderbolt/tmu.c
index 1374813bfc5c..bbb5b1df8409 100644
--- a/drivers/thunderbolt/tmu.c
+++ b/drivers/thunderbolt/tmu.c
@@ -152,21 +152,29 @@ static int tb_port_tmu_time_sync_enable(struct tb_port *port)
 
 static int tb_switch_tmu_set_time_disruption(struct tb_switch *sw, bool set)
 {
+	u32 val, offset, bit;
 	int ret;
-	u32 val;
+
+	if (tb_switch_is_usb4(sw)) {
+		offset = sw->tmu.cap + TMU_RTR_CS_0;
+		bit = TMU_RTR_CS_0_TD;
+	} else {
+		offset = sw->cap_vsec_tmu + TB_TIME_VSEC_3_CS_26;
+		bit = TB_TIME_VSEC_3_CS_26_TD;
+	}
 
 	ret = tb_sw_read(sw, &val, TB_CFG_SWITCH,
-			 sw->tmu.cap + TMU_RTR_CS_0, 1);
+			 offset, 1);
 	if (ret)
 		return ret;
 
 	if (set)
-		val |= TMU_RTR_CS_0_TD;
+		val |= bit;
 	else
-		val &= ~TMU_RTR_CS_0_TD;
+		val &= ~bit;
 
 	return tb_sw_write(sw, &val, TB_CFG_SWITCH,
-			   sw->tmu.cap + TMU_RTR_CS_0, 1);
+			   offset, 1);
 }
 
 /**
-- 
2.17.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/7] thunderbolt: Rename Intel VSC capability
  2021-11-25 14:38 [PATCH 0/7] thunderbolt: CLx support for USB4 and Titan Ridge Gil Fine
                   ` (3 preceding siblings ...)
  2021-11-25 14:38 ` [PATCH 4/7] thunderbolt: Enable TMU for Titan Ridge device Gil Fine
@ 2021-11-25 14:38 ` Gil Fine
  2021-11-25 14:38 ` [PATCH 6/7] thunderbolt: Enable CLx for Titan Ridge device Gil Fine
  2021-11-25 14:38 ` [PATCH 7/7] thunderbolt: Add kernel param for CLx disabling Gil Fine
  6 siblings, 0 replies; 12+ messages in thread
From: Gil Fine @ 2021-11-25 14:38 UTC (permalink / raw)
  To: andreas.noever, michael.jamet, mika.westerberg, YehezkelShB
  Cc: gil.fine, linux-usb, lukas

Rename the VSC capability: TB_VSE_CAP_IECS to TB_VSE_CAP_CP_LP to follow the
Intel devices namings as appear in the datasheet.
This capability is used for controlling CLx (Low Power states of the link).

Signed-off-by: Gil Fine <gil.fine@intel.com>
---
 drivers/thunderbolt/tb_regs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 2ceed608573f..08f1213ea565 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -33,7 +33,7 @@ enum tb_switch_cap {
 enum tb_switch_vse_cap {
 	TB_VSE_CAP_PLUG_EVENTS		= 0x01, /* also EEPROM */
 	TB_VSE_CAP_TIME2		= 0x03,
-	TB_VSE_CAP_IECS			= 0x04,
+	TB_VSE_CAP_CP_LP		= 0x04,
 	TB_VSE_CAP_LINK_CONTROLLER	= 0x06, /* also IECS */
 };
 
-- 
2.17.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/7] thunderbolt: Enable CLx for Titan Ridge device
  2021-11-25 14:38 [PATCH 0/7] thunderbolt: CLx support for USB4 and Titan Ridge Gil Fine
                   ` (4 preceding siblings ...)
  2021-11-25 14:38 ` [PATCH 5/7] thunderbolt: Rename Intel VSC capability Gil Fine
@ 2021-11-25 14:38 ` Gil Fine
  2021-11-29  8:28   ` Mika Westerberg
  2021-11-25 14:38 ` [PATCH 7/7] thunderbolt: Add kernel param for CLx disabling Gil Fine
  6 siblings, 1 reply; 12+ messages in thread
From: Gil Fine @ 2021-11-25 14:38 UTC (permalink / raw)
  To: andreas.noever, michael.jamet, mika.westerberg, YehezkelShB
  Cc: gil.fine, linux-usb, lukas

In this patch we enable Low Power state (CLx) of the link for Titan Ridge
device.
Low Power states are used to reduce transmitter and receiver power
when a Lane is idle and this way to save power.
For now we only enable CL0s state. We enable it, if both sides of the link
support it and only for the first depth device.
(i.e. the first device that connected to the Host router).

Signed-off-by: Gil Fine <gil.fine@intel.com>
---
 drivers/thunderbolt/lc.c      |  27 +++++
 drivers/thunderbolt/switch.c  | 191 +++++++++++++++++++++++++++++++++-
 drivers/thunderbolt/tb.c      |   7 ++
 drivers/thunderbolt/tb.h      |   7 ++
 drivers/thunderbolt/tb_regs.h |  35 ++++++-
 drivers/thunderbolt/tmu.c     |  68 +++++++++++-
 6 files changed, 328 insertions(+), 7 deletions(-)

diff --git a/drivers/thunderbolt/lc.c b/drivers/thunderbolt/lc.c
index c178f0d7beab..29ead7d75d9c 100644
--- a/drivers/thunderbolt/lc.c
+++ b/drivers/thunderbolt/lc.c
@@ -508,3 +508,30 @@ int tb_lc_force_power(struct tb_switch *sw)
 
 	return tb_sw_write(sw, &in, TB_CFG_SWITCH, TB_LC_POWER, 1);
 }
+
+/**
+ * tb_lc_clx_supported() - Check whether CLx is supported by the link
+ * @sw: Port to check
+ *
+ * TB_LC_LINK_ATTR_CPS bit reflects if the link supports CLx including
+ * active cables (if connected on the link).
+ */
+bool tb_lc_clx_supported(struct tb_port *port)
+{
+	struct tb_switch *sw = port->sw;
+	int cap, ret;
+	u32 val;
+
+	if (!tb_switch_is_titan_ridge(sw))
+		return false;
+
+	cap = find_port_lc_cap(port);
+	if (cap < 0)
+		return false;
+
+	ret = tb_sw_read(sw, &val, TB_CFG_SWITCH, cap + TB_LC_LINK_ATTR, 1);
+	if (ret)
+		return false;
+
+	return !!(val & TB_LC_LINK_ATTR_CPS);
+}
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 6ff2866a4c2b..945672b8fa25 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -2202,6 +2202,10 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct device *parent,
 	if (ret > 0)
 		sw->cap_lc = ret;
 
+	ret = tb_switch_find_vse_cap(sw, TB_VSE_CAP_CP_LP);
+	if (ret > 0)
+		sw->cap_lp = ret;
+
 	/* Root switch is always authorized */
 	if (!route)
 		sw->authorized = true;
@@ -3008,6 +3012,14 @@ void tb_switch_suspend(struct tb_switch *sw, bool runtime)
 
 	tb_sw_dbg(sw, "suspending switch\n");
 
+	/*
+	 * Actually shall be done only for Titan Ridge,
+	 * but for simplicity can be done for USB4 device too
+	 * as CLx is re-enabled at resume.
+	 */
+	if (tb_switch_disable_clx(sw, TB_CL0S))
+		tb_sw_warn(sw, "failed to disable CLx on upstream port\n");
+
 	err = tb_plug_events_active(sw, false);
 	if (err)
 		return;
@@ -3326,8 +3338,12 @@ static bool tb_port_clx_supported(struct tb_port *port, enum tb_clx clx)
 	if (port->xdomain)
 		return false;
 
-	if (!usb4_port_clx_supported(port))
+	if (tb_switch_is_usb4(port->sw) && !usb4_port_clx_supported(port)) {
+		return false;
+	} else if (tb_switch_is_titan_ridge(port->sw) &&
+		   !tb_lc_clx_supported(port)) {
 		return false;
+	}
 
 	switch (clx) {
 	case TB_CL0S:
@@ -3393,7 +3409,7 @@ static int tb_switch_enable_cl0s(struct tb_switch *sw)
 	struct tb_port *up, *down;
 	int ret;
 
-	if (!tb_switch_is_usb4(sw))
+	if (!tb_switch_clx_supported(sw))
 		return 0;
 
 	/*
@@ -3432,6 +3448,10 @@ static int tb_switch_enable_cl0s(struct tb_switch *sw)
 	if (ret)
 		goto out;
 
+	ret = tb_port_titan_ridge_clx_objection_mask(sw);
+	if (ret)
+		goto out;
+
 	sw->clx = TB_CL0S;
 	tb_sw_dbg(sw, "enabled CL0s on upstream port\n");
 	return 0;
@@ -3480,7 +3500,7 @@ static int tb_switch_disable_cl0s(struct tb_switch *sw)
 	struct tb_port *up, *down;
 	int ret;
 
-	if (!tb_switch_is_usb4(sw))
+	if (!tb_switch_clx_supported(sw))
 		return 0;
 
 	/*
@@ -3552,3 +3572,168 @@ bool tb_switch_is_cl0s_enabled(struct tb_switch *sw)
 {
 	return sw->clx == TB_CL0S;
 }
+
+/**
+ * tb_switch_clx_supported() - Is CLx supported on this type of switch
+ *
+ * @sw: The switch to check CLx support for
+ */
+bool tb_switch_clx_supported(struct tb_switch *sw)
+{
+	if (tb_switch_is_usb4(sw) || tb_switch_is_titan_ridge(sw))
+		return true;
+	return false;
+}
+
+/**
+ * tb_port_titan_ridge_clx_objection_mask() - Mask CLx objections for a switch
+ * @sw: Switch to mask objections for
+ *
+ * Mask the objections coming from the second depth routers in order
+ * to stop these objections from interfering with the CL states of the first
+ * depth link.
+ */
+int tb_port_titan_ridge_clx_objection_mask(struct tb_switch *sw)
+{
+	int up_port = sw->config.upstream_port_number;
+	u32 offset, val[2], mask_obj, unmask_obj;
+	int ret, i;
+
+	if (!tb_switch_is_titan_ridge(sw))
+		return 0;
+
+	if (!tb_route(sw))
+		return 0;
+
+	/*
+	 * In Titan Ridge there are only 2 dual-lane Thunderbolt ports:
+	 * Port A consists of lane adapters 1,2 and
+	 * Port B consists of lane adapters 3,4
+	 * If upstream port is A, (lanes are 1,2), we mask objections from
+	 * port B (lanes 3,4) and unmask objections from Port A and vice-versa.
+	 */
+	if (up_port == 1) {
+		mask_obj = TB_LOW_PWR_C0_PORT_B_MASK;
+		unmask_obj = TB_LOW_PWR_C1_PORT_A_MASK;
+		offset = TB_LOW_PWR_C1_CL1;
+	} else {
+		mask_obj = TB_LOW_PWR_C1_PORT_A_MASK;
+		unmask_obj = TB_LOW_PWR_C0_PORT_B_MASK;
+		offset = TB_LOW_PWR_C3_CL1;
+	}
+
+	ret = tb_sw_read(sw, &val, TB_CFG_SWITCH,
+			 sw->cap_lp + offset, ARRAY_SIZE(val));
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(val); i++) {
+		val[i] |= mask_obj;
+		val[i] &= ~unmask_obj;
+	}
+
+	return tb_sw_write(sw, &val, TB_CFG_SWITCH,
+			   sw->cap_lp + offset, ARRAY_SIZE(val));
+}
+
+static unsigned int tb_switch_titan_ridge_pcie_bridge_mask(unsigned int br)
+{
+
+	return BIT(br + TB_PLUG_EVENTS_PCIE_CMD_BR_SHIFT);
+}
+
+/*
+ * Can be used for read/write a specified PCIe bridge for any
+ * Thunderbolt 3 device. For now used only for Titan Ridge.
+ */
+static int tb_switch_titan_ridge_access_pcie_bridge(struct tb_switch *sw,
+						    unsigned int bridge,
+						    unsigned int pcie_offset,
+						    unsigned int *value,
+						    bool write)
+{
+	u32 offset, command, val;
+	int ret;
+
+	if (sw->generation != 3)
+		return -EOPNOTSUPP;
+
+	if (write) {
+		offset = sw->cap_plug_events + TB_PLUG_EVENTS_PCIE_WR_DATA;
+		ret = tb_sw_write(sw, value, TB_CFG_SWITCH, offset, 1);
+		if (ret)
+			return ret;
+	}
+
+	command = pcie_offset & TB_PLUG_EVENTS_PCIE_CMD_DW_OFFSET_MASK;
+	command |= tb_switch_titan_ridge_pcie_bridge_mask(bridge);
+	command |= write ? TB_PLUG_EVENTS_PCIE_CMD_RD_WR_MASK : 0;
+	command |= TB_PLUG_EVENTS_PCIE_CMD_COMMAND_VAL <<
+		   TB_PLUG_EVENTS_PCIE_CMD_COMMAND_SHIFT;
+	command |= TB_PLUG_EVENTS_PCIE_CMD_REQ_ACK_MASK;
+
+	offset = sw->cap_plug_events + TB_PLUG_EVENTS_PCIE_CMD;
+
+	ret = tb_sw_write(sw, &command, TB_CFG_SWITCH, offset, 1);
+	if (ret)
+		return ret;
+
+	ret = tb_switch_wait_for_bit(sw, offset,
+				     TB_PLUG_EVENTS_PCIE_CMD_REQ_ACK_MASK, 0, 100);
+	if (ret)
+		return ret;
+
+	ret = tb_sw_read(sw, &val, TB_CFG_SWITCH, offset, 1);
+	if (ret)
+		return ret;
+
+	if (val & TB_PLUG_EVENTS_PCIE_CMD_TIMEOUT_MASK)
+		return -ETIMEDOUT;
+
+	if (!write) {
+		offset = sw->cap_plug_events + TB_PLUG_EVENTS_PCIE_CMD_RD_DATA;
+		ret = tb_sw_read(sw, value, TB_CFG_SWITCH, offset, 1);
+		if (ret)
+			return ret;
+	}
+
+	return ret;
+}
+
+/**
+ * tb_switch_titan_ridge_pcie_l1_enable() - Enable PCIe link to enter L1 state
+ * @sw: Titan Ridge switch to enable PCIe L1 for
+ *
+ * For Titan Ridge switch to enter CLx state, its PCIe bridges
+ * shall enable entry to PCIe L1 state. Shall be called after
+ * the upstream PCIe tunnel was configured. Due to Intel platforms limitation,
+ * shall be called only for first hop switch.
+ */
+int tb_switch_titan_ridge_pcie_l1_enable(struct tb_switch *sw)
+{
+	struct tb_switch *parent = tb_switch_parent(sw);
+	unsigned int value;
+	int ret;
+
+	if (!tb_route(sw))
+		return 0;
+
+	if (!tb_switch_is_titan_ridge(sw))
+		return 0;
+
+	/* Enable PCIe L1 enable only for first hop router (depth = 1) */
+	if (tb_route(parent))
+		return 0;
+
+	/* Write to Downstream PCIe bridge #5 aka Dn4 */
+	value = 0x0C7806B1;
+	ret = tb_switch_titan_ridge_access_pcie_bridge(sw, 5, 0x143,
+						       &value, true);
+	if (ret)
+		return ret;
+
+	/* Write to Upstream PCIe bridge #0 aka Up0 */
+	value = 0x0C5806B1;
+	return tb_switch_titan_ridge_access_pcie_bridge(sw, 0, 0x143,
+							&value, true);
+}
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index f241d42c1c6e..091567235e8e 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -1092,6 +1092,13 @@ static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw)
 		return -EIO;
 	}
 
+	/*
+	 * In case of Titan Ridge CLx shall be enabled, enable PCIe L1 here,
+	 * after PCIe tunnelling was enabled.
+	 */
+	if (tb_switch_titan_ridge_pcie_l1_enable(sw))
+		tb_sw_warn(sw, "failed to enable PCIe L1 for Titan Ridge\n");
+
 	list_add_tail(&tunnel->list, &tcm->tunnel_list);
 	return 0;
 }
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 0fcc279277df..2ff3ecb6269c 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -140,6 +140,7 @@ enum tb_clx {
  * @cap_plug_events: Offset to the plug events capability (%0 if not found)
  * @cap_vsec_tmu: Offset to the TMU vendor specific capability (%0 if not found)
  * @cap_lc: Offset to the link controller capability (%0 if not found)
+ * @cap_lp: Offset to the low power (CLx for TBT) capability (%0 if not found)
  * @is_unplugged: The switch is going away
  * @drom: DROM of the switch (%NULL if not found)
  * @nvm: Pointer to the NVM if the switch has one (%NULL otherwise)
@@ -192,6 +193,7 @@ struct tb_switch {
 	int cap_plug_events;
 	int cap_vsec_tmu;
 	int cap_lc;
+	int cap_lp;
 	bool is_unplugged;
 	u8 *drom;
 	struct tb_nvm *nvm;
@@ -933,6 +935,10 @@ void tb_switch_tmu_configure(struct tb_switch *sw,
 int tb_switch_enable_clx(struct tb_switch *sw, enum tb_clx clx);
 int tb_switch_disable_clx(struct tb_switch *sw, enum tb_clx clx);
 bool tb_switch_is_clx_enabled(struct tb_switch *sw);
+bool tb_switch_is_cl0s_enabled(struct tb_switch *sw);
+bool tb_switch_clx_supported(struct tb_switch *sw);
+int tb_switch_titan_ridge_pcie_l1_enable(struct tb_switch *sw);
+int tb_port_titan_ridge_clx_objection_mask(struct tb_switch *sw);
 
 int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged);
 int tb_port_add_nfc_credits(struct tb_port *port, int credits);
@@ -1034,6 +1040,7 @@ bool tb_lc_dp_sink_query(struct tb_switch *sw, struct tb_port *in);
 int tb_lc_dp_sink_alloc(struct tb_switch *sw, struct tb_port *in);
 int tb_lc_dp_sink_dealloc(struct tb_switch *sw, struct tb_port *in);
 int tb_lc_force_power(struct tb_switch *sw);
+bool tb_lc_clx_supported(struct tb_port *port);
 
 static inline int tb_route_length(u64 route)
 {
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 08f1213ea565..4786e15241c8 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -307,6 +307,10 @@ struct tb_regs_port_header {
 #define TMU_ADP_CS_3_UDM			BIT(29)
 #define TMU_ADP_CS_6				0x06
 #define TMU_ADP_CS_6_DTS			BIT(1)
+/* Used for Titan Ridge only */
+#define TMU_ADP_CS_6_DISABLE_TMU_OBJ_MASK	GENMASK(3, 2)
+#define TMU_ADP_CS_6_DISABLE_TMU_OBJ_CL1	BIT(2)
+#define TMU_ADP_CS_6_DISABLE_TMU_OBJ_CL2	BIT(3)
 
 /* Lane adapter registers */
 #define LANE_ADP_CS_0				0x00
@@ -447,8 +451,33 @@ struct tb_regs_hop {
 } __packed;
 
 /* TMU Thunderbolt 3 registers */
-#define TB_TIME_VSEC_3_CS_26		0x1a
-#define TB_TIME_VSEC_3_CS_26_TD		BIT(22)
+#define TB_TIME_VSEC_3_CS_9			0x9
+#define TB_TIME_VSEC_3_CS_9_TMU_OBJ_MASK	GENMASK(17, 16)
+#define TB_TIME_VSEC_3_CS_26			0x1a
+#define TB_TIME_VSEC_3_CS_26_TD			BIT(22)
+
+/* Plug Events registers */
+#define TB_PLUG_EVENTS_PCIE_WR_DATA		0x1b
+#define TB_PLUG_EVENTS_PCIE_CMD			0x1c
+#define TB_PLUG_EVENTS_PCIE_CMD_DW_OFFSET_MASK	GENMASK(9, 0)
+#define TB_PLUG_EVENTS_PCIE_CMD_BR_SHIFT	10
+#define TB_PLUG_EVENTS_PCIE_CMD_BR_MASK		GENMASK(17, 10)
+#define TB_PLUG_EVENTS_PCIE_CMD_RD_WR_MASK	BIT(21)
+#define TB_PLUG_EVENTS_PCIE_CMD_WR		0x1
+#define TB_PLUG_EVENTS_PCIE_CMD_COMMAND_SHIFT	22
+#define TB_PLUG_EVENTS_PCIE_CMD_COMMAND_MASK	GENMASK(24, 22)
+#define TB_PLUG_EVENTS_PCIE_CMD_COMMAND_VAL	0x2
+#define TB_PLUG_EVENTS_PCIE_CMD_REQ_ACK_MASK	BIT(30)
+#define TB_PLUG_EVENTS_PCIE_CMD_TIMEOUT_MASK	BIT(31)
+#define TB_PLUG_EVENTS_PCIE_CMD_RD_DATA		0x1d
+
+/* CP Low Power registers */
+#define TB_LOW_PWR_C1_CL1		0x1
+#define TB_LOW_PWR_C1_CL1_OBJ_MASK	GENMASK(4, 1)
+#define TB_LOW_PWR_C1_CL2_OBJ_MASK	GENMASK(4, 1)
+#define TB_LOW_PWR_C1_PORT_A_MASK	GENMASK(2, 1)
+#define TB_LOW_PWR_C0_PORT_B_MASK	GENMASK(4, 3)
+#define TB_LOW_PWR_C3_CL1		0x3
 
 /* Common link controller registers */
 #define TB_LC_DESC			0x02
@@ -484,5 +513,7 @@ struct tb_regs_hop {
 #define TB_LC_SX_CTRL_SLI		BIT(29)
 #define TB_LC_SX_CTRL_UPSTREAM		BIT(30)
 #define TB_LC_SX_CTRL_SLP		BIT(31)
+#define TB_LC_LINK_ATTR			0x97
+#define TB_LC_LINK_ATTR_CPS		BIT(18)
 
 #endif
diff --git a/drivers/thunderbolt/tmu.c b/drivers/thunderbolt/tmu.c
index bbb5b1df8409..d21a7270c535 100644
--- a/drivers/thunderbolt/tmu.c
+++ b/drivers/thunderbolt/tmu.c
@@ -332,7 +332,12 @@ int tb_switch_tmu_disable(struct tb_switch *sw)
 	struct tb_port *up, *down;
 	int ret;
 
-	if (!tb_switch_is_usb4(sw))
+	/*
+	 * No need to disable TMU on devices that don't support CLx since on
+	 * these devices e.g. Alpine Ridge and earlier, the TMU mode HiFi-BiDir
+	 * is enabled by default and we don't change it.
+	 */
+	if (!tb_switch_clx_supported(sw))
 		return 0;
 
 	/* Already disabled? */
@@ -416,6 +421,53 @@ static int __tb_switch_tmu_enable_bidir(struct tb_switch *sw)
 	return ret;
 }
 
+static int tb_switch_titan_ridge_tmu_objection_mask(struct tb_switch *sw)
+{
+	u32 val;
+	int ret;
+
+	ret = tb_sw_read(sw, &val, TB_CFG_SWITCH,
+			 sw->cap_vsec_tmu + TB_TIME_VSEC_3_CS_9, 1);
+	if (ret)
+		return ret;
+
+	val &= ~TB_TIME_VSEC_3_CS_9_TMU_OBJ_MASK;
+
+	return tb_sw_write(sw, &val, TB_CFG_SWITCH,
+			   sw->cap_vsec_tmu + TB_TIME_VSEC_3_CS_9, 1);
+}
+
+static int tb_switch_titan_ridge_tmu_uni_enable(struct tb_switch *sw)
+{
+	struct tb_port *up;
+
+	if (!sw->tmu.unidirect_request)
+		return 0;
+
+	up = tb_upstream_port(sw);
+	return tb_port_tmu_write(up, TMU_ADP_CS_6,
+				 TMU_ADP_CS_6_DISABLE_TMU_OBJ_MASK,
+				 TMU_ADP_CS_6_DISABLE_TMU_OBJ_MASK);
+}
+
+static int tb_switch_titan_ridge_tmu_enable(struct tb_switch *sw)
+{
+	int ret;
+
+	if (!tb_switch_is_titan_ridge(sw))
+		return 0;
+
+	/* Titan Ridge supports only CL0s */
+	if (!tb_switch_is_cl0s_enabled(sw))
+		return -EOPNOTSUPP;
+
+	ret = tb_switch_titan_ridge_tmu_objection_mask(sw);
+	if (ret)
+		return ret;
+
+	return tb_switch_titan_ridge_tmu_uni_enable(sw);
+}
+
 /*
  * This function is called when the previous TMU mode was
  * TB_SWITCH_TMU_RATE_OFF
@@ -470,7 +522,12 @@ static int tb_switch_tmu_hifi_enable(struct tb_switch *sw)
 	if (unidirect && !sw->tmu.has_ucap)
 		return -EOPNOTSUPP;
 
-	if (!tb_switch_is_usb4(sw))
+	/*
+	 * No need to enable TMU on devices that don't support CLx since on
+	 * these devices e.g. Alpine Ridge and earlier, the TMU mode HiFi-BiDir
+	 * is enabled by default.
+	 */
+	if (!tb_switch_clx_supported(sw))
 		return 0;
 
 	if (tb_switch_tmu_hifi_is_enabled(sw, sw->tmu.unidirect_request))
@@ -519,9 +576,16 @@ static int tb_switch_tmu_hifi_enable(struct tb_switch *sw)
  */
 int tb_switch_tmu_enable(struct tb_switch *sw)
 {
+	int ret;
+
 	if (sw->tmu.rate_request == TB_SWITCH_TMU_RATE_NORMAL)
 		return -EOPNOTSUPP;
 
+	/* Titan Ridge specific operations to enable CL0s */
+	ret = tb_switch_titan_ridge_tmu_enable(sw);
+	if (ret)
+		return ret;
+
 	return tb_switch_tmu_hifi_enable(sw);
 }
 
-- 
2.17.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 7/7] thunderbolt: Add kernel param for CLx disabling
  2021-11-25 14:38 [PATCH 0/7] thunderbolt: CLx support for USB4 and Titan Ridge Gil Fine
                   ` (5 preceding siblings ...)
  2021-11-25 14:38 ` [PATCH 6/7] thunderbolt: Enable CLx for Titan Ridge device Gil Fine
@ 2021-11-25 14:38 ` Gil Fine
  6 siblings, 0 replies; 12+ messages in thread
From: Gil Fine @ 2021-11-25 14:38 UTC (permalink / raw)
  To: andreas.noever, michael.jamet, mika.westerberg, YehezkelShB
  Cc: gil.fine, linux-usb, lukas

Add a module parameter that allows user to completely disable CLx
functionality in case problems are found.

Signed-off-by: Gil Fine <gil.fine@intel.com>
---
 drivers/thunderbolt/switch.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 945672b8fa25..89868632932d 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -13,6 +13,7 @@
 #include <linux/sched/signal.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
+#include <linux/module.h>
 
 #include "tb.h"
 
@@ -26,6 +27,10 @@ struct nvm_auth_status {
 	u32 status;
 };
 
+static bool clx_enabled = true;
+module_param_named(clx, clx_enabled, bool, 0444);
+MODULE_PARM_DESC(clx, "allow CLx on the High-Speed link (default: true)");
+
 /*
  * Hold NVM authentication failure status per switch This information
  * needs to stay around even when the switch gets power cycled so we
@@ -3478,6 +3483,9 @@ int tb_switch_enable_clx(struct tb_switch *sw, enum tb_clx clx)
 {
 	struct tb_switch *root_sw = sw->tb->root_switch;
 
+	if (!clx_enabled)
+		return 0;
+
 	/* CLx is not enabled and validated on USB4 platforms before ADL */
 	if (root_sw->generation < 4 ||
 	    tb_switch_is_tiger_lake(root_sw))
@@ -3538,6 +3546,9 @@ static int tb_switch_disable_cl0s(struct tb_switch *sw)
  */
 int tb_switch_disable_clx(struct tb_switch *sw, enum tb_clx clx)
 {
+	if (!clx_enabled)
+		return 0;
+
 	switch (clx) {
 	case TB_CL0S:
 		return tb_switch_disable_cl0s(sw);
-- 
2.17.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/7] thunderbolt: Add TMU unidirectional mode
  2021-11-25 14:38 ` [PATCH 1/7] thunderbolt: Add TMU unidirectional mode Gil Fine
@ 2021-11-26 11:58   ` Yehezkel Bernat
  2021-11-29  8:19   ` Mika Westerberg
  1 sibling, 0 replies; 12+ messages in thread
From: Yehezkel Bernat @ 2021-11-26 11:58 UTC (permalink / raw)
  To: Gil Fine
  Cc: Andreas Noever, Michael Jamet, Mika Westerberg, USB list, Lukas Wunner

> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 3fae40670b72..0205361ff89a 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -89,15 +89,24 @@ enum tb_switch_tmu_rate {
>   * @cap: Offset to the TMU capability (%0 if not found)
>   * @has_ucap: Does the switch support uni-directional mode
>   * @rate: TMU refresh rate related to upstream switch. In case of root
> - *       switch this holds the domain rate.
> + *       switch this holds the domain rate. Reflects the HW setting.
>   * @unidirectional: Is the TMU in uni-directional or bi-directional mode
> - *                 related to upstream switch. Don't case for root switch.
> + *                 related to upstream switch. Don't care for root switch.
> + *                 Reflects the HW setting.
> + * @rate_request: TMU new refresh rate related to upstream switch that is
> + *               requested to be set. In case of root switch, this holds
> + *               the new domain rate that is requested to be set.
> + * @unidirect_request: Is the new TMU mode: uni-directional or bi-directional
> + *                    that is requested to be set. Related to upstream switch.
> + *                    Don't care for root switch.
>   */
>  struct tb_switch_tmu {
>         int cap;
>         bool has_ucap;
>         enum tb_switch_tmu_rate rate;
>         bool unidirectional;
> +       bool unidirect_request;
> +       enum tb_switch_tmu_rate rate_request;
>  };

Match the order of the documentation and the struct members?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/7] thunderbolt: Add TMU unidirectional mode
  2021-11-25 14:38 ` [PATCH 1/7] thunderbolt: Add TMU unidirectional mode Gil Fine
  2021-11-26 11:58   ` Yehezkel Bernat
@ 2021-11-29  8:19   ` Mika Westerberg
  1 sibling, 0 replies; 12+ messages in thread
From: Mika Westerberg @ 2021-11-29  8:19 UTC (permalink / raw)
  To: Gil Fine; +Cc: andreas.noever, michael.jamet, YehezkelShB, linux-usb, lukas

Hi,

On Thu, Nov 25, 2021 at 04:38:15PM +0200, Gil Fine wrote:
> Up until Titan Ridge (Thunderbolt 3) device, routers only supported
> bidirectional mode. In this patch, we add to TMU, a unidirectional mode.
> Unidirectional mode shall be used for enabling of low power state of the link.

Probably should mention here that this is needed to enable CLx-states.

> The Clx enabling is implemented in the next patch of this series.

This line is not necessary.

> Signed-off-by: Gil Fine <gil.fine@intel.com>
> ---
>  drivers/thunderbolt/tb.c      |   9 +-
>  drivers/thunderbolt/tb.h      |  24 ++--
>  drivers/thunderbolt/tb_regs.h |   2 +
>  drivers/thunderbolt/tmu.c     | 244 ++++++++++++++++++++++++++++------
>  4 files changed, 231 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index a231191b06c6..533fe48e85be 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -221,7 +221,7 @@ static int tb_enable_tmu(struct tb_switch *sw)
>  	int ret;
>  
>  	/* If it is already enabled in correct mode, don't touch it */
> -	if (tb_switch_tmu_is_enabled(sw))
> +	if (tb_switch_tmu_hifi_is_enabled(sw, sw->tmu.unidirect_request))
>  		return 0;
>  
>  	ret = tb_switch_tmu_disable(sw);
> @@ -669,6 +669,7 @@ static void tb_scan_port(struct tb_port *port)
>  	tb_switch_lane_bonding_enable(sw);
>  	/* Set the link configured */
>  	tb_switch_configure_link(sw);
> +	tb_switch_tmu_configure(sw, TB_SWITCH_TMU_RATE_HIFI, false);
>  
>  	if (tb_enable_tmu(sw))
>  		tb_sw_warn(sw, "failed to enable TMU\n");
> @@ -1375,6 +1376,7 @@ static int tb_start(struct tb *tb)
>  		return ret;
>  	}
>  
> +	tb_switch_tmu_configure(tb->root_switch, TB_SWITCH_TMU_RATE_HIFI, false);
>  	/* Enable TMU if it is off */
>  	tb_switch_tmu_enable(tb->root_switch);
>  	/* Full scan to discover devices added before the driver was loaded. */
> @@ -1418,6 +1420,11 @@ static void tb_restore_children(struct tb_switch *sw)
>  	if (sw->is_unplugged)
>  		return;
>  
> +	/*
> +	 * tb_switch_tmu_configure() was already called when the switch was
> +	 * added before entering SX/RPM, so no need to call it again

Nit: I suggest using "system sleep" and "runtime suspend" here or
similar.

> +	 * before enabling TMU.
> +	 */
>  	if (tb_enable_tmu(sw))
>  		tb_sw_warn(sw, "failed to restore TMU configuration\n");
>  
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 3fae40670b72..0205361ff89a 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -89,15 +89,24 @@ enum tb_switch_tmu_rate {
>   * @cap: Offset to the TMU capability (%0 if not found)
>   * @has_ucap: Does the switch support uni-directional mode
>   * @rate: TMU refresh rate related to upstream switch. In case of root
> - *	  switch this holds the domain rate.
> + *	  switch this holds the domain rate. Reflects the HW setting.
>   * @unidirectional: Is the TMU in uni-directional or bi-directional mode
> - *		    related to upstream switch. Don't case for root switch.
> + *		    related to upstream switch. Don't care for root switch.
> + *		    Reflects the HW setting.
> + * @rate_request: TMU new refresh rate related to upstream switch that is
> + *		  requested to be set. In case of root switch, this holds
> + *		  the new domain rate that is requested to be set.
> + * @unidirect_request: Is the new TMU mode: uni-directional or bi-directional
> + *		       that is requested to be set. Related to upstream switch.
> + *		       Don't care for root switch.
>   */
>  struct tb_switch_tmu {
>  	int cap;
>  	bool has_ucap;
>  	enum tb_switch_tmu_rate rate;
>  	bool unidirectional;
> +	bool unidirect_request;
> +	enum tb_switch_tmu_rate rate_request;
>  };
>  
>  /**
> @@ -891,13 +900,10 @@ int tb_switch_tmu_init(struct tb_switch *sw);
>  int tb_switch_tmu_post_time(struct tb_switch *sw);
>  int tb_switch_tmu_disable(struct tb_switch *sw);
>  int tb_switch_tmu_enable(struct tb_switch *sw);
> -
> -static inline bool tb_switch_tmu_is_enabled(const struct tb_switch *sw)
> -{
> -	return sw->tmu.rate == TB_SWITCH_TMU_RATE_HIFI &&
> -	       !sw->tmu.unidirectional;
> -}
> -
> +bool tb_switch_tmu_hifi_is_enabled(struct tb_switch *sw, bool unidirect);
> +void tb_switch_tmu_configure(struct tb_switch *sw,
> +			     enum tb_switch_tmu_rate rate,
> +			     bool unidirectional);
>  int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged);
>  int tb_port_add_nfc_credits(struct tb_port *port, int credits);
>  int tb_port_clear_counter(struct tb_port *port, int counter);
> diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
> index d8ab6c820451..38bc15680d06 100644
> --- a/drivers/thunderbolt/tb_regs.h
> +++ b/drivers/thunderbolt/tb_regs.h
> @@ -305,6 +305,8 @@ struct tb_regs_port_header {
>  /* TMU adapter registers */
>  #define TMU_ADP_CS_3				0x03
>  #define TMU_ADP_CS_3_UDM			BIT(29)
> +#define TMU_ADP_CS_6				0x06
> +#define TMU_ADP_CS_6_DTS			BIT(1)
>  
>  /* Lane adapter registers */
>  #define LANE_ADP_CS_0				0x00
> diff --git a/drivers/thunderbolt/tmu.c b/drivers/thunderbolt/tmu.c
> index 039c42a06000..1374813bfc5c 100644
> --- a/drivers/thunderbolt/tmu.c
> +++ b/drivers/thunderbolt/tmu.c
> @@ -115,6 +115,11 @@ static inline int tb_port_tmu_unidirectional_disable(struct tb_port *port)
>  	return tb_port_tmu_set_unidirectional(port, false);
>  }
>  
> +static inline int tb_port_tmu_unidirectional_enable(struct tb_port *port)
> +{
> +	return tb_port_tmu_set_unidirectional(port, true);
> +}
> +
>  static bool tb_port_tmu_is_unidirectional(struct tb_port *port)
>  {
>  	int ret;
> @@ -128,6 +133,23 @@ static bool tb_port_tmu_is_unidirectional(struct tb_port *port)
>  	return val & TMU_ADP_CS_3_UDM;
>  }
>  
> +static int tb_port_tmu_time_sync(struct tb_port *port, bool time_sync)
> +{
> +	u32 val = time_sync ? TMU_ADP_CS_6_DTS : 0;
> +
> +	return tb_port_tmu_write(port, TMU_ADP_CS_6, TMU_ADP_CS_6_DTS, val);
> +}
> +
> +static int tb_port_tmu_time_sync_disable(struct tb_port *port)
> +{
> +	return tb_port_tmu_time_sync(port, true);
> +}
> +
> +static int tb_port_tmu_time_sync_enable(struct tb_port *port)
> +{
> +	return tb_port_tmu_time_sync(port, false);
> +}
> +
>  static int tb_switch_tmu_set_time_disruption(struct tb_switch *sw, bool set)
>  {
>  	int ret;
> @@ -297,6 +319,9 @@ int tb_switch_tmu_post_time(struct tb_switch *sw)
>   */
>  int tb_switch_tmu_disable(struct tb_switch *sw)
>  {
> +	struct tb_switch *sw_conf, *parent = tb_switch_parent(sw);

Can you drop this line..

> +	bool unidirect = tb_switch_tmu_hifi_is_enabled(sw, true);
> +	struct tb_port *up, *down;
>  	int ret;
>  
>  	if (!tb_switch_is_usb4(sw))
> @@ -306,21 +331,28 @@ int tb_switch_tmu_disable(struct tb_switch *sw)
>  	if (sw->tmu.rate == TB_SWITCH_TMU_RATE_OFF)
>  		return 0;
>  
> -	if (sw->tmu.unidirectional) {
> -		struct tb_switch *parent = tb_switch_parent(sw);
> -		struct tb_port *up, *down;
> +	up = tb_upstream_port(sw);
> +	down = tb_port_at(tb_route(sw), parent);
>  
> -		up = tb_upstream_port(sw);
> -		down = tb_port_at(tb_route(sw), parent);
> +	if (tb_route(sw)) {
> +		sw_conf = unidirect ? parent : sw;
> +		tb_switch_tmu_rate_write(sw_conf, TB_SWITCH_TMU_RATE_OFF);

.. and here do

	if (unidirect)
		tb_switch_tmu_rate_write(parent, TB_SWITCH_TMU_RATE_OFF);
	else
		tb_switch_tmu_rate_write(sw, TB_SWITCH_TMU_RATE_OFF);

Also add a comment why we need to configure it in the parent when the
TMU mode is unidirectional.

>  
> -		/* The switch may be unplugged so ignore any errors */
> -		tb_port_tmu_unidirectional_disable(up);
> -		ret = tb_port_tmu_unidirectional_disable(down);
> +		tb_port_tmu_time_sync_disable(up);
> +		ret = tb_port_tmu_time_sync_disable(down);
>  		if (ret)
>  			return ret;
> -	}
>  
> -	tb_switch_tmu_rate_write(sw, TB_SWITCH_TMU_RATE_OFF);
> +		if (unidirect) {
> +			/* The switch may be unplugged so ignore any errors */
> +			tb_port_tmu_unidirectional_disable(up);
> +			ret = tb_port_tmu_unidirectional_disable(down);
> +			if (ret)
> +				return ret;
> +		}
> +	} else {
> +		tb_switch_tmu_rate_write(sw, TB_SWITCH_TMU_RATE_OFF);
> +	}
>  
>  	sw->tmu.unidirectional = false;
>  	sw->tmu.rate = TB_SWITCH_TMU_RATE_OFF;
> @@ -329,55 +361,191 @@ int tb_switch_tmu_disable(struct tb_switch *sw)
>  	return 0;
>  }
>  
> -/**
> - * tb_switch_tmu_enable() - Enable TMU on a switch
> - * @sw: Switch whose TMU to enable
> - *
> - * Enables TMU of a switch to be in bi-directional, HiFi mode. In this mode
> - * all tunneling should work.
> +/*
> + * This function is called when the previous TMU mode was
> + * TB_SWITCH_TMU_RATE_OFF
>   */
> -int tb_switch_tmu_enable(struct tb_switch *sw)
> +static int __tb_switch_tmu_enable_bidir(struct tb_switch *sw)
>  {
> +	struct tb_switch *parent = tb_switch_parent(sw);
> +	struct tb_port *up, *down;
>  	int ret;
>  
> -	if (!tb_switch_is_usb4(sw))
> -		return 0;
> +	up = tb_upstream_port(sw);
> +	down = tb_port_at(tb_route(sw), parent);
>  
> -	if (tb_switch_tmu_is_enabled(sw))
> -		return 0;
> +	ret = tb_port_tmu_unidirectional_enable(up);
> +	if (ret)
> +		return ret;
>  
> -	ret = tb_switch_tmu_set_time_disruption(sw, true);
> +	ret = tb_port_tmu_unidirectional_enable(down);
> +	if (ret)
> +		goto out;
> +
> +	ret = tb_switch_tmu_rate_write(parent, TB_SWITCH_TMU_RATE_HIFI);
> +	if (ret)
> +		goto out;
> +
> +	ret = tb_port_tmu_time_sync_enable(up);
> +	if (ret)
> +		goto out;
> +
> +	ret = tb_port_tmu_time_sync_enable(down);
> +	if (ret)
> +		goto out;
> +
> +	return 0;
> +out:
> +	/*
> +	 * In case of any failure in one of the steps, get back to the
> +	 * TMU configurations in OFF mode. In case of additional failures in
> +	 * the functions below, ignore them since we already report a failure.
> +	 */
> +	tb_port_tmu_time_sync_disable(up);
> +	tb_switch_tmu_rate_write(parent, TB_SWITCH_TMU_RATE_OFF);
> +	tb_port_tmu_unidirectional_disable(down);
> +	tb_port_tmu_unidirectional_disable(up);

Empty line here.

> +	return ret;
> +}
> +
> +/*
> + * This function is called when the previous TMU mode was
> + * TB_SWITCH_TMU_RATE_OFF

Add ending '.' to the sentence.

> + */
> +static int __tb_switch_tmu_enable_uni(struct tb_switch *sw)
> +{
> +	struct tb_switch *parent = tb_switch_parent(sw);
> +	struct tb_port *up, *down;
> +	int ret;
> +
> +	up = tb_upstream_port(sw);
> +	down = tb_port_at(tb_route(sw), parent);
> +	ret = tb_switch_tmu_rate_write(parent, TB_SWITCH_TMU_RATE_HIFI);
>  	if (ret)
>  		return ret;
>  
> -	/* Change mode to bi-directional */
> -	if (tb_route(sw) && sw->tmu.unidirectional) {
> -		struct tb_switch *parent = tb_switch_parent(sw);
> -		struct tb_port *up, *down;
> +	ret = tb_port_tmu_unidirectional_enable(up);
> +	if (ret)
> +		goto out;
> +
> +	ret = tb_port_tmu_time_sync_enable(up);
> +	if (ret)
> +		goto out;
>  
> -		up = tb_upstream_port(sw);
> -		down = tb_port_at(tb_route(sw), parent);
> +	ret = tb_port_tmu_unidirectional_enable(down);
> +	if (ret)
> +		goto out;
>  
> -		ret = tb_port_tmu_unidirectional_disable(down);
> -		if (ret)
> -			return ret;
> +	ret = tb_port_tmu_time_sync_enable(down);
> +	if (ret)
> +		goto out;
>  
> -		ret = tb_switch_tmu_rate_write(sw, TB_SWITCH_TMU_RATE_HIFI);
> -		if (ret)
> -			return ret;
> +	return 0;
> +out:
> +	/*
> +	 * In case of any failure in one of the steps, get back to the
> +	 * TMU configurations in OFF mode. In case of additional failures in
> +	 * the functions below, ignore them since we already report a failure.
> +	 */
> +	tb_port_tmu_unidirectional_disable(down);
> +	tb_port_tmu_time_sync_disable(up);
> +	tb_port_tmu_unidirectional_disable(up);
> +	tb_switch_tmu_rate_write(parent, TB_SWITCH_TMU_RATE_OFF);

Empty line.

I wonder if it is better to separate these into own function. So you
don't need to duplicate them in error paths.

> +	return ret;
> +}
>  
> -		ret = tb_port_tmu_unidirectional_disable(up);
> -		if (ret)
> -			return ret;
> +static int tb_switch_tmu_hifi_enable(struct tb_switch *sw)
> +{
> +	bool unidirect = sw->tmu.unidirect_request;
> +	int ret;
> +
> +	if (unidirect && !sw->tmu.has_ucap)
> +		return -EOPNOTSUPP;
> +
> +	if (!tb_switch_is_usb4(sw))
> +		return 0;
> +
> +	if (tb_switch_tmu_hifi_is_enabled(sw, sw->tmu.unidirect_request))
> +		return 0;
> +
> +	ret = tb_switch_tmu_set_time_disruption(sw, true);
> +	if (ret)
> +		return ret;
> +
> +	if (tb_route(sw)) {
> +		/* The used mode changes are from OFF to HiFi-Uni/HiFi-BiDir */
> +		if (sw->tmu.rate == TB_SWITCH_TMU_RATE_OFF) {
> +			ret = unidirect ? __tb_switch_tmu_enable_uni(sw)
> +					 : __tb_switch_tmu_enable_bidir(sw);
> +			if (ret)
> +				return ret;

I think it is better if you avoid the ternary operator here:

	if (unidirect)
		ret = __tb_switch_tmu_enable_uni(sw);
	else
		ret = __tb_switch_tmu_enable_bidir(sw);
	if (ret)
		return ret;


> +		}
> +		sw->tmu.unidirectional = unidirect;
>  	} else {
> +		/*
> +		 * Host-router port configurations are written as

Host router

ditto everywhere.

> +		 * configurations for down-stream port of the parent of the

downstream

> +		 * child node - see above.
> +		 * Here only the host's router rate configuration is written

host routers'

> +		 */
>  		ret = tb_switch_tmu_rate_write(sw, TB_SWITCH_TMU_RATE_HIFI);
>  		if (ret)
>  			return ret;
>  	}
>  
> -	sw->tmu.unidirectional = false;
>  	sw->tmu.rate = TB_SWITCH_TMU_RATE_HIFI;
>  	tb_sw_dbg(sw, "TMU: mode set to: %s\n", tb_switch_tmu_mode_name(sw));
>  
>  	return tb_switch_tmu_set_time_disruption(sw, false);
>  }
> +
> +/**
> + * tb_switch_tmu_enable() - Enable TMU on a switch
> + * @sw: Switch whose TMU to enable
> + *
> + * Enables TMU of a switch to be in unidirectional or bidirectional HiFi mode.
> + * Calling tb_switch_tmu_configure() is required before calling this function,
> + * to select the mode HiFi and directionality (unidirectional/bidirectional).
> + * In both modes all tunneling should work. Unidirectional mode is required for
> + * CLx (Link Low-Power) to work. LowRes mode is not used currently.
> + */
> +int tb_switch_tmu_enable(struct tb_switch *sw)
> +{
> +	if (sw->tmu.rate_request == TB_SWITCH_TMU_RATE_NORMAL)
> +		return -EOPNOTSUPP;
> +
> +	return tb_switch_tmu_hifi_enable(sw);
> +}
> +
> +/**
> + * tb_switch_tmu_configure() - Configure the TMU rate and directionality
> + * @sw: Switch whose mode to change
> + * @rate: Rate to configure Off/LowRes/HiFi
> + * @unidirectional: Unidirectionality selection: Unidirectional or Bidirectional
> + *
> + * Selects the rate of the TMU (Off, LowRes, HiFi), and Directionality
> + * (Unidirectional or Bidirectional)
> + * Shall be called before tb_switch_tmu_enable()
> + */
> +void tb_switch_tmu_configure(struct tb_switch *sw,
> +			     enum tb_switch_tmu_rate rate, bool unidirectional)
> +{
> +	sw->tmu.unidirect_request = unidirectional;
> +	sw->tmu.rate_request = rate;
> +}
> +
> +/**
> + * tb_switch_tmu_hifi_is_enabled() - Checks if the specified TMU mode
> + *				     bidir/uni enabled correctly
> + * @sw: Switch whose TMU mode to check
> + * @unidirect: Select bidirectional or unidirectional mode to check
> + *
> + * Read TMU directionality and rate from HW, and return true,
> + * if matches to bidirectional/unidirectional HiFi mode settings.
> + * Otherwise returns false.
> + */
> +bool tb_switch_tmu_hifi_is_enabled(struct tb_switch *sw, bool unidirect)
> +{
> +	return sw->tmu.rate == TB_SWITCH_TMU_RATE_HIFI &&
> +			       sw->tmu.unidirectional == unidirect;
> +}
> -- 
> 2.17.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 6/7] thunderbolt: Enable CLx for Titan Ridge device
  2021-11-25 14:38 ` [PATCH 6/7] thunderbolt: Enable CLx for Titan Ridge device Gil Fine
@ 2021-11-29  8:28   ` Mika Westerberg
  0 siblings, 0 replies; 12+ messages in thread
From: Mika Westerberg @ 2021-11-29  8:28 UTC (permalink / raw)
  To: Gil Fine; +Cc: andreas.noever, michael.jamet, YehezkelShB, linux-usb, lukas

Hi,

On Thu, Nov 25, 2021 at 04:38:20PM +0200, Gil Fine wrote:
> diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
> index 08f1213ea565..4786e15241c8 100644
> --- a/drivers/thunderbolt/tb_regs.h
> +++ b/drivers/thunderbolt/tb_regs.h
> @@ -307,6 +307,10 @@ struct tb_regs_port_header {
>  #define TMU_ADP_CS_3_UDM			BIT(29)
>  #define TMU_ADP_CS_6				0x06
>  #define TMU_ADP_CS_6_DTS			BIT(1)
> +/* Used for Titan Ridge only */
> +#define TMU_ADP_CS_6_DISABLE_TMU_OBJ_MASK	GENMASK(3, 2)
> +#define TMU_ADP_CS_6_DISABLE_TMU_OBJ_CL1	BIT(2)
> +#define TMU_ADP_CS_6_DISABLE_TMU_OBJ_CL2	BIT(3)

I think these should be part of the Thunderbolt (legacy) specific
registers as below (these are not in the USB4 spec).

>  
>  /* Lane adapter registers */
>  #define LANE_ADP_CS_0				0x00
> @@ -447,8 +451,33 @@ struct tb_regs_hop {
>  } __packed;
>  
>  /* TMU Thunderbolt 3 registers */
> -#define TB_TIME_VSEC_3_CS_26		0x1a
> -#define TB_TIME_VSEC_3_CS_26_TD		BIT(22)
> +#define TB_TIME_VSEC_3_CS_9			0x9
> +#define TB_TIME_VSEC_3_CS_9_TMU_OBJ_MASK	GENMASK(17, 16)
> +#define TB_TIME_VSEC_3_CS_26			0x1a
> +#define TB_TIME_VSEC_3_CS_26_TD			BIT(22)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/7] thunderbolt: Add CL0s support for USB4
  2021-11-25 14:38 ` [PATCH 2/7] thunderbolt: Add CL0s support for USB4 Gil Fine
@ 2021-11-29  8:38   ` Mika Westerberg
  0 siblings, 0 replies; 12+ messages in thread
From: Mika Westerberg @ 2021-11-29  8:38 UTC (permalink / raw)
  To: Gil Fine; +Cc: andreas.noever, michael.jamet, YehezkelShB, linux-usb, lukas

Hi,

On Thu, Nov 25, 2021 at 04:38:16PM +0200, Gil Fine wrote:
> +static int tb_switch_enable_cl0s(struct tb_switch *sw)
> +{
> +	struct tb_switch *parent = tb_switch_parent(sw);
> +	bool up_cl0s_support, down_cl0s_support;
> +	struct tb_port *up, *down;
> +	int ret;
> +
> +	if (!tb_switch_is_usb4(sw))
> +		return 0;
> +
> +	/*
> +	 * Enable CLx for host router's downstream port as part of the
> +	 * downstream router enabling procedure.
> +	 */
> +	if (!tb_route(sw))
> +		return 0;
> +
> +	/* Enable CLx only for first hop router (depth = 1) */
> +	if (tb_route(parent))
> +		return 0;
> +
> +	if (tb_switch_pm_secondary_resolve(sw))
> +		return -EINVAL;
> +
> +	up = tb_upstream_port(sw);
> +	down = tb_port_at(tb_route(sw), parent);
> +
> +	up_cl0s_support = tb_port_cl0s_supported(up);
> +	down_cl0s_support = tb_port_cl0s_supported(down);
> +
> +	tb_port_dbg(up, "CL0s %ssupported\n",
> +		    up_cl0s_support ? "" : "not ");
> +	tb_port_dbg(down, "CL0s %ssupported\n",
> +		    down_cl0s_support ? "" : "not ");
> +
> +	if (!up_cl0s_support || !down_cl0s_support)
> +		return -EOPNOTSUPP;
> +
> +	ret = tb_port_cl0s_enable(up);
> +	if (ret)
> +		return ret;
> +
> +	ret = tb_port_cl0s_enable(down);
> +	if (ret)

Better to get rid of the goto here and do:

	if (ret) {
		tb_port_cl0s_disable(up);
		return ret;
	}

> +		goto out;
> +
> +	sw->clx = TB_CL0S;
> +	tb_sw_dbg(sw, "enabled CL0s on upstream port\n");
> +	return 0;
> +out:
> +	tb_port_cl0s_disable(up);
> +	return ret;
> +}
> +
> +/**
> + * tb_switch_enable_clx() - Enable CLx on upstream port of specified router
> + * @sw: The switch to enable CLx for
> + * @clx: The CLx state to enable
> + *
> + * Enable CLx state only for first hop router. This is because of the HW
> + * limitation on Intel platforms.

Okay but in general do we need to enable it over the whole topology or
is it enough to enable it for the first hop router? I think most of this
is because it allows better thermal management which probably is
applicable for any USB4 host.

> + * CLx is enabled only if both sides of the link support CLx, and if
> + * both sides of the link are not configured as two single lane links
> + * and only if the link is not inter-domain link.
> + * The conditions are descibed in CM Guide 1.0 section 8.1.
> + *
> + * Return: Returns 0 on success or an error code on failure.
> + */
> +int tb_switch_enable_clx(struct tb_switch *sw, enum tb_clx clx)
> +{
> +	struct tb_switch *root_sw = sw->tb->root_switch;
> +
> +	/* CLx is not enabled and validated on USB4 platforms before ADL */
> +	if (root_sw->generation < 4 ||
> +	    tb_switch_is_tiger_lake(root_sw))
> +		return 0;
> +
> +	switch (clx) {
> +	case TB_CL0S:
> +		return tb_switch_enable_cl0s(sw);
> +
> +	case TB_CL1:
> +	case TB_CL2:

You can drpo the two lines above.

> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int tb_switch_disable_cl0s(struct tb_switch *sw)
> +{
> +	struct tb_switch *parent = tb_switch_parent(sw);
> +	struct tb_port *up, *down;
> +	int ret;
> +
> +	if (!tb_switch_is_usb4(sw))
> +		return 0;
> +
> +	/*
> +	 * Disable CLx for host router's downstream port as part of the
> +	 * downstream router enabling procedure.
> +	 */
> +	if (!tb_route(sw))
> +		return 0;
> +
> +	/* Disable CLx only for first hop router (depth = 1) */
> +	if (tb_route(parent))
> +		return 0;
> +
> +	up = tb_upstream_port(sw);
> +	down = tb_port_at(tb_route(sw), parent);
> +	ret = tb_port_cl0s_disable(up);
> +	if (ret)
> +		return ret;
> +
> +	ret = tb_port_cl0s_disable(down);
> +	if (ret)
> +		return ret;
> +
> +	sw->clx = TB_CLX_DISABLE;
> +	tb_sw_dbg(sw, "disabled CL0s on upstream port\n");
> +	return 0;
> +}
> +
> +/**
> + * tb_switch_disable_clx() - Disable CLx on upstream port of specified router
> + * @sw: The switch to disable CLx for
> + * @clx: The CLx state to disable
> + *
> + * Return: Returns 0 on success or an error code on failure.
> + */
> +int tb_switch_disable_clx(struct tb_switch *sw, enum tb_clx clx)
> +{
> +	switch (clx) {
> +	case TB_CL0S:
> +		return tb_switch_disable_cl0s(sw);
> +
> +	case TB_CL1:
> +	case TB_CL2:
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +/**
> + * tb_switch_is_clx_enabled() - Checks if the CLx is enabled
> + * @sw: Router to check the CLx state for
> + *
> + * Checks if the CLx is enabled on the router upstream link.
> + * Not applicable for a host router.
> + */
> +bool tb_switch_is_clx_enabled(struct tb_switch *sw)
> +{
> +	return sw->clx != TB_CLX_DISABLE;
> +}
> +
> +/**
> + * tb_switch_is_cl0s_enabled() - Checks if the CL0s is enabled
> + * @sw: Router to check the CLx state for
> + *
> + * Checks if the CL0s is enabled on the router upstream link.
> + * Not applicable for a host router.
> + */
> +bool tb_switch_is_cl0s_enabled(struct tb_switch *sw)
> +{
> +	return sw->clx == TB_CL0S;
> +}
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index 533fe48e85be..f241d42c1c6e 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -669,7 +669,11 @@ static void tb_scan_port(struct tb_port *port)
>  	tb_switch_lane_bonding_enable(sw);
>  	/* Set the link configured */
>  	tb_switch_configure_link(sw);
> -	tb_switch_tmu_configure(sw, TB_SWITCH_TMU_RATE_HIFI, false);
> +	if (tb_switch_enable_clx(sw, TB_CL0S))
> +		tb_sw_warn(sw, "failed to enable CLx on upstream port\n");
> +
> +	tb_switch_tmu_configure(sw, TB_SWITCH_TMU_RATE_HIFI,
> +				tb_switch_is_clx_enabled(sw) ? true : false);

tb_switch_is_clx_enabled() returns boolean so you can use it directly
here.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-11-29  8:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-25 14:38 [PATCH 0/7] thunderbolt: CLx support for USB4 and Titan Ridge Gil Fine
2021-11-25 14:38 ` [PATCH 1/7] thunderbolt: Add TMU unidirectional mode Gil Fine
2021-11-26 11:58   ` Yehezkel Bernat
2021-11-29  8:19   ` Mika Westerberg
2021-11-25 14:38 ` [PATCH 2/7] thunderbolt: Add CL0s support for USB4 Gil Fine
2021-11-29  8:38   ` Mika Westerberg
2021-11-25 14:38 ` [PATCH 3/7] thunderbolt: Move usb4_switch_wait_for_bit() to switch.c Gil Fine
2021-11-25 14:38 ` [PATCH 4/7] thunderbolt: Enable TMU for Titan Ridge device Gil Fine
2021-11-25 14:38 ` [PATCH 5/7] thunderbolt: Rename Intel VSC capability Gil Fine
2021-11-25 14:38 ` [PATCH 6/7] thunderbolt: Enable CLx for Titan Ridge device Gil Fine
2021-11-29  8:28   ` Mika Westerberg
2021-11-25 14:38 ` [PATCH 7/7] thunderbolt: Add kernel param for CLx disabling Gil Fine

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.