* [PATCH 5/6] rtw88: mac: remove dangerous while (1)
From: yhchuang @ 2019-05-03 10:31 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
In-Reply-To: <1556879502-16211-1-git-send-email-yhchuang@realtek.com>
From: Yan-Hsuan Chuang <yhchuang@realtek.com>
Not to use while (1) to parse power sequence commands in an array.
Put the statement (when cmd is not NULL) instead to make the loop stop
when the next fetched command is NULL.
Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
drivers/net/wireless/realtek/rtw88/mac.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
index 25a923b..7487b2e 100644
--- a/drivers/net/wireless/realtek/rtw88/mac.c
+++ b/drivers/net/wireless/realtek/rtw88/mac.c
@@ -203,17 +203,14 @@ static int rtw_pwr_seq_parser(struct rtw_dev *rtwdev,
return -EINVAL;
}
- do {
- cmd = cmd_seq[idx];
- if (!cmd)
- break;
-
+ while ((cmd = cmd_seq[idx])) {
ret = rtw_sub_pwr_seq_parser(rtwdev, intf_mask, cut_mask, cmd);
if (ret)
return -EBUSY;
+ /* fetch next command */
idx++;
- } while (1);
+ };
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 6/6] rtw88: more descriptions about LPS
From: yhchuang @ 2019-05-03 10:31 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
In-Reply-To: <1556879502-16211-1-git-send-email-yhchuang@realtek.com>
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..6953013 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, turn radio off between beacons");
MODULE_PARM_DESC(debug_mask, "Debugging mask");
static struct ieee80211_channel rtw_channeltable_2g[] = {
--
2.7.4
^ permalink raw reply related
* [PATCH 0/6] rtw88: minor fixes from suggestions during review
From: yhchuang @ 2019-05-03 10:31 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
From: Yan-Hsuan Chuang <yhchuang@realtek.com>
The series fix some small problems for rtw88, most of the suggestions
are from the review process.
Yan-Hsuan Chuang (6):
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: mac: remove dangerous while (1)
rtw88: more descriptions about LPS
drivers/net/wireless/realtek/rtw88/Makefile | 2 ++
drivers/net/wireless/realtek/rtw88/mac.c | 9 +++------
drivers/net/wireless/realtek/rtw88/main.c | 2 +-
drivers/net/wireless/realtek/rtw88/pci.c | 10 ++++++----
drivers/net/wireless/realtek/rtw88/phy.c | 4 ++--
5 files changed, 14 insertions(+), 13 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 1/6] rtw88: add license for Makefile
From: yhchuang @ 2019-05-03 10:31 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
In-Reply-To: <1556879502-16211-1-git-send-email-yhchuang@realtek.com>
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..74cd066 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
+
obj-$(CONFIG_RTW88_CORE) += rtw88.o
rtw88-y += main.o \
mac80211.o \
--
2.7.4
^ permalink raw reply related
* [PATCH 3/6] rtw88: pci: check if queue mapping exceeds size of ac_to_hwq
From: yhchuang @ 2019-05-03 10:31 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
In-Reply-To: <1556879502-16211-1-git-send-email-yhchuang@realtek.com>
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
^ permalink raw reply related
* [PATCH 2/6] rtw88: pci: use ieee80211_ac_numbers instead of 0-3
From: yhchuang @ 2019-05-03 10:31 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
In-Reply-To: <1556879502-16211-1-git-send-email-yhchuang@realtek.com>
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
^ permalink raw reply related
* [PATCH 4/6] rtw88: fix unassigned rssi_level in rtw_sta_info
From: yhchuang @ 2019-05-03 10:31 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
In-Reply-To: <1556879502-16211-1-git-send-email-yhchuang@realtek.com>
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
^ permalink raw reply related
* Re: [PATCH 5/6] rtw88: mac: remove dangerous while (1)
From: Johannes Berg @ 2019-05-03 10:48 UTC (permalink / raw)
To: yhchuang, kvalo; +Cc: linux-wireless
In-Reply-To: <1556879502-16211-6-git-send-email-yhchuang@realtek.com>
On Fri, 2019-05-03 at 18:31 +0800, yhchuang@realtek.com wrote:
>
> + while ((cmd = cmd_seq[idx])) {
...
> + };
That semicolon is pretty pointless there :-)
johannes
^ permalink raw reply
* RE: [PATCH 5/6] rtw88: mac: remove dangerous while (1)
From: Tony Chuang @ 2019-05-03 10:52 UTC (permalink / raw)
To: Johannes Berg, kvalo@codeaurora.org; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <315fea6071bc29c20b3f71f8e725433c64ad195d.camel@sipsolutions.net>
>
> On Fri, 2019-05-03 at 18:31 +0800, yhchuang@realtek.com wrote:
> >
> > + while ((cmd = cmd_seq[idx])) {
> ...
> > + };
>
> That semicolon is pretty pointless there :-)
>
> johannes
>
>
Missed it. Will send v2.
Thanks!
Yan-Hsuan
^ permalink raw reply
* Re: [PATCH 6/6] rtw88: more descriptions about LPS
From: Kalle Valo @ 2019-05-03 11:01 UTC (permalink / raw)
To: yhchuang; +Cc: linux-wireless
In-Reply-To: <1556879502-16211-7-git-send-email-yhchuang@realtek.com>
<yhchuang@realtek.com> writes:
> 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..6953013 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, turn radio off between beacons");
I think it would help to add:
", to turn radio off between beacons"
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 1/6] rtw88: add license for Makefile
From: Kalle Valo @ 2019-05-03 11:03 UTC (permalink / raw)
To: yhchuang; +Cc: linux-wireless
In-Reply-To: <1556879502-16211-2-git-send-email-yhchuang@realtek.com>
<yhchuang@realtek.com> writes:
> 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..74cd066 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
Other files in the driver are "GPL-2.0 OR BSD-3-Clause", why not
Makefile? I prefer that the whole driver has the same license, keeps
things simple.
--
Kalle Valo
^ permalink raw reply
* [PATCH v2] {nl,mac}80211: allow 4addr AP operation on crypto controlled devices
From: Manikanta Pubbisetty @ 2019-05-03 11:03 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, Manikanta Pubbisetty
As per the current design, in the case of sw crypto controlled devices,
it is the device which advertises the support for AP/VLAN iftype based
on it's ability to tranmsit packets encrypted in software
(In VLAN functionality, group traffic generated for a specific
VLAN group is always encrypted in software). Commit db3bdcb9c3ff
("mac80211: allow AP_VLAN operation on crypto controlled devices")
has introduced this change.
Since 4addr AP operation also uses AP/VLAN iftype, this conditional
way of advertising AP/VLAN support has broken 4addr AP mode operation on
crypto controlled devices which do not support VLAN functionality.
In the case of ath10k driver, not all firmwares have support for VLAN
functionality but all can support 4addr AP operation. Because AP/VLAN
support is not advertised for these devices, 4addr AP operations are
also blocked.
Fix this by allowing 4addr operation on devices which do not support
AP/VLAN iftype but can support 4addr AP operation (decision is based on
the wiphy flag WIPHY_FLAG_4ADDR_AP).
Fixes: Commit db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on crypto controlled devices")
Signed-off-by: Manikanta Pubbisetty <mpubbise@codeaurora.org>
---
include/net/cfg80211.h | 3 ++-
net/mac80211/util.c | 4 +++-
net/wireless/core.c | 6 +++++-
net/wireless/nl80211.c | 8 ++++++--
4 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 87dae86..9481396 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3839,7 +3839,8 @@ struct cfg80211_ops {
* on wiphy_new(), but can be changed by the driver if it has a good
* reason to override the default
* @WIPHY_FLAG_4ADDR_AP: supports 4addr mode even on AP (with a single station
- * on a VLAN interface)
+ * on a VLAN interface). This flag also serves an extra purpose of
+ * supporting 4ADDR AP mode on devices which do not support AP/VLAN iftype.
* @WIPHY_FLAG_4ADDR_STATION: supports 4addr mode even as a station
* @WIPHY_FLAG_CONTROL_PORT_PROTOCOL: This device supports setting the
* control port protocol ethertype. The device also honours the
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index cba4633..1c8384f 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -3795,7 +3795,9 @@ int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
}
/* Always allow software iftypes */
- if (local->hw.wiphy->software_iftypes & BIT(iftype)) {
+ if (local->hw.wiphy->software_iftypes & BIT(iftype) ||
+ (iftype == NL80211_IFTYPE_AP_VLAN &&
+ local->hw.wiphy->flags & WIPHY_FLAG_4ADDR_AP)) {
if (radar_detect)
return -EINVAL;
return 0;
diff --git a/net/wireless/core.c b/net/wireless/core.c
index b36ad8e..4e83892 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1396,8 +1396,12 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
}
break;
case NETDEV_PRE_UP:
- if (!(wdev->wiphy->interface_modes & BIT(wdev->iftype)))
+ if (!(wdev->wiphy->interface_modes & BIT(wdev->iftype)) &&
+ !(wdev->iftype == NL80211_IFTYPE_AP_VLAN &&
+ rdev->wiphy.flags & WIPHY_FLAG_4ADDR_AP &&
+ wdev->use_4addr))
return notifier_from_errno(-EOPNOTSUPP);
+
if (rfkill_blocked(rdev->rfkill))
return notifier_from_errno(-ERFKILL);
break;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index fffe4b3..4b3c528 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3419,8 +3419,7 @@ static int nl80211_new_interface(struct sk_buff *skb, struct genl_info *info)
if (info->attrs[NL80211_ATTR_IFTYPE])
type = nla_get_u32(info->attrs[NL80211_ATTR_IFTYPE]);
- if (!rdev->ops->add_virtual_intf ||
- !(rdev->wiphy.interface_modes & (1 << type)))
+ if (!rdev->ops->add_virtual_intf)
return -EOPNOTSUPP;
if ((type == NL80211_IFTYPE_P2P_DEVICE || type == NL80211_IFTYPE_NAN ||
@@ -3439,6 +3438,11 @@ static int nl80211_new_interface(struct sk_buff *skb, struct genl_info *info)
return err;
}
+ if (!(rdev->wiphy.interface_modes & (1 << type)) &&
+ !(type == NL80211_IFTYPE_AP_VLAN && params.use_4addr &&
+ rdev->wiphy.flags & WIPHY_FLAG_4ADDR_AP))
+ return -EOPNOTSUPP;
+
err = nl80211_parse_mon_options(rdev, type, info, ¶ms);
if (err < 0)
return err;
--
2.7.4
^ permalink raw reply related
* RE: [PATCH 6/6] rtw88: more descriptions about LPS
From: Tony Chuang @ 2019-05-03 11:04 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <87zho3kdiy.fsf@kamboji.qca.qualcomm.com>
> Subject: Re: [PATCH 6/6] rtw88: more descriptions about LPS
>
> <yhchuang@realtek.com> writes:
>
> > 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..6953013 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, turn radio off between beacons");
>
> I think it would help to add:
>
> ", to turn radio off between beacons"
>
Looks better, will include it in v2.
Thanks.
Yan-Hsuan
^ permalink raw reply
* RE: [PATCH 1/6] rtw88: add license for Makefile
From: Tony Chuang @ 2019-05-03 11:05 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <87v9yrkdfj.fsf@kamboji.qca.qualcomm.com>
> Subject: Re: [PATCH 1/6] rtw88: add license for Makefile
>
> <yhchuang@realtek.com> writes:
>
> > 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..74cd066 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
>
> Other files in the driver are "GPL-2.0 OR BSD-3-Clause", why not
> Makefile? I prefer that the whole driver has the same license, keeps
> things simple.
>
> --
> Kalle Valo
>
You are right, different license is strange.
Will add it in v2, thanks.
Yan-Hsuan
^ permalink raw reply
* Re: [PATCH 5/6] rtw88: mac: remove dangerous while (1)
From: Kalle Valo @ 2019-05-03 11:07 UTC (permalink / raw)
To: yhchuang; +Cc: linux-wireless
In-Reply-To: <1556879502-16211-6-git-send-email-yhchuang@realtek.com>
<yhchuang@realtek.com> writes:
> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
>
> Not to use while (1) to parse power sequence commands in an array.
> Put the statement (when cmd is not NULL) instead to make the loop stop
> when the next fetched command is NULL.
>
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
> ---
> drivers/net/wireless/realtek/rtw88/mac.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
> index 25a923b..7487b2e 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac.c
> @@ -203,17 +203,14 @@ static int rtw_pwr_seq_parser(struct rtw_dev *rtwdev,
> return -EINVAL;
> }
>
> - do {
> - cmd = cmd_seq[idx];
> - if (!cmd)
> - break;
> -
> + while ((cmd = cmd_seq[idx])) {
> ret = rtw_sub_pwr_seq_parser(rtwdev, intf_mask, cut_mask, cmd);
> if (ret)
> return -EBUSY;
>
> + /* fetch next command */
> idx++;
> - } while (1);
> + };
I dount see how this is any better, with a suitable bug you can still
have a neverending loop, right? I was thinking more something like this:
count = 100;
do {
....
} while (count--);
That way the won't be more than 100 loops no matter how many bugs there
are :) Of course I have no idea what would be a good value for count.
--
Kalle Valo
^ permalink raw reply
* RE: [PATCH 5/6] rtw88: mac: remove dangerous while (1)
From: Tony Chuang @ 2019-05-03 11:22 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <87r29fkd9k.fsf@kamboji.qca.qualcomm.com>
> Subject: Re: [PATCH 5/6] rtw88: mac: remove dangerous while (1)
>
> <yhchuang@realtek.com> writes:
>
> > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> >
> > Not to use while (1) to parse power sequence commands in an array.
> > Put the statement (when cmd is not NULL) instead to make the loop stop
> > when the next fetched command is NULL.
> >
> > Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
> > ---
> > drivers/net/wireless/realtek/rtw88/mac.c | 9 +++------
> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/wireless/realtek/rtw88/mac.c
> b/drivers/net/wireless/realtek/rtw88/mac.c
> > index 25a923b..7487b2e 100644
> > --- a/drivers/net/wireless/realtek/rtw88/mac.c
> > +++ b/drivers/net/wireless/realtek/rtw88/mac.c
> > @@ -203,17 +203,14 @@ static int rtw_pwr_seq_parser(struct rtw_dev
> *rtwdev,
> > return -EINVAL;
> > }
> >
> > - do {
> > - cmd = cmd_seq[idx];
> > - if (!cmd)
> > - break;
> > -
> > + while ((cmd = cmd_seq[idx])) {
> > ret = rtw_sub_pwr_seq_parser(rtwdev, intf_mask, cut_mask, cmd);
> > if (ret)
> > return -EBUSY;
> >
> > + /* fetch next command */
> > idx++;
> > - } while (1);
> > + };
>
> I dount see how this is any better, with a suitable bug you can still
> have a neverending loop, right? I was thinking more something like this:
>
> count = 100;
> do {
> ....
> } while (count--);
>
> That way the won't be more than 100 loops no matter how many bugs there
> are :) Of course I have no idea what would be a good value for count.
>
To make this totally safe, I think we need to re-write the power seq parsing code.
I think I should drop this patch, and write a better code later.
And also re-write the polling command, to remove the while (1).
Yan-Hsuan
^ permalink raw reply
* Re: [PATCH 5/6] rtw88: mac: remove dangerous while (1)
From: Kalle Valo @ 2019-05-03 11:26 UTC (permalink / raw)
To: Tony Chuang; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D17E871A@RTITMBSVM04.realtek.com.tw>
Tony Chuang <yhchuang@realtek.com> writes:
>> Subject: Re: [PATCH 5/6] rtw88: mac: remove dangerous while (1)
>>
>> <yhchuang@realtek.com> writes:
>>
>> > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
>> >
>> > Not to use while (1) to parse power sequence commands in an array.
>> > Put the statement (when cmd is not NULL) instead to make the loop stop
>> > when the next fetched command is NULL.
>> >
>> > Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
>> > ---
>> > drivers/net/wireless/realtek/rtw88/mac.c | 9 +++------
>> > 1 file changed, 3 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/net/wireless/realtek/rtw88/mac.c
>> b/drivers/net/wireless/realtek/rtw88/mac.c
>> > index 25a923b..7487b2e 100644
>> > --- a/drivers/net/wireless/realtek/rtw88/mac.c
>> > +++ b/drivers/net/wireless/realtek/rtw88/mac.c
>> > @@ -203,17 +203,14 @@ static int rtw_pwr_seq_parser(struct rtw_dev
>> *rtwdev,
>> > return -EINVAL;
>> > }
>> >
>> > - do {
>> > - cmd = cmd_seq[idx];
>> > - if (!cmd)
>> > - break;
>> > -
>> > + while ((cmd = cmd_seq[idx])) {
>> > ret = rtw_sub_pwr_seq_parser(rtwdev, intf_mask, cut_mask, cmd);
>> > if (ret)
>> > return -EBUSY;
>> >
>> > + /* fetch next command */
>> > idx++;
>> > - } while (1);
>> > + };
>>
>> I dount see how this is any better, with a suitable bug you can still
>> have a neverending loop, right? I was thinking more something like this:
>>
>> count = 100;
>> do {
>> ....
>> } while (count--);
>>
>> That way the won't be more than 100 loops no matter how many bugs there
>> are :) Of course I have no idea what would be a good value for count.
>>
>
> To make this totally safe, I think we need to re-write the power seq parsing code.
> I think I should drop this patch, and write a better code later.
>
> And also re-write the polling command, to remove the while (1).
Sounds good to me.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH v2] {nl,mac}80211: allow 4addr AP operation on crypto controlled devices
From: Kalle Valo @ 2019-05-03 11:28 UTC (permalink / raw)
To: Manikanta Pubbisetty; +Cc: johannes, linux-wireless
In-Reply-To: <1556881402-28078-1-git-send-email-mpubbise@codeaurora.org>
Manikanta Pubbisetty <mpubbise@codeaurora.org> writes:
> As per the current design, in the case of sw crypto controlled devices,
> it is the device which advertises the support for AP/VLAN iftype based
> on it's ability to tranmsit packets encrypted in software
> (In VLAN functionality, group traffic generated for a specific
> VLAN group is always encrypted in software). Commit db3bdcb9c3ff
> ("mac80211: allow AP_VLAN operation on crypto controlled devices")
> has introduced this change.
>
> Since 4addr AP operation also uses AP/VLAN iftype, this conditional
> way of advertising AP/VLAN support has broken 4addr AP mode operation on
> crypto controlled devices which do not support VLAN functionality.
>
> In the case of ath10k driver, not all firmwares have support for VLAN
> functionality but all can support 4addr AP operation. Because AP/VLAN
> support is not advertised for these devices, 4addr AP operations are
> also blocked.
>
> Fix this by allowing 4addr operation on devices which do not support
> AP/VLAN iftype but can support 4addr AP operation (decision is based on
> the wiphy flag WIPHY_FLAG_4ADDR_AP).
>
> Fixes: Commit db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on crypto controlled devices")
The correct format for the Fixes line is:
Fixes: db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on crypto controlled devices")
--
Kalle Valo
^ permalink raw reply
* [RFC] rtw88: fix subscript above array bounds compiler warning
From: Stanislaw Gruszka @ 2019-05-03 11:53 UTC (permalink / raw)
To: Tony Chuang; +Cc: linux-wireless
My compiler complain about:
drivers/net/wireless/realtek/rtw88/phy.c: In function ‘rtw_phy_rf_power_2_rssi’:
drivers/net/wireless/realtek/rtw88/phy.c:430:26: warning: array subscript is above array bounds [-Warray-bounds]
linear = db_invert_table[i][j];
According to comment power_db should be in range 1 ~ 96 .
Correct rtw_phy_power_2_db() to make max power 96 db
(still min is 0). This make the warning gone.
However power >= 20 check still looks somewhat suspicious to me.
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/wireless/realtek/rtw88/phy.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/phy.c b/drivers/net/wireless/realtek/rtw88/phy.c
index 35a35dbca85f..a716a44d78b0 100644
--- a/drivers/net/wireless/realtek/rtw88/phy.c
+++ b/drivers/net/wireless/realtek/rtw88/phy.c
@@ -410,12 +410,12 @@ void rtw_phy_dynamic_mechanism(struct rtw_dev *rtwdev)
static u8 rtw_phy_power_2_db(s8 power)
{
- if (power <= -100 || power >= 20)
+ if (power <= -96 || power >= 20)
return 0;
else if (power >= 0)
- return 100;
+ return 96;
else
- return 100 + power;
+ return 96 + power;
}
static u64 rtw_phy_db_2_linear(u8 power_db)
--
1.9.3
^ permalink raw reply related
* [PATCH v2 0/5] rtw88: minor fixes from suggestions during review
From: yhchuang @ 2019-05-03 11:53 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
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
^ permalink raw reply
* [PATCH v2 3/5] rtw88: pci: check if queue mapping exceeds size of ac_to_hwq
From: yhchuang @ 2019-05-03 11:53 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
In-Reply-To: <1556884415-23474-1-git-send-email-yhchuang@realtek.com>
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
^ permalink raw reply related
* [PATCH v2 5/5] rtw88: more descriptions about LPS
From: yhchuang @ 2019-05-03 11:53 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
In-Reply-To: <1556884415-23474-1-git-send-email-yhchuang@realtek.com>
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
^ permalink raw reply related
* [PATCH v2 4/5] rtw88: fix unassigned rssi_level in rtw_sta_info
From: yhchuang @ 2019-05-03 11:53 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
In-Reply-To: <1556884415-23474-1-git-send-email-yhchuang@realtek.com>
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
^ permalink raw reply related
* [PATCH v2 1/5] rtw88: add license for Makefile
From: yhchuang @ 2019-05-03 11:53 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
In-Reply-To: <1556884415-23474-1-git-send-email-yhchuang@realtek.com>
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
^ permalink raw reply related
* [PATCH v2 2/5] rtw88: pci: use ieee80211_ac_numbers instead of 0-3
From: yhchuang @ 2019-05-03 11:53 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless
In-Reply-To: <1556884415-23474-1-git-send-email-yhchuang@realtek.com>
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
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox