linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/14] brcmfmac: driver cleanup and rework
@ 2014-02-25 19:30 Arend van Spriel
  2014-02-25 19:30 ` [PATCH 01/14] brcmfmac: add delay before unregistering the network device Arend van Spriel
                   ` (13 more replies)
  0 siblings, 14 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Arend van Spriel

This series includes a number of cleanup changes and rework mainly
in the sdio driver. These are intended for 3.15 kernel and apply
on the master branch of the wireless-next repository.

Arend van Spriel (5):
  brcmfmac: add delay before unregistering the network device
  brcmfmac: fix use of skb control buffer in SDIO driver part
  brcmfmac: remove unused variable data_len from
    brcmf_sdio_bus_txdata()
  brcmfmac: use pre-allocated scatter-gather table for txglomming
  brcmfmac: reset suspend flag upon sdio suspend failure

Daniel Kim (1):
  brcmfmac: Correct mcs index report

Hante Meuleman (8):
  brcmfmac: Make firmeware roaming a module param.
  brcmfmac: Correct header debug dump for sdio tx hdrs.
  brcmfmac: de-init driver layers in correct order.
  brcmfmac: Minimize SDIO dpc scheduling.
  brcmfmac: Remove immediate sleep support from SDIO.
  brcmfmac: Small cleanup of redundant code.
  brcmfmac: Use atomic functions for intstatus update.
  brcmfmac: Put frame sdio tx error handling in sub function.

 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c   |   53 +++--
 .../net/wireless/brcm80211/brcmfmac/dhd_linux.c    |    8 +-
 drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |  209 +++++++-------------
 .../net/wireless/brcm80211/brcmfmac/sdio_host.h    |    2 +
 .../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c  |   64 +++---
 .../net/wireless/brcm80211/brcmfmac/wl_cfg80211.h  |    1 -
 6 files changed, 154 insertions(+), 183 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 19:59   ` Johannes Berg
  2014-02-25 19:30 ` [PATCH 02/14] brcmfmac: Make firmeware roaming a module param Arend van Spriel
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Arend van Spriel

Upon deleting the interface a cfg80211_disconnected() is called under
rtnl_lock. Right after the unlocking the rtnl_lock we unregister the
network device. This patch adds delay before unregister so cfg80211
can handle disconnect and notify wpa_supplicant.

Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Reviewed-by: Hante Meuleman <meuleman@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c   |    4 ++++
 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c |    9 +--------
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
index d4d966b..9832b11 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
@@ -848,6 +848,10 @@ void brcmf_del_if(struct brcmf_pub *drvr, s32 bssidx)
 				rtnl_lock();
 				brcmf_netdev_stop(ifp->ndev);
 				rtnl_unlock();
+				/* make sure cfg80211 can send disconnect event
+				 * before unregistering the netdevice below.
+				 */
+				msleep(100);
 			}
 		} else {
 			netif_stop_queue(ifp->ndev);
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index a54db91..51ee7f0 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -5443,16 +5443,9 @@ static s32 __brcmf_cfg80211_down(struct brcmf_if *ifp)
 	 * While going down, if associated with AP disassociate
 	 * from AP to save power
 	 */
-	if (check_vif_up(ifp->vif)) {
+	if (check_vif_up(ifp->vif))
 		brcmf_link_down(ifp->vif);
 
-		/* Make sure WPA_Supplicant receives all the event
-		   generated due to DISASSOC call to the fw to keep
-		   the state fw and WPA_Supplicant state consistent
-		 */
-		brcmf_delay(500);
-	}
-
 	brcmf_abort_scanning(cfg);
 	clear_bit(BRCMF_VIF_STATUS_READY, &ifp->vif->sme_state);
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 02/14] brcmfmac: Make firmeware roaming a module param.
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
  2014-02-25 19:30 ` [PATCH 01/14] brcmfmac: add delay before unregistering the network device Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 19:30 ` [PATCH 03/14] brcmfmac: fix use of skb control buffer in SDIO driver part Arend van Spriel
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Hante Meuleman, Arend van Spriel

From: Hante Meuleman <meuleman@broadcom.com>

Internal firmware roaming is enabled by default. This patch
makes it possible to disable internal firmware roaming by
specifying roamoff=1 as module param.

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 .../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c  |   38 +++++++++++++-------
 .../net/wireless/brcm80211/brcmfmac/wl_cfg80211.h  |    1 -
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index 51ee7f0..81be69d 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -18,6 +18,7 @@
 
 #include <linux/kernel.h>
 #include <linux/etherdevice.h>
+#include <linux/module.h>
 #include <net/cfg80211.h>
 #include <net/netlink.h>
 
@@ -251,6 +252,10 @@ struct parsed_vndr_ies {
 	struct parsed_vndr_ie_info ie_info[VNDR_IE_PARSE_LIMIT];
 };
 
+static int brcmf_roamoff;
+module_param_named(roamoff, brcmf_roamoff, int, S_IRUSR);
+MODULE_PARM_DESC(roamoff, "do not use internal roaming engine");
+
 /* Quarter dBm units to mW
  * Table starts at QDBM_OFFSET, so the first entry is mW for qdBm=153
  * Table is offset so the last entry is largest mW value that fits in
@@ -4444,7 +4449,9 @@ static bool brcmf_is_linkdown(const struct brcmf_event_msg *e)
 	u32 event = e->event_code;
 	u16 flags = e->flags;
 
-	if (event == BRCMF_E_LINK && (!(flags & BRCMF_EVENT_MSG_LINK))) {
+	if ((event == BRCMF_E_DEAUTH) || (event == BRCMF_E_DEAUTH_IND) ||
+	    (event == BRCMF_E_DISASSOC_IND) ||
+	    ((event == BRCMF_E_LINK) && (!(flags & BRCMF_EVENT_MSG_LINK)))) {
 		brcmf_dbg(CONN, "Processing link down\n");
 		return true;
 	}
@@ -4688,6 +4695,7 @@ brcmf_notify_connect_status(struct brcmf_if *ifp,
 	struct brcmf_cfg80211_profile *profile = &ifp->vif->profile;
 	struct ieee80211_channel *chan;
 	s32 err = 0;
+	u16 reason;
 
 	if (ifp->vif->mode == WL_MODE_AP) {
 		err = brcmf_notify_connect_status_ap(cfg, ndev, e, data);
@@ -4709,9 +4717,15 @@ brcmf_notify_connect_status(struct brcmf_if *ifp,
 		if (!brcmf_is_ibssmode(ifp->vif)) {
 			brcmf_bss_connect_done(cfg, ndev, e, false);
 			if (test_and_clear_bit(BRCMF_VIF_STATUS_CONNECTED,
-					       &ifp->vif->sme_state))
-				cfg80211_disconnected(ndev, 0, NULL, 0,
+					       &ifp->vif->sme_state)) {
+				reason = 0;
+				if (((e->event_code == BRCMF_E_DEAUTH_IND) ||
+				     (e->event_code == BRCMF_E_DISASSOC_IND)) &&
+				    (e->reason != WLAN_REASON_UNSPECIFIED))
+					reason = e->reason;
+				cfg80211_disconnected(ndev, reason, NULL, 0,
 						      GFP_KERNEL);
+			}
 		}
 		brcmf_link_down(ifp->vif);
 		brcmf_init_prof(ndev_to_prof(ndev));
@@ -4905,11 +4919,8 @@ static s32 wl_init_priv(struct brcmf_cfg80211_info *cfg)
 
 	cfg->scan_request = NULL;
 	cfg->pwr_save = true;
-	cfg->roam_on = true;	/* roam on & off switch.
-				 we enable roam per default */
-	cfg->active_scan = true;	/* we do active scan for
-				 specific scan per default */
-	cfg->dongle_up = false;	/* dongle is not up yet */
+	cfg->active_scan = true;	/* we do active scan per default */
+	cfg->dongle_up = false;		/* dongle is not up yet */
 	err = brcmf_init_priv_mem(cfg);
 	if (err)
 		return err;
@@ -5029,7 +5040,7 @@ void brcmf_cfg80211_detach(struct brcmf_cfg80211_info *cfg)
 }
 
 static s32
