Received-SPF: pass (google.com: domain of linux-wireless+bounces-4915-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <6989461b-295e-4886-9d39-64f15cc01074@quicinc.com>
Date: Tue, 19 Mar 2024 21:52:55 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 12/12] wifi: ath12k: support get_survey mac op for
 single wiphy
Content-Language: en-US
To: Jeff Johnson <quic_jjohnson@quicinc.com>, <ath12k@lists.infradead.org>
CC: <linux-wireless@vger.kernel.org>, Sriram R <quic_srirrama@quicinc.com>
References: <20240312135557.1778379-1-quic_ramess@quicinc.com>
 <20240312135557.1778379-13-quic_ramess@quicinc.com>
 <036abbcc-eb90-48db-a82e-5565bcc336ef@quicinc.com>
From: Rameshkumar Sundaram <quic_ramess@quicinc.com>
In-Reply-To: <036abbcc-eb90-48db-a82e-5565bcc336ef@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 3/13/2024 4:55 AM, Jeff Johnson wrote:
> On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
>> From: Sriram R <quic_srirrama@quicinc.com>
>>
>> The radio for which the survey info needs to be collected
>> depends on the channel idx which could be based on the band.
>> Use the idx to identify the appropriate sband since multiple
>> bands could be combined for single wiphy case.
>>
>> Also use the channel idx and sband to identify the corresponding
>> radio on which the survey results needs to be populated.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>
>> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
>> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/mac.c | 25 ++++++++++++++++---------
>>   1 file changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index 6123d7db2edc..a31003f8325d 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -7821,7 +7821,6 @@ ath12k_mac_update_bss_chan_survey(struct ath12k *ar,
>>   static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>>   				    struct survey_info *survey)
>>   {
>> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>>   	struct ath12k *ar;
>>   	struct ieee80211_supported_band *sband;
>>   	struct survey_info *ar_survey;
>> @@ -7830,12 +7829,6 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>>   	if (idx >= ATH12K_NUM_CHANS)
>>   		return -ENOENT;
>>   
>> -	ar = ath12k_ah_to_ar(ah, 0);
>> -
>> -	ar_survey = &ar->survey[idx];
>> -
>> -	mutex_lock(&ar->conf_mutex);
>> -
>>   	sband = hw->wiphy->bands[NL80211_BAND_2GHZ];
>>   	if (sband && idx >= sband->n_channels) {
>>   		idx -= sband->n_channels;
>> @@ -7850,6 +7843,21 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>>   		goto exit;
>>   	}
>>   
>> +	ar = ath12k_mac_get_ar_by_chan(hw, &sband->channels[idx]);
>> +	if (!ar) {
>> +		if (sband->channels[idx].flags & IEEE80211_CHAN_DISABLED) {
>> +			ret = 0;
>> +			memset(survey, 0, sizeof(*survey));
>> +			goto exit;
>> +		}
>> +		ret = -ENOENT;
>> +		goto exit;
>> +	}
>> +
>> +	ar_survey = &ar->survey[idx];
>> +
>> +	mutex_lock(&ar->conf_mutex);
>> +
>>   	ath12k_mac_update_bss_chan_survey(ar, &sband->channels[idx]);
>>   
>>   	spin_lock_bh(&ar->data_lock);
>> @@ -7861,9 +7869,8 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>>   	if (ar->rx_channel == survey->channel)
>>   		survey->filled |= SURVEY_INFO_IN_USE;
>>   
>> -exit:
>>   	mutex_unlock(&ar->conf_mutex);
>> -
>> +exit:
> 
> goto should normally only be used when there is centralized cleanup.
> since now there is no cleanup required, all of the goto exit calls should just
> directly return the appropriate error
> 
Sure, this sneaked through while rebasing, thanks for pointing out. Will 
fix it in next version.
>>   	return ret;
>>   }
>>   
>