linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] wcn36xx: populate band before determining rate on RX
@ 2021-11-04  1:05 Benjamin Li
  2021-11-04  1:05 ` [PATCH v3 1/2] " Benjamin Li
  2021-11-04  1:05 ` [PATCH v3 2/2] wcn36xx: fix RX BD rate mapping for 5GHz legacy rates Benjamin Li
  0 siblings, 2 replies; 6+ messages in thread
From: Benjamin Li @ 2021-11-04  1:05 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Bryan O'Donoghue, Loic Poulain, linux-arm-msm, Benjamin Li,
	David S. Miller, Jakub Kicinski, wcn36xx, linux-wireless, netdev,
	linux-kernel

v3:
Tweak commit message of patch 1 (probe response -> beacon/probe response).

Check for rate_idx >= 4 in patch 2, per Bryan's observation and Loic's
confirmation that FW sometimes sends rate_idx = 0 for 5GHz legacy rate
frames.

No warn per feedback from Kalle, since this a confirmed FW bug & logging
could be spammy.

v2:
Fix unused variable warning.

Benjamin Li (2):
  wcn36xx: populate band before determining rate on RX
  wcn36xx: fix RX BD rate mapping for 5GHz legacy rates

 drivers/net/wireless/ath/wcn36xx/txrx.c | 41 ++++++++++++-------------
 1 file changed, 20 insertions(+), 21 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] wcn36xx: populate band before determining rate on RX
  2021-11-04  1:05 [PATCH v3 0/2] wcn36xx: populate band before determining rate on RX Benjamin Li
@ 2021-11-04  1:05 ` Benjamin Li
  2021-11-04 18:57   ` Loic Poulain
  2021-11-08 13:23   ` Kalle Valo
  2021-11-04  1:05 ` [PATCH v3 2/2] wcn36xx: fix RX BD rate mapping for 5GHz legacy rates Benjamin Li
  1 sibling, 2 replies; 6+ messages in thread
From: Benjamin Li @ 2021-11-04  1:05 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Bryan O'Donoghue, Loic Poulain, linux-arm-msm, Benjamin Li,
	David S. Miller, Jakub Kicinski, wcn36xx, linux-wireless, netdev,
	linux-kernel

status.band is used in determination of status.rate -- for 5GHz on legacy
rates there is a linear shift between the BD descriptor's rate field and
the wcn36xx driver's rate table (wcn_5ghz_rates).

We have a special clause to populate status.band for hardware scan offload
frames. However, this block occurs after status.rate is already populated.
Correctly handle this dependency by moving the band block before the rate
block.

This patch addresses kernel warnings & missing scan results for 5GHz APs
that send their beacons/probe responses at the higher four legacy rates
(24-54 Mbps), when using hardware scan offload:

  ------------[ cut here ]------------
  WARNING: CPU: 0 PID: 0 at net/mac80211/rx.c:4532 ieee80211_rx_napi+0x744/0x8d8
  Modules linked in: wcn36xx [...]
  CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W         4.19.107-g73909fa #1
  Hardware name: Square, Inc. T2 (all variants) (DT)
  Call trace:
  dump_backtrace+0x0/0x148
  show_stack+0x14/0x1c
  dump_stack+0xb8/0xf0
  __warn+0x2ac/0x2d8
  warn_slowpath_null+0x44/0x54
  ieee80211_rx_napi+0x744/0x8d8
  ieee80211_tasklet_handler+0xa4/0xe0
  tasklet_action_common+0xe0/0x118
  tasklet_action+0x20/0x28
  __do_softirq+0x108/0x1ec
  irq_exit+0xd4/0xd8
  __handle_domain_irq+0x84/0xbc
  gic_handle_irq+0x4c/0xb8
  el1_irq+0xe8/0x190
  lpm_cpuidle_enter+0x220/0x260
  cpuidle_enter_state+0x114/0x1c0
  cpuidle_enter+0x34/0x48
  do_idle+0x150/0x268
  cpu_startup_entry+0x20/0x24
  rest_init+0xd4/0xe0
  start_kernel+0x398/0x430
  ---[ end trace ae28cb759352b403 ]---

Fixes: 8a27ca394782 ("wcn36xx: Correct band/freq reporting on RX")
Signed-off-by: Benjamin Li <benl@squareup.com>
---
 drivers/net/wireless/ath/wcn36xx/txrx.c | 37 +++++++++++++------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/txrx.c b/drivers/net/wireless/ath/wcn36xx/txrx.c