-brcmf_dongle_roam(struct brcmf_if *ifp, u32 roamvar, u32 bcn_timeout)
+brcmf_dongle_roam(struct brcmf_if *ifp, u32 bcn_timeout)
 {
 	s32 err = 0;
 	__le32 roamtrigger[2];
@@ -5039,7 +5050,7 @@ brcmf_dongle_roam(struct brcmf_if *ifp, u32 roamvar, u32 bcn_timeout)
 	 * Setup timeout if Beacons are lost and roam is
 	 * off to report link down
 	 */
-	if (roamvar) {
+	if (brcmf_roamoff) {
 		err = brcmf_fil_iovar_int_set(ifp, "bcn_timeout", bcn_timeout);
 		if (err) {
 			brcmf_err("bcn_timeout error (%d)\n", err);
@@ -5051,8 +5062,9 @@ brcmf_dongle_roam(struct brcmf_if *ifp, u32 roamvar, u32 bcn_timeout)
 	 * Enable/Disable built-in roaming to allow supplicant
 	 * to take care of roaming
 	 */
-	brcmf_dbg(INFO, "Internal Roaming = %s\n", roamvar ? "Off" : "On");
-	err = brcmf_fil_iovar_int_set(ifp, "roam_off", roamvar);
+	brcmf_dbg(INFO, "Internal Roaming = %s\n",
+		  brcmf_roamoff ? "Off" : "On");
+	err = brcmf_fil_iovar_int_set(ifp, "roam_off", !!(brcmf_roamoff));
 	if (err) {
 		brcmf_err("roam_off error (%d)\n", err);
 		goto dongle_rom_out;
@@ -5408,7 +5420,7 @@ static s32 brcmf_config_dongle(struct brcmf_cfg80211_info *cfg)
 	brcmf_dbg(INFO, "power save set to %s\n",
 		  (power_mode ? "enabled" : "disabled"));
 
-	err = brcmf_dongle_roam(ifp, (cfg->roam_on ? 0 : 1), WL_BEACON_TIMEOUT);
+	err = brcmf_dongle_roam(ifp, WL_BEACON_TIMEOUT);
 	if (err)
 		goto default_conf_out;
 	err = brcmf_cfg80211_change_iface(wdev->wiphy, ndev, wdev->iftype,
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
index 254feed..5715bb0 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.h
@@ -402,7 +402,6 @@ struct brcmf_cfg80211_info {
 	bool ibss_starter;
 	bool pwr_save;
 	bool dongle_up;
-	bool roam_on;
 	bool scan_tried;
 	u8 *dcmd_buf;
 	u8 *extra_buf;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 03/14] brcmfmac: fix use of skb control buffer in SDIO driver part
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
  2014-02-25 19:30 ` [PATCH 01/14] brcmfmac: add delay before unregistering the network device Arend van Spriel
  2014-02-25 19:30 ` [PATCH 02/14] brcmfmac: Make firmeware roaming a module param Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 19:30 ` [PATCH 04/14] brcmfmac: remove unused variable data_len from brcmf_sdio_bus_txdata() Arend van Spriel
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Arend van Spriel

The SDIO driver has a 16-bit field defined in the skbuff control buffer.
However, it is accessed as a u32 overwriting other control info. Another
issue is that the field is not initialized for networking packets, but
the control buffer content is unspecified as other networking layers can
use it.

Reviewed-by: Hante Meuleman <meuleman@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index 631d5dc..fa4ec69 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
@@ -2112,7 +2112,7 @@ static int brcmf_sdio_txpkt_prep_sg(struct brcmf_sdio *bus,
 		memcpy(pkt_pad->data,
 		       pkt->data + pkt->len - tail_chop,
 		       tail_chop);
-		*(u32 *)(pkt_pad->cb) = ALIGN_SKB_FLAG + tail_chop;
+		*(u16 *)(pkt_pad->cb) = ALIGN_SKB_FLAG + tail_chop;
 		skb_trim(pkt, pkt->len - tail_chop);
 		__skb_queue_after(pktq, pkt, pkt_pad);
 	} else {
@@ -2159,7 +2159,7 @@ brcmf_sdio_txpkt_prep(struct brcmf_sdio *bus, struct sk_buff_head *pktq,
 		 * already properly aligned and does not
 		 * need an sdpcm header.
 		 */
-		if (*(u32 *)(pkt_next->cb) & ALIGN_SKB_FLAG)
+		if (*(u16 *)(pkt_next->cb) & ALIGN_SKB_FLAG)
 			continue;
 
 		/* align packet data pointer */
@@ -2223,11 +2223,11 @@ brcmf_sdio_txpkt_postp(struct brcmf_sdio *bus, struct sk_buff_head *pktq)
 	u8 *hdr;
 	u32 dat_offset;
 	u16 tail_pad;
-	u32 dummy_flags, chop_len;
+	u16 dummy_flags, chop_len;
 	struct sk_buff *pkt_next, *tmp, *pkt_prev;
 
 	skb_queue_walk_safe(pktq, pkt_next, tmp) {
-		dummy_flags = *(u32 *)(pkt_next->cb);
+		dummy_flags = *(u16 *)(pkt_next->cb);
 		if (dummy_flags & ALIGN_SKB_FLAG) {
 			chop_len = dummy_flags & ALIGN_SKB_CHOP_LEN_MASK;
 			if (chop_len) {
@@ -2709,6 +2709,8 @@ static int brcmf_sdio_bus_txdata(struct device *dev, struct sk_buff *pkt)
 
 	/* Priority based enq */
 	spin_lock_irqsave(&bus->txqlock, flags);
+	/* reset bus_flags in packet cb */
+	*(u16 *)(pkt->cb) = 0;
 	if (!brcmf_c_prec_enq(bus->sdiodev->dev, &bus->txq, pkt, prec)) {
 		skb_pull(pkt, bus->tx_hdrlen);
 		brcmf_err("out of bus->txq !!!\n");
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 04/14] brcmfmac: remove unused variable data_len from brcmf_sdio_bus_txdata()
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
                   ` (2 preceding siblings ...)
  2014-02-25 19:30 ` [PATCH 03/14] brcmfmac: fix use of skb control buffer in SDIO driver part Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 19:30 ` [PATCH 05/14] brcmfmac: Correct header debug dump for sdio tx hdrs Arend van Spriel
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Arend van Spriel

The local variable data_len is assigned but never used so get rid
of it.

Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Reviewed-by: Hante Meuleman <meuleman@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index fa4ec69..ce1ee1e 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
@@ -2686,15 +2686,13 @@ static struct pktq *brcmf_sdio_bus_gettxq(struct device *dev)
 static int brcmf_sdio_bus_txdata(struct device *dev, struct sk_buff *pkt)
 {
 	int ret = -EBADE;
-	uint datalen, prec;
+	uint prec;
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
 	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
 	struct brcmf_sdio *bus = sdiodev->bus;
 	ulong flags;
 
-	brcmf_dbg(TRACE, "Enter\n");
-
-	datalen = pkt->len;
+	brcmf_dbg(TRACE, "Enter: pkt: data %p len %d\n", pkt->data, pkt->len);
 
 	/* Add space for the header */
 	skb_push(pkt, bus->tx_hdrlen);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 05/14] brcmfmac: Correct header debug dump for sdio tx hdrs.
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
                   ` (3 preceding siblings ...)
  2014-02-25 19:30 ` [PATCH 04/14] brcmfmac: remove unused variable data_len from brcmf_sdio_bus_txdata() Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 19:30 ` [PATCH 06/14] brcmfmac: de-init driver layers in correct order Arend van Spriel
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Hante Meuleman, Arend van Spriel

From: Hante Meuleman <meuleman@broadcom.com>

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index ce1ee1e..ce99732 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
@@ -2193,10 +2193,10 @@ brcmf_sdio_txpkt_prep(struct brcmf_sdio *bus, struct sk_buff_head *pktq,
 		if (BRCMF_BYTES_ON() &&
 		    ((BRCMF_CTL_ON() && chan == SDPCM_CONTROL_CHANNEL) ||
 		     (BRCMF_DATA_ON() && chan != SDPCM_CONTROL_CHANNEL)))
-			brcmf_dbg_hex_dump(true, pkt_next, hd_info.len,
+			brcmf_dbg_hex_dump(true, pkt_next->data, hd_info.len,
 					   "Tx Frame:\n");
 		else if (BRCMF_HDRS_ON())
-			brcmf_dbg_hex_dump(true, pkt_next,
+			brcmf_dbg_hex_dump(true, pkt_next->data,
 					   head_pad + bus->tx_hdrlen,
 					   "Tx Header:\n");
 	}
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 06/14] brcmfmac: de-init driver layers in correct order.
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
                   ` (4 preceding siblings ...)
  2014-02-25 19:30 ` [PATCH 05/14] brcmfmac: Correct header debug dump for sdio tx hdrs Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 19:30 ` [PATCH 07/14] brcmfmac: Minimize SDIO dpc scheduling Arend van Spriel
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Hante Meuleman, Arend van Spriel

From: Hante Meuleman <meuleman@broadcom.com>

First clean up fw signalling, before cleaning up the bus and
proto layer. Old order can cause oops in some circumstances.

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
index 9832b11..2fb922f 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
@@ -1044,12 +1044,12 @@ void brcmf_detach(struct device *dev)
 
 	brcmf_cfg80211_detach(drvr->config);
 
+	brcmf_fws_deinit(drvr);
+
 	brcmf_bus_detach(drvr);
 
 	brcmf_proto_detach(drvr);
 
-	brcmf_fws_deinit(drvr);
-
 	brcmf_debugfs_detach(drvr);
 	bus_if->drvr = NULL;
 	kfree(drvr);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 07/14] brcmfmac: Minimize SDIO dpc scheduling.
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
                   ` (5 preceding siblings ...)
  2014-02-25 19:30 ` [PATCH 06/14] brcmfmac: de-init driver layers in correct order Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 19:30 ` [PATCH 08/14] brcmfmac: Remove immediate sleep support from SDIO Arend van Spriel
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Hante Meuleman, Arend van Spriel

From: Hante Meuleman <meuleman@broadcom.com>

SDIO dpc scheduling is done (repeated) when counter is set. This
counter gets decreased when dpc is finished. It is more efficient
to set counter to 0 before the dpc is actullay run. This will
minimize the frequency with which dpc is executed.

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index ce99732..cc0f1ad 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
@@ -3758,8 +3758,8 @@ static void brcmf_sdio_dataworker(struct work_struct *work)
 					      datawork);
 
 	while (atomic_read(&bus->dpc_tskcnt)) {
+		atomic_set(&bus->dpc_tskcnt, 0);
 		brcmf_sdio_dpc(bus);
-		atomic_dec(&bus->dpc_tskcnt);
 	}
 }
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 08/14] brcmfmac: Remove immediate sleep support from SDIO.
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
                   ` (6 preceding siblings ...)
  2014-02-25 19:30 ` [PATCH 07/14] brcmfmac: Minimize SDIO dpc scheduling Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 19:30 ` [PATCH 09/14] brcmfmac: Small cleanup of redundant code Arend van Spriel
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Hante Meuleman, Arend van Spriel

From: Hante Meuleman <meuleman@broadcom.com>

Immediate sleep support is an aggressive power saving option
that has not been enabled in brcmfmac and is removed to
simplify code.

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |   20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index cc0f1ad..04f5e32c 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
@@ -304,7 +304,6 @@ struct rte_console {
 /* Flags for SDH calls */
 #define F2SYNC	(SDIO_REQ_4BYTE | SDIO_REQ_FIXED)
 
-#define BRCMF_IDLE_IMMEDIATE	(-1)	/* Enter idle immediately */
 #define BRCMF_IDLE_ACTIVE	0	/* Do not request any SD clock change
 					 * when idle
 					 */
@@ -2662,16 +2661,6 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 		    data_ok(bus)) || PKT_AVAILABLE()) {
 		atomic_inc(&bus->dpc_tskcnt);
 	}
-
-	/* If we're done for now, turn off clock request. */
-	if ((bus->clkstate != CLK_PENDING)
-	    && bus->idletime == BRCMF_IDLE_IMMEDIATE) {
-		bus->activity = false;
-		brcmf_dbg(SDIO, "idle state\n");
-		sdio_claim_host(bus->sdiodev->func[1]);
-		brcmf_sdio_bus_sleep(bus, true, false);
-		sdio_release_host(bus->sdiodev->func[1]);
-	}
 }
 
 static struct pktq *brcmf_sdio_bus_gettxq(struct device *dev)
@@ -2948,15 +2937,6 @@ brcmf_sdio_bus_txctl(struct device *dev, unsigned char *msg, uint msglen)
 		} while (ret < 0 && retries++ < TXRETRIES);
 	}
 
-	if ((bus->idletime == BRCMF_IDLE_IMMEDIATE) &&
-	    atomic_read(&bus->dpc_tskcnt) == 0) {
-		bus->activity = false;
-		sdio_claim_host(bus->sdiodev->func[1]);
-		brcmf_dbg(INFO, "idle\n");
-		brcmf_sdio_clkctl(bus, CLK_NONE, true);
-		sdio_release_host(bus->sdiodev->func[1]);
-	}
-
 	if (ret)
 		bus->sdcnt.tx_ctlerrs++;
 	else
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 09/14] brcmfmac: Small cleanup of redundant code.
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
                   ` (7 preceding siblings ...)
  2014-02-25 19:30 ` [PATCH 08/14] brcmfmac: Remove immediate sleep support from SDIO Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 19:30 ` [PATCH 10/14] brcmfmac: Use atomic functions for intstatus update Arend van Spriel
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Hante Meuleman, Arend van Spriel

From: Hante Meuleman <meuleman@broadcom.com>

In time some of the code got redundant, without being noticed.
This patch does not change any functionality, just removes
redundant code.

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |   18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index 04f5e32c..ac61419 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
@@ -770,8 +770,6 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
 	return err;
 }
 
-#define PKT_AVAILABLE()		(intstatus & I_HMB_FRAME_IND)
-
 #define HOSTINTMASK		(I_HMB_SW_MASK | I_CHIPACTIVE)
 
 /* Turn backplane clock on or off */
@@ -870,7 +868,6 @@ static int brcmf_sdio_htclk(struct brcmf_sdio *bus, bool on, bool pendok)
 		}
 #endif				/* defined (DEBUG) */
 
-		bus->activity = true;
 	} else {
 		clkreq = 0;
 
@@ -2341,7 +2338,7 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes)
 		cnt += i;
 
 		/* In poll mode, need to check for other events */
-		if (!bus->intr && cnt) {
+		if (!bus->intr) {
 			/* Check device status, signal pending interrupt */
 			sdio_claim_host(bus->sdiodev->func[1]);
 			ret = r_sdreg32(bus, &intstatus,
@@ -2485,9 +2482,8 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 {
 	u32 newstatus = 0;
 	unsigned long intstatus;
-	uint rxlimit = bus->rxbound;	/* Rx frames to read before resched */
 	uint txlimit = bus->txbound;	/* Tx frames to send before resched */
-	uint framecnt = 0;	/* Temporary counter of tx/rx frames */
+	uint framecnt;			/* Temporary counter of tx/rx frames */
 	int err = 0, n;
 
 	brcmf_dbg(TRACE, "Enter\n");
@@ -2585,11 +2581,10 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 		intstatus &= ~I_HMB_FRAME_IND;
 
 	/* On frame indication, read available frames */
-	if (PKT_AVAILABLE() && bus->clkstate == CLK_AVAIL) {
-		framecnt = brcmf_sdio_readframes(bus, rxlimit);
+	if ((intstatus & I_HMB_FRAME_IND) && (bus->clkstate == CLK_AVAIL)) {
+		brcmf_sdio_readframes(bus, bus->rxbound);
 		if (!bus->rxpending)
 			intstatus &= ~I_HMB_FRAME_IND;
-		rxlimit -= min(framecnt, rxlimit);
 	}
 
 	/* Keep still-pending events for next scheduling */
@@ -2647,8 +2642,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 		 && data_ok(bus)) {
 		framecnt = bus->rxpending ? min(txlimit, bus->txminmax) :
 					    txlimit;
-		framecnt = brcmf_sdio_sendfromq(bus, framecnt);
-		txlimit -= framecnt;
+		brcmf_sdio_sendfromq(bus, framecnt);
 	}
 
 	if (!brcmf_bus_ready(bus->sdiodev->bus_if) || (err != 0)) {
@@ -2658,7 +2652,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 		   atomic_read(&bus->ipend) > 0 ||
 		   (!atomic_read(&bus->fcstate) &&
 		    brcmu_pktq_mlen(&bus->txq, ~bus->flowcontrol) &&
-		    data_ok(bus)) || PKT_AVAILABLE()) {
+		    data_ok(bus))) {
 		atomic_inc(&bus->dpc_tskcnt);
 	}
 }
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 10/14] brcmfmac: Use atomic functions for intstatus update.
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
                   ` (8 preceding siblings ...)
  2014-02-25 19:30 ` [PATCH 09/14] brcmfmac: Small cleanup of redundant code Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 23:38   ` Florian Fainelli
  2014-02-25 19:30 ` [PATCH 11/14] brcmfmac: Put frame sdio tx error handling in sub function Arend van Spriel
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Hante Meuleman, Arend van Spriel

From: Hante Meuleman <meuleman@broadcom.com>

The intstatus in sdio code can be updated from different
threads. To protect intstatus access, atomic functions are
used. One of them is set_bit, but this function is not
guaranteed atomic on all platforms. The loop was replaced
with local created OR function.

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |   29 ++++++++++----------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index ac61419..90ff406 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
@@ -2444,12 +2444,21 @@ static inline void brcmf_sdio_clrintr(struct brcmf_sdio *bus)
 	}
 }
 
+static void atomic_orr(int val, atomic_t *v)
+{
+	int old_val;
+
+	old_val = atomic_read(v);
+	while (atomic_cmpxchg(v, old_val, val | old_val) != old_val)
+		old_val = atomic_read(v);
+}
+
 static int brcmf_sdio_intr_rstatus(struct brcmf_sdio *bus)
 {
 	struct brcmf_core *buscore;
 	u32 addr;
 	unsigned long val;
-	int n, ret;
+	int ret;
 
 	buscore = brcmf_chip_get_core(bus->ci, BCMA_CORE_SDIO_DEV);
 	addr = buscore->base + offsetof(struct sdpcmd_regs, intstatus);
@@ -2457,7 +2466,7 @@ static int brcmf_sdio_intr_rstatus(struct brcmf_sdio *bus)
 	val = brcmf_sdiod_regrl(bus->sdiodev, addr, &ret);
 	bus->sdcnt.f1regdata++;
 	if (ret != 0)
-		val = 0;
+		return ret;
 
 	val &= bus->hostintmask;
 	atomic_set(&bus->fcstate, !!(val & I_HMB_FC_STATE));
@@ -2466,13 +2475,7 @@ static int brcmf_sdio_intr_rstatus(struct brcmf_sdio *bus)
 	if (val) {
 		brcmf_sdiod_regwl(bus->sdiodev, addr, val, &ret);
 		bus->sdcnt.f1regdata++;
-	}
-
-	if (ret) {
-		atomic_set(&bus->intstatus, 0);
-	} else if (val) {
-		for_each_set_bit(n, &val, 32)
-			set_bit(n, (unsigned long *)&bus->intstatus.counter);
+		atomic_orr(val, &bus->intstatus);
 	}
 
 	return ret;
@@ -2484,7 +2487,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 	unsigned long intstatus;
 	uint txlimit = bus->txbound;	/* Tx frames to send before resched */
 	uint framecnt;			/* Temporary counter of tx/rx frames */
-	int err = 0, n;
+	int err = 0;
 
 	brcmf_dbg(TRACE, "Enter\n");
 
@@ -2588,10 +2591,8 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 	}
 
 	/* Keep still-pending events for next scheduling */
-	if (intstatus) {
-		for_each_set_bit(n, &intstatus, 32)
-			set_bit(n, (unsigned long *)&bus->intstatus.counter);
-	}
+	if (intstatus)
+		atomic_orr(intstatus, &bus->intstatus);
 
 	brcmf_sdio_clrintr(bus);
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 11/14] brcmfmac: Put frame sdio tx error handling in sub function.
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
                   ` (9 preceding siblings ...)
  2014-02-25 19:30 ` [PATCH 10/14] brcmfmac: Use atomic functions for intstatus update Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 19:30 ` [PATCH 12/14] brcmfmac: Correct mcs index report Arend van Spriel
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Hante Meuleman, Arend van Spriel

