[PATCH v2 0/5] rtw88: minor fixes from suggestions during review

[PATCH v2 0/5] rtw88: minor fixes from suggestions during review

[yhchuang]

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

The series fix some small problems for rtw88, most of the suggestions
are from the review process.


v1 -> v2

 - modify description for LPS, ", turn off" -> ", to turn off"
 - drop patch "rtw88: mac: remove dangerous while (1)",
   should re-write the power sequence parsing code to make sense of avoiding
   infinite loop
 - unify Makefile license to Dual GPL/BSD


Yan-Hsuan Chuang (5):
  rtw88: add license for Makefile
  rtw88: pci: use ieee80211_ac_numbers instead of 0-3
  rtw88: pci: check if queue mapping exceeds size of ac_to_hwq
  rtw88: fix unassigned rssi_level in rtw_sta_info
  rtw88: more descriptions about LPS

 drivers/net/wireless/realtek/rtw88/Makefile |  2 ++
 drivers/net/wireless/realtek/rtw88/main.c   |  2 +-
 drivers/net/wireless/realtek/rtw88/pci.c    | 10 ++++++----
 drivers/net/wireless/realtek/rtw88/phy.c    |  4 ++--
 4 files changed, 11 insertions(+), 7 deletions(-)

-- 
2.7.4

[PATCH v2 1/5] rtw88: add license for Makefile

[yhchuang]

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

Add missing license for Makefile

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/Makefile b/drivers/net/wireless/realtek/rtw88/Makefile
index da5e36e..e0bfefd 100644
--- a/drivers/net/wireless/realtek/rtw88/Makefile
+++ b/drivers/net/wireless/realtek/rtw88/Makefile
@@ -1,3 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+
 obj-$(CONFIG_RTW88_CORE)	+= rtw88.o
 rtw88-y += main.o \
 	   mac80211.o \
-- 
2.7.4

Re: [PATCH v2 1/5] rtw88: add license for Makefile

[Kalle Valo]

<yhchuang@realtek.com> wrote:

> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> Add missing license for Makefile
> 
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>

Patch applied to wireless-drivers-next.git, thanks.

f9b628d61fae rtw88: add license for Makefile

-- 
https://patchwork.kernel.org/patch/10928421/

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

[PATCH v2 2/5] rtw88: pci: use ieee80211_ac_numbers instead of 0-3

[yhchuang]

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

AC numbers are defined as enum in mac80211, use them instead of bare
0-3 indexing.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/pci.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index cfe05ba..87bfcb3 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -487,10 +487,10 @@ static void rtw_pci_stop(struct rtw_dev *rtwdev)
 }
 
 static u8 ac_to_hwq[] = {
-	[0] = RTW_TX_QUEUE_VO,
-	[1] = RTW_TX_QUEUE_VI,
-	[2] = RTW_TX_QUEUE_BE,
-	[3] = RTW_TX_QUEUE_BK,
+	[IEEE80211_AC_VO] = RTW_TX_QUEUE_VO,
+	[IEEE80211_AC_VI] = RTW_TX_QUEUE_VI,
+	[IEEE80211_AC_BE] = RTW_TX_QUEUE_BE,
+	[IEEE80211_AC_BK] = RTW_TX_QUEUE_BK,
 };
 
 static u8 rtw_hw_queue_mapping(struct sk_buff *skb)
-- 
2.7.4

Re: [PATCH v2 2/5] rtw88: pci: use ieee80211_ac_numbers instead of 0-3

[Kalle Valo]

<yhchuang@realtek.com> wrote:

> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> AC numbers are defined as enum in mac80211, use them instead of bare
> 0-3 indexing.
> 
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>

3 patches applied to wireless-drivers-next.git, thanks.

82dea406c509 rtw88: pci: use ieee80211_ac_numbers instead of 0-3
0d7882950c73 rtw88: pci: check if queue mapping exceeds size of ac_to_hwq
a3b0c66c5928 rtw88: more descriptions about LPS

-- 
https://patchwork.kernel.org/patch/10928423/

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

[PATCH v2 3/5] rtw88: pci: check if queue mapping exceeds size of ac_to_hwq

[yhchuang]

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