index 75951ccbc840e..f0a9f069a92a9 100644
--- a/drivers/net/wireless/ath/wcn36xx/txrx.c
+++ b/drivers/net/wireless/ath/wcn36xx/txrx.c
@@ -314,8 +314,6 @@ int wcn36xx_rx_skb(struct wcn36xx *wcn, struct sk_buff *skb)
 	fc = __le16_to_cpu(hdr->frame_control);
 	sn = IEEE80211_SEQ_TO_SN(__le16_to_cpu(hdr->seq_ctrl));
 
-	status.freq = WCN36XX_CENTER_FREQ(wcn);
-	status.band = WCN36XX_BAND(wcn);
 	status.mactime = 10;
 	status.signal = -get_rssi0(bd);
 	status.antenna = 1;
@@ -327,6 +325,25 @@ int wcn36xx_rx_skb(struct wcn36xx *wcn, struct sk_buff *skb)
 
 	wcn36xx_dbg(WCN36XX_DBG_RX, "status.flags=%x\n", status.flag);
 
+	if (bd->scan_learn) {
+		/* If packet originate from hardware scanning, extract the
+		 * band/channel from bd descriptor.
+		 */
+		u8 hwch = (bd->reserved0 << 4) + bd->rx_ch;
+
+		if (bd->rf_band != 1 && hwch <= sizeof(ab_rx_ch_map) && hwch >= 1) {
+			status.band = NL80211_BAND_5GHZ;
+			status.freq = ieee80211_channel_to_frequency(ab_rx_ch_map[hwch - 1],
+								     status.band);
+		} else {
+			status.band = NL80211_BAND_2GHZ;
+			status.freq = ieee80211_channel_to_frequency(hwch, status.band);
+		}
+	} else {
+		status.band = WCN36XX_BAND(wcn);
+		status.freq = WCN36XX_CENTER_FREQ(wcn);
+	}
+
 	if (bd->rate_id < ARRAY_SIZE(wcn36xx_rate_table)) {
 		rate = &wcn36xx_rate_table[bd->rate_id];
 		status.encoding = rate->encoding;
@@ -353,22 +370,6 @@ int wcn36xx_rx_skb(struct wcn36xx *wcn, struct sk_buff *skb)
 	    ieee80211_is_probe_resp(hdr->frame_control))
 		status.boottime_ns = ktime_get_boottime_ns();
 
-	if (bd->scan_learn) {
-		/* If packet originates from hardware scanning, extract the
-		 * band/channel from bd descriptor.
-		 */
-		u8 hwch = (bd->reserved0 << 4) + bd->rx_ch;
-
-		if (bd->rf_band != 1 && hwch <= sizeof(ab_rx_ch_map) && hwch >= 1) {
-			status.band = NL80211_BAND_5GHZ;
-			status.freq = ieee80211_channel_to_frequency(ab_rx_ch_map[hwch - 1],
-								     status.band);
-		} else {
-			status.band = NL80211_BAND_2GHZ;
-			status.freq = ieee80211_channel_to_frequency(hwch, status.band);
-		}
-	}
-
 	memcpy(IEEE80211_SKB_RXCB(skb), &status, sizeof(status));
 
 	if (ieee80211_is_beacon(hdr->frame_control)) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] wcn36xx: fix RX BD rate mapping for 5GHz legacy rates
  2021-11-04  1:05 [PATCH v3 0/2] wcn36xx: populate band before determining rate on RX Benjamin Li
  2021-11-04  1:05 ` [PATCH v3 1/2] " Benjamin Li
@ 2021-11-04  1:05 ` Benjamin Li
  2021-11-04 18:57   ` Loic Poulain
  1 sibling, 1 reply; 6+ messages in thread
From: Benjamin Li @ 2021-11-04  1:05 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Bryan O'Donoghue, Loic Poulain, linux-arm-msm, Benjamin Li,
	David S. Miller, Jakub Kicinski, wcn36xx, linux-wireless, netdev,
	linux-kernel

The linear mapping between the BD rate field and the driver's 5GHz
legacy rates table (wcn_5ghz_rates) does not only apply for the latter
four rates -- it applies to all eight rates.

Fixes: 6ea131acea98 ("wcn36xx: Fix warning due to bad rate_idx")
Signed-off-by: Benjamin Li <benl@squareup.com>
---
 drivers/net/wireless/ath/wcn36xx/txrx.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/txrx.c b/drivers/net/wireless/ath/wcn36xx/txrx.c
index f0a9f069a92a9..dd58dde8c8363 100644
--- a/drivers/net/wireless/ath/wcn36xx/txrx.c
+++ b/drivers/net/wireless/ath/wcn36xx/txrx.c
@@ -272,7 +272,6 @@ int wcn36xx_rx_skb(struct wcn36xx *wcn, struct sk_buff *skb)
 	const struct wcn36xx_rate *rate;
 	struct ieee80211_hdr *hdr;
 	struct wcn36xx_rx_bd *bd;
-	struct ieee80211_supported_band *sband;
 	u16 fc, sn;
 
 	/*
@@ -350,12 +349,11 @@ int wcn36xx_rx_skb(struct wcn36xx *wcn, struct sk_buff *skb)
 		status.enc_flags = rate->encoding_flags;
 		status.bw = rate->bw;
 		status.rate_idx = rate->mcs_or_legacy_index;
-		sband = wcn->hw->wiphy->bands[status.band];
 		status.nss = 1;
 
 		if (status.band == NL80211_BAND_5GHZ &&
 		    status.encoding == RX_ENC_LEGACY &&
-		    status.rate_idx >= sband->n_bitrates) {
+		    status.rate_idx >= 4) {
 			/* no dsss rates in 5Ghz rates table */
 			status.rate_idx -= 4;
 		}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] wcn36xx: populate band before determining rate on RX
  2021-11-04  1:05 ` [PATCH v3 1/2] " Benjamin Li
