Re: [PATCH v4] thunderbolt: Disable Gen 4 Recovery on Asymmetric Transitions

Mika Westerberg
2025-02-04T11:39:59Z

Hi Again,

On Tue, Feb 04, 2025 at 01:16:25PM +0200, Mika Westerberg wrote:
> On Mon, Feb 03, 2025 at 02:59:07PM +0200, Mika Westerberg wrote:
> > On Mon, Feb 03, 2025 at 12:43:04PM +0000, Mohammad Rahimi wrote:
> > > Hello again.
> > >
> > > On Mon, Feb 03, 2025 at 02:39:44PM GMT, Mika Westerberg wrote:
> > > > Hi,
> > > >
> > > > On Mon, Feb 03, 2025 at 12:25:21PM +0000, Mohammad Rahimi wrote:
> > > > > Hello.
> > > > >
> > > > > On Mon, Feb 03, 2025 at 10:42:24AM GMT, Mika Westerberg wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Fri, Jan 31, 2025 at 01:41:27AM +0000, Mohammad Rahimi wrote:
> > > > > > > Updates the Connection Manager to disable the Gen 4 Link Recovery
> > > > > > > flow before transitioning from a Symmetric Link to an Asymmetric
> > > > > > > Link, as specified in recent changes to the USB4 v2 specification.
> > > > > > >
> > > > > > > According to the "USB4 2.0 ENGINEERING CHANGE NOTICE FORM"
> > > > > > > published in September 2024, the rationale for this change is:
> > > > > > >
> > > > > > > "Since the default value of the Target Asymmetric Link might be
> > > > > > > different than Symmetric Link and Gen 4 Link Recovery flow checks
> > > > > > > this field to make sure it matched the actual Negotiated Link Width,
> > > > > > > we’re removing the condition for a Disconnect in the Gen 4 Link
> > > > > > > Recovery flow when Target Asymmetric Link doesn’t match the actual
> > > > > > > Link width and adding a Connection Manager note to Disable Gen 4 Link
> > > > > > > Recovery flow before doing Asymmetric Transitions."
> > > > > > >
> > > > > > > Signed-off-by: Mohammad Rahimi <rahimi.mhmmd@gmail.com>
> > > > > >
> > > > > > I did some minor modifications and applied to thunderbolt.git/next. Please
> > > > > > let me know if I missed something.
> > > > > >
> > > > >
> > > > > Looks great. Just one question:
> > > > >
> > > > > In tb_configure_asym(), if tb_switch_set_link_width() fails, we won’t restore
> > > > > the link recovery status. Is that okay?
> > > >
> > > > Good question :) Looking at the ECR, does it actually say anywhere that the
> > > > CM should needs to re-enable it? I'm thinking that we could just disable it
> > > > and be done with it?
> > >
> > > Right. Thanks for clearing that up.
> >
> > I'll check with the HW/FW folks still if they have any suggestions. I'll
> > keep you updated.
>
> Did not hear anything from them yet but I adjusted the patch slightly to
> restore ELR if tb_switch_set_link_width() and also added back the check > 0
> which I missed. Let me know if you find issues with this one. Thanks!

Okay got response now. It turns out this feature is something CM needs to
enable if needed (e.g it is not automatically enabled by any router) and we
don't do that which means this patch is not necessary, sorry. I'm going to
drop it for now. Let me know if you think otherwise. Thanks!