* [PATCH net-next v3] bridge: multicast to unicast
From: Linus Lüssing @ 2017-01-19 0:23 UTC (permalink / raw)
To: netdev
Cc: David S . Miller, Stephen Hemminger, Felix Fietkau,
Nikolay Aleksandrov, bridge, linux-kernel, linux-wireless,
Linus Lüssing
Implements an optional, per bridge port flag and feature to deliver
multicast packets to any host on the according port via unicast
individually. This is done by copying the packet per host and
changing the multicast destination MAC to a unicast one accordingly.
multicast-to-unicast works on top of the multicast snooping feature of
the bridge. Which means unicast copies are only delivered to hosts which
are interested in it and signalized this via IGMP/MLD reports
previously.
This feature is intended for interface types which have a more reliable
and/or efficient way to deliver unicast packets than broadcast ones
(e.g. wifi).
However, it should only be enabled on interfaces where no IGMPv2/MLDv1
report suppression takes place. This feature is disabled by default.
The initial patch and idea is from Felix Fietkau.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
[linus.luessing@c0d3.blue: various bug + style fixes, commit message]
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
This feature is used and enabled by default in OpenWRT and LEDE for AP
interfaces for more than a year now to allow both a more robust multicast
delivery and multicast at higher rates (e.g. multicast streaming).
In OpenWRT/LEDE the IGMP/MLD report suppression issue is overcome by
the network daemon enabling AP isolation and by that separating all STAs.
Delivery of STA-to-STA IP mulitcast is made possible again by
enabling and utilizing the bridge hairpin mode, which considers the
incoming port as a potential outgoing port, too.
Hairpin-mode is performed after multicast snooping, therefore leading to
only deliver reports to STAs running a multicast router.
Changes in v3:
* fix an uninitialized variable bug introduced in br_multicast_flood()
in v2, found by kbuild test bot
Changes in v2:
* netlink support (thanks Nik!)
* fixed switching between multicast_to_unicast on/off
-> even after toggling an already existing entry would
stale in its mode and would never be replaced
-> new extra check in br_port_group_equal)
* reduced checks in br_multicast_flood() from two to one
to address fast-path concerns (thanks Nik!)
* rev-christmas tree ordering (thanks Nik!)
* removed "net_bridge_port_group::unicast", using
::flags instead (thanks Nik!)
* BR_MULTICAST_TO_UCAST -> BR_MULTICAST_TO_UNICAST
(BR_MULTICAST_FLAST_LEAVE has the same length anyway)
* simplified maybe_deliver_addr()
(no return, no "prev" paramater -> was a NOP anyway)
* added "From: Felix Fietkau [...]"
* added "Signed-off-by: Felix Fietkau [...]"
---
include/linux/if_bridge.h | 1 +
include/uapi/linux/if_link.h | 1 +
net/bridge/br_forward.c | 37 ++++++++++++++++-
net/bridge/br_mdb.c | 2 +-
net/bridge/br_multicast.c | 96 ++++++++++++++++++++++++++++++++------------
net/bridge/br_netlink.c | 5 +++
net/bridge/br_private.h | 8 ++--
net/bridge/br_sysfs_if.c | 2 +
8 files changed, 121 insertions(+), 31 deletions(-)
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index c6587c0..debc9d5 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -46,6 +46,7 @@ struct br_ip_list {
#define BR_LEARNING_SYNC BIT(9)
#define BR_PROXYARP_WIFI BIT(10)
#define BR_MCAST_FLOOD BIT(11)
+#define BR_MULTICAST_TO_UNICAST BIT(12)
#define BR_DEFAULT_AGEING_TIME (300 * HZ)
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 6b13e59..4e59565 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -321,6 +321,7 @@ enum {
IFLA_BRPORT_MULTICAST_ROUTER,
IFLA_BRPORT_PAD,
IFLA_BRPORT_MCAST_FLOOD,
+ IFLA_BRPORT_MCAST_TO_UCAST,
__IFLA_BRPORT_MAX
};
#define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 7cb41ae..a6c8a27 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -174,6 +174,29 @@ static struct net_bridge_port *maybe_deliver(
return p;
}
+static void maybe_deliver_addr(struct net_bridge_port *p, struct sk_buff *skb,
+ const unsigned char *addr, bool local_orig)
+{
+ struct net_device *dev = BR_INPUT_SKB_CB(skb)->brdev;
+ const unsigned char *src = eth_hdr(skb)->h_source;
+
+ if (!should_deliver(p, skb))
+ return;
+
+ /* Even with hairpin, no soliloquies - prevent breaking IPv6 DAD */
+ if (skb->dev == p->dev && ether_addr_equal(src, addr))
+ return;
+
+ skb = skb_copy(skb, GFP_ATOMIC);
+ if (!skb) {
+ dev->stats.tx_dropped++;
+ return;
+ }
+
+ memcpy(eth_hdr(skb)->h_dest, addr, ETH_ALEN);
+ __br_forward(p, skb, local_orig);
+}
+
/* called under rcu_read_lock */
void br_flood(struct net_bridge *br, struct sk_buff *skb,
enum br_pkt_type pkt_type, bool local_rcv, bool local_orig)
@@ -241,10 +264,20 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
rport = rp ? hlist_entry(rp, struct net_bridge_port, rlist) :
NULL;
- port = (unsigned long)lport > (unsigned long)rport ?
- lport : rport;
+ if ((unsigned long)lport > (unsigned long)rport) {
+ port = lport;
+
+ if (p->flags & MDB_PG_FLAGS_MCAST_TO_UCAST) {
+ maybe_deliver_addr(lport, skb, p->eth_addr,
+ local_orig);
+ goto delivered;
+ }
+ } else {
+ port = rport;
+ }
prev = maybe_deliver(prev, port, skb, local_orig);
+delivered:
if (IS_ERR(prev))
goto out;
if (prev == port)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 7dbc80d..056e6ac 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -531,7 +531,7 @@ static int br_mdb_add_group(struct net_bridge *br, struct net_bridge_port *port,
break;
}
- p = br_multicast_new_port_group(port, group, *pp, state);
+ p = br_multicast_new_port_group(port, group, *pp, state, NULL);
if (unlikely(!p))
return -ENOMEM;
rcu_assign_pointer(*pp, p);
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index b30e77e..b127bb7 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -43,12 +43,14 @@ static void br_multicast_add_router(struct net_bridge *br,
static void br_ip4_multicast_leave_group(struct net_bridge *br,
struct net_bridge_port *port,
__be32 group,
- __u16 vid);
+ __u16 vid,
+ const unsigned char *src);
+
#if IS_ENABLED(CONFIG_IPV6)
static void br_ip6_multicast_leave_group(struct net_bridge *br,
struct net_bridge_port *port,
const struct in6_addr *group,
- __u16 vid);
+ __u16 vid, const unsigned char *src);
#endif
unsigned int br_mdb_rehash_seq;
@@ -711,7 +713,8 @@ struct net_bridge_port_group *br_multicast_new_port_group(
struct net_bridge_port *port,
struct br_ip *group,
struct net_bridge_port_group __rcu *next,
- unsigned char flags)
+ unsigned char flags,
+ const unsigned char *src)
{
struct net_bridge_port_group *p;
@@ -726,12 +729,39 @@ struct net_bridge_port_group *br_multicast_new_port_group(
hlist_add_head(&p->mglist, &port->mglist);
setup_timer(&p->timer, br_multicast_port_group_expired,
(unsigned long)p);
+
+ if ((port->flags & BR_MULTICAST_TO_UNICAST) && src) {
+ memcpy(p->eth_addr, src, ETH_ALEN);
+ p->flags |= MDB_PG_FLAGS_MCAST_TO_UCAST;
+ }
+
return p;
}
+static bool br_port_group_equal(struct net_bridge_port_group *p,
+ struct net_bridge_port *port,
+ const unsigned char *src)
+{
+ if (p->port != port)
+ return false;
+
+ if (!(p->flags & MDB_PG_FLAGS_MCAST_TO_UCAST) !=
+ !(port->flags & BR_MULTICAST_TO_UNICAST))
+ return false;
+
+ if (!(p->flags & MDB_PG_FLAGS_MCAST_TO_UCAST))
+ return true;
+
+ if (!src)
+ return false;
+
+ return ether_addr_equal(src, p->eth_addr);
+}
+
static int br_multicast_add_group(struct net_bridge *br,
struct net_bridge_port *port,
- struct br_ip *group)
+ struct br_ip *group,
+ const unsigned char *src)
{
struct net_bridge_port_group __rcu **pp;
struct net_bridge_port_group *p;
@@ -758,13 +788,13 @@ static int br_multicast_add_group(struct net_bridge *br,
for (pp = &mp->ports;
(p = mlock_dereference(*pp, br)) != NULL;
pp = &p->next) {
- if (p->port == port)
+ if (br_port_group_equal(p, port, src))
goto found;
if ((unsigned long)p->port < (unsigned long)port)
break;
}
- p = br_multicast_new_port_group(port, group, *pp, 0);
+ p = br_multicast_new_port_group(port, group, *pp, 0, src);
if (unlikely(!p))
goto err;
rcu_assign_pointer(*pp, p);
@@ -783,7 +813,8 @@ static int br_multicast_add_group(struct net_bridge *br,
static int br_ip4_multicast_add_group(struct net_bridge *br,
struct net_bridge_port *port,
__be32 group,
- __u16 vid)
+ __u16 vid,
+ const unsigned char *src)
{
struct br_ip br_group;
@@ -794,14 +825,15 @@ static int br_ip4_multicast_add_group(struct net_bridge *br,
br_group.proto = htons(ETH_P_IP);
br_group.vid = vid;
- return br_multicast_add_group(br, port, &br_group);
+ return br_multicast_add_group(br, port, &br_group, src);
}
#if IS_ENABLED(CONFIG_IPV6)
static int br_ip6_multicast_add_group(struct net_bridge *br,
struct net_bridge_port *port,
const struct in6_addr *group,
- __u16 vid)
+ __u16 vid,
+ const unsigned char *src)
{
struct br_ip br_group;
@@ -812,7 +844,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
br_group.proto = htons(ETH_P_IPV6);
br_group.vid = vid;
- return br_multicast_add_group(br, port, &br_group);
+ return br_multicast_add_group(br, port, &br_group, src);
}
#endif
@@ -1081,6 +1113,7 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
struct sk_buff *skb,
u16 vid)
{
+ const unsigned char *src;
struct igmpv3_report *ih;
struct igmpv3_grec *grec;
int i;
@@ -1121,12 +1154,14 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
continue;
}
+ src = eth_hdr(skb)->h_source;
if ((type == IGMPV3_CHANGE_TO_INCLUDE ||
type == IGMPV3_MODE_IS_INCLUDE) &&
ntohs(grec->grec_nsrcs) == 0) {
- br_ip4_multicast_leave_group(br, port, group, vid);
+ br_ip4_multicast_leave_group(br, port, group, vid, src);
} else {
- err = br_ip4_multicast_add_group(br, port, group, vid);
+ err = br_ip4_multicast_add_group(br, port, group, vid,
+ src);
if (err)
break;
}
@@ -1141,6 +1176,7 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
struct sk_buff *skb,
u16 vid)
{
+ const unsigned char *src = eth_hdr(skb)->h_source;
struct icmp6hdr *icmp6h;
struct mld2_grec *grec;
int i;
@@ -1192,10 +1228,11 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
ntohs(*nsrcs) == 0) {
br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
- vid);
+ vid, src);
} else {
err = br_ip6_multicast_add_group(br, port,
- &grec->grec_mca, vid);
+ &grec->grec_mca, vid,
+ src);
if (err)
break;
}
@@ -1511,7 +1548,8 @@ br_multicast_leave_group(struct net_bridge *br,
struct net_bridge_port *port,
struct br_ip *group,
struct bridge_mcast_other_query *other_query,
- struct bridge_mcast_own_query *own_query)
+ struct bridge_mcast_own_query *own_query,
+ const unsigned char *src)
{
struct net_bridge_mdb_htable *mdb;
struct net_bridge_mdb_entry *mp;
@@ -1535,7 +1573,7 @@ br_multicast_leave_group(struct net_bridge *br,
for (pp = &mp->ports;
(p = mlock_dereference(*pp, br)) != NULL;
pp = &p->next) {
- if (p->port != port)
+ if (!br_port_group_equal(p, port, src))
continue;
rcu_assign_pointer(*pp, p->next);
@@ -1566,7 +1604,7 @@ br_multicast_leave_group(struct net_bridge *br,
for (p = mlock_dereference(mp->ports, br);
p != NULL;
p = mlock_dereference(p->next, br)) {
- if (p->port != port)
+ if (!br_port_group_equal(p, port, src))
continue;
if (!hlist_unhashed(&p->mglist) &&
@@ -1617,7 +1655,8 @@ br_multicast_leave_group(struct net_bridge *br,
static void br_ip4_multicast_leave_group(struct net_bridge *br,
struct net_bridge_port *port,
__be32 group,
- __u16 vid)
+ __u16 vid,
+ const unsigned char *src)
{
struct br_ip br_group;
struct bridge_mcast_own_query *own_query;
@@ -1632,14 +1671,15 @@ static void br_ip4_multicast_leave_group(struct net_bridge *br,
br_group.vid = vid;
br_multicast_leave_group(br, port, &br_group, &br->ip4_other_query,
- own_query);
+ own_query, src);
}
#if IS_ENABLED(CONFIG_IPV6)
static void br_ip6_multicast_leave_group(struct net_bridge *br,
struct net_bridge_port *port,
const struct in6_addr *group,
- __u16 vid)
+ __u16 vid,
+ const unsigned char *src)
{
struct br_ip br_group;
struct bridge_mcast_own_query *own_query;
@@ -1654,7 +1694,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
br_group.vid = vid;
br_multicast_leave_group(br, port, &br_group, &br->ip6_other_query,
- own_query);
+ own_query, src);
}
#endif
@@ -1712,6 +1752,7 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
u16 vid)
{
struct sk_buff *skb_trimmed = NULL;
+ const unsigned char *src;
struct igmphdr *ih;
int err;
@@ -1731,13 +1772,14 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
}
ih = igmp_hdr(skb);
+ src = eth_hdr(skb)->h_source;
BR_INPUT_SKB_CB(skb)->igmp = ih->type;
switch (ih->type) {
case IGMP_HOST_MEMBERSHIP_REPORT:
case IGMPV2_HOST_MEMBERSHIP_REPORT:
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
- err = br_ip4_multicast_add_group(br, port, ih->group, vid);
+ err = br_ip4_multicast_add_group(br, port, ih->group, vid, src);
break;
case IGMPV3_HOST_MEMBERSHIP_REPORT:
err = br_ip4_multicast_igmp3_report(br, port, skb_trimmed, vid);
@@ -1746,7 +1788,7 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
err = br_ip4_multicast_query(br, port, skb_trimmed, vid);
break;
case IGMP_HOST_LEAVE_MESSAGE:
- br_ip4_multicast_leave_group(br, port, ih->group, vid);
+ br_ip4_multicast_leave_group(br, port, ih->group, vid, src);
break;
}
@@ -1766,6 +1808,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
u16 vid)
{
struct sk_buff *skb_trimmed = NULL;
+ const unsigned char *src;
struct mld_msg *mld;
int err;
@@ -1785,8 +1828,10 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
switch (mld->mld_type) {
case ICMPV6_MGM_REPORT:
+ src = eth_hdr(skb)->h_source;
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
- err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid);
+ err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid,
+ src);
break;
case ICMPV6_MLD2_REPORT:
err = br_ip6_multicast_mld2_report(br, port, skb_trimmed, vid);
@@ -1795,7 +1840,8 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
err = br_ip6_multicast_query(br, port, skb_trimmed, vid);
break;
case ICMPV6_MGM_REDUCTION:
- br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid);
+ src = eth_hdr(skb)->h_source;
+ br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid, src);
break;
}
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 71c7453..6c087cd 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -123,6 +123,7 @@ static inline size_t br_port_info_size(void)
+ nla_total_size(1) /* IFLA_BRPORT_GUARD */
+ nla_total_size(1) /* IFLA_BRPORT_PROTECT */
+ nla_total_size(1) /* IFLA_BRPORT_FAST_LEAVE */
+ + nla_total_size(1) /* IFLA_BRPORT_MCAST_TO_UCAST */
+ nla_total_size(1) /* IFLA_BRPORT_LEARNING */
+ nla_total_size(1) /* IFLA_BRPORT_UNICAST_FLOOD */
+ nla_total_size(1) /* IFLA_BRPORT_PROXYARP */
@@ -173,6 +174,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
!!(p->flags & BR_ROOT_BLOCK)) ||
nla_put_u8(skb, IFLA_BRPORT_FAST_LEAVE,
!!(p->flags & BR_MULTICAST_FAST_LEAVE)) ||
+ nla_put_u8(skb, IFLA_BRPORT_MCAST_TO_UCAST,
+ !!(p->flags & BR_MULTICAST_TO_UNICAST)) ||
nla_put_u8(skb, IFLA_BRPORT_LEARNING, !!(p->flags & BR_LEARNING)) ||
nla_put_u8(skb, IFLA_BRPORT_UNICAST_FLOOD,
!!(p->flags & BR_FLOOD)) ||
@@ -586,6 +589,7 @@ static const struct nla_policy br_port_policy[IFLA_BRPORT_MAX + 1] = {
[IFLA_BRPORT_PROXYARP] = { .type = NLA_U8 },
[IFLA_BRPORT_PROXYARP_WIFI] = { .type = NLA_U8 },
[IFLA_BRPORT_MULTICAST_ROUTER] = { .type = NLA_U8 },
+ [IFLA_BRPORT_MCAST_TO_UCAST] = { .type = NLA_U8 },
};
/* Change the state of the port and notify spanning tree */
@@ -636,6 +640,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
br_set_port_flag(p, tb, IFLA_BRPORT_LEARNING, BR_LEARNING);
br_set_port_flag(p, tb, IFLA_BRPORT_UNICAST_FLOOD, BR_FLOOD);
br_set_port_flag(p, tb, IFLA_BRPORT_MCAST_FLOOD, BR_MCAST_FLOOD);
+ br_set_port_flag(p, tb, IFLA_BRPORT_MCAST_TO_UCAST, BR_MULTICAST_TO_UNICAST);
br_set_port_flag(p, tb, IFLA_BRPORT_PROXYARP, BR_PROXYARP);
br_set_port_flag(p, tb, IFLA_BRPORT_PROXYARP_WIFI, BR_PROXYARP_WIFI);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 8ce621e..f7db79f 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -166,8 +166,9 @@ struct net_bridge_fdb_entry
struct rcu_head rcu;
};
-#define MDB_PG_FLAGS_PERMANENT BIT(0)
-#define MDB_PG_FLAGS_OFFLOAD BIT(1)
+#define MDB_PG_FLAGS_PERMANENT BIT(0)
+#define MDB_PG_FLAGS_OFFLOAD BIT(1)
+#define MDB_PG_FLAGS_MCAST_TO_UCAST BIT(2)
struct net_bridge_port_group {
struct net_bridge_port *port;
@@ -177,6 +178,7 @@ struct net_bridge_port_group {
struct timer_list timer;
struct br_ip addr;
unsigned char flags;
+ unsigned char eth_addr[ETH_ALEN];
};
struct net_bridge_mdb_entry
@@ -599,7 +601,7 @@ void br_multicast_free_pg(struct rcu_head *head);
struct net_bridge_port_group *
br_multicast_new_port_group(struct net_bridge_port *port, struct br_ip *group,
struct net_bridge_port_group __rcu *next,
- unsigned char flags);
+ unsigned char flags, const unsigned char *src);
void br_mdb_init(void);
void br_mdb_uninit(void);
void br_mdb_notify(struct net_device *dev, struct net_bridge_port *port,
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 8bd5696..05e8946 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -188,6 +188,7 @@ static BRPORT_ATTR(multicast_router, S_IRUGO | S_IWUSR, show_multicast_router,
store_multicast_router);
BRPORT_ATTR_FLAG(multicast_fast_leave, BR_MULTICAST_FAST_LEAVE);
+BRPORT_ATTR_FLAG(multicast_to_unicast, BR_MULTICAST_TO_UNICAST);
#endif
static const struct brport_attribute *brport_attrs[] = {
@@ -214,6 +215,7 @@ static const struct brport_attribute *brport_attrs[] = {
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
&brport_attr_multicast_router,
&brport_attr_multicast_fast_leave,
+ &brport_attr_multicast_to_unicast,
#endif
&brport_attr_proxyarp,
&brport_attr_proxyarp_wifi,
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v3] brcmfmac: fix incorrect event channel deduction
From: Arend Van Spriel @ 2017-01-18 22:35 UTC (permalink / raw)
To: Gavin Li
Cc: Franky Lin, Hante Meuleman, linux-wireless,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER, Stable,
Gavin Li, Rafał Miłecki
In-Reply-To: <CA+GxvY43AbKOVJFWAxHg1Gku=LD-dveHY8m6=zPPu9qR4+r=Fg@mail.gmail.com>
On 18-1-2017 19:39, Gavin Li wrote:
> I think calling this a performance regression is a bit understated; my
> download speed jumped from 1Mbit/s back up to 40MBit/s after applying
> the patch due to the sheer amount of packets being incorrectly
> processed.
I was merely using your own words although admittedly I left out the
word "major".
> In addition, processing arbitrary data frames as firmware events might
> be a security vulnerability.
True. So I am not saying there is no need to backport it to stable
kernel. I meant there are no reported crashes related to this bug so
there is no need to push it through the current release cycle.
Regards,
Arend
> On Wed, Jan 18, 2017 at 4:38 AM, Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 18-1-2017 0:24, gavinli@thegavinli.com wrote:
>>> From: Gavin Li <git@thegavinli.com>
>>>
>>> brcmf_sdio_fromevntchan() was being called on the the data frame
>>> rather than the software header, causing some frames to be
>>> mischaracterized as on the event channel rather than the data channel.
>>>
>>> This fixes a major performance regression (due to dropped packets).
>>>
>>> Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet")
>>
>> Actually would prefer Franky to chime in as well as he made the change
>> and confirm correctness. It was introduced a couple of kernel versions
>> back and only a performance regression so seems ok to let this go in
>> 4.11 for now and backport as needed for stable later.
>>
>> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>>> Signed-off-by: Gavin Li <git@thegavinli.com>
>>> Cc: <stable@vger.kernel.org> [4.7+]
>>> ---
>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>>> index dfb0658713d9..d2219885071f 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>>> @@ -1661,7 +1661,7 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
>>> pfirst->len, pfirst->next,
>>> pfirst->prev);
>>> skb_unlink(pfirst, &bus->glom);
>>> - if (brcmf_sdio_fromevntchan(pfirst->data))
>>> + if (brcmf_sdio_fromevntchan(&dptr[SDPCM_HWHDR_LEN]))
>>> brcmf_rx_event(bus->sdiodev->dev, pfirst);
>>> else
>>> brcmf_rx_frame(bus->sdiodev->dev, pfirst,
>>>
^ permalink raw reply
* Re: [PATCH v3 1/2] devicetree: document new marvell-8xxx and pwrseq-sd8787 options
From: Rob Herring @ 2017-01-18 22:02 UTC (permalink / raw)
To: Matt Ranostay; +Cc: linux-wireless, linux-kernel, linux-mmc, devicetree, tony
In-Reply-To: <20170113052937.12538-2-matt@ranostay.consulting>
On Thu, Jan 12, 2017 at 09:29:36PM -0800, Matt Ranostay wrote:
Changelog?
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> ---
> .../devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt | 14 ++++++++++++++
> .../devicetree/bindings/net/wireless/marvell-8xxx.txt | 7 ++++++-
> 2 files changed, 20 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt
> new file mode 100644
> index 000000000000..1b658351629b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-sd8787.txt
> @@ -0,0 +1,14 @@
> +* Marvell SD8787 power sequence provider
> +
> +Required properties:
> +- compatible: must be "mmc-pwrseq-sd8787".
> +- pwndn-gpio: contains a power down GPIO specifier.
powerdown-gpios
> +- reset-gpio: contains a reset GPIO specifier.
reset-gpios
And state the active state for both.
> +
> +Example:
> +
> + wifi_pwrseq: wifi_pwrseq {
> + compatible = "mmc-pwrseq-sd8787";
> + pwrdn-gpio = <&twl_gpio 0 GPIO_ACTIVE_LOW>;
> + reset-gpio = <&twl_gpio 1 GPIO_ACTIVE_LOW>;
> + }
> diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> index 980b16df74c3..0854451ff91d 100644
> --- a/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> @@ -1,4 +1,4 @@
> -Marvell 8897/8997 (sd8897/sd8997/pcie8997) SDIO/PCIE devices
> +Marvell 8787/8897/8997 (sd8787/sd8897/sd8997/pcie8997) SDIO/PCIE devices
> ------
>
> This node provides properties for controlling the Marvell SDIO/PCIE wireless device.
> @@ -8,6 +8,7 @@ connects the device to the system.
> Required properties:
>
> - compatible : should be one of the following:
> + * "marvell,sd8787"
> * "marvell,sd8897"
> * "marvell,sd8997"
> * "pci11ab,2b42"
> @@ -34,6 +35,9 @@ Optional properties:
> so that the wifi chip can wakeup host platform under certain condition.
> during system resume, the irq will be disabled to make sure
> unnecessary interrupt is not received.
> + - vmmc-supply: a phandle of a regulator, supplying VCC to the card
> + - mmc-pwrseq: phandle to the MMC power sequence node. See "mmc-pwrseq-*"
> + for documentation of MMC power sequence bindings.
>
> Example:
>
> @@ -46,6 +50,7 @@ so that firmware can wakeup host using this device side pin.
> &mmc3 {
> status = "okay";
> vmmc-supply = <&wlan_en_reg>;
> + mmc-pwrseq = <&wifi_pwrseq>;
> bus-width = <4>;
> cap-power-off-card;
> keep-power-in-suspend;
> --
> 2.10.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/2] devicetree: document vmmc-supply and mmc-pwrseq options
From: Rob Herring @ 2017-01-18 21:59 UTC (permalink / raw)
To: Matt Ranostay; +Cc: linux-wireless, linux-kernel, linux-mmc, devicetree, tony
In-Reply-To: <20170113052218.10534-2-matt@ranostay.consulting>
On Thu, Jan 12, 2017 at 09:22:17PM -0800, Matt Ranostay wrote:
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> ---
> Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3] brcmfmac: fix incorrect event channel deduction
From: Gavin Li @ 2017-01-18 18:39 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Franky Lin, Hante Meuleman, linux-wireless,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER, Stable,
Gavin Li, Rafał Miłecki
In-Reply-To: <bab86bb5-7815-e5ea-053d-ed6b18af6677@broadcom.com>
I think calling this a performance regression is a bit understated; my
download speed jumped from 1Mbit/s back up to 40MBit/s after applying
the patch due to the sheer amount of packets being incorrectly
processed.
In addition, processing arbitrary data frames as firmware events might
be a security vulnerability.
On Wed, Jan 18, 2017 at 4:38 AM, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 18-1-2017 0:24, gavinli@thegavinli.com wrote:
>> From: Gavin Li <git@thegavinli.com>
>>
>> brcmf_sdio_fromevntchan() was being called on the the data frame
>> rather than the software header, causing some frames to be
>> mischaracterized as on the event channel rather than the data channel.
>>
>> This fixes a major performance regression (due to dropped packets).
>>
>> Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet")
>
> Actually would prefer Franky to chime in as well as he made the change
> and confirm correctness. It was introduced a couple of kernel versions
> back and only a performance regression so seems ok to let this go in
> 4.11 for now and backport as needed for stable later.
>
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>> Signed-off-by: Gavin Li <git@thegavinli.com>
>> Cc: <stable@vger.kernel.org> [4.7+]
>> ---
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> index dfb0658713d9..d2219885071f 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> @@ -1661,7 +1661,7 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
>> pfirst->len, pfirst->next,
>> pfirst->prev);
>> skb_unlink(pfirst, &bus->glom);
>> - if (brcmf_sdio_fromevntchan(pfirst->data))
>> + if (brcmf_sdio_fromevntchan(&dptr[SDPCM_HWHDR_LEN]))
>> brcmf_rx_event(bus->sdiodev->dev, pfirst);
>> else
>> brcmf_rx_frame(bus->sdiodev->dev, pfirst,
>>
^ permalink raw reply
* Re: [3/3] rtlwifi: Enable debug mask to be set from sysfs
From: Larry Finger @ 2017-01-18 16:38 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, Ping-Ke Shih
In-Reply-To: <20170118144936.91B3E6080C@smtp.codeaurora.org>
On 01/18/2017 08:49 AM, Kalle Valo wrote:
> Larry Finger <Larry.Finger@lwfinger.net> wrote:
>> The previous commit changes the debug system to use a mask to select
>> which components are to be debugged. Now, we add the necessary code
>> to change the mask from sysfs.
>>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> Cc: Ping-Ke Shih <pkshih@realtek.com>
>
> There's already a generic sysfs interface to change module parameters in
> /sys/module/<driver>/parameters, drivers should not have a private interface for
> that.
Kalle,
I was not aware of that part of sysfs. I will respin the patch as well as remove
the old code that set the debug level.
Thanks,
Larry
^ permalink raw reply
* Re: [PATCH v2 08/14] rt2x00: rt2800mmio: add a workaround for spurious TX_FIFO_STATUS interrupts
From: Stanislaw Gruszka @ 2017-01-18 15:13 UTC (permalink / raw)
To: Daniel Golle
Cc: linux-wireless, Johannes Berg, roman, michel.stempin, c.mignanti,
evaxige, Kalle Valo, Felix Fietkau, John Crispin, Gabor Juhos
In-Reply-To: <20170118144446.GA22837@redhat.com>
On Wed, Jan 18, 2017 at 03:44:46PM +0100, Stanislaw Gruszka wrote:
> On Mon, Jan 16, 2017 at 04:05:47AM +0100, Daniel Golle wrote:
> > irqreturn_t rt2800mmio_interrupt(int irq, void *dev_instance)
> > {
> > struct rt2x00_dev *rt2x00dev = dev_instance;
> > u32 reg, mask;
> > + u32 txstatus = 0;
> >
> > - /* Read status and ACK all interrupts */
> > + /* Read status */
> > rt2x00mmio_register_read(rt2x00dev, INT_SOURCE_CSR, ®);
> > +
> > + if (rt2x00_get_field32(reg, INT_SOURCE_CSR_TX_FIFO_STATUS)) {
> > + /* Due to unknown reason the hardware generates a
> > + * TX_FIFO_STATUS interrupt before the TX_STA_FIFO
> > + * register contain valid data. Read the TX status
> > + * here to see if we have to process the actual
> > + * request.
> > + */
> > + rt2x00mmio_register_read(rt2x00dev, TX_STA_FIFO, &txstatus);
> > + if (rt2800mmio_txstatus_is_spurious(rt2x00dev, txstatus)) {
> > + /* Remove the TX_FIFO_STATUS bit so it won't be
> > + * processed in this turn. The hardware will
> > + * generate another IRQ for us.
> > + */
> > + rt2x00_set_field32(®,
> > + INT_SOURCE_CSR_TX_FIFO_STATUS, 0);
> > + }
> > + }
> > +
> > + /* ACK interrupts */
> > rt2x00mmio_register_write(rt2x00dev, INT_SOURCE_CSR, reg);
>
> I think spurious TX_STA_FIFO problem happen because we first ACK
> interrupt and then read in bulk all statuses from TX_STA_FIFO
> register, while hardware generate new interrupt (as previous
> was ACKed), then in new interrupt we have no more statues to
> read.
>
> This is inherently racy situation and first ACK interrupt and
> then read statuses is safer than reverse order which make risk we
> will have pending status and not get interrupt about that.
>
> Hence I think we should not apply this patch.
Actually patch is safe in regard that we first ACK interrupt
and then read all statuses TX_STA_FIFO. We only do not ACK
interrupt if we do not have valid status in TX_STA_FIFO .
However I don't really see point of the patch, we should get
next interrupt when new status will be places in TX_STA_FIFO
regardless we ACK this interrupt or don't and if we don't
ACK we possibly can get series of spurious interrupts.
Stanislaw
^ permalink raw reply
* Re: [PATCH v2 14/14] rt2x00: add support for RT5350 WiSoC
From: Stanislaw Gruszka @ 2017-01-18 14:47 UTC (permalink / raw)
To: Daniel Golle
Cc: linux-wireless, Johannes Berg, roman, michel.stempin, c.mignanti,
evaxige, Kalle Valo, Felix Fietkau, John Crispin, Gabor Juhos
In-Reply-To: <20170117014806.GE1912@makrotopia.org>
On Tue, Jan 17, 2017 at 02:48:07AM +0100, Daniel Golle wrote:
> On Mon, Jan 16, 2017 at 11:17:44AM +0100, Stanislaw Gruszka wrote:
> > On Mon, Jan 16, 2017 at 04:17:58AM +0100, Daniel Golle wrote:
> > > @@ -7131,6 +7236,12 @@ static int rt2800_validate_eeprom(struct rt2x00_dev *rt2x00dev)
> > > rt2x00_set_field16(&word, EEPROM_NIC_CONF0_RF_TYPE, RF2820);
> > > rt2800_eeprom_write(rt2x00dev, EEPROM_NIC_CONF0, word);
> > > rt2x00_eeprom_dbg(rt2x00dev, "Antenna: 0x%04x\n", word);
> > > + } else if (rt2x00_rt(rt2x00dev, RT5350)) {
> > > + rt2x00_set_field16(&word, EEPROM_NIC_CONF0_RXPATH, 1);
> > > + rt2x00_set_field16(&word, EEPROM_NIC_CONF0_TXPATH, 1);
> > > + rt2x00_set_field16(&word, EEPROM_NIC_CONF0_RF_TYPE, RF3320);
>
> Good catch. This line was probably left over when trying to implement
> support for RT5350 based on adding codepaths to RF3320 which ended up
> messy... As EEPROM_NIC_CONF0_RF_TYPE aparently isn't used anywhere else
> in the code apart from setting rf type which is later on overwritten
> for RT5350 anyway. So no need to set it at all. I suggest to simply
> drop that line.
I'm not sure about that, but overwrite EEPROM_NIC_CONF0_RXPATH
and EEPROM_NIC_CONF0_TXPATH should not be needed as well. EEPROM
should contain proper values.
Stanislaw
^ permalink raw reply
* Re: [3/3] rtlwifi: Enable debug mask to be set from sysfs
From: Kalle Valo @ 2017-01-18 14:49 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless, Larry Finger, Ping-Ke Shih
In-Reply-To: <20170101010706.13234-4-Larry.Finger@lwfinger.net>
Larry Finger <Larry.Finger@lwfinger.net> wrote:
> The previous commit changes the debug system to use a mask to select
> which components are to be debugged. Now, we add the necessary code
> to change the mask from sysfs.
>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Ping-Ke Shih <pkshih@realtek.com>
There's already a generic sysfs interface to change module parameters in
/sys/module/<driver>/parameters, drivers should not have a private interface for
that.
--
https://patchwork.kernel.org/patch/9492849/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH v2 08/14] rt2x00: rt2800mmio: add a workaround for spurious TX_FIFO_STATUS interrupts
From: Stanislaw Gruszka @ 2017-01-18 14:44 UTC (permalink / raw)
To: Daniel Golle
Cc: linux-wireless, Johannes Berg, roman, michel.stempin, c.mignanti,
evaxige, Kalle Valo, Felix Fietkau, John Crispin, Gabor Juhos
In-Reply-To: <20170116030535.GA32238@makrotopia.org>
On Mon, Jan 16, 2017 at 04:05:47AM +0100, Daniel Golle wrote:
> irqreturn_t rt2800mmio_interrupt(int irq, void *dev_instance)
> {
> struct rt2x00_dev *rt2x00dev = dev_instance;
> u32 reg, mask;
> + u32 txstatus = 0;
>
> - /* Read status and ACK all interrupts */
> + /* Read status */
> rt2x00mmio_register_read(rt2x00dev, INT_SOURCE_CSR, ®);
> +
> + if (rt2x00_get_field32(reg, INT_SOURCE_CSR_TX_FIFO_STATUS)) {
> + /* Due to unknown reason the hardware generates a
> + * TX_FIFO_STATUS interrupt before the TX_STA_FIFO
> + * register contain valid data. Read the TX status
> + * here to see if we have to process the actual
> + * request.
> + */
> + rt2x00mmio_register_read(rt2x00dev, TX_STA_FIFO, &txstatus);
> + if (rt2800mmio_txstatus_is_spurious(rt2x00dev, txstatus)) {
> + /* Remove the TX_FIFO_STATUS bit so it won't be
> + * processed in this turn. The hardware will
> + * generate another IRQ for us.
> + */
> + rt2x00_set_field32(®,
> + INT_SOURCE_CSR_TX_FIFO_STATUS, 0);
> + }
> + }
> +
> + /* ACK interrupts */
> rt2x00mmio_register_write(rt2x00dev, INT_SOURCE_CSR, reg);
I think spurious TX_STA_FIFO problem happen because we first ACK
interrupt and then read in bulk all statuses from TX_STA_FIFO
register, while hardware generate new interrupt (as previous
was ACKed), then in new interrupt we have no more statues to
read.
This is inherently racy situation and first ACK interrupt and
then read statuses is safer than reverse order which make risk we
will have pending status and not get interrupt about that.
Hence I think we should not apply this patch.
Stanislaw
^ permalink raw reply
* Re: [PATCH v2 13/14] rt2x00: rt2800lib: add support for RT3352 with 20MHz crystal
From: Stanislaw Gruszka @ 2017-01-18 14:30 UTC (permalink / raw)
To: Daniel Golle
Cc: linux-wireless, Johannes Berg, roman, michel.stempin, c.mignanti,
evaxige, Kalle Valo, Felix Fietkau, John Crispin, Gabor Juhos
In-Reply-To: <20170116031541.GA32313@makrotopia.org>
On Mon, Jan 16, 2017 at 04:15:56AM +0100, Daniel Golle wrote:
> Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
> Signed-off-by: Mathias Kresin <dev@kresin.me>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 50 +++++++++++++++++++++++++-
> drivers/net/wireless/ralink/rt2x00/rt2x00.h | 2 ++
> 2 files changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> index 93c97eade334..cb1457595f05 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
> @@ -36,6 +36,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/slab.h>
> +#include <linux/clk.h>
>
> #include "rt2x00.h"
> #include "rt2800lib.h"
> @@ -7675,6 +7676,27 @@ static const struct rf_channel rf_vals_5592_xtal40[] = {
> {196, 83, 0, 12, 1},
> };
>
> +/*
> + * RF value list for rt3xxx with Xtal20MHz
> + * Supports: 2.4 GHz (all) (RF3322)
> + */
> +static const struct rf_channel rf_vals_xtal20mhz_3x[] = {
Please locate this values in alphabetical order (i.e. after _3x and
before _5592 ).
> struct hw_mode_spec *spec = &rt2x00dev->spec;
> @@ -7764,7 +7786,10 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
> case RF5390:
> case RF5392:
> spec->num_channels = 14;
> - spec->channels = rf_vals_3x;
> + if (spec->clk_is_20mhz)
> + spec->channels = rf_vals_xtal20mhz_3x;
> + else
> + spec->channels = rf_vals_3x;
> break;
How does vendor drivers recognize xtal (I assume rf_vals_xtal20mhz_3x
values were taken from vendor driver) ? It should be possible to get
clock frequency from device register like is is done on RF5592, without
adding additional clock recognition code. But if such code is needed
I prefer that low level board/platform routines do it and place clock
frequency for rt2x00 in rt2x00dev->dev->platform_data.
Stanislaw
^ permalink raw reply
* Re: [1/1] net: wireless: marvell: fix improper return value
From: Kalle Valo @ 2017-01-18 14:43 UTC (permalink / raw)
To: Pan Bian
Cc: Andreas Kemnade, Johannes Berg, libertas-dev, linux-wireless,
netdev, linux-kernel, Pan Bian
In-Reply-To: <1480760857-4549-1-git-send-email-bianpan2016@163.com>
Pan Bian <bianpan2016@163.com> wrote:
> Function lbs_cmd_802_11_sleep_params() always return 0, even if the call
> to lbs_cmd_with_response() fails. In this case, the parameter @sp will
> keep uninitialized. Because the return value is 0, its caller (say
> lbs_sleepparams_read()) will not detect the error, and will copy the
> uninitialized stack memory to user sapce, resulting in stack information
> leak. To avoid the bug, this patch returns variable ret (which takes
> the return value of lbs_cmd_with_response()) instead of 0.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=188451
>
> Signed-off-by: Pan Bian <bianpan2016@163.com>
The prefix should be "libertas:", I'll fix that.
--
https://patchwork.kernel.org/patch/9459597/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [1/5] ath10k: Remove unused 'buf_len' variable
From: Kalle Valo @ 2017-01-18 14:37 UTC (permalink / raw)
To: Kirtika Ruchandani
Cc: Kalle Valo, Arnd Bergmann, netdev, linux-wireless, Raja Mani,
Michal Kazior
In-Reply-To: <196e46eee00fd3ebb56da1373c36f3dff66f2ae1.1479974100.git.kirtika@chromium.org>
Kirtika Ruchandani <kirtika.ruchandani@gmail.com> wrote:
> Commit 32653cf19554 removed the call to 'skb_trim(skb, buf_len)'
> in ath10k_wmi_event_mgmt_rx(), leaving the buf_len variable set but
> unused. Compiling with W=1 gives the following warning, fix it.
> drivers/net/wireless/ath/ath10k/wmi.c: In function ‘ath10k_wmi_event_mgmt_rx’:
> drivers/net/wireless/ath/ath10k/wmi.c:2280:6: warning: variable ‘buf_len’ set but not used [-Wunused-but-set-variable]
>
> This is a harmless warning, and is only being fixed to reduce the
> noise with W=1 in the kernel.
>
> Fixes: 32653cf19554 ("ath10k: implement intermediate event args")
> Cc: Michal Kazior <michal.kazior@tieto.com>
> Cc: Kalle Valo <kvalo@qca.qualcomm.com>
> Signed-off-by: Kirtika Ruchandani <kirtika@chromium.org>
These patches seem to be corrupted:
Applying: ath10k: Remove unused 'buf_len' variable
fatal: corrupt patch at line 13
error: could not build fake ancestor
Patch failed at 0001 ath10k: Remove unused 'buf_len' variable
When resending please also cc ath10k list:
https://wireless.wiki.kernel.org/en/users/drivers/ath10k/sources#submitting_patches
4 patches set to Changes Requested.
9444973 [1/5] ath10k: Remove unused 'buf_len' variable
9444935 [2/5] ath10k: Remove unused 'num_vdev_stats' variable
9444971 [4/5] ath10k: Removed unused 'dev' in ath10k_ahb_clock_enable()
9444975 [5/5] ath10k: Removed unused 'dev' in ath10k_ahb_resource_init
--
https://patchwork.kernel.org/patch/9444973/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [1/3] rsi: Renamed the file rsi_91x_pkt.c to rsi_91x_hal.c
From: Kalle Valo @ 2017-01-18 14:28 UTC (permalink / raw)
To: Prameela Rani Garnepudi
Cc: linux-wireless, johannes.berg, hofrat, xypron.glpk,
prameela.garnepudi, Prameela Rani Garnepudi
In-Reply-To: <1481173676-18808-1-git-send-email-prameela.j04cs@gmail.com>
Prameela Rani Garnepudi <prameela.j04cs@gmail.com> wrote:
> The file 'rsi_91x_hal.c' is going to contain device specific
> code like firmware loading etc. As the file 'rsi_91x_pkt.c'
> contains code for preparing device (frimware understandable)
> specific descriptors for the transmit frames, it is moved to
> 'rsi_91x_hal.c'
>
> Signed-off-by: Prameela Rani Garnepudi <prameela.j04cs@gmail.com>
These patches break endianess support again, at least in struct bl_header. And
also I see this AGAIN:
+/**
+ * bl_cmd() - This function initiates the BL command
+ *
+ * @adapter: Pointer to the hardware structure.
+ * @cmd: Command to write
+ * @exp_resp: Expected Response
+ * @str: Command string
+ *
+ * Return: 0 on success, -1 on failure.
+ */
+static int bl_cmd(struct rsi_hw *adapter, u8 cmd, u8 exp_resp, char *str)
+{
[...]
+fail:
+ return -EINVAL;
+}
If you insist keeping these, in my opinion useless, function documentation at
least make sure they are valid. How many times have I mentioned about this? If
you don't bother to follow this minor, but easy to fix, detail I'm starting to
wonder why I bother to use my time to review these patches and provide these
same comments over and over again.
Really, start with small patches and once you get more familiar start making
more complex patches. Or ask someone to help with reviewing the patches. And
LISTEN to feedback, just don't ignore it.
3 patches set to Changes Requested.
9465833 [1/3] rsi: Renamed the file rsi_91x_pkt.c to rsi_91x_hal.c
9465835 [2/3] rsi: Add new firmware loading method for RS9113 chip set
9465837 [3/3] rsi: Remove old firmware loading method for RS9113 chip set
--
https://patchwork.kernel.org/patch/9465833/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [PATCH RFC 2/2] brcmfmac: add brcmf_err_pub macro for better error messages
From: Rafał Miłecki @ 2017-01-18 14:27 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, Rafał Miłecki
In-Reply-To: <20170118142703.9824-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
This macro accepts an extra argument of struct brcmf_pub pointer. It
allows referencing struct device and printing error using dev_err. This
is very useful on devices with more than one wireless card suppored by
brcmfmac. Thanks for dev_err it's possible to identify device that error
is related to.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
I could convert few more brcmf_err calls to this new brcmf_err_pub but I don't
want to spent too much time on this in case someone has a better idea.
If you do, comment! :)
Example
Before:
[ 14.735640] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Sep 18 2015 03:30:01 version 7.35.177.56 (r587209) FWID 01-6cb8e269
[ 15.155732] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Sep 18 2015 03:30:01 version 7.35.177.56 (r587209) FWID 01-6cb8e269
[ 15.535682] brcmfmac: brcmf_c_preinit_dcmds: Firmware version = wl0: Sep 18 2015 03:30:01 version 7.35.177.56 (r587209) FWID 01-6cb8e269
After:
[ 14.746754] brcmfmac 0000:01:00.0: brcmf_c_preinit_dcmds: Firmware version = wl0: Sep 18 2015 03:30:01 version 7.35.177.56 (r587209) FWID 01-6cb8e269
[ 15.166854] brcmfmac 0001:03:00.0: brcmf_c_preinit_dcmds: Firmware version = wl0: Sep 18 2015 03:30:01 version 7.35.177.56 (r587209) FWID 01-6cb8e269
[ 15.546807] brcmfmac 0001:04:00.0: brcmf_c_preinit_dcmds: Firmware version = wl0: Sep 18 2015 03:30:01 version 7.35.177.56 (r587209) FWID 01-6cb8e269
---
.../wireless/broadcom/brcm80211/brcmfmac/common.c | 19 ++++++-------
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 31 +++++++++++-----------
.../wireless/broadcom/brcm80211/brcmfmac/debug.h | 17 ++++++++----
.../broadcom/brcm80211/brcmfmac/tracepoint.c | 4 +--
4 files changed, 40 insertions(+), 31 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 5fb4a14..7a05de78 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -108,6 +108,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
{
s8 eventmask[BRCMF_EVENTING_MASK_LEN];
u8 buf[BRCMF_DCMD_SMLEN];
+ struct brcmf_pub *pub = ifp->drvr;
struct brcmf_rev_info_le revinfo;
struct brcmf_rev_info *ri;
char *ptr;
@@ -117,7 +118,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
err = brcmf_fil_iovar_data_get(ifp, "cur_etheraddr", ifp->mac_addr,
sizeof(ifp->mac_addr));
if (err < 0) {
- brcmf_err("Retreiving cur_etheraddr failed, %d\n", err);
+ brcmf_err_pub(pub, "Retreiving cur_etheraddr failed, %d\n", err);
goto done;
}
memcpy(ifp->drvr->mac, ifp->mac_addr, sizeof(ifp->drvr->mac));
@@ -126,7 +127,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
&revinfo, sizeof(revinfo));
ri = &ifp->drvr->revinfo;
if (err < 0) {
- brcmf_err("retrieving revision info failed, %d\n", err);
+ brcmf_err_pub(pub, "retrieving revision info failed, %d\n", err);
} else {
ri->vendorid = le32_to_cpu(revinfo.vendorid);
ri->deviceid = le32_to_cpu(revinfo.deviceid);
@@ -153,7 +154,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
strcpy(buf, "ver");
err = brcmf_fil_iovar_data_get(ifp, "ver", buf, sizeof(buf));
if (err < 0) {
- brcmf_err("Retreiving version information failed, %d\n",
+ brcmf_err_pub(pub, "Retreiving version information failed, %d\n",
err);
goto done;
}
@@ -161,7 +162,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
strsep(&ptr, "\n");
/* Print fw version info */
- brcmf_err("Firmware version = %s\n", buf);
+ brcmf_err_pub(pub, "Firmware version = %s\n", buf);
/* locate firmware version number for ethtool */
ptr = strrchr(buf, ' ') + 1;
@@ -170,7 +171,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
/* set mpc */
err = brcmf_fil_iovar_int_set(ifp, "mpc", 1);
if (err) {
- brcmf_err("failed setting mpc\n");
+ brcmf_err_pub(pub, "failed setting mpc\n");
goto done;
}
@@ -180,14 +181,14 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
err = brcmf_fil_iovar_data_get(ifp, "event_msgs", eventmask,
BRCMF_EVENTING_MASK_LEN);
if (err) {
- brcmf_err("Get event_msgs error (%d)\n", err);
+ brcmf_err_pub(pub, "Get event_msgs error (%d)\n", err);
goto done;
}
setbit(eventmask, BRCMF_E_IF);
err = brcmf_fil_iovar_data_set(ifp, "event_msgs", eventmask,
BRCMF_EVENTING_MASK_LEN);
if (err) {
- brcmf_err("Set event_msgs error (%d)\n", err);
+ brcmf_err_pub(pub, "Set event_msgs error (%d)\n", err);
goto done;
}
@@ -195,7 +196,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_SCAN_CHANNEL_TIME,
BRCMF_DEFAULT_SCAN_CHANNEL_TIME);
if (err) {
- brcmf_err("BRCMF_C_SET_SCAN_CHANNEL_TIME error (%d)\n",
+ brcmf_err_pub(pub, "BRCMF_C_SET_SCAN_CHANNEL_TIME error (%d)\n",
err);
goto done;
}
@@ -204,7 +205,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_SCAN_UNASSOC_TIME,
BRCMF_DEFAULT_SCAN_UNASSOC_TIME);
if (err) {
- brcmf_err("BRCMF_C_SET_SCAN_UNASSOC_TIME error (%d)\n",
+ brcmf_err_pub(pub, "BRCMF_C_SET_SCAN_UNASSOC_TIME error (%d)\n",
err);
goto done;
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index b73a55b..a42007b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -59,7 +59,7 @@ struct brcmf_if *brcmf_get_ifp(struct brcmf_pub *drvr, int ifidx)
s32 bsscfgidx;
if (ifidx < 0 || ifidx >= BRCMF_MAX_IFS) {
- brcmf_err("ifidx %d out of range\n", ifidx);
+ brcmf_err_pub(drvr, "ifidx %d out of range\n", ifidx);
return NULL;
}
@@ -204,7 +204,8 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
/* Can the device send data? */
if (drvr->bus_if->state != BRCMF_BUS_UP) {
- brcmf_err("xmit rejected state=%d\n", drvr->bus_if->state);
+ brcmf_err_pub(drvr, "xmit rejected state=%d\n",
+ drvr->bus_if->state);
netif_stop_queue(ndev);
dev_kfree_skb(skb);
ret = -ENODEV;
@@ -222,7 +223,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
dev_kfree_skb(skb);
skb = skb2;
if (skb == NULL) {
- brcmf_err("%s: skb_realloc_headroom failed\n",
+ brcmf_err_pub(drvr, "%s: skb_realloc_headroom failed\n",
brcmf_ifname(ifp));
ret = -ENOMEM;
goto done;
@@ -466,7 +467,7 @@ static int brcmf_netdev_open(struct net_device *ndev)
/* If bus is not ready, can't continue */
if (bus_if->state != BRCMF_BUS_UP) {
- brcmf_err("failed bus is not ready\n");
+ brcmf_err_pub(drvr, "failed bus is not ready\n");
return -EAGAIN;
}
@@ -480,7 +481,7 @@ static int brcmf_netdev_open(struct net_device *ndev)
ndev->features &= ~NETIF_F_IP_CSUM;
if (brcmf_cfg80211_up(ndev)) {
- brcmf_err("failed to bring up cfg80211\n");
+ brcmf_err_pub(drvr, "failed to bring up cfg80211\n");
return -EIO;
}
@@ -525,7 +526,7 @@ int brcmf_net_attach(struct brcmf_if *ifp, bool rtnl_locked)
else
err = register_netdev(ndev);
if (err != 0) {
- brcmf_err("couldn't register the net device\n");
+ brcmf_err_pub(drvr, "couldn't register the net device\n");
goto fail;
}
@@ -643,7 +644,7 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
*/
if (ifp) {
if (ifidx) {
- brcmf_err("ERROR: netdev:%s already exists\n",
+ brcmf_err_pub(drvr, "ERROR: netdev:%s already exists\n",
ifp->ndev->name);
netif_stop_queue(ifp->ndev);
brcmf_net_detach(ifp->ndev, false);
@@ -702,7 +703,7 @@ static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx,
ifp = drvr->iflist[bsscfgidx];
drvr->iflist[bsscfgidx] = NULL;
if (!ifp) {
- brcmf_err("Null interface, bsscfgidx=%d\n", bsscfgidx);
+ brcmf_err_pub(drvr, "Null interface, bsscfgidx=%d\n", bsscfgidx);
return;
}
brcmf_dbg(TRACE, "Enter, bsscfgidx=%d, ifidx=%d\n", bsscfgidx,
@@ -787,7 +788,7 @@ static int brcmf_inetaddr_changed(struct notifier_block *nb,
ret = brcmf_fil_iovar_data_get(ifp, "arp_hostip", addr_table,
sizeof(addr_table));
if (ret) {
- brcmf_err("fail to get arp ip table err:%d\n", ret);
+ brcmf_err_pub(drvr, "fail to get arp ip table err:%d\n", ret);
return NOTIFY_OK;
}
@@ -804,7 +805,7 @@ static int brcmf_inetaddr_changed(struct notifier_block *nb,
ret = brcmf_fil_iovar_data_set(ifp, "arp_hostip",
&ifa->ifa_address, sizeof(ifa->ifa_address));
if (ret)
- brcmf_err("add arp ip err %d\n", ret);
+ brcmf_err_pub(drvr, "add arp ip err %d\n", ret);
}
break;
case NETDEV_DOWN:
@@ -816,7 +817,7 @@ static int brcmf_inetaddr_changed(struct notifier_block *nb,
ret = brcmf_fil_iovar_data_set(ifp, "arp_hostip_clear",
NULL, 0);
if (ret) {
- brcmf_err("fail to clear arp ip table err:%d\n",
+ brcmf_err_pub(drvr, "fail to clear arp ip table err:%d\n",
ret);
return NOTIFY_OK;
}
@@ -827,7 +828,7 @@ static int brcmf_inetaddr_changed(struct notifier_block *nb,
&addr_table[i],
sizeof(addr_table[i]));
if (ret)
- brcmf_err("add arp ip err %d\n",
+ brcmf_err_pub(drvr, "add arp ip err %d\n",
ret);
}
}
@@ -923,7 +924,7 @@ int brcmf_attach(struct device *dev, struct brcmf_mp_device *settings)
/* Attach and link in the protocol */
ret = brcmf_proto_attach(drvr);
if (ret != 0) {
- brcmf_err("brcmf_prot_attach failed\n");
+ brcmf_err_pub(drvr, "brcmf_prot_attach failed\n");
goto fail;
}
@@ -1045,7 +1046,7 @@ int brcmf_bus_started(struct device *dev)
return 0;
fail:
- brcmf_err("failed: %d\n", ret);
+ brcmf_err_pub(drvr, "failed: %d\n", ret);
if (drvr->config) {
brcmf_cfg80211_detach(drvr->config);
drvr->config = NULL;
@@ -1152,7 +1153,7 @@ int brcmf_netdev_wait_pend8021x(struct brcmf_if *ifp)
MAX_WAIT_FOR_8021X_TX);
if (!err)
- brcmf_err("Timed out waiting for no pending 802.1x packets\n");
+ brcmf_err_pub(ifp->drvr, "Timed out waiting for no pending 802.1x packets\n");
return !err;
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
index 1fe4aa9..aab8c71 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
@@ -50,16 +50,23 @@
* debugging is not selected. When debugging the driver error
* messages are as important as other tracing or even more so.
*/
-#define brcmf_err(fmt, ...) \
+#define __brcmf_err(dev, fmt, ...) \
do { \
if (IS_ENABLED(CONFIG_BRCMDBG) || net_ratelimit()) \
- pr_err("%s: " fmt, __func__, ##__VA_ARGS__); \
+ dev_err(dev, "%s: " fmt, __func__, \
+ ##__VA_ARGS__); \
} while (0)
+#define brcmf_err(fmt, ...) \
+ __brcmf_err(NULL, fmt, ##__VA_ARGS__)
+#define brcmf_err_pub(pub, fmt, ...) \
+ __brcmf_err(pub->bus_if->dev, fmt, ##__VA_ARGS__)
#else
-__printf(2, 3)
-void __brcmf_err(const char *func, const char *fmt, ...);
+__printf(3, 4)
+void __brcmf_err(struct brcmf_pub *pub, const char *func, const char *fmt, ...);
#define brcmf_err(fmt, ...) \
- __brcmf_err(__func__, fmt, ##__VA_ARGS__)
+ __brcmf_err(NULL, __func__, fmt, ##__VA_ARGS__)
+#define brcmf_err_pub(fmt, ...) \
+ __brcmf_err(pub, __func__, fmt, ##__VA_ARGS__)
#endif
#if defined(DEBUG) || defined(CONFIG_BRCM_TRACING)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c
index fe67559..3e8d60b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/tracepoint.c
@@ -21,7 +21,7 @@
#include "tracepoint.h"
#include "debug.h"
-void __brcmf_err(const char *func, const char *fmt, ...)
+void __brcmf_err(struct brcmf_pub *pub, const char *func, const char *fmt, ...)
{
struct va_format vaf = {
.fmt = fmt,
@@ -30,7 +30,7 @@ void __brcmf_err(const char *func, const char *fmt, ...)
va_start(args, fmt);
vaf.va = &args;
- pr_err("%s: %pV", func, &vaf);
+ dev_err(pub ? pub->bus_if->dev, NULL, "%s: %pV", func, &vaf);
trace_brcmf_err(func, &vaf);
va_end(args);
}
--
2.10.1
^ permalink raw reply related
* [PATCH RFC 1/2] brcmfmac: merge two brcmf_err macros into one
From: Rafał Miłecki @ 2017-01-18 14:27 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, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
This allows simplifying the code by adding a simple IS_ENABLED check for
CONFIG_BRCMDB symbol.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
index 6687812..1fe4aa9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h
@@ -45,20 +45,16 @@
#undef pr_fmt
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#ifndef CONFIG_BRCM_TRACING
/* Macro for error messages. net_ratelimit() is used when driver
* debugging is not selected. When debugging the driver error
* messages are as important as other tracing or even more so.
*/
-#ifndef CONFIG_BRCM_TRACING
-#ifdef CONFIG_BRCMDBG
-#define brcmf_err(fmt, ...) pr_err("%s: " fmt, __func__, ##__VA_ARGS__)
-#else
#define brcmf_err(fmt, ...) \
do { \
- if (net_ratelimit()) \
+ if (IS_ENABLED(CONFIG_BRCMDBG) || net_ratelimit()) \
pr_err("%s: " fmt, __func__, ##__VA_ARGS__); \
} while (0)
-#endif
#else
__printf(2, 3)
void __brcmf_err(const char *func, const char *fmt, ...);
--
2.10.1
^ permalink raw reply related
* Re: [PATCH 1/3] rsi: Renamed the file rsi_91x_pkt.c to rsi_91x_hal.c
From: Valo, Kalle @ 2017-01-18 13:56 UTC (permalink / raw)
To: Prameela Rani Garnepudi
Cc: Prameela Rani Garnepudi, linux-wireless@vger.kernel.org,
johannes.berg@intel.com, hofrat@osadl.org, xypron.glpk@gmx.de
In-Reply-To: <87eg0brv2f.fsf@kamboji.qca.qualcomm.com>
Kalle Valo <kvalo@codeaurora.org> writes:
> Prameela Rani Garnepudi <prameela.garnepudi@redpinesignals.com> writes:
>
>> Can you please review these patches. You slipped through these patches =
=20
>> and reviewing resent patches. Is there a specific reason?=20
>
> As you are new, your patches just take more time to review than others
> and haven't had a good slot to look at those yet.
Prameela, I noticed that patchwork seems to drop your emails for some
reason. For example, in this patch I do not see your mail above, only my
reply:
https://patchwork.kernel.org/patch/9465833/
That might create problems because I usually check the discussion from
patchwork, not from my own mail archives. Keep this in mind then
commenting.
--=20
Kalle Valo=
^ permalink raw reply
* RE: Re: [PATCH v2 2/3] mwifiex: pcie: don't loop/retry interrupt status checks
From: Amitkumar Karwar @ 2017-01-18 13:13 UTC (permalink / raw)
To: Brian Norris, Dmitry Torokhov
Cc: Nishant Sarmukadam, linux-kernel@vger.kernel.org, Kalle Valo,
linux-wireless@vger.kernel.org, Cathy Luo
In-Reply-To: <20170117194822.GA29749@google.com>
Hi Brian,
> From: Brian Norris [mailto:briannorris@chromium.org]
> Sent: Wednesday, January 18, 2017 1:18 AM
> To: Dmitry Torokhov
> Cc: Amitkumar Karwar; Nishant Sarmukadam; linux-kernel@vger.kernel.org;
> Kalle Valo; linux-wireless@vger.kernel.org; Cathy Luo
> Subject: Re: [PATCH v2 2/3] mwifiex: pcie: don't loop/retry
> interrupt status checks
>
> On Sun, Jan 15, 2017 at 04:54:52PM -0800, Dmitry Torokhov wrote:
> > On Fri, Jan 13, 2017 at 03:35:37PM -0800, Brian Norris wrote:
> > > The following sequence occurs when using IEEE power-save on 8997:
> > > (a) driver sees SLEEP event
> > > (b) driver issues SLEEP CONFIRM
> > > (c) driver recevies CMD interrupt; within the interrupt processing
> loop,
> > > we do (d) and (e):
> > > (d) wait for FW sleep cookie (and often time out; it takes a
> while), FW
> > > is putting card into low power mode
> > > (e) re-check PCIE_HOST_INT_STATUS register; quit loop with 0 value
> > >
> > > But at (e), no one actually signaled an interrupt (i.e., we didn't
> > > check
> > > adapter->int_status). And what's more, because the card is going to
> > > sleep, this register read appears to take a very long time in some
> > > cases
> > > -- 3 milliseconds in my case!
> > >
> > > Now, I propose that (e) is completely unnecessary. If there were
> any
> > > additional interrupts signaled after the start of this loop, then
> > > the interrupt handler would have set adapter->int_status to non-
> zero
> > > and queued more work for the main loop -- and we'd catch it on the
> > > next iteration of the main loop.
> > >
> > > So this patch drops all the looping/re-reading of
> > > PCIE_HOST_INT_STATUS, which avoids the problematic (and slow)
> register read in step (e).
> > >
> > > Incidentally, this is a very similar issue to the one fixed in
> > > commit
> > > ec815dd2a5f1 ("mwifiex: prevent register accesses after host is
> > > sleeping"), except that the register read is just very slow instead
> > > of fatal in this case.
> > >
> > > Tested on 8997 in both MSI and (though not technically supported at
> > > the
> > > moment) MSI-X mode.
> >
> > Well, that kills interrupt mitigation and with PCIE that might be
> > somewhat important (SDIO is too slow to be important I think) and
> > might cost you throughput.
>
> Hmm, well I don't see us disabling interrupts in here, at least for MSI
> mode, so it doesn't actually look like a mitigation mechanism. More
> like a redundancy. But I'm not an expert on MSI, and definitely not on
> network performance.
>
> Also, FWIW, I did some fairly non-scientific tests of this on my
> systems, and I didn't see much difference. I can run better tests, and
> even collect data on how often we loop here vs. see new interrupts.
>
> > OTOH maybe Marvell should convert PICE to NAPI to make this more
> > obvious and probably more correct.
>
> From my brief reading, that sounds like a better way to make this
> configurable.
>
> So I'm not sure which way you'd suggest then; take a patch like this,
> which makes the driver more clear and less buggy? Or write some
> different patch that isolates just the power-save related condition, so
> we break out of this look [1]?
>
> I'm also interested in any opinions from the Marvell side --
> potentially testing results, rationale behind this code structure, or
> even a better alternative patch.
>
Thanks for the fix. It looks fine to me. Alternate fix would be below change.
We ran throughput tests with your patch vs below change. Results are almost same.
--------
@@ -2370,7 +2370,7 @@ static int mwifiex_process_pcie_int(struct mwifiex_adapter *adapter)
ret = mwifiex_pcie_process_cmd_complete(adapter);
if (ret)
return ret;
- if (adapter->hs_activated)
+ if (adapter->hs_activated || adapter->ps_state == PS_STATE_SLEEP)
return ret;
--------
The logic of having while loop here was to avoid scheduling latency. As Dmitry pointed out in other email packet aggregation takes care of interrupts arriving too closely together. We have Rx buffer ring of size 32. So we may get a single interrupt to deliver 32 Rx packets.
Regards,
Amitkumar
^ permalink raw reply
* Re: [PATCH V2 4/4] brcmfmac: rename brcmf_bus_start function to brcmf_bus_started
From: Arend Van Spriel @ 2017-01-18 12:40 UTC (permalink / raw)
To: Rafał Miłecki, Kalle Valo
Cc: Franky Lin, Hante Meuleman, Pieter-Paul Giesberts, Franky Lin,
linux-wireless, brcm80211-dev-list.pdl, Rafał Miłecki
In-Reply-To: <20170118104854.32231-4-zajec5@gmail.com>
On 18-1-2017 11:48, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This intends to make init/attach process slightly easier to follow.
>
> What driver was doing in brcmf_bus_start wasn't bus specific at all and
> function brcmf_bus_stop wasn't undoing things done there. This function
> is supposed to be called by bus specific code when the bus is ready.
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Use "brcmf_bus_started" name instead of "brcmf_attach_phase2", don't rename
> brcmf_attach.
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h | 2 +-
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +-
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 2 +-
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 2 +-
> 6 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
> index b5bb971..76693df 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
> @@ -238,7 +238,7 @@ void brcmf_txcomplete(struct device *dev, struct sk_buff *txp, bool success);
> /* Configure the "global" bus state used by upper layers */
> void brcmf_bus_change_state(struct brcmf_bus *bus, enum brcmf_bus_state state);
>
> -int brcmf_bus_start(struct device *dev);
> +int brcmf_bus_started(struct device *dev);
> s32 brcmf_iovar_data_set(struct device *dev, char *name, void *data, u32 len);
> void brcmf_bus_add_txhdrlen(struct device *dev, uint len);
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> index 3e15d64..5fb4a14 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> @@ -74,7 +74,7 @@ module_param_named(roamoff, brcmf_roamoff, int, S_IRUSR);
> MODULE_PARM_DESC(roamoff, "Do not use internal roaming engine");
>
> #ifdef DEBUG
> -/* always succeed brcmf_bus_start() */
> +/* always succeed brcmf_bus_started() */
> static int brcmf_ignore_probe_fail;
> module_param_named(ignore_probe_fail, brcmf_ignore_probe_fail, int, 0);
> MODULE_PARM_DESC(ignore_probe_fail, "always succeed probe for debugging");
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> index 372d2c4..b73a55b 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> @@ -966,7 +966,7 @@ static int brcmf_revinfo_read(struct seq_file *s, void *data)
> return 0;
> }
>
> -int brcmf_bus_start(struct device *dev)
> +int brcmf_bus_started(struct device *dev)
> {
> int ret = -1;
> struct brcmf_bus *bus_if = dev_get_drvdata(dev);
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index 048027f..19db8f2 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -1572,7 +1572,7 @@ static int brcmf_pcie_attach_bus(struct brcmf_pciedev_info *devinfo)
> if (ret) {
> brcmf_err("brcmf_attach failed\n");
> } else {
> - ret = brcmf_bus_start(&devinfo->pdev->dev);
> + ret = brcmf_bus_started(&devinfo->pdev->dev);
> if (ret)
> brcmf_err("dongle is not responding\n");
> }
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index dfb0658..5298582 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -4065,7 +4065,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev,
>
> sdio_release_host(sdiodev->func[1]);
>
> - err = brcmf_bus_start(dev);
> + err = brcmf_bus_started(dev);
> if (err != 0) {
> brcmf_err("dongle is not responding\n");
> goto fail;
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
> index 2f978a3..d93ebbd 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
> @@ -1148,7 +1148,7 @@ static int brcmf_usb_bus_setup(struct brcmf_usbdev_info *devinfo)
> if (ret)
> goto fail;
>
> - ret = brcmf_bus_start(devinfo->dev);
> + ret = brcmf_bus_started(devinfo->dev);
> if (ret)
> goto fail;
>
>
^ permalink raw reply
* Re: [PATCH v3] brcmfmac: fix incorrect event channel deduction
From: Arend Van Spriel @ 2017-01-18 12:38 UTC (permalink / raw)
To: gavinli, franky.lin, hante.meuleman, linux-wireless,
brcm80211-dev-list.pdl
Cc: stable, Gavin Li
In-Reply-To: <20170117232405.7672-1-gavinli@thegavinli.com>
On 18-1-2017 0:24, gavinli@thegavinli.com wrote:
> From: Gavin Li <git@thegavinli.com>
>
> brcmf_sdio_fromevntchan() was being called on the the data frame
> rather than the software header, causing some frames to be
> mischaracterized as on the event channel rather than the data channel.
>
> This fixes a major performance regression (due to dropped packets).
>
> Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet")
Actually would prefer Franky to chime in as well as he made the change
and confirm correctness. It was introduced a couple of kernel versions
back and only a performance regression so seems ok to let this go in
4.11 for now and backport as needed for stable later.
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Gavin Li <git@thegavinli.com>
> Cc: <stable@vger.kernel.org> [4.7+]
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index dfb0658713d9..d2219885071f 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -1661,7 +1661,7 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
> pfirst->len, pfirst->next,
> pfirst->prev);
> skb_unlink(pfirst, &bus->glom);
> - if (brcmf_sdio_fromevntchan(pfirst->data))
> + if (brcmf_sdio_fromevntchan(&dptr[SDPCM_HWHDR_LEN]))
> brcmf_rx_event(bus->sdiodev->dev, pfirst);
> else
> brcmf_rx_frame(bus->sdiodev->dev, pfirst,
>
^ permalink raw reply
* Re: [2/2] rsi: Device initialization sequence is changed
From: Kalle Valo @ 2017-01-18 11:46 UTC (permalink / raw)
To: Prameela Rani Garnepudi
Cc: linux-wireless, johannes.berg, hofrat, xypron.glpk,
prameela.garnepudi, Prameela Rani Garnepudi
In-Reply-To: <1477044656-10402-1-git-send-email-prameela.j04cs@gmail.com>
Prameela Rani Garnepudi <prameela.j04cs@gmail.com> wrote:
> BT Co-ex support has been added in the firmware and hence BT, COEX queues are
> introducted. To support the latest firmware queues, packet processing,
> and initialization sequence is modified in the Host.
>
> Common device configuration parameters (tx command frame) need to be send after
> common card ready indication from the firmware. This frame contains information
> about the driver mode, coex mode, protocols to support etc. Once it is sent,
> then second level card ready indication comes for each protocol (WLAN, BT etc).
>
> Currently host supports only WLAN mode. New command frames are added as part
> of these changes.
>
> Signed-off-by: Prameela Rani Garnepudi <prameela.j04cs@gmail.com>
Few review comments:
* no C++ '//' style comments
* avoid magic numbers
* use structs for new firmware commands instead of cmd[offset] style
* struct rsi_ulp_gpio_vals, struct rsi_soc_gpio_vals and struct rsi_config_vals
don't look endian safe
--
https://patchwork.kernel.org/patch/9388627/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] brcmfmac: fix incorrect event channel deduction
From: Kalle Valo @ 2017-01-18 10:40 UTC (permalink / raw)
To: gavinli
Cc: arend.vanspriel, franky.lin, hante.meuleman, linux-wireless,
brcm80211-dev-list.pdl, stable, Gavin Li
In-Reply-To: <87r340hdde.fsf@purkki.adurom.net>
Kalle Valo <kvalo@codeaurora.org> writes:
> gavinli@thegavinli.com writes:
>
>> From: Gavin Li <git@thegavinli.com>
>>
>> brcmf_sdio_fromevntchan() was being called on the the data frame
>> rather than the software header, causing some frames to be
>> mischaracterized as on the event channel rather than the data channel.
>>
>> This fixes a major performance regression (due to dropped packets).
>>
>> Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet")
>> Signed-off-by: Gavin Li <git@thegavinli.com>
>> Cc: <stable@vger.kernel.org> [4.7+]
>
> In the future please add version to the subject so that maintainers can
> easily find the latest version:
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#subject
Ah, I missed that you had submitted v3:
https://patchwork.kernel.org/patch/9522185/
That's the version I'm planning to take (if Arend acks it).
--
Kalle Valo
^ permalink raw reply
* [PATCH V2 4/4] brcmfmac: rename brcmf_bus_start function to brcmf_bus_started
From: Rafał Miłecki @ 2017-01-18 10:48 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, Rafał Miłecki
In-Reply-To: <20170118104854.32231-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
This intends to make init/attach process slightly easier to follow.
What driver was doing in brcmf_bus_start wasn't bus specific at all and
function brcmf_bus_stop wasn't undoing things done there. This function
is supposed to be called by bus specific code when the bus is ready.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Use "brcmf_bus_started" name instead of "brcmf_attach_phase2", don't rename
brcmf_attach.
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h | 2 +-
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +-
drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 2 +-
drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 2 +-
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
index b5bb971..76693df 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
@@ -238,7 +238,7 @@ void brcmf_txcomplete(struct device *dev, struct sk_buff *txp, bool success);
/* Configure the "global" bus state used by upper layers */
void brcmf_bus_change_state(struct brcmf_bus *bus, enum brcmf_bus_state state);
-int brcmf_bus_start(struct device *dev);
+int brcmf_bus_started(struct device *dev);
s32 brcmf_iovar_data_set(struct device *dev, char *name, void *data, u32 len);
void brcmf_bus_add_txhdrlen(struct device *dev, uint len);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index 3e15d64..5fb4a14 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -74,7 +74,7 @@ module_param_named(roamoff, brcmf_roamoff, int, S_IRUSR);
MODULE_PARM_DESC(roamoff, "Do not use internal roaming engine");
#ifdef DEBUG
-/* always succeed brcmf_bus_start() */
+/* always succeed brcmf_bus_started() */
static int brcmf_ignore_probe_fail;
module_param_named(ignore_probe_fail, brcmf_ignore_probe_fail, int, 0);
MODULE_PARM_DESC(ignore_probe_fail, "always succeed probe for debugging");
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 372d2c4..b73a55b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -966,7 +966,7 @@ static int brcmf_revinfo_read(struct seq_file *s, void *data)
return 0;
}
-int brcmf_bus_start(struct device *dev)
+int brcmf_bus_started(struct device *dev)
{
int ret = -1;
struct brcmf_bus *bus_if = dev_get_drvdata(dev);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 048027f..19db8f2 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1572,7 +1572,7 @@ static int brcmf_pcie_attach_bus(struct brcmf_pciedev_info *devinfo)
if (ret) {
brcmf_err("brcmf_attach failed\n");
} else {
- ret = brcmf_bus_start(&devinfo->pdev->dev);
+ ret = brcmf_bus_started(&devinfo->pdev->dev);
if (ret)
brcmf_err("dongle is not responding\n");
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index dfb0658..5298582 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4065,7 +4065,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev,
sdio_release_host(sdiodev->func[1]);
- err = brcmf_bus_start(dev);
+ err = brcmf_bus_started(dev);
if (err != 0) {
brcmf_err("dongle is not responding\n");
goto fail;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
index 2f978a3..d93ebbd 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
@@ -1148,7 +1148,7 @@ static int brcmf_usb_bus_setup(struct brcmf_usbdev_info *devinfo)
if (ret)
goto fail;
- ret = brcmf_bus_start(devinfo->dev);
+ ret = brcmf_bus_started(devinfo->dev);
if (ret)
goto fail;
--
2.10.1
^ permalink raw reply related
* [PATCH V2 3/4] brcmfmac: drop brcmf_bus_detach and inline its code
From: Rafał Miłecki @ 2017-01-18 10:48 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, Rafał Miłecki
In-Reply-To: <20170118104854.32231-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
Driver used to call brcmf_bus_detach only from one place and it already
contained a check for drvr not being NULL. We can get rid of this extra
function, call brcmf_bus_stop directly and simplify the code.
There also isn't brcmf_bus_attach function which one could expect so it
looks more consistent this way.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 9e6f60a..372d2c4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -1075,16 +1075,6 @@ void brcmf_bus_add_txhdrlen(struct device *dev, uint len)
}
}
-static void brcmf_bus_detach(struct brcmf_pub *drvr)
-{
- brcmf_dbg(TRACE, "Enter\n");
-
- if (drvr) {
- /* Stop the bus module */
- brcmf_bus_stop(drvr->bus_if);
- }
-}
-
void brcmf_dev_reset(struct device *dev)
{
struct brcmf_bus *bus_if = dev_get_drvdata(dev);
@@ -1131,7 +1121,7 @@ void brcmf_detach(struct device *dev)
brcmf_fws_deinit(drvr);
- brcmf_bus_detach(drvr);
+ brcmf_bus_stop(drvr->bus_if);
brcmf_proto_detach(drvr);
--
2.10.1
^ permalink raw reply related
* [PATCH V2 2/4] brcmfmac: move brcmf_c_set_joinpref_default declaration to common.h
From: Rafał Miłecki @ 2017-01-18 10:48 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, Rafał Miłecki
In-Reply-To: <20170118104854.32231-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
Function brcmf_c_set_joinpref_default is in common.c, so move it to the
related header.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
V2: Move brcmf_c_set_joinpref_default only
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h | 2 ++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h | 1 -
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
index bd095ab..a62f8e7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.h
@@ -65,6 +65,8 @@ struct brcmf_mp_device {
} bus;
};
+void brcmf_c_set_joinpref_default(struct brcmf_if *ifp);
+
struct brcmf_mp_device *brcmf_get_module_param(struct device *dev,
enum brcmf_bus_type bus_type,
u32 chip, u32 chiprev);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
index c94dcab..de3197b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
@@ -216,7 +216,6 @@ void brcmf_txflowblock_if(struct brcmf_if *ifp,
void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success);
void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb);
void brcmf_net_setcarrier(struct brcmf_if *ifp, bool on);
-void brcmf_c_set_joinpref_default(struct brcmf_if *ifp);
int __init brcmf_core_init(void);
void __exit brcmf_core_exit(void);
--
2.10.1
^ 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