Hi Mario,
On Fri, Sep 23, 2022 at 01:39:44PM -0500, Mario Limonciello wrote:
Software that has run before the USB4 CM in Linux runs may have disabled hotplug events for a given lane adapter.
Other CMs such as that one distributed with Windows 11 will enable hotplug events. Do the same thing in the Linux CM which fixes hotplug events on "AMD Pink Sardine".
Cc: stable@vger.kernel.org Signed-off-by: Mario Limonciello mario.limonciello@amd.com
v2->v3:
- Guard with tb_switch_is_icm to avoid risk to Intel FW CM case
v1->v2:
- s/usb4_enable_hotplug/usb4_port_hotplug_enable/
- Clarify intended users in documentation comment
- Only call for lane adapters
- Add stable tag
Note: v2 it was suggested to move this to tb_switch_configure, but port->config is not yet read at that time. This should be the right time to call it, so this version of the patch just guards against the code running on Intel's controllers that have a FW CM.
Can we put it in a separate function that is called from tb_switch_add()? I explained in the previous reply (to v2) that the tb_init_port() is pretty much just reading stuff and therefore I would not like to add there anything that does writing.
While at it, the USB4 v2 spec (not yet public but can be found from the working group site) adds a CM requirement that forbids writing lane 1 adapter configuration registers so in addition to that please check port->cap_usb4 there so we know we deal with the lane 0 adapter only (as ->cap_usb4 is only set for the lane 0 adapters).
So something like this:
static void tb_switch_port_hotplug_enable(struct tb_switch *sw) { if (tb_switch_is_icm(sw)) return;
tb_switch_for_each_port(sw, port) { if (!port->cap_usb4) continue;
// here enable the hotplug } }
int tb_switch_add(struct tb_switch *sw) { ... // init ports and stuff happens here
tb_switch_default_link_ports(sw); tb_switch_port_hotplug_enable(sw); }
(we should probably create a new macro tb_switch_for_each_usb4_port() that just enumerates all USB4 ports).