From: Hante Meuleman <meuleman@broadcom.com>

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |  112 ++++++--------------
 1 file changed, 33 insertions(+), 79 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index 90ff406..8871d94 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
@@ -1237,6 +1237,28 @@ static void brcmf_sdio_rxfail(struct brcmf_sdio *bus, bool abort, bool rtx)
 	bus->cur_read.len = 0;
 }
 
+static void brcmf_sdio_txfail(struct brcmf_sdio *bus)
+{
+	struct brcmf_sdio_dev *sdiodev = bus->sdiodev;
+	u8 i, hi, lo;
+
+	/* On failure, abort the command and terminate the frame */
+	brcmf_err("sdio error, abort command and terminate frame\n");
+	bus->sdcnt.tx_sderrs++;
+
+	brcmf_sdiod_abort(sdiodev, SDIO_FUNC_2);
+	brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_FRAMECTRL, SFC_WF_TERM, NULL);
+	bus->sdcnt.f1regdata++;
+
+	for (i = 0; i < 3; i++) {
+		hi = brcmf_sdiod_regrb(sdiodev, SBSDIO_FUNC1_WFRAMEBCHI, NULL);
+		lo = brcmf_sdiod_regrb(sdiodev, SBSDIO_FUNC1_WFRAMEBCLO, NULL);
+		bus->sdcnt.f1regdata += 2;
+		if ((hi == 0) && (lo == 0))
+			break;
+	}
+}
+
 /* return total length of buffer chain */
 static uint brcmf_sdio_glom_len(struct brcmf_sdio *bus)
 {
@@ -2252,7 +2274,6 @@ static int brcmf_sdio_txpkt(struct brcmf_sdio *bus, struct sk_buff_head *pktq,
 			    uint chan)
 {
 	int ret;
-	int i;
 	struct sk_buff *pkt_next, *tmp;
 
 	brcmf_dbg(TRACE, "Enter\n");
@@ -2265,28 +2286,9 @@ static int brcmf_sdio_txpkt(struct brcmf_sdio *bus, struct sk_buff_head *pktq,
 	ret = brcmf_sdiod_send_pkt(bus->sdiodev, pktq);
 	bus->sdcnt.f2txdata++;
 
-	if (ret < 0) {
-		/* On failure, abort the command and terminate the frame */
-		brcmf_dbg(INFO, "sdio error %d, abort command and terminate frame\n",
-			  ret);
-		bus->sdcnt.tx_sderrs++;
+	if (ret < 0)
+		brcmf_sdio_txfail(bus);
 
-		brcmf_sdiod_abort(bus->sdiodev, SDIO_FUNC_2);
-		brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_FUNC1_FRAMECTRL,
-				  SFC_WF_TERM, NULL);
-		bus->sdcnt.f1regdata++;
-
-		for (i = 0; i < 3; i++) {
-			u8 hi, lo;
-			hi = brcmf_sdiod_regrb(bus->sdiodev,
-					       SBSDIO_FUNC1_WFRAMEBCHI, NULL);
-			lo = brcmf_sdiod_regrb(bus->sdiodev,
-					       SBSDIO_FUNC1_WFRAMEBCLO, NULL);
-			bus->sdcnt.f1regdata += 2;
-			if ((hi == 0) && (lo == 0))
-				break;
-		}
-	}
 	sdio_release_host(bus->sdiodev->func[1]);
 
 done:
@@ -2597,42 +2599,17 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 	brcmf_sdio_clrintr(bus);
 
 	if (data_ok(bus) && bus->ctrl_frame_stat &&
-		(bus->clkstate == CLK_AVAIL)) {
-		int i;
+	    (bus->clkstate == CLK_AVAIL)) {
 
 		sdio_claim_host(bus->sdiodev->func[1]);
 		err = brcmf_sdiod_send_buf(bus->sdiodev, bus->ctrl_frame_buf,
 					   (u32)bus->ctrl_frame_len);
 
-		if (err < 0) {
-			/* On failure, abort the command and
-				terminate the frame */
-			brcmf_dbg(INFO, "sdio error %d, abort command and terminate frame\n",
-				  err);
-			bus->sdcnt.tx_sderrs++;
-
-			brcmf_sdiod_abort(bus->sdiodev, SDIO_FUNC_2);
-
-			brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_FUNC1_FRAMECTRL,
-					  SFC_WF_TERM, &err);
-			bus->sdcnt.f1regdata++;
-
-			for (i = 0; i < 3; i++) {
-				u8 hi, lo;
-				hi = brcmf_sdiod_regrb(bus->sdiodev,
-						       SBSDIO_FUNC1_WFRAMEBCHI,
-						       &err);
-				lo = brcmf_sdiod_regrb(bus->sdiodev,
-						       SBSDIO_FUNC1_WFRAMEBCLO,
-						       &err);
-				bus->sdcnt.f1regdata += 2;
-				if ((hi == 0) && (lo == 0))
-					break;
-			}
-
-		} else {
+		if (err < 0)
+			brcmf_sdio_txfail(bus);
+		else
 			bus->tx_seq = (bus->tx_seq + 1) % SDPCM_SEQ_WRAP;
-		}
+
 		sdio_release_host(bus->sdiodev->func[1]);
 		bus->ctrl_frame_stat = false;
 		brcmf_sdio_wait_event_wakeup(bus);
@@ -2802,38 +2779,15 @@ break2:
 
 static int brcmf_sdio_tx_frame(struct brcmf_sdio *bus, u8 *frame, u16 len)
 {
-	int i;
 	int ret;
 
 	bus->ctrl_frame_stat = false;
 	ret = brcmf_sdiod_send_buf(bus->sdiodev, frame, len);
 
-	if (ret < 0) {
-		/* On failure, abort the command and terminate the frame */
-		brcmf_dbg(INFO, "sdio error %d, abort command and terminate frame\n",
-			  ret);
-		bus->sdcnt.tx_sderrs++;
-
-		brcmf_sdiod_abort(bus->sdiodev, SDIO_FUNC_2);
-
-		brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_FUNC1_FRAMECTRL,
-				  SFC_WF_TERM, NULL);
-		bus->sdcnt.f1regdata++;
-
-		for (i = 0; i < 3; i++) {
-			u8 hi, lo;
-			hi = brcmf_sdiod_regrb(bus->sdiodev,
-					       SBSDIO_FUNC1_WFRAMEBCHI, NULL);
-			lo = brcmf_sdiod_regrb(bus->sdiodev,
-					       SBSDIO_FUNC1_WFRAMEBCLO, NULL);
-			bus->sdcnt.f1regdata += 2;
-			if (hi == 0 && lo == 0)
-				break;
-		}
-		return ret;
-	}
-
-	bus->tx_seq = (bus->tx_seq + 1) % SDPCM_SEQ_WRAP;
+	if (ret < 0)
+		brcmf_sdio_txfail(bus);
+	else
+		bus->tx_seq = (bus->tx_seq + 1) % SDPCM_SEQ_WRAP;
 
 	return ret;
 }
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 12/14] brcmfmac: Correct mcs index report
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
                   ` (10 preceding siblings ...)
  2014-02-25 19:30 ` [PATCH 11/14] brcmfmac: Put frame sdio tx error handling in sub function Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 19:30 ` [PATCH 13/14] brcmfmac: use pre-allocated scatter-gather table for txglomming Arend van Spriel
  2014-02-25 19:30 ` [PATCH 14/14] brcmfmac: reset suspend flag upon sdio suspend failure Arend van Spriel
  13 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Daniel Kim, Arend van Spriel