@ 2021-11-04 18:57   ` Loic Poulain
  2021-11-08 13:23   ` Kalle Valo
  1 sibling, 0 replies; 6+ messages in thread
From: Loic Poulain @ 2021-11-04 18:57 UTC (permalink / raw)
  To: Benjamin Li
  Cc: Kalle Valo, Bryan O'Donoghue, linux-arm-msm, David S. Miller,
	Jakub Kicinski, wcn36xx, linux-wireless, netdev, linux-kernel

On Thu, 4 Nov 2021 at 02:05, Benjamin Li <benl@squareup.com> wrote:
>
> status.band is used in determination of status.rate -- for 5GHz on legacy
> rates there is a linear shift between the BD descriptor's rate field and
> the wcn36xx driver's rate table (wcn_5ghz_rates).
>
> We have a special clause to populate status.band for hardware scan offload
> frames. However, this block occurs after status.rate is already populated.
> Correctly handle this dependency by moving the band block before the rate
> block.
>
> This patch addresses kernel warnings & missing scan results for 5GHz APs
> that send their beacons/probe responses at the higher four legacy rates
> (24-54 Mbps), when using hardware scan offload:
>
>   ------------[ cut here ]------------
>   WARNING: CPU: 0 PID: 0 at net/mac80211/rx.c:4532 ieee80211_rx_napi+0x744/0x8d8
>   Modules linked in: wcn36xx [...]
>   CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W         4.19.107-g73909fa #1
>   Hardware name: Square, Inc. T2 (all variants) (DT)
>   Call trace:
>   dump_backtrace+0x0/0x148
>   show_stack+0x14/0x1c
>   dump_stack+0xb8/0xf0
>   __warn+0x2ac/0x2d8
>   warn_slowpath_null+0x44/0x54
>   ieee80211_rx_napi+0x744/0x8d8
>   ieee80211_tasklet_handler+0xa4/0xe0
>   tasklet_action_common+0xe0/0x118
>   tasklet_action+0x20/0x28
>   __do_softirq+0x108/0x1ec
>   irq_exit+0xd4/0xd8
>   __handle_domain_irq+0x84/0xbc
>   gic_handle_irq+0x4c/0xb8
>   el1_irq+0xe8/0x190
>   lpm_cpuidle_enter+0x220/0x260
>   cpuidle_enter_state+0x114/0x1c0
>   cpuidle_enter+0x34/0x48
>   do_idle+0x150/0x268
>   cpu_startup_entry+0x20/0x24
>   rest_init+0xd4/0xe0
>   start_kernel+0x398/0x430
>   ---[ end trace ae28cb759352b403 ]---
>
> Fixes: 8a27ca394782 ("wcn36xx: Correct band/freq reporting on RX")
> Signed-off-by: Benjamin Li <benl@squareup.com>

Tested-by: Loic Poulain <loic.poulain@linaro.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] wcn36xx: fix RX BD rate mapping for 5GHz legacy rates
  2021-11-04  1:05 ` [PATCH v3 2/2] wcn36xx: fix RX BD rate mapping for 5GHz legacy rates Benjamin Li
@ 2021-11-04 18:57   ` Loic Poulain
  0 siblings, 0 replies; 6+ messages in thread
