Re: [PATCH] thunderbolt: Confirm the necessity to configure asym link first

[zhangjianrong (E)]

On 6/27/2025 12:46 PM, Mika Westerberg wrote:
> Hi,
>
> On Thu, Jun 26, 2025 at 11:50:00AM +0000, Charalampos Mitrodimas wrote:
> > Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> writes:
> >
> > > Hi,
> > >
> > > On Thu, Jun 26, 2025 at 04:41:07PM +0800, zhangjianrong wrote:
> > > > Current implementation can cause allocation failures in
> > > > tb_alloc_dp_bandwidth() in some cases. For example:
> > > > allocated_down(30Gbps), allocated_up(50Gbps),
> > > > requested_down(10Gbps).
> > >
> > > I'm not sure I understand the above.
> > >
> > > Can you describe in which real life situation this can happen?
> >
> > I suppose this can happen when reducing bandwidth while total upstream
> > bandwidth usage on the link exceeds TB_ASYM_MIN (36 Gbps). The
> > allocation fails at the asymmetric limit check before checking whether
> > the downstream request actually needs asymmetric mode.
>
> Right, but I would like to see here in the changelog explanation of the
> situation and preferably parts of the dmesg showing the error as well.

OK, I will update the commit message, but I don't have the dmesg because 
I can't find a host router that contains two dp adapters.

> @zhangjianrong, can you do that and resend?
>
> Note, I will be on vacation after today so expect delay from my side.
>
> > > > Signed-off-by: zhangjianrong <zhangjianrong5@xxxxxxxxxx>
> > > > ---
> > > >   drivers/thunderbolt/tb.c | 10 +++++-----
> > > >   1 file changed, 5 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> > > > index a7c6919fbf97..558455d9716b 100644
> > > > --- a/drivers/thunderbolt/tb.c
> > > > +++ b/drivers/thunderbolt/tb.c
> > > > @@ -1039,6 +1039,9 @@ static int tb_configure_asym(struct tb *tb, struct tb_port *src_port,
> > > >   			break;
> > > >   
> > > >   		if (downstream) {
> > > > +			/* Does consumed + requested exceed the threshold */
> > > > +			if (consumed_down + requested_down < asym_threshold)
> > > > +				continue;
> > > >   			/*
> > > >   			 * Downstream so make sure upstream is within the 36G
> > > >   			 * (40G - guard band 10%), and the requested is above
> > > > @@ -1048,20 +1051,17 @@ static int tb_configure_asym(struct tb *tb, struct tb_port *src_port,
> > > >   				ret = -ENOBUFS;
> > > >   				break;
> > > >   			}
> > > > -			/* Does consumed + requested exceed the threshold */
> > > > -			if (consumed_down + requested_down < asym_threshold)
> > > > -				continue;
> > > >   
> > > >   			width_up = TB_LINK_WIDTH_ASYM_RX;
> > > >   			width_down = TB_LINK_WIDTH_ASYM_TX;
> > > >   		} else {
> > > >   			/* Upstream, the opposite of above */
> > > > +			if (consumed_up + requested_up < asym_threshold)
> > > > +				continue;
> > > >   			if (consumed_down + requested_down >= TB_ASYM_MIN) {
> > > >   				ret = -ENOBUFS;
> > > >   				break;
> > > >   			}
> > > > -			if (consumed_up + requested_up < asym_threshold)
> > > > -				continue;
> > > >   
> > > >   			width_up = TB_LINK_WIDTH_ASYM_TX;
> > > >   			width_down = TB_LINK_WIDTH_ASYM_RX;
> > > > --
> > > > 2.34.1