[PATCH 1/2] wcn36xx: Correct band/freq reporting on RX

[PATCH 1/2] wcn36xx: Correct band/freq reporting on RX

[Loic Poulain]

For packets originating from hardware scan, the channel and band is
included in the buffer descriptor (bd->rf_band & bd->rx_ch).

For 2Ghz band the channel value is directly reported in the 4-bit
rx_ch field. For 5Ghz band, the rx_ch field contains a mapping
index (given the 4-bit limitation).

The reserved0 value field is also used to extend 4-bit mapping to
5-bit mapping to support more than 16 5Ghz channels.

This change adds correct reporting of the frequency/band, that is
used in scan mechanism. And is required for 5Ghz hardware scan
support.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/txrx.c | 23 +++++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/txrx.h |  3 ++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/txrx.c b/drivers/net/wireless/ath/wcn36xx/txrx.c
index cab196b..a3ef497 100644
--- a/drivers/net/wireless/ath/wcn36xx/txrx.c
+++ b/drivers/net/wireless/ath/wcn36xx/txrx.c
@@ -31,6 +31,13 @@ struct wcn36xx_rate {
 	enum rate_info_bw bw;
 };
 
+/* Buffer descriptor rx_ch field is limited to 5-bit (4+1), a mapping is used
+ * for 11A Channels.
+ */
+static const u8 ab_rx_ch_map[] = { 36, 40, 44, 48, 52, 56, 60, 64, 100, 104,
+				   108, 112, 116, 120, 124, 128, 132, 136, 140,
+				   149, 153, 157, 161, 165, 144 };
+
 static const struct wcn36xx_rate wcn36xx_rate_table[] = {
 	/* 11b rates */
 	{  10, 0, RX_ENC_LEGACY, 0, RATE_INFO_BW_20 },
@@ -291,6 +298,22 @@ 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)) {
diff --git a/drivers/net/wireless/ath/wcn36xx/txrx.h b/drivers/net/wireless/ath/wcn36xx/txrx.h
index 032216e..b54311f 100644
--- a/drivers/net/wireless/ath/wcn36xx/txrx.h
+++ b/drivers/net/wireless/ath/wcn36xx/txrx.h
@@ -110,7 +110,8 @@ struct wcn36xx_rx_bd {
 	/* 0x44 */
 	u32	exp_seq_num:12;
 	u32	cur_seq_num:12;
-	u32	fr_type_subtype:8;
+	u32	rf_band:2;
+	u32	fr_type_subtype:6;
 
 	/* 0x48 */
 	u32	msdu_size:16;
-- 
2.7.4

[PATCH 2/2] wcn36xx: Enable hardware scan offload for 5Ghz band

[Loic Poulain]

Until now, offload scanning for 5Ghz channels was considered broken.
However it was mostly a driver issue, caused by bad reporting of the
beacons/probe-resp bands and frequencies, which has been fixed.

We can now allow offload scan for 5GHz band, this reduces the scanning
time comparing to software driven scanning.

Note that offloaded scan is limited to 48 channels, check for this.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 263af65..2ac8efa 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -659,19 +659,19 @@ static int wcn36xx_hw_scan(struct ieee80211_hw *hw,
 			   struct ieee80211_scan_request *hw_req)
 {
 	struct wcn36xx *wcn = hw->priv;
-	int i;
 
 	if (!get_feat_caps(wcn->fw_feat_caps, SCAN_OFFLOAD)) {
 		/* fallback to mac80211 software scan */
 		return 1;
 	}
 
-	/* For unknown reason, the hardware offloaded scan only works with
-	 * 2.4Ghz channels, fallback to software scan in other cases.
+	/* Firmware scan offload is limited to 48 channels, fallback to
+	 * software driven scanning otherwise.
 	 */
-	for (i = 0; i < hw_req->req.n_channels; i++) {
-		if (hw_req->req.channels[i]->band != NL80211_BAND_2GHZ)
-			return 1;
+	if (hw_req->req.n_channels > 48) {
+		wcn36xx_warn("Offload scan aborted, n_channels=%u",
+			     hw_req->req.n_channels);
+		return 1;
 	}
 
 	mutex_lock(&wcn->scan_lock);
-- 
2.7.4

Re: [PATCH 1/2] wcn36xx: Correct band/freq reporting on RX

[Bryan O'Donoghue]

On 18/10/2021 11:57, Loic Poulain wrote:
>   	    ieee80211_is_probe_resp(hdr->frame_control))
>   		status.boottime_ns = ktime_get_boottime_ns();

I think this is dangling in your tree, doesn't apply cleanly for me anyway

Other than that

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH 2/2] wcn36xx: Enable hardware scan offload for 5Ghz band

[Bryan O'Donoghue]

On 18/10/2021 11:57, Loic Poulain wrote:
> Until now, offload scanning for 5Ghz channels was considered broken.
> However it was mostly a driver issue, caused by bad reporting of the
> beacons/probe-resp bands and frequencies, which has been fixed.
> 
> We can now allow offload scan for 5GHz band, this reduces the scanning
> time comparing to software driven scanning.
> 
> Note that offloaded scan is limited to 48 channels, check for this.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>   drivers/net/wireless/ath/wcn36xx/main.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index 263af65..2ac8efa 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -659,19 +659,19 @@ static int wcn36xx_hw_scan(struct ieee80211_hw *hw,
>   			   struct ieee80211_scan_request *hw_req)
>   {
>   	struct wcn36xx *wcn = hw->priv;
> -	int i;
>   
>   	if (!get_feat_caps(wcn->fw_feat_caps, SCAN_OFFLOAD)) {
>   		/* fallback to mac80211 software scan */
>   		return 1;
>   	}
>   
> -	/* For unknown reason, the hardware offloaded scan only works with
> -	 * 2.4Ghz channels, fallback to software scan in other cases.
> +	/* Firmware scan offload is limited to 48 channels, fallback to
> +	 * software driven scanning otherwise.
>   	 */
> -	for (i = 0; i < hw_req->req.n_channels; i++) {
> -		if (hw_req->req.channels[i]->band != NL80211_BAND_2GHZ)
> -			return 1;
> +	if (hw_req->req.n_channels > 48) {
> +		wcn36xx_warn("Offload scan aborted, n_channels=%u",
> +			     hw_req->req.n_channels);
> +		return 1;
>   	}
>   
>   	mutex_lock(&wcn->scan_lock);
> 

Really sweet fix, good job

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH 1/2] wcn36xx: Correct band/freq reporting on RX

[Bryan O'Donoghue]

On 18/10/2021 23:51, Bryan O'Donoghue wrote:
> On 18/10/2021 11:57, Loic Poulain wrote:
>>           ieee80211_is_probe_resp(hdr->frame_control))
>>           status.boottime_ns = ktime_get_boottime_ns();
> 
> I think this is dangling in your tree, doesn't apply cleanly for me anyway
> 
> Other than that
> 
> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Hmm.

I'm told by a colleague with access to a router that has channel 144 - I 
do not BTW - that 144 is not showing up with the firmware offload scan.

We should probably hold off on applying for the time being :(

---
bod

Re: [PATCH 1/2] wcn36xx: Correct band/freq reporting on RX

[Loic Poulain]

Hi Bryan, Kalle,

On Tue, 19 Oct 2021 at 02:26, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 18/10/2021 23:51, Bryan O'Donoghue wrote:
> > On 18/10/2021 11:57, Loic Poulain wrote:
> >>           ieee80211_is_probe_resp(hdr->frame_control))
> >>           status.boottime_ns = ktime_get_boottime_ns();
> >
> > I think this is dangling in your tree, doesn't apply cleanly for me anyway
> >
> > Other than that
> >
> > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>
> Hmm.
>
> I'm told by a colleague with access to a router that has channel 144 - I
> do not BTW - that 144 is not showing up with the firmware offload scan.
>
> We should probably hold off on applying for the time being :(

So the missing channel 144 is due to a different problem, and is now fixed
in a subsequent patch:
    wcn36xx: Channel list update before hardware scan

So I think we can go with this change :-).

Regards,
Loic

Re: [PATCH 1/2] wcn36xx: Correct band/freq reporting on RX

[Bryan O'Donoghue]

On 20/10/2021 14:54, Loic Poulain wrote:
> Hi Bryan, Kalle,
> 
> On Tue, 19 Oct 2021 at 02:26, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>> On 18/10/2021 23:51, Bryan O'Donoghue wrote:
>>> On 18/10/2021 11:57, Loic Poulain wrote:
>>>>            ieee80211_is_probe_resp(hdr->frame_control))
>>>>            status.boottime_ns = ktime_get_boottime_ns();
>>>
>>> I think this is dangling in your tree, doesn't apply cleanly for me anyway
>>>
>>> Other than that
>>>
>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>
>> Hmm.
>>
>> I'm told by a colleague with access to a router that has channel 144 - I
>> do not BTW - that 144 is not showing up with the firmware offload scan.
>>
>> We should probably hold off on applying for the time being :(
> 
> So the missing channel 144 is due to a different problem, and is now fixed
> in a subsequent patch:
>      wcn36xx: Channel list update before hardware scan
> 
> So I think we can go with this change :-).
> 
> Regards,
> Loic
> 

Cool, nice job

Re: [PATCH 1/2] wcn36xx: Correct band/freq reporting on RX

[Kalle Valo]

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> On 20/10/2021 14:54, Loic Poulain wrote:
>> Hi Bryan, Kalle,
>>
>> On Tue, 19 Oct 2021 at 02:26, Bryan O'Donoghue
>> <bryan.odonoghue@linaro.org> wrote:
>>>
>>> On 18/10/2021 23:51, Bryan O'Donoghue wrote:
>>>> On 18/10/2021 11:57, Loic Poulain wrote:
>>>>>            ieee80211_is_probe_resp(hdr->frame_control))
>>>>>            status.boottime_ns = ktime_get_boottime_ns();
>>>>
>>>> I think this is dangling in your tree, doesn't apply cleanly for me anyway
>>>>
>>>> Other than that
>>>>
>>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>>
>>> Hmm.
>>>
>>> I'm told by a colleague with access to a router that has channel 144 - I
>>> do not BTW - that 144 is not showing up with the firmware offload scan.
>>>
>>> We should probably hold off on applying for the time being :(
>>
>> So the missing channel 144 is due to a different problem, and is now fixed
>> in a subsequent patch:
>>      wcn36xx: Channel list update before hardware scan
>>
>> So I think we can go with this change :-).
>>
>> Regards,
>> Loic
>>
>
> Cool, nice job

I had already dropped this patch from my queue, but added it back now.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

Re: [PATCH 1/2] wcn36xx: Correct band/freq reporting on RX

[Kalle Valo]

Loic Poulain <loic.poulain@linaro.org> wrote:

> For packets originating from hardware scan, the channel and band is
> included in the buffer descriptor (bd->rf_band & bd->rx_ch).
> 
> For 2Ghz band the channel value is directly reported in the 4-bit
> rx_ch field. For 5Ghz band, the rx_ch field contains a mapping
> index (given the 4-bit limitation).
> 
> The reserved0 value field is also used to extend 4-bit mapping to
> 5-bit mapping to support more than 16 5Ghz channels.
> 
> This change adds correct reporting of the frequency/band, that is
> used in scan mechanism. And is required for 5Ghz hardware scan
> support.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

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

8a27ca394782 wcn36xx: Correct band/freq reporting on RX
2371b15f8eeb wcn36xx: Enable hardware scan offload for 5Ghz band

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/1634554678-7993-1-git-send-email-loic.poulain@linaro.org/

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