Received-SPF: pass (google.com: domain of linux-wireless+bounces-5593-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <8b0f6f62-ff3a-483c-bceb-2f9c56526f44@quicinc.com>
Date: Fri, 29 Mar 2024 22:43:54 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC 2/2] wifi: mac80211: Add support for link reconfigure
 removal
Content-Language: en-US
To: Johannes Berg <johannes@sipsolutions.net>
CC: <linux-wireless@vger.kernel.org>
References: <20240328055235.3034174-1-quic_mdharane@quicinc.com>
 <20240328055235.3034174-3-quic_mdharane@quicinc.com>
 <3f77e4e0455fbfdec7e14777cf116175eae32f49.camel@sipsolutions.net>
From: Manish Dharanenthiran <quic_mdharane@quicinc.com>
In-Reply-To: <3f77e4e0455fbfdec7e14777cf116175eae32f49.camel@sipsolutions.net>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 3/28/2024 11:53 PM, Johannes Berg wrote:
> On Thu, 2024-03-28 at 11:22 +0530, Manish Dharanenthiran wrote:
>>
>> +int __ieee80211_link_reconfig_remove(struct ieee80211_local *local,
>> +				     struct ieee80211_sub_if_data *sdata,
>> +				     const struct cfg80211_link_reconfig_removal_params *params)
>> +{
>> +	struct ieee80211_link_data *link;
>> +	int ret;
>> +
>> +	if (!ieee80211_sdata_running(sdata))
>> +		return -ENETDOWN;
>> +
>> +	if (sdata->vif.type != NL80211_IFTYPE_AP)
>> +		return -EINVAL;
>> +
>> +	link = sdata_dereference(sdata->link[params->link_id], sdata);
>> +	if (!link)
>> +		return -ENOLINK;
>> +
>> +	ret = drv_link_reconfig_remove(local, sdata, params);
>> +
>> +	return ret;
> 
> Again ... remove the 'ret' variable, it serves no purpose at all.
> 
>> +}
>> +
>> +int ieee80211_update_link_reconfig_remove_status(struct ieee80211_vif *vif,
>> +						 unsigned int link_id,
>> +						 u8 tbtt_count, u64 tsf,
>> +						 enum ieee80211_link_reconfig_remove_state status)
>> +{
>> +	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
>> +
>> +	if (vif->type != NL80211_IFTYPE_AP) {
>> +		sdata_err(sdata, "Discarding link reconfig status for unsupported vif type\n");
> 
> Uh, no. Remove that message please.
> 
Sure, will make this if block in different way.

>> +TRACE_EVENT(drv_link_reconfig_remove,
>> +	    TP_PROTO(struct ieee80211_local *local,
>> +		     struct ieee80211_sub_if_data *sdata,
>> +		     const struct cfg80211_link_reconfig_removal_params *params),
>> +
>> +	TP_ARGS(local, sdata, params),
>> +
>> +	TP_STRUCT__entry(LOCAL_ENTRY
>> +			 VIF_ENTRY
>> +			 __field(u32, link_id)
>> +			 __field(u16, count)
>> +			 __dynamic_array(u8, frame, params->ie_len)
>> +	),
> 
> All the same things about indentation apply here.
> 
> johannes
>