Received-SPF: pass (google.com: domain of linux-wireless+bounces-459-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <2337a414-4aa2-47ad-8cc4-a631de15ea6f@quicinc.com>
Date: Wed, 6 Dec 2023 13:34:21 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v8 12/12] wifi: ath11k: discard BSS_CHANGED_TXPOWER when
 EXT_TPC_REG_SUPPORT for 6 GHz
To: Aditya Kumar Singh <quic_adisi@quicinc.com>, <ath11k@lists.infradead.org>
CC: <linux-wireless@vger.kernel.org>
References: <20231204081323.5582-1-quic_bqiang@quicinc.com>
 <20231204081323.5582-13-quic_bqiang@quicinc.com>
 <33d629b1-674e-42c1-8969-18fa636d370c@quicinc.com>
Content-Language: en-US
From: Baochen Qiang <quic_bqiang@quicinc.com>
In-Reply-To: <33d629b1-674e-42c1-8969-18fa636d370c@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit


On 12/4/2023 11:53 PM, Aditya Kumar Singh wrote:
> On 12/4/23 13:43, Baochen Qiang wrote:
>> From: Wen Gong <quic_wgong@quicinc.com>
>>
>> When station is connected to a 6 GHz AP, it has 2 ways to configure
>> the power limit to firmware. The first way is to send 2 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.
>>
>> 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 and select the second way.
>>
>> 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>
>> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
>> ---
> ...snip...
>> @@ -3596,9 +3608,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);
>> +        }
> 
> Could you check v6 once? I remember Wen told he would drop this check 
> and let FW take the min value. If we do like this, then user could not 
> set his own desired value even if that is well inside the reg limits.
I did notice this comment in V6, but came out of a different opinion: it 
is OK to discard the TX power here, because that will be sent to 
firmware using WMI_VDEV_SET_TPC_POWER_CMDID command in another patch. 
Please correct me if wrong.

>