Re: [PATCH v3] thunderbolt: Explicitly enable lane adapter hotplug events at startup

[Mika Westerberg]

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@xxxxxxxxxxxxxxx
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
> 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).