From: Daniel Kim <dekim@broadcom.com>

There is a mismatch between the mcs index(0-7) reported to cfg80211
and the actual mcs index(0-15) in use. This patch resolves the mismatch
by setting mcs info with the number of chains read from FW.

Reviewed-by: Arend Van Spriel <arend@broadcom.com>
Signed-off-by: Daniel Kim <dekim@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c |   17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index 81be69d..da7d055 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -5306,6 +5306,8 @@ static s32 brcmf_update_wiphybands(struct brcmf_cfg80211_info *cfg)
 	u32 band_list[3];
 	u32 nmode;
 	u32 bw_cap[2] = { 0, 0 };
+	u32 rxchain;
+	u32 nchain;
 	s8 phy;
 	s32 err;
 	u32 nband;
@@ -5342,6 +5344,16 @@ static s32 brcmf_update_wiphybands(struct brcmf_cfg80211_info *cfg)
 	brcmf_dbg(INFO, "nmode=%d, bw_cap=(%d, %d)\n", nmode,
 		  bw_cap[IEEE80211_BAND_2GHZ], bw_cap[IEEE80211_BAND_5GHZ]);
 
+	err = brcmf_fil_iovar_int_get(ifp, "rxchain", &rxchain);
+	if (err) {
+		brcmf_err("rxchain error (%d)\n", err);
+		nchain = 1;
+	} else {
+		for (nchain = 0; rxchain; nchain++)
+			rxchain = rxchain & (rxchain - 1);
+	}
+	brcmf_dbg(INFO, "nchain=%d\n", nchain);
+
 	err = brcmf_construct_reginfo(cfg, bw_cap);
 	if (err) {
 		brcmf_err("brcmf_construct_reginfo failed (%d)\n", err);
@@ -5370,10 +5382,7 @@ static s32 brcmf_update_wiphybands(struct brcmf_cfg80211_info *cfg)
 		band->ht_cap.ht_supported = true;
 		band->ht_cap.ampdu_factor = IEEE80211_HT_MAX_AMPDU_64K;
 		band->ht_cap.ampdu_density = IEEE80211_HT_MPDU_DENSITY_16;
-		/* An HT shall support all EQM rates for one spatial
-		 * stream
-		 */
-		band->ht_cap.mcs.rx_mask[0] = 0xff;
+		memset(band->ht_cap.mcs.rx_mask, 0xff, nchain);
 		band->ht_cap.mcs.tx_params = IEEE80211_HT_MCS_TX_DEFINED;
 		bands[band->band] = band;
 	}
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 13/14] brcmfmac: use pre-allocated scatter-gather table for txglomming
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
                   ` (11 preceding siblings ...)
  2014-02-25 19:30 ` [PATCH 12/14] brcmfmac: Correct mcs index report Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  2014-02-25 19:30 ` [PATCH 14/14] brcmfmac: reset suspend flag upon sdio suspend failure Arend van Spriel
  13 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Arend van Spriel

Instead of allocating a scatter-gather table for every transmit
reuse a pre-allocated table. The transmit path will be faster by
taking out this allocation.

Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Reviewed-by: Hante Meuleman <meuleman@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c   |   45 ++++++++++++++++----
 drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |    8 +---
 .../net/wireless/brcm80211/brcmfmac/sdio_host.h    |    2 +
 3 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 07e7d25..3866209 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -53,6 +53,12 @@
 /* Maximum milliseconds to wait for F2 to come up */
 #define SDIO_WAIT_F2RDY	3000
 
+#define BRCMF_DEFAULT_TXGLOM_SIZE	32  /* max tx frames in glom chain */
+#define BRCMF_DEFAULT_RXGLOM_SIZE	32  /* max rx frames in glom chain */
+
+static int brcmf_sdiod_txglomsz = BRCMF_DEFAULT_TXGLOM_SIZE;
+module_param_named(txglomsz, brcmf_sdiod_txglomsz, int, 0);
+MODULE_PARM_DESC(txglomsz, "maximum tx packet chain size [SDIO]");
 
 static irqreturn_t brcmf_sdiod_oob_irqhandler(int irq, void *dev_id)
 {
@@ -487,7 +493,6 @@ static int brcmf_sdiod_sglist_rw(struct brcmf_sdio_dev *sdiodev, uint fn,
 	struct mmc_request mmc_req;
 	struct mmc_command mmc_cmd;
 	struct mmc_data mmc_dat;
-	struct sg_table st;
 	struct scatterlist *sgl;
 	int ret = 0;
 
@@ -532,16 +537,11 @@ static int brcmf_sdiod_sglist_rw(struct brcmf_sdio_dev *sdiodev, uint fn,
 	pkt_offset = 0;
 	pkt_next = target_list->next;
 
-	if (sg_alloc_table(&st, max_seg_cnt, GFP_KERNEL)) {
-		ret = -ENOMEM;
-		goto exit;
-	}
-
 	memset(&mmc_req, 0, sizeof(struct mmc_request));
 	memset(&mmc_cmd, 0, sizeof(struct mmc_command));
 	memset(&mmc_dat, 0, sizeof(struct mmc_data));
 
-	mmc_dat.sg = st.sgl;
+	mmc_dat.sg = sdiodev->sgtable.sgl;
 	mmc_dat.blksz = func_blk_sz;
 	mmc_dat.flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
 	mmc_cmd.opcode = SD_IO_RW_EXTENDED;
@@ -557,7 +557,7 @@ static int brcmf_sdiod_sglist_rw(struct brcmf_sdio_dev *sdiodev, uint fn,
 	while (seg_sz) {
 		req_sz = 0;
 		sg_cnt = 0;
-		sgl = st.sgl;
+		sgl = sdiodev->sgtable.sgl;
 		/* prep sg table */
 		while (pkt_next != (struct sk_buff *)target_list) {
 			pkt_data = pkt_next->data + pkt_offset;
@@ -639,7 +639,7 @@ static int brcmf_sdiod_sglist_rw(struct brcmf_sdio_dev *sdiodev, uint fn,
 	}
 
 exit:
-	sg_free_table(&st);
+	sg_init_table(sdiodev->sgtable.sgl, sdiodev->sgtable.orig_nents);
 	while ((pkt_next = __skb_dequeue(&local_list)) != NULL)
 		brcmu_pkt_buf_free_skb(pkt_next);
 
@@ -863,6 +863,27 @@ int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, uint fn)
 	return 0;
 }
 
+static void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev)
+{
+	uint nents;
+	int err;
+
+	if (!sdiodev->sg_support)
+		return;
+
+	nents = max_t(uint, BRCMF_DEFAULT_RXGLOM_SIZE, brcmf_sdiod_txglomsz);
+	nents += (nents >> 4) + 1;
+
+	WARN_ON(nents > sdiodev->max_segment_count);
+
+	brcmf_dbg(TRACE, "nents=%d\n", nents);
+	err = sg_alloc_table(&sdiodev->sgtable, nents, GFP_KERNEL);
+	if (err < 0) {
+		brcmf_err("allocation failed: disable scatter-gather");
+		sdiodev->sg_support = false;
+	}
+}
+
 static int brcmf_sdiod_remove(struct brcmf_sdio_dev *sdiodev)
 {
 	if (sdiodev->bus) {
@@ -880,6 +901,7 @@ static int brcmf_sdiod_remove(struct brcmf_sdio_dev *sdiodev)
 	sdio_disable_func(sdiodev->func[1]);
 	sdio_release_host(sdiodev->func[1]);
 
+	sg_free_table(&sdiodev->sgtable);
 	sdiodev->sbwad = 0;
 
 	return 0;
@@ -935,6 +957,11 @@ static int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
 					   SG_MAX_SINGLE_ALLOC);
 	sdiodev->max_segment_size = host->max_seg_size;
 
+	/* allocate scatter-gather table. sg support
+	 * will be disabled upon allocation failure.
+	 */
+	brcmf_sdiod_sgtable_alloc(sdiodev);
+
 	/* try to attach to the target device */
 	sdiodev->bus = brcmf_sdio_probe(sdiodev);
 	if (!sdiodev->bus) {
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index 8871d94..3b8f46e 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
@@ -113,8 +113,6 @@ struct rte_console {
 #define BRCMF_TXBOUND	20	/* Default for max tx frames in
 				 one scheduling */
 
-#define BRCMF_DEFAULT_TXGLOM_SIZE	32  /* max tx frames in glom chain */
-
 #define BRCMF_TXMINMAX	1	/* Max tx frames if rx still pending */
 
 #define MEMBLOCK	2048	/* Block size used for downloading
@@ -511,10 +509,6 @@ static const uint max_roundup = 512;
 
 #define ALIGNMENT  4
 
-static int brcmf_sdio_txglomsz = BRCMF_DEFAULT_TXGLOM_SIZE;
-module_param_named(txglomsz, brcmf_sdio_txglomsz, int, 0);
-MODULE_PARM_DESC(txglomsz, "maximum tx packet chain size [SDIO]");
-
 enum brcmf_sdio_frmtype {
 	BRCMF_SDIO_FT_NORMAL,
 	BRCMF_SDIO_FT_SUPER,
@@ -2321,7 +2315,7 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes)
 		__skb_queue_head_init(&pktq);
 		if (bus->txglom)
 			pkt_num = min_t(u8, bus->tx_max - bus->tx_seq,
-					brcmf_sdio_txglomsz);
+					bus->sdiodev->txglomsz);
 		pkt_num = min_t(u32, pkt_num,
 				brcmu_pktq_mlen(&bus->txq, ~bus->flowcontrol));
 		spin_lock_bh(&bus->txqlock);
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/sdio_host.h b/drivers/net/wireless/brcm80211/brcmfmac/sdio_host.h
index 5e53eb1..3deab79 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/sdio_host.h
+++ b/drivers/net/wireless/brcm80211/brcmfmac/sdio_host.h
@@ -180,6 +180,8 @@ struct brcmf_sdio_dev {
 	uint max_request_size;
 	ushort max_segment_count;
 	uint max_segment_size;
+	uint txglomsz;
+	struct sg_table sgtable;
 };
 
 /* sdio core registers */
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 14/14] brcmfmac: reset suspend flag upon sdio suspend failure
  2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
                   ` (12 preceding siblings ...)
  2014-02-25 19:30 ` [PATCH 13/14] brcmfmac: use pre-allocated scatter-gather table for txglomming Arend van Spriel
@ 2014-02-25 19:30 ` Arend van Spriel
  13 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-25 19:30 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Arend van Spriel

