Messages in this thread | | | Date | Tue, 12 Dec 2023 13:25:38 -0600 | Subject | Re: [Patch v2 2/2] thunderbolt: Teardown tunnels and reset downstream ports created by boot firmware | From | Mario Limonciello <> |
| |
On 12/12/2023 13:24, Mario Limonciello wrote: > On 12/12/2023 13:16, Sanath S wrote: >> Boot firmware might have created tunnels of its own. Since we cannot >> be sure they are usable for us. Tear them down and reset the ports >> to handle it as a new hotplug for USB3 routers. > s/3/4/ > >> >> Suggested-by: Mario Limonciello <mario.limonciello@amd.com> >> Signed-off-by: Sanath S <Sanath.S@amd.com> >> --- >> drivers/thunderbolt/tb.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c >> index fd49f86e0353..febd0b6972e3 100644 >> --- a/drivers/thunderbolt/tb.c >> +++ b/drivers/thunderbolt/tb.c >> @@ -2598,6 +2598,17 @@ static int tb_start(struct tb *tb) >> tb_switch_tmu_enable(tb->root_switch); >> /* Full scan to discover devices added before the driver was >> loaded. */ >> tb_scan_switch(tb->root_switch); >> + /* >> + * Boot firmware might have created tunnels of its own. Since we >> cannot >> + * be sure they are usable for us, Tear them down and reset the >> ports >> + * to handle it as new hotplug for USB4 routers. >> + */ >> + if (tb_switch_is_usb4(tb->root_switch)) { >> + tb_switch_discover_tunnels(tb->root_switch, >> + &tcm->tunnel_list, false); >> + tcm->hotplug_active = true; >> + return tb_switch_reset_ports(tb->root_switch); >> + } >> /* Find out tunnels created by the boot firmware */ >> tb_discover_tunnels(tb); >> /* Add DP resources from the DP tunnels created by the boot >> firmware */ > > Doesn't this cause the following to not run and thus break hotplug? > > tcm->hotplug_active = true; > > > I think it would be better to do this like this flow: > > if (tb_switch_is_usb4(tb->root_switch)) { > tb_switch_discover_tunnels(tb->root_switch, > &tcm->tunnel_list, false); > tcm->hotplug_active = true; > ret = tb_switch_reset_ports(tb->root_switch); > if (ret) > return ret; > } else { > /* keep existing tunnel flow */ > } > > tcm->hotplug_active = true; > > return 0; > > That makes it crystal clear that hotplug isn't enabled until it's done > being setup, which means either getting the existing tunnels or doing > the reset.
My apologies I completely missed replacement call even though I pasted it.
Besides the s/3/4 this looks fine to me.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
| |