Re: [PATCH v2 0/7] thunderbolt: CLx support for USB4 and Titan Ridge

[Gil Fine <gil.fine@intel.com>]

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

Hi Mika,
About latest patches series adding CL0s in thunderbolt/next.git
1.
Attached fix for CL0s enabled check - w/o it, the CL0s enabling fails

2.
Maybe it is better not to mention "Intel limitation..." about the first-hop
router since I am not sure this is correct. Attached patch w/ my proposal.

3.
Don't we prefer to stay with the name: "tb_switch_titan_ridge_pcie_l1_enable" ?
so that it is clear that the function is Titan Ridge specific?
If so, sending patch for this

Thanks,
Gil

On Wed, Dec 22, 2021 at 04:18:42PM +0200, Mika Westerberg wrote:
> Hi Gil,
> 
> On Fri, Dec 17, 2021 at 03:16:37AM +0200, Gil Fine wrote:
> > v1 can be found here:
> > 
> >   https://lore.kernel.org/linux-usb/20211125143821.16558-1-gil.fine@intel.com/
> > 
> > Changes from v1:
> > 
> > * Fixed comment from Yehezkel
> > * Fixed comments from Mika
> > * Fixed commit messages
> > * Fixed time-posting procedure to follow the USB4 Connection Manager Guide 1.0
> >   (this resolves the time posting issue after resume from suspend-to-disk)
> > * Fixed TMU config in bidirectional mode - disable unidirectional
> > * Fixed TMU config in bidirectional mode - set HiFi rate for downstream router
> > 
> > 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 CL0s for Titan Ridge device
> >   thunderbolt: Add kernel param for CLx disabling
> 
> I did some minor tweaks and applied to thunderbolt.git/next, thanks!
> Please check that it still works for you.

-- 
Thanks,
Gil
---------------------------------------------------------------------
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.

[-- Attachment #2: 0001-thunderbolt-Fix-CLx-support-check-function-tb_port_c.patch --]
[-- Type: text/x-diff, Size: 1840 bytes --]

From 258e536a68845ebfff77c5bdef4a3174510cbaa6 Mon Sep 17 00:00:00 2001
From: Gil Fine <gil.fine@intel.com>
Date: Mon, 27 Dec 2021 17:09:42 +0200
Subject: [PATCH 1/3] thunderbolt: Fix CLx support check function
 tb_port_clx_supported()

Fix CLx support check function tb_port_clx_supported() to work both for
USB4 and for Titan Ridge.

Signed-off-by: Gil Fine <gil.fine@intel.com>
---
 drivers/thunderbolt/lc.c     | 3 ---
 drivers/thunderbolt/switch.c | 9 ++++++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/thunderbolt/lc.c b/drivers/thunderbolt/lc.c
index 817e2dd0dc80..53495a38b4eb 100644
--- a/drivers/thunderbolt/lc.c
+++ b/drivers/thunderbolt/lc.c
@@ -206,9 +206,6 @@ bool tb_lc_is_clx_supported(struct tb_port *port)
 	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;
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 718dd8dcf35b..d026e305fe5d 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -3325,6 +3325,7 @@ 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;
@@ -3338,10 +3339,12 @@ static bool tb_port_clx_supported(struct tb_port *port, enum tb_clx clx)
 	if (port->xdomain)
 		return false;
 
-	if (tb_switch_is_usb4(port->sw) && !usb4_port_clx_supported(port))
-		return false;
-	else if (!tb_lc_is_clx_supported(port))
+	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_CL0S:
-- 
2.17.1


[-- Attachment #3: 0002-thunderbolt-Fix-function-tb_switch_pcie_l1_enable-de.patch --]
[-- Type: text/x-diff, Size: 1210 bytes --]

From 19e02816fbbda932c66f5bba72bd45b389303102 Mon Sep 17 00:00:00 2001
From: Gil Fine <gil.fine@intel.com>
Date: Mon, 27 Dec 2021 17:28:25 +0200
Subject: [PATCH 2/3] thunderbolt: Fix function tb_switch_pcie_l1_enable()
 description

Fixed kernel-doc function description for tb_switch_pcie_l1_enable()

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

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index d026e305fe5d..31b4f3f11bfb 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -3663,8 +3663,8 @@ static int tb_switch_pcie_bridge_write(struct tb_switch *sw, unsigned int bridge
  *
  * 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.
+ * was configured. Shall be called only for first hop switch as this is the
+ * most common use-case and the only that is supported on Intel platforms.
  */
 int tb_switch_pcie_l1_enable(struct tb_switch *sw)
 {
-- 
2.17.1


[-- Attachment #4: 0003-thunderbolt-Fix-name-of-tb_switch_pcie_l1_enable-fun.patch --]
[-- Type: text/x-diff, Size: 2690 bytes --]

From f86523bd26ee76be45c59736f49259484ce75fee Mon Sep 17 00:00:00 2001
From: Gil Fine <gil.fine@intel.com>
Date: Mon, 27 Dec 2021 17:38:35 +0200
Subject: [PATCH 3/3] thunderbolt: Fix name of tb_switch_pcie_l1_enable()
 function

The function tb_switch_pcie_l1_enable() is used for Intel Titan Ridge router
only, therefore, fix the name accordingly.

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

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 31b4f3f11bfb..5de8cb0062b1 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -3658,7 +3658,7 @@ static int tb_switch_pcie_bridge_write(struct tb_switch *sw, unsigned int bridge
 }
 
 /**
- * tb_switch_pcie_l1_enable() - Enable PCIe link to enter L1 state
+ * tb_switch_titan_ridge_pcie_l1_enable() - Enable PCIe link to enter L1 state
  * @sw: Router to enable PCIe L1
  *
  * For Titan Ridge switch to enter CLx state, its PCIe bridges shall enable
@@ -3666,7 +3666,7 @@ static int tb_switch_pcie_bridge_write(struct tb_switch *sw, unsigned int bridge
  * was configured. Shall be called only for first hop switch as this is the
  * most common use-case and the only that is supported on Intel platforms.
  */
-int tb_switch_pcie_l1_enable(struct tb_switch *sw)
+int tb_switch_titan_ridge_pcie_l1_enable(struct tb_switch *sw)
 {
 	struct tb_switch *parent = tb_switch_parent(sw);
 	int ret;
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index cbd0ad85ffb1..fc2e688d2250 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -1096,7 +1096,7 @@ static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw)
 	 * PCIe L1 is needed to enable CL0s for Titan Ridge so enable it
 	 * here.
 	 */
-	if (tb_switch_pcie_l1_enable(sw))
+	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);
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 74d3b14f004e..40551b040da0 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -985,7 +985,7 @@ static inline bool tb_switch_is_clx_supported(const struct tb_switch *sw)
 
 int tb_switch_mask_clx_objections(struct tb_switch *sw);
 
-int tb_switch_pcie_l1_enable(struct tb_switch *sw);
+int tb_switch_titan_ridge_pcie_l1_enable(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);
-- 
2.17.1