2019-05-03 11:54:10

by Tony Chuang

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

From: Yan-Hsuan Chuang <[email protected]>

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


2019-05-03 11:54:10

by Tony Chuang

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

From: Yan-Hsuan Chuang <[email protected]>

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 <[email protected]>
---
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

2019-05-03 11:54:10

by Tony Chuang

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

From: Yan-Hsuan Chuang <[email protected]>

Add missing license for Makefile

Signed-off-by: Yan-Hsuan Chuang <[email protected]>
---
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

2019-05-03 11:54:57

by Tony Chuang

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

From: Yan-Hsuan Chuang <[email protected]>

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 <[email protected]>
---
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

2019-05-03 11:54:57

by Tony Chuang

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

From: Yan-Hsuan Chuang <[email protected]>

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 <[email protected]>
---
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

2019-05-03 11:55:31

by Tony Chuang

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

From: Yan-Hsuan Chuang <[email protected]>

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

Signed-off-by: Yan-Hsuan Chuang <[email protected]>
---
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

2019-05-03 12:10:42

by Kalle Valo

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

<[email protected]> writes:

> From: Yan-Hsuan Chuang <[email protected]>
>
> 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

2019-05-03 12:13:37

by Kalle Valo

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

<[email protected]> wrote:

> From: Yan-Hsuan Chuang <[email protected]>
>
> Add missing license for Makefile
>
> Signed-off-by: Yan-Hsuan Chuang <[email protected]>

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

2019-05-06 08:42:39

by Stanislaw Gruszka

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

On Fri, May 03, 2019 at 03:04:58PM +0300, Kalle Valo wrote:
> <[email protected]> writes:
>
> > From: Yan-Hsuan Chuang <[email protected]>
> >
> > 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

2019-05-06 08:49:16

by Kalle Valo

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

<[email protected]> writes:

> From: Yan-Hsuan Chuang <[email protected]>
>
> 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 <[email protected]>

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

2019-05-06 08:54:43

by Tony Chuang

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



> -----Original Message-----
> From: Kalle Valo [mailto:[email protected]]
> Sent: Monday, May 06, 2019 4:49 PM
> To: Tony Chuang
> Cc: [email protected]
> Subject: Re: [PATCH v2 4/5] rtw88: fix unassigned rssi_level in rtw_sta_info
>
> <[email protected]> writes:
>
> > From: Yan-Hsuan Chuang <[email protected]>
> >
> > 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 <[email protected]>
>
> 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

2019-05-06 12:37:34

by Kalle Valo

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

Tony Chuang <[email protected]> writes:

>> -----Original Message-----
>> From: Kalle Valo [mailto:[email protected]]
>> Sent: Monday, May 06, 2019 4:49 PM
>> To: Tony Chuang
>> Cc: [email protected]
>> Subject: Re: [PATCH v2 4/5] rtw88: fix unassigned rssi_level in rtw_sta_info
>>
>> <[email protected]> writes:
>>
>> > From: Yan-Hsuan Chuang <[email protected]>
>> >
>> > 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 <[email protected]>
>>
>> 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

2019-05-28 11:53:00

by Kalle Valo

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

<[email protected]> wrote:

> From: Yan-Hsuan Chuang <[email protected]>
>
> AC numbers are defined as enum in mac80211, use them instead of bare
> 0-3 indexing.
>
> Signed-off-by: Yan-Hsuan Chuang <[email protected]>

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