| Message ID | 20230912051857.2284-4-quic_adisi@quicinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 77f1ee6fd8b6e470f721d05a2e269039d5cafcb7 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: ath11k: fix CAC running state | expand |
On 9/11/2023 10:18 PM, Aditya Kumar Singh wrote: > Tx power is fetched from firmware's pdev stats. However, during active > CAC, firmware does not fill the current Tx power and sends the max > initialised value filled during firmware init. If host sends this power > to user space, this is wrong since in certain situations, the Tx power > could be greater than the max allowed by the regulatory. Hence, host > should not be fetching the Tx power during an active CAC. > > Fix this issue by returning Tx power as 0 during active CAC since it > is known that during CAC, there will be no transmission happening. > > Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1 > > Fixes: 9a2aa68afe3d ("wifi: ath11k: add get_txpower mac ops") > Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> > --- > drivers/net/wireless/ath/ath11k/mac.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c > index 6b5f032197ff..a36698688f89 100644 > --- a/drivers/net/wireless/ath/ath11k/mac.c > +++ b/drivers/net/wireless/ath/ath11k/mac.c > @@ -9045,6 +9045,14 @@ static int ath11k_mac_op_get_txpower(struct ieee80211_hw *hw, > if (ar->state != ATH11K_STATE_ON) > goto err_fallback; > > + /* Firmware doesn't provide Tx power during CAC hence no need to fetch > + * the stats. > + */ > + if (test_bit(ATH11K_CAC_RUNNING, &ar->dev_flags)) { > + mutex_unlock(&ar->conf_mutex); > + return -EAGAIN; > + } > + > req_param.pdev_id = ar->pdev->pdev_id; > req_param.stats_id = WMI_REQUEST_PDEV_STAT; >
Aditya Kumar Singh <quic_adisi@quicinc.com> wrote: > Tx power is fetched from firmware's pdev stats. However, during active > CAC, firmware does not fill the current Tx power and sends the max > initialised value filled during firmware init. If host sends this power > to user space, this is wrong since in certain situations, the Tx power > could be greater than the max allowed by the regulatory. Hence, host > should not be fetching the Tx power during an active CAC. > > Fix this issue by returning Tx power as 0 during active CAC since it > is known that during CAC, there will be no transmission happening. The returning as 0 doesn't seem to match the code. Should I change the sentence to: "Fix this issue by returning -EAGAIN error so that the user space knows there's no value available right now."
On 10/3/23 20:01, Kalle Valo wrote: > Aditya Kumar Singh <quic_adisi@quicinc.com> wrote: > >> Tx power is fetched from firmware's pdev stats. However, during active >> CAC, firmware does not fill the current Tx power and sends the max >> initialised value filled during firmware init. If host sends this power >> to user space, this is wrong since in certain situations, the Tx power >> could be greater than the max allowed by the regulatory. Hence, host >> should not be fetching the Tx power during an active CAC. >> >> Fix this issue by returning Tx power as 0 during active CAC since it >> is known that during CAC, there will be no transmission happening. > > The returning as 0 doesn't seem to match the code. Should I change the sentence to: > > "Fix this issue by returning -EAGAIN error so that the user space knows there's > no value available right now." Oops. Looks like only in commit message its still zero. Its changed to return -EAGAIN in code. + if (test_bit(ATH11K_CAC_RUNNING, &ar->dev_flags)) { + mutex_unlock(&ar->conf_mutex); + return -EAGAIN; + } So could you just rectify while applying or should I resend?
On 10/4/23 10:24, Kalle Valo wrote: > Aditya Kumar Singh <quic_adisi@quicinc.com> writes: > >> On 10/3/23 20:01, Kalle Valo wrote: >>> Aditya Kumar Singh <quic_adisi@quicinc.com> wrote: >>> >>>> Tx power is fetched from firmware's pdev stats. However, during active >>>> CAC, firmware does not fill the current Tx power and sends the max >>>> initialised value filled during firmware init. If host sends this power >>>> to user space, this is wrong since in certain situations, the Tx power >>>> could be greater than the max allowed by the regulatory. Hence, host >>>> should not be fetching the Tx power during an active CAC. >>>> >>>> Fix this issue by returning Tx power as 0 during active CAC since it >>>> is known that during CAC, there will be no transmission happening. >>> The returning as 0 doesn't seem to match the code. Should I change >>> the sentence to: >>> "Fix this issue by returning -EAGAIN error so that the user space >>> knows there's >>> no value available right now." >> Oops. Looks like only in commit message its still zero. Its changed to >> return -EAGAIN in code. >> >> + if (test_bit(ATH11K_CAC_RUNNING, &ar->dev_flags)) { >> + mutex_unlock(&ar->conf_mutex); >> + return -EAGAIN; >> + } >> >> So could you just rectify while applying or should I resend? > > No need to resend because of this. I changed the commit message now to > this in the pending branch: > > https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6eacc3b5a70ab3f92f9410839870edbb21c9d051Sure, thanks!
Aditya Kumar Singh <quic_adisi@quicinc.com> writes: > On 10/3/23 20:01, Kalle Valo wrote: >> Aditya Kumar Singh <quic_adisi@quicinc.com> wrote: >> >>> Tx power is fetched from firmware's pdev stats. However, during active >>> CAC, firmware does not fill the current Tx power and sends the max >>> initialised value filled during firmware init. If host sends this power >>> to user space, this is wrong since in certain situations, the Tx power >>> could be greater than the max allowed by the regulatory. Hence, host >>> should not be fetching the Tx power during an active CAC. >>> >>> Fix this issue by returning Tx power as 0 during active CAC since it >>> is known that during CAC, there will be no transmission happening. >> The returning as 0 doesn't seem to match the code. Should I change >> the sentence to: >> "Fix this issue by returning -EAGAIN error so that the user space >> knows there's >> no value available right now." > Oops. Looks like only in commit message its still zero. Its changed to > return -EAGAIN in code. > > + if (test_bit(ATH11K_CAC_RUNNING, &ar->dev_flags)) { > + mutex_unlock(&ar->conf_mutex); > + return -EAGAIN; > + } > > So could you just rectify while applying or should I resend? No need to resend because of this. I changed the commit message now to this in the pending branch: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6eacc3b5a70ab3f92f9410839870edbb21c9d051
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c index 6b5f032197ff..a36698688f89 100644 --- a/drivers/net/wireless/ath/ath11k/mac.c +++ b/drivers/net/wireless/ath/ath11k/mac.c @@ -9045,6 +9045,14 @@ static int ath11k_mac_op_get_txpower(struct ieee80211_hw *hw, if (ar->state != ATH11K_STATE_ON) goto err_fallback; + /* Firmware doesn't provide Tx power during CAC hence no need to fetch + * the stats. + */ + if (test_bit(ATH11K_CAC_RUNNING, &ar->dev_flags)) { + mutex_unlock(&ar->conf_mutex); + return -EAGAIN; + } + req_param.pdev_id = ar->pdev->pdev_id; req_param.stats_id = WMI_REQUEST_PDEV_STAT;
Tx power is fetched from firmware's pdev stats. However, during active CAC, firmware does not fill the current Tx power and sends the max initialised value filled during firmware init. If host sends this power to user space, this is wrong since in certain situations, the Tx power could be greater than the max allowed by the regulatory. Hence, host should not be fetching the Tx power during an active CAC. Fix this issue by returning Tx power as 0 during active CAC since it is known that during CAC, there will be no transmission happening. Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1 Fixes: 9a2aa68afe3d ("wifi: ath11k: add get_txpower mac ops") Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com> --- drivers/net/wireless/ath/ath11k/mac.c | 8 ++++++++ 1 file changed, 8 insertions(+)