Hi,
On Fri, Sep 23, 2022 at 11:08:30AM -0500, Limonciello, Mario wrote:
On 9/23/2022 04:16, Mika Westerberg wrote:
Hi Mario,
On Thu, Sep 22, 2022 at 11:07:29AM -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
v1->v2:
- Only send second patch as first was merged already
- s/usb4_enable_hotplug/usb4_port_hotplug_enable/
- Clarify intended users in documentation comment
- Only call for lane adapters
- Add stable tag
drivers/thunderbolt/switch.c | 4 ++++ drivers/thunderbolt/tb.h | 1 + drivers/thunderbolt/tb_regs.h | 1 + drivers/thunderbolt/usb4.c | 20 ++++++++++++++++++++ 4 files changed, 26 insertions(+)
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index 77d7f07ca075..3213239d12c8 100644 --- a/drivers/thunderbolt/switch.c +++ b/drivers/thunderbolt/switch.c @@ -778,6 +778,10 @@ static int tb_init_port(struct tb_port *port) if (!tb_port_read(port, &hop, TB_CFG_HOPS, 0, 2)) port->ctl_credits = hop.initial_credits;
res = usb4_port_hotplug_enable(port);if (res)I think this does not belong here in tb_init_port(). This is called from both FW and SW CM paths and we don't want to confuse the FW CM more than necessary ;-)
So instead I think this should be added to tb_plug_events_active().
The problem with that location is that tb_plug_events_active() is called from tb_switch_configure() which is before tb_switch_add() is called. tb_switch_add() calls tb_init_port() which reads port->config for the first time.
Ah indeed, I missed that.
So if this is only to be called in tb_switch_configure() it means reading port->config "earlier" too.
So it definitely needs to be called in tb_init_port() or a later function but before the device is announced to satisfy only running on the appropriate port types.
tb_init_port() or tb_switch_add feels like the right place to me. How about leaving it where it is but guarding with a "if (!tb_switch_is_icm())" to avoid the risk to the FW CM case?
What about adding a new function that does this and it called from tb_switch_add() before announcing devices to the world? tb_init_port() is pretty much reading stuff from adapters so I would not like to add there anything that does writing if possible.