Received-SPF: pass (google.com: domain of linux-wireless+bounces-5619-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <c26cf3e5-16c4-430c-ad9e-be20c10bb570@quicinc.com>
Date: Fri, 29 Mar 2024 11:20:45 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 5/6] wifi: ath12k: refactor smps configuration
Content-Language: en-US
To: Pradeep Kumar Chitrapu <quic_pradeepc@quicinc.com>,
        <ath12k@lists.infradead.org>
CC: <linux-wireless@vger.kernel.org>
References: <20240329173926.17741-1-quic_pradeepc@quicinc.com>
 <20240329173926.17741-6-quic_pradeepc@quicinc.com>
From: Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <20240329173926.17741-6-quic_pradeepc@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit

On 3/29/2024 10:39 AM, Pradeep Kumar Chitrapu wrote:
> Move fetching smps value to a new function and use u16_get_bits

s/smps/SMPS/ here and in the next line

> to extract smps value from capabilities. This will help in
> extending the functionality when SMPS support in 6 GHz band gets
> added in subsequent patches.
> 
> 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: Pradeep Kumar Chitrapu <quic_pradeepc@quicinc.com>
> ---
>  drivers/net/wireless/ath/ath12k/mac.c | 32 ++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 7e296140439a..b4114dd22bf0 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -1963,18 +1963,29 @@ static void ath12k_peer_assoc_h_he_6ghz(struct ath12k *ar,
>  				     ampdu_factor)) - 1;
>  }
>  
> +static inline int ath12k_get_smps_from_capa(const struct ieee80211_sta_ht_cap *ht_cap,

remove inline
since it is static the compiler will inline it if it makes sense to do so

(doesn't that fix the ath12k-check issue in the 6/6 patch)?

> +					    int *smps)
> +{
> +	if (!ht_cap->ht_supported)
> +		return -EOPNOTSUPP;
> +
> +	*smps = u16_get_bits(ht_cap->cap, IEEE80211_HT_CAP_SM_PS);
> +
> +	if (*smps >= ARRAY_SIZE(ath12k_smps_map))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
>  static void ath12k_peer_assoc_h_smps(struct ieee80211_sta *sta,
>  				     struct ath12k_wmi_peer_assoc_arg *arg)
>  {
>  	const struct ieee80211_sta_ht_cap *ht_cap = &sta->deflink.ht_cap;
>  	int smps;
>  
> -	if (!ht_cap->ht_supported)
> +	if (ath12k_get_smps_from_capa(ht_cap, &smps))
>  		return;
>  
> -	smps = ht_cap->cap & IEEE80211_HT_CAP_SM_PS;
> -	smps >>= IEEE80211_HT_CAP_SM_PS_SHIFT;
> -
>  	switch (smps) {
>  	case WLAN_HT_CAP_SM_PS_STATIC:
>  		arg->static_mimops_flag = true;
> @@ -2448,16 +2459,11 @@ static int ath12k_setup_peer_smps(struct ath12k *ar, struct ath12k_vif *arvif,
>  				  const u8 *addr,
>  				  const struct ieee80211_sta_ht_cap *ht_cap)
>  {
> -	int smps;
> +	int smps, ret = 0;
>  
> -	if (!ht_cap->ht_supported)
> -		return 0;
> -
> -	smps = ht_cap->cap & IEEE80211_HT_CAP_SM_PS;
> -	smps >>= IEEE80211_HT_CAP_SM_PS_SHIFT;
> -
> -	if (smps >= ARRAY_SIZE(ath12k_smps_map))
> -		return -EINVAL;
> +	ret = ath12k_get_smps_from_capa(ht_cap, &smps);
> +	if (ret < 0)
> +		return ret;
>  
>  	return ath12k_wmi_set_peer_param(ar, addr, arvif->vdev_id,
>  					 WMI_PEER_MIMO_PS_STATE,