* [PATCH 3/3] mac80211: cache the only AP_VLAN station
From: Michael Braun @ 2016-09-25 16:39 UTC (permalink / raw)
To: johannes; +Cc: Michael Braun, linux-wireless
In-Reply-To: <1474821596-12155-1-git-send-email-michael-dev@fami-braun.de>
If an AP_VLAN is only used with one station, cache a pointer to this station to
avoid lookup. This should speed up station lookup when there is only one
station assigned to the AP_VLAN interface, e.g. when using hostapd with
per_sta_vif.
Assigning only one station per AP_VLAN interfaces enables bridge IGMP snooping
to track multicast subscriptions by station to selectively forward to only
those stations that subscribed.
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
net/mac80211/cfg.c | 10 ++++++++--
net/mac80211/ieee80211_i.h | 14 ++++++++++----
net/mac80211/sta_info.c | 33 +++++++++++++++++++++++++++++++--
net/mac80211/tx.c | 5 +++++
4 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 078e837..a69e6f2 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -67,6 +67,7 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
if (type == NL80211_IFTYPE_AP_VLAN &&
params && params->use_4addr == 0) {
RCU_INIT_POINTER(sdata->u.vlan.sta, NULL);
+ RCU_INIT_POINTER(sdata->u.vlan.sta0, NULL);
ieee80211_check_fast_rx_iface(sdata);
} else if (type == NL80211_IFTYPE_STATION &&
params && params->use_4addr >= 0) {
@@ -1379,8 +1380,13 @@ static int ieee80211_change_station(struct wiphy *wiphy,
sta->sdata = vlansdata;
ieee80211_check_fast_xmit(sta);
- if (test_sta_flag(sta, WLAN_STA_AUTHORIZED))
- ieee80211_vif_inc_num_mcast(sta->sdata);
+ if (test_sta_flag(sta, WLAN_STA_AUTHORIZED) &&
+ ieee80211_vif_inc_num_mcast(sta->sdata) == 1 &&
+ sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+ rcu_assign_pointer(vlansdata->u.vlan.sta0, sta);
+ else if (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+ test_sta_flag(sta, WLAN_STA_AUTHORIZED))
+ RCU_INIT_POINTER(vlansdata->u.vlan.sta0, NULL);
ieee80211_send_layer2_update(sta);
}
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 7b3de28..48a141f 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -307,6 +307,8 @@ struct ieee80211_if_vlan {
/* used for all tx if the VLAN is configured to 4-addr mode */
struct sta_info __rcu *sta;
+ /* the one and only authenticated station in non 4-addr mode if set */
+ struct sta_info __rcu *sta0;
atomic_t num_mcast_sta_if; /* number of stations receiving multicast */
};
@@ -1499,21 +1501,25 @@ ieee80211_have_rx_timestamp(struct ieee80211_rx_status *status)
return false;
}
-static inline void
+static inline int
ieee80211_vif_inc_num_mcast(struct ieee80211_sub_if_data *sdata)
{
+ int ret;
+
if (sdata->vif.type != NL80211_IFTYPE_AP &&
sdata->vif.type != NL80211_IFTYPE_AP_VLAN)
- return;
+ return -1;
if (sdata->vif.type == NL80211_IFTYPE_AP)
- atomic_inc(&sdata->u.ap.num_mcast_sta_if);
+ ret = atomic_inc_return(&sdata->u.ap.num_mcast_sta_if);
else if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
- atomic_inc(&sdata->u.vlan.num_mcast_sta_if);
+ ret = atomic_inc_return(&sdata->u.vlan.num_mcast_sta_if);
if (sdata->vif.type != NL80211_IFTYPE_AP_VLAN ||
!sdata->u.vlan.sta) /* except 4addr mode */
atomic_inc(&sdata->bss->num_mcast_sta);
+
+ return ret;
}
static inline void
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 216ef65..d1c5d96 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -162,11 +162,28 @@ struct sta_info *sta_info_get(struct ieee80211_sub_if_data *sdata,
const u8 *addr)
{
struct ieee80211_local *local = sdata->local;
- struct sta_info *sta;
+ struct sta_info *sta = NULL;
struct rhash_head *tmp;
const struct bucket_table *tbl;
rcu_read_lock();
+
+ if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+ !sdata->u.vlan.sta)
+ sta = rcu_dereference(sdata->u.vlan.sta0);
+
+ WARN_ONCE((sta && sta->sdata != sdata),
+ "sdata->u.vlan.sta0->sdata != sdata");
+
+ if (sta && sta->sdata == sdata &&
+ ether_addr_equal(sta->sta.addr, addr)) {
+ rcu_read_unlock();
+ /* this is safe as the caller must already hold
+ * another rcu read section or the mutex
+ */
+ return sta;
+ }
+
tbl = rht_dereference_rcu(local->sta_hash.tbl, &local->sta_hash);
for_each_sta_info(local, tbl, addr, sta, tmp) {
@@ -920,6 +937,10 @@ static int __must_check __sta_info_destroy_part1(struct sta_info *sta)
rcu_access_pointer(sdata->u.vlan.sta) == sta)
RCU_INIT_POINTER(sdata->u.vlan.sta, NULL);
+ if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+ rcu_access_pointer(sdata->u.vlan.sta0) == sta)
+ RCU_INIT_POINTER(sdata->u.vlan.sta0, NULL);
+
return 0;
}
@@ -1883,6 +1904,9 @@ int sta_info_move_state(struct sta_info *sta,
ieee80211_recalc_p2p_go_ps_allowed(sta->sdata);
} else if (sta->sta_state == IEEE80211_STA_AUTHORIZED) {
ieee80211_vif_dec_num_mcast(sta->sdata);
+ if (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+ rcu_access_pointer(sta->sdata->u.vlan.sta0) == sta)
+ RCU_INIT_POINTER(sta->sdata->u.vlan.sta0, NULL);
clear_bit(WLAN_STA_AUTHORIZED, &sta->_flags);
ieee80211_clear_fast_xmit(sta);
ieee80211_clear_fast_rx(sta);
@@ -1890,7 +1914,12 @@ int sta_info_move_state(struct sta_info *sta,
break;
case IEEE80211_STA_AUTHORIZED:
if (sta->sta_state == IEEE80211_STA_ASSOC) {
- ieee80211_vif_inc_num_mcast(sta->sdata);
+ if (ieee80211_vif_inc_num_mcast(sta->sdata) == 1 &&
+ sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+ rcu_assign_pointer(sta->sdata->u.vlan.sta0,
+ sta);
+ else if (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+ RCU_INIT_POINTER(sta->sdata->u.vlan.sta0, NULL);
set_bit(WLAN_STA_AUTHORIZED, &sta->_flags);
ieee80211_check_fast_xmit(sta);
ieee80211_check_fast_rx(sta);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 9c82fd8..c06d5f9 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1808,6 +1808,7 @@ ieee80211_tx_multicast_to_unicast(struct ieee80211_sub_if_data *sdata,
sta = rcu_dereference(sdata->u.vlan.sta);
if (sta) /* 4addr */
return 0;
+ prev = rcu_dereference(sdata->u.vlan.sta0);
case NL80211_IFTYPE_AP:
break;
default:
@@ -1839,6 +1840,9 @@ ieee80211_tx_multicast_to_unicast(struct ieee80211_sub_if_data *sdata,
return 0;
}
+ if (prev)
+ goto skip_lookup;
+
/* clone packets and update destination mac */
list_for_each_entry_rcu(sta, &local->sta_list, list) {
if (sdata != sta->sdata)
@@ -1862,6 +1866,7 @@ ieee80211_tx_multicast_to_unicast(struct ieee80211_sub_if_data *sdata,
prev = sta;
}
+skip_lookup:
if (likely(prev)) {
ieee80211_tx_dnat(skb, prev);
return 0;
--
2.1.4
^ permalink raw reply related
* [PATCH 2/3] mac80211: multicast to unicast conversion
From: Michael Braun @ 2016-09-25 16:39 UTC (permalink / raw)
To: johannes; +Cc: Michael Braun, linux-wireless
In-Reply-To: <1474821596-12155-1-git-send-email-michael-dev@fami-braun.de>
This patch adds support for sending multicast data packets with ARP, IPv4 and
IPv6 payload (possible 802.1q tagged) as 802.11 unicast frames to all stations.
IEEE 802.11 multicast has well known issues, among them:
1. packets are not acked and hence not retransmitted, resulting in decreased
reliablity
2. packets are send at low rate, increasing time required on air
When used with AP_VLAN, there is another disadvantage:
3. all stations in the BSS are woken up, regardsless of their AP_VLAN
assignment.
By doing multicast to unicast conversion, all three issus are solved.
IEEE802.11-2012 proposes directed multicast service (DMS) using A-MSDU frames
and a station initiated control protocol. It has the advantage that the station
can recover the destination multicast mac address, but it is not backward
compatible with non QOS stations and does not enable the administrator of a BSS
to force this mode of operation within a BSS. Additionally, it would require
both the ap and the station to implement the control protocol, which is
optional on both ends. Furthermore, I've seen a few mobile phone stations
locally that indicate qos support but won't complete DHCP if their broadcasts
are encapsulated as A-MSDU. Though they work fine with this series approach.
This patch therefore does not opt to implement DMS but instead just replicates
the packet and changes the destination address. As this works fine with ARP,
IPv4 and IPv6, it is limited to these protocols and normal 802.11 multicast
frames are send out for all other payload protocols.
There is a runtime toggle to enable multicast conversion in a per-bss fashion.
When there is only a single station assigned to the AP_VLAN interface, no
packet replication will occur. 4addr mode of operation is unchanged.
This change opts for iterating all BSS stations for finding the stations
assigned to this AP/AP_VLAN interface, as there currently is no per AP_VLAN
list to iterate and multicast packets are expected to be few. If needed, such
a list could be added later.
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
net/mac80211/debugfs_netdev.c | 27 +++++++++++
net/mac80211/ieee80211_i.h | 1 +
net/mac80211/tx.c | 105 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 133 insertions(+)
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index d97756d..cfffbc0 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -488,6 +488,32 @@ static ssize_t ieee80211_if_fmt_num_buffered_multicast(
}
IEEE80211_IF_FILE_R(num_buffered_multicast);
+static ssize_t ieee80211_if_fmt_unicast(
+ const struct ieee80211_sub_if_data *sdata, char *buf, int buflen)
+{
+ const struct ieee80211_if_ap *ifap = &sdata->u.ap;
+
+ return snprintf(buf, buflen, "0x%x\n", ifap->unicast);
+}
+
+static ssize_t ieee80211_if_parse_unicast(
+ struct ieee80211_sub_if_data *sdata, const char *buf, int buflen)
+{
+ struct ieee80211_if_ap *ifap = &sdata->u.ap;
+ u8 val;
+ int ret;
+
+ ret = kstrtou8(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ ifap->unicast = val ? 1 : 0;
+
+ return buflen;
+}
+
+IEEE80211_IF_FILE_RW(unicast);
+
/* IBSS attributes */
static ssize_t ieee80211_if_fmt_tsf(
const struct ieee80211_sub_if_data *sdata, char *buf, int buflen)
@@ -644,6 +670,7 @@ static void add_ap_files(struct ieee80211_sub_if_data *sdata)
DEBUGFS_ADD(dtim_count);
DEBUGFS_ADD(num_buffered_multicast);
DEBUGFS_ADD_MODE(tkip_mic_test, 0200);
+ DEBUGFS_ADD_MODE(unicast, 0600);
}
static void add_vlan_files(struct ieee80211_sub_if_data *sdata)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index b10e8be..7b3de28 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -294,6 +294,7 @@ struct ieee80211_if_ap {
driver_smps_mode; /* smps mode request */
struct work_struct request_smps_work;
+ int unicast;
};
struct ieee80211_if_wds {
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 7130862..9c82fd8 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -16,6 +16,7 @@
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/skbuff.h>
+#include <linux/if_vlan.h>
#include <linux/etherdevice.h>
#include <linux/bitmap.h>
#include <linux/rcupdate.h>
@@ -1770,6 +1771,106 @@ bool ieee80211_tx_prepare_skb(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_tx_prepare_skb);
+/* rewrite destination mac address */
+static int ieee80211_tx_dnat(struct sk_buff *skb, struct sta_info *sta)
+{
+ struct ethhdr *eth;
+ int err;
+
+ err = skb_ensure_writable(skb, ETH_HLEN);
+ if (unlikely(err))
+ return err;
+
+ eth = (void *)skb->data;
+ ether_addr_copy(eth->h_dest, sta->sta.addr);
+
+ return 0;
+}
+
+/* Check if multicast to unicast conversion is needed and do it.
+ *
+ * Returns: 1 if skb was freed and should not be send out
+ */
+static int
+ieee80211_tx_multicast_to_unicast(struct ieee80211_sub_if_data *sdata,
+ struct sk_buff *skb, u32 info_flags)
+{
+ struct ieee80211_local *local = sdata->local;
+ const struct ethhdr *eth = (void *)skb->data;
+ const struct vlan_ethhdr *ethvlan = (void *)skb->data;
+ struct sta_info *sta, *prev = NULL;
+ struct sk_buff *cloned_skb;
+ u16 ethertype;
+
+ /* multicast to unicast conversion only for AP interfaces */
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_AP_VLAN:
+ sta = rcu_dereference(sdata->u.vlan.sta);
+ if (sta) /* 4addr */
+ return 0;
+ case NL80211_IFTYPE_AP:
+ break;
+ default:
+ return 0;
+ }
+
+ /* check runtime toggle for this bss */
+ if (!sdata->bss->unicast)
+ return 0;
+
+ /* check if this is a multicast frame */
+ if (!is_multicast_ether_addr(eth->h_dest))
+ return 0;
+
+ /* info_flags would not get preserved, used only by TLDS*/
+ if (info_flags)
+ return 0;
+
+ /* multicast to unicast conversion only for some payload */
+ ethertype = ntohs(eth->h_proto);
+ if (ethertype == ETH_P_8021Q && skb->len >= VLAN_ETH_HLEN)
+ ethertype = ntohs(ethvlan->h_vlan_encapsulated_proto);
+ switch (ethertype) {
+ case ETH_P_ARP:
+ case ETH_P_IP:
+ case ETH_P_IPV6:
+ break;
+ default:
+ return 0;
+ }
+
+ /* clone packets and update destination mac */
+ list_for_each_entry_rcu(sta, &local->sta_list, list) {
+ if (sdata != sta->sdata)
+ continue;
+ if (unlikely(!memcmp(eth->h_source, sta->sta.addr, ETH_ALEN)))
+ /* do not send back to source */
+ continue;
+ if (unlikely(is_multicast_ether_addr(sta->sta.addr))) {
+ WARN_ONCE(1, "sta with multicast address %pM",
+ sta->sta.addr);
+ continue;
+ }
+ if (prev) {
+ cloned_skb = skb_clone(skb, GFP_ATOMIC);
+ if (likely(!ieee80211_tx_dnat(cloned_skb, prev)))
+ ieee80211_subif_start_xmit(cloned_skb,
+ cloned_skb->dev);
+ else
+ dev_kfree_skb(cloned_skb);
+ }
+ prev = sta;
+ }
+
+ if (likely(prev)) {
+ ieee80211_tx_dnat(skb, prev);
+ return 0;
+ }
+
+ /* no STA connected, drop */
+ return 1;
+}
+
/*
* Returns false if the frame couldn't be transmitted but was queued instead.
*/
@@ -3353,6 +3454,10 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
rcu_read_lock();
+ /* AP multicast to unicast conversion */
+ if (ieee80211_tx_multicast_to_unicast(sdata, skb, info_flags))
+ goto out_free;
+
if (ieee80211_lookup_ra_sta(sdata, skb, &sta))
goto out_free;
--
2.1.4
^ permalink raw reply related
* [PATCH 0/3] mac80211: multicast with AP_VLAN optimizations
From: Michael Braun @ 2016-09-25 16:39 UTC (permalink / raw)
To: johannes; +Cc: Michael Braun, linux-wireless
Hi,
this series tries to optimize multicast delivery on access points with
AP_VLAN interfaces.
My setup is as follows: hostapd creates one AP_VLAN interface per station
(per_sta_vif=1), which enables bridge igmp snooping to decide which
stations need to receive a multicast packet.
This series then
- avoids multicast packets by tracking the number of authenticated
stations per interface more accurately and drops packets if there is no
receiver connected,
- converts multicast packets to unicast packets for the most common
protocols, so they get delivered faster and more reliable. Additionally,
waking up non-receivers in other VLANs is avoided.
- speeds up station lookup if there is only one authenticated station
assigned to the AP_VLAN interface.
Sincerely,
M. Braun
Michael Braun (3):
mac80211: filter multicast data packets on AP / AP_VLAN
mac80211: multicast to unicast conversion
mac80211: cache the only AP_VLAN station
net/mac80211/cfg.c | 26 +++++-----
net/mac80211/debugfs_netdev.c | 38 ++++++++++++++
net/mac80211/ieee80211_i.h | 61 ++++++++++++++++++++++
net/mac80211/rx.c | 5 +-
net/mac80211/sta_info.c | 41 +++++++++++----
net/mac80211/tx.c | 115 ++++++++++++++++++++++++++++++++++++++++--
6 files changed, 258 insertions(+), 28 deletions(-)
--
2.1.4
^ permalink raw reply
* wil6200
From: Vikram @ 2016-09-25 15:15 UTC (permalink / raw)
To: linux-wireless
Hi,
Could you please let me know as to how to change the mode of wil6200
chip from WBE to WiFi?
Regards,
Vikram
^ permalink raw reply
* [PATCHv2] mac80211: check A-MSDU inner frame source address on AP interfaces
From: Michael Braun @ 2016-09-25 11:28 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, projekt-wlan, Michael Braun
When using WPA security, the station and thus the required key is
identified by its mac address when packets are received. So a
station usually cannot spoof its source mac address.
But when a station sends an A-MSDU frame, port control and crypto
is done using the outer mac address, while the packets delivered
and forwarded use the inner mac address.
IEEE 802.11-2012 mandates that the outer source mac address should
match the inner source address (section 8.3.2.2). For the
destination mac address, matching is not required (section 10.23.15).
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
--
v2: fix typo inversed source mac address check
---
net/wireless/util.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index b7d1592..86ca5d2 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -747,13 +747,13 @@ void ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
u16 ethertype;
u8 *payload;
int offset = 0, remaining, err;
- struct ethhdr eth;
+ struct ethhdr eth, eth_80211;
bool reuse_frag = skb->head_frag && !skb_has_frag_list(skb);
bool reuse_skb = false;
bool last = false;
if (has_80211_header) {
- err = __ieee80211_data_to_8023(skb, ð, addr, iftype);
+ err = __ieee80211_data_to_8023(skb, ð_80211, addr, iftype);
if (err)
goto out;
}
@@ -768,6 +768,13 @@ void ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
subframe_len = sizeof(struct ethhdr) + len;
padding = (4 - subframe_len) & 0x3;
+ if (unlikely(has_80211_header &&
+ (iftype == NL80211_IFTYPE_AP ||
+ iftype == NL80211_IFTYPE_AP_VLAN) &&
+ !ether_addr_equal(eth_80211.h_source, eth.h_source)
+ ))
+ goto purge;
+
/* the last MSDU has no padding */
remaining = skb->len - offset;
if (subframe_len > remaining)
--
2.1.4
^ permalink raw reply related
* [PATCH] mac80211: check A-MSDU inner frame source address on AP interfaces
From: Michael Braun @ 2016-09-25 11:25 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, projekt-wlan, Michael Braun
When using WPA security, the station and thus the required key is
identified by its mac address when packets are received. So a
station usually cannot spoof its source mac address.
But when a station sends an A-MSDU frame, port control and crypto
is done using the outer mac address, while the packets delivered
and forwarded use the inner mac address.
IEEE 802.11-2012 mandates that the outer source mac address should
match the inner source address (section 8.3.2.2). For the
destination mac address, matching is not required (section 10.23.15).
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
net/wireless/util.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/wireless/util.c b/net/wireless/util.c
index b7d1592..7ea56fe 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -747,13 +747,13 @@ void ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
u16 ethertype;
u8 *payload;
int offset = 0, remaining, err;
- struct ethhdr eth;
+ struct ethhdr eth, eth_80211;
bool reuse_frag = skb->head_frag && !skb_has_frag_list(skb);
bool reuse_skb = false;
bool last = false;
if (has_80211_header) {
- err = __ieee80211_data_to_8023(skb, ð, addr, iftype);
+ err = __ieee80211_data_to_8023(skb, ð_80211, addr, iftype);
if (err)
goto out;
}
@@ -768,6 +768,13 @@ void ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
subframe_len = sizeof(struct ethhdr) + len;
padding = (4 - subframe_len) & 0x3;
+ if (unlikely(has_80211_header &&
+ (iftype == NL80211_IFTYPE_AP ||
+ iftype == NL80211_IFTYPE_AP_VLAN) &&
+ ether_addr_equal(eth_80211.h_source, eth.h_source))
+ )
+ goto purge;
+
/* the last MSDU has no padding */
remaining = skb->len - offset;
if (subframe_len > remaining)
--
2.1.4
^ permalink raw reply related
* [PATCH] rtl8xxxu: mark rtl8192eu_power_off() static
From: Baoyou Xie @ 2016-09-25 9:22 UTC (permalink / raw)
To: Jes.Sorensen, kvalo
Cc: linux-wireless, netdev, linux-kernel, arnd, baoyou.xie,
xie.baoyou, han.fei, tang.qiang007
We get 1 warning when building kernel with W=1:
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c:1557:6: warning: no previous prototype for 'rtl8192eu_power_off' [-Wmissing-prototypes]
In fact, this function is only used in the file in which it is
declared and don't need a declaration, but can be made static.
so this patch marks this function with 'static'.
Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index df54d27..7b5128a 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -1554,7 +1554,7 @@ static int rtl8192eu_power_on(struct rtl8xxxu_priv *priv)
return ret;
}
-void rtl8192eu_power_off(struct rtl8xxxu_priv *priv)
+static void rtl8192eu_power_off(struct rtl8xxxu_priv *priv)
{
u8 val8;
u16 val16;
--
2.7.4
^ permalink raw reply related
* [PATCH] mac80211: fix CMD_FRAME for AP_VLAN
From: Michael Braun @ 2016-09-25 6:47 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, projekt-wlan, Michael Braun
When using IEEE 802.11r FT OVER-DS roaming with AP_VLAN, hostapd needs to send
out a frame using CMD_FRAME for a station assigned to an AP_VLAN interface.
Right now, the userspace needs to give the exact AP_VLAN interface index
for CMD_FRAME; hostapd does not do this. Additionally, userspace cannot
use GET_STATION to query the AP_VLAN ifidx, as while GET_STATION finds
stations assigned to AP_VLAN even if the AP iface is queried, it does not
return AP_VLAN ifidx (it returns the queried one).
This breaks IEEE 802.11r over_ds with vlans, as the reply frame does not
get out. This patch fixes this by using get_sta_bss for CMD_FRAME.
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
net/mac80211/offchannel.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index 55a9c5b..2afd329 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -819,7 +819,10 @@ int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
mgmt->u.action.category == WLAN_CATEGORY_SPECTRUM_MGMT)
break;
rcu_read_lock();
- sta = sta_info_get(sdata, mgmt->da);
+ if (ieee80211_vif_is_mesh(&sdata->vif))
+ sta = sta_info_get(sdata, mgmt->da);
+ else
+ sta = sta_info_get_bss(sdata, mgmt->da);
rcu_read_unlock();
if (!sta)
return -ENOLINK;
--
2.1.4
^ permalink raw reply related
* device stops working ("failed to install key for vdev" error in kernel log)
From: Martin Blumenstingl @ 2016-09-24 23:10 UTC (permalink / raw)
To: ath10k, linux-wireless
Hello,
sometimes my AP simply stops working.
Typical situation:
- (me coming home from work)
- client (Nexus 5, Xperia Z3 Compact or another ath10k device) can see
my ath10k AP for a brief moment
- AP is gone, errors from below show up in the kernel log
- reboot of my AP is required to fix this
[54064.293597] ath10k_pci 0000:02:00.0: failed to install key for vdev
0 peer [AP MAC addr]: -145
[54064.301234] wlan0: failed to remove key (1, ff:ff:ff:ff:ff:ff) from
hardware (-145)
[54067.305703] ath10k_pci 0000:02:00.0: failed to install key for vdev
0 peer [AP MAC addr]: -145
[54067.313307] wlan0: failed to set key (1, ff:ff:ff:ff:ff:ff) to
hardware (-145)
Some OpenWrt and LEDE users are reporting the same problem: [1]
my AP is a (Lantiq VRX268) MIPS based device running a recent version
of LEDE and I am using ath10k firmware version 10.2.4.70.54. I
attached a part of my boot log at the end of this email in case more
details are needed.
What should I (or any other user who experiences this) do whenever
this happens to help you track down the issue?
Also please let us know if there is anything else you need to know
about the affected systems.
Regards,
Martin
[0] http://www.spinics.net/lists/linux-wireless/msg143735.html
[1] https://dev.openwrt.org/ticket/20854
[ 10.782614] Loading modules backported from Linux version
wt-2016-06-20-0-gbc17424
[ 10.788815] Backport generated by backports.git backports-20160216-7-g5735958
[ 11.567576] PCI: Enabling device 0000:01:00.0 (0000 -> 0002)
[ 11.571941] PCI: Enabling device 0000:02:00.0 (0000 -> 0002)
[ 11.577741] ath10k_pci 0000:02:00.0: pci irq legacy oper_irq_mode 1
irq_mode 0 reset_mode 0
[ 11.760931] ath10k_pci 0000:02:00.0: Direct firmware load for
ath10k/pre-cal-pci-0000:02:00.0.bin failed with error -2
[ 11.770298] ath10k_pci 0000:02:00.0: Falling back to user helper
[ 11.830735] firmware ath10k!pre-cal-pci-0000:02:00.0.bin:
firmware_loading_store: map pages failed
[ 11.838906] ath10k_pci 0000:02:00.0: Direct firmware load for
ath10k/cal-pci-0000:02:00.0.bin failed with error -2
[ 11.848692] ath10k_pci 0000:02:00.0: Falling back to user helper
[ 12.395640] ath10k_pci 0000:02:00.0: qca988x hw2.0 target
0x4100016c chip_id 0x043202ff sub 0000:0000
[ 12.403552] ath10k_pci 0000:02:00.0: kconfig debug 1 debugfs 1
tracing 0 dfs 1 testmode 1
[ 12.419020] ath10k_pci 0000:02:00.0: firmware ver 10.2.4.70.54 api
5 features no-p2p,raw-mode,mfp crc32 9d340dd9
[ 12.428018] ath10k_pci 0000:02:00.0: Direct firmware load for
ath10k/QCA988X/hw2.0/board-2.bin failed with error -2
[ 12.438265] ath10k_pci 0000:02:00.0: Falling back to user helper
[ 12.496901] firmware ath10k!QCA988X!hw2.0!board-2.bin:
firmware_loading_store: map pages failed
[ 12.507714] ath10k_pci 0000:02:00.0: board_file api 1 bmi_id N/A
crc32 bebc7c08
[ 13.562650] ath10k_pci 0000:02:00.0: htt-ver 2.1 wmi-op 5 htt-op 2
cal file max-sta 128 raw 0 hwcrypto 1
[ 13.648696] ath: EEPROM regdomain: 0x833a
[ 13.648721] ath: EEPROM indicates we should expect a country code
[ 13.648742] ath: doing EEPROM country->regdmn map search
[ 13.648758] ath: country maps to regdmn code: 0x37
[ 13.648772] ath: Country alpha2 being used: GB
[ 13.648784] ath: Regpair used: 0x37
^ permalink raw reply
* [PATCH 2/2] brcmfmac: compile fws(ignal) code only with BCDC support enabled
From: Rafał Miłecki @ 2016-09-24 20:44 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless,
brcm80211-dev-list.pdl, netdev, linux-kernel,
Rafał Miłecki
In-Reply-To: <20160924204419.10213-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
It's not needed by the other (msgbuf) protocol, so let's save some size
and compile it conditionally.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
.../wireless/broadcom/brcm80211/brcmfmac/Makefile | 4 +-
.../broadcom/brcm80211/brcmfmac/fwsignal.h | 59 ++++++++++++++++++++++
2 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
index 9e4b505..ad3b06e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile
@@ -27,7 +27,6 @@ brcmfmac-objs += \
chip.o \
fwil.o \
fweh.o \
- fwsignal.o \
p2p.o \
proto.o \
common.o \
@@ -37,7 +36,8 @@ brcmfmac-objs += \
btcoex.o \
vendor.o
brcmfmac-$(CONFIG_BRCMFMAC_PROTO_BCDC) += \
- bcdc.o
+ bcdc.o \
+ fwsignal.o
brcmfmac-$(CONFIG_BRCMFMAC_PROTO_MSGBUF) += \
commonring.o \
flowring.o \
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
index 8f7c1d7..ba0c1bc 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
@@ -18,6 +18,7 @@
#ifndef FWSIGNAL_H_
#define FWSIGNAL_H_
+#ifdef CONFIG_BRCMFMAC_PROTO_BCDC
int brcmf_fws_init(struct brcmf_pub *drvr);
void brcmf_fws_deinit(struct brcmf_pub *drvr);
bool brcmf_fws_skbs_queueing(struct brcmf_fws_info *fws);
@@ -31,5 +32,63 @@ void brcmf_fws_del_interface(struct brcmf_if *ifp);
void brcmf_fws_bustxfail(struct brcmf_fws_info *fws, struct sk_buff *skb);
void brcmf_fws_bus_blocked(struct brcmf_pub *drvr, bool flow_blocked);
void brcmf_fws_rxreorder(struct brcmf_if *ifp, struct sk_buff *skb);
+#else
+static inline int brcmf_fws_init(struct brcmf_pub *drvr)
+{
+ return -ENOTSUPP;
+}
+
+static inline void brcmf_fws_deinit(struct brcmf_pub *drvr)
+{
+}
+
+static inline bool brcmf_fws_skbs_queueing(struct brcmf_fws_info *fws)
+{
+ return false;
+}
+
+static inline bool brcmf_fws_fc_active(struct brcmf_fws_info *fws)
+{
+ return false;
+}
+
+static inline void brcmf_fws_hdrpull(struct brcmf_if *ifp, s16 siglen,
+ struct sk_buff *skb)
+{
+}
+
+static inline int brcmf_fws_process_skb(struct brcmf_if *ifp,
+ struct sk_buff *skb)
+{
+ return -ENOTSUPP;
+}
+
+static inline void brcmf_fws_reset_interface(struct brcmf_if *ifp)
+{
+}
+
+static inline void brcmf_fws_add_interface(struct brcmf_if *ifp)
+{
+}
+
+static inline void brcmf_fws_del_interface(struct brcmf_if *ifp)
+{
+}
+
+static inline void brcmf_fws_bustxfail(struct brcmf_fws_info *fws,
+ struct sk_buff *skb)
+{
+}
+
+static inline void brcmf_fws_bus_blocked(struct brcmf_pub *drvr,
+ bool flow_blocked)
+{
+}
+
+static inline void brcmf_fws_rxreorder(struct brcmf_if *ifp,
+ struct sk_buff *skb)
+{
+}
+#endif
#endif /* FWSIGNAL_H_ */
--
2.9.3
^ permalink raw reply related
* [PATCH 1/2] brcmfmac: initialize fws(ignal) for BCDC protocol only
From: Rafał Miłecki @ 2016-09-24 20:44 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless,
brcm80211-dev-list.pdl, netdev, linux-kernel,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
There are two protocols used by Broadcom FullMAC devices: BCDC and
msgbuf. They use different ways for (some part of) communication with
the firmware. Firmware Signaling is required for the first one only
(BCDC).
So far we were always initializing fws and always calling it's skb
processing function. It was fws that was passing skb processing to the
protocol specific function. It was redundant for the msgbuf case.
Simply taking few lines of code out of fws allows us to totally avoid
using it. This simplifies code flow, saves some memory & will allow
further optimizations like not compiling fwsignal.c.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 24 ++++++++++++++++------
.../broadcom/brcm80211/brcmfmac/fwsignal.c | 17 ++++++---------
.../broadcom/brcm80211/brcmfmac/fwsignal.h | 1 +
3 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 27cd50a..bc3d8ab 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -250,7 +250,17 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
if (eh->h_proto == htons(ETH_P_PAE))
atomic_inc(&ifp->pend_8021x_cnt);
- ret = brcmf_fws_process_skb(ifp, skb);
+ /* determine the priority */
+ if (skb->priority == 0 || skb->priority > 7)
+ skb->priority = cfg80211_classify8021d(skb, NULL);
+
+ if (drvr->fws && brcmf_fws_skbs_queueing(drvr->fws)) {
+ ret = brcmf_fws_process_skb(ifp, skb);
+ } else {
+ ret = brcmf_proto_txdata(drvr, ifp->ifidx, 0, skb);
+ if (ret < 0)
+ brcmf_txfinalize(ifp, skb, false);
+ }
done:
if (ret) {
@@ -405,7 +415,7 @@ void brcmf_txcomplete(struct device *dev, struct sk_buff *txp, bool success)
struct brcmf_if *ifp;
/* await txstatus signal for firmware if active */
- if (brcmf_fws_fc_active(drvr->fws)) {
+ if (drvr->fws && brcmf_fws_fc_active(drvr->fws)) {
if (!success)
brcmf_fws_bustxfail(drvr->fws, txp);
} else {
@@ -1006,11 +1016,13 @@ int brcmf_bus_start(struct device *dev)
}
brcmf_feat_attach(drvr);
- ret = brcmf_fws_init(drvr);
- if (ret < 0)
- goto fail;
+ if (bus_if->proto_type == BRCMF_PROTO_BCDC) {
+ ret = brcmf_fws_init(drvr);
+ if (ret < 0)
+ goto fail;
- brcmf_fws_add_interface(ifp);
+ brcmf_fws_add_interface(ifp);
+ }
drvr->config = brcmf_cfg80211_attach(drvr, bus_if->dev,
drvr->settings->p2p_enable);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
index a190f53..495eaf8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
@@ -2100,16 +2100,6 @@ int brcmf_fws_process_skb(struct brcmf_if *ifp, struct sk_buff *skb)
int rc = 0;
brcmf_dbg(DATA, "tx proto=0x%X\n", ntohs(eh->h_proto));
- /* determine the priority */
- if ((skb->priority == 0) || (skb->priority > 7))
- skb->priority = cfg80211_classify8021d(skb, NULL);
-
- if (fws->avoid_queueing) {
- rc = brcmf_proto_txdata(drvr, ifp->ifidx, 0, skb);
- if (rc < 0)
- brcmf_txfinalize(ifp, skb, false);
- return rc;
- }
/* set control buffer information */
skcb->if_flags = 0;
@@ -2155,7 +2145,7 @@ void brcmf_fws_add_interface(struct brcmf_if *ifp)
struct brcmf_fws_info *fws = ifp->drvr->fws;
struct brcmf_fws_mac_descriptor *entry;
- if (!ifp->ndev)
+ if (!fws || !ifp->ndev)
return;
entry = &fws->desc.iface[ifp->ifidx];
@@ -2442,6 +2432,11 @@ void brcmf_fws_deinit(struct brcmf_pub *drvr)
kfree(fws);
}
+bool brcmf_fws_skbs_queueing(struct brcmf_fws_info *fws)
+{
+ return !fws->avoid_queueing;
+}
+
bool brcmf_fws_fc_active(struct brcmf_fws_info *fws)
{
if (!fws->creditmap_received)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
index ef0ad85..8f7c1d7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
@@ -20,6 +20,7 @@
int brcmf_fws_init(struct brcmf_pub *drvr);
void brcmf_fws_deinit(struct brcmf_pub *drvr);
+bool brcmf_fws_skbs_queueing(struct brcmf_fws_info *fws);
bool brcmf_fws_fc_active(struct brcmf_fws_info *fws);
void brcmf_fws_hdrpull(struct brcmf_if *ifp, s16 siglen, struct sk_buff *skb);
int brcmf_fws_process_skb(struct brcmf_if *ifp, struct sk_buff *skb);
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] realtek: Add switch variable to 'switch case not processed' messages
From: Jes Sorensen @ 2016-09-24 20:35 UTC (permalink / raw)
To: Joe Perches
Cc: Larry Finger, Jean Delvare, Chaoming Li, Kalle Valo,
linux-wireless, netdev, linux-kernel
In-Reply-To: <1474748954.23838.21.camel@perches.com>
Joe Perches <joe@perches.com> writes:
> On Sat, 2016-09-24 at 14:06 -0500, Larry Finger wrote:
>> On 09/24/2016 12:32 PM, Joe Perches wrote:
> []
>> o Reindent all the switch/case blocks to a more normal
>> kernel style (git diff -w would show no changes here)
>> That sounds like busy work to me, but if you want to do it, go ahead.
>
> It's really just to make the comparison case block reductions
> easier to verify for later steps done
>
>> > o cast, spacing and parenthesis reductions
>> > Lots of odd and somewhat unique styles in various
>> > drivers, looks like too many individual authors without
>> > a style guide / code enforcer using slightly different
>> > personalized code. Glancing at the code, it looks to be
>> > similar logic, just written in different styles.
>> Same comment.
>
> Same rationale
>
>> > o Logic changes like
>> > from:
>> > if (foo) func(..., bar, ...); else func(..., baz, ...);
>> > to:
>> > func(..., foo ? bar : baz, ...);
>> > to make the case statement code blocks more consistent
>> > and emit somewhat smaller object code.
>> I find if .. else constructs much easier to read than the cond ? xxxx : yyyy
>> form. I would reject any such patches.
>
> <shrug> I think object code reduction generally a good thing
> but then again, I'm not a maintainer here.
I missed this part, but I am with Larry here - 'foo ? bar : boo' are
just obfuscating the code and far less clear than if or switch
statements.
Jes
^ permalink raw reply
* Re: [PATCH] realtek: Add switch variable to 'switch case not processed' messages
From: Joe Perches @ 2016-09-24 20:29 UTC (permalink / raw)
To: Larry Finger, Jean Delvare, Jes Sorensen
Cc: Chaoming Li, Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <cef6d8da-c390-f435-7a5e-d294dfbebbfb@lwfinger.net>
On Sat, 2016-09-24 at 14:06 -0500, Larry Finger wrote:
> On 09/24/2016 12:32 PM, Joe Perches wrote:
[]
> o Reindent all the switch/case blocks to a more normal
> kernel style (git diff -w would show no changes here)
> That sounds like busy work to me, but if you want to do it, go ahead.
It's really just to make the comparison case block reductions
easier to verify for later steps done
> > o cast, spacing and parenthesis reductions
> > Lots of odd and somewhat unique styles in various
> > drivers, looks like too many individual authors without
> > a style guide / code enforcer using slightly different
> > personalized code. Glancing at the code, it looks to be
> > similar logic, just written in different styles.
> Same comment.
Same rationale
> > o Logic changes like
> > from:
> > if (foo) func(..., bar, ...); else func(..., baz, ...);
> > to:
> > func(..., foo ? bar : baz, ...);
> > to make the case statement code blocks more consistent
> > and emit somewhat smaller object code.
> I find if .. else constructs much easier to read than the cond ? xxxx : yyyy
> form. I would reject any such patches.
<shrug> I think object code reduction generally a good thing
but then again, I'm not a maintainer here.
> > o Consolidation of equivalent function spanning drivers
> > With the style only changes minimized, where possible
> > make the drivers use common ops/callback functions.
> The is no question that there are similar routines in different drivers. I would
> like to place as much as possible into common routines, but I never seem to find
> the time. There are too many bugs in other things I support to consider these
> niceties.
Consolidation generally reduces defects and improves ease of
updating.
>
^ permalink raw reply
* Re: [PATCH] realtek: Add switch variable to 'switch case not processed' messages
From: Jes Sorensen @ 2016-09-24 20:02 UTC (permalink / raw)
To: Larry Finger
Cc: Joe Perches, Jean Delvare, Chaoming Li, Kalle Valo,
linux-wireless, netdev, linux-kernel
In-Reply-To: <cef6d8da-c390-f435-7a5e-d294dfbebbfb@lwfinger.net>
Larry Finger <Larry.Finger@lwfinger.net> writes:
> On 09/24/2016 12:32 PM, Joe Perches wrote:
>> Is there any value in that or is Jes' work going to make
>> doing any or all of this unnecessary and futile?
>
> That is not yet determined. The only driver that is to be replaced at
> this point is rtl8192cu. Jes only has USB I/O for his driver. We are
> looking at adding SDIO, and once that is done, PCI should be possible.
If someone else wants to address PCI then it could happen quite soon,
but at the current schedule I don't see PCI happen in my driver for at
least a year, probably more.
If you can reduce the size of rtlwifi in the mean time that probably
isn't going to upset a lot of people.
Jes
^ permalink raw reply
* Re: [PATCH] realtek: Add switch variable to 'switch case not processed' messages
From: Larry Finger @ 2016-09-24 19:06 UTC (permalink / raw)
To: Joe Perches, Jean Delvare, Jes Sorensen
Cc: Chaoming Li, Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <1474738358.23838.11.camel@perches.com>
On 09/24/2016 12:32 PM, Joe Perches wrote:
> (adding Jes Sorensen to recipients)
>
> On Sat, 2016-09-24 at 11:35 -0500, Larry Finger wrote:
>> I have patches that makes HAL_DEF_WOWLAN be a no-op for the rest of the drivers,
>> and one that sets the enum values for that particular statement to hex values. I
>> also looked at the other large enums and decided that they never need the human
>> lookup.
>
> Hey Larry.
>
> There are many somewhat common realtek wireless drivers.
>
> Not to step on your toes, but what do you think of
> rationalizing the switch/case statements of all the
> realtek drivers in a few steps:
>
> o Reindent all the switch/case blocks to a more normal
> kernel style (git diff -w would show no changes here)
That sounds like busy work to me, but if you want to do it, go ahead.
> o cast, spacing and parenthesis reductions
> Lots of odd and somewhat unique styles in various
> drivers, looks like too many individual authors without
> a style guide / code enforcer using slightly different
> personalized code. Glancing at the code, it looks to be
> similar logic, just written in different styles.
Same comment.
> o Logic changes like
> from:
> if (foo) func(..., bar, ...); else func(..., baz, ...);
> to:
> func(..., foo ? bar : baz, ...);
> to make the case statement code blocks more consistent
> and emit somewhat smaller object code.
I find if .. else constructs much easier to read than the cond ? xxxx : yyyy
form. I would reject any such patches.
> o Consolidation of equivalent function spanning drivers
> With the style only changes minimized, where possible
> make the drivers use common ops/callback functions.
The is no question that there are similar routines in different drivers. I would
like to place as much as possible into common routines, but I never seem to find
the time. There are too many bugs in other things I support to consider these
niceties.
> Is there any value in that or is Jes' work going to make
> doing any or all of this unnecessary and futile?
That is not yet determined. The only driver that is to be replaced at this point
is rtl8192cu. Jes only has USB I/O for his driver. We are looking at adding
SDIO, and once that is done, PCI should be possible.
Larry
^ permalink raw reply
* Re: [PATCH 2/2] rtlwifi: Add explicit values to hw_variables enum
From: Larry Finger @ 2016-09-24 18:06 UTC (permalink / raw)
To: Joe Perches, kvalo; +Cc: devel, linux-wireless
In-Reply-To: <1474738987.23838.13.camel@perches.com>
On 09/24/2016 12:43 PM, Joe Perches wrote:
> On Sat, 2016-09-24 at 11:57 -0500, Larry Finger wrote:
>> The entries in this enum may be referenced in debug output. Adding explicit
>> values simplifies the lookup.
>
> A negative aspect to this style is forced renumbering
> if a HW_VAR entry is inserted.
>
> Is that possible?
Any new one would be added at the end.
Larry
^ permalink raw reply
* Re: [PATCH 1/2] rtlwifi: Add HAL_DEF_WOWLAN case to *_get_hw() routines
From: Joe Perches @ 2016-09-24 17:47 UTC (permalink / raw)
To: Larry Finger, kvalo; +Cc: devel, linux-wireless
In-Reply-To: <1474736239-9826-2-git-send-email-Larry.Finger@lwfinger.net>
On Sat, 2016-09-24 at 11:57 -0500, Larry Finger wrote:
> Only rtl8821ae implements WOWLAN; however, the other drivers may receive
> a call requesting information about this mode. The other drivers need to
> ignore the request rather than logging that the default branch of the
> switch statement has been reached.
How about later rationalizing the DBG_EMERG vs DBG_LOUD differences too?
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c
[]
> @@ -355,6 +355,8 @@ void rtl88ee_get_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
[]
> default:
> RT_TRACE(rtlpriv, COMP_ERR, DBG_EMERG,
> "switch case %#x not processed\n", variable);
[]
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c
[]
> @@ -141,6 +141,8 @@ void rtl8723e_get_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
[]
> default:
> RT_TRACE(rtlpriv, COMP_ERR, DBG_LOUD,
> "switch case %#x not processed\n", variable);
etc...
^ permalink raw reply
* Re: [PATCH 2/2] rtlwifi: Add explicit values to hw_variables enum
From: Joe Perches @ 2016-09-24 17:43 UTC (permalink / raw)
To: Larry Finger, kvalo; +Cc: devel, linux-wireless
In-Reply-To: <1474736239-9826-3-git-send-email-Larry.Finger@lwfinger.net>
On Sat, 2016-09-24 at 11:57 -0500, Larry Finger wrote:
> The entries in this enum may be referenced in debug output. Adding explicit
> values simplifies the lookup.
A negative aspect to this style is forced renumbering
if a HW_VAR entry is inserted.
Is that possible?
^ permalink raw reply
* Re: [PATCH] realtek: Add switch variable to 'switch case not processed' messages
From: Joe Perches @ 2016-09-24 17:32 UTC (permalink / raw)
To: Larry Finger, Jean Delvare, Jes Sorensen
Cc: Chaoming Li, Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <49094659-662d-a902-6740-ea3d1fea6660@lwfinger.net>
(adding Jes Sorensen to recipients)
On Sat, 2016-09-24 at 11:35 -0500, Larry Finger wrote:
> I have patches that makes HAL_DEF_WOWLAN be a no-op for the rest of the drivers,
> and one that sets the enum values for that particular statement to hex values. I
> also looked at the other large enums and decided that they never need the human
> lookup.
Hey Larry.
There are many somewhat common realtek wireless drivers.
Not to step on your toes, but what do you think of
rationalizing the switch/case statements of all the
realtek drivers in a few steps:
o Reindent all the switch/case blocks to a more normal
kernel style (git diff -w would show no changes here)
o cast, spacing and parenthesis reductions
Lots of odd and somewhat unique styles in various
drivers, looks like too many individual authors without
a style guide / code enforcer using slightly different
personalized code. Glancing at the code, it looks to be
similar logic, just written in different styles.
o Logic changes like
from:
if (foo) func(..., bar, ...); else func(..., baz, ...);
to:
func(..., foo ? bar : baz, ...);
to make the case statement code blocks more consistent
and emit somewhat smaller object code.
o Consolidation of equivalent function spanning drivers
With the style only changes minimized, where possible
make the drivers use common ops/callback functions.
Is there any value in that or is Jes' work going to make
doing any or all of this unnecessary and futile?
^ permalink raw reply
* [PATCH 2/2] rtlwifi: Add explicit values to hw_variables enum
From: Larry Finger @ 2016-09-24 16:57 UTC (permalink / raw)
To: kvalo; +Cc: devel, linux-wireless, Larry Finger
In-Reply-To: <1474736239-9826-1-git-send-email-Larry.Finger@lwfinger.net>
The entries in this enum may be referenced in debug output. Adding explicit
values simplifies the lookup.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
drivers/net/wireless/realtek/rtlwifi/wifi.h | 208 ++++++++++++++--------------
1 file changed, 104 insertions(+), 104 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h b/drivers/net/wireless/realtek/rtlwifi/wifi.h
index c5086c2..595f7d5 100644
--- a/drivers/net/wireless/realtek/rtlwifi/wifi.h
+++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h
@@ -394,110 +394,110 @@ enum io_type {
};
enum hw_variables {
- HW_VAR_ETHER_ADDR,
- HW_VAR_MULTICAST_REG,
- HW_VAR_BASIC_RATE,
- HW_VAR_BSSID,
- HW_VAR_MEDIA_STATUS,
- HW_VAR_SECURITY_CONF,
- HW_VAR_BEACON_INTERVAL,
- HW_VAR_ATIM_WINDOW,
- HW_VAR_LISTEN_INTERVAL,
- HW_VAR_CS_COUNTER,
- HW_VAR_DEFAULTKEY0,
- HW_VAR_DEFAULTKEY1,
- HW_VAR_DEFAULTKEY2,
- HW_VAR_DEFAULTKEY3,
- HW_VAR_SIFS,
- HW_VAR_R2T_SIFS,
- HW_VAR_DIFS,
- HW_VAR_EIFS,
- HW_VAR_SLOT_TIME,
- HW_VAR_ACK_PREAMBLE,
- HW_VAR_CW_CONFIG,
- HW_VAR_CW_VALUES,
- HW_VAR_RATE_FALLBACK_CONTROL,
- HW_VAR_CONTENTION_WINDOW,
- HW_VAR_RETRY_COUNT,
- HW_VAR_TR_SWITCH,
- HW_VAR_COMMAND,
- HW_VAR_WPA_CONFIG,
- HW_VAR_AMPDU_MIN_SPACE,
- HW_VAR_SHORTGI_DENSITY,
- HW_VAR_AMPDU_FACTOR,
- HW_VAR_MCS_RATE_AVAILABLE,
- HW_VAR_AC_PARAM,
- HW_VAR_ACM_CTRL,
- HW_VAR_DIS_Req_Qsize,
- HW_VAR_CCX_CHNL_LOAD,
- HW_VAR_CCX_NOISE_HISTOGRAM,
- HW_VAR_CCX_CLM_NHM,
- HW_VAR_TxOPLimit,
- HW_VAR_TURBO_MODE,
- HW_VAR_RF_STATE,
- HW_VAR_RF_OFF_BY_HW,
- HW_VAR_BUS_SPEED,
- HW_VAR_SET_DEV_POWER,
-
- HW_VAR_RCR,
- HW_VAR_RATR_0,
- HW_VAR_RRSR,
- HW_VAR_CPU_RST,
- HW_VAR_CHECK_BSSID,
- HW_VAR_LBK_MODE,
- HW_VAR_AES_11N_FIX,
- HW_VAR_USB_RX_AGGR,
- HW_VAR_USER_CONTROL_TURBO_MODE,
- HW_VAR_RETRY_LIMIT,
- HW_VAR_INIT_TX_RATE,
- HW_VAR_TX_RATE_REG,
- HW_VAR_EFUSE_USAGE,
- HW_VAR_EFUSE_BYTES,
- HW_VAR_AUTOLOAD_STATUS,
- HW_VAR_RF_2R_DISABLE,
- HW_VAR_SET_RPWM,
- HW_VAR_H2C_FW_PWRMODE,
- HW_VAR_H2C_FW_JOINBSSRPT,
- HW_VAR_H2C_FW_MEDIASTATUSRPT,
- HW_VAR_H2C_FW_P2P_PS_OFFLOAD,
- HW_VAR_FW_PSMODE_STATUS,
- HW_VAR_INIT_RTS_RATE,
- HW_VAR_RESUME_CLK_ON,
- HW_VAR_FW_LPS_ACTION,
- HW_VAR_1X1_RECV_COMBINE,
- HW_VAR_STOP_SEND_BEACON,
- HW_VAR_TSF_TIMER,
- HW_VAR_IO_CMD,
-
- HW_VAR_RF_RECOVERY,
- HW_VAR_H2C_FW_UPDATE_GTK,
- HW_VAR_WF_MASK,
- HW_VAR_WF_CRC,
- HW_VAR_WF_IS_MAC_ADDR,
- HW_VAR_H2C_FW_OFFLOAD,
- HW_VAR_RESET_WFCRC,
-
- HW_VAR_HANDLE_FW_C2H,
- HW_VAR_DL_FW_RSVD_PAGE,
- HW_VAR_AID,
- HW_VAR_HW_SEQ_ENABLE,
- HW_VAR_CORRECT_TSF,
- HW_VAR_BCN_VALID,
- HW_VAR_FWLPS_RF_ON,
- HW_VAR_DUAL_TSF_RST,
- HW_VAR_SWITCH_EPHY_WoWLAN,
- HW_VAR_INT_MIGRATION,
- HW_VAR_INT_AC,
- HW_VAR_RF_TIMING,
-
- HAL_DEF_WOWLAN,
- HW_VAR_MRC,
- HW_VAR_KEEP_ALIVE,
- HW_VAR_NAV_UPPER,
-
- HW_VAR_MGT_FILTER,
- HW_VAR_CTRL_FILTER,
- HW_VAR_DATA_FILTER,
+ HW_VAR_ETHER_ADDR = 0x0,
+ HW_VAR_MULTICAST_REG = 0x1,
+ HW_VAR_BASIC_RATE = 0x2,
+ HW_VAR_BSSID = 0x3,
+ HW_VAR_MEDIA_STATUS= 0x4,
+ HW_VAR_SECURITY_CONF= 0x5,
+ HW_VAR_BEACON_INTERVAL = 0x6,
+ HW_VAR_ATIM_WINDOW = 0x7,
+ HW_VAR_LISTEN_INTERVAL = 0x8,
+ HW_VAR_CS_COUNTER = 0x9,
+ HW_VAR_DEFAULTKEY0 = 0xa,
+ HW_VAR_DEFAULTKEY1 = 0xb,
+ HW_VAR_DEFAULTKEY2 = 0xc,
+ HW_VAR_DEFAULTKEY3 = 0xd,
+ HW_VAR_SIFS = 0xe,
+ HW_VAR_R2T_SIFS = 0xf,
+ HW_VAR_DIFS = 0x10,
+ HW_VAR_EIFS = 0x11,
+ HW_VAR_SLOT_TIME = 0x12,
+ HW_VAR_ACK_PREAMBLE = 0x13,
+ HW_VAR_CW_CONFIG = 0x14,
+ HW_VAR_CW_VALUES = 0x15,
+ HW_VAR_RATE_FALLBACK_CONTROL= 0x16,
+ HW_VAR_CONTENTION_WINDOW = 0x17,
+ HW_VAR_RETRY_COUNT = 0x18,
+ HW_VAR_TR_SWITCH = 0x19,
+ HW_VAR_COMMAND = 0x1a,
+ HW_VAR_WPA_CONFIG = 0x1b,
+ HW_VAR_AMPDU_MIN_SPACE = 0x1c,
+ HW_VAR_SHORTGI_DENSITY = 0x1d,
+ HW_VAR_AMPDU_FACTOR = 0x1e,
+ HW_VAR_MCS_RATE_AVAILABLE = 0x1f,
+ HW_VAR_AC_PARAM = 0x20,
+ HW_VAR_ACM_CTRL = 0x21,
+ HW_VAR_DIS_Req_Qsize = 0x22,
+ HW_VAR_CCX_CHNL_LOAD = 0x23,
+ HW_VAR_CCX_NOISE_HISTOGRAM = 0x24,
+ HW_VAR_CCX_CLM_NHM = 0x25,
+ HW_VAR_TxOPLimit = 0x26,
+ HW_VAR_TURBO_MODE = 0x27,
+ HW_VAR_RF_STATE = 0x28,
+ HW_VAR_RF_OFF_BY_HW = 0x29,
+ HW_VAR_BUS_SPEED = 0x2a,
+ HW_VAR_SET_DEV_POWER = 0x2b,
+
+ HW_VAR_RCR = 0x2c,
+ HW_VAR_RATR_0 = 0x2d,
+ HW_VAR_RRSR = 0x2e,
+ HW_VAR_CPU_RST = 0x2f,
+ HW_VAR_CHECK_BSSID = 0x30,
+ HW_VAR_LBK_MODE = 0x31,
+ HW_VAR_AES_11N_FIX = 0x32,
+ HW_VAR_USB_RX_AGGR = 0x33,
+ HW_VAR_USER_CONTROL_TURBO_MODE = 0x34,
+ HW_VAR_RETRY_LIMIT = 0x35,
+ HW_VAR_INIT_TX_RATE = 0x36,
+ HW_VAR_TX_RATE_REG = 0x37,
+ HW_VAR_EFUSE_USAGE = 0x38,
+ HW_VAR_EFUSE_BYTES = 0x39,
+ HW_VAR_AUTOLOAD_STATUS = 0x3a,
+ HW_VAR_RF_2R_DISABLE = 0x3b,
+ HW_VAR_SET_RPWM = 0x3c,
+ HW_VAR_H2C_FW_PWRMODE = 0x3d,
+ HW_VAR_H2C_FW_JOINBSSRPT = 0x3e,
+ HW_VAR_H2C_FW_MEDIASTATUSRPT = 0x3f,
+ HW_VAR_H2C_FW_P2P_PS_OFFLOAD = 0x40,
+ HW_VAR_FW_PSMODE_STATUS = 0x41,
+ HW_VAR_INIT_RTS_RATE = 0x42,
+ HW_VAR_RESUME_CLK_ON = 0x43,
+ HW_VAR_FW_LPS_ACTION = 0x44,
+ HW_VAR_1X1_RECV_COMBINE = 0x45,
+ HW_VAR_STOP_SEND_BEACON = 0x46,
+ HW_VAR_TSF_TIMER = 0x47,
+ HW_VAR_IO_CMD = 0x48,
+
+ HW_VAR_RF_RECOVERY = 0x49,
+ HW_VAR_H2C_FW_UPDATE_GTK = 0x4a,
+ HW_VAR_WF_MASK = 0x4b,
+ HW_VAR_WF_CRC = 0x4c,
+ HW_VAR_WF_IS_MAC_ADDR = 0x4d,
+ HW_VAR_H2C_FW_OFFLOAD = 0x4e,
+ HW_VAR_RESET_WFCRC = 0x4f,
+
+ HW_VAR_HANDLE_FW_C2H = 0x50,
+ HW_VAR_DL_FW_RSVD_PAGE = 0x51,
+ HW_VAR_AID = 0x52,
+ HW_VAR_HW_SEQ_ENABLE = 0x53,
+ HW_VAR_CORRECT_TSF = 0x54,
+ HW_VAR_BCN_VALID = 0x55,
+ HW_VAR_FWLPS_RF_ON = 0x56,
+ HW_VAR_DUAL_TSF_RST = 0x57,
+ HW_VAR_SWITCH_EPHY_WoWLAN = 0x58,
+ HW_VAR_INT_MIGRATION = 0x59,
+ HW_VAR_INT_AC = 0x5a,
+ HW_VAR_RF_TIMING = 0x5b,
+
+ HAL_DEF_WOWLAN = 0x5c,
+ HW_VAR_MRC = 0x5d,
+ HW_VAR_KEEP_ALIVE = 0x5e,
+ HW_VAR_NAV_UPPER = 0x5f,
+
+ HW_VAR_MGT_FILTER = 0x60,
+ HW_VAR_CTRL_FILTER = 0x61,
+ HW_VAR_DATA_FILTER = 0x62,
};
enum rt_media_status {
--
2.6.6
^ permalink raw reply related
* [PATCH 1/2] rtlwifi: Add HAL_DEF_WOWLAN case to *_get_hw() routines
From: Larry Finger @ 2016-09-24 16:57 UTC (permalink / raw)
To: kvalo; +Cc: devel, linux-wireless, Larry Finger
In-Reply-To: <1474736239-9826-1-git-send-email-Larry.Finger@lwfinger.net>
Only rtl8821ae implements WOWLAN; however, the other drivers may receive
a call requesting information about this mode. The other drivers need to
ignore the request rather than logging that the default branch of the
switch statement has been reached.
Reported by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
This patch depends on "realtek: Add switch variable to 'switch case not
processed' messages" submitted by Joe Perches.
Larry
---
drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c | 2 ++
drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c | 2 ++
drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c | 2 ++
drivers/net/wireless/realtek/rtlwifi/rtl8192ee/hw.c | 2 ++
drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c | 2 ++
drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c | 2 ++
drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c | 2 ++
7 files changed, 14 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c
index 3285117..37d6efc 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/hw.c
@@ -355,6 +355,8 @@ void rtl88ee_get_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
*((u64 *)(val)) = tsf;
break; }
+ case HAL_DEF_WOWLAN:
+ break;
default:
RT_TRACE(rtlpriv, COMP_ERR, DBG_EMERG,
"switch case %#x not processed\n", variable);
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
index 6d308f9..a47be73 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/hw.c
@@ -141,6 +141,8 @@ void rtl92ce_get_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
break;
}
+ case HAL_DEF_WOWLAN:
+ break;
default:
RT_TRACE(rtlpriv, COMP_ERR, DBG_EMERG,
"switch case %#x not processed\n", variable);
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
index 5369011..d91f8bb 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/hw.c
@@ -164,6 +164,8 @@ void rtl92de_get_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
case HW_VAR_INT_AC:
*((bool *)(val)) = rtlpriv->dm.disable_tx_int;
break;
+ case HAL_DEF_WOWLAN:
+ break;
default:
RT_TRACE(rtlpriv, COMP_ERR, DBG_EMERG,
"switch case %#x not processed\n", variable);
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/hw.c
index 47bb6d8..ebf663e 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/hw.c
@@ -338,6 +338,8 @@ void rtl92ee_get_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
*((u64 *)(val)) = tsf;
}
break;
+ case HAL_DEF_WOWLAN:
+ break;
default:
RT_TRACE(rtlpriv, COMP_ERR, DBG_DMESG,
"switch case %#x not processed\n", variable);
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c
index 5bad9c9..52e4430 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c
@@ -77,6 +77,8 @@ void rtl92se_get_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
*((bool *)(val)) = rtlpriv->dm.current_mrc_switch;
break;
}
+ case HAL_DEF_WOWLAN:
+ break;
default: {
RT_TRACE(rtlpriv, COMP_ERR, DBG_EMERG,
"switch case %#x not processed\n", variable);
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c
index 2bf603b..f8be0bd 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c
@@ -141,6 +141,8 @@ void rtl8723e_get_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
break;
}
+ case HAL_DEF_WOWLAN:
+ break;
default:
RT_TRACE(rtlpriv, COMP_ERR, DBG_LOUD,
"switch case %#x not processed\n", variable);
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
index 999c1ac..aba60c3 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/hw.c
@@ -348,6 +348,8 @@ void rtl8723be_get_hw_reg(struct ieee80211_hw *hw, u8 variable, u8 *val)
*((u64 *)(val)) = tsf;
}
break;
+ case HAL_DEF_WOWLAN:
+ break;
default:
RT_TRACE(rtlpriv, COMP_ERR, DBG_LOUD,
"switch case %#x not processed\n", variable);
--
2.6.6
^ permalink raw reply related
* [PATCH 0/2] rtlwifi: Add missing values to case statements and simplify debugging
From: Larry Finger @ 2016-09-24 16:57 UTC (permalink / raw)
To: kvalo; +Cc: devel, linux-wireless, Larry Finger
Routines *_get_hw_reg() for some of the drivers were missing entries for
HAL_DEF_WOWLAN. No action should be taken for these drivers.
Patch 1 depends on "realtek: Add switch variable to 'switch case not
processed' messages" submitted by Joe Perches. That patch will log
missing cases with the hex value. To aid in debugging, the values for
entries in the hardware variables enum are explicitly defined in hex.
Larry Finger (2):
rtlwifi: Add HAL_DEF_WOWLAN case to *_get_hw() routines
rtlwifi: Add explicit values to hw_variables enum
.../net/wireless/realtek/rtlwifi/rtl8188ee/hw.c | 2 +
.../net/wireless/realtek/rtlwifi/rtl8192ce/hw.c | 2 +
.../net/wireless/realtek/rtlwifi/rtl8192de/hw.c | 2 +
.../net/wireless/realtek/rtlwifi/rtl8192ee/hw.c | 2 +
.../net/wireless/realtek/rtlwifi/rtl8192se/hw.c | 2 +
.../net/wireless/realtek/rtlwifi/rtl8723ae/hw.c | 2 +
.../net/wireless/realtek/rtlwifi/rtl8723be/hw.c | 2 +
drivers/net/wireless/realtek/rtlwifi/wifi.h | 208 ++++++++++-----------
8 files changed, 118 insertions(+), 104 deletions(-)
--
2.6.6
^ permalink raw reply
* Re: [PATCH] realtek: Add switch variable to 'switch case not processed' messages
From: Larry Finger @ 2016-09-24 16:35 UTC (permalink / raw)
To: Joe Perches, Jean Delvare
Cc: Chaoming Li, Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <1474733754.23838.3.camel@perches.com>
On 09/24/2016 11:15 AM, Joe Perches wrote:
> On Sat, 2016-09-24 at 17:55 +0200, Jean Delvare wrote:
>> Would it make sense to explicitly set the enum values, or add them as
>> comments, to make such look-ups easier?
>
> If you want to create enum->#ENUM structs and
> "const char *" lookup functions, please be my guest.
>
> otherwise, hex is at least a consistent way to display
> what should be infrequent output.
Displaying those values as hex is OK. As Joe says, they will not be shown very
often.
I have patches that makes HAL_DEF_WOWLAN be a no-op for the rest of the drivers,
and one that sets the enum values for that particular statement to hex values. I
also looked at the other large enums and decided that they never need the human
lookup.
Larry
^ permalink raw reply
* Re: [PATCH] realtek: Add switch variable to 'switch case not processed' messages
From: Joe Perches @ 2016-09-24 16:15 UTC (permalink / raw)
To: Jean Delvare
Cc: Larry Finger, Chaoming Li, Kalle Valo, linux-wireless, netdev,
linux-kernel
In-Reply-To: <20160924175555.01cae6dd@endymion>
On Sat, 2016-09-24 at 17:55 +0200, Jean Delvare wrote:
> Would it make sense to explicitly set the enum values, or add them as
> comments, to make such look-ups easier?
If you want to create enum->#ENUM structs and
"const char *" lookup functions, please be my guest.
otherwise, hex is at least a consistent way to display
what should be infrequent output.
^ permalink raw reply
* Re: [PATCH] realtek: Add switch variable to 'switch case not processed' messages
From: Jean Delvare @ 2016-09-24 15:55 UTC (permalink / raw)
To: Joe Perches
Cc: Larry Finger, Chaoming Li, Kalle Valo, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1474657363.1849.9.camel@perches.com>
Hi Joe, Larry,
On Fri, 23 Sep 2016 12:02:43 -0700, Joe Perches wrote:
> On Fri, 2016-09-23 at 13:59 -0500, Larry Finger wrote:
> > I'm not familiar with the %#x format. What does it do?
>
> Outputs SPECIAL prefix, it's the same as "0x%x"
>
> lib/vsprintf.c:
> #define SPECIAL 64 /* prefix hex with "0x", octal with "0" */
Is hexadecimal actually the best way to display these values? I guess it
depends how they are listed in the datasheets (if there's anything like
that for these chips?)
I found it a bit difficult to look up the meaning of the value.
HAL_DEF_WOWLAN is an enum value, the number is not set and there's no
comment. I had to count the line numbers, taking blank lines into
account... I ended up pasting the whole enum to a random C file and
printing the value of HAL_DEF_WOWLAN to make sure it was 92.
Would it make sense to explicitly set the enum values, or add them as
comments, to make such look-ups easier?
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
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