The suspend callback first sets the suspend flag used in the driver
but after that the actual suspend is done, which may fail. Reset the
flag upon suspend failure.

Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 3866209..2471e55 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -1099,9 +1099,7 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
 	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
 	int ret = 0;
 
-	brcmf_dbg(SDIO, "\n");
-
-	atomic_set(&sdiodev->suspend, true);
+	brcmf_dbg(SDIO, "Enter\n");
 
 	sdio_flags = sdio_get_host_pm_caps(sdiodev->func[1]);
 	if (!(sdio_flags & MMC_PM_KEEP_POWER)) {
@@ -1109,9 +1107,12 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
 		return -EINVAL;
 	}
 
+	atomic_set(&sdiodev->suspend, true);
+
 	ret = sdio_set_host_pm_flags(sdiodev->func[1], MMC_PM_KEEP_POWER);
 	if (ret) {
 		brcmf_err("Failed to set pm_flags\n");
+		atomic_set(&sdiodev->suspend, false);
 		return ret;
 	}
 
@@ -1125,6 +1126,7 @@ static int brcmf_ops_sdio_resume(struct device *dev)
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
 	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
 
+	brcmf_dbg(SDIO, "Enter\n");
 	brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);
 	atomic_set(&sdiodev->suspend, false);
 	return 0;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-25 19:30 ` [PATCH 01/14] brcmfmac: add delay before unregistering the network device Arend van Spriel
@ 2014-02-25 19:59   ` Johannes Berg
  2014-02-26  9:07     ` Arend van Spriel
  0 siblings, 1 reply; 29+ messages in thread
From: Johannes Berg @ 2014-02-25 19:59 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: John W. Linville, linux-wireless

On Tue, 2014-02-25 at 20:30 +0100, Arend van Spriel wrote:
> Upon deleting the interface a cfg80211_disconnected() is called under
> rtnl_lock. Right after the unlocking the rtnl_lock we unregister the
> network device. This patch adds delay before unregister so cfg80211
> can handle disconnect and notify wpa_supplicant.

> +				/* make sure cfg80211 can send disconnect event
> +				 * before unregistering the netdevice below.
> +				 */
> +				msleep(100);

This has got to be one of the worst hacks I've seen in wireless so
far ... :)

johannes


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 10/14] brcmfmac: Use atomic functions for intstatus update.
  2014-02-25 19:30 ` [PATCH 10/14] brcmfmac: Use atomic functions for intstatus update Arend van Spriel
