Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Message-ID: <734f2969-4846-4e9b-97f3-00f9891e6922@quicinc.com>
Date:   Thu, 21 Sep 2023 13:39:42 -0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v6 11/13] wifi: ath11k: discard BSS_CHANGED_TXPOWER when
 EXT_TPC_REG_SUPPORT for 6 GHz
Content-Language: en-US
To:     Wen Gong <quic_wgong@quicinc.com>, <ath11k@lists.infradead.org>
CC:     <linux-wireless@vger.kernel.org>, <kvalo@kernel.org>
References: <20230920082349.29111-1-quic_wgong@quicinc.com>
 <20230920082349.29111-12-quic_wgong@quicinc.com>
From:   Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <20230920082349.29111-12-quic_wgong@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 9/20/2023 1:23 AM, Wen Gong wrote:
> When station is connected to a 6 GHz AP, it has 2 way to configure

s/way/ways/

> the power limit to firmware. The first way is to send 2 wmi command

s/wmi command/WMI commands/

> WMI_PDEV_PARAM_TXPOWER_LIMIT2G/WMI_PDEV_PARAM_TXPOWER_LIMIT5G to
> firmware, the second way is to send WMI_VDEV_SET_TPC_POWER_CMDID to
> firmware which include more parameters for power control.

Why would LIMIT2G/LIMIT5G be used for 6 GHz AP?

> 
> When firmware support SERVICE_EXT_TPC_REG, it means firmware support
> the second way for WMI_VDEV_SET_TPC_POWER_CMDID, then ath11k discard
> BSS_CHANGED_TXPOWER flag from mac80211 which is used to the first way
> for 6 GHz band in this patch and select the second way in the subsequent
> patch.

So if i bisect at this patch and if firmware supports 
SERVICE_EXT_TPC_REG, then there will not be any power limits sent to 
firmware?

Ideally all patches in a series should be able to be bisected without 
introducing regressive behavior.

Does it make sense to squash patches 11 and 13 so that there is no 
regression? Patch 12 would then come before the squashed patch.

> 
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23
> 
> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
> ---
>   drivers/net/wireless/ath/ath11k/mac.c | 22 +++++++++++++++++++---
>   1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
> index f05d66913abd..a8ae281d2635 100644
> --- a/drivers/net/wireless/ath/ath11k/mac.c
> +++ b/drivers/net/wireless/ath/ath11k/mac.c
> @@ -3396,6 +3396,18 @@ static int ath11k_mac_config_obss_pd(struct ath11k *ar,
>   	return 0;
>   }
>   
> +static bool ath11k_mac_supports_station_tpc(struct ath11k *ar,
> +					    struct ath11k_vif *arvif,
> +					    const struct cfg80211_chan_def *chandef)
> +{
> +	return ath11k_mac_supports_6ghz_cc_ext(ar) &&
> +		test_bit(WMI_TLV_SERVICE_EXT_TPC_REG_SUPPORT, ar->ab->wmi_ab.svc_map) &&
> +		arvif->vdev_type == WMI_VDEV_TYPE_STA &&
> +		arvif->vdev_subtype == WMI_VDEV_SUBTYPE_NONE &&
> +		chandef->chan &&
> +		chandef->chan->band == NL80211_BAND_6GHZ;
> +}
> +
>   static void ath11k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
>   					   struct ieee80211_vif *vif,
>   					   struct ieee80211_bss_conf *info,
> @@ -3595,9 +3607,13 @@ static void ath11k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
>   	if (changed & BSS_CHANGED_TXPOWER) {
>   		ath11k_dbg(ar->ab, ATH11K_DBG_MAC, "vdev_id %i txpower %d\n",
>   			   arvif->vdev_id, info->txpower);
> -
> -		arvif->txpower = info->txpower;
> -		ath11k_mac_txpower_recalc(ar);
> +		if (ath11k_mac_supports_station_tpc(ar, arvif, &info->chandef)) {
> +			ath11k_dbg(ar->ab, ATH11K_DBG_MAC,
> +				   "discard tx power, change to set TPC power\n");
> +		} else {
> +			arvif->txpower = info->txpower;
> +			ath11k_mac_txpower_recalc(ar);
> +		}
>   	}
>   
>   	if (changed & BSS_CHANGED_PS &&