Hi all,
I'm sending this as RFC because the series is still missing important
features such as power management so not ready for merging. However, I
think it is good to get any early feedback from the community. We are
working to add support for the missing features.
USB4 is the public specification of Thunderbolt 3 protocol and can be
downloaded here:
https://www.usb.org/sites/default/files/USB4%20Specification_1.zip
USB4 is about tunneling different protocols over a single cable (in the
same way as Thunderbolt). The current USB4 spec supports PCIe, Display Port
and USB 3.x, and also software based protocols such as networking between
domains (hosts).
So far PCs have been using firmware based Connection Manager and Apple
systems have been using software based one. A Connection Manager is the
entity that handles creation of different tunnel types through the USB4
(and Thunderbolt) fabric. With USB4 the plan is to have software based
Connection Manager everywhere but some early systems will also support
firmware to allow OS downgrade for example.
Current Linux Thunderbolt driver supports both "modes" and can detect which
one to use dynamically.
This series first adds support for Thunderbolt 3 devices to the software
connection manager and then extends that to support USB4 compliant hosts
and devices (this applies to both firmware and software based connection
managers). With this series the following features are supported also for
USB4 compliant devices:
- PCIe tunneling
- Display Port tunneling
- USB 3.x tunneling
- P2P networking (implemented in drivers/net/thunderbolt.c)
- Host and device NVM firmware upgrade
We also add two new sysfs attributes under each device that expose link
speed and width to userspace. The rest of the userspace ABI stays the same.
I'm including Linux USB folks as well because USB4 is officially coming
from USB-IF which puts us on same boat now.
While I changed the user visible Kconfig string to mention "USB4" (the
Kconfig option is still CONFIG_THUNDERBOLT), I'm wondering whether we
should move the whole Thunderbolt driver under drivers/usb/thunderbolt as
well?
Mika Westerberg (19):
thunderbolt: Introduce tb_switch_is_icm()
thunderbolt: Log switch route string on config read/write timeout
thunderbolt: Log warning if adding switch fails
thunderbolt: Make tb_sw_write() take const parameter
thunderbolt: Add helper macros to iterate over switch ports
thunderbolt: Add support for lane bonding
thunderbolt: Add default linking between ports if not provided by DROM
thunderbolt: Add downstream PCIe port mappings for Alpine and Titan Ridge
thunderbolt: Convert basic adapter register names to follow the USB4 spec
thunderbolt: Convert PCIe adapter register names to use USB4 names
thunderbolt: Convert DP adapter register names to follow the USB4 spec
thunderbolt: Add Display Port CM handshake for Titan Ridge devices
thunderbolt: Add Display Port adapter pairing and resource management
thunderbolt: Add bandwidth management for Display Port tunnels
thunderbolt: Make tb_find_port() available to other files
thunderbolt: Call tb_eeprom_get_drom_offset() from tb_eeprom_read_n()
thunderbolt: Add initial support for USB4
thunderbolt: Update documentation with the USB4 information
thunderbolt: Do not start firmware unless asked by the user
Rajmohan Mani (3):
thunderbolt: Make tb_switch_find_cap() available to other files
thunderbolt: Add support for Time Management Unit
thunderbolt: Add support for USB tunnels
.../ABI/testing/sysfs-bus-thunderbolt | 17 +
Documentation/admin-guide/thunderbolt.rst | 27 +-
drivers/thunderbolt/Kconfig | 9 +-
drivers/thunderbolt/Makefile | 2 +-
drivers/thunderbolt/cap.c | 11 +-
drivers/thunderbolt/ctl.c | 8 +-
drivers/thunderbolt/eeprom.c | 146 +--
drivers/thunderbolt/icm.c | 36 +-
drivers/thunderbolt/lc.c | 193 +++-
drivers/thunderbolt/nhi.c | 3 +
drivers/thunderbolt/nhi.h | 2 +
drivers/thunderbolt/path.c | 52 +-
drivers/thunderbolt/switch.c | 870 +++++++++++++++---
drivers/thunderbolt/tb.c | 529 +++++++++--
drivers/thunderbolt/tb.h | 198 +++-
drivers/thunderbolt/tb_msgs.h | 2 +
drivers/thunderbolt/tb_regs.h | 160 +++-
drivers/thunderbolt/tmu.c | 380 ++++++++
drivers/thunderbolt/tunnel.c | 517 ++++++++++-
drivers/thunderbolt/tunnel.h | 19 +-
drivers/thunderbolt/usb4.c | 761 +++++++++++++++
drivers/thunderbolt/xdomain.c | 8 +-
22 files changed, 3578 insertions(+), 372 deletions(-)
create mode 100644 drivers/thunderbolt/tmu.c
create mode 100644 drivers/thunderbolt/usb4.c
--
2.23.0
This helps to point out which switch config read/write triggered the
timeout.
Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/ctl.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/thunderbolt/ctl.c b/drivers/thunderbolt/ctl.c
index 2ec1af8f7968..d97813e80e5f 100644
--- a/drivers/thunderbolt/ctl.c
+++ b/drivers/thunderbolt/ctl.c
@@ -962,8 +962,8 @@ int tb_cfg_read(struct tb_ctl *ctl, void *buffer, u64 route, u32 port,
return tb_cfg_get_error(ctl, space, &res);
case -ETIMEDOUT:
- tb_ctl_warn(ctl, "timeout reading config space %u from %#x\n",
- space, offset);
+ tb_ctl_warn(ctl, "%llx: timeout reading config space %u from %#x\n",
+ route, space, offset);
break;
default:
@@ -988,8 +988,8 @@ int tb_cfg_write(struct tb_ctl *ctl, const void *buffer, u64 route, u32 port,
return tb_cfg_get_error(ctl, space, &res);
case -ETIMEDOUT:
- tb_ctl_warn(ctl, "timeout writing config space %u to %#x\n",
- space, offset);
+ tb_ctl_warn(ctl, "%llx: timeout writing config space %u to %#x\n",
+ route, space, offset);
break;
default:
--
2.23.0
In order to keep PCIe hierarchies consistent across hotplugs, add
hard-coded PCIe downstream port to Thunderbolt port for Alpine Ridge and
Titan Ridge as well.
Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/tb.c | 4 +++-
drivers/thunderbolt/tb.h | 25 +++++++++++++++++++++++++
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index dbbe9afb9fb7..704455a4f763 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -344,10 +344,12 @@ static struct tb_port *tb_find_pcie_down(struct tb_switch *sw,
* Hard-coded Thunderbolt port to PCIe down port mapping
* per controller.
*/
- if (tb_switch_is_cr(sw))
+ if (tb_switch_is_cr(sw) || tb_switch_is_ar(sw))
index = !phy_port ? 6 : 7;
else if (tb_switch_is_fr(sw))
index = !phy_port ? 6 : 8;
+ else if (tb_switch_is_tr(sw))
+ index = !phy_port ? 8 : 9;
else
goto out;
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index e641dcebd50a..dbab06551eaa 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -632,6 +632,31 @@ static inline bool tb_switch_is_fr(const struct tb_switch *sw)
}
}
+static inline bool tb_switch_is_ar(const struct tb_switch *sw)
+{
+ switch (sw->config.device_id) {
+ case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_BRIDGE:
+ case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_BRIDGE:
+ case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_BRIDGE:
+ case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_BRIDGE:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static inline bool tb_switch_is_tr(const struct tb_switch *sw)
+{
+ switch (sw->config.device_id) {
+ case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_BRIDGE:
+ case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_BRIDGE:
+ case PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_BRIDGE:
+ return true;
+ default:
+ return false;
+ }
+}
+
/**
* tb_switch_is_icm() - Is the switch handled by ICM firmware
* @sw: Switch to check
--
2.23.0
Lane bonding allows aggregating the two 10/20 Gb/s (depending on the
generation) lanes into a single 20/40 Gb/s bonded link. This allows
sharing the full bandwidth more efficiently. In order to establish lane
bonding we need to check that the lane bonding is possible through LC
and that both end of the link actually supports 2x widths. This also
means that all the paths should be established through the primary port
so update tb_path_alloc() to handle this as well.
Lane bonding is supported starting from Falcon Ridge (2nd generation)
controllers.
Signed-off-by: Mika Westerberg <[email protected]>
---
.../ABI/testing/sysfs-bus-thunderbolt | 17 ++
drivers/thunderbolt/icm.c | 18 +-
drivers/thunderbolt/lc.c | 28 ++
drivers/thunderbolt/path.c | 30 +-
drivers/thunderbolt/switch.c | 274 ++++++++++++++++++
drivers/thunderbolt/tb.c | 21 ++
drivers/thunderbolt/tb.h | 10 +
drivers/thunderbolt/tb_msgs.h | 2 +
drivers/thunderbolt/tb_regs.h | 20 ++
drivers/thunderbolt/tunnel.c | 19 +-
10 files changed, 429 insertions(+), 10 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt
index b21fba14689b..2c9166f6fa97 100644
--- a/Documentation/ABI/testing/sysfs-bus-thunderbolt
+++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt
@@ -104,6 +104,23 @@ Contact: [email protected]
Description: This attribute contains name of this device extracted from
the device DROM.
+What: /sys/bus/thunderbolt/devices/.../link_speed
+Date: Apr 2020
+KernelVersion: 5.6
+Contact: Mika Westerberg <[email protected]>
+Description: This attribute reports the current upstream link speed
+ in Gb/s per lane. If there are two lanes they both are
+ running at the same speed. Use link_width to determine
+ whether the two lanes are bonded or not.
+
+What: /sys/bus/thunderbolt/devices/.../link_width
+Date: Apr 2020
+KernelVersion: 5.6
+Contact: Mika Westerberg <[email protected]>
+Description: This attribute reports the current upstream link width.
+ It is 1 for single lane link (or two single lane links)
+ and 2 for bonded dual lane link.
+
What: /sys/bus/thunderbolt/devices/.../vendor
Date: Sep 2017
KernelVersion: 4.13
diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index 6550f68f92ce..9c9c6ea2b790 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -567,7 +567,8 @@ static struct tb_switch *add_switch(struct tb_switch *parent_sw, u64 route,
size_t ep_name_size, u8 connection_id,
u8 connection_key, u8 link, u8 depth,
enum tb_security_level security_level,
- bool authorized, bool boot)
+ bool authorized, bool boot, bool dual_lane,
+ bool speed_gen3)
{
const struct intel_vss *vss;
struct tb_switch *sw;
@@ -592,6 +593,8 @@ static struct tb_switch *add_switch(struct tb_switch *parent_sw, u64 route,
sw->authorized = authorized;
sw->security_level = security_level;
sw->boot = boot;
+ sw->link_speed = speed_gen3 ? 20 : 10;
+ sw->link_width = dual_lane ? 2 : 1;
init_completion(&sw->rpm_complete);
vss = parse_intel_vss(ep_name, ep_name_size);
@@ -697,11 +700,11 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
(const struct icm_fr_event_device_connected *)hdr;
enum tb_security_level security_level;
struct tb_switch *sw, *parent_sw;
+ bool boot, dual_lane, speed_gen3;
struct icm *icm = tb_priv(tb);
bool authorized = false;
struct tb_xdomain *xd;
u8 link, depth;
- bool boot;
u64 route;
int ret;
@@ -714,6 +717,8 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
security_level = (pkg->hdr.flags & ICM_FLAGS_SLEVEL_MASK) >>
ICM_FLAGS_SLEVEL_SHIFT;
boot = pkg->link_info & ICM_LINK_INFO_BOOT;
+ dual_lane = pkg->hdr.flags & ICM_FLAGS_DUAL_LANE;
+ speed_gen3 = pkg->hdr.flags & ICM_FLAGS_SPEED_GEN3;
if (pkg->link_info & ICM_LINK_INFO_REJECTED) {
tb_info(tb, "switch at %u.%u was rejected by ICM firmware because topology limit exceeded\n",
@@ -814,7 +819,7 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
add_switch(parent_sw, route, &pkg->ep_uuid, (const u8 *)pkg->ep_name,
sizeof(pkg->ep_name), pkg->connection_id,
pkg->connection_key, link, depth, security_level,
- authorized, boot);
+ authorized, boot, dual_lane, speed_gen3);
tb_switch_put(parent_sw);
}
@@ -1142,10 +1147,10 @@ __icm_tr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr,
{
const struct icm_tr_event_device_connected *pkg =
(const struct icm_tr_event_device_connected *)hdr;
+ bool authorized, boot, dual_lane, speed_gen3;
enum tb_security_level security_level;
struct tb_switch *sw, *parent_sw;
struct tb_xdomain *xd;
- bool authorized, boot;
u64 route;
icm_postpone_rescan(tb);
@@ -1163,6 +1168,8 @@ __icm_tr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr,
security_level = (pkg->hdr.flags & ICM_FLAGS_SLEVEL_MASK) >>
ICM_FLAGS_SLEVEL_SHIFT;
boot = pkg->link_info & ICM_LINK_INFO_BOOT;
+ dual_lane = pkg->hdr.flags & ICM_FLAGS_DUAL_LANE;
+ speed_gen3 = pkg->hdr.flags & ICM_FLAGS_SPEED_GEN3;
if (pkg->link_info & ICM_LINK_INFO_REJECTED) {
tb_info(tb, "switch at %llx was rejected by ICM firmware because topology limit exceeded\n",
@@ -1207,7 +1214,8 @@ __icm_tr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr,
sw = add_switch(parent_sw, route, &pkg->ep_uuid, (const u8 *)pkg->ep_name,
sizeof(pkg->ep_name), pkg->connection_id, 0, 0, 0,
- security_level, authorized, boot);
+ security_level, authorized, boot, dual_lane,
+ speed_gen3);
if (!IS_ERR(sw) && force_rtd3)
sw->rpm = true;
diff --git a/drivers/thunderbolt/lc.c b/drivers/thunderbolt/lc.c
index af38076088f6..df56523eb822 100644
--- a/drivers/thunderbolt/lc.c
+++ b/drivers/thunderbolt/lc.c
@@ -177,3 +177,31 @@ int tb_lc_set_sleep(struct tb_switch *sw)
return 0;
}
+
+/**
+ * tb_lc_lane_bonding_possible() - Is lane bonding possible towards switch
+ * @sw: Switch to check
+ *
+ * Checks whether conditions for lane bonding from parent to @sw are
+ * possible.
+ */
+bool tb_lc_lane_bonding_possible(struct tb_switch *sw)
+{
+ struct tb_port *up;
+ int cap, ret;
+ u32 val;
+
+ if (sw->generation < 2)
+ return false;
+
+ up = tb_upstream_port(sw);
+ cap = find_port_lc_cap(up);
+ if (cap < 0)
+ return false;
+
+ ret = tb_sw_read(sw, &val, TB_CFG_SWITCH, cap + TB_LC_PORT_ATTR, 1);
+ if (ret)
+ return false;
+
+ return !!(val & TB_LC_PORT_ATTR_BE);
+}
diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
index afe5f8391ebf..6cf66597d5d8 100644
--- a/drivers/thunderbolt/path.c
+++ b/drivers/thunderbolt/path.c
@@ -220,7 +220,8 @@ struct tb_path *tb_path_discover(struct tb_port *src, int src_hopid,
* Creates path between two ports starting with given @src_hopid. Reserves
* HopIDs for each port (they can be different from @src_hopid depending on
* how many HopIDs each port already have reserved). If there are dual
- * links on the path, prioritizes using @link_nr.
+ * links on the path, prioritizes using @link_nr but takes into account
+ * that the lanes may be bonded.
*
* Return: Returns a tb_path on success or NULL on failure.
*/
@@ -259,7 +260,9 @@ struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
if (!in_port)
goto err;
- if (in_port->dual_link_port && in_port->link_nr != link_nr)
+ /* When lanes are bonded primary link must be used */
+ if (!in_port->bonded && in_port->dual_link_port &&
+ in_port->link_nr != link_nr)
in_port = in_port->dual_link_port;
ret = tb_port_alloc_in_hopid(in_port, in_hopid, in_hopid);
@@ -271,8 +274,27 @@ struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
if (!out_port)
goto err;
- if (out_port->dual_link_port && out_port->link_nr != link_nr)
- out_port = out_port->dual_link_port;
+ /*
+ * Pick up right port when going from non-bonded to
+ * bonded or from bonded to non-bonded.
+ */
+ if (out_port->dual_link_port) {
+ if (!in_port->bonded && out_port->bonded &&
+ out_port->link_nr) {
+ /*
+ * Use primary link when going from
+ * non-bonded to bonded.
+ */
+ out_port = out_port->dual_link_port;
+ } else if (!out_port->bonded &&
+ out_port->link_nr != link_nr) {
+ /*
+ * If out port is not bonded follow
+ * link_nr.
+ */
+ out_port = out_port->dual_link_port;
+ }
+ }
if (i == num_hops - 1)
ret = tb_port_alloc_out_hopid(out_port, dst_hopid,
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index cc2670dd2698..2b00ea7a979a 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -775,6 +775,132 @@ struct tb_port *tb_next_port_on_path(struct tb_port *start, struct tb_port *end,
return next;
}
+static int tb_port_get_link_speed(struct tb_port *port)
+{
+ u32 val, speed;
+ int ret;
+
+ if (!port->cap_phy)
+ return -EINVAL;
+
+ ret = tb_port_read(port, &val, TB_CFG_PORT,
+ port->cap_phy + LANE_ADP_CS_1, 1);
+ if (ret)
+ return ret;
+
+ speed = (val & LANE_ADP_CS_1_CURRENT_SPEED_MASK) >>
+ LANE_ADP_CS_1_CURRENT_SPEED_SHIFT;
+ return speed == LANE_ADP_CS_1_CURRENT_SPEED_GEN3 ? 20 : 10;
+}
+
+static int tb_port_get_link_width(struct tb_port *port)
+{
+ u32 val;
+ int ret;
+
+ if (!port->cap_phy)
+ return -EINVAL;
+
+ ret = tb_port_read(port, &val, TB_CFG_PORT,
+ port->cap_phy + LANE_ADP_CS_1, 1);
+ if (ret)
+ return ret;
+
+ return (val & LANE_ADP_CS_1_CURRENT_WIDTH_MASK) >>
+ LANE_ADP_CS_1_CURRENT_WIDTH_SHIFT;
+}
+
+static bool tb_port_is_width_supported(struct tb_port *port, int width)
+{
+ u32 phy, widths;
+ int ret;
+
+ if (!port->cap_phy)
+ return false;
+
+ ret = tb_port_read(port, &phy, TB_CFG_PORT,
+ port->cap_phy + LANE_ADP_CS_0, 1);
+ if (ret)
+ return ret;
+
+ widths = (phy & LANE_ADP_CS_0_SUPPORTED_WIDTH_MASK) >>
+ LANE_ADP_CS_0_SUPPORTED_WIDTH_SHIFT;
+
+ return !!(widths & width);
+}
+
+static int tb_port_set_link_width(struct tb_port *port, unsigned int width)
+{
+ u32 val;
+ int ret;
+
+ if (!port->cap_phy)
+ return -EINVAL;
+
+ ret = tb_port_read(port, &val, TB_CFG_PORT,
+ port->cap_phy + LANE_ADP_CS_1, 1);
+ if (ret)
+ return ret;
+
+ val &= ~LANE_ADP_CS_1_TARGET_WIDTH_MASK;
+ switch (width) {
+ case 1:
+ val |= LANE_ADP_CS_1_TARGET_WIDTH_SINGLE <<
+ LANE_ADP_CS_1_TARGET_WIDTH_SHIFT;
+ break;
+ case 2:
+ val |= LANE_ADP_CS_1_TARGET_WIDTH_DUAL <<
+ LANE_ADP_CS_1_TARGET_WIDTH_SHIFT;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ val |= LANE_ADP_CS_1_LB;
+
+ return tb_port_write(port, &val, TB_CFG_PORT,
+ port->cap_phy + LANE_ADP_CS_1, 1);
+}
+
+static int tb_port_lane_bonding_enable(struct tb_port *port)
+{
+ int ret;
+
+ /*
+ * Enable lane bonding for both links if not already enabled by
+ * for example the boot firmware.
+ */
+ ret = tb_port_get_link_width(port);
+ if (ret == 1) {
+ ret = tb_port_set_link_width(port, 2);
+ if (ret)
+ return ret;
+ }
+
+ ret = tb_port_get_link_width(port->dual_link_port);
+ if (ret == 1) {
+ ret = tb_port_set_link_width(port->dual_link_port, 2);
+ if (ret) {
+ tb_port_set_link_width(port, 1);
+ return ret;
+ }
+ }
+
+ port->bonded = true;
+ port->dual_link_port->bonded = true;
+
+ return 0;
+}
+
+static void tb_port_lane_bonding_disable(struct tb_port *port)
+{
+ port->dual_link_port->bonded = false;
+ port->bonded = false;
+
+ tb_port_set_link_width(port->dual_link_port, 1);
+ tb_port_set_link_width(port, 1);
+}
+
/**
* tb_port_is_enabled() - Is the adapter port enabled
* @port: Port to check
@@ -1166,6 +1292,26 @@ static ssize_t key_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR(key, 0600, key_show, key_store);
+static ssize_t link_speed_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct tb_switch *sw = tb_to_switch(dev);
+
+ return sprintf(buf, "%u.0 Gb/s\n", sw->link_speed);
+
+}
+static DEVICE_ATTR_RO(link_speed);
+
+static ssize_t link_width_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct tb_switch *sw = tb_to_switch(dev);
+
+ return sprintf(buf, "%u\n", sw->link_width);
+
+}
+static DEVICE_ATTR_RO(link_width);
+
static void nvm_authenticate_start(struct tb_switch *sw)
{
struct pci_dev *root_port;
@@ -1320,6 +1466,8 @@ static struct attribute *switch_attrs[] = {
&dev_attr_device.attr,
&dev_attr_device_name.attr,
&dev_attr_key.attr,
+ &dev_attr_link_speed.attr,
+ &dev_attr_link_width.attr,
&dev_attr_nvm_authenticate.attr,
&dev_attr_nvm_version.attr,
&dev_attr_vendor.attr,
@@ -1352,6 +1500,11 @@ static umode_t switch_attr_is_visible(struct kobject *kobj,
sw->security_level == TB_SECURITY_SECURE)
return attr->mode;
return 0;
+ } else if (attr == &dev_attr_link_speed.attr ||
+ attr == &dev_attr_link_width.attr) {
+ if (tb_route(sw))
+ return attr->mode;
+ return 0;
} else if (attr == &dev_attr_nvm_authenticate.attr) {
if (sw->dma_port && !sw->no_nvm_upgrade)
return attr->mode;
@@ -1751,6 +1904,123 @@ static int tb_switch_add_dma_port(struct tb_switch *sw)
return -ESHUTDOWN;
}
+static bool tb_switch_lane_bonding_possible(struct tb_switch *sw)
+{
+ const struct tb_port *up = tb_upstream_port(sw);
+
+ if (!up->dual_link_port || !up->dual_link_port->remote)
+ return false;
+
+ return tb_lc_lane_bonding_possible(sw);
+}
+
+static int tb_switch_update_link_attributes(struct tb_switch *sw)
+{
+ struct tb_port *up;
+ bool change = false;
+ int ret;
+
+ if (!tb_route(sw) || tb_switch_is_icm(sw))
+ return 0;
+
+ up = tb_upstream_port(sw);
+
+ ret = tb_port_get_link_speed(up);
+ if (ret < 0)
+ return ret;
+ if (sw->link_speed != ret)
+ change = true;
+ sw->link_speed = ret;
+
+ ret = tb_port_get_link_width(up);
+ if (ret < 0)
+ return ret;
+ if (sw->link_width != ret)
+ change = true;
+ sw->link_width = ret;
+
+ /* Notify userspace that there is possible link attribute change */
+ if (device_is_registered(&sw->dev) && change)
+ kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE);
+
+ return 0;
+}
+
+/**
+ * tb_switch_lane_bonding_enable() - Enable lane bonding
+ * @sw: Switch to enable lane bonding
+ *
+ * Connection manager can call this function to enable lane bonding of a
+ * switch. If conditions are correct and both switches support the feature,
+ * lanes are bonded. It is safe to call this to any switch.
+ */
+int tb_switch_lane_bonding_enable(struct tb_switch *sw)
+{
+ struct tb_switch *parent = tb_to_switch(sw->dev.parent);
+ struct tb_port *up, *down;
+ u64 route = tb_route(sw);
+ int ret;
+
+ if (!route)
+ return 0;
+
+ if (!tb_switch_lane_bonding_possible(sw))
+ return 0;
+
+ up = tb_upstream_port(sw);
+ down = tb_port_at(route, parent);
+
+ if (!tb_port_is_width_supported(up, 2) ||
+ !tb_port_is_width_supported(down, 2))
+ return 0;
+
+ ret = tb_port_lane_bonding_enable(up);
+ if (ret) {
+ tb_port_warn(up, "failed to enable lane bonding\n");
+ return ret;
+ }
+
+ ret = tb_port_lane_bonding_enable(down);
+ if (ret) {
+ tb_port_warn(down, "failed to enable lane bonding\n");
+ tb_port_lane_bonding_disable(up);
+ return ret;
+ }
+
+ tb_switch_update_link_attributes(sw);
+
+ tb_sw_dbg(sw, "lane bonding enabled\n");
+ return ret;
+}
+
+/**
+ * tb_switch_lane_bonding_disable() - Disable lane bonding
+ * @sw: Switch whose lane bonding to disable
+ *
+ * Disables lane bonding between @sw and parent. This can be called even
+ * if lanes were not bonded originally.
+ */
+void tb_switch_lane_bonding_disable(struct tb_switch *sw)
+{
+ struct tb_switch *parent = tb_to_switch(sw->dev.parent);
+ struct tb_port *up, *down;
+
+ if (!tb_route(sw))
+ return;
+
+ up = tb_upstream_port(sw);
+ if (!up->bonded)
+ return;
+
+ down = tb_port_at(tb_route(sw), parent);
+
+ tb_port_lane_bonding_disable(up);
+ tb_port_lane_bonding_disable(down);
+
+ tb_switch_update_link_attributes(sw);
+ tb_sw_dbg(sw, "lane bonding disabled\n");
+}
+
/**
* tb_switch_add() - Add a switch to the domain
* @sw: Switch to add
@@ -1800,6 +2070,10 @@ int tb_switch_add(struct tb_switch *sw)
if (ret)
return ret;
}
+
+ ret = tb_switch_update_link_attributes(sw);
+ if (ret)
+ return ret;
}
ret = device_add(&sw->dev);
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index ab42f0fea787..dbbe9afb9fb7 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -219,6 +219,10 @@ static void tb_scan_port(struct tb_port *port)
upstream_port->dual_link_port->remote = port->dual_link_port;
}
+ /* Enable lane bonding if supported */
+ if (tb_switch_lane_bonding_enable(sw))
+ tb_sw_warn(sw, "failed to enable lane bonding\n");
+
tb_scan_switch(sw);
}
@@ -271,6 +275,7 @@ static void tb_free_unplugged_children(struct tb_switch *sw)
struct tb_port *port = &sw->ports[i];
if (port->remote->sw->is_unplugged) {
+ tb_switch_lane_bonding_disable(port->remote->sw);
tb_switch_remove(port->remote->sw);
port->remote = NULL;
if (port->dual_link_port)
@@ -536,6 +541,7 @@ static void tb_handle_hotplug(struct work_struct *work)
tb_port_dbg(port, "switch unplugged\n");
tb_sw_set_unplugged(port->remote->sw);
tb_free_invalid_tunnels(tb);
+ tb_switch_lane_bonding_disable(port->remote->sw);
tb_switch_remove(port->remote->sw);
port->remote = NULL;
if (port->dual_link_port)
@@ -705,6 +711,20 @@ static int tb_suspend_noirq(struct tb *tb)
return 0;
}
+static void tb_restore_children(struct tb_switch *sw)
+{
+ int i;
+
+ tb_switch_for_each_remote_port(sw, i) {
+ struct tb_port *port = &sw->ports[i];
+
+ if (tb_switch_lane_bonding_enable(port->remote->sw))
+ dev_warn(&sw->dev, "failed to restore lane bonding\n");
+
+ tb_restore_children(port->remote->sw);
+ }
+}
+
static int tb_resume_noirq(struct tb *tb)
{
struct tb_cm *tcm = tb_priv(tb);
@@ -718,6 +738,7 @@ static int tb_resume_noirq(struct tb *tb)
tb_switch_resume(tb->root_switch);
tb_free_invalid_tunnels(tb);
tb_free_unplugged_children(tb->root_switch);
+ tb_restore_children(tb->root_switch);
list_for_each_entry_safe(tunnel, n, &tcm->tunnel_list, list)
tb_tunnel_restart(tunnel);
if (!list_empty(&tcm->tunnel_list)) {
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index b723b86f4e72..e641dcebd50a 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -61,6 +61,8 @@ struct tb_switch_nvm {
* @device: Device ID of the switch
* @vendor_name: Name of the vendor (or %NULL if not known)
* @device_name: Name of the device (or %NULL if not known)
+ * @link_speed: Speed of the link in Gb/s
+ * @link_width: Width of the link (1 or 2)
* @generation: Switch Thunderbolt generation
* @cap_plug_events: Offset to the plug events capability (%0 if not found)
* @cap_lc: Offset to the link controller capability (%0 if not found)
@@ -97,6 +99,8 @@ struct tb_switch {
u16 device;
const char *vendor_name;
const char *device_name;
+ unsigned int link_speed;
+ unsigned int link_width;
unsigned int generation;
int cap_plug_events;
int cap_lc;
@@ -127,6 +131,7 @@ struct tb_switch {
* @cap_adap: Offset of the adapter specific capability (%0 if not present)
* @port: Port number on switch
* @disabled: Disabled by eeprom
+ * @bonded: true if the port is bonded (two lanes combined as one)
* @dual_link_port: If the switch is connected using two ports, points
* to the other port.
* @link_nr: Is this primary or secondary port on the dual_link.
@@ -142,6 +147,7 @@ struct tb_port {
int cap_adap;
u8 port;
bool disabled;
+ bool bonded;
struct tb_port *dual_link_port;
u8 link_nr:1;
struct ida in_hopids;
@@ -640,6 +646,9 @@ static inline bool tb_switch_is_icm(const struct tb_switch *sw)
return !sw->config.enabled;
}
+int tb_switch_lane_bonding_enable(struct tb_switch *sw);
+void tb_switch_lane_bonding_disable(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_set_initial_credits(struct tb_port *port, u32 credits);
@@ -683,6 +692,7 @@ int tb_lc_read_uuid(struct tb_switch *sw, u32 *uuid);
int tb_lc_configure_link(struct tb_switch *sw);
void tb_lc_unconfigure_link(struct tb_switch *sw);
int tb_lc_set_sleep(struct tb_switch *sw);
+bool tb_lc_lane_bonding_possible(struct tb_switch *sw);
static inline int tb_route_length(u64 route)
{
diff --git a/drivers/thunderbolt/tb_msgs.h b/drivers/thunderbolt/tb_msgs.h
index 4b641e4ee0c5..3705057723b6 100644
--- a/drivers/thunderbolt/tb_msgs.h
+++ b/drivers/thunderbolt/tb_msgs.h
@@ -122,6 +122,8 @@ struct icm_pkg_header {
#define ICM_FLAGS_NO_KEY BIT(1)
#define ICM_FLAGS_SLEVEL_SHIFT 3
#define ICM_FLAGS_SLEVEL_MASK GENMASK(4, 3)
+#define ICM_FLAGS_DUAL_LANE BIT(5)
+#define ICM_FLAGS_SPEED_GEN3 BIT(7)
#define ICM_FLAGS_WRITE BIT(7)
struct icm_pkg_driver_ready {
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index deb9d4a977b9..6d4e072f1f63 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -219,6 +219,23 @@ struct tb_regs_port_header {
#define TB_PORT_LCA_SHIFT 22
#define TB_PORT_LCA_MASK GENMASK(28, 22)
+/* Lane adapter registers */
+#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_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_LB BIT(15)
+#define LANE_ADP_CS_1_CURRENT_SPEED_MASK GENMASK(19, 16)
+#define LANE_ADP_CS_1_CURRENT_SPEED_SHIFT 16
+#define LANE_ADP_CS_1_CURRENT_SPEED_GEN2 0x8
+#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
+
/* Display Port adapter registers */
/* DWORD 0 */
@@ -280,6 +297,9 @@ struct tb_regs_hop {
#define TB_LC_FUSE 0x03
/* Link controller registers */
+#define TB_LC_PORT_ATTR 0x8d
+#define TB_LC_PORT_ATTR_BE BIT(12)
+
#define TB_LC_SX_CTRL 0x96
#define TB_LC_SX_CTRL_L1C BIT(16)
#define TB_LC_SX_CTRL_L2C BIT(20)
diff --git a/drivers/thunderbolt/tunnel.c b/drivers/thunderbolt/tunnel.c
index 5a99234826e7..ff55a114825a 100644
--- a/drivers/thunderbolt/tunnel.c
+++ b/drivers/thunderbolt/tunnel.c
@@ -90,6 +90,22 @@ static int tb_pci_activate(struct tb_tunnel *tunnel, bool activate)
return 0;
}
+static int tb_initial_credits(const struct tb_switch *sw)
+{
+ /* If the path is complete sw is not NULL */
+ if (sw) {
+ /* More credits for faster link */
+ switch (sw->link_speed * sw->link_width) {
+ case 40:
+ return 32;
+ case 20:
+ return 24;
+ }
+ }
+
+ return 16;
+}
+
static void tb_pci_init_path(struct tb_path *path)
{
path->egress_fc_enable = TB_PATH_SOURCE | TB_PATH_INTERNAL;
@@ -101,7 +117,8 @@ static void tb_pci_init_path(struct tb_path *path)
path->drop_packages = 0;
path->nfc_credits = 0;
path->hops[0].initial_credits = 7;
- path->hops[1].initial_credits = 16;
+ path->hops[1].initial_credits =
+ tb_initial_credits(path->hops[1].in_port->sw);
}
/**
--
2.23.0
To perform proper Display Port tunneling for Thunderbolt 3 devices we
need to allocate DP resources for DP IN port before they can be used.
The reason for this is that the user can also connect a monitor directly
to the Type-C ports in which case the Thunderbolt controller acts as
re-driver for Display Port (no tunneling takes place) taking the DP
sinks away from the connection manager. This allocation is done using
special sink allocation registers available through the link controller.
We can pair DP IN to DP OUT only if
* DP IN has sink allocated via link controller
* DP OUT port receives hotplug event
For DP IN adapters (only for the root switch) we first query whether
there is DP resource available (it may be the previous instance of the
driver for example already allocated it) and if it is we add it to the
list. We then update the list when after each plug/unplug event to a DP
IN/OUT adapter. Each time the list is updated we try to find additional
DP IN <-> DP OUT pairs for tunnel establishment. This strategy also
makes it possible to establish another tunnel in case there are 3
monitors connected and one gets unplugged releasing the DP IN adapter
for the new tunnel.
Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/lc.c | 161 +++++++++++++++++++++++++++
drivers/thunderbolt/switch.c | 44 ++++++++
drivers/thunderbolt/tb.c | 203 ++++++++++++++++++++++++++++------
drivers/thunderbolt/tb.h | 9 ++
drivers/thunderbolt/tb_regs.h | 6 +
5 files changed, 389 insertions(+), 34 deletions(-)
diff --git a/drivers/thunderbolt/lc.c b/drivers/thunderbolt/lc.c
index df56523eb822..13a5e39b0043 100644
--- a/drivers/thunderbolt/lc.c
+++ b/drivers/thunderbolt/lc.c
@@ -205,3 +205,164 @@ bool tb_lc_lane_bonding_possible(struct tb_switch *sw)
return !!(val & TB_LC_PORT_ATTR_BE);
}
+
+static int tb_lc_dp_sink_from_port(const struct tb_switch *sw,
+ struct tb_port *in)
+{
+ int i;
+
+ /* The first DP IN port is sink 0 and second is sink 1 */
+ tb_switch_for_each_port(sw, i) {
+ if (tb_port_is_dpin(&sw->ports[i]))
+ return in != &sw->ports[i];
+ }
+
+ return -EINVAL;
+}
+
+static int tb_lc_dp_sink_available(struct tb_switch *sw, int sink)
+{
+ u32 val, alloc;
+ int ret;
+
+ ret = tb_sw_read(sw, &val, TB_CFG_SWITCH,
+ sw->cap_lc + TB_LC_SNK_ALLOCATION, 1);
+ if (ret)
+ return ret;
+
+ /*
+ * Sink is available for CM/SW to use if the allocation valie is
+ * either 0 or 1.
+ */
+ if (!sink) {
+ alloc = val & TB_LC_SNK_ALLOCATION_SNK0_MASK;
+ if (!alloc || alloc == TB_LC_SNK_ALLOCATION_SNK0_CM)
+ return 0;
+ } else {
+ alloc = (val & TB_LC_SNK_ALLOCATION_SNK1_MASK) >>
+ TB_LC_SNK_ALLOCATION_SNK1_SHIFT;
+ if (!alloc || alloc == TB_LC_SNK_ALLOCATION_SNK1_CM)
+ return 0;
+ }
+
+ return -EBUSY;
+}
+
+/**
+ * tb_lc_dp_sink_query() - Is DP sink available for DP IN port
+ * @sw: Switch whose DP sink is queried
+ * @in: DP IN port to check
+ *
+ * Queries through LC SNK_ALLOCATION registers whether DP sink is available
+ * for the given DP IN port or not.
+ */
+bool tb_lc_dp_sink_query(struct tb_switch *sw, struct tb_port *in)
+{
+ int sink;
+
+ /*
+ * For older generations sink is always available as there is no
+ * allocation mechanism.
+ */
+ if (sw->generation < 3)
+ return true;
+
+ sink = tb_lc_dp_sink_from_port(sw, in);
+ if (sink < 0)
+ return false;
+
+ return !tb_lc_dp_sink_available(sw, sink);
+}
+
+/**
+ * tb_lc_dp_sink_alloc() - Allocate DP sink
+ * @sw: Switch whose DP sink is allocated
+ * @in: DP IN port the DP sink is allocated for
+ *
+ * Allocate DP sink for @in via LC SNK_ALLOCATION registers. If the
+ * resource is available and allocation is successful returns %0. In all
+ * other cases returs negative errno. In particular %-EBUSY is returned if
+ * the resource was not available.
+ */
+int tb_lc_dp_sink_alloc(struct tb_switch *sw, struct tb_port *in)
+{
+ int ret, sink;
+ u32 val;
+
+ if (sw->generation < 3)
+ return 0;
+
+ sink = tb_lc_dp_sink_from_port(sw, in);
+ if (sink < 0)
+ return sink;
+
+ ret = tb_lc_dp_sink_available(sw, sink);
+ if (ret)
+ return ret;
+
+ ret = tb_sw_read(sw, &val, TB_CFG_SWITCH,
+ sw->cap_lc + TB_LC_SNK_ALLOCATION, 1);
+ if (ret)
+ return ret;
+
+ if (!sink) {
+ val &= ~TB_LC_SNK_ALLOCATION_SNK0_MASK;
+ val |= TB_LC_SNK_ALLOCATION_SNK0_CM;
+ } else {
+ val &= ~TB_LC_SNK_ALLOCATION_SNK1_MASK;
+ val |= TB_LC_SNK_ALLOCATION_SNK1_CM <<
+ TB_LC_SNK_ALLOCATION_SNK1_SHIFT;
+ }
+
+ ret = tb_sw_write(sw, &val, TB_CFG_SWITCH,
+ sw->cap_lc + TB_LC_SNK_ALLOCATION, 1);
+
+ if (ret)
+ return ret;
+
+ tb_port_dbg(in, "sink %d allocated\n", sink);
+ return 0;
+}
+
+/**
+ * tb_lc_dp_sink_dealloc() - De-allocate DP sink
+ * @sw: Switch whose DP sink is de-allocated
+ * @in: DP IN port whose DP sink is de-allocated
+ *
+ * De-allocate DP sink from @in using LC SNK_ALLOCATION registers.
+ */
+int tb_lc_dp_sink_dealloc(struct tb_switch *sw, struct tb_port *in)
+{
+ int ret, sink;
+ u32 val;
+
+ if (sw->generation < 3)
+ return 0;
+
+ sink = tb_lc_dp_sink_from_port(sw, in);
+ if (sink < 0)
+ return sink;
+
+ /* Needs to be owned by CM/SW */
+ ret = tb_lc_dp_sink_available(sw, sink);
+ if (ret)
+ return ret;
+
+ ret = tb_sw_read(sw, &val, TB_CFG_SWITCH,
+ sw->cap_lc + TB_LC_SNK_ALLOCATION, 1);
+ if (ret)
+ return ret;
+
+ if (!sink)
+ val &= ~TB_LC_SNK_ALLOCATION_SNK0_MASK;
+ else
+ val &= ~TB_LC_SNK_ALLOCATION_SNK1_MASK;
+
+ ret = tb_sw_write(sw, &val, TB_CFG_SWITCH,
+ sw->cap_lc + TB_LC_SNK_ALLOCATION, 1);
+ if (ret)
+ return ret;
+
+ tb_port_dbg(in, "sink %d de-allocated\n", sink);
+ return 0;
+}
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 604cb3ef4985..87c74c916a7c 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -645,6 +645,7 @@ static int tb_init_port(struct tb_port *port)
ida_init(&port->out_hopids);
}
+ INIT_LIST_HEAD(&port->list);
return 0;
}
@@ -2292,6 +2293,49 @@ void tb_switch_suspend(struct tb_switch *sw)
tb_lc_set_sleep(sw);
}
+/**
+ * tb_switch_query_dp_resource() - Query availability of DP resource
+ * @sw: Switch whose DP resource is queried
+ * @in: DP IN port
+ *
+ * Queries availability of DP resource for DP tunneling using switch
+ * specific means. Returns %true if resource is available.
+ */
+bool tb_switch_query_dp_resource(struct tb_switch *sw, struct tb_port *in)
+{
+ return tb_lc_dp_sink_query(sw, in);
+}
+
+/**
+ * tb_switch_alloc_dp_resource() - Allocate available DP resource
+ * @sw: Switch whose DP resource is allocated
+ * @in: DP IN port
+ *
+ * Allocates DP resource for DP tunneling. The resource must be
+ * available for this to succeed (see tb_switch_query_dp_resource()).
+ * Returns %0 in success and negative errno otherwise.
+ */
+int tb_switch_alloc_dp_resource(struct tb_switch *sw, struct tb_port *in)
+{
+ return tb_lc_dp_sink_alloc(sw, in);
+}
+
+/**
+ * tb_switch_dealloc_dp_resource() - De-allocate DP resource
+ * @sw: Switch whose DP resource is de-allocated
+ * @in: DP IN port
+ *
+ * De-allocates DP resource that was previously allocated for DP
+ * tunneling.
+ */
+void tb_switch_dealloc_dp_resource(struct tb_switch *sw, struct tb_port *in)
+{
+ if (tb_lc_dp_sink_dealloc(sw, in)) {
+ tb_sw_warn(sw, "failed to de-allocate DP resource for port %d\n",
+ in->port);
+ }
+}
+
struct tb_sw_lookup {
struct tb *tb;
u8 link;
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 704455a4f763..5b457874e51a 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -18,6 +18,7 @@
/**
* struct tb_cm - Simple Thunderbolt connection manager
* @tunnel_list: List of active tunnels
+ * @dp_resources: List of available DP resources for DP tunneling
* @hotplug_active: tb_handle_hotplug will stop progressing plug
* events and exit if this is not set (it needs to
* acquire the lock one more time). Used to drain wq
@@ -25,6 +26,7 @@
*/
struct tb_cm {
struct list_head tunnel_list;
+ struct list_head dp_resources;
bool hotplug_active;
};
@@ -56,6 +58,44 @@ static void tb_queue_hotplug(struct tb *tb, u64 route, u8 port, bool unplug)
/* enumeration & hot plug handling */
+static void tb_add_dp_resources(struct tb_switch *sw)
+{
+ struct tb_cm *tcm = tb_priv(sw->tb);
+ struct tb_port *port;
+ int i;
+
+ tb_switch_for_each_port(sw, i) {
+ port = &sw->ports[i];
+
+ if (!tb_port_is_dpin(port))
+ continue;
+
+ if (!tb_switch_query_dp_resource(sw, port))
+ continue;
+
+ list_add_tail(&port->list, &tcm->dp_resources);
+ tb_port_dbg(port, "DP IN resource available\n");
+ }
+}
+
+static void tb_remove_dp_resources(struct tb_switch *sw)
+{
+ struct tb_cm *tcm = tb_priv(sw->tb);
+ struct tb_port *port, *tmp;
+ int i;
+
+ /* Clear children resources first */
+ tb_switch_for_each_remote_port(sw, i)
+ tb_remove_dp_resources(sw->ports[i].remote->sw);
+
+ list_for_each_entry_safe(port, tmp, &tcm->dp_resources, list) {
+ if (port->sw == sw) {
+ tb_port_dbg(port, "DP OUT resource unavailable\n");
+ list_del_init(&port->list);
+ }
+ }
+}
+
static void tb_discover_tunnels(struct tb_switch *sw)
{
struct tb *tb = sw->tb;
@@ -226,8 +266,9 @@ static void tb_scan_port(struct tb_port *port)
tb_scan_switch(sw);
}
-static int tb_free_tunnel(struct tb *tb, enum tb_tunnel_type type,
- struct tb_port *src_port, struct tb_port *dst_port)
+static struct tb_tunnel *tb_find_tunnel(struct tb *tb, enum tb_tunnel_type type,
+ struct tb_port *src_port,
+ struct tb_port *dst_port)
{
struct tb_cm *tcm = tb_priv(tb);
struct tb_tunnel *tunnel;
@@ -236,14 +277,32 @@ static int tb_free_tunnel(struct tb *tb, enum tb_tunnel_type type,
if (tunnel->type == type &&
((src_port && src_port == tunnel->src_port) ||
(dst_port && dst_port == tunnel->dst_port))) {
- tb_tunnel_deactivate(tunnel);
- list_del(&tunnel->list);
- tb_tunnel_free(tunnel);
- return 0;
+ return tunnel;
}
}
- return -ENODEV;
+ return NULL;
+}
+
+static void tb_deactivate_and_free_tunnel(struct tb_tunnel *tunnel)
+{
+ if (!tunnel)
+ return;
+
+ tb_tunnel_deactivate(tunnel);
+ list_del(&tunnel->list);
+
+ /*
+ * In case of DP tunnel make sure the DP IN resource is deallocated
+ * properly.
+ */
+ if (tb_tunnel_is_dp(tunnel)) {
+ struct tb_port *in = tunnel->src_port;
+
+ tb_switch_dealloc_dp_resource(in->sw, in);
+ }
+
+ tb_tunnel_free(tunnel);
}
/**
@@ -256,11 +315,8 @@ static void tb_free_invalid_tunnels(struct tb *tb)
struct tb_tunnel *n;
list_for_each_entry_safe(tunnel, n, &tcm->tunnel_list, list) {
- if (tb_tunnel_is_invalid(tunnel)) {
- tb_tunnel_deactivate(tunnel);
- list_del(&tunnel->list);
- tb_tunnel_free(tunnel);
- }
+ if (tb_tunnel_is_invalid(tunnel))
+ tb_deactivate_and_free_tunnel(tunnel);
}
}
@@ -275,6 +331,7 @@ static void tb_free_unplugged_children(struct tb_switch *sw)
struct tb_port *port = &sw->ports[i];
if (port->remote->sw->is_unplugged) {
+ tb_remove_dp_resources(port->remote->sw);
tb_switch_lane_bonding_disable(port->remote->sw);
tb_switch_remove(port->remote->sw);
port->remote = NULL;
@@ -368,42 +425,112 @@ static struct tb_port *tb_find_pcie_down(struct tb_switch *sw,
return tb_find_unused_port(sw, TB_TYPE_PCIE_DOWN);
}
-static int tb_tunnel_dp(struct tb *tb, struct tb_port *out)
+static void tb_tunnel_dp(struct tb *tb)
{
struct tb_cm *tcm = tb_priv(tb);
- struct tb_switch *sw = out->sw;
+ struct tb_port *port, *in, *out;
struct tb_tunnel *tunnel;
- struct tb_port *in;
- if (tb_port_is_enabled(out))
- return 0;
+ /*
+ * Find pair of inactive DP IN and DP OUT adapters and then
+ * establish a DP tunnel between them.
+ */
+ tb_dbg(tb, "looking for DP IN <-> DP OUT pairs:\n");
+
+ in = NULL;
+ out = NULL;
+ list_for_each_entry(port, &tcm->dp_resources, list) {
+ if (tb_port_is_enabled(port)) {
+ tb_port_dbg(port, "in use\n");
+ continue;
+ }
+
+ tb_port_dbg(port, "available\n");
+
+ if (!in && tb_port_is_dpin(port))
+ in = port;
+ else if (!out && tb_port_is_dpout(port))
+ out = port;
+ }
+
+ if (!in) {
+ tb_dbg(tb, "no suitable DP IN adapter available, not tunneling\n");
+ return;
+ }
+ if (!out) {
+ tb_dbg(tb, "no suitable DP OUT adapter available, not tunneling\n");
+ return;
+ }
- do {
- sw = tb_to_switch(sw->dev.parent);
- if (!sw)
- return 0;
- in = tb_find_unused_port(sw, TB_TYPE_DP_HDMI_IN);
- } while (!in);
+ if (tb_switch_alloc_dp_resource(in->sw, in)) {
+ tb_port_dbg(in, "no resource available for DP IN, not tunneling\n");
+ return;
+ }
tunnel = tb_tunnel_alloc_dp(tb, in, out);
if (!tunnel) {
- tb_port_dbg(out, "DP tunnel allocation failed\n");
- return -ENOMEM;
+ tb_port_dbg(out, "could not allocate DP tunnel\n");
+ goto dealloc_dp;
}
if (tb_tunnel_activate(tunnel)) {
tb_port_info(out, "DP tunnel activation failed, aborting\n");
tb_tunnel_free(tunnel);
- return -EIO;
+ goto dealloc_dp;
}
list_add_tail(&tunnel->list, &tcm->tunnel_list);
- return 0;
+ return;
+
+dealloc_dp:
+ tb_switch_dealloc_dp_resource(in->sw, in);
}
-static void tb_teardown_dp(struct tb *tb, struct tb_port *out)
+static void tb_dp_resource_unavailable(struct tb *tb, struct tb_port *port)
{
- tb_free_tunnel(tb, TB_TUNNEL_DP, NULL, out);
+ struct tb_port *in, *out;
+ struct tb_tunnel *tunnel;
+
+ if (tb_port_is_dpin(port)) {
+ tb_port_dbg(port, "DP IN resource unavailable\n");
+ in = port;
+ out = NULL;
+ } else {
+ tb_port_dbg(port, "DP OUT resource unavailable\n");
+ in = NULL;
+ out = port;
+ }
+
+ tunnel = tb_find_tunnel(tb, TB_TUNNEL_DP, in, out);
+ tb_deactivate_and_free_tunnel(tunnel);
+ list_del_init(&port->list);
+
+ /*
+ * See if there is another DP OUT port that can be used for
+ * to create another tunnel.
+ */
+ tb_tunnel_dp(tb);
+}
+
+static void tb_dp_resource_available(struct tb *tb, struct tb_port *port)
+{
+ struct tb_cm *tcm = tb_priv(tb);
+ struct tb_port *p;
+
+ if (tb_port_is_enabled(port))
+ return;
+
+ list_for_each_entry(p, &tcm->dp_resources, list) {
+ if (p == port)
+ return;
+ }
+
+ tb_port_dbg(port, "DP %s resource available\n",
+ tb_port_is_dpin(port) ? "IN" : "OUT");
+ list_add_tail(&port->list, &tcm->dp_resources);
+
+ /* Look for suitable DP IN <-> DP OUT pairs now */
+ tb_tunnel_dp(tb);
}
static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw)
@@ -478,6 +605,7 @@ static int tb_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd)
static void __tb_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd)
{
struct tb_port *dst_port;
+ struct tb_tunnel *tunnel;
struct tb_switch *sw;
sw = tb_to_switch(xd->dev.parent);
@@ -488,7 +616,8 @@ static void __tb_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd)
* case of cable disconnect) so it is fine if we cannot find it
* here anymore.
*/
- tb_free_tunnel(tb, TB_TUNNEL_DMA, NULL, dst_port);
+ tunnel = tb_find_tunnel(tb, TB_TUNNEL_DMA, NULL, dst_port);
+ tb_deactivate_and_free_tunnel(tunnel);
}
static int tb_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd)
@@ -543,11 +672,14 @@ static void tb_handle_hotplug(struct work_struct *work)
tb_port_dbg(port, "switch unplugged\n");
tb_sw_set_unplugged(port->remote->sw);
tb_free_invalid_tunnels(tb);
+ tb_remove_dp_resources(port->remote->sw);
tb_switch_lane_bonding_disable(port->remote->sw);
tb_switch_remove(port->remote->sw);
port->remote = NULL;
if (port->dual_link_port)
port->dual_link_port->remote = NULL;
+ /* Maybe we can create another DP tunnel */
+ tb_tunnel_dp(tb);
} else if (port->xdomain) {
struct tb_xdomain *xd = tb_xdomain_get(port->xdomain);
@@ -564,8 +696,8 @@ static void tb_handle_hotplug(struct work_struct *work)
port->xdomain = NULL;
__tb_disconnect_xdomain_paths(tb, xd);
tb_xdomain_put(xd);
- } else if (tb_port_is_dpout(port)) {
- tb_teardown_dp(tb, port);
+ } else if (tb_port_is_dpout(port) || tb_port_is_dpin(port)) {
+ tb_dp_resource_unavailable(tb, port);
} else {
tb_port_dbg(port,
"got unplug event for disconnected port, ignoring\n");
@@ -578,8 +710,8 @@ static void tb_handle_hotplug(struct work_struct *work)
tb_scan_port(port);
if (!port->remote)
tb_port_dbg(port, "hotplug: no switch found\n");
- } else if (tb_port_is_dpout(port)) {
- tb_tunnel_dp(tb, port);
+ } else if (tb_port_is_dpout(port) || tb_port_is_dpin(port)) {
+ tb_dp_resource_available(tb, port);
}
}
@@ -692,6 +824,8 @@ static int tb_start(struct tb *tb)
tb_scan_switch(tb->root_switch);
/* Find out tunnels created by the boot firmware */
tb_discover_tunnels(tb->root_switch);
+ /* Add DP IN resources for the root switch */
+ tb_add_dp_resources(tb->root_switch);
/* Make the discovered switches available to the userspace */
device_for_each_child(&tb->root_switch->dev, NULL,
tb_scan_finalize_switch);
@@ -821,6 +955,7 @@ struct tb *tb_probe(struct tb_nhi *nhi)
tcm = tb_priv(tb);
INIT_LIST_HEAD(&tcm->tunnel_list);
+ INIT_LIST_HEAD(&tcm->dp_resources);
return tb;
}
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index dbab06551eaa..48f3725249a9 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -137,6 +137,7 @@ struct tb_switch {
* @link_nr: Is this primary or secondary port on the dual_link.
* @in_hopids: Currently allocated input HopIDs
* @out_hopids: Currently allocated output HopIDs
+ * @list: Used to link ports to DP resources list
*/
struct tb_port {
struct tb_regs_port_header config;
@@ -152,6 +153,7 @@ struct tb_port {
u8 link_nr:1;
struct ida in_hopids;
struct ida out_hopids;
+ struct list_head list;
};
/**
@@ -674,6 +676,10 @@ static inline bool tb_switch_is_icm(const struct tb_switch *sw)
int tb_switch_lane_bonding_enable(struct tb_switch *sw);
void tb_switch_lane_bonding_disable(struct tb_switch *sw);
+bool tb_switch_query_dp_resource(struct tb_switch *sw, struct tb_port *in);
+int tb_switch_alloc_dp_resource(struct tb_switch *sw, struct tb_port *in);
+void tb_switch_dealloc_dp_resource(struct tb_switch *sw, struct tb_port *in);
+
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_set_initial_credits(struct tb_port *port, u32 credits);
@@ -718,6 +724,9 @@ int tb_lc_configure_link(struct tb_switch *sw);
void tb_lc_unconfigure_link(struct tb_switch *sw);
int tb_lc_set_sleep(struct tb_switch *sw);
bool tb_lc_lane_bonding_possible(struct tb_switch *sw);
+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);
static inline int tb_route_length(u64 route)
{
diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
index 8d11b4a2d552..aec35e61cc14 100644
--- a/drivers/thunderbolt/tb_regs.h
+++ b/drivers/thunderbolt/tb_regs.h
@@ -295,6 +295,12 @@ struct tb_regs_hop {
#define TB_LC_DESC_PORT_SIZE_SHIFT 16
#define TB_LC_DESC_PORT_SIZE_MASK GENMASK(27, 16)
#define TB_LC_FUSE 0x03
+#define TB_LC_SNK_ALLOCATION 0x10
+#define TB_LC_SNK_ALLOCATION_SNK0_MASK GENMASK(3, 0)
+#define TB_LC_SNK_ALLOCATION_SNK0_CM 0x1
+#define TB_LC_SNK_ALLOCATION_SNK1_SHIFT 4
+#define TB_LC_SNK_ALLOCATION_SNK1_MASK GENMASK(7, 4)
+#define TB_LC_SNK_ALLOCATION_SNK1_CM 0x1
/* Link controller registers */
#define TB_LC_PORT_ATTR 0x8d
--
2.23.0
The function does not modify the argument in any way so make it const.
Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/tb.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 1565af2e48cb..455ca490ea87 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -399,7 +399,7 @@ static inline int tb_sw_read(struct tb_switch *sw, void *buffer,
length);
}
-static inline int tb_sw_write(struct tb_switch *sw, void *buffer,
+static inline int tb_sw_write(struct tb_switch *sw, const void *buffer,
enum tb_cfg_space space, u32 offset, u32 length)
{
if (sw->is_unplugged)
--
2.23.0
If we fail to add a switch for some reason log a warning with the error
code. This is useful for debugging.
Signed-off-by: Mika Westerberg <[email protected]>
---
drivers/thunderbolt/tb.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 1f7a9e1cc09c..541295be9bfc 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -143,6 +143,7 @@ static void tb_scan_port(struct tb_port *port)
struct tb_cm *tcm = tb_priv(port->sw->tb);
struct tb_port *upstream_port;
struct tb_switch *sw;
+ int ret;
if (tb_is_upstream_port(port))
return;
@@ -203,7 +204,9 @@ static void tb_scan_port(struct tb_port *port)
if (!tcm->hotplug_active)
dev_set_uevent_suppress(&sw->dev, true);
- if (tb_switch_add(sw)) {
+ ret = tb_switch_add(sw);
+ if (ret) {
+ dev_warn(&sw->dev, "failed to register switch: %d\n", ret);
tb_switch_put(sw);
return;
}
--
2.23.0
On Tue, Oct 01, 2019 at 02:38:11PM +0300, Mika Westerberg wrote:
> If we fail to add a switch for some reason log a warning with the error
> code. This is useful for debugging.
>
> Signed-off-by: Mika Westerberg <[email protected]>
> ---
> drivers/thunderbolt/tb.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index 1f7a9e1cc09c..541295be9bfc 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -143,6 +143,7 @@ static void tb_scan_port(struct tb_port *port)
> struct tb_cm *tcm = tb_priv(port->sw->tb);
> struct tb_port *upstream_port;
> struct tb_switch *sw;
> + int ret;
>
> if (tb_is_upstream_port(port))
> return;
> @@ -203,7 +204,9 @@ static void tb_scan_port(struct tb_port *port)
> if (!tcm->hotplug_active)
> dev_set_uevent_suppress(&sw->dev, true);
>
> - if (tb_switch_add(sw)) {
> + ret = tb_switch_add(sw);
> + if (ret) {
> + dev_warn(&sw->dev, "failed to register switch: %d\n", ret);
Shouldn't tb_switch_add() do the error reporting instead? That way it
makes it easier to call functions and not always have to print failure
messages :)
thanks,
greg k-h
On Tue, Oct 01, 2019 at 02:38:14PM +0300, Mika Westerberg wrote:
> Lane bonding allows aggregating the two 10/20 Gb/s (depending on the
> generation) lanes into a single 20/40 Gb/s bonded link. This allows
> sharing the full bandwidth more efficiently. In order to establish lane
> bonding we need to check that the lane bonding is possible through LC
> and that both end of the link actually supports 2x widths. This also
> means that all the paths should be established through the primary port
> so update tb_path_alloc() to handle this as well.
>
> Lane bonding is supported starting from Falcon Ridge (2nd generation)
> controllers.
Are we only going to be allowed to "bond" two links together? Or will
we be doing more than 2 in the future? If more, then we might want to
think of a different way to specify these...
Anyway, one tiny nit below:
>
> Signed-off-by: Mika Westerberg <[email protected]>
> ---
> .../ABI/testing/sysfs-bus-thunderbolt | 17 ++
> drivers/thunderbolt/icm.c | 18 +-
> drivers/thunderbolt/lc.c | 28 ++
> drivers/thunderbolt/path.c | 30 +-
> drivers/thunderbolt/switch.c | 274 ++++++++++++++++++
> drivers/thunderbolt/tb.c | 21 ++
> drivers/thunderbolt/tb.h | 10 +
> drivers/thunderbolt/tb_msgs.h | 2 +
> drivers/thunderbolt/tb_regs.h | 20 ++
> drivers/thunderbolt/tunnel.c | 19 +-
> 10 files changed, 429 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt
> index b21fba14689b..2c9166f6fa97 100644
> --- a/Documentation/ABI/testing/sysfs-bus-thunderbolt
> +++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt
> @@ -104,6 +104,23 @@ Contact: [email protected]
> Description: This attribute contains name of this device extracted from
> the device DROM.
>
> +What: /sys/bus/thunderbolt/devices/.../link_speed
> +Date: Apr 2020
> +KernelVersion: 5.6
> +Contact: Mika Westerberg <[email protected]>
> +Description: This attribute reports the current upstream link speed
> + in Gb/s per lane. If there are two lanes they both are
> + running at the same speed. Use link_width to determine
> + whether the two lanes are bonded or not.
> +
> +What: /sys/bus/thunderbolt/devices/.../link_width
> +Date: Apr 2020
> +KernelVersion: 5.6
> +Contact: Mika Westerberg <[email protected]>
> +Description: This attribute reports the current upstream link width.
> + It is 1 for single lane link (or two single lane links)
> + and 2 for bonded dual lane link.
> +
> What: /sys/bus/thunderbolt/devices/.../vendor
> Date: Sep 2017
> KernelVersion: 4.13
> diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
> index 6550f68f92ce..9c9c6ea2b790 100644
> --- a/drivers/thunderbolt/icm.c
> +++ b/drivers/thunderbolt/icm.c
> @@ -567,7 +567,8 @@ static struct tb_switch *add_switch(struct tb_switch *parent_sw, u64 route,
> size_t ep_name_size, u8 connection_id,
> u8 connection_key, u8 link, u8 depth,
> enum tb_security_level security_level,
> - bool authorized, bool boot)
> + bool authorized, bool boot, bool dual_lane,
> + bool speed_gen3)
That's just a crazy amount of function parameters, with no way of
remembering what is what, especially when you add 2 more booleans at the
end.
It's your code, but ugh, that's going to be hard to maintain over time
:(
> {
> const struct intel_vss *vss;
> struct tb_switch *sw;
> @@ -592,6 +593,8 @@ static struct tb_switch *add_switch(struct tb_switch *parent_sw, u64 route,
> sw->authorized = authorized;
> sw->security_level = security_level;
> sw->boot = boot;
> + sw->link_speed = speed_gen3 ? 20 : 10;
> + sw->link_width = dual_lane ? 2 : 1;
> init_completion(&sw->rpm_complete);
>
> vss = parse_intel_vss(ep_name, ep_name_size);
> @@ -697,11 +700,11 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
> (const struct icm_fr_event_device_connected *)hdr;
> enum tb_security_level security_level;
> struct tb_switch *sw, *parent_sw;
> + bool boot, dual_lane, speed_gen3;
> struct icm *icm = tb_priv(tb);
> bool authorized = false;
> struct tb_xdomain *xd;
> u8 link, depth;
> - bool boot;
> u64 route;
> int ret;
>
> @@ -714,6 +717,8 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
> security_level = (pkg->hdr.flags & ICM_FLAGS_SLEVEL_MASK) >>
> ICM_FLAGS_SLEVEL_SHIFT;
> boot = pkg->link_info & ICM_LINK_INFO_BOOT;
> + dual_lane = pkg->hdr.flags & ICM_FLAGS_DUAL_LANE;
> + speed_gen3 = pkg->hdr.flags & ICM_FLAGS_SPEED_GEN3;
>
> if (pkg->link_info & ICM_LINK_INFO_REJECTED) {
> tb_info(tb, "switch at %u.%u was rejected by ICM firmware because topology limit exceeded\n",
> @@ -814,7 +819,7 @@ icm_fr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr)
> add_switch(parent_sw, route, &pkg->ep_uuid, (const u8 *)pkg->ep_name,
> sizeof(pkg->ep_name), pkg->connection_id,
> pkg->connection_key, link, depth, security_level,
> - authorized, boot);
> + authorized, boot, dual_lane, speed_gen3);
>
> tb_switch_put(parent_sw);
> }
> @@ -1142,10 +1147,10 @@ __icm_tr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr,
> {
> const struct icm_tr_event_device_connected *pkg =
> (const struct icm_tr_event_device_connected *)hdr;
> + bool authorized, boot, dual_lane, speed_gen3;
> enum tb_security_level security_level;
> struct tb_switch *sw, *parent_sw;
> struct tb_xdomain *xd;
> - bool authorized, boot;
> u64 route;
>
> icm_postpone_rescan(tb);
> @@ -1163,6 +1168,8 @@ __icm_tr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr,
> security_level = (pkg->hdr.flags & ICM_FLAGS_SLEVEL_MASK) >>
> ICM_FLAGS_SLEVEL_SHIFT;
> boot = pkg->link_info & ICM_LINK_INFO_BOOT;
> + dual_lane = pkg->hdr.flags & ICM_FLAGS_DUAL_LANE;
> + speed_gen3 = pkg->hdr.flags & ICM_FLAGS_SPEED_GEN3;
>
> if (pkg->link_info & ICM_LINK_INFO_REJECTED) {
> tb_info(tb, "switch at %llx was rejected by ICM firmware because topology limit exceeded\n",
> @@ -1207,7 +1214,8 @@ __icm_tr_device_connected(struct tb *tb, const struct icm_pkg_header *hdr,
>
> sw = add_switch(parent_sw, route, &pkg->ep_uuid, (const u8 *)pkg->ep_name,
> sizeof(pkg->ep_name), pkg->connection_id, 0, 0, 0,
> - security_level, authorized, boot);
> + security_level, authorized, boot, dual_lane,
> + speed_gen3);
> if (!IS_ERR(sw) && force_rtd3)
> sw->rpm = true;
>
> diff --git a/drivers/thunderbolt/lc.c b/drivers/thunderbolt/lc.c
> index af38076088f6..df56523eb822 100644
> --- a/drivers/thunderbolt/lc.c
> +++ b/drivers/thunderbolt/lc.c
> @@ -177,3 +177,31 @@ int tb_lc_set_sleep(struct tb_switch *sw)
>
> return 0;
> }
> +
> +/**
> + * tb_lc_lane_bonding_possible() - Is lane bonding possible towards switch
> + * @sw: Switch to check
> + *
> + * Checks whether conditions for lane bonding from parent to @sw are
> + * possible.
> + */
> +bool tb_lc_lane_bonding_possible(struct tb_switch *sw)
> +{
> + struct tb_port *up;
> + int cap, ret;
> + u32 val;
> +
> + if (sw->generation < 2)
> + return false;
> +
> + up = tb_upstream_port(sw);
> + cap = find_port_lc_cap(up);
> + if (cap < 0)
> + return false;
> +
> + ret = tb_sw_read(sw, &val, TB_CFG_SWITCH, cap + TB_LC_PORT_ATTR, 1);
> + if (ret)
> + return false;
> +
> + return !!(val & TB_LC_PORT_ATTR_BE);
> +}
> diff --git a/drivers/thunderbolt/path.c b/drivers/thunderbolt/path.c
> index afe5f8391ebf..6cf66597d5d8 100644
> --- a/drivers/thunderbolt/path.c
> +++ b/drivers/thunderbolt/path.c
> @@ -220,7 +220,8 @@ struct tb_path *tb_path_discover(struct tb_port *src, int src_hopid,
> * Creates path between two ports starting with given @src_hopid. Reserves
> * HopIDs for each port (they can be different from @src_hopid depending on
> * how many HopIDs each port already have reserved). If there are dual
> - * links on the path, prioritizes using @link_nr.
> + * links on the path, prioritizes using @link_nr but takes into account
> + * that the lanes may be bonded.
> *
> * Return: Returns a tb_path on success or NULL on failure.
> */
> @@ -259,7 +260,9 @@ struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
> if (!in_port)
> goto err;
>
> - if (in_port->dual_link_port && in_port->link_nr != link_nr)
> + /* When lanes are bonded primary link must be used */
> + if (!in_port->bonded && in_port->dual_link_port &&
> + in_port->link_nr != link_nr)
> in_port = in_port->dual_link_port;
>
> ret = tb_port_alloc_in_hopid(in_port, in_hopid, in_hopid);
> @@ -271,8 +274,27 @@ struct tb_path *tb_path_alloc(struct tb *tb, struct tb_port *src, int src_hopid,
> if (!out_port)
> goto err;
>
> - if (out_port->dual_link_port && out_port->link_nr != link_nr)
> - out_port = out_port->dual_link_port;
> + /*
> + * Pick up right port when going from non-bonded to
> + * bonded or from bonded to non-bonded.
> + */
> + if (out_port->dual_link_port) {
> + if (!in_port->bonded && out_port->bonded &&
> + out_port->link_nr) {
> + /*
> + * Use primary link when going from
> + * non-bonded to bonded.
> + */
> + out_port = out_port->dual_link_port;
> + } else if (!out_port->bonded &&
> + out_port->link_nr != link_nr) {
> + /*
> + * If out port is not bonded follow
> + * link_nr.
> + */
> + out_port = out_port->dual_link_port;
> + }
> + }
>
> if (i == num_hops - 1)
> ret = tb_port_alloc_out_hopid(out_port, dst_hopid,
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> index cc2670dd2698..2b00ea7a979a 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -775,6 +775,132 @@ struct tb_port *tb_next_port_on_path(struct tb_port *start, struct tb_port *end,
> return next;
> }
>
> +static int tb_port_get_link_speed(struct tb_port *port)
> +{
> + u32 val, speed;
> + int ret;
> +
> + if (!port->cap_phy)
> + return -EINVAL;
> +
> + ret = tb_port_read(port, &val, TB_CFG_PORT,
> + port->cap_phy + LANE_ADP_CS_1, 1);
> + if (ret)
> + return ret;
> +
> + speed = (val & LANE_ADP_CS_1_CURRENT_SPEED_MASK) >>
> + LANE_ADP_CS_1_CURRENT_SPEED_SHIFT;
> + return speed == LANE_ADP_CS_1_CURRENT_SPEED_GEN3 ? 20 : 10;
> +}
> +
> +static int tb_port_get_link_width(struct tb_port *port)
> +{
> + u32 val;
> + int ret;
> +
> + if (!port->cap_phy)
> + return -EINVAL;
> +
> + ret = tb_port_read(port, &val, TB_CFG_PORT,
> + port->cap_phy + LANE_ADP_CS_1, 1);
> + if (ret)
> + return ret;
> +
> + return (val & LANE_ADP_CS_1_CURRENT_WIDTH_MASK) >>
> + LANE_ADP_CS_1_CURRENT_WIDTH_SHIFT;
> +}
> +
> +static bool tb_port_is_width_supported(struct tb_port *port, int width)
> +{
> + u32 phy, widths;
> + int ret;
> +
> + if (!port->cap_phy)
> + return false;
> +
> + ret = tb_port_read(port, &phy, TB_CFG_PORT,
> + port->cap_phy + LANE_ADP_CS_0, 1);
> + if (ret)
> + return ret;
> +
> + widths = (phy & LANE_ADP_CS_0_SUPPORTED_WIDTH_MASK) >>
> + LANE_ADP_CS_0_SUPPORTED_WIDTH_SHIFT;
> +
> + return !!(widths & width);
> +}
> +
> +static int tb_port_set_link_width(struct tb_port *port, unsigned int width)
> +{
> + u32 val;
> + int ret;
> +
> + if (!port->cap_phy)
> + return -EINVAL;
> +
> + ret = tb_port_read(port, &val, TB_CFG_PORT,
> + port->cap_phy + LANE_ADP_CS_1, 1);
> + if (ret)
> + return ret;
> +
> + val &= ~LANE_ADP_CS_1_TARGET_WIDTH_MASK;
> + switch (width) {
> + case 1:
> + val |= LANE_ADP_CS_1_TARGET_WIDTH_SINGLE <<
> + LANE_ADP_CS_1_TARGET_WIDTH_SHIFT;
> + break;
> + case 2:
> + val |= LANE_ADP_CS_1_TARGET_WIDTH_DUAL <<
> + LANE_ADP_CS_1_TARGET_WIDTH_SHIFT;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + val |= LANE_ADP_CS_1_LB;
> +
> + return tb_port_write(port, &val, TB_CFG_PORT,
> + port->cap_phy + LANE_ADP_CS_1, 1);
> +}
> +
> +static int tb_port_lane_bonding_enable(struct tb_port *port)
> +{
> + int ret;
> +
> + /*
> + * Enable lane bonding for both links if not already enabled by
> + * for example the boot firmware.
> + */
> + ret = tb_port_get_link_width(port);
> + if (ret == 1) {
> + ret = tb_port_set_link_width(port, 2);
> + if (ret)
> + return ret;
> + }
> +
> + ret = tb_port_get_link_width(port->dual_link_port);
> + if (ret == 1) {
> + ret = tb_port_set_link_width(port->dual_link_port, 2);
> + if (ret) {
> + tb_port_set_link_width(port, 1);
> + return ret;
> + }
> + }
> +
> + port->bonded = true;
> + port->dual_link_port->bonded = true;
> +
> + return 0;
> +}
> +
> +static void tb_port_lane_bonding_disable(struct tb_port *port)
> +{
> + port->dual_link_port->bonded = false;
> + port->bonded = false;
> +
> + tb_port_set_link_width(port->dual_link_port, 1);
> + tb_port_set_link_width(port, 1);
> +}
> +
> /**
> * tb_port_is_enabled() - Is the adapter port enabled
> * @port: Port to check
> @@ -1166,6 +1292,26 @@ static ssize_t key_store(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR(key, 0600, key_show, key_store);
>
> +static ssize_t link_speed_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct tb_switch *sw = tb_to_switch(dev);
> +
> + return sprintf(buf, "%u.0 Gb/s\n", sw->link_speed);
Why units? ".0" is implied as it always will be the case, so why not
just have the units be Gb in the sysfs documentation file and then just
report link_speed here with nothing else? Makes it much easier for
userspace to parse as well.
Otherwise, looks sane to me, nice job.
greg k-h
On Tue, Oct 01, 2019 at 02:38:16PM +0300, Mika Westerberg wrote:
> In order to keep PCIe hierarchies consistent across hotplugs, add
> hard-coded PCIe downstream port to Thunderbolt port for Alpine Ridge and
> Titan Ridge as well.
Oh, that makes me nervous, how could a hard-coded pcie port ever get set
up incorrectly :)
How do we "know" that this is correct? Is there any ACPI guarantees
that this "always will be so"? If not, we all know someone will mess
this up...
>
> Signed-off-by: Mika Westerberg <[email protected]>
> ---
> drivers/thunderbolt/tb.c | 4 +++-
> drivers/thunderbolt/tb.h | 25 +++++++++++++++++++++++++
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index dbbe9afb9fb7..704455a4f763 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -344,10 +344,12 @@ static struct tb_port *tb_find_pcie_down(struct tb_switch *sw,
> * Hard-coded Thunderbolt port to PCIe down port mapping
> * per controller.
> */
> - if (tb_switch_is_cr(sw))
> + if (tb_switch_is_cr(sw) || tb_switch_is_ar(sw))
> index = !phy_port ? 6 : 7;
> else if (tb_switch_is_fr(sw))
> index = !phy_port ? 6 : 8;
> + else if (tb_switch_is_tr(sw))
> + index = !phy_port ? 8 : 9;
> else
> goto out;
>
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index e641dcebd50a..dbab06551eaa 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -632,6 +632,31 @@ static inline bool tb_switch_is_fr(const struct tb_switch *sw)
> }
> }
>
> +static inline bool tb_switch_is_ar(const struct tb_switch *sw)
"ar"? Can you spell it out?
> +{
> + switch (sw->config.device_id) {
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_BRIDGE:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static inline bool tb_switch_is_tr(const struct tb_switch *sw)
Same for "tr" please.
thanks,
greg k-h
On Tue, Oct 01, 2019 at 02:18:04PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 01, 2019 at 02:38:11PM +0300, Mika Westerberg wrote:
> > If we fail to add a switch for some reason log a warning with the error
> > code. This is useful for debugging.
> >
> > Signed-off-by: Mika Westerberg <[email protected]>
> > ---
> > drivers/thunderbolt/tb.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> > index 1f7a9e1cc09c..541295be9bfc 100644
> > --- a/drivers/thunderbolt/tb.c
> > +++ b/drivers/thunderbolt/tb.c
> > @@ -143,6 +143,7 @@ static void tb_scan_port(struct tb_port *port)
> > struct tb_cm *tcm = tb_priv(port->sw->tb);
> > struct tb_port *upstream_port;
> > struct tb_switch *sw;
> > + int ret;
> >
> > if (tb_is_upstream_port(port))
> > return;
> > @@ -203,7 +204,9 @@ static void tb_scan_port(struct tb_port *port)
> > if (!tcm->hotplug_active)
> > dev_set_uevent_suppress(&sw->dev, true);
> >
> > - if (tb_switch_add(sw)) {
> > + ret = tb_switch_add(sw);
> > + if (ret) {
> > + dev_warn(&sw->dev, "failed to register switch: %d\n", ret);
>
> Shouldn't tb_switch_add() do the error reporting instead? That way it
> makes it easier to call functions and not always have to print failure
> messages :)
Yes, that's better - I'll move it there.
On Tue, Oct 01, 2019 at 02:38:08PM +0300, Mika Westerberg wrote:
> Hi all,
>
> I'm sending this as RFC because the series is still missing important
> features such as power management so not ready for merging. However, I
> think it is good to get any early feedback from the community. We are
> working to add support for the missing features.
>
> USB4 is the public specification of Thunderbolt 3 protocol and can be
> downloaded here:
>
> https://www.usb.org/sites/default/files/USB4%20Specification_1.zip
>
> USB4 is about tunneling different protocols over a single cable (in the
> same way as Thunderbolt). The current USB4 spec supports PCIe, Display Port
> and USB 3.x, and also software based protocols such as networking between
> domains (hosts).
>
> So far PCs have been using firmware based Connection Manager and Apple
> systems have been using software based one. A Connection Manager is the
> entity that handles creation of different tunnel types through the USB4
> (and Thunderbolt) fabric. With USB4 the plan is to have software based
> Connection Manager everywhere but some early systems will also support
> firmware to allow OS downgrade for example.
>
> Current Linux Thunderbolt driver supports both "modes" and can detect which
> one to use dynamically.
>
> This series first adds support for Thunderbolt 3 devices to the software
> connection manager and then extends that to support USB4 compliant hosts
> and devices (this applies to both firmware and software based connection
> managers). With this series the following features are supported also for
> USB4 compliant devices:
>
> - PCIe tunneling
> - Display Port tunneling
> - USB 3.x tunneling
> - P2P networking (implemented in drivers/net/thunderbolt.c)
> - Host and device NVM firmware upgrade
>
> We also add two new sysfs attributes under each device that expose link
> speed and width to userspace. The rest of the userspace ABI stays the same.
>
> I'm including Linux USB folks as well because USB4 is officially coming
> from USB-IF which puts us on same boat now.
>
> While I changed the user visible Kconfig string to mention "USB4" (the
> Kconfig option is still CONFIG_THUNDERBOLT), I'm wondering whether we
> should move the whole Thunderbolt driver under drivers/usb/thunderbolt as
> well?
Looks "interesting", nice work!
I stopped at patch "Add initial support for USB4" as I don't think we
want to add USB4 code to a system that we know does not have it, right?
Everything up to then is just "normal" thunderbolt, and with the
exception of a few minor comments, all look fine to me.
I didn't actually read the USB4 patch just yet, as I figured we needed
to argue about that some more :)
thanks,
greg k-h
On Tue, Oct 01, 2019 at 02:38:08PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 01, 2019 at 02:38:14PM +0300, Mika Westerberg wrote:
> > Lane bonding allows aggregating the two 10/20 Gb/s (depending on the
> > generation) lanes into a single 20/40 Gb/s bonded link. This allows
> > sharing the full bandwidth more efficiently. In order to establish lane
> > bonding we need to check that the lane bonding is possible through LC
> > and that both end of the link actually supports 2x widths. This also
> > means that all the paths should be established through the primary port
> > so update tb_path_alloc() to handle this as well.
> >
> > Lane bonding is supported starting from Falcon Ridge (2nd generation)
> > controllers.
>
> Are we only going to be allowed to "bond" two links together? Or will
> we be doing more than 2 in the future? If more, then we might want to
> think of a different way to specify these...
AFAICT only two lanes are available in USB4. This goes over USB type-C
using the two lanes there.
Of course I don't know if in future there will be USB4 1.1 or something
that adds more lanes so if you think there is a better way to specify
these, I'm happy to implement that instead :)
> Anyway, one tiny nit below:
>
> >
> > Signed-off-by: Mika Westerberg <[email protected]>
> > ---
> > .../ABI/testing/sysfs-bus-thunderbolt | 17 ++
> > drivers/thunderbolt/icm.c | 18 +-
> > drivers/thunderbolt/lc.c | 28 ++
> > drivers/thunderbolt/path.c | 30 +-
> > drivers/thunderbolt/switch.c | 274 ++++++++++++++++++
> > drivers/thunderbolt/tb.c | 21 ++
> > drivers/thunderbolt/tb.h | 10 +
> > drivers/thunderbolt/tb_msgs.h | 2 +
> > drivers/thunderbolt/tb_regs.h | 20 ++
> > drivers/thunderbolt/tunnel.c | 19 +-
> > 10 files changed, 429 insertions(+), 10 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt
> > index b21fba14689b..2c9166f6fa97 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-thunderbolt
> > +++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt
> > @@ -104,6 +104,23 @@ Contact: [email protected]
> > Description: This attribute contains name of this device extracted from
> > the device DROM.
> >
> > +What: /sys/bus/thunderbolt/devices/.../link_speed
> > +Date: Apr 2020
> > +KernelVersion: 5.6
> > +Contact: Mika Westerberg <[email protected]>
> > +Description: This attribute reports the current upstream link speed
> > + in Gb/s per lane. If there are two lanes they both are
> > + running at the same speed. Use link_width to determine
> > + whether the two lanes are bonded or not.
> > +
> > +What: /sys/bus/thunderbolt/devices/.../link_width
> > +Date: Apr 2020
> > +KernelVersion: 5.6
> > +Contact: Mika Westerberg <[email protected]>
> > +Description: This attribute reports the current upstream link width.
> > + It is 1 for single lane link (or two single lane links)
> > + and 2 for bonded dual lane link.
> > +
> > What: /sys/bus/thunderbolt/devices/.../vendor
> > Date: Sep 2017
> > KernelVersion: 4.13
> > diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
> > index 6550f68f92ce..9c9c6ea2b790 100644
> > --- a/drivers/thunderbolt/icm.c
> > +++ b/drivers/thunderbolt/icm.c
> > @@ -567,7 +567,8 @@ static struct tb_switch *add_switch(struct tb_switch *parent_sw, u64 route,
> > size_t ep_name_size, u8 connection_id,
> > u8 connection_key, u8 link, u8 depth,
> > enum tb_security_level security_level,
> > - bool authorized, bool boot)
> > + bool authorized, bool boot, bool dual_lane,
> > + bool speed_gen3)
>
> That's just a crazy amount of function parameters, with no way of
> remembering what is what, especially when you add 2 more booleans at the
> end.
>
> It's your code, but ugh, that's going to be hard to maintain over time
> :(
Good point. I'll see if I can simplify it in the next version.
On Tue, Oct 01, 2019 at 02:40:54PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 01, 2019 at 02:38:16PM +0300, Mika Westerberg wrote:
> > In order to keep PCIe hierarchies consistent across hotplugs, add
> > hard-coded PCIe downstream port to Thunderbolt port for Alpine Ridge and
> > Titan Ridge as well.
>
> Oh, that makes me nervous, how could a hard-coded pcie port ever get set
> up incorrectly :)
>
> How do we "know" that this is correct? Is there any ACPI guarantees
> that this "always will be so"? If not, we all know someone will mess
> this up...
For Alpine Ridge and Titan Ridge the PCIe ports are always the same.
Basically what this is about is that you have up to two Thunderbolt
ports (type-C ports). When you plug in Thunderbolt device and PCIe gets
tunneled, the PCIe hierarchy always is always extended from the same
PCIe downstream port.
If we don't do this then the PCIe device may be changing its address
each plug/unplug. Also for older generations (that code is already in
mainline) there are PCIe downstream ports that do not have enough
resources for additional devices.
> > Signed-off-by: Mika Westerberg <[email protected]>
> > ---
> > drivers/thunderbolt/tb.c | 4 +++-
> > drivers/thunderbolt/tb.h | 25 +++++++++++++++++++++++++
> > 2 files changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> > index dbbe9afb9fb7..704455a4f763 100644
> > --- a/drivers/thunderbolt/tb.c
> > +++ b/drivers/thunderbolt/tb.c
> > @@ -344,10 +344,12 @@ static struct tb_port *tb_find_pcie_down(struct tb_switch *sw,
> > * Hard-coded Thunderbolt port to PCIe down port mapping
> > * per controller.
> > */
> > - if (tb_switch_is_cr(sw))
> > + if (tb_switch_is_cr(sw) || tb_switch_is_ar(sw))
> > index = !phy_port ? 6 : 7;
> > else if (tb_switch_is_fr(sw))
> > index = !phy_port ? 6 : 8;
> > + else if (tb_switch_is_tr(sw))
> > + index = !phy_port ? 8 : 9;
> > else
> > goto out;
> >
> > diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> > index e641dcebd50a..dbab06551eaa 100644
> > --- a/drivers/thunderbolt/tb.h
> > +++ b/drivers/thunderbolt/tb.h
> > @@ -632,6 +632,31 @@ static inline bool tb_switch_is_fr(const struct tb_switch *sw)
> > }
> > }
> >
> > +static inline bool tb_switch_is_ar(const struct tb_switch *sw)
>
> "ar"? Can you spell it out?
You mean call this tb_switch_is_alpine_ridge()? Sure, I will then do
the same for tb_switch_is_fr() and the existing ones.
>
> > +{
> > + switch (sw->config.device_id) {
> > + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_BRIDGE:
> > + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_BRIDGE:
> > + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_BRIDGE:
> > + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_BRIDGE:
> > + return true;
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +static inline bool tb_switch_is_tr(const struct tb_switch *sw)
>
> Same for "tr" please.
Sure.
On Tue, Oct 01, 2019 at 02:49:54PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 01, 2019 at 02:38:08PM +0300, Mika Westerberg wrote:
> > Hi all,
> >
> > I'm sending this as RFC because the series is still missing important
> > features such as power management so not ready for merging. However, I
> > think it is good to get any early feedback from the community. We are
> > working to add support for the missing features.
> >
> > USB4 is the public specification of Thunderbolt 3 protocol and can be
> > downloaded here:
> >
> > https://www.usb.org/sites/default/files/USB4%20Specification_1.zip
> >
> > USB4 is about tunneling different protocols over a single cable (in the
> > same way as Thunderbolt). The current USB4 spec supports PCIe, Display Port
> > and USB 3.x, and also software based protocols such as networking between
> > domains (hosts).
> >
> > So far PCs have been using firmware based Connection Manager and Apple
> > systems have been using software based one. A Connection Manager is the
> > entity that handles creation of different tunnel types through the USB4
> > (and Thunderbolt) fabric. With USB4 the plan is to have software based
> > Connection Manager everywhere but some early systems will also support
> > firmware to allow OS downgrade for example.
> >
> > Current Linux Thunderbolt driver supports both "modes" and can detect which
> > one to use dynamically.
> >
> > This series first adds support for Thunderbolt 3 devices to the software
> > connection manager and then extends that to support USB4 compliant hosts
> > and devices (this applies to both firmware and software based connection
> > managers). With this series the following features are supported also for
> > USB4 compliant devices:
> >
> > - PCIe tunneling
> > - Display Port tunneling
> > - USB 3.x tunneling
> > - P2P networking (implemented in drivers/net/thunderbolt.c)
> > - Host and device NVM firmware upgrade
> >
> > We also add two new sysfs attributes under each device that expose link
> > speed and width to userspace. The rest of the userspace ABI stays the same.
> >
> > I'm including Linux USB folks as well because USB4 is officially coming
> > from USB-IF which puts us on same boat now.
> >
> > While I changed the user visible Kconfig string to mention "USB4" (the
> > Kconfig option is still CONFIG_THUNDERBOLT), I'm wondering whether we
> > should move the whole Thunderbolt driver under drivers/usb/thunderbolt as
> > well?
>
> Looks "interesting", nice work!
Thanks!
> I stopped at patch "Add initial support for USB4" as I don't think we
> want to add USB4 code to a system that we know does not have it, right?
You can connect a USB4 device to Thunderbolt 3 system. USB4 hubs
specifically are required to support this by the spec. Of course then it
is in Thunderbolt 3 alternate mode (not in USB4 native mode) but the
USB4 register set is still there. So for example we still need to
configure TMU (Time Management Unit) and access the DROM via router
operations and so on.
Do you mean we don't want that?
> Everything up to then is just "normal" thunderbolt, and with the
> exception of a few minor comments, all look fine to me.
>
> I didn't actually read the USB4 patch just yet, as I figured we needed
> to argue about that some more :)
Heh, sure and thanks for the feedback so far :)
Am Dienstag, den 01.10.2019, 15:53 +0300 schrieb Mika Westerberg:
> >
> > Are we only going to be allowed to "bond" two links together? Or will
> > we be doing more than 2 in the future? If more, then we might want to
> > think of a different way to specify these...
>
> AFAICT only two lanes are available in USB4. This goes over USB type-C
> using the two lanes there.
>
> Of course I don't know if in future there will be USB4 1.1 or something
> that adds more lanes so if you think there is a better way to specify
> these, I'm happy to implement that instead :)
If this ever can become asymmetric this interface is going to turn
around and bite.
Regards
Oliver
On Wed, Oct 02, 2019 at 04:21:06PM +0200, Oliver Neukum wrote:
> Am Dienstag, den 01.10.2019, 15:53 +0300 schrieb Mika Westerberg:
> > >
> > > Are we only going to be allowed to "bond" two links together? Or will
> > > we be doing more than 2 in the future? If more, then we might want to
> > > think of a different way to specify these...
> >
> > AFAICT only two lanes are available in USB4. This goes over USB type-C
> > using the two lanes there.
> >
> > Of course I don't know if in future there will be USB4 1.1 or something
> > that adds more lanes so if you think there is a better way to specify
> > these, I'm happy to implement that instead :)
>
> If this ever can become asymmetric this interface is going to turn
> around and bite.
Don't think it can be asymmetric but I'm open to all ideas how to make
it more flexible :-)
Am Mittwoch, den 02.10.2019, 17:30 +0300 schrieb Mika Westerberg:
> On Wed, Oct 02, 2019 at 04:21:06PM +0200, Oliver Neukum wrote:
> > Am Dienstag, den 01.10.2019, 15:53 +0300 schrieb Mika Westerberg:
> > > >
> > > > Are we only going to be allowed to "bond" two links together? Or will
> > > > we be doing more than 2 in the future? If more, then we might want to
> > > > think of a different way to specify these...
> > >
> > > AFAICT only two lanes are available in USB4. This goes over USB type-C
> > > using the two lanes there.
> > >
> > > Of course I don't know if in future there will be USB4 1.1 or something
> > > that adds more lanes so if you think there is a better way to specify
> > > these, I'm happy to implement that instead :)
> >
> > If this ever can become asymmetric this interface is going to turn
> > around and bite.
>
> Don't think it can be asymmetric but I'm open to all ideas how to make
> it more flexible :-)
Split the the attributes into link_speed_rx and link_speed_tx. For
link_width likewise. We had the same issue with USB.
Regards
Oliver
On Thu, Oct 03, 2019 at 10:25:32AM +0200, Oliver Neukum wrote:
> Am Mittwoch, den 02.10.2019, 17:30 +0300 schrieb Mika Westerberg:
> > On Wed, Oct 02, 2019 at 04:21:06PM +0200, Oliver Neukum wrote:
> > > Am Dienstag, den 01.10.2019, 15:53 +0300 schrieb Mika Westerberg:
> > > > >
> > > > > Are we only going to be allowed to "bond" two links together? Or will
> > > > > we be doing more than 2 in the future? If more, then we might want to
> > > > > think of a different way to specify these...
> > > >
> > > > AFAICT only two lanes are available in USB4. This goes over USB type-C
> > > > using the two lanes there.
> > > >
> > > > Of course I don't know if in future there will be USB4 1.1 or something
> > > > that adds more lanes so if you think there is a better way to specify
> > > > these, I'm happy to implement that instead :)
> > >
> > > If this ever can become asymmetric this interface is going to turn
> > > around and bite.
> >
> > Don't think it can be asymmetric but I'm open to all ideas how to make
> > it more flexible :-)
>
> Split the the attributes into link_speed_rx and link_speed_tx. For
> link_width likewise. We had the same issue with USB.
OK, thanks for the suggestion.