Received-SPF: pass (google.com: domain of linux-wireless+bounces-2814-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <3b37b302-aaf5-421c-af70-288aa8256685@quicinc.com>
Date: Tue, 30 Jan 2024 19:20:44 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v7 3/5] wifi: mac80211: handle set csa/after_csa beacon on
 per link basis
Content-Language: en-US
From: Aditya Kumar Singh <quic_adisi@quicinc.com>
To: Johannes Berg <johannes@sipsolutions.net>
CC: <linux-wireless@vger.kernel.org>
References: <20240130043225.942202-1-quic_adisi@quicinc.com>
 <20240130043225.942202-4-quic_adisi@quicinc.com>
 <1e73c061653abf8e5f1671ae026e0905b2fb4ded.camel@sipsolutions.net>
 <55d1aea6-8721-4774-9f09-549245461ba8@quicinc.com>
In-Reply-To: <55d1aea6-8721-4774-9f09-549245461ba8@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit

On 1/30/24 16:13, Aditya Kumar Singh wrote:
> On 1/30/24 15:47, Johannes Berg wrote:
>> On Tue, 2024-01-30 at 10:02 +0530, Aditya Kumar Singh wrote:
>>> In order to support CSA with MLO, there is a need to handle the 
>>> functions
>>> ieee80211_set_csa_beacon() and ieee80211_set_after_csa_beacon() on per
>>> link basis.
>>
>> nit: "on a per link"
> 
> Will address in next version. Thanks.
> 
>>
>>> Add changes for the same.
>>
>> Is that some cultural thing?
>>
>> I always find this phrasing with "for the same" very odd, and would
>> rather say something useful such as "Implement this by passing the
>> correct link data"... but I see this a lot, hence the question.
>>
> 
> No idea :). Even I have seen these quite a few times and thought that 
> may be it is fine to use it that way. But I do agree that instead we 
> could put something useful instead. Thanks for pointing it out, I will 
> address this in next version.
> 
> 
>>> @@ -3658,7 +3659,7 @@ static int __ieee80211_csa_finalize(struct 
>>> ieee80211_link_data *link_data)
>>>       sdata->vif.bss_conf.csa_active = false;
>>> -    err = ieee80211_set_after_csa_beacon(sdata, &changed);
>>> +    err = ieee80211_set_after_csa_beacon(&sdata->deflink, &changed);
>>
>> weren't you just saying deflink shouldn't be used?
>>
> 
> Correct. This patch's aim is to form the base - basically modify the 
> helper function to accept the link data argument. But at this point, CSA 
> is not started on per link basis hence in order to keep CSA working 
> still as it is before this patch, have used deflink here. Functionality 
> wise, this patch is not bringing any change yet.
> 
> 
>>> @@ -3928,7 +3930,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, 
>>> struct net_device *dev,
>>>       if (sdata->vif.bss_conf.color_change_active)
>>>           ieee80211_color_change_abort(sdata);
>>> -    err = ieee80211_set_csa_beacon(sdata, params, &changed);
>>> +    err = ieee80211_set_csa_beacon(&sdata->deflink, params, &changed);
>>
>> dito
>>
> 
> Addressed above.
> 

Any other comments? So that I can address those and send v8