Dump warning messages when we get a q_mapping larger than the AC
numbers. And pick BE queue as default.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 87bfcb3..353871c 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -504,6 +504,8 @@ static u8 rtw_hw_queue_mapping(struct sk_buff *skb)
 		queue = RTW_TX_QUEUE_BCN;
 	else if (unlikely(ieee80211_is_mgmt(fc) || ieee80211_is_ctl(fc)))
 		queue = RTW_TX_QUEUE_MGMT;
+	else if (WARN_ON_ONCE(q_mapping >= ARRAY_SIZE(ac_to_hwq)))
+		queue = ac_to_hwq[IEEE80211_AC_BE];
 	else
 		queue = ac_to_hwq[q_mapping];
 
-- 
2.7.4

[PATCH v2 4/5] rtw88: fix unassigned rssi_level in rtw_sta_info

[yhchuang]

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

The new rssi_level should be stored in si, otherwise the rssi_level will
never be updated and get a wrong RA mask, which is calculated by the
rssi level

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/phy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/phy.c b/drivers/net/wireless/realtek/rtw88/phy.c
index 4381b36..7f437e2 100644
--- a/drivers/net/wireless/realtek/rtw88/phy.c
+++ b/drivers/net/wireless/realtek/rtw88/phy.c
@@ -144,10 +144,10 @@ static void rtw_phy_stat_rssi_iter(void *data, struct ieee80211_sta *sta)
 	struct rtw_phy_stat_iter_data *iter_data = data;
 	struct rtw_dev *rtwdev = iter_data->rtwdev;
 	struct rtw_sta_info *si = (struct rtw_sta_info *)sta->drv_priv;
-	u8 rssi, rssi_level;
+	u8 rssi;
 
 	rssi = ewma_rssi_read(&si->avg_rssi);
-	rssi_level = rtw_phy_get_rssi_level(si->rssi_level, rssi);
+	si->rssi_level = rtw_phy_get_rssi_level(si->rssi_level, rssi);
 
 	rtw_fw_send_rssi_info(rtwdev, si);
 
-- 
2.7.4

Re: [PATCH v2 4/5] rtw88: fix unassigned rssi_level in rtw_sta_info

[Kalle Valo]

<yhchuang@realtek.com> writes:

> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
>
> The new rssi_level should be stored in si, otherwise the rssi_level will
> never be updated and get a wrong RA mask, which is calculated by the
> rssi level
>
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>

Stanislaw suggested that this should go to 5.2. So what breaks from
user's point of view if this is not applied?

-- 
Kalle Valo

RE: [PATCH v2 4/5] rtw88: fix unassigned rssi_level in rtw_sta_info

[Tony Chuang]

> -----Original Message-----
> From: Kalle Valo [mailto:kvalo@codeaurora.org]
> Sent: Monday, May 06, 2019 4:49 PM
> To: Tony Chuang
> Cc: linux-wireless@vger.kernel.org
> Subject: Re: [PATCH v2 4/5] rtw88: fix unassigned rssi_level in rtw_sta_info
> 
> <yhchuang@realtek.com> writes:
> 
> > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> >
> > The new rssi_level should be stored in si, otherwise the rssi_level will
> > never be updated and get a wrong RA mask, which is calculated by the
> > rssi level
> >
> > Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> Stanislaw suggested that this should go to 5.2. So what breaks from
> user's point of view if this is not applied?
> 

If the rssi level remains unchanged, then we could choose wrong ra_mask.
And some *bad rates* we be chosen by firmware.
The most hurtful scene would be *noisy environment* such as office, or public.
The latency would be high and overall throughput would be only half.
(This was tested, such as 4x Mbps -> 1x Mbps)

Yan-Hsuan

Re: [PATCH v2 4/5] rtw88: fix unassigned rssi_level in rtw_sta_info

[Kalle Valo]

Tony Chuang <yhchuang@realtek.com> writes:

>> -----Original Message-----
>> From: Kalle Valo [mailto:kvalo@codeaurora.org]
>> Sent: Monday, May 06, 2019 4:49 PM
>> To: Tony Chuang
>> Cc: linux-wireless@vger.kernel.org
>> Subject: Re: [PATCH v2 4/5] rtw88: fix unassigned rssi_level in rtw_sta_info
>> 
>> <yhchuang@realtek.com> writes:
>> 
>> > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
>> >
>> > The new rssi_level should be stored in si, otherwise the rssi_level will
>> > never be updated and get a wrong RA mask, which is calculated by the
>> > rssi level
>> >
>> > Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
>> 
>> Stanislaw suggested that this should go to 5.2. So what breaks from
>> user's point of view if this is not applied?
>> 
>
> If the rssi level remains unchanged, then we could choose wrong ra_mask.
> And some *bad rates* we be chosen by firmware.
> The most hurtful scene would be *noisy environment* such as office, or public.
> The latency would be high and overall throughput would be only half.
> (This was tested, such as 4x Mbps -> 1x Mbps)

