Re: [PATCH 2/7] thunderbolt: Add CL0s support for USB4
From: Mika Westerberg <mika.westerberg@linux.intel.com>
Date: 2021-11-29 08:43:42
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.
quoted hunk ↗ jump to hunk
+ 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.