@ 2014-02-25 23:38   ` Florian Fainelli
  2014-02-26 12:20     ` Arend van Spriel
  0 siblings, 1 reply; 29+ messages in thread
From: Florian Fainelli @ 2014-02-25 23:38 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: John W. Linville, linux-wireless, Hante Meuleman

Hi Arend,

2014-02-25 11:30 GMT-08:00 Arend van Spriel <arend@broadcom.com>:
> From: Hante Meuleman <meuleman@broadcom.com>
>
> The intstatus in sdio code can be updated from different
> threads. To protect intstatus access, atomic functions are
> used. One of them is set_bit, but this function is not
> guaranteed atomic on all platforms. The loop was replaced
> with local created OR function.
>
> Reviewed-by: Arend Van Spriel <arend@broadcom.com>
> Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
> Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
> Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
> Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
> Signed-off-by: Arend van Spriel <arend@broadcom.com>
> ---
>  drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |   29 ++++++++++----------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
> index ac61419..90ff406 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
> @@ -2444,12 +2444,21 @@ static inline void brcmf_sdio_clrintr(struct brcmf_sdio *bus)
>         }
>  }
>
> +static void atomic_orr(int val, atomic_t *v)
> +{
> +       int old_val;
> +
> +       old_val = atomic_read(v);
> +       while (atomic_cmpxchg(v, old_val, val | old_val) != old_val)
> +               old_val = atomic_read(v);
> +}

Is not atomic_set_mask() doing the same thing?

> +
>  static int brcmf_sdio_intr_rstatus(struct brcmf_sdio *bus)
>  {
>         struct brcmf_core *buscore;
>         u32 addr;
>         unsigned long val;
> -       int n, ret;
> +       int ret;
>
>         buscore = brcmf_chip_get_core(bus->ci, BCMA_CORE_SDIO_DEV);
>         addr = buscore->base + offsetof(struct sdpcmd_regs, intstatus);
> @@ -2457,7 +2466,7 @@ static int brcmf_sdio_intr_rstatus(struct brcmf_sdio *bus)
>         val = brcmf_sdiod_regrl(bus->sdiodev, addr, &ret);
>         bus->sdcnt.f1regdata++;
>         if (ret != 0)
> -               val = 0;
> +               return ret;
>
>         val &= bus->hostintmask;
>         atomic_set(&bus->fcstate, !!(val & I_HMB_FC_STATE));
> @@ -2466,13 +2475,7 @@ static int brcmf_sdio_intr_rstatus(struct brcmf_sdio *bus)
>         if (val) {
>                 brcmf_sdiod_regwl(bus->sdiodev, addr, val, &ret);
>                 bus->sdcnt.f1regdata++;
> -       }
> -
> -       if (ret) {
> -               atomic_set(&bus->intstatus, 0);
> -       } else if (val) {
> -               for_each_set_bit(n, &val, 32)
> -                       set_bit(n, (unsigned long *)&bus->intstatus.counter);
> +               atomic_orr(val, &bus->intstatus);
>         }
>
>         return ret;
> @@ -2484,7 +2487,7 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
>         unsigned long intstatus;
>         uint txlimit = bus->txbound;    /* Tx frames to send before resched */
>         uint framecnt;                  /* Temporary counter of tx/rx frames */
> -       int err = 0, n;
> +       int err = 0;
>
>         brcmf_dbg(TRACE, "Enter\n");
>
> @@ -2588,10 +2591,8 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
>         }
>
>         /* Keep still-pending events for next scheduling */
> -       if (intstatus) {
> -               for_each_set_bit(n, &intstatus, 32)
> -                       set_bit(n, (unsigned long *)&bus->intstatus.counter);
> -       }
> +       if (intstatus)
> +               atomic_orr(intstatus, &bus->intstatus);
>
>         brcmf_sdio_clrintr(bus);
>
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Florian

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-25 19:59   ` Johannes Berg
@ 2014-02-26  9:07     ` Arend van Spriel
  2014-02-26  9:17       ` Johannes Berg
  0 siblings, 1 reply; 29+ messages in thread
From: Arend van Spriel @ 2014-02-26  9:07 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John W. Linville, linux-wireless

On 02/25/2014 08:59 PM, Johannes Berg wrote:
> On Tue, 2014-02-25 at 20:30 +0100, Arend van Spriel wrote:
>> Upon deleting the interface a cfg80211_disconnected() is called under
>> rtnl_lock. Right after the unlocking the rtnl_lock we unregister the
>> network device. This patch adds delay before unregister so cfg80211
>> can handle disconnect and notify wpa_supplicant.
> 
>> +				/* make sure cfg80211 can send disconnect event
>> +				 * before unregistering the netdevice below.
>> +				 */
>> +				msleep(100);
> 
> This has got to be one of the worst hacks I've seen in wireless so
> far ... :)

Did you see I removed a sleep as well in this patch :-p

I just don't see how I can assure cfg80211 has actually done the
disconnect work. If we don't do a cfg80211_disconnected() I get a WARN
from the cfg80211 netdev notifier (or at least I did in previous kernel).

Should we consider a clean solution, ie. modify cfg80211 for this scenario?

Gr. AvS



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-26  9:07     ` Arend van Spriel
@ 2014-02-26  9:17       ` Johannes Berg
  2014-02-26  9:28         ` Arend van Spriel
  0 siblings, 1 reply; 29+ messages in thread
