Re: [PATCH v3] thunderbolt: Add DP out resource when DP tunnel is discovered.

[Sanjay R Mehta <sanmehta@amd.com>]

On 8/4/2022 12:00 PM, Mika Westerberg wrote:
> On Wed, Aug 03, 2022 at 11:29:54PM -0500, Sanjay R Mehta wrote:
>> From: Sanjay R Mehta <sanju.mehta@amd.com>
>>
>> If the boot firmware implements a connection manager of its
>> own it may create a DP tunnel and will be handed off to Linux
>> CM, but the DP out resource is not saved in the dp_resource
>> list.
>>
>> This patch adds tunnelled DP out port to the dp_resource list
>> once the DP tunnel is discovered.
>>
>> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com>
>> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>>
>> ---
>> v3: make tb_dp_resource_available_discovered as static function.
> 
> Hmm, I suggested this:
> 
>   Please call this tb_discover_dp_resources() make it static and call it
>   right after tb_discover_tunnels() in tb_start() or in
>   tb_discover_tunnels().      
> 
> Anything preventing you to do that? Or you missed my comment?
Sorry, I missed the comment of changing function name. I'll resend the
patch.
> 
>> v2: Re-ordering the function declaration as per Greg's comment.
>> ---
>>  drivers/thunderbolt/tb.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
>> index 9a3214f..53abce3 100644
>> --- a/drivers/thunderbolt/tb.c
>> +++ b/drivers/thunderbolt/tb.c
>> @@ -105,6 +105,21 @@ static void tb_remove_dp_resources(struct tb_switch *sw)
>>  	}
>>  }
>>  
>> +static void tb_dp_resource_available_discovered(struct tb *tb, struct tb_port *port)
>> +{
>> +	struct tb_cm *tcm = tb_priv(tb);
>> +	struct tb_port *p;
>> +
>> +	list_for_each_entry(p, &tcm->dp_resources, list) {
>> +		if (p == port)
>> +			return;
>> +	}
>> +
>> +	tb_port_dbg(port, "DP %s resource available discovered\n",
>> +		    tb_port_is_dpin(port) ? "IN" : "OUT");
>> +	list_add_tail(&port->list, &tcm->dp_resources);
>> +}
>> +
>>  static void tb_switch_discover_tunnels(struct tb_switch *sw,
>>  				       struct list_head *list,
>>  				       bool alloc_hopids)
>> @@ -118,6 +133,7 @@ static void tb_switch_discover_tunnels(struct tb_switch *sw,
>>  		switch (port->config.type) {
>>  		case TB_TYPE_DP_HDMI_IN:
>>  			tunnel = tb_tunnel_discover_dp(tb, port, alloc_hopids);
> 
> Here tunnel can be NULL...
> 
>> +			tb_dp_resource_available_discovered(tb, tunnel->dst_port);
> 
> ... so this will crash and burn.

Thanks. Agree, I will add check here and resend the patch.