From: Loic Poulain @ 2021-11-04 18:57 UTC (permalink / raw)
  To: Benjamin Li
  Cc: Kalle Valo, Bryan O'Donoghue, linux-arm-msm, David S. Miller,
	Jakub Kicinski, wcn36xx, linux-wireless, netdev, linux-kernel

On Thu, 4 Nov 2021 at 02:06, Benjamin Li <benl@squareup.com> wrote:
>
> The linear mapping between the BD rate field and the driver's 5GHz
> legacy rates table (wcn_5ghz_rates) does not only apply for the latter
> four rates -- it applies to all eight rates.
>
> Fixes: 6ea131acea98 ("wcn36xx: Fix warning due to bad rate_idx")
> Signed-off-by: Benjamin Li <benl@squareup.com>

Tested-by: Loic Poulain <loic.poulain@linaro.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] wcn36xx: populate band before determining rate on RX
  2021-11-04  1:05 ` [PATCH v3 1/2] " Benjamin Li
  2021-11-04 18:57   ` Loic Poulain
@ 2021-11-08 13:23   ` Kalle Valo
  1 sibling, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2021-11-08 13:23 UTC (permalink / raw)
  To: Benjamin Li
  Cc: Bryan O'Donoghue, Loic Poulain, linux-arm-msm, Benjamin Li,
	David S. Miller, Jakub Kicinski, wcn36xx, linux-wireless, netdev,
	linux-kernel

Benjamin Li <benl@squareup.com> wrote:

> status.band is used in determination of status.rate -- for 5GHz on legacy
> rates there is a linear shift between the BD descriptor's rate field and
> the wcn36xx driver's rate table (wcn_5ghz_rates).
> 
> We have a special clause to populate status.band for hardware scan offload
> frames. However, this block occurs after status.rate is already populated.
> Correctly handle this dependency by moving the band block before the rate
> block.
> 
> This patch addresses kernel warnings & missing scan results for 5GHz APs
> that send their beacons/probe responses at the higher four legacy rates
> (24-54 Mbps), when using hardware scan offload:
> 
>   ------------[ cut here ]------------
>   WARNING: CPU: 0 PID: 0 at net/mac80211/rx.c:4532 ieee80211_rx_napi+0x744/0x8d8
>   Modules linked in: wcn36xx [...]
>   CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W         4.19.107-g73909fa #1
>   Hardware name: Square, Inc. T2 (all variants) (DT)
>   Call trace:
>   dump_backtrace+0x0/0x148
>   show_stack+0x14/0x1c
>   dump_stack+0xb8/0xf0
>   __warn+0x2ac/0x2d8
>   warn_slowpath_null+0x44/0x54
>   ieee80211_rx_napi+0x744/0x8d8
>   ieee80211_tasklet_handler+0xa4/0xe0
>   tasklet_action_common+0xe0/0x118
>   tasklet_action+0x20/0x28
>   __do_softirq+0x108/0x1ec
>   irq_exit+0xd4/0xd8
>   __handle_domain_irq+0x84/0xbc
>   gic_handle_irq+0x4c/0xb8
>   el1_irq+0xe8/0x190
>   lpm_cpuidle_enter+0x220/0x260
>   cpuidle_enter_state+0x114/0x1c0
>   cpuidle_enter+0x34/0x48
>   do_idle+0x150/0x268
>   cpu_startup_entry+0x20/0x24
>   rest_init+0xd4/0xe0
>   start_kernel+0x398/0x430
>   ---[ end trace ae28cb759352b403 ]---
> 
> Fixes: 8a27ca394782 ("wcn36xx: Correct band/freq reporting on RX")
> Signed-off-by: Benjamin Li <benl@squareup.com>
> Tested-by: Loic Poulain <loic.poulain@linaro.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

2 patches applied to ath-next branch of ath.git, thanks.

c9c5608fafe4 wcn36xx: populate band before determining rate on RX
cfdf6b19e750 wcn36xx: fix RX BD rate mapping for 5GHz legacy rates

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20211104010548.1107405-2-benl@squareup.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-08 13:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-04  1:05 [PATCH v3 0/2] wcn36xx: populate band before determining rate on RX Benjamin Li
2021-11-04  1:05 ` [PATCH v3 1/2] " Benjamin Li
2021-11-04 18:57   ` Loic Poulain
2021-11-08 13:23   ` Kalle Valo
2021-11-04  1:05 ` [PATCH v3 2/2] wcn36xx: fix RX BD rate mapping for 5GHz legacy rates Benjamin Li
2021-11-04 18:57   ` Loic Poulain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).