Yeah, then this is definitely suitable for 5.2. Could you please resend
the patch and mention the symtomps in the commit log? And mark the patch
as "[PATCH 5.2 v3]" so that I can easily see it's for v5.2, please.

-- 
Kalle Valo

[PATCH v2 5/5] rtw88: more descriptions about LPS

[yhchuang]

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

The LPS represents Leisure Power Save. When enabled, firmware will be in
charge of turning radio off between beacons. Also firmware should turn
on the radio when beacon is coming, and the data queued should be
transmitted in TBTT period.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index f447361..142e530 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -20,7 +20,7 @@ EXPORT_SYMBOL(rtw_debug_mask);
 module_param_named(support_lps, rtw_fw_support_lps, bool, 0644);
 module_param_named(debug_mask, rtw_debug_mask, uint, 0644);
 
-MODULE_PARM_DESC(support_lps, "Set Y to enable LPS support");
+MODULE_PARM_DESC(support_lps, "Set Y to enable Leisure Power Save support, to turn radio off between beacons");
 MODULE_PARM_DESC(debug_mask, "Debugging mask");
 
 static struct ieee80211_channel rtw_channeltable_2g[] = {
-- 
2.7.4

Re: [PATCH v2 0/5] rtw88: minor fixes from suggestions during review

[Kalle Valo]

<yhchuang@realtek.com> writes:

> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
>
> The series fix some small problems for rtw88, most of the suggestions
> are from the review process.
>
>
> v1 -> v2
>
>  - modify description for LPS, ", turn off" -> ", to turn off"
>  - drop patch "rtw88: mac: remove dangerous while (1)",
>    should re-write the power sequence parsing code to make sense of avoiding
>    infinite loop
>  - unify Makefile license to Dual GPL/BSD
>
>
> Yan-Hsuan Chuang (5):
>   rtw88: add license for Makefile
>   rtw88: pci: use ieee80211_ac_numbers instead of 0-3
>   rtw88: pci: check if queue mapping exceeds size of ac_to_hwq
>   rtw88: fix unassigned rssi_level in rtw_sta_info
>   rtw88: more descriptions about LPS

I was just in the next few minutes about to tag the last -next pull for
5.2. I'll apply patch 1 now so that we have consistent licenses for 5.2
but the rest have to wait for 5.3.

-- 
Kalle Valo

Re: [PATCH v2 0/5] rtw88: minor fixes from suggestions during review

[Stanislaw Gruszka]

On Fri, May 03, 2019 at 03:04:58PM +0300, Kalle Valo wrote:
> <yhchuang@realtek.com> writes:
> 
> > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> >
> > The series fix some small problems for rtw88, most of the suggestions
> > are from the review process.
> >
> >
> > v1 -> v2
> >
> >  - modify description for LPS, ", turn off" -> ", to turn off"
> >  - drop patch "rtw88: mac: remove dangerous while (1)",
> >    should re-write the power sequence parsing code to make sense of avoiding
> >    infinite loop
> >  - unify Makefile license to Dual GPL/BSD
> >
> >
> > Yan-Hsuan Chuang (5):
> >   rtw88: add license for Makefile
> >   rtw88: pci: use ieee80211_ac_numbers instead of 0-3
> >   rtw88: pci: check if queue mapping exceeds size of ac_to_hwq
> >   rtw88: fix unassigned rssi_level in rtw_sta_info
> >   rtw88: more descriptions about LPS
> 
> I was just in the next few minutes about to tag the last -next pull for
> 5.2. I'll apply patch 1 now so that we have consistent licenses for 5.2
> but the rest have to wait for 5.3.

I think '[PATCH v2 4/5] rtw88: fix unassigned rssi_level in rtw_sta_inf'
should go to 5.2 .

Stanislaw