From: Johannes Berg @ 2014-02-26  9:17 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: John W. Linville, linux-wireless

On Wed, 2014-02-26 at 10:07 +0100, Arend van Spriel wrote:
> On 02/25/2014 08:59 PM, Johannes Berg wrote:
> > On Tue, 2014-02-25 at 20:30 +0100, Arend van Spriel wrote:
> >> Upon deleting the interface a cfg80211_disconnected() is called under
> >> rtnl_lock. Right after the unlocking the rtnl_lock we unregister the
> >> network device. This patch adds delay before unregister so cfg80211
> >> can handle disconnect and notify wpa_supplicant.
> > 
> >> +				/* make sure cfg80211 can send disconnect event
> >> +				 * before unregistering the netdevice below.
> >> +				 */
> >> +				msleep(100);
> > 
> > This has got to be one of the worst hacks I've seen in wireless so
> > far ... :)
> 
> Did you see I removed a sleep as well in this patch :-p

Yeah, I did :-)

> I just don't see how I can assure cfg80211 has actually done the
> disconnect work. If we don't do a cfg80211_disconnected() I get a WARN
> from the cfg80211 netdev notifier (or at least I did in previous kernel).
> 
> Should we consider a clean solution, ie. modify cfg80211 for this scenario?

Yes. Can't we just flush the work at some strategic place?

Actually you're not talking about the "disconnect_work" (which is
related to regulatory) but the "event_work" so I was confused here for a
second.

What was the warning? cfg80211 already calls
cfg80211_process_wdev_events() from within the REMOVE netdev notifier,
so that *shouldn't* have happened.

johannes


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-26  9:17       ` Johannes Berg
@ 2014-02-26  9:28         ` Arend van Spriel
  2014-02-26  9:37           ` Johannes Berg
  0 siblings, 1 reply; 29+ messages in thread
From: Arend van Spriel @ 2014-02-26  9:28 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John W. Linville, linux-wireless

On 02/26/2014 10:17 AM, Johannes Berg wrote:
> On Wed, 2014-02-26 at 10:07 +0100, Arend van Spriel wrote:
>> On 02/25/2014 08:59 PM, Johannes Berg wrote:
>>> On Tue, 2014-02-25 at 20:30 +0100, Arend van Spriel wrote:
>>>> Upon deleting the interface a cfg80211_disconnected() is called under
>>>> rtnl_lock. Right after the unlocking the rtnl_lock we unregister the
>>>> network device. This patch adds delay before unregister so cfg80211
>>>> can handle disconnect and notify wpa_supplicant.
>>>
>>>> +				/* make sure cfg80211 can send disconnect event
>>>> +				 * before unregistering the netdevice below.
>>>> +				 */
>>>> +				msleep(100);
>>>
>>> This has got to be one of the worst hacks I've seen in wireless so
>>> far ... :)
>>
>> Did you see I removed a sleep as well in this patch :-p
> 
> Yeah, I did :-)
> 
>> I just don't see how I can assure cfg80211 has actually done the
>> disconnect work. If we don't do a cfg80211_disconnected() I get a WARN
>> from the cfg80211 netdev notifier (or at least I did in previous kernel).
>>
>> Should we consider a clean solution, ie. modify cfg80211 for this scenario?
> 
> Yes. Can't we just flush the work at some strategic place?
> 
> Actually you're not talking about the "disconnect_work" (which is
> related to regulatory) but the "event_work" so I was confused here for a
> second.
> 
> What was the warning? cfg80211 already calls
> cfg80211_process_wdev_events() from within the REMOVE netdev notifier,
> so that *shouldn't* have happened.

I guess that means some wdev event is missing? It was the
WARN_ON(current->bss) that fired.

Gr. AvS

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-26  9:28         ` Arend van Spriel
@ 2014-02-26  9:37           ` Johannes Berg
  2014-02-26 10:43             ` Arend van Spriel
  0 siblings, 1 reply; 29+ messages in thread
From: Johannes Berg @ 2014-02-26  9:37 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: John W. Linville, linux-wireless

On Wed, 2014-02-26 at 10:28 +0100, Arend van Spriel wrote:

> > Yes. Can't we just flush the work at some strategic place?
> > 
> > Actually you're not talking about the "disconnect_work" (which is
> > related to regulatory) but the "event_work" so I was confused here for a
> > second.
> > 
> > What was the warning? cfg80211 already calls
> > cfg80211_process_wdev_events() from within the REMOVE netdev notifier,
> > so that *shouldn't* have happened.
> 
> I guess that means some wdev event is missing? It was the
> WARN_ON(current->bss) that fired.

Hmm, that's odd, the process_wdev_events() call is right before that, so
I don't see how that could have happened?

johannes


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-26  9:37           ` Johannes Berg
@ 2014-02-26 10:43             ` Arend van Spriel
  2014-02-26 11:10               ` Johannes Berg
  0 siblings, 1 reply; 29+ messages in thread
From: Arend van Spriel @ 2014-02-26 10:43 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John W. Linville, linux-wireless

On 02/26/2014 10:37 AM, Johannes Berg wrote:
> On Wed, 2014-02-26 at 10:28 +0100, Arend van Spriel wrote:
> 
>>> Yes. Can't we just flush the work at some strategic place?
>>>
>>> Actually you're not talking about the "disconnect_work" (which is
>>> related to regulatory) but the "event_work" so I was confused here for a
>>> second.
>>>
>>> What was the warning? cfg80211 already calls
>>> cfg80211_process_wdev_events() from within the REMOVE netdev notifier,
>>> so that *shouldn't* have happened.
>>
>> I guess that means some wdev event is missing? It was the
>> WARN_ON(current->bss) that fired.
> 
> Hmm, that's odd, the process_wdev_events() call is right before that, so
> I don't see how that could have happened?

Ok. The warning probably happened in earlier kernel, because our module
exit was buggy. I fixed that a while ago and I just verified I don't see
it without this patch. Sorry for bringing that diversion up.

Actually, this was not the reason for the doing the msleep(). The
problem was that wpa_supplicant received RTM_DELLINK, before getting the
NL08211_DISCONNECT. This resulted in RTM_DELLINK, immediately followed
by RTM_ADDLINK, followed by NL80211_DISCONNECT.

Gr. AvS

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-26 10:43             ` Arend van Spriel
@ 2014-02-26 11:10               ` Johannes Berg
  2014-02-26 11:34                 ` Arend van Spriel
  0 siblings, 1 reply; 29+ messages in thread
From: Johannes Berg @ 2014-02-26 11:10 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: John W. Linville, linux-wireless

On Wed, 2014-02-26 at 11:43 +0100, Arend van Spriel wrote:

> Actually, this was not the reason for the doing the msleep(). The
> problem was that wpa_supplicant received RTM_DELLINK, before getting the
> NL08211_DISCONNECT. This resulted in RTM_DELLINK, immediately followed
> by RTM_ADDLINK, followed by NL80211_DISCONNECT.

I'm not sure this isn't still possible? I don't know when DELLINK is
sent.

