* [PATCH 0/3] thunderbolt: Expose details about tunneling @ 2021-03-09 13:48 Mika Westerberg 2021-03-09 13:48 ` [PATCH 1/3] thunderbolt: Add details to router uevent Mika Westerberg ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Mika Westerberg @ 2021-03-09 13:48 UTC (permalink / raw) To: linux-usb Cc: Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas, Greg Kroah-Hartman, Mika Westerberg Hi all, There has been ask if we can expose more details about the connected devices and the tunneling to userspace, so it can then provide more detailed information to the user. This series is not meant to be merged just yet but more like start of a discussion if the information here is enough or we perhaps want to do something differently. First we add uevent details for each device (USB4 router) that adds USB4_TYPE=host|device|hub and USB4_VERSION=1.0 (if the device actually is USB4). The host|device|hub definitions follow the USB4 spec. Then for each device router we expose two new attributes: "usb3" and "dp" that if present mean that the device has corresponding adapter (USB 3.x upstream adapter and DP OUT adapter). The contents of the attributes then hold number of tunnels ending to this router. So if USB 3.x is tunneled "usb3" reads 1. Since there can be multiple DP OUT adaptes the "dp" attribute holds number of DP tunnels ending to this router. For PCIe tunneling the "authorized" attribute works the same way. I'm including the folks who have been working on the userspace side of Thunderbolt in hope to get some feedback whether this is useful approach or perhaps there are better ideas. Thanks! Mika Westerberg (3): thunderbolt: Add details to router uevent thunderbolt: Hide authorized attribute if router does not support PCIe tunnels thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels .../ABI/testing/sysfs-bus-thunderbolt | 26 ++++++ drivers/thunderbolt/domain.c | 10 +++ drivers/thunderbolt/switch.c | 90 ++++++++++++++++++- drivers/thunderbolt/tb.c | 49 +++++++--- drivers/thunderbolt/tb.h | 4 + include/linux/thunderbolt.h | 6 ++ 6 files changed, 173 insertions(+), 12 deletions(-) -- 2.30.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] thunderbolt: Add details to router uevent 2021-03-09 13:48 [PATCH 0/3] thunderbolt: Expose details about tunneling Mika Westerberg @ 2021-03-09 13:48 ` Mika Westerberg 2021-03-09 14:10 ` Greg Kroah-Hartman 2021-03-10 17:28 ` Limonciello, Mario 2021-03-09 13:48 ` [PATCH 2/3] thunderbolt: Hide authorized attribute if router does not support PCIe tunnels Mika Westerberg 2021-03-09 13:48 ` [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels Mika Westerberg 2 siblings, 2 replies; 18+ messages in thread From: Mika Westerberg @ 2021-03-09 13:48 UTC (permalink / raw) To: linux-usb Cc: Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas, Greg Kroah-Hartman, Mika Westerberg Expose two environment variables for routers as part of the initial uevent: USB4_VERSION=1.0 USB4_TYPE=host|device|hub Userspace can use this information to expose more details about each connected device. Only USB4 devices have USB4_VERSION but all devices have USB4_TYPE. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/thunderbolt/switch.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index 72b43c7c0651..a82032c081e8 100644 --- a/drivers/thunderbolt/switch.c +++ b/drivers/thunderbolt/switch.c @@ -1827,6 +1827,39 @@ static void tb_switch_release(struct device *dev) kfree(sw); } +static int tb_switch_uevent(struct device *dev, struct kobj_uevent_env *env) +{ + struct tb_switch *sw = tb_to_switch(dev); + const char *type; + + if (sw->config.thunderbolt_version == USB4_VERSION_1_0) { + if (add_uevent_var(env, "USB4_VERSION=1.0")) + return -ENOMEM; + } + + if (!tb_route(sw)) { + type = "host"; + } else { + const struct tb_port *port; + bool hub = false; + + /* Device is hub if it has any downstream ports */ + tb_switch_for_each_port(sw, port) { + if (!port->disabled && !tb_is_upstream_port(port) && + tb_port_is_null(port)) { + hub = true; + break; + } + } + + type = hub ? "hub" : "device"; + } + + if (add_uevent_var(env, "USB4_TYPE=%s", type)) + return -ENOMEM; + return 0; +} + /* * Currently only need to provide the callbacks. Everything else is handled * in the connection manager. @@ -1860,6 +1893,7 @@ static const struct dev_pm_ops tb_switch_pm_ops = { struct device_type tb_switch_type = { .name = "thunderbolt_device", .release = tb_switch_release, + .uevent = tb_switch_uevent, .pm = &tb_switch_pm_ops, }; -- 2.30.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] thunderbolt: Add details to router uevent 2021-03-09 13:48 ` [PATCH 1/3] thunderbolt: Add details to router uevent Mika Westerberg @ 2021-03-09 14:10 ` Greg Kroah-Hartman 2021-03-10 17:28 ` Limonciello, Mario 1 sibling, 0 replies; 18+ messages in thread From: Greg Kroah-Hartman @ 2021-03-09 14:10 UTC (permalink / raw) To: Mika Westerberg Cc: linux-usb, Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas On Tue, Mar 09, 2021 at 04:48:16PM +0300, Mika Westerberg wrote: > Expose two environment variables for routers as part of the initial > uevent: > > USB4_VERSION=1.0 > USB4_TYPE=host|device|hub > > Userspace can use this information to expose more details about each > connected device. Only USB4 devices have USB4_VERSION but all devices > have USB4_TYPE. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 1/3] thunderbolt: Add details to router uevent 2021-03-09 13:48 ` [PATCH 1/3] thunderbolt: Add details to router uevent Mika Westerberg 2021-03-09 14:10 ` Greg Kroah-Hartman @ 2021-03-10 17:28 ` Limonciello, Mario 2021-03-11 7:59 ` Mika Westerberg 1 sibling, 1 reply; 18+ messages in thread From: Limonciello, Mario @ 2021-03-10 17:28 UTC (permalink / raw) To: Mika Westerberg, linux-usb Cc: Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas, Greg Kroah-Hartman > -----Original Message----- > From: Mika Westerberg <mika.westerberg@linux.intel.com> > Sent: Tuesday, March 9, 2021 7:48 > To: linux-usb@vger.kernel.org > Cc: Michael Jamet; Yehezkel Bernat; Andreas Noever; Lukas Wunner; Limonciello, > Mario; Christian Kellner; Benson Leung; Prashant Malani; Diego Rivas; Greg > Kroah-Hartman; Mika Westerberg > Subject: [PATCH 1/3] thunderbolt: Add details to router uevent > > > [EXTERNAL EMAIL] > > Expose two environment variables for routers as part of the initial > uevent: > > USB4_VERSION=1.0 > USB4_TYPE=host|device|hub Presumably this will then show up in the uevent like this for a host controller: DEVTYPE=thunderbolt_device USB4_VERSION=1.0 USB4_TYPE=host Since it's specifically for USB4, how about if you instead have new devtypes? TBT3: DEVTYPE=thunderbolt_device USB4: DEVTYPE=usb4_host|usb4_device|usb4_hub That would at least make it clearer to userspace to make a delineation if it's legacy device or not. I don't know if that's actually valuable information however. > > Userspace can use this information to expose more details about each > connected device. Only USB4 devices have USB4_VERSION but all devices > have USB4_TYPE. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/thunderbolt/switch.c | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c > index 72b43c7c0651..a82032c081e8 100644 > --- a/drivers/thunderbolt/switch.c > +++ b/drivers/thunderbolt/switch.c > @@ -1827,6 +1827,39 @@ static void tb_switch_release(struct device *dev) > kfree(sw); > } > > +static int tb_switch_uevent(struct device *dev, struct kobj_uevent_env *env) > +{ > + struct tb_switch *sw = tb_to_switch(dev); > + const char *type; > + > + if (sw->config.thunderbolt_version == USB4_VERSION_1_0) { > + if (add_uevent_var(env, "USB4_VERSION=1.0")) > + return -ENOMEM; > + } > + > + if (!tb_route(sw)) { > + type = "host"; > + } else { > + const struct tb_port *port; > + bool hub = false; > + > + /* Device is hub if it has any downstream ports */ > + tb_switch_for_each_port(sw, port) { > + if (!port->disabled && !tb_is_upstream_port(port) && > + tb_port_is_null(port)) { > + hub = true; > + break; > + } > + } > + > + type = hub ? "hub" : "device"; > + } > + > + if (add_uevent_var(env, "USB4_TYPE=%s", type)) > + return -ENOMEM; > + return 0; > +} > + > /* > * Currently only need to provide the callbacks. Everything else is handled > * in the connection manager. > @@ -1860,6 +1893,7 @@ static const struct dev_pm_ops tb_switch_pm_ops = { > struct device_type tb_switch_type = { > .name = "thunderbolt_device", > .release = tb_switch_release, > + .uevent = tb_switch_uevent, > .pm = &tb_switch_pm_ops, > }; > > -- > 2.30.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] thunderbolt: Add details to router uevent 2021-03-10 17:28 ` Limonciello, Mario @ 2021-03-11 7:59 ` Mika Westerberg 0 siblings, 0 replies; 18+ messages in thread From: Mika Westerberg @ 2021-03-11 7:59 UTC (permalink / raw) To: Limonciello, Mario Cc: linux-usb, Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas, Greg Kroah-Hartman Hi Mario, On Wed, Mar 10, 2021 at 05:28:19PM +0000, Limonciello, Mario wrote: > > > > -----Original Message----- > > From: Mika Westerberg <mika.westerberg@linux.intel.com> > > Sent: Tuesday, March 9, 2021 7:48 > > To: linux-usb@vger.kernel.org > > Cc: Michael Jamet; Yehezkel Bernat; Andreas Noever; Lukas Wunner; Limonciello, > > Mario; Christian Kellner; Benson Leung; Prashant Malani; Diego Rivas; Greg > > Kroah-Hartman; Mika Westerberg > > Subject: [PATCH 1/3] thunderbolt: Add details to router uevent > > > > > > [EXTERNAL EMAIL] > > > > Expose two environment variables for routers as part of the initial > > uevent: > > > > USB4_VERSION=1.0 > > USB4_TYPE=host|device|hub > > Presumably this will then show up in the uevent like this for a host controller: > DEVTYPE=thunderbolt_device > USB4_VERSION=1.0 > USB4_TYPE=host > > Since it's specifically for USB4, how about if you instead have new devtypes? > TBT3: > DEVTYPE=thunderbolt_device > > USB4: > DEVTYPE=usb4_host|usb4_device|usb4_hub > > That would at least make it clearer to userspace to make a delineation if it's > legacy device or not. I don't know if that's actually valuable information however. Unfortunately we can't do that. DEVTYPE is generated by the driver core based on the struct device_type we register for routers (switches). Also bolt, and I think fwupd too, already use DEVTYPE to distinguish routers from other devices (like XDomains etc). ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/3] thunderbolt: Hide authorized attribute if router does not support PCIe tunnels 2021-03-09 13:48 [PATCH 0/3] thunderbolt: Expose details about tunneling Mika Westerberg 2021-03-09 13:48 ` [PATCH 1/3] thunderbolt: Add details to router uevent Mika Westerberg @ 2021-03-09 13:48 ` Mika Westerberg 2021-03-09 14:10 ` Greg Kroah-Hartman 2021-03-09 13:48 ` [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels Mika Westerberg 2 siblings, 1 reply; 18+ messages in thread From: Mika Westerberg @ 2021-03-09 13:48 UTC (permalink / raw) To: linux-usb Cc: Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas, Greg Kroah-Hartman, Mika Westerberg With USB4 devices PCIe tunneling is optional so for device routers without PCIe upstream adapter it does not make much sense to expose the authorized attribute. For this reason hide it if PCIe tunneling is not supported by the device router. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/thunderbolt/switch.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index a82032c081e8..e73cd296db7e 100644 --- a/drivers/thunderbolt/switch.c +++ b/drivers/thunderbolt/switch.c @@ -1740,6 +1740,18 @@ static struct attribute *switch_attrs[] = { NULL, }; +static bool has_port(const struct tb_switch *sw, enum tb_port_type type) +{ + const struct tb_port *port; + + tb_switch_for_each_port(sw, port) { + if (!port->disabled && port->config.type == type) + return true; + } + + return false; +} + static umode_t switch_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n) { @@ -1748,7 +1760,8 @@ static umode_t switch_attr_is_visible(struct kobject *kobj, if (attr == &dev_attr_authorized.attr) { if (sw->tb->security_level == TB_SECURITY_NOPCIE || - sw->tb->security_level == TB_SECURITY_DPONLY) + sw->tb->security_level == TB_SECURITY_DPONLY || + !has_port(sw, TB_TYPE_PCIE_UP)) return 0; } else if (attr == &dev_attr_device.attr) { if (!sw->device) -- 2.30.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] thunderbolt: Hide authorized attribute if router does not support PCIe tunnels 2021-03-09 13:48 ` [PATCH 2/3] thunderbolt: Hide authorized attribute if router does not support PCIe tunnels Mika Westerberg @ 2021-03-09 14:10 ` Greg Kroah-Hartman 0 siblings, 0 replies; 18+ messages in thread From: Greg Kroah-Hartman @ 2021-03-09 14:10 UTC (permalink / raw) To: Mika Westerberg Cc: linux-usb, Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas On Tue, Mar 09, 2021 at 04:48:17PM +0300, Mika Westerberg wrote: > With USB4 devices PCIe tunneling is optional so for device routers > without PCIe upstream adapter it does not make much sense to expose the > authorized attribute. For this reason hide it if PCIe tunneling is not > supported by the device router. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels 2021-03-09 13:48 [PATCH 0/3] thunderbolt: Expose details about tunneling Mika Westerberg 2021-03-09 13:48 ` [PATCH 1/3] thunderbolt: Add details to router uevent Mika Westerberg 2021-03-09 13:48 ` [PATCH 2/3] thunderbolt: Hide authorized attribute if router does not support PCIe tunnels Mika Westerberg @ 2021-03-09 13:48 ` Mika Westerberg 2021-03-09 14:09 ` Greg Kroah-Hartman 2021-03-17 10:19 ` Mika Westerberg 2 siblings, 2 replies; 18+ messages in thread From: Mika Westerberg @ 2021-03-09 13:48 UTC (permalink / raw) To: linux-usb Cc: Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas, Greg Kroah-Hartman, Mika Westerberg This exposes two new attributes under each device router: usb3 and dp that hold number of tunnels ending to this switch. These attributes are only available if the connection manager supports it (tunneling_details attribute reads 1). Currently only the software connection manager supports this. Based on these userspace can show the user more detailed information what is going on. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- .../ABI/testing/sysfs-bus-thunderbolt | 26 ++++++++++ drivers/thunderbolt/domain.c | 10 ++++ drivers/thunderbolt/switch.c | 41 ++++++++++++++++ drivers/thunderbolt/tb.c | 49 ++++++++++++++----- drivers/thunderbolt/tb.h | 4 ++ include/linux/thunderbolt.h | 6 +++ 6 files changed, 125 insertions(+), 11 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt index c41c68f64693..1569be391ca6 100644 --- a/Documentation/ABI/testing/sysfs-bus-thunderbolt +++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt @@ -61,6 +61,14 @@ Description: This attribute holds current Thunderbolt security level the BIOS. ======= ================================================== +What: /sys/bus/thunderbolt/devices/.../domainX/tunneling_details +Date: July 2021 +KernelVersion: 5.13 +Contact: Mika Westerberg <mika.westerberg@linux.intel.com> +Description: The connection manager implementation may expose + additional details about tunneling. If it supports this + the attribute reads 1. + What: /sys/bus/thunderbolt/devices/.../authorized Date: Sep 2017 KernelVersion: 4.13 @@ -102,6 +110,15 @@ Contact: thunderbolt-software@lists.01.org Description: This attribute contains 1 if Thunderbolt device was already authorized on boot and 0 otherwise. +What: /sys/bus/thunderbolt/devices/.../dp +Date: Jul 2021 +KernelVersion: 5.13 +Contact: Mika Westerberg <mika.westerberg@linux.intel.com> +Description: Only available if the domain tunneling_details attribute + reads 1. If present means that the device router has + DisplayPort sink. Contents will be number how many + active DisplayPort tunnels end up to this router. + What: /sys/bus/thunderbolt/devices/.../generation Date: Jan 2020 KernelVersion: 5.5 @@ -169,6 +186,15 @@ Contact: Mika Westerberg <mika.westerberg@linux.intel.com> Description: This attribute reports number of TX lanes the device is using simultaneusly through its upstream port. +What: /sys/bus/thunderbolt/devices/.../usb3 +Date: Jul 2021 +KernelVersion: 5.13 +Contact: Mika Westerberg <mika.westerberg@linux.intel.com> +Description: Only available if the domain tunneling_details attribute + reads 1. If present means that the device router has + USB 3.x upstream adapter. Reads 1 if there is an active + USB 3.x tunnel to this router. + What: /sys/bus/thunderbolt/devices/.../vendor Date: Sep 2017 KernelVersion: 4.13 diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c index a7d83eec3d15..3a1fb7a39f90 100644 --- a/drivers/thunderbolt/domain.c +++ b/drivers/thunderbolt/domain.c @@ -282,11 +282,21 @@ static ssize_t security_show(struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR_RO(security); +static ssize_t tunneling_details_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + const struct tb *tb = container_of(dev, struct tb, dev); + + return sprintf(buf, "%d\n", !!(tb->cm_caps & TB_CAP_TUNNEL_DETAILS)); +} +static DEVICE_ATTR_RO(tunneling_details); + static struct attribute *domain_attrs[] = { &dev_attr_boot_acl.attr, &dev_attr_deauthorization.attr, &dev_attr_iommu_dma_protection.attr, &dev_attr_security.attr, + &dev_attr_tunneling_details.attr, NULL, }; diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index e73cd296db7e..b72589eabf0c 100644 --- a/drivers/thunderbolt/switch.c +++ b/drivers/thunderbolt/switch.c @@ -1487,6 +1487,21 @@ device_name_show(struct device *dev, struct device_attribute *attr, char *buf) } static DEVICE_ATTR_RO(device_name); +static ssize_t dp_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct tb_switch *sw = tb_to_switch(dev); + int ret; + + if (!mutex_trylock(&sw->tb->lock)) + return restart_syscall(); + ret = sprintf(buf, "%u\n", sw->dp); + mutex_unlock(&sw->tb->lock); + + return ret; +} +static DEVICE_ATTR_RO(dp); + static ssize_t generation_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -1693,6 +1708,21 @@ static ssize_t nvm_version_show(struct device *dev, } static DEVICE_ATTR_RO(nvm_version); +static ssize_t usb3_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct tb_switch *sw = tb_to_switch(dev); + int ret; + + if (!mutex_trylock(&sw->tb->lock)) + return restart_syscall(); + ret = sprintf(buf, "%u\n", sw->usb3); + mutex_unlock(&sw->tb->lock); + + return ret; +} +static DEVICE_ATTR_RO(usb3); + static ssize_t vendor_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -1725,6 +1755,7 @@ static struct attribute *switch_attrs[] = { &dev_attr_boot.attr, &dev_attr_device.attr, &dev_attr_device_name.attr, + &dev_attr_dp.attr, &dev_attr_generation.attr, &dev_attr_key.attr, &dev_attr_nvm_authenticate.attr, @@ -1734,6 +1765,7 @@ static struct attribute *switch_attrs[] = { &dev_attr_rx_lanes.attr, &dev_attr_tx_speed.attr, &dev_attr_tx_lanes.attr, + &dev_attr_usb3.attr, &dev_attr_vendor.attr, &dev_attr_vendor_name.attr, &dev_attr_unique_id.attr, @@ -1757,6 +1789,7 @@ static umode_t switch_attr_is_visible(struct kobject *kobj, { struct device *dev = kobj_to_dev(kobj); struct tb_switch *sw = tb_to_switch(dev); + const struct tb *tb = sw->tb; if (attr == &dev_attr_authorized.attr) { if (sw->tb->security_level == TB_SECURITY_NOPCIE || @@ -1769,6 +1802,10 @@ static umode_t switch_attr_is_visible(struct kobject *kobj, } else if (attr == &dev_attr_device_name.attr) { if (!sw->device_name) return 0; + } else if (attr == &dev_attr_dp.attr) { + if (!(tb->cm_caps & TB_CAP_TUNNEL_DETAILS) || + !has_port(sw, TB_TYPE_DP_HDMI_OUT)) + return 0; } else if (attr == &dev_attr_vendor.attr) { if (!sw->vendor) return 0; @@ -1788,6 +1825,10 @@ static umode_t switch_attr_is_visible(struct kobject *kobj, if (tb_route(sw)) return attr->mode; return 0; + } else if (attr == &dev_attr_usb3.attr) { + if (!(tb->cm_caps & TB_CAP_TUNNEL_DETAILS) || + !has_port(sw, TB_TYPE_USB3_UP)) + return 0; } else if (attr == &dev_attr_nvm_authenticate.attr) { if (nvm_upgradeable(sw)) return attr->mode; diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c index 7e6dc2b03bed..32b79dce134f 100644 --- a/drivers/thunderbolt/tb.c +++ b/drivers/thunderbolt/tb.c @@ -104,10 +104,39 @@ static void tb_remove_dp_resources(struct tb_switch *sw) } } +static void tb_add_tunnel(struct tb *tb, struct tb_tunnel *tunnel) +{ + struct tb_switch *sw = tunnel->dst_port->sw; + struct tb_cm *tcm = tb_priv(tb); + + if (tb_tunnel_is_usb3(tunnel)) { + sw->usb3++; + } else if (tb_tunnel_is_dp(tunnel)) { + sw->dp++; + /* Inform userspace about DP tunneling change */ + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); + } + + list_add_tail(&tunnel->list, &tcm->tunnel_list); +} + +static void tb_remove_tunnel(struct tb_tunnel *tunnel) +{ + struct tb_switch *sw = tunnel->dst_port->sw; + + if (tb_tunnel_is_usb3(tunnel) && sw->usb3) { + sw->usb3--; + } else if (tb_tunnel_is_dp(tunnel) && sw->dp) { + sw->dp--; + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); + } + + list_del(&tunnel->list); +} + static void tb_discover_tunnels(struct tb_switch *sw) { struct tb *tb = sw->tb; - struct tb_cm *tcm = tb_priv(tb); struct tb_port *port; tb_switch_for_each_port(sw, port) { @@ -146,7 +175,7 @@ static void tb_discover_tunnels(struct tb_switch *sw) pm_runtime_get_sync(&tunnel->dst_port->sw->dev); } - list_add_tail(&tunnel->list, &tcm->tunnel_list); + tb_add_tunnel(tb, tunnel); } tb_switch_for_each_port(sw, port) { @@ -440,7 +469,6 @@ static int tb_tunnel_usb3(struct tb *tb, struct tb_switch *sw) struct tb_switch *parent = tb_switch_parent(sw); int ret, available_up, available_down; struct tb_port *up, *down, *port; - struct tb_cm *tcm = tb_priv(tb); struct tb_tunnel *tunnel; if (!tb_acpi_may_tunnel_usb3()) { @@ -503,7 +531,7 @@ static int tb_tunnel_usb3(struct tb *tb, struct tb_switch *sw) goto err_free; } - list_add_tail(&tunnel->list, &tcm->tunnel_list); + tb_add_tunnel(tb, tunnel); if (tb_route(parent)) tb_reclaim_usb3_bandwidth(tb, down, up); @@ -686,7 +714,7 @@ static void tb_deactivate_and_free_tunnel(struct tb_tunnel *tunnel) return; tb_tunnel_deactivate(tunnel); - list_del(&tunnel->list); + tb_remove_tunnel(tunnel); tb = tunnel->tb; src_port = tunnel->src_port; @@ -937,7 +965,7 @@ static void tb_tunnel_dp(struct tb *tb) goto err_free; } - list_add_tail(&tunnel->list, &tcm->tunnel_list); + tb_add_tunnel(tb, tunnel); tb_reclaim_usb3_bandwidth(tb, in, out); return; @@ -1038,7 +1066,7 @@ static int tb_disconnect_pci(struct tb *tb, struct tb_switch *sw) return -ENODEV; tb_tunnel_deactivate(tunnel); - list_del(&tunnel->list); + tb_remove_tunnel(tunnel); tb_tunnel_free(tunnel); return 0; } @@ -1046,7 +1074,6 @@ static int tb_disconnect_pci(struct tb *tb, struct tb_switch *sw) static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw) { struct tb_port *up, *down, *port; - struct tb_cm *tcm = tb_priv(tb); struct tb_switch *parent_sw; struct tb_tunnel *tunnel; @@ -1075,7 +1102,7 @@ static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw) return -EIO; } - list_add_tail(&tunnel->list, &tcm->tunnel_list); + tb_add_tunnel(tb, tunnel); return 0; } @@ -1083,7 +1110,6 @@ static int tb_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd, int transmit_path, int transmit_ring, int receive_path, int receive_ring) { - struct tb_cm *tcm = tb_priv(tb); struct tb_port *nhi_port, *dst_port; struct tb_tunnel *tunnel; struct tb_switch *sw; @@ -1108,7 +1134,7 @@ static int tb_approve_xdomain_paths(struct tb *tb, struct tb_xdomain *xd, return -EIO; } - list_add_tail(&tunnel->list, &tcm->tunnel_list); + tb_add_tunnel(tb, tunnel); mutex_unlock(&tb->lock); return 0; } @@ -1586,6 +1612,7 @@ struct tb *tb_probe(struct tb_nhi *nhi) tb->security_level = TB_SECURITY_NOPCIE; tb->cm_ops = &tb_cm_ops; + tb->cm_caps |= TB_CAP_TUNNEL_DETAILS; tcm = tb_priv(tb); INIT_LIST_HEAD(&tcm->tunnel_list); diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h index 2af6d632e3d0..b4bc656a06b6 100644 --- a/drivers/thunderbolt/tb.h +++ b/drivers/thunderbolt/tb.h @@ -123,6 +123,8 @@ struct tb_switch_tmu { * @safe_mode: The switch is in safe-mode * @boot: Whether the switch was already authorized on boot or not * @rpm: The switch supports runtime PM + * @usb3: Number of USB 3.x tunnels to this switch (0 or 1) + * @dp: Number of DisplayPort tunnels ending to this switch * @authorized: Whether the switch is authorized by user or policy * @security_level: Switch supported security level * @debugfs_dir: Pointer to the debugfs structure @@ -167,6 +169,8 @@ struct tb_switch { bool safe_mode; bool boot; bool rpm; + unsigned int usb3; + unsigned int dp; unsigned int authorized; enum tb_security_level security_level; struct dentry *debugfs_dir; diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h index e7c96c37174f..dc6cfb4237d1 100644 --- a/include/linux/thunderbolt.h +++ b/include/linux/thunderbolt.h @@ -57,6 +57,9 @@ enum tb_security_level { TB_SECURITY_NOPCIE, }; +/* Connection manager exposes details about tunneling */ +#define TB_CAP_TUNNEL_DETAILS BIT(0) + /** * struct tb - main thunderbolt bus structure * @dev: Domain device @@ -67,6 +70,8 @@ enum tb_security_level { * @wq: Ordered workqueue for all domain specific work * @root_switch: Root switch of this domain * @cm_ops: Connection manager specific operations vector + * @cm_caps: Extra capabilities supported by the connection manager + * implementation * @index: Linux assigned domain number * @security_level: Current security level * @nboot_acl: Number of boot ACLs the domain supports @@ -80,6 +85,7 @@ struct tb { struct workqueue_struct *wq; struct tb_switch *root_switch; const struct tb_cm_ops *cm_ops; + unsigned int cm_caps; int index; enum tb_security_level security_level; size_t nboot_acl; -- 2.30.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels 2021-03-09 13:48 ` [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels Mika Westerberg @ 2021-03-09 14:09 ` Greg Kroah-Hartman 2021-03-10 7:54 ` Mika Westerberg 2021-03-17 10:19 ` Mika Westerberg 1 sibling, 1 reply; 18+ messages in thread From: Greg Kroah-Hartman @ 2021-03-09 14:09 UTC (permalink / raw) To: Mika Westerberg Cc: linux-usb, Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas On Tue, Mar 09, 2021 at 04:48:18PM +0300, Mika Westerberg wrote: > This exposes two new attributes under each device router: usb3 and dp > that hold number of tunnels ending to this switch. These attributes are > only available if the connection manager supports it (tunneling_details > attribute reads 1). Currently only the software connection manager > supports this. > > Based on these userspace can show the user more detailed information > what is going on. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > .../ABI/testing/sysfs-bus-thunderbolt | 26 ++++++++++ > drivers/thunderbolt/domain.c | 10 ++++ > drivers/thunderbolt/switch.c | 41 ++++++++++++++++ > drivers/thunderbolt/tb.c | 49 ++++++++++++++----- > drivers/thunderbolt/tb.h | 4 ++ > include/linux/thunderbolt.h | 6 +++ > 6 files changed, 125 insertions(+), 11 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt > index c41c68f64693..1569be391ca6 100644 > --- a/Documentation/ABI/testing/sysfs-bus-thunderbolt > +++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt > @@ -61,6 +61,14 @@ Description: This attribute holds current Thunderbolt security level > the BIOS. > ======= ================================================== > > +What: /sys/bus/thunderbolt/devices/.../domainX/tunneling_details > +Date: July 2021 > +KernelVersion: 5.13 > +Contact: Mika Westerberg <mika.westerberg@linux.intel.com> > +Description: The connection manager implementation may expose > + additional details about tunneling. If it supports this > + the attribute reads 1. > + > What: /sys/bus/thunderbolt/devices/.../authorized > Date: Sep 2017 > KernelVersion: 4.13 > @@ -102,6 +110,15 @@ Contact: thunderbolt-software@lists.01.org > Description: This attribute contains 1 if Thunderbolt device was already > authorized on boot and 0 otherwise. > > +What: /sys/bus/thunderbolt/devices/.../dp > +Date: Jul 2021 > +KernelVersion: 5.13 > +Contact: Mika Westerberg <mika.westerberg@linux.intel.com> > +Description: Only available if the domain tunneling_details attribute > + reads 1. If present means that the device router has > + DisplayPort sink. Contents will be number how many > + active DisplayPort tunnels end up to this router. > + > What: /sys/bus/thunderbolt/devices/.../generation > Date: Jan 2020 > KernelVersion: 5.5 > @@ -169,6 +186,15 @@ Contact: Mika Westerberg <mika.westerberg@linux.intel.com> > Description: This attribute reports number of TX lanes the device is > using simultaneusly through its upstream port. > > +What: /sys/bus/thunderbolt/devices/.../usb3 > +Date: Jul 2021 > +KernelVersion: 5.13 > +Contact: Mika Westerberg <mika.westerberg@linux.intel.com> > +Description: Only available if the domain tunneling_details attribute > + reads 1. If present means that the device router has > + USB 3.x upstream adapter. Reads 1 if there is an active > + USB 3.x tunnel to this router. > + > What: /sys/bus/thunderbolt/devices/.../vendor > Date: Sep 2017 > KernelVersion: 4.13 > diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c > index a7d83eec3d15..3a1fb7a39f90 100644 > --- a/drivers/thunderbolt/domain.c > +++ b/drivers/thunderbolt/domain.c > @@ -282,11 +282,21 @@ static ssize_t security_show(struct device *dev, struct device_attribute *attr, > } > static DEVICE_ATTR_RO(security); > > +static ssize_t tunneling_details_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + const struct tb *tb = container_of(dev, struct tb, dev); > + > + return sprintf(buf, "%d\n", !!(tb->cm_caps & TB_CAP_TUNNEL_DETAILS)); sysfs_emit() please. For all of these attributes. > +} > +static DEVICE_ATTR_RO(tunneling_details); > + > static struct attribute *domain_attrs[] = { > &dev_attr_boot_acl.attr, > &dev_attr_deauthorization.attr, > &dev_attr_iommu_dma_protection.attr, > &dev_attr_security.attr, > + &dev_attr_tunneling_details.attr, > NULL, > }; > > diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c > index e73cd296db7e..b72589eabf0c 100644 > --- a/drivers/thunderbolt/switch.c > +++ b/drivers/thunderbolt/switch.c > @@ -1487,6 +1487,21 @@ device_name_show(struct device *dev, struct device_attribute *attr, char *buf) > } > static DEVICE_ATTR_RO(device_name); > > +static ssize_t dp_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct tb_switch *sw = tb_to_switch(dev); > + int ret; > + > + if (!mutex_trylock(&sw->tb->lock)) > + return restart_syscall(); Huh? Why? > + ret = sprintf(buf, "%u\n", sw->dp); > + mutex_unlock(&sw->tb->lock); Why is the lock needed? It's an attribute, can it change quickly? Can it change at all? What happens if it changes _right_ after you read the value? What happens? I don't think you need locking here, do you? > + > + return ret; > +} > +static DEVICE_ATTR_RO(dp); > + > static ssize_t > generation_show(struct device *dev, struct device_attribute *attr, char *buf) > { > @@ -1693,6 +1708,21 @@ static ssize_t nvm_version_show(struct device *dev, > } > static DEVICE_ATTR_RO(nvm_version); > > +static ssize_t usb3_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct tb_switch *sw = tb_to_switch(dev); > + int ret; > + > + if (!mutex_trylock(&sw->tb->lock)) > + return restart_syscall(); Same here for locking comment. > + ret = sprintf(buf, "%u\n", sw->usb3); > + mutex_unlock(&sw->tb->lock); > + > + return ret; > +} > +static DEVICE_ATTR_RO(usb3); > + > static ssize_t vendor_show(struct device *dev, struct device_attribute *attr, > char *buf) > { > @@ -1725,6 +1755,7 @@ static struct attribute *switch_attrs[] = { > &dev_attr_boot.attr, > &dev_attr_device.attr, > &dev_attr_device_name.attr, > + &dev_attr_dp.attr, > &dev_attr_generation.attr, > &dev_attr_key.attr, > &dev_attr_nvm_authenticate.attr, > @@ -1734,6 +1765,7 @@ static struct attribute *switch_attrs[] = { > &dev_attr_rx_lanes.attr, > &dev_attr_tx_speed.attr, > &dev_attr_tx_lanes.attr, > + &dev_attr_usb3.attr, > &dev_attr_vendor.attr, > &dev_attr_vendor_name.attr, > &dev_attr_unique_id.attr, > @@ -1757,6 +1789,7 @@ static umode_t switch_attr_is_visible(struct kobject *kobj, > { > struct device *dev = kobj_to_dev(kobj); > struct tb_switch *sw = tb_to_switch(dev); > + const struct tb *tb = sw->tb; > > if (attr == &dev_attr_authorized.attr) { > if (sw->tb->security_level == TB_SECURITY_NOPCIE || > @@ -1769,6 +1802,10 @@ static umode_t switch_attr_is_visible(struct kobject *kobj, > } else if (attr == &dev_attr_device_name.attr) { > if (!sw->device_name) > return 0; > + } else if (attr == &dev_attr_dp.attr) { > + if (!(tb->cm_caps & TB_CAP_TUNNEL_DETAILS) || > + !has_port(sw, TB_TYPE_DP_HDMI_OUT)) > + return 0; > } else if (attr == &dev_attr_vendor.attr) { > if (!sw->vendor) > return 0; > @@ -1788,6 +1825,10 @@ static umode_t switch_attr_is_visible(struct kobject *kobj, > if (tb_route(sw)) > return attr->mode; > return 0; > + } else if (attr == &dev_attr_usb3.attr) { > + if (!(tb->cm_caps & TB_CAP_TUNNEL_DETAILS) || > + !has_port(sw, TB_TYPE_USB3_UP)) > + return 0; > } else if (attr == &dev_attr_nvm_authenticate.attr) { > if (nvm_upgradeable(sw)) > return attr->mode; > diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c > index 7e6dc2b03bed..32b79dce134f 100644 > --- a/drivers/thunderbolt/tb.c > +++ b/drivers/thunderbolt/tb.c > @@ -104,10 +104,39 @@ static void tb_remove_dp_resources(struct tb_switch *sw) > } > } > > +static void tb_add_tunnel(struct tb *tb, struct tb_tunnel *tunnel) > +{ > + struct tb_switch *sw = tunnel->dst_port->sw; > + struct tb_cm *tcm = tb_priv(tb); > + > + if (tb_tunnel_is_usb3(tunnel)) { > + sw->usb3++; > + } else if (tb_tunnel_is_dp(tunnel)) { > + sw->dp++; > + /* Inform userspace about DP tunneling change */ > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); What really "changed" here about this device that userspace now needs to know about it? > + } > + > + list_add_tail(&tunnel->list, &tcm->tunnel_list); > +} > + > +static void tb_remove_tunnel(struct tb_tunnel *tunnel) > +{ > + struct tb_switch *sw = tunnel->dst_port->sw; > + > + if (tb_tunnel_is_usb3(tunnel) && sw->usb3) { > + sw->usb3--; > + } else if (tb_tunnel_is_dp(tunnel) && sw->dp) { > + sw->dp--; > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); Same here, what causes tunnels to be added or removed? thanks, greg k-h ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels 2021-03-09 14:09 ` Greg Kroah-Hartman @ 2021-03-10 7:54 ` Mika Westerberg 2021-03-10 8:53 ` Greg Kroah-Hartman 0 siblings, 1 reply; 18+ messages in thread From: Mika Westerberg @ 2021-03-10 7:54 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-usb, Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas Hi Greg, On Tue, Mar 09, 2021 at 03:09:59PM +0100, Greg Kroah-Hartman wrote: > On Tue, Mar 09, 2021 at 04:48:18PM +0300, Mika Westerberg wrote: > > This exposes two new attributes under each device router: usb3 and dp > > that hold number of tunnels ending to this switch. These attributes are > > only available if the connection manager supports it (tunneling_details > > attribute reads 1). Currently only the software connection manager > > supports this. > > > > Based on these userspace can show the user more detailed information > > what is going on. > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > --- > > .../ABI/testing/sysfs-bus-thunderbolt | 26 ++++++++++ > > drivers/thunderbolt/domain.c | 10 ++++ > > drivers/thunderbolt/switch.c | 41 ++++++++++++++++ > > drivers/thunderbolt/tb.c | 49 ++++++++++++++----- > > drivers/thunderbolt/tb.h | 4 ++ > > include/linux/thunderbolt.h | 6 +++ > > 6 files changed, 125 insertions(+), 11 deletions(-) > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt > > index c41c68f64693..1569be391ca6 100644 > > --- a/Documentation/ABI/testing/sysfs-bus-thunderbolt > > +++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt > > @@ -61,6 +61,14 @@ Description: This attribute holds current Thunderbolt security level > > the BIOS. > > ======= ================================================== > > > > +What: /sys/bus/thunderbolt/devices/.../domainX/tunneling_details > > +Date: July 2021 > > +KernelVersion: 5.13 > > +Contact: Mika Westerberg <mika.westerberg@linux.intel.com> > > +Description: The connection manager implementation may expose > > + additional details about tunneling. If it supports this > > + the attribute reads 1. > > + > > What: /sys/bus/thunderbolt/devices/.../authorized > > Date: Sep 2017 > > KernelVersion: 4.13 > > @@ -102,6 +110,15 @@ Contact: thunderbolt-software@lists.01.org > > Description: This attribute contains 1 if Thunderbolt device was already > > authorized on boot and 0 otherwise. > > > > +What: /sys/bus/thunderbolt/devices/.../dp > > +Date: Jul 2021 > > +KernelVersion: 5.13 > > +Contact: Mika Westerberg <mika.westerberg@linux.intel.com> > > +Description: Only available if the domain tunneling_details attribute > > + reads 1. If present means that the device router has > > + DisplayPort sink. Contents will be number how many > > + active DisplayPort tunnels end up to this router. > > + > > What: /sys/bus/thunderbolt/devices/.../generation > > Date: Jan 2020 > > KernelVersion: 5.5 > > @@ -169,6 +186,15 @@ Contact: Mika Westerberg <mika.westerberg@linux.intel.com> > > Description: This attribute reports number of TX lanes the device is > > using simultaneusly through its upstream port. > > > > +What: /sys/bus/thunderbolt/devices/.../usb3 > > +Date: Jul 2021 > > +KernelVersion: 5.13 > > +Contact: Mika Westerberg <mika.westerberg@linux.intel.com> > > +Description: Only available if the domain tunneling_details attribute > > + reads 1. If present means that the device router has > > + USB 3.x upstream adapter. Reads 1 if there is an active > > + USB 3.x tunnel to this router. > > + > > What: /sys/bus/thunderbolt/devices/.../vendor > > Date: Sep 2017 > > KernelVersion: 4.13 > > diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c > > index a7d83eec3d15..3a1fb7a39f90 100644 > > --- a/drivers/thunderbolt/domain.c > > +++ b/drivers/thunderbolt/domain.c > > @@ -282,11 +282,21 @@ static ssize_t security_show(struct device *dev, struct device_attribute *attr, > > } > > static DEVICE_ATTR_RO(security); > > > > +static ssize_t tunneling_details_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + const struct tb *tb = container_of(dev, struct tb, dev); > > + > > + return sprintf(buf, "%d\n", !!(tb->cm_caps & TB_CAP_TUNNEL_DETAILS)); > > sysfs_emit() please. For all of these attributes. Sure. > > +} > > +static DEVICE_ATTR_RO(tunneling_details); > > + > > static struct attribute *domain_attrs[] = { > > &dev_attr_boot_acl.attr, > > &dev_attr_deauthorization.attr, > > &dev_attr_iommu_dma_protection.attr, > > &dev_attr_security.attr, > > + &dev_attr_tunneling_details.attr, > > NULL, > > }; > > > > diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c > > index e73cd296db7e..b72589eabf0c 100644 > > --- a/drivers/thunderbolt/switch.c > > +++ b/drivers/thunderbolt/switch.c > > @@ -1487,6 +1487,21 @@ device_name_show(struct device *dev, struct device_attribute *attr, char *buf) > > } > > static DEVICE_ATTR_RO(device_name); > > > > +static ssize_t dp_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + struct tb_switch *sw = tb_to_switch(dev); > > + int ret; > > + > > + if (!mutex_trylock(&sw->tb->lock)) > > + return restart_syscall(); > > Huh? Why? This was copied from another attribute but not needed here as you point out. > > + ret = sprintf(buf, "%u\n", sw->dp); > > + mutex_unlock(&sw->tb->lock); > > Why is the lock needed? It's an attribute, can it change quickly? Can > it change at all? What happens if it changes _right_ after you read the > value? What happens? This particular attribute can change dynamically as DisplayPort tunnels are created/torn down based on user action plugging in monitor. > I don't think you need locking here, do you? I agree, no need for locking here. > > + > > + return ret; > > +} > > +static DEVICE_ATTR_RO(dp); > > + > > static ssize_t > > generation_show(struct device *dev, struct device_attribute *attr, char *buf) > > { > > @@ -1693,6 +1708,21 @@ static ssize_t nvm_version_show(struct device *dev, > > } > > static DEVICE_ATTR_RO(nvm_version); > > > > +static ssize_t usb3_show(struct device *dev, struct device_attribute *attr, > > + char *buf) > > +{ > > + struct tb_switch *sw = tb_to_switch(dev); > > + int ret; > > + > > + if (!mutex_trylock(&sw->tb->lock)) > > + return restart_syscall(); > > Same here for locking comment. OK. > > + ret = sprintf(buf, "%u\n", sw->usb3); > > + mutex_unlock(&sw->tb->lock); > > + > > + return ret; > > +} > > +static DEVICE_ATTR_RO(usb3); > > + > > static ssize_t vendor_show(struct device *dev, struct device_attribute *attr, > > char *buf) > > { > > @@ -1725,6 +1755,7 @@ static struct attribute *switch_attrs[] = { > > &dev_attr_boot.attr, > > &dev_attr_device.attr, > > &dev_attr_device_name.attr, > > + &dev_attr_dp.attr, > > &dev_attr_generation.attr, > > &dev_attr_key.attr, > > &dev_attr_nvm_authenticate.attr, > > @@ -1734,6 +1765,7 @@ static struct attribute *switch_attrs[] = { > > &dev_attr_rx_lanes.attr, > > &dev_attr_tx_speed.attr, > > &dev_attr_tx_lanes.attr, > > + &dev_attr_usb3.attr, > > &dev_attr_vendor.attr, > > &dev_attr_vendor_name.attr, > > &dev_attr_unique_id.attr, > > @@ -1757,6 +1789,7 @@ static umode_t switch_attr_is_visible(struct kobject *kobj, > > { > > struct device *dev = kobj_to_dev(kobj); > > struct tb_switch *sw = tb_to_switch(dev); > > + const struct tb *tb = sw->tb; > > > > if (attr == &dev_attr_authorized.attr) { > > if (sw->tb->security_level == TB_SECURITY_NOPCIE || > > @@ -1769,6 +1802,10 @@ static umode_t switch_attr_is_visible(struct kobject *kobj, > > } else if (attr == &dev_attr_device_name.attr) { > > if (!sw->device_name) > > return 0; > > + } else if (attr == &dev_attr_dp.attr) { > > + if (!(tb->cm_caps & TB_CAP_TUNNEL_DETAILS) || > > + !has_port(sw, TB_TYPE_DP_HDMI_OUT)) > > + return 0; > > } else if (attr == &dev_attr_vendor.attr) { > > if (!sw->vendor) > > return 0; > > @@ -1788,6 +1825,10 @@ static umode_t switch_attr_is_visible(struct kobject *kobj, > > if (tb_route(sw)) > > return attr->mode; > > return 0; > > + } else if (attr == &dev_attr_usb3.attr) { > > + if (!(tb->cm_caps & TB_CAP_TUNNEL_DETAILS) || > > + !has_port(sw, TB_TYPE_USB3_UP)) > > + return 0; > > } else if (attr == &dev_attr_nvm_authenticate.attr) { > > if (nvm_upgradeable(sw)) > > return attr->mode; > > diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c > > index 7e6dc2b03bed..32b79dce134f 100644 > > --- a/drivers/thunderbolt/tb.c > > +++ b/drivers/thunderbolt/tb.c > > @@ -104,10 +104,39 @@ static void tb_remove_dp_resources(struct tb_switch *sw) > > } > > } > > > > +static void tb_add_tunnel(struct tb *tb, struct tb_tunnel *tunnel) > > +{ > > + struct tb_switch *sw = tunnel->dst_port->sw; > > + struct tb_cm *tcm = tb_priv(tb); > > + > > + if (tb_tunnel_is_usb3(tunnel)) { > > + sw->usb3++; > > + } else if (tb_tunnel_is_dp(tunnel)) { > > + sw->dp++; > > + /* Inform userspace about DP tunneling change */ > > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); > > What really "changed" here about this device that userspace now needs to > know about it? DisplayPort tunnel came up, so the "dp" attribute value changed. I'm not entirely sure we need to notify the userspace from changes like these, though. We do this when PCIe tunnel comes up already so this kind of follows that (and USB 3.x tunnel is always created at the same time the device itself is announced so does not require any change event). > > + } > > + > > + list_add_tail(&tunnel->list, &tcm->tunnel_list); > > +} > > + > > +static void tb_remove_tunnel(struct tb_tunnel *tunnel) > > +{ > > + struct tb_switch *sw = tunnel->dst_port->sw; > > + > > + if (tb_tunnel_is_usb3(tunnel) && sw->usb3) { > > + sw->usb3--; > > + } else if (tb_tunnel_is_dp(tunnel) && sw->dp) { > > + sw->dp--; > > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); > > Same here, what causes tunnels to be added or removed? Here the opposite, a DisplayPort tunnel was torn down so the "dp" attribute changed. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels 2021-03-10 7:54 ` Mika Westerberg @ 2021-03-10 8:53 ` Greg Kroah-Hartman 2021-03-10 9:21 ` Mika Westerberg 0 siblings, 1 reply; 18+ messages in thread From: Greg Kroah-Hartman @ 2021-03-10 8:53 UTC (permalink / raw) To: Mika Westerberg Cc: linux-usb, Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas On Wed, Mar 10, 2021 at 09:54:44AM +0200, Mika Westerberg wrote: > > > +static void tb_add_tunnel(struct tb *tb, struct tb_tunnel *tunnel) > > > +{ > > > + struct tb_switch *sw = tunnel->dst_port->sw; > > > + struct tb_cm *tcm = tb_priv(tb); > > > + > > > + if (tb_tunnel_is_usb3(tunnel)) { > > > + sw->usb3++; > > > + } else if (tb_tunnel_is_dp(tunnel)) { > > > + sw->dp++; > > > + /* Inform userspace about DP tunneling change */ > > > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); > > > > What really "changed" here about this device that userspace now needs to > > know about it? > > DisplayPort tunnel came up, so the "dp" attribute value changed. > > I'm not entirely sure we need to notify the userspace from changes like > these, though. We do this when PCIe tunnel comes up already so this kind > of follows that (and USB 3.x tunnel is always created at the same time > the device itself is announced so does not require any change event). > > > > + } > > > + > > > + list_add_tail(&tunnel->list, &tcm->tunnel_list); > > > +} > > > + > > > +static void tb_remove_tunnel(struct tb_tunnel *tunnel) > > > +{ > > > + struct tb_switch *sw = tunnel->dst_port->sw; > > > + > > > + if (tb_tunnel_is_usb3(tunnel) && sw->usb3) { > > > + sw->usb3--; > > > + } else if (tb_tunnel_is_dp(tunnel) && sw->dp) { > > > + sw->dp--; > > > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); > > > > Same here, what causes tunnels to be added or removed? > > Here the opposite, a DisplayPort tunnel was torn down so the "dp" > attribute changed. Did just the value in the attribute change, or did the visability of attributes change? And if it is just the value, who is going to care about this value? Is userspace going to want to know this type of change? What causes this to change in the first place (physical event? Virtual event? Something else?) thanks, greg k-h ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels 2021-03-10 8:53 ` Greg Kroah-Hartman @ 2021-03-10 9:21 ` Mika Westerberg 2021-03-10 9:34 ` Greg Kroah-Hartman 0 siblings, 1 reply; 18+ messages in thread From: Mika Westerberg @ 2021-03-10 9:21 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-usb, Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas Hi, On Wed, Mar 10, 2021 at 09:53:31AM +0100, Greg Kroah-Hartman wrote: > On Wed, Mar 10, 2021 at 09:54:44AM +0200, Mika Westerberg wrote: > > > > +static void tb_add_tunnel(struct tb *tb, struct tb_tunnel *tunnel) > > > > +{ > > > > + struct tb_switch *sw = tunnel->dst_port->sw; > > > > + struct tb_cm *tcm = tb_priv(tb); > > > > + > > > > + if (tb_tunnel_is_usb3(tunnel)) { > > > > + sw->usb3++; > > > > + } else if (tb_tunnel_is_dp(tunnel)) { > > > > + sw->dp++; > > > > + /* Inform userspace about DP tunneling change */ > > > > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); > > > > > > What really "changed" here about this device that userspace now needs to > > > know about it? > > > > DisplayPort tunnel came up, so the "dp" attribute value changed. > > > > I'm not entirely sure we need to notify the userspace from changes like > > these, though. We do this when PCIe tunnel comes up already so this kind > > of follows that (and USB 3.x tunnel is always created at the same time > > the device itself is announced so does not require any change event). > > > > > > + } > > > > + > > > > + list_add_tail(&tunnel->list, &tcm->tunnel_list); > > > > +} > > > > + > > > > +static void tb_remove_tunnel(struct tb_tunnel *tunnel) > > > > +{ > > > > + struct tb_switch *sw = tunnel->dst_port->sw; > > > > + > > > > + if (tb_tunnel_is_usb3(tunnel) && sw->usb3) { > > > > + sw->usb3--; > > > > + } else if (tb_tunnel_is_dp(tunnel) && sw->dp) { > > > > + sw->dp--; > > > > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); > > > > > > Same here, what causes tunnels to be added or removed? > > > > Here the opposite, a DisplayPort tunnel was torn down so the "dp" > > attribute changed. > > Did just the value in the attribute change, or did the visability of > attributes change? The value changes. > And if it is just the value, who is going to care about this value? Is > userspace going to want to know this type of change? What causes this > to change in the first place (physical event? Virtual event? Something > else?) It is physical event. Typically this happens when user plugs a monitor to a Thunderbolt dock, or plugs it out. This value changes accordingly. The caring part is the userspace application, I think bolt in case of non-ChromeOS distro and the ChromeOS equivalent but I'm not sure if this is something they find useful or not. That's why I have Cc'd (hopefully all) the people who work with the userspace side :) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels 2021-03-10 9:21 ` Mika Westerberg @ 2021-03-10 9:34 ` Greg Kroah-Hartman 2021-03-10 9:39 ` Mika Westerberg 0 siblings, 1 reply; 18+ messages in thread From: Greg Kroah-Hartman @ 2021-03-10 9:34 UTC (permalink / raw) To: Mika Westerberg Cc: linux-usb, Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas On Wed, Mar 10, 2021 at 11:21:55AM +0200, Mika Westerberg wrote: > Hi, > > On Wed, Mar 10, 2021 at 09:53:31AM +0100, Greg Kroah-Hartman wrote: > > On Wed, Mar 10, 2021 at 09:54:44AM +0200, Mika Westerberg wrote: > > > > > +static void tb_add_tunnel(struct tb *tb, struct tb_tunnel *tunnel) > > > > > +{ > > > > > + struct tb_switch *sw = tunnel->dst_port->sw; > > > > > + struct tb_cm *tcm = tb_priv(tb); > > > > > + > > > > > + if (tb_tunnel_is_usb3(tunnel)) { > > > > > + sw->usb3++; > > > > > + } else if (tb_tunnel_is_dp(tunnel)) { > > > > > + sw->dp++; > > > > > + /* Inform userspace about DP tunneling change */ > > > > > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); > > > > > > > > What really "changed" here about this device that userspace now needs to > > > > know about it? > > > > > > DisplayPort tunnel came up, so the "dp" attribute value changed. > > > > > > I'm not entirely sure we need to notify the userspace from changes like > > > these, though. We do this when PCIe tunnel comes up already so this kind > > > of follows that (and USB 3.x tunnel is always created at the same time > > > the device itself is announced so does not require any change event). > > > > > > > > + } > > > > > + > > > > > + list_add_tail(&tunnel->list, &tcm->tunnel_list); > > > > > +} > > > > > + > > > > > +static void tb_remove_tunnel(struct tb_tunnel *tunnel) > > > > > +{ > > > > > + struct tb_switch *sw = tunnel->dst_port->sw; > > > > > + > > > > > + if (tb_tunnel_is_usb3(tunnel) && sw->usb3) { > > > > > + sw->usb3--; > > > > > + } else if (tb_tunnel_is_dp(tunnel) && sw->dp) { > > > > > + sw->dp--; > > > > > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); > > > > > > > > Same here, what causes tunnels to be added or removed? > > > > > > Here the opposite, a DisplayPort tunnel was torn down so the "dp" > > > attribute changed. > > > > Did just the value in the attribute change, or did the visability of > > attributes change? > > The value changes. > > > And if it is just the value, who is going to care about this value? Is > > userspace going to want to know this type of change? What causes this > > to change in the first place (physical event? Virtual event? Something > > else?) > > It is physical event. Typically this happens when user plugs a monitor > to a Thunderbolt dock, or plugs it out. This value changes accordingly. So are there other events that show up to userspace when this happens? Like devices added/removed? If so, then sending a kobject change event here doesn't make much sense as userspace can go re-read this based on the fact that other things changed. > The caring part is the userspace application, I think bolt in case of > non-ChromeOS distro and the ChromeOS equivalent but I'm not sure if this > is something they find useful or not. That's why I have Cc'd (hopefully > all) the people who work with the userspace side :) What userspace apps read this file today in any Linux distro? thanks, greg k-h ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels 2021-03-10 9:34 ` Greg Kroah-Hartman @ 2021-03-10 9:39 ` Mika Westerberg 2021-03-10 16:24 ` Limonciello, Mario 0 siblings, 1 reply; 18+ messages in thread From: Mika Westerberg @ 2021-03-10 9:39 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-usb, Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas On Wed, Mar 10, 2021 at 10:34:38AM +0100, Greg Kroah-Hartman wrote: > On Wed, Mar 10, 2021 at 11:21:55AM +0200, Mika Westerberg wrote: > > Hi, > > > > On Wed, Mar 10, 2021 at 09:53:31AM +0100, Greg Kroah-Hartman wrote: > > > On Wed, Mar 10, 2021 at 09:54:44AM +0200, Mika Westerberg wrote: > > > > > > +static void tb_add_tunnel(struct tb *tb, struct tb_tunnel *tunnel) > > > > > > +{ > > > > > > + struct tb_switch *sw = tunnel->dst_port->sw; > > > > > > + struct tb_cm *tcm = tb_priv(tb); > > > > > > + > > > > > > + if (tb_tunnel_is_usb3(tunnel)) { > > > > > > + sw->usb3++; > > > > > > + } else if (tb_tunnel_is_dp(tunnel)) { > > > > > > + sw->dp++; > > > > > > + /* Inform userspace about DP tunneling change */ > > > > > > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); > > > > > > > > > > What really "changed" here about this device that userspace now needs to > > > > > know about it? > > > > > > > > DisplayPort tunnel came up, so the "dp" attribute value changed. > > > > > > > > I'm not entirely sure we need to notify the userspace from changes like > > > > these, though. We do this when PCIe tunnel comes up already so this kind > > > > of follows that (and USB 3.x tunnel is always created at the same time > > > > the device itself is announced so does not require any change event). > > > > > > > > > > + } > > > > > > + > > > > > > + list_add_tail(&tunnel->list, &tcm->tunnel_list); > > > > > > +} > > > > > > + > > > > > > +static void tb_remove_tunnel(struct tb_tunnel *tunnel) > > > > > > +{ > > > > > > + struct tb_switch *sw = tunnel->dst_port->sw; > > > > > > + > > > > > > + if (tb_tunnel_is_usb3(tunnel) && sw->usb3) { > > > > > > + sw->usb3--; > > > > > > + } else if (tb_tunnel_is_dp(tunnel) && sw->dp) { > > > > > > + sw->dp--; > > > > > > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); > > > > > > > > > > Same here, what causes tunnels to be added or removed? > > > > > > > > Here the opposite, a DisplayPort tunnel was torn down so the "dp" > > > > attribute changed. > > > > > > Did just the value in the attribute change, or did the visability of > > > attributes change? > > > > The value changes. > > > > > And if it is just the value, who is going to care about this value? Is > > > userspace going to want to know this type of change? What causes this > > > to change in the first place (physical event? Virtual event? Something > > > else?) > > > > It is physical event. Typically this happens when user plugs a monitor > > to a Thunderbolt dock, or plugs it out. This value changes accordingly. > > So are there other events that show up to userspace when this happens? > Like devices added/removed? If so, then sending a kobject change event > here doesn't make much sense as userspace can go re-read this based on > the fact that other things changed. From Thunderbolt driver side in this event (monitor plugged in/out) there are no new devices added or removed. It all happens to the device that already exists (the dock for instance). > > The caring part is the userspace application, I think bolt in case of > > non-ChromeOS distro and the ChromeOS equivalent but I'm not sure if this > > is something they find useful or not. That's why I have Cc'd (hopefully > > all) the people who work with the userspace side :) > > What userspace apps read this file today in any Linux distro? None at the moment. This is completely new attribute. ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels 2021-03-10 9:39 ` Mika Westerberg @ 2021-03-10 16:24 ` Limonciello, Mario 2021-03-10 18:49 ` Lukas Wunner 0 siblings, 1 reply; 18+ messages in thread From: Limonciello, Mario @ 2021-03-10 16:24 UTC (permalink / raw) To: Mika Westerberg, Greg Kroah-Hartman Cc: linux-usb, Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas > -----Original Message----- > From: Mika Westerberg <mika.westerberg@linux.intel.com> > Sent: Wednesday, March 10, 2021 3:40 > To: Greg Kroah-Hartman > Cc: linux-usb@vger.kernel.org; Michael Jamet; Yehezkel Bernat; Andreas Noever; > Lukas Wunner; Limonciello, Mario; Christian Kellner; Benson Leung; Prashant > Malani; Diego Rivas > Subject: Re: [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and > DisplayPort tunnels > > > [EXTERNAL EMAIL] > > On Wed, Mar 10, 2021 at 10:34:38AM +0100, Greg Kroah-Hartman wrote: > > On Wed, Mar 10, 2021 at 11:21:55AM +0200, Mika Westerberg wrote: > > > Hi, > > > > > > On Wed, Mar 10, 2021 at 09:53:31AM +0100, Greg Kroah-Hartman wrote: > > > > On Wed, Mar 10, 2021 at 09:54:44AM +0200, Mika Westerberg wrote: > > > > > > > +static void tb_add_tunnel(struct tb *tb, struct tb_tunnel > *tunnel) > > > > > > > +{ > > > > > > > + struct tb_switch *sw = tunnel->dst_port->sw; > > > > > > > + struct tb_cm *tcm = tb_priv(tb); > > > > > > > + > > > > > > > + if (tb_tunnel_is_usb3(tunnel)) { > > > > > > > + sw->usb3++; > > > > > > > + } else if (tb_tunnel_is_dp(tunnel)) { > > > > > > > + sw->dp++; > > > > > > > + /* Inform userspace about DP tunneling change */ > > > > > > > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); > > > > > > > > > > > > What really "changed" here about this device that userspace now > needs to > > > > > > know about it? > > > > > > > > > > DisplayPort tunnel came up, so the "dp" attribute value changed. > > > > > > > > > > I'm not entirely sure we need to notify the userspace from changes > like > > > > > these, though. We do this when PCIe tunnel comes up already so this > kind > > > > > of follows that (and USB 3.x tunnel is always created at the same time > > > > > the device itself is announced so does not require any change event). > > > > > > > > > > > > + } > > > > > > > + > > > > > > > + list_add_tail(&tunnel->list, &tcm->tunnel_list); > > > > > > > +} > > > > > > > + > > > > > > > +static void tb_remove_tunnel(struct tb_tunnel *tunnel) > > > > > > > +{ > > > > > > > + struct tb_switch *sw = tunnel->dst_port->sw; > > > > > > > + > > > > > > > + if (tb_tunnel_is_usb3(tunnel) && sw->usb3) { > > > > > > > + sw->usb3--; > > > > > > > + } else if (tb_tunnel_is_dp(tunnel) && sw->dp) { > > > > > > > + sw->dp--; > > > > > > > + kobject_uevent(&sw->dev.kobj, KOBJ_CHANGE); > > > > > > > > > > > > Same here, what causes tunnels to be added or removed? > > > > > > > > > > Here the opposite, a DisplayPort tunnel was torn down so the "dp" > > > > > attribute changed. > > > > > > > > Did just the value in the attribute change, or did the visability of > > > > attributes change? > > > > > > The value changes. > > > > > > > And if it is just the value, who is going to care about this value? Is > > > > userspace going to want to know this type of change? What causes this > > > > to change in the first place (physical event? Virtual event? Something > > > > else?) > > > > > > It is physical event. Typically this happens when user plugs a monitor > > > to a Thunderbolt dock, or plugs it out. This value changes accordingly. > > > > So are there other events that show up to userspace when this happens? > > Like devices added/removed? If so, then sending a kobject change event > > here doesn't make much sense as userspace can go re-read this based on > > the fact that other things changed. > > From Thunderbolt driver side in this event (monitor plugged in/out) > there are no new devices added or removed. It all happens to the device > that already exists (the dock for instance). > > > > The caring part is the userspace application, I think bolt in case of > > > non-ChromeOS distro and the ChromeOS equivalent but I'm not sure if this > > > is something they find useful or not. That's why I have Cc'd (hopefully > > > all) the people who work with the userspace side :) > > > > What userspace apps read this file today in any Linux distro? > > None at the moment. This is completely new attribute. In practice software based connection manager isn't in any production system in the field today and these attributes are to determine what sort of useful information userspace can obtain and make messaging or let user's make decisions. For example you might envision "at some point" you might let a user configure a policy to prefer to allocate bandwidth to certain types of devices, but to enact that policy you'll need to know everything that is connected. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels 2021-03-10 16:24 ` Limonciello, Mario @ 2021-03-10 18:49 ` Lukas Wunner 2021-03-10 19:32 ` Limonciello, Mario 0 siblings, 1 reply; 18+ messages in thread From: Lukas Wunner @ 2021-03-10 18:49 UTC (permalink / raw) To: Limonciello, Mario Cc: Mika Westerberg, Greg Kroah-Hartman, linux-usb, Michael Jamet, Yehezkel Bernat, Andreas Noever, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas On Wed, Mar 10, 2021 at 04:24:20PM +0000, Limonciello, Mario wrote: > In practice software based connection manager isn't in any production > system in the field today ... except everything Apple. ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels 2021-03-10 18:49 ` Lukas Wunner @ 2021-03-10 19:32 ` Limonciello, Mario 0 siblings, 0 replies; 18+ messages in thread From: Limonciello, Mario @ 2021-03-10 19:32 UTC (permalink / raw) To: Lukas Wunner Cc: Mika Westerberg, Greg Kroah-Hartman, linux-usb, Michael Jamet, Yehezkel Bernat, Andreas Noever, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas > -----Original Message----- > From: Lukas Wunner <lukas@wunner.de> > Sent: Wednesday, March 10, 2021 12:50 > To: Limonciello, Mario > Cc: Mika Westerberg; Greg Kroah-Hartman; linux-usb@vger.kernel.org; Michael > Jamet; Yehezkel Bernat; Andreas Noever; Christian Kellner; Benson Leung; > Prashant Malani; Diego Rivas > Subject: Re: [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and > DisplayPort tunnels > > > [EXTERNAL EMAIL] > > On Wed, Mar 10, 2021 at 04:24:20PM +0000, Limonciello, Mario wrote: > > In practice software based connection manager isn't in any production > > system in the field today > > ... except everything Apple. I was referring to USB4 products... but yeah you're right. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels 2021-03-09 13:48 ` [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels Mika Westerberg 2021-03-09 14:09 ` Greg Kroah-Hartman @ 2021-03-17 10:19 ` Mika Westerberg 1 sibling, 0 replies; 18+ messages in thread From: Mika Westerberg @ 2021-03-17 10:19 UTC (permalink / raw) To: linux-usb Cc: Michael Jamet, Yehezkel Bernat, Andreas Noever, Lukas Wunner, Mario Limonciello, Christian Kellner, Benson Leung, Prashant Malani, Diego Rivas, Greg Kroah-Hartman Hi Benson, Prashant, Diego, Christian and Mario (and others who may be interested) On Tue, Mar 09, 2021 at 04:48:18PM +0300, Mika Westerberg wrote: > +What: /sys/bus/thunderbolt/devices/.../domainX/tunneling_details > +Date: July 2021 > +KernelVersion: 5.13 > +Contact: Mika Westerberg <mika.westerberg@linux.intel.com> > +Description: The connection manager implementation may expose > + additional details about tunneling. If it supports this > + the attribute reads 1. > + > > +What: /sys/bus/thunderbolt/devices/.../dp > +Date: Jul 2021 > +KernelVersion: 5.13 > +Contact: Mika Westerberg <mika.westerberg@linux.intel.com> > +Description: Only available if the domain tunneling_details attribute > + reads 1. If present means that the device router has > + DisplayPort sink. Contents will be number how many > + active DisplayPort tunnels end up to this router. > + > > +What: /sys/bus/thunderbolt/devices/.../usb3 > +Date: Jul 2021 > +KernelVersion: 5.13 > +Contact: Mika Westerberg <mika.westerberg@linux.intel.com> > +Description: Only available if the domain tunneling_details attribute > + reads 1. If present means that the device router has > + USB 3.x upstream adapter. Reads 1 if there is an active > + USB 3.x tunnel to this router. Do you think these attributes help the userspace at all? I mean if you think they are not usable as is we can consider some alternatives too. I was going to drop the KOBJ_CHANGE event (and the unnecessary locking) from v2 but before sending a new version, I would like to get some feedback if this is even needed. Thanks! ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-03-17 10:20 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-09 13:48 [PATCH 0/3] thunderbolt: Expose details about tunneling Mika Westerberg 2021-03-09 13:48 ` [PATCH 1/3] thunderbolt: Add details to router uevent Mika Westerberg 2021-03-09 14:10 ` Greg Kroah-Hartman 2021-03-10 17:28 ` Limonciello, Mario 2021-03-11 7:59 ` Mika Westerberg 2021-03-09 13:48 ` [PATCH 2/3] thunderbolt: Hide authorized attribute if router does not support PCIe tunnels Mika Westerberg 2021-03-09 14:10 ` Greg Kroah-Hartman 2021-03-09 13:48 ` [PATCH 3/3] thunderbolt: Expose more details about USB 3.x and DisplayPort tunnels Mika Westerberg 2021-03-09 14:09 ` Greg Kroah-Hartman 2021-03-10 7:54 ` Mika Westerberg 2021-03-10 8:53 ` Greg Kroah-Hartman 2021-03-10 9:21 ` Mika Westerberg 2021-03-10 9:34 ` Greg Kroah-Hartman 2021-03-10 9:39 ` Mika Westerberg 2021-03-10 16:24 ` Limonciello, Mario 2021-03-10 18:49 ` Lukas Wunner 2021-03-10 19:32 ` Limonciello, Mario 2021-03-17 10:19 ` Mika Westerberg
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.