Message-ID: <31632d31-8c98-081f-bcf4-0966efe1256a@quicinc.com>
Date:   Tue, 31 Jan 2023 14:07:54 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [PATCH] wifi: cfg80211: call reg_notifier for self managed wiphy
 from driver hint
Content-Language: en-US
To:     Johannes Berg <johannes@sipsolutions.net>,
        <ath11k@lists.infradead.org>
CC:     <linux-wireless@vger.kernel.org>
References: <20221214093937.14987-1-quic_wgong@quicinc.com>
 <0881ee1d0ae43ae18ab4b71ec8b5fd62263bbb69.camel@sipsolutions.net>
From:   Wen Gong <quic_wgong@quicinc.com>
In-Reply-To: <0881ee1d0ae43ae18ab4b71ec8b5fd62263bbb69.camel@sipsolutions.net>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 1/19/2023 9:52 PM, Johannes Berg wrote:
> On Wed, 2022-12-14 at 04:39 -0500, Wen Gong wrote:
>> Currently the regulatory driver does not call the regulatory callback
>> reg_notifier for self managed wiphys. Sometimes driver needs cfg80211
>> to calculate the info of ieee80211_channel such as flags and power,
>> and driver needs to get the info of ieee80211_channel after hint of
>> driver, but driver does not know when calculation of the info of
>> ieee80211_channel become finished, so add notify to driver after
>> reg_process_self_managed_hint() from cfg80211 is a good way, then
>> driver could get the correct info in callback of reg_notifier.
> Seems reasonable - but maybe unexpected to some drivers, perhaps it
> should be opt-in?
>
> Though I guess not many drivers actually use this infrastructure in the
> first place?

Yes, I will add a new flag such as WIPHY_FLAG_NOTIFY_REGDOM_BY_DRIVER 
for this driver.

is it ok?

>> @@ -3095,6 +3107,13 @@ static void notify_self_managed_wiphys(struct regulatory_request *request)
>>   		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED &&
>>   		    request->initiator == NL80211_REGDOM_SET_BY_USER)
>>   			reg_call_notifier(wiphy, request);
>> +
>> +		if (wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED &&
>> +		    request->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
>> +		    request->wiphy_idx == get_wiphy_idx(wiphy)) {
>> +			reg_call_notifier(wiphy, request);
>> +			request->wiphy_idx = WIPHY_IDX_INVALID;
>> +		}
> Why set the request->wiphy_idx here? Should this even go through
> reg_process_pending_hints() at all?

it is to skip handle for NL80211_REGDOM_SET_BY_DRIVER in 
reg_process_pending_hints()/reg_process_hint().

After change to use reg_call_notifier(), then it is not need again.

>
>> +	driver_request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
>> +	if (!driver_request)
>> +		return;
>> +
>> +	memcpy(driver_request, &request, sizeof(*driver_request));
> kmemdup()?

yes.

After change to use reg_call_notifier(), then it is not need again.

>
>> +	queue_regulatory_request(driver_request);
> But again not sure you should do this, rather than calling the notifier
> directly?
>
> I mean, you could just do reg_call_notifier() here, it's already async?
Yes, I will change to use reg_call_notifier() here, then it will be simple.
> johannes
>