johannes


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-26 11:10               ` Johannes Berg
@ 2014-02-26 11:34                 ` Arend van Spriel
  2014-02-26 11:48                   ` Johannes Berg
  0 siblings, 1 reply; 29+ messages in thread
From: Arend van Spriel @ 2014-02-26 11:34 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John W. Linville, linux-wireless, David Miller, netdev

On 02/26/2014 12:10 PM, Johannes Berg wrote:
> On Wed, 2014-02-26 at 11:43 +0100, Arend van Spriel wrote:
> 
>> Actually, this was not the reason for the doing the msleep(). The
>> problem was that wpa_supplicant received RTM_DELLINK, before getting the
>> NL08211_DISCONNECT. This resulted in RTM_DELLINK, immediately followed
>> by RTM_ADDLINK, followed by NL80211_DISCONNECT.
> 
> I'm not sure this isn't still possible? I don't know when DELLINK is
> sent.

Given the behaviour I would say before the netdev notifier, which may be
considered wrong. It depends whether the notifier was intended for the
kind of thing that cfg80211 is doing, ie. send a netlink event to
user-space?

Gr. AvS

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-26 11:34                 ` Arend van Spriel
@ 2014-02-26 11:48                   ` Johannes Berg
  2014-02-26 12:18                     ` Arend van Spriel
  0 siblings, 1 reply; 29+ messages in thread
From: Johannes Berg @ 2014-02-26 11:48 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: John W. Linville, linux-wireless, David Miller, netdev

On Wed, 2014-02-26 at 12:34 +0100, Arend van Spriel wrote:

> > I'm not sure this isn't still possible? I don't know when DELLINK is
> > sent.
> 
> Given the behaviour I would say before the netdev notifier, which may be
> considered wrong. It depends whether the notifier was intended for the
> kind of thing that cfg80211 is doing, ie. send a netlink event to
> user-space?

Well, I checked:

                call_netdevice_notifiers(NETDEV_UNREGISTER, dev);

                if (!dev->rtnl_link_ops ||
                    dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
                        rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);

so ... :)

johannes


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-26 11:48                   ` Johannes Berg
@ 2014-02-26 12:18                     ` Arend van Spriel
  2014-02-26 12:22                       ` Johannes Berg
  0 siblings, 1 reply; 29+ messages in thread
From: Arend van Spriel @ 2014-02-26 12:18 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John W. Linville, linux-wireless, David Miller, netdev

On 02/26/2014 12:48 PM, Johannes Berg wrote:
> On Wed, 2014-02-26 at 12:34 +0100, Arend van Spriel wrote:
> 
>>> I'm not sure this isn't still possible? I don't know when DELLINK is
>>> sent.
>>
>> Given the behaviour I would say before the netdev notifier, which may be
>> considered wrong. It depends whether the notifier was intended for the
>> kind of thing that cfg80211 is doing, ie. send a netlink event to
>> user-space?
> 
> Well, I checked:
> 
>                 call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
> 
>                 if (!dev->rtnl_link_ops ||
>                     dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
>                         rtmsg_ifinfo(RTM_DELLINK, dev, ~0U, GFP_KERNEL);
> 
> so ... :)

Feeling sad :-( but thanks for checking. Clearly need to look into this
some more. John, can you please drop this one.

Gr. AvS

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 10/14] brcmfmac: Use atomic functions for intstatus update.
  2014-02-25 23:38   ` Florian Fainelli
@ 2014-02-26 12:20     ` Arend van Spriel
  0 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-26 12:20 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: John W. Linville, linux-wireless, Hante Meuleman

On 02/26/2014 12:38 AM, Florian Fainelli wrote:
> Hi Arend,
> 
> 2014-02-25 11:30 GMT-08:00 Arend van Spriel <arend@broadcom.com>:
>> From: Hante Meuleman <meuleman@broadcom.com>
>>
>> The intstatus in sdio code can be updated from different
>> threads. To protect intstatus access, atomic functions are
>> used. One of them is set_bit, but this function is not
>> guaranteed atomic on all platforms. The loop was replaced
>> with local created OR function.
>>
>> Reviewed-by: Arend Van Spriel <arend@broadcom.com>
>> Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
>> Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
>> Reviewed-by: Daniel (Deognyoun) Kim <dekim@broadcom.com>
>> Signed-off-by: Hante Meuleman <meuleman@broadcom.com>
>> Signed-off-by: Arend van Spriel <arend@broadcom.com>
>> ---
>>  drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c |   29 ++++++++++----------
>>  1 file changed, 15 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> index ac61419..90ff406 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
>> @@ -2444,12 +2444,21 @@ static inline void brcmf_sdio_clrintr(struct brcmf_sdio *bus)
>>         }
>>  }
>>
>> +static void atomic_orr(int val, atomic_t *v)
>> +{
>> +       int old_val;
>> +
>> +       old_val = atomic_read(v);
>> +       while (atomic_cmpxchg(v, old_val, val | old_val) != old_val)
>> +               old_val = atomic_read(v);
>> +}
> 
> Is not atomic_set_mask() doing the same thing?

You are right. Hante thought that one was not supported on all archs,
but asm-generic provides a fallback for those. I will revise this patch.

Thanks,
Arend


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-26 12:18                     ` Arend van Spriel
@ 2014-02-26 12:22                       ` Johannes Berg
  2014-02-26 12:35                         ` Arend van Spriel
  0 siblings, 1 reply; 29+ messages in thread
From: Johannes Berg @ 2014-02-26 12:22 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: John W. Linville, linux-wireless, David Miller, netdev

On Wed, 2014-02-26 at 13:18 +0100, Arend van Spriel wrote:

> Feeling sad :-( but thanks for checking. Clearly need to look into this
> some more. John, can you please drop this one.

I think you probably still want to drop the other sleep?

johannes


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/14] brcmfmac: add delay before unregistering the network device
  2014-02-26 12:22                       ` Johannes Berg
@ 2014-02-26 12:35                         ` Arend van Spriel
  0 siblings, 0 replies; 29+ messages in thread
From: Arend van Spriel @ 2014-02-26 12:35 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John W. Linville, linux-wireless, David Miller, netdev

On 02/26/2014 01:22 PM, Johannes Berg wrote:
> On Wed, 2014-02-26 at 13:18 +0100, Arend van Spriel wrote:
> 
>> Feeling sad :-( but thanks for checking. Clearly need to look into this
>> some more. John, can you please drop this one.
> 
> I think you probably still want to drop the other sleep?

Yeah. That one is pretty useless anyway given that it is under
rtnl_lock. Just want to check that it works when I remove the sleep
between cfg80211_disconnected() and unregister_netdev() as well.

Gr. AvS


^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2014-02-26 12:35 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-25 19:30 [PATCH 00/14] brcmfmac: driver cleanup and rework Arend van Spriel
2014-02-25 19:30 ` [PATCH 01/14] brcmfmac: add delay before unregistering the network device Arend van Spriel
2014-02-25 19:59   ` Johannes Berg
2014-02-26  9:07     ` Arend van Spriel
2014-02-26  9:17       ` Johannes Berg
2014-02-26  9:28         ` Arend van Spriel
2014-02-26  9:37           ` Johannes Berg
2014-02-26 10:43             ` Arend van Spriel
2014-02-26 11:10               ` Johannes Berg
2014-02-26 11:34                 ` Arend van Spriel
2014-02-26 11:48                   ` Johannes Berg
2014-02-26 12:18                     ` Arend van Spriel
2014-02-26 12:22                       ` Johannes Berg
2014-02-26 12:35                         ` Arend van Spriel
2014-02-25 19:30 ` [PATCH 02/14] brcmfmac: Make firmeware roaming a module param Arend van Spriel
2014-02-25 19:30 ` [PATCH 03/14] brcmfmac: fix use of skb control buffer in SDIO driver part Arend van Spriel
2014-02-25 19:30 ` [PATCH 04/14] brcmfmac: remove unused variable data_len from brcmf_sdio_bus_txdata() Arend van Spriel
2014-02-25 19:30 ` [PATCH 05/14] brcmfmac: Correct header debug dump for sdio tx hdrs Arend van Spriel
2014-02-25 19:30 ` [PATCH 06/14] brcmfmac: de-init driver layers in correct order Arend van Spriel
2014-02-25 19:30 ` [PATCH 07/14] brcmfmac: Minimize SDIO dpc scheduling Arend van Spriel
2014-02-25 19:30 ` [PATCH 08/14] brcmfmac: Remove immediate sleep support from SDIO Arend van Spriel
2014-02-25 19:30 ` [PATCH 09/14] brcmfmac: Small cleanup of redundant code Arend van Spriel
2014-02-25 19:30 ` [PATCH 10/14] brcmfmac: Use atomic functions for intstatus update Arend van Spriel
2014-02-25 23:38   ` Florian Fainelli
2014-02-26 12:20     ` Arend van Spriel
2014-02-25 19:30 ` [PATCH 11/14] brcmfmac: Put frame sdio tx error handling in sub function Arend van Spriel
2014-02-25 19:30 ` [PATCH 12/14] brcmfmac: Correct mcs index report Arend van Spriel
2014-02-25 19:30 ` [PATCH 13/14] brcmfmac: use pre-allocated scatter-gather table for txglomming Arend van Spriel
2014-02-25 19:30 ` [PATCH 14/14] brcmfmac: reset suspend flag upon sdio suspend failure Arend van Spriel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).