RFC: Broadcom fmac wireless driver cleanup

RFC: Broadcom fmac wireless driver cleanup

[Ian Molton]

Hi folks,

I've been doing some cleanups in the broadcome brcmfmac driver, trying to
understand how it works.

I think this makes the driver a *lot* more readable.

Of note:

* Gets rid of the arbitrary and completely 1:1 mapping of
 struct brcmf_{core,chip}_priv/struct brcmf_{core,chip} structures
 which add unreadability whilst offering no real seperation.

* The patch titled HACK - stabilise the value of ->sbwad in use
 highlights an issue that is either bizarre undocumented behaviour or a
 genuine bug - without datasheets, I dont know. Essentially the value of sbwad
 is taken as the base address in several functions, even though it is subject
 to change should other IO occur that moves the window offset

Obviously this is a first cut at this and needs substantial cleanup itself,
however I wanted to get some idea of wether I should continue working on this.

I only have a 43430 SDIO WiFi / BT chip to test on, but other chips *should*
be unaffected by these changes.

-Ian

[PATCH 01/21] net: brcmfmac: Write function depends on size of regs not types.

[Ian Molton]

The choice of read/write function depends on the size of the
device registers, not compipler types.


Signed-off-by: Ian Molton <ian@mnementh.co.uk>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 9b970dc2b922..fd398d27a30c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -266,7 +266,7 @@ static int brcmf_sdiod_request_data(struct brcmf_sdio_dev *sdiodev, u8 fn,
 	func = sdiodev->func[fn];
 
 	switch (regsz) {
-	case sizeof(u8):
+	case 1:
 		if (write) {
 			if (fn)
 				sdio_writeb(func, *(u8 *)data, addr, &ret);
@@ -280,13 +280,13 @@ static int brcmf_sdiod_request_data(struct brcmf_sdio_dev *sdiodev, u8 fn,
 				*(u8 *)data = sdio_f0_readb(func, addr, &ret);
 		}
 		break;
-	case sizeof(u16):
+	case 2:
 		if (write)
 			sdio_writew(func, *(u16 *)data, addr, &ret);
 		else
 			*(u16 *)data = sdio_readw(func, addr, &ret);
 		break;
-	case sizeof(u32):
+	case 4:
 		if (write)
 			sdio_writel(func, *(u32 *)data, addr, &ret);
 		else
-- 
2.11.0

[PATCH 02/21] net: brcmfmac: Fix brain damaged fn parameter order

[Ian Molton]

---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index fd398d27a30c..dadbba2f82ed 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -232,8 +232,7 @@ void brcmf_sdiod_change_state(struct brcmf_sdio_dev *sdiodev,
 	sdiodev->state = state;
 }
 
-static inline int brcmf_sdiod_f0_writeb(struct sdio_func *func,
-					uint regaddr, u8 byte)
+static inline int brcmf_sdiod_f0_writeb(struct sdio_func *func, u8 byte, uint regaddr)
 {
 	int err_ret;
 
@@ -271,8 +270,7 @@ static int brcmf_sdiod_request_data(struct brcmf_sdio_dev *sdiodev, u8 fn,
 			if (fn)
 				sdio_writeb(func, *(u8 *)data, addr, &ret);
 			else
-				ret = brcmf_sdiod_f0_writeb(func, addr,
-							    *(u8 *)data);
+				ret = brcmf_sdiod_f0_writeb(func, *(u8 *)data, addr);
 		} else {
 			if (fn)
 				*(u8 *)data = sdio_readb(func, addr, &ret);
-- 
2.11.0

[PATCH 03/21] bcmfmac: simplify brcmf_sdiod_abort

[Ian Molton]

---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index dadbba2f82ed..6ff4f10e36bb 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -872,12 +872,11 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 
 int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, uint fn)
 {
-	char t_func = (char)fn;
 	brcmf_dbg(SDIO, "Enter\n");
 
 	/* issue abort cmd52 command through F0 */
-	brcmf_sdiod_request_data(sdiodev, SDIO_FUNC_0, SDIO_CCCR_ABORT,
-				 sizeof(t_func), &t_func, true);
+	brcmf_sdiod_f0_writeb(sdiodev->func[0], (u8)fn, SDIO_CCCR_ABORT);
+
 
 	brcmf_dbg(SDIO, "Exit\n");
 	return 0;
-- 
2.11.0

[PATCH 04/21] brcmfmac: Do away with this abomination:

[Ian Molton]

brcmf_sdiod_regrw_helper()
---
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 105 +++++++++++++++------
 1 file changed, 77 insertions(+), 28 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 6ff4f10e36bb..1012df347c84 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -302,8 +302,8 @@ static int brcmf_sdiod_request_data(struct brcmf_sdio_dev *sdiodev, u8 fn,
 	return ret;
 }
 
-static int brcmf_sdiod_regrw_helper(struct brcmf_sdio_dev *sdiodev, u32 addr,
-				   u8 regsz, void *data, bool write)
+static int brcmf_sdiod_reg_write(struct brcmf_sdio_dev *sdiodev, u32 addr,
+				   u8 regsz, void *data)
 {
 	u8 func;
 	s32 retry = 0;
@@ -324,13 +324,66 @@ static int brcmf_sdiod_regrw_helper(struct brcmf_sdio_dev *sdiodev, u32 addr,
 		func = SDIO_FUNC_1;
 
 	do {
-		if (!write)
-			memset(data, 0, regsz);
 		/* for retry wait for 1 ms till bus get settled down */
 		if (retry)
 			usleep_range(1000, 2000);
+
+		ret = brcmf_sdiod_request_data(sdiodev, func, addr, regsz,
+					       data, true);
+
+	} while (ret != 0 && ret != -ENOMEDIUM &&
+		 retry++ < SDIOH_API_ACCESS_RETRY_LIMIT);
+
+	if (ret == -ENOMEDIUM)
+		brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_NOMEDIUM);
+	else if (ret != 0) {
+		/*
+		 * SleepCSR register access can fail when
+		 * waking up the device so reduce this noise
+		 * in the logs.
+		 */
+		if (addr != SBSDIO_FUNC1_SLEEPCSR)
+			brcmf_err("failed to write data F%d@0x%05x, err: %d\n",
+				  func, addr, ret);
+		else
+			brcmf_dbg(SDIO, "failed to write data F%d@0x%05x, err: %d\n",
+				  func, addr, ret);
+	}
+
+	return ret;
+}
+
+static int brcmf_sdiod_reg_read(struct brcmf_sdio_dev *sdiodev, u32 addr,
+				   u8 regsz, void *data)
+{
+	u8 func;
+	s32 retry = 0;
+	int ret;
+
+	if (sdiodev->state == BRCMF_SDIOD_NOMEDIUM)
+		return -ENOMEDIUM;
+
+	/*
+	 * figure out how to read the register based on address range
+	 * 0x00 ~ 0x7FF: function 0 CCCR and FBR
+	 * 0x10000 ~ 0x1FFFF: function 1 miscellaneous registers
+	 * The rest: function 1 silicon backplane core registers
+	 */
+	if ((addr & ~REG_F0_REG_MASK) == 0)
+		func = SDIO_FUNC_0;
+	else
+		func = SDIO_FUNC_1;
+
+	do {
+		memset(data, 0, regsz);
+
+		/* for retry wait for 1 ms till bus get settled down */
+		if (retry)
+			usleep_range(1000, 2000);
+
 		ret = brcmf_sdiod_request_data(sdiodev, func, addr, regsz,
-					       data, write);
+					       data, false);
+
 	} while (ret != 0 && ret != -ENOMEDIUM &&
 		 retry++ < SDIOH_API_ACCESS_RETRY_LIMIT);
 
@@ -343,12 +396,13 @@ static int brcmf_sdiod_regrw_helper(struct brcmf_sdio_dev *sdiodev, u32 addr,
 		 * in the logs.
 		 */
 		if (addr != SBSDIO_FUNC1_SLEEPCSR)
-			brcmf_err("failed to %s data F%d@0x%05x, err: %d\n",
-				  write ? "write" : "read", func, addr, ret);
+			brcmf_err("failed to read data F%d@0x%05x, err: %d\n",
+				  func, addr, ret);
 		else
-			brcmf_dbg(SDIO, "failed to %s data F%d@0x%05x, err: %d\n",
-				  write ? "write" : "read", func, addr, ret);
+			brcmf_dbg(SDIO, "failed to read data F%d@0x%05x, err: %d\n",
+				  func, addr, ret);
 	}
+
 	return ret;
 }
 
@@ -356,24 +410,20 @@ static int
 brcmf_sdiod_set_sbaddr_window(struct brcmf_sdio_dev *sdiodev, u32 address)
 {
 	int err = 0, i;
-	u8 addr[3];
+	u32 addr;
 
 	if (sdiodev->state == BRCMF_SDIOD_NOMEDIUM)
 		return -ENOMEDIUM;
 
-	addr[0] = (address >> 8) & SBSDIO_SBADDRLOW_MASK;
-	addr[1] = (address >> 16) & SBSDIO_SBADDRMID_MASK;
-	addr[2] = (address >> 24) & SBSDIO_SBADDRHIGH_MASK;
+	addr = (address & SBSDIO_SBWINDOW_MASK) >> 8;
 
 	for (i = 0; i < 3; i++) {
-		err = brcmf_sdiod_regrw_helper(sdiodev,
-					       SBSDIO_FUNC1_SBADDRLOW + i,
-					       sizeof(u8), &addr[i], true);
-		if (err) {
-			brcmf_err("failed at addr: 0x%0x\n",
-				  SBSDIO_FUNC1_SBADDRLOW + i);
+		brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_SBADDRLOW + i, addr & 0xff,
+					&err);
+		if (err)
 			break;
-		}
+
+		addr = addr >> 8;
 	}
 
 	return err;
@@ -407,8 +457,7 @@ u8 brcmf_sdiod_regrb(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
 	int retval;
 
 	brcmf_dbg(SDIO, "addr:0x%08x\n", addr);
-	retval = brcmf_sdiod_regrw_helper(sdiodev, addr, sizeof(data), &data,
-					  false);
+	retval = brcmf_sdiod_reg_read(sdiodev, addr, sizeof(data), &data);
 	brcmf_dbg(SDIO, "data:0x%02x\n", data);
 
 	if (ret)
@@ -426,8 +475,8 @@ u32 brcmf_sdiod_regrl(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
 	retval = brcmf_sdiod_addrprep(sdiodev, sizeof(data), &addr);
 	if (retval)
 		goto done;
-	retval = brcmf_sdiod_regrw_helper(sdiodev, addr, sizeof(data), &data,
-					  false);
+	retval = brcmf_sdiod_reg_read(sdiodev, addr, sizeof(data), &data);
+
 	brcmf_dbg(SDIO, "data:0x%08x\n", data);
 
 done:
@@ -443,8 +492,8 @@ void brcmf_sdiod_regwb(struct brcmf_sdio_dev *sdiodev, u32 addr,
 	int retval;
 
 	brcmf_dbg(SDIO, "addr:0x%08x, data:0x%02x\n", addr, data);
-	retval = brcmf_sdiod_regrw_helper(sdiodev, addr, sizeof(data), &data,
-					  true);
+	retval = brcmf_sdiod_reg_write(sdiodev, addr, sizeof(data), &data);
+
 	if (ret)
 		*ret = retval;
 }
@@ -458,8 +507,8 @@ void brcmf_sdiod_regwl(struct brcmf_sdio_dev *sdiodev, u32 addr,
 	retval = brcmf_sdiod_addrprep(sdiodev, sizeof(data), &addr);
 	if (retval)
 		goto done;
-	retval = brcmf_sdiod_regrw_helper(sdiodev, addr, sizeof(data), &data,
-					  true);
+
+	retval = brcmf_sdiod_reg_write(sdiodev, addr, sizeof(data), &data);
 
 done:
 	if (ret)
-- 
2.11.0

[PATCH 05/21] brcmfmac: its ALWAYS 4.

[Ian Molton]

---
 .../net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c   | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 1012df347c84..2b527d2a2f8a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -430,7 +430,7 @@ brcmf_sdiod_set_sbaddr_window(struct brcmf_sdio_dev *sdiodev, u32 address)
 }
 
 static int
-brcmf_sdiod_addrprep(struct brcmf_sdio_dev *sdiodev, uint width, u32 *addr)
+brcmf_sdiod_addrprep(struct brcmf_sdio_dev *sdiodev, u32 *addr)
 {
 	uint bar0 = *addr & ~SBSDIO_SB_OFT_ADDR_MASK;
 	int err = 0;
@@ -445,8 +445,7 @@ brcmf_sdiod_addrprep(struct brcmf_sdio_dev *sdiodev, uint width, u32 *addr)
 
 	*addr &= SBSDIO_SB_OFT_ADDR_MASK;
 
-	if (width == 4)
-		*addr |= SBSDIO_SB_ACCESS_2_4B_FLAG;
+	*addr |= SBSDIO_SB_ACCESS_2_4B_FLAG;
 
 	return 0;
 }
@@ -472,7 +471,7 @@ u32 brcmf_sdiod_regrl(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
 	int retval;
 
 	brcmf_dbg(SDIO, "addr:0x%08x\n", addr);
-	retval = brcmf_sdiod_addrprep(sdiodev, sizeof(data), &addr);
+	retval = brcmf_sdiod_addrprep(sdiodev, &addr);
 	if (retval)
 		goto done;
 	retval = brcmf_sdiod_reg_read(sdiodev, addr, sizeof(data), &data);
@@ -504,7 +503,7 @@ void brcmf_sdiod_regwl(struct brcmf_sdio_dev *sdiodev, u32 addr,
 	int retval;
 
 	brcmf_dbg(SDIO, "addr:0x%08x, data:0x%08x\n", addr, data);
-	retval = brcmf_sdiod_addrprep(sdiodev, sizeof(data), &addr);
+	retval = brcmf_sdiod_addrprep(sdiodev, &addr);
 	if (retval)
 		goto done;
 
@@ -740,7 +739,7 @@ int brcmf_sdiod_recv_pkt(struct brcmf_sdio_dev *sdiodev, struct sk_buff *pkt)
 
 	brcmf_dbg(SDIO, "addr = 0x%x, size = %d\n", addr, pkt->len);
 
-	err = brcmf_sdiod_addrprep(sdiodev, 4, &addr);
+	err = brcmf_sdiod_addrprep(sdiodev, &addr);
 	if (err)
 		goto done;
 
@@ -761,7 +760,7 @@ int brcmf_sdiod_recv_chain(struct brcmf_sdio_dev *sdiodev,
 	brcmf_dbg(SDIO, "addr = 0x%x, size = %d\n",
 		  addr, pktq->qlen);
 
-	err = brcmf_sdiod_addrprep(sdiodev, 4, &addr);
+	err = brcmf_sdiod_addrprep(sdiodev, &addr);
 	if (err)
 		goto done;
 
@@ -806,7 +805,7 @@ int brcmf_sdiod_send_buf(struct brcmf_sdio_dev *sdiodev, u8 *buf, uint nbytes)
 
 	memcpy(mypkt->data, buf, nbytes);
 
-	err = brcmf_sdiod_addrprep(sdiodev, 4, &addr);
+	err = brcmf_sdiod_addrprep(sdiodev, &addr);
 
 	if (!err)
 		err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, true, addr,
@@ -826,7 +825,7 @@ int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev,
 
 	brcmf_dbg(SDIO, "addr = 0x%x, size = %d\n", addr, pktq->qlen);
 
-	err = brcmf_sdiod_addrprep(sdiodev, 4, &addr);
+	err = brcmf_sdiod_addrprep(sdiodev, &addr);
 	if (err)
 		return err;
 
-- 
2.11.0

[PATCH 06/21] brcmfmac: Clean up SDIO IO functions.

[Ian Molton]

---
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 266 ++++++++-------------
 1 file changed, 105 insertions(+), 161 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 2b527d2a2f8a..06e6a72245a7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -232,6 +232,52 @@ void brcmf_sdiod_change_state(struct brcmf_sdio_dev *sdiodev,
 	sdiodev->state = state;
 }
 
+static int
+brcmf_sdiod_set_sbaddr_window(struct brcmf_sdio_dev *sdiodev, u32 address)
+{
+	int err = 0, i;
+	u32 addr;
+
+	addr = (address & SBSDIO_SBWINDOW_MASK) >> 8;
+
+	for (i = 0; i < 3; i++) {
+		brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_SBADDRLOW + i, addr & 0xff,
+					&err);
+		if (err)
+			break;
+
+		addr = addr >> 8;
+	}
+
+	return err;
+}
+
+// bar0 = top 17 bits. lowest of which seems to be SBSDIO_SB_ACCESS_2_4B_FLAG
+// if accessing a different bar, program the bar0 value into SBSDIO_FUNC1_SBADDRLOW
+// addr loses its top 17 bits and is ORed with SBSDIO_SB_ACCESS_2_4B_FLAG
+// Appears to only be required for 4 byte operations.
+// Value of sdiodev->sbwad is set here and used elsewhere (Why?)
+static int
+brcmf_sdiod_addrprep(struct brcmf_sdio_dev *sdiodev, u32 *addr)
+{
+	uint bar0 = *addr & SBSDIO_SBWINDOW_MASK;
+	int err = 0;
+
+	if (bar0 != sdiodev->sbwad) {
+		printk("WTAF? %08x %08x\n", bar0, sdiodev->sbwad);
+		err = brcmf_sdiod_set_sbaddr_window(sdiodev, bar0);
+		if (err)
+			return err;
+
+		sdiodev->sbwad = bar0;
+	}
+
+	*addr &= SBSDIO_SB_OFT_ADDR_MASK;
+	*addr |= SBSDIO_SB_ACCESS_2_4B_FLAG;
+
+	return 0;
+}
+
 static inline int brcmf_sdiod_f0_writeb(struct sdio_func *func, u8 byte, uint regaddr)
 {
 	int err_ret;
@@ -250,93 +296,45 @@ static inline int brcmf_sdiod_f0_writeb(struct sdio_func *func, u8 byte, uint re
 	return err_ret;
 }
 
-static int brcmf_sdiod_request_data(struct brcmf_sdio_dev *sdiodev, u8 fn,
-				    u32 addr, u8 regsz, void *data, bool write)
-{
-	struct sdio_func *func;
-	int ret = -EINVAL;
-
-	brcmf_dbg(SDIO, "rw=%d, func=%d, addr=0x%05x, nbytes=%d\n",
-		  write, fn, addr, regsz);
-
-	/* only allow byte access on F0 */
-	if (WARN_ON(regsz > 1 && !fn))
-		return -EINVAL;
-	func = sdiodev->func[fn];
-
-	switch (regsz) {
-	case 1:
-		if (write) {
-			if (fn)
-				sdio_writeb(func, *(u8 *)data, addr, &ret);
-			else
-				ret = brcmf_sdiod_f0_writeb(func, *(u8 *)data, addr);
-		} else {
-			if (fn)
-				*(u8 *)data = sdio_readb(func, addr, &ret);
-			else
-				*(u8 *)data = sdio_f0_readb(func, addr, &ret);
-		}
-		break;
-	case 2:
-		if (write)
-			sdio_writew(func, *(u16 *)data, addr, &ret);
-		else
-			*(u16 *)data = sdio_readw(func, addr, &ret);
-		break;
-	case 4:
-		if (write)
-			sdio_writel(func, *(u32 *)data, addr, &ret);
-		else
-			*(u32 *)data = sdio_readl(func, addr, &ret);
-		break;
-	default:
-		brcmf_err("invalid size: %d\n", regsz);
-		break;
-	}
-
-	if (ret)
-		brcmf_dbg(SDIO, "failed to %s data F%d@0x%05x, err: %d\n",
-			  write ? "write" : "read", fn, addr, ret);
-
-	return ret;
-}
-
 static int brcmf_sdiod_reg_write(struct brcmf_sdio_dev *sdiodev, u32 addr,
 				   u8 regsz, void *data)
 {
-	u8 func;
-	s32 retry = 0;
 	int ret;
 
-	if (sdiodev->state == BRCMF_SDIOD_NOMEDIUM)
-		return -ENOMEDIUM;
-
 	/*
 	 * figure out how to read the register based on address range
 	 * 0x00 ~ 0x7FF: function 0 CCCR and FBR
 	 * 0x10000 ~ 0x1FFFF: function 1 miscellaneous registers
 	 * The rest: function 1 silicon backplane core registers
+	 * f0 writes must be bytewise
 	 */
-	if ((addr & ~REG_F0_REG_MASK) == 0)
-		func = SDIO_FUNC_0;
-	else
-		func = SDIO_FUNC_1;
 
-	do {
-		/* for retry wait for 1 ms till bus get settled down */
-		if (retry)
-			usleep_range(1000, 2000);
-
-		ret = brcmf_sdiod_request_data(sdiodev, func, addr, regsz,
-					       data, true);
+	if ((addr & ~REG_F0_REG_MASK) == 0) {
+		if (WARN_ON(regsz > 1))
+			return -EINVAL;
+		ret = brcmf_sdiod_f0_writeb(sdiodev->func[0], *(u8 *)data, addr);
+	}
+	else {
+		switch (regsz) {
+			case 1:
+				sdio_writeb(sdiodev->func[1], *(u8 *)data, addr, &ret);
+				break;
+			case 4:
+				ret = brcmf_sdiod_addrprep(sdiodev, &addr);
+				if (ret)
+					goto done;
 
-	} while (ret != 0 && ret != -ENOMEDIUM &&
-		 retry++ < SDIOH_API_ACCESS_RETRY_LIMIT);
+				sdio_writel(sdiodev->func[1], *(u32 *)data, addr, &ret);
+				break;
+			default:
+				BUG();
+				ret = -EINVAL;
+				break;
+		}
+	}
 
-	if (ret == -ENOMEDIUM)
-		brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_NOMEDIUM);
-	else if (ret != 0) {
+#if 0
+	if (ret != 0) {
 		/*
 		 * SleepCSR register access can fail when
 		 * waking up the device so reduce this noise
@@ -349,47 +347,50 @@ static int brcmf_sdiod_reg_write(struct brcmf_sdio_dev *sdiodev, u32 addr,
 			brcmf_dbg(SDIO, "failed to write data F%d@0x%05x, err: %d\n",
 				  func, addr, ret);
 	}
+#endif
 
+done:
 	return ret;
 }
 
 static int brcmf_sdiod_reg_read(struct brcmf_sdio_dev *sdiodev, u32 addr,
 				   u8 regsz, void *data)
 {
-	u8 func;
-	s32 retry = 0;
 	int ret;
 
-	if (sdiodev->state == BRCMF_SDIOD_NOMEDIUM)
-		return -ENOMEDIUM;
-
 	/*
 	 * figure out how to read the register based on address range
 	 * 0x00 ~ 0x7FF: function 0 CCCR and FBR
 	 * 0x10000 ~ 0x1FFFF: function 1 miscellaneous registers
 	 * The rest: function 1 silicon backplane core registers
+	 * f0 reads must be bytewise
 	 */
-	if ((addr & ~REG_F0_REG_MASK) == 0)
-		func = SDIO_FUNC_0;
-	else
-		func = SDIO_FUNC_1;
-
-	do {
-		memset(data, 0, regsz);
-
-		/* for retry wait for 1 ms till bus get settled down */
-		if (retry)
-			usleep_range(1000, 2000);
-
-		ret = brcmf_sdiod_request_data(sdiodev, func, addr, regsz,
-					       data, false);
+	if ((addr & ~REG_F0_REG_MASK) == 0) {
+		if (WARN_ON(regsz > 1))
+			return -EINVAL;
+		*(u8 *)data = sdio_f0_readb(sdiodev->func[0], addr, &ret);
+	}
+	else {
+		switch (regsz) {
+			case 1:
+				*(u8 *)data = sdio_readb(sdiodev->func[1], addr, &ret);
+				break;
+			case 4:
+				ret = brcmf_sdiod_addrprep(sdiodev, &addr);
+				if (ret)
+					goto done;
 
-	} while (ret != 0 && ret != -ENOMEDIUM &&
-		 retry++ < SDIOH_API_ACCESS_RETRY_LIMIT);
+				*(u32 *)data = sdio_readl(sdiodev->func[1], addr, &ret);
+				break;
+			default:
+				BUG();
+				ret = -EINVAL;
+				break;
+		}
+	}
 
-	if (ret == -ENOMEDIUM)
-		brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_NOMEDIUM);
-	else if (ret != 0) {
+#if 0
+	if (ret != 0) {
 		/*
 		 * SleepCSR register access can fail when
 		 * waking up the device so reduce this noise
@@ -402,62 +403,19 @@ static int brcmf_sdiod_reg_read(struct brcmf_sdio_dev *sdiodev, u32 addr,
 			brcmf_dbg(SDIO, "failed to read data F%d@0x%05x, err: %d\n",
 				  func, addr, ret);
 	}
+#endif
 
+done:
 	return ret;
 }
 
-static int
-brcmf_sdiod_set_sbaddr_window(struct brcmf_sdio_dev *sdiodev, u32 address)
-{
-	int err = 0, i;
-	u32 addr;
-
-	if (sdiodev->state == BRCMF_SDIOD_NOMEDIUM)
-		return -ENOMEDIUM;
-
-	addr = (address & SBSDIO_SBWINDOW_MASK) >> 8;
-
-	for (i = 0; i < 3; i++) {
-		brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_SBADDRLOW + i, addr & 0xff,
-					&err);
-		if (err)
-			break;
-
-		addr = addr >> 8;
-	}
-
-	return err;
-}
-
-static int
-brcmf_sdiod_addrprep(struct brcmf_sdio_dev *sdiodev, u32 *addr)
-{
-	uint bar0 = *addr & ~SBSDIO_SB_OFT_ADDR_MASK;
-	int err = 0;
-
-	if (bar0 != sdiodev->sbwad) {
-		err = brcmf_sdiod_set_sbaddr_window(sdiodev, bar0);
-		if (err)
-			return err;
-
-		sdiodev->sbwad = bar0;
-	}
-
-	*addr &= SBSDIO_SB_OFT_ADDR_MASK;
-
-	*addr |= SBSDIO_SB_ACCESS_2_4B_FLAG;
-
-	return 0;
-}
 
 u8 brcmf_sdiod_regrb(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
 {
-	u8 data;
+	u8 data = 0;
 	int retval;
 
-	brcmf_dbg(SDIO, "addr:0x%08x\n", addr);
-	retval = brcmf_sdiod_reg_read(sdiodev, addr, sizeof(data), &data);
-	brcmf_dbg(SDIO, "data:0x%02x\n", data);
+	retval = brcmf_sdiod_reg_read(sdiodev, addr, 1, &data);
 
 	if (ret)
 		*ret = retval;
@@ -470,15 +428,8 @@ u32 brcmf_sdiod_regrl(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
 	u32 data = 0;
 	int retval;
 
-	brcmf_dbg(SDIO, "addr:0x%08x\n", addr);
-	retval = brcmf_sdiod_addrprep(sdiodev, &addr);
-	if (retval)
-		goto done;
-	retval = brcmf_sdiod_reg_read(sdiodev, addr, sizeof(data), &data);
-
-	brcmf_dbg(SDIO, "data:0x%08x\n", data);
+	retval = brcmf_sdiod_reg_read(sdiodev, addr, 4, &data);
 
-done:
 	if (ret)
 		*ret = retval;
 
@@ -490,8 +441,7 @@ void brcmf_sdiod_regwb(struct brcmf_sdio_dev *sdiodev, u32 addr,
 {
 	int retval;
 
-	brcmf_dbg(SDIO, "addr:0x%08x, data:0x%02x\n", addr, data);
-	retval = brcmf_sdiod_reg_write(sdiodev, addr, sizeof(data), &data);
+	retval = brcmf_sdiod_reg_write(sdiodev, addr, 1, &data);
 
 	if (ret)
 		*ret = retval;
@@ -502,14 +452,8 @@ void brcmf_sdiod_regwl(struct brcmf_sdio_dev *sdiodev, u32 addr,
 {
 	int retval;
 
-	brcmf_dbg(SDIO, "addr:0x%08x, data:0x%08x\n", addr, data);
-	retval = brcmf_sdiod_addrprep(sdiodev, &addr);
-	if (retval)
-		goto done;
-
-	retval = brcmf_sdiod_reg_write(sdiodev, addr, sizeof(data), &data);
+	retval = brcmf_sdiod_reg_write(sdiodev, addr, 4, &data);
 
-done:
 	if (ret)
 		*ret = retval;
 }
-- 
2.11.0

[PATCH 07/21] brcmfmac: Split buff_rw function up + cleanup annoying bcmerror variable

[Ian Molton]

---
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 76 +++++++++++++++-------
 1 file changed, 52 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 06e6a72245a7..0d5004e18cb0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -458,8 +458,8 @@ void brcmf_sdiod_regwl(struct brcmf_sdio_dev *sdiodev, u32 addr,
 		*ret = retval;
 }
 
-static int brcmf_sdiod_buffrw(struct brcmf_sdio_dev *sdiodev, uint fn,
-			     bool write, u32 addr, struct sk_buff *pkt)
+static int brcmf_sdiod_buff_read(struct brcmf_sdio_dev *sdiodev, uint fn,
+				u32 addr, struct sk_buff *pkt)
 {
 	unsigned int req_sz;
 	int err;
@@ -468,18 +468,36 @@ static int brcmf_sdiod_buffrw(struct brcmf_sdio_dev *sdiodev, uint fn,
 	req_sz = pkt->len + 3;
 	req_sz &= (uint)~3;
 
-	if (write)
-		err = sdio_memcpy_toio(sdiodev->func[fn], addr,
-				       ((u8 *)(pkt->data)), req_sz);
-	else if (fn == 1)
-		err = sdio_memcpy_fromio(sdiodev->func[fn], ((u8 *)(pkt->data)),
-					 addr, req_sz);
+	if (fn == 1)
+		err = sdio_memcpy_fromio(sdiodev->func[fn],
+				((u8 *)(pkt->data)), addr, req_sz);
 	else
 		/* function 2 read is FIFO operation */
-		err = sdio_readsb(sdiodev->func[fn], ((u8 *)(pkt->data)), addr,
-				  req_sz);
+		err = sdio_readsb(sdiodev->func[fn],
+				((u8 *)(pkt->data)), addr, req_sz);
+
 	if (err == -ENOMEDIUM)
 		brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_NOMEDIUM);
+
+	return err;
+}
+
+static int brcmf_sdiod_buff_write(struct brcmf_sdio_dev *sdiodev, uint fn,
+				u32 addr, struct sk_buff *pkt)
+{
+	unsigned int req_sz;
+	int err;
+
+	/* Single skb use the standard mmc interface */
+	req_sz = pkt->len + 3;
+	req_sz &= (uint)~3;
+
+	err = sdio_memcpy_toio(sdiodev->func[fn], addr,
+			       ((u8 *)(pkt->data)), req_sz);
+
+	if (err == -ENOMEDIUM)
+		brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_NOMEDIUM);
+
 	return err;
 }
 
@@ -687,7 +705,7 @@ int brcmf_sdiod_recv_pkt(struct brcmf_sdio_dev *sdiodev, struct sk_buff *pkt)
 	if (err)
 		goto done;
 
-	err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, false, addr, pkt);
+	err = brcmf_sdiod_buff_read(sdiodev, SDIO_FUNC_2, addr, pkt);
 
 done:
 	return err;
@@ -709,13 +727,13 @@ int brcmf_sdiod_recv_chain(struct brcmf_sdio_dev *sdiodev,
 		goto done;
 
 	if (pktq->qlen == 1)
-		err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, false, addr,
+		err = brcmf_sdiod_buff_read(sdiodev, SDIO_FUNC_2, addr,
 					 pktq->next);
 	else if (!sdiodev->sg_support) {
 		glom_skb = brcmu_pkt_buf_get_skb(totlen);
 		if (!glom_skb)
 			return -ENOMEM;
-		err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, false, addr,
+		err = brcmf_sdiod_buff_read(sdiodev, SDIO_FUNC_2, addr,
 					 glom_skb);
 		if (err) {
 			brcmu_pkt_buf_free_skb(glom_skb);
@@ -741,6 +759,7 @@ int brcmf_sdiod_send_buf(struct brcmf_sdio_dev *sdiodev, u8 *buf, uint nbytes)
 	int err;
 
 	mypkt = brcmu_pkt_buf_get_skb(nbytes);
+
 	if (!mypkt) {
 		brcmf_err("brcmu_pkt_buf_get_skb failed: len %d\n",
 			  nbytes);
@@ -752,10 +771,10 @@ int brcmf_sdiod_send_buf(struct brcmf_sdio_dev *sdiodev, u8 *buf, uint nbytes)
 	err = brcmf_sdiod_addrprep(sdiodev, &addr);
 
 	if (!err)
-		err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, true, addr,
-					 mypkt);
+		err = brcmf_sdiod_buff_write(sdiodev, SDIO_FUNC_2, addr, mypkt);
 
 	brcmu_pkt_buf_free_skb(mypkt);
+
 	return err;
 
 }
@@ -775,7 +794,7 @@ int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev,
 
 	if (pktq->qlen == 1 || !sdiodev->sg_support)
 		skb_queue_walk(pktq, skb) {
-			err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_2, true,
+			err = brcmf_sdiod_buff_write(sdiodev, SDIO_FUNC_2,
 						 addr, skb);
 			if (err)
 				break;
@@ -791,7 +810,7 @@ int
 brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 		  u8 *data, uint size)
 {
-	int bcmerror = 0;
+	int err = 0;
 	struct sk_buff *pkt;
 	u32 sdaddr;
 	uint dsize;
@@ -815,9 +834,10 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 
 	/* Do the transfer(s) */
 	while (size) {
+
 		/* Set the backplane window to include the start address */
-		bcmerror = brcmf_sdiod_set_sbaddr_window(sdiodev, address);
-		if (bcmerror)
+		err = brcmf_sdiod_set_sbaddr_window(sdiodev, address);
+		if (err)
 			break;
 
 		brcmf_dbg(SDIO, "%s %d bytes at offset 0x%08x in window 0x%08x\n",
@@ -828,14 +848,22 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 		sdaddr |= SBSDIO_SB_ACCESS_2_4B_FLAG;
 
 		skb_put(pkt, dsize);
-		if (write)
+
+		if (write) {
 			memcpy(pkt->data, data, dsize);
-		bcmerror = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_1, write,
-					      sdaddr, pkt);
-		if (bcmerror) {
+			err = brcmf_sdiod_buff_write(sdiodev, SDIO_FUNC_1,
+						      sdaddr, pkt);
+		}
+		else {
+			err = brcmf_sdiod_buff_read(sdiodev, SDIO_FUNC_1,
+						      sdaddr, pkt);
+		}
+
+		if (err) {
 			brcmf_err("membytes transfer failed\n");
 			break;
 		}
+
 		if (!write)
 			memcpy(data, pkt->data, dsize);
 		skb_trim(pkt, 0);
@@ -859,7 +887,7 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 
 	sdio_release_host(sdiodev->func[1]);
 
-	return bcmerror;
+	return err;
 }
 
 int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, uint fn)
-- 
2.11.0

[PATCH 08/21] brcmfmac: Sanitise all byte-wise IO

[Ian Molton]

---
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 181 +++------------------
 .../wireless/broadcom/brcm80211/brcmfmac/sdio.c    |  96 +++++------
 .../wireless/broadcom/brcm80211/brcmfmac/sdio.h    |  19 ++-
 3 files changed, 83 insertions(+), 213 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 0d5004e18cb0..84ae67c11970 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -143,23 +143,22 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
 			gpiocontrol |= 0x2;
 			brcmf_sdiod_regwl(sdiodev, addr, gpiocontrol, &ret);
 
-			brcmf_sdiod_regwb(sdiodev, SBSDIO_GPIO_SELECT, 0xf,
+			brcm_sdio_func1_wb(sdiodev, SBSDIO_GPIO_SELECT, 0xf,
 					  &ret);
-			brcmf_sdiod_regwb(sdiodev, SBSDIO_GPIO_OUT, 0, &ret);
-			brcmf_sdiod_regwb(sdiodev, SBSDIO_GPIO_EN, 0x2, &ret);
+			brcm_sdio_func1_wb(sdiodev, SBSDIO_GPIO_OUT, 0, &ret);
+			brcm_sdio_func1_wb(sdiodev, SBSDIO_GPIO_EN, 0x2, &ret);
 		}
 
 		/* must configure SDIO_CCCR_IENx to enable irq */
-		data = brcmf_sdiod_regrb(sdiodev, SDIO_CCCR_IENx, &ret);
+		data = brcm_sdio_func0_rb(sdiodev, SDIO_CCCR_IENx, &ret);
 		data |= 1 << SDIO_FUNC_1 | 1 << SDIO_FUNC_2 | 1;
-		brcmf_sdiod_regwb(sdiodev, SDIO_CCCR_IENx, data, &ret);
+		brcm_sdio_func0_wb(sdiodev, SDIO_CCCR_IENx, data, &ret);
 
 		/* redirect, configure and enable io for interrupt signal */
 		data = SDIO_SEPINT_MASK | SDIO_SEPINT_OE;
 		if (pdata->oob_irq_flags & IRQF_TRIGGER_HIGH)
 			data |= SDIO_SEPINT_ACT_HI;
-		brcmf_sdiod_regwb(sdiodev, SDIO_CCCR_BRCM_SEPINT, data, &ret);
-
+		brcm_sdio_func0_wb(sdiodev, SDIO_CCCR_BRCM_SEPINT, data, &ret);
 		sdio_release_host(sdiodev->func[1]);
 	} else {
 		brcmf_dbg(SDIO, "Entering\n");
@@ -185,8 +184,8 @@ void brcmf_sdiod_intr_unregister(struct brcmf_sdio_dev *sdiodev)
 
 		pdata = &sdiodev->settings->bus.sdio;
 		sdio_claim_host(sdiodev->func[1]);
-		brcmf_sdiod_regwb(sdiodev, SDIO_CCCR_BRCM_SEPINT, 0, NULL);
-		brcmf_sdiod_regwb(sdiodev, SDIO_CCCR_IENx, 0, NULL);
+		brcm_sdio_func0_wb(sdiodev, SDIO_CCCR_BRCM_SEPINT, 0, NULL);
+		brcm_sdio_func0_wb(sdiodev, SDIO_CCCR_IENx, 0, NULL);
 		sdio_release_host(sdiodev->func[1]);
 
 		sdiodev->oob_irq_requested = false;
@@ -240,13 +239,11 @@ brcmf_sdiod_set_sbaddr_window(struct brcmf_sdio_dev *sdiodev, u32 address)
 
 	addr = (address & SBSDIO_SBWINDOW_MASK) >> 8;
 
-	for (i = 0; i < 3; i++) {
-		brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_SBADDRLOW + i, addr & 0xff,
-					&err);
-		if (err)
-			break;
-
+	for (i = 0 ; i < 3 && !err ; i++) {
+		u8 data = addr & 0xff;
 		addr = addr >> 8;
+
+		brcm_sdio_func1_wb(sdiodev, SBSDIO_FUNC1_SBADDRLOW + i, data, &err);
 	}
 
 	return err;
@@ -278,117 +275,6 @@ brcmf_sdiod_addrprep(struct brcmf_sdio_dev *sdiodev, u32 *addr)
 	return 0;
 }
 
-static inline int brcmf_sdiod_f0_writeb(struct sdio_func *func, u8 byte, uint regaddr)
-{
-	int err_ret;
-
-	/*
-	 * Can only directly write to some F0 registers.
-	 * Handle CCCR_IENx and CCCR_ABORT command
-	 * as a special case.
-	 */
-	if ((regaddr == SDIO_CCCR_ABORT) ||
-	    (regaddr == SDIO_CCCR_IENx))
-		sdio_writeb(func, byte, regaddr, &err_ret);
-	else
-		sdio_f0_writeb(func, byte, regaddr, &err_ret);
-
-	return err_ret;
-}
-
-static int brcmf_sdiod_reg_write(struct brcmf_sdio_dev *sdiodev, u32 addr,
-				   u8 regsz, void *data)
-{
-	int ret;
-
-	/*
-	 * figure out how to read the register based on address range
-	 * 0x00 ~ 0x7FF: function 0 CCCR and FBR
-	 * 0x10000 ~ 0x1FFFF: function 1 miscellaneous registers
-	 * The rest: function 1 silicon backplane core registers
-	 * f0 writes must be bytewise
-	 */
-
-	if ((addr & ~REG_F0_REG_MASK) == 0) {
-		if (WARN_ON(regsz > 1))
-			return -EINVAL;
-		ret = brcmf_sdiod_f0_writeb(sdiodev->func[0], *(u8 *)data, addr);
-	}
-	else {
-		switch (regsz) {
-			case 1:
-				sdio_writeb(sdiodev->func[1], *(u8 *)data, addr, &ret);
-				break;
-			case 4:
-				ret = brcmf_sdiod_addrprep(sdiodev, &addr);
-				if (ret)
-					goto done;
-
-				sdio_writel(sdiodev->func[1], *(u32 *)data, addr, &ret);
-				break;
-			default:
-				BUG();
-				ret = -EINVAL;
-				break;
-		}
-	}
-
-#if 0
-	if (ret != 0) {
-		/*
-		 * SleepCSR register access can fail when
-		 * waking up the device so reduce this noise
-		 * in the logs.
-		 */
-		if (addr != SBSDIO_FUNC1_SLEEPCSR)
-			brcmf_err("failed to write data F%d@0x%05x, err: %d\n",
-				  func, addr, ret);
-		else
-			brcmf_dbg(SDIO, "failed to write data F%d@0x%05x, err: %d\n",
-				  func, addr, ret);
-	}
-#endif
-
-done:
-	return ret;
-}
-
-static int brcmf_sdiod_reg_read(struct brcmf_sdio_dev *sdiodev, u32 addr,
-				   u8 regsz, void *data)
-{
-	int ret;
-
-	/*
-	 * figure out how to read the register based on address range
-	 * 0x00 ~ 0x7FF: function 0 CCCR and FBR
-	 * 0x10000 ~ 0x1FFFF: function 1 miscellaneous registers
-	 * The rest: function 1 silicon backplane core registers
-	 * f0 reads must be bytewise
-	 */
-	if ((addr & ~REG_F0_REG_MASK) == 0) {
-		if (WARN_ON(regsz > 1))
-			return -EINVAL;
-		*(u8 *)data = sdio_f0_readb(sdiodev->func[0], addr, &ret);
-	}
-	else {
-		switch (regsz) {
-			case 1:
-				*(u8 *)data = sdio_readb(sdiodev->func[1], addr, &ret);
-				break;
-			case 4:
-				ret = brcmf_sdiod_addrprep(sdiodev, &addr);
-				if (ret)
-					goto done;
-
-				*(u32 *)data = sdio_readl(sdiodev->func[1], addr, &ret);
-				break;
-			default:
-				BUG();
-				ret = -EINVAL;
-				break;
-		}
-	}
-
 #if 0
 	if (ret != 0) {
 		/*
@@ -405,30 +291,15 @@ static int brcmf_sdiod_reg_read(struct brcmf_sdio_dev *sdiodev, u32 addr,
 	}
 #endif
 
-done:
-	return ret;
-}
-
-
-u8 brcmf_sdiod_regrb(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
-{
-	u8 data = 0;
-	int retval;
-
-	retval = brcmf_sdiod_reg_read(sdiodev, addr, 1, &data);
-
-	if (ret)
-		*ret = retval;
-
-	return data;
-}
-
 u32 brcmf_sdiod_regrl(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
 {
 	u32 data = 0;
 	int retval;
 
-	retval = brcmf_sdiod_reg_read(sdiodev, addr, 4, &data);
+	retval = brcmf_sdiod_addrprep(sdiodev, &addr);
+
+	if (!retval)
+		data = sdio_readl(sdiodev->func[1], addr, &retval);
 
 	if (ret)
 		*ret = retval;
@@ -436,23 +307,15 @@ u32 brcmf_sdiod_regrl(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
 	return data;
 }
 
-void brcmf_sdiod_regwb(struct brcmf_sdio_dev *sdiodev, u32 addr,
-		      u8 data, int *ret)
-{
-	int retval;
-
-	retval = brcmf_sdiod_reg_write(sdiodev, addr, 1, &data);
-
-	if (ret)
-		*ret = retval;
-}
-
 void brcmf_sdiod_regwl(struct brcmf_sdio_dev *sdiodev, u32 addr,
 		      u32 data, int *ret)
 {
 	int retval;
 
-	retval = brcmf_sdiod_reg_write(sdiodev, addr, 4, &data);
+	retval = brcmf_sdiod_addrprep(sdiodev, &addr);
+
+	if (!retval)
+		sdio_writel(sdiodev->func[1], data, addr, &retval);
 
 	if (ret)
 		*ret = retval;
@@ -890,12 +753,12 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 	return err;
 }
 
-int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, uint fn)
+int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, u8 fn)
 {
 	brcmf_dbg(SDIO, "Enter\n");
 
 	/* issue abort cmd52 command through F0 */
-	brcmf_sdiod_f0_writeb(sdiodev->func[0], (u8)fn, SDIO_CCCR_ABORT);
+	brcm_sdio_func0_wb(sdiodev, SDIO_CCCR_ABORT, fn, NULL);//FIXME we dont care about errors, right?
 
 
 	brcmf_dbg(SDIO, "Exit\n");
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 5653d6dd38f6..ce540625c29e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -691,7 +691,7 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
 
 	wr_val = (on << SBSDIO_FUNC1_SLEEPCSR_KSO_SHIFT);
 	/* 1st KSO write goes to AOS wake up core if device is asleep  */
-	brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR,
+	brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR,
 			  wr_val, &err);
 
 	if (on) {
@@ -718,7 +718,7 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
 		 * just one write attempt may fail,
 		 * read it back until it matches written value
 		 */
-		rd_val = brcmf_sdiod_regrb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR,
+		rd_val = brcm_sdio_func1_rb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR,
 					   &err);
 		if (!err) {
 			if ((rd_val & bmask) == cmp_val)
@@ -729,7 +729,7 @@ brcmf_sdio_kso_control(struct brcmf_sdio *bus, bool on)
 		if (err && (err_cnt++ > BRCMF_SDIO_MAX_ACCESS_ERRORS))
 			break;
 		udelay(KSO_WAIT_US);
-		brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR,
+		brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR,
 				  wr_val, &err);
 	} while (try_cnt++ < MAX_KSO_ATTEMPTS);
 
@@ -766,7 +766,7 @@ static int brcmf_sdio_htclk(struct brcmf_sdio *bus, bool on, bool pendok)
 		clkreq =
 		    bus->alp_only ? SBSDIO_ALP_AVAIL_REQ : SBSDIO_HT_AVAIL_REQ;
 
-		brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
+		brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
 				  clkreq, &err);
 		if (err) {
 			brcmf_err("HT Avail request error: %d\n", err);
@@ -774,7 +774,7 @@ static int brcmf_sdio_htclk(struct brcmf_sdio *bus, bool on, bool pendok)
 		}
 
 		/* Check current status */
-		clkctl = brcmf_sdiod_regrb(bus->sdiodev,
+		clkctl = brcm_sdio_func1_rb(bus->sdiodev,
 					   SBSDIO_FUNC1_CHIPCLKCSR, &err);
 		if (err) {
 			brcmf_err("HT Avail read error: %d\n", err);
@@ -784,7 +784,7 @@ static int brcmf_sdio_htclk(struct brcmf_sdio *bus, bool on, bool pendok)
 		/* Go to pending and await interrupt if appropriate */
 		if (!SBSDIO_CLKAV(clkctl, bus->alp_only) && pendok) {
 			/* Allow only clock-available interrupt */
-			devctl = brcmf_sdiod_regrb(bus->sdiodev,
+			devctl = brcm_sdio_func1_rb(bus->sdiodev,
 						   SBSDIO_DEVICE_CTL, &err);
 			if (err) {
 				brcmf_err("Devctl error setting CA: %d\n",
@@ -793,7 +793,7 @@ static int brcmf_sdio_htclk(struct brcmf_sdio *bus, bool on, bool pendok)
 			}
 
 			devctl |= SBSDIO_DEVCTL_CA_INT_ONLY;
-			brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_DEVICE_CTL,
+			brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_DEVICE_CTL,
 					  devctl, &err);
 			brcmf_dbg(SDIO, "CLKCTL: set PENDING\n");
 			bus->clkstate = CLK_PENDING;
@@ -801,10 +801,10 @@ static int brcmf_sdio_htclk(struct brcmf_sdio *bus, bool on, bool pendok)
 			return 0;
 		} else if (bus->clkstate == CLK_PENDING) {
 			/* Cancel CA-only interrupt filter */
-			devctl = brcmf_sdiod_regrb(bus->sdiodev,
+			devctl = brcm_sdio_func1_rb(bus->sdiodev,
 						   SBSDIO_DEVICE_CTL, &err);
 			devctl &= ~SBSDIO_DEVCTL_CA_INT_ONLY;
-			brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_DEVICE_CTL,
+			brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_DEVICE_CTL,
 					  devctl, &err);
 		}
 
@@ -812,7 +812,7 @@ static int brcmf_sdio_htclk(struct brcmf_sdio *bus, bool on, bool pendok)
 		timeout = jiffies +
 			  msecs_to_jiffies(PMU_MAX_TRANSITION_DLY/1000);
 		while (!SBSDIO_CLKAV(clkctl, bus->alp_only)) {
-			clkctl = brcmf_sdiod_regrb(bus->sdiodev,
+			clkctl = brcm_sdio_func1_rb(bus->sdiodev,
 						   SBSDIO_FUNC1_CHIPCLKCSR,
 						   &err);
 			if (time_after(jiffies, timeout))
@@ -846,15 +846,15 @@ static int brcmf_sdio_htclk(struct brcmf_sdio *bus, bool on, bool pendok)
 
 		if (bus->clkstate == CLK_PENDING) {
 			/* Cancel CA-only interrupt filter */
-			devctl = brcmf_sdiod_regrb(bus->sdiodev,
+			devctl = brcm_sdio_func1_rb(bus->sdiodev,
 						   SBSDIO_DEVICE_CTL, &err);
 			devctl &= ~SBSDIO_DEVCTL_CA_INT_ONLY;
-			brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_DEVICE_CTL,
+			brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_DEVICE_CTL,
 					  devctl, &err);
 		}
 
 		bus->clkstate = CLK_SDONLY;
-		brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
+		brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
 				  clkreq, &err);
 		brcmf_dbg(SDIO, "CLKCTL: turned OFF\n");
 		if (err) {
@@ -945,12 +945,12 @@ brcmf_sdio_bus_sleep(struct brcmf_sdio *bus, bool sleep, bool pendok)
 
 		/* Going to sleep */
 		if (sleep) {
-			clkcsr = brcmf_sdiod_regrb(bus->sdiodev,
+			clkcsr = brcm_sdio_func1_rb(bus->sdiodev,
 						   SBSDIO_FUNC1_CHIPCLKCSR,
 						   &err);
 			if ((clkcsr & SBSDIO_CSR_MASK) == 0) {
 				brcmf_dbg(SDIO, "no clock, set ALP\n");
-				brcmf_sdiod_regwb(bus->sdiodev,
+				brcm_sdio_func1_wb(bus->sdiodev,
 						  SBSDIO_FUNC1_CHIPCLKCSR,
 						  SBSDIO_ALP_AVAIL_REQ, &err);
 			}
@@ -1167,15 +1167,15 @@ static void brcmf_sdio_rxfail(struct brcmf_sdio *bus, bool abort, bool rtx)
 	if (abort)
 		brcmf_sdiod_abort(bus->sdiodev, SDIO_FUNC_2);
 
-	brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_FUNC1_FRAMECTRL,
+	brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_FUNC1_FRAMECTRL,
 			  SFC_RF_TERM, &err);
 	bus->sdcnt.f1regdata++;
 
 	/* Wait until the packet has been flushed (device/FIFO stable) */
 	for (lastrbc = retries = 0xffff; retries > 0; retries--) {
-		hi = brcmf_sdiod_regrb(bus->sdiodev,
+		hi = brcm_sdio_func1_rb(bus->sdiodev,
 				       SBSDIO_FUNC1_RFRAMEBCHI, &err);
-		lo = brcmf_sdiod_regrb(bus->sdiodev,
+		lo = brcm_sdio_func1_rb(bus->sdiodev,
 				       SBSDIO_FUNC1_RFRAMEBCLO, &err);
 		bus->sdcnt.f1regdata += 2;
 
@@ -1218,12 +1218,12 @@ static void brcmf_sdio_txfail(struct brcmf_sdio *bus)
 	bus->sdcnt.tx_sderrs++;
 
 	brcmf_sdiod_abort(sdiodev, SDIO_FUNC_2);
-	brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_FRAMECTRL, SFC_WF_TERM, NULL);
+	brcm_sdio_func1_wb(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);
+		hi = brcm_sdio_func1_rb(sdiodev, SBSDIO_FUNC1_WFRAMEBCHI, NULL);
+		lo = brcm_sdio_func1_rb(sdiodev, SBSDIO_FUNC1_WFRAMEBCLO, NULL);
 		bus->sdcnt.f1regdata += 2;
 		if ((hi == 0) && (lo == 0))
 			break;
@@ -2431,10 +2431,10 @@ static void brcmf_sdio_bus_stop(struct device *dev)
 		bus->hostintmask = 0;
 
 		/* Force backplane clocks to assure F2 interrupt propagates */
-		saveclk = brcmf_sdiod_regrb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
+		saveclk = brcm_sdio_func1_rb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
 					    &err);
 		if (!err)
-			brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
+			brcm_sdio_func1_wb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
 					  (saveclk | SBSDIO_FORCE_HT), &err);
 		if (err)
 			brcmf_err("Failed to force clock for F2: err %d\n",
@@ -2530,22 +2530,22 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 
 #ifdef DEBUG
 		/* Check for inconsistent device control */
-		devctl = brcmf_sdiod_regrb(bus->sdiodev,
+		devctl = brcm_sdio_func1_rb(bus->sdiodev,
 					   SBSDIO_DEVICE_CTL, &err);
 #endif				/* DEBUG */
 
 		/* Read CSR, if clock on switch to AVAIL, else ignore */
-		clkctl = brcmf_sdiod_regrb(bus->sdiodev,
+		clkctl = brcm_sdio_func1_rb(bus->sdiodev,
 					   SBSDIO_FUNC1_CHIPCLKCSR, &err);
 
 		brcmf_dbg(SDIO, "DPC: PENDING, devctl 0x%02x clkctl 0x%02x\n",
 			  devctl, clkctl);
 
 		if (SBSDIO_HTAV(clkctl)) {
-			devctl = brcmf_sdiod_regrb(bus->sdiodev,
+			devctl = brcm_sdio_func1_rb(bus->sdiodev,
 						   SBSDIO_DEVICE_CTL, &err);
 			devctl &= ~SBSDIO_DEVCTL_CA_INT_ONLY;
-			brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_DEVICE_CTL,
+			brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_DEVICE_CTL,
 					  devctl, &err);
 			bus->clkstate = CLK_AVAIL;
 		}
@@ -3332,21 +3332,21 @@ static void brcmf_sdio_sr_init(struct brcmf_sdio *bus)
 
 	brcmf_dbg(TRACE, "Enter\n");
 
-	val = brcmf_sdiod_regrb(bus->sdiodev, SBSDIO_FUNC1_WAKEUPCTRL, &err);
+	val = brcm_sdio_func1_rb(bus->sdiodev, SBSDIO_FUNC1_WAKEUPCTRL, &err);
 	if (err) {
 		brcmf_err("error reading SBSDIO_FUNC1_WAKEUPCTRL\n");
 		return;
 	}
 
 	val |= 1 << SBSDIO_FUNC1_WCTRL_HTWAIT_SHIFT;
-	brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_FUNC1_WAKEUPCTRL, val, &err);
+	brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_FUNC1_WAKEUPCTRL, val, &err);
 	if (err) {
 		brcmf_err("error writing SBSDIO_FUNC1_WAKEUPCTRL\n");
 		return;
 	}
 
 	/* Add CMD14 Support */
-	brcmf_sdiod_regwb(bus->sdiodev, SDIO_CCCR_BRCM_CARDCAP,
+	brcm_sdio_func0_wb(bus->sdiodev, SDIO_CCCR_BRCM_CARDCAP,
 			  (SDIO_CCCR_BRCM_CARDCAP_CMD14_SUPPORT |
 			   SDIO_CCCR_BRCM_CARDCAP_CMD14_EXT),
 			  &err);
@@ -3355,7 +3355,7 @@ static void brcmf_sdio_sr_init(struct brcmf_sdio *bus)
 		return;
 	}
 
-	brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
+	brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
 			  SBSDIO_FORCE_HT, &err);
 	if (err) {
 		brcmf_err("error writing SBSDIO_FUNC1_CHIPCLKCSR\n");
@@ -3379,7 +3379,7 @@ static int brcmf_sdio_kso_init(struct brcmf_sdio *bus)
 	if (brcmf_chip_get_core(bus->ci, BCMA_CORE_SDIO_DEV)->rev < 12)
 		return 0;
 
-	val = brcmf_sdiod_regrb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR, &err);
+	val = brcm_sdio_func1_rb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR, &err);
 	if (err) {
 		brcmf_err("error reading SBSDIO_FUNC1_SLEEPCSR\n");
 		return err;
@@ -3388,7 +3388,7 @@ static int brcmf_sdio_kso_init(struct brcmf_sdio *bus)
 	if (!(val & SBSDIO_FUNC1_SLEEPCSR_KSO_MASK)) {
 		val |= (SBSDIO_FUNC1_SLEEPCSR_KSO_EN <<
 			SBSDIO_FUNC1_SLEEPCSR_KSO_SHIFT);
-		brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR,
+		brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR,
 				  val, &err);
 		if (err) {
 			brcmf_err("error writing SBSDIO_FUNC1_SLEEPCSR\n");
@@ -3550,7 +3550,7 @@ static void brcmf_sdio_bus_watchdog(struct brcmf_sdio *bus)
 				u8 devpend;
 
 				sdio_claim_host(bus->sdiodev->func[1]);
-				devpend = brcmf_sdiod_regrb(bus->sdiodev,
+				devpend = brcm_sdio_func0_rb(bus->sdiodev,
 							    SDIO_CCCR_INTx,
 							    NULL);
 				sdio_release_host(bus->sdiodev->func[1]);
@@ -3710,7 +3710,7 @@ static int brcmf_sdio_buscoreprep(void *ctx)
 
 	/* Try forcing SDIO core to do ALPAvail request only */
 	clkset = SBSDIO_FORCE_HW_CLKREQ_OFF | SBSDIO_ALP_AVAIL_REQ;
-	brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR, clkset, &err);
+	brcm_sdio_func1_wb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR, clkset, &err);
 	if (err) {
 		brcmf_err("error writing for HT off\n");
 		return err;
@@ -3718,7 +3718,7 @@ static int brcmf_sdio_buscoreprep(void *ctx)
 
 	/* If register supported, wait for ALPAvail and then force ALP */
 	/* This may take up to 15 milliseconds */
-	clkval = brcmf_sdiod_regrb(sdiodev,
+	clkval = brcm_sdio_func1_rb(sdiodev,
 				   SBSDIO_FUNC1_CHIPCLKCSR, NULL);
 
 	if ((clkval & ~SBSDIO_AVBITS) != clkset) {
@@ -3727,7 +3727,7 @@ static int brcmf_sdio_buscoreprep(void *ctx)
 		return -EACCES;
 	}
 
-	SPINWAIT(((clkval = brcmf_sdiod_regrb(sdiodev,
+	SPINWAIT(((clkval = brcm_sdio_func1_rb(sdiodev,
 					      SBSDIO_FUNC1_CHIPCLKCSR, NULL)),
 			!SBSDIO_ALPAV(clkval)),
 			PMU_MAX_TRANSITION_DLY);
@@ -3738,11 +3738,11 @@ static int brcmf_sdio_buscoreprep(void *ctx)
 	}
 
 	clkset = SBSDIO_FORCE_HW_CLKREQ_OFF | SBSDIO_FORCE_ALP;
-	brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR, clkset, &err);
+	brcm_sdio_func1_wb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR, clkset, &err);
 	udelay(65);
 
 	/* Also, disable the extra SDIO pull-ups */
-	brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_SDIOPULLUP, 0, NULL);
+	brcm_sdio_func1_wb(sdiodev, SBSDIO_FUNC1_SDIOPULLUP, 0, NULL);
 
 	return 0;
 }
@@ -3818,10 +3818,10 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
 	 * programs PLL control regs
 	 */
 
-	brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
+	brcm_sdio_func1_wb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
 			  BRCMF_INIT_CLKCTL1, &err);
 	if (!err)
-		clkctl = brcmf_sdiod_regrb(sdiodev,
+		clkctl = brcm_sdio_func1_rb(sdiodev,
 					   SBSDIO_FUNC1_CHIPCLKCSR, &err);
 
 	if (err || ((clkctl & ~SBSDIO_AVBITS) != BRCMF_INIT_CLKCTL1)) {
@@ -3882,13 +3882,13 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
 	brcmf_sdio_drivestrengthinit(sdiodev, bus->ci, drivestrength);
 
 	/* Set card control so an SDIO card reset does a WLAN backplane reset */
-	reg_val = brcmf_sdiod_regrb(sdiodev, SDIO_CCCR_BRCM_CARDCTRL, &err);
+	reg_val = brcm_sdio_func0_rb(sdiodev, SDIO_CCCR_BRCM_CARDCTRL, &err);
 	if (err)
 		goto fail;
 
 	reg_val |= SDIO_CCCR_BRCM_CARDCTRL_WLANRESET;
 
-	brcmf_sdiod_regwb(sdiodev, SDIO_CCCR_BRCM_CARDCTRL, reg_val, &err);
+	brcm_sdio_func0_wb(sdiodev, SDIO_CCCR_BRCM_CARDCTRL, reg_val, &err);
 	if (err)
 		goto fail;
 
@@ -4021,9 +4021,9 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 		goto release;
 
 	/* Force clocks on backplane to be sure F2 interrupt propagates */
-	saveclk = brcmf_sdiod_regrb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR, &err);
+	saveclk = brcm_sdio_func1_rb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR, &err);
 	if (!err) {
-		brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
+		brcm_sdio_func1_wb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
 				  (saveclk | SBSDIO_FORCE_HT), &err);
 	}
 	if (err) {
@@ -4046,7 +4046,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 		w_sdreg32(bus, bus->hostintmask,
 			  offsetof(struct sdpcmd_regs, hostintmask));
 
-		brcmf_sdiod_regwb(sdiodev, SBSDIO_WATERMARK, 8, &err);
+		brcm_sdio_func1_wb(sdiodev, SBSDIO_WATERMARK, 8, &err);
 	} else {
 		/* Disable F2 again */
 		sdio_disable_func(sdiodev->func[SDIO_FUNC_2]);
@@ -4057,7 +4057,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 		brcmf_sdio_sr_init(bus);
 	} else {
 		/* Restore previous clock setting */
-		brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
+		brcm_sdio_func1_wb(sdiodev, SBSDIO_FUNC1_CHIPCLKCSR,
 				  saveclk, &err);
 	}
 
@@ -4198,7 +4198,7 @@ struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev)
 	bus->rxflow = false;
 
 	/* Done with backplane-dependent accesses, can drop clock... */
-	brcmf_sdiod_regwb(bus->sdiodev, SBSDIO_FUNC1_CHIPCLKCSR, 0, NULL);
+	brcm_sdio_func1_wb(bus->sdiodev, SBSDIO_FUNC1_CHIPCLKCSR, 0, NULL);
 
 	sdio_release_host(bus->sdiodev->func[1]);
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
index f3da32fc6360..5220a878bed3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
@@ -50,6 +50,18 @@
 #define SBSDIO_NUM_FUNCTION		3
 
 /* function 0 vendor specific CCCR registers */
+
+#define brcm_sdio_func0_rb(sdiodev, addr, r) \
+	sdio_readb((sdiodev)->func[0], (addr), (r))
+
+#define brcm_sdio_func0_wb(sdiodev, addr, v, ret) \
+	sdio_writeb((sdiodev)->func[0], (v), (addr), (ret))
+
+#define brcm_sdio_func1_rb(sdiodev, addr, r) \
+	sdio_readb((sdiodev)->func[1], (addr), (r))
+
+#define brcm_sdio_func1_wb(sdiodev, addr, v, ret) \
+	sdio_writeb((sdiodev)->func[1], (v), (addr), (ret))
 #define SDIO_CCCR_BRCM_CARDCAP			0xf0
 #define SDIO_CCCR_BRCM_CARDCAP_CMD14_SUPPORT	0x02
 #define SDIO_CCCR_BRCM_CARDCAP_CMD14_EXT	0x04
@@ -131,11 +143,6 @@
 /* with b15, maps to 32-bit SB access */
 #define SBSDIO_SB_ACCESS_2_4B_FLAG	0x08000
 
-/* valid bits in SBSDIO_FUNC1_SBADDRxxx regs */
-
-#define SBSDIO_SBADDRLOW_MASK		0x80	/* Valid bits in SBADDRLOW */
-#define SBSDIO_SBADDRMID_MASK		0xff	/* Valid bits in SBADDRMID */
-#define SBSDIO_SBADDRHIGH_MASK		0xffU	/* Valid bits in SBADDRHIGH */
 /* Address bits from SBADDR regs */
 #define SBSDIO_SBWINDOW_MASK		0xffff8000
 
@@ -342,7 +349,7 @@ int brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 		      u8 *data, uint size);
 
 /* Issue an abort to the specified function */
-int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, uint fn);
+int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, u8 fn);
 void brcmf_sdiod_sgtable_alloc(struct brcmf_sdio_dev *sdiodev);
 void brcmf_sdiod_change_state(struct brcmf_sdio_dev *sdiodev,
 			      enum brcmf_sdiod_state state);
-- 
2.11.0

[PATCH 09/21] brcmfmac: tidy header a bit

[Ian Molton]

---
 .../net/wireless/broadcom/brcm80211/brcmfmac/sdio.h  | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
index 5220a878bed3..d582e39652b9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
@@ -62,17 +62,19 @@
 
 #define brcm_sdio_func1_wb(sdiodev, addr, v, ret) \
 	sdio_writeb((sdiodev)->func[1], (v), (addr), (ret))
-#define SDIO_CCCR_BRCM_CARDCAP			0xf0
-#define SDIO_CCCR_BRCM_CARDCAP_CMD14_SUPPORT	0x02
-#define SDIO_CCCR_BRCM_CARDCAP_CMD14_EXT	0x04
-#define SDIO_CCCR_BRCM_CARDCAP_CMD_NODEC	0x08
+
+#define SDIO_CCCR_BRCM_CARDCAP		0xf0
+#define  SDIO_CCCR_BRCM_CARDCAP_CMD14_SUPPORT	0x02
+#define  SDIO_CCCR_BRCM_CARDCAP_CMD14_EXT	0x04
+#define  SDIO_CCCR_BRCM_CARDCAP_CMD_NODEC	0x08
+
 #define SDIO_CCCR_BRCM_CARDCTRL		0xf1
-#define SDIO_CCCR_BRCM_CARDCTRL_WLANRESET	0x02
-#define SDIO_CCCR_BRCM_SEPINT			0xf2
+#define  SDIO_CCCR_BRCM_CARDCTRL_WLANRESET	0x02
 
-#define  SDIO_SEPINT_MASK		0x01
-#define  SDIO_SEPINT_OE			0x02
-#define  SDIO_SEPINT_ACT_HI		0x04
+#define SDIO_CCCR_BRCM_SEPINT		0xf2
+#define  SDIO_SEPINT_MASK			0x01
+#define  SDIO_SEPINT_OE				0x02
+#define  SDIO_SEPINT_ACT_HI			0x04
 
 /* function 1 miscellaneous registers */
 
-- 
2.11.0

[PATCH 10/21] brcmfmac: Further SDIO addressing cleanup

[Ian Molton]

---
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 39 +++++++++-------------
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 84ae67c11970..5214556465d1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -232,19 +232,23 @@ void brcmf_sdiod_change_state(struct brcmf_sdio_dev *sdiodev,
 }
 
 static int
-brcmf_sdiod_set_sbaddr_window(struct brcmf_sdio_dev *sdiodev, u32 address)
+brcmf_sdiod_set_backplane_window(struct brcmf_sdio_dev *sdiodev, u32 addr)
 {
+	u32 a, bar0 = addr & SBSDIO_SBWINDOW_MASK;
 	int err = 0, i;
-	u32 addr;
 
-	addr = (address & SBSDIO_SBWINDOW_MASK) >> 8;
+	if (bar0 == sdiodev->sbwad)
+		return 0;
 
-	for (i = 0 ; i < 3 && !err ; i++) {
-		u8 data = addr & 0xff;
-		addr = addr >> 8;
 
-		brcm_sdio_func1_wb(sdiodev, SBSDIO_FUNC1_SBADDRLOW + i, data, &err);
-	}
+	a = bar0 >> 8;
+
+	for (i = 0 ; i < 3 && !err ; i++, a>>=8)
+		brcm_sdio_func1_wb(sdiodev, SBSDIO_FUNC1_SBADDRLOW + i, a & 0xff, &err);
+
+
+	if(!err)
+		sdiodev->sbwad = bar0;
 
 	return err;
 }
@@ -257,17 +261,11 @@ brcmf_sdiod_set_sbaddr_window(struct brcmf_sdio_dev *sdiodev, u32 address)
 static int
 brcmf_sdiod_addrprep(struct brcmf_sdio_dev *sdiodev, u32 *addr)
 {
-	uint bar0 = *addr & SBSDIO_SBWINDOW_MASK;
 	int err = 0;
 
-	if (bar0 != sdiodev->sbwad) {
-		printk("WTAF? %08x %08x\n", bar0, sdiodev->sbwad);
-		err = brcmf_sdiod_set_sbaddr_window(sdiodev, bar0);
-		if (err)
-			return err;
-
-		sdiodev->sbwad = bar0;
-	}
+	err = brcmf_sdiod_set_backplane_window(sdiodev, *addr);
+	if (err)
+		return err;
 
 	*addr &= SBSDIO_SB_OFT_ADDR_MASK;
 	*addr |= SBSDIO_SB_ACCESS_2_4B_FLAG;
@@ -699,7 +697,7 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 	while (size) {
 
 		/* Set the backplane window to include the start address */
-		err = brcmf_sdiod_set_sbaddr_window(sdiodev, address);
+		err = brcmf_sdiod_set_backplane_window(sdiodev, address);
 		if (err)
 			break;
 
@@ -743,11 +741,6 @@ brcmf_sdiod_ramrw(struct brcmf_sdio_dev *sdiodev, bool write, u32 address,
 
 	dev_kfree_skb(pkt);
 
-	/* Return the window to backplane enumeration space for core access */
-	if (brcmf_sdiod_set_sbaddr_window(sdiodev, sdiodev->sbwad))
-		brcmf_err("FAILED to set window back to 0x%x\n",
-			  sdiodev->sbwad);
-
 	sdio_release_host(sdiodev->func[1]);
 
 	return err;
-- 
2.11.0

[PATCH 11/21] brcmfmac: cleanup horrid offsetof() mess

[Ian Molton]

---
 .../wireless/broadcom/brcm80211/brcmfmac/sdio.c    | 37 ++++++++--------------
 1 file changed, 14 insertions(+), 23 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index ce540625c29e..2d819d44358a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -159,8 +159,8 @@ struct rte_console {
 /* manfid tuple length, include tuple, link bytes */
 #define SBSDIO_CIS_MANFID_TUPLE_LEN	6
 
-#define CORE_BUS_REG(base, field) \
-		(base + offsetof(struct sdpcmd_regs, field))
+#define __sd_reg(field) \
+		(offsetof(struct sdpcmd_regs, field))
 
 /* SDIO function 1 register CHIPCLKCSR */
 /* Force ALP request to backplane */
@@ -1081,12 +1081,10 @@ static u32 brcmf_sdio_hostmail(struct brcmf_sdio *bus)
 	brcmf_dbg(SDIO, "Enter\n");
 
 	/* Read mailbox data and ack that we did so */
-	ret = r_sdreg32(bus, &hmb_data,
-			offsetof(struct sdpcmd_regs, tohostmailboxdata));
+	ret = r_sdreg32(bus, &hmb_data,	__sd_reg(tohostmailboxdata));
 
 	if (ret == 0)
-		w_sdreg32(bus, SMB_INT_ACK,
-			  offsetof(struct sdpcmd_regs, tosbmailbox));
+		w_sdreg32(bus, SMB_INT_ACK, __sd_reg(tosbmailbox));
 	bus->sdcnt.f1regdata += 2;
 
 	/* Dongle recomposed rx frames, accept them again */
@@ -1196,8 +1194,7 @@ static void brcmf_sdio_rxfail(struct brcmf_sdio *bus, bool abort, bool rtx)
 
 	if (rtx) {
 		bus->sdcnt.rxrtx++;
-		err = w_sdreg32(bus, SMB_NAK,
-				offsetof(struct sdpcmd_regs, tosbmailbox));
+		err = w_sdreg32(bus, SMB_NAK, __sd_reg(tosbmailbox));
 
 		bus->sdcnt.f1regdata++;
 		if (err == 0)
@@ -2318,9 +2315,7 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes)
 		if (!bus->intr) {
 			/* Check device status, signal pending interrupt */
 			sdio_claim_host(bus->sdiodev->func[1]);
-			ret = r_sdreg32(bus, &intstatus,
-					offsetof(struct sdpcmd_regs,
-						 intstatus));
+			ret = r_sdreg32(bus, &intstatus, __sd_reg(intstatus));
 			sdio_release_host(bus->sdiodev->func[1]);
 			bus->sdcnt.f2txdata++;
 			if (ret != 0)
@@ -2426,7 +2421,7 @@ static void brcmf_sdio_bus_stop(struct device *dev)
 		brcmf_sdio_bus_sleep(bus, false, false);
 
 		/* Disable and clear interrupts at the chip level also */
-		w_sdreg32(bus, 0, offsetof(struct sdpcmd_regs, hostintmask));
+		w_sdreg32(bus, 0, __sd_reg(hostintmask));
 		local_hostintmask = bus->hostintmask;
 		bus->hostintmask = 0;
 
@@ -2445,8 +2440,7 @@ static void brcmf_sdio_bus_stop(struct device *dev)
 		sdio_disable_func(sdiodev->func[SDIO_FUNC_2]);
 
 		/* Clear any pending interrupts now that F2 is disabled */
-		w_sdreg32(bus, local_hostintmask,
-			  offsetof(struct sdpcmd_regs, intstatus));
+		w_sdreg32(bus, local_hostintmask, __sd_reg(intstatus));
 
 		sdio_release_host(sdiodev->func[1]);
 	}
@@ -2492,7 +2486,7 @@ static int brcmf_sdio_intr_rstatus(struct brcmf_sdio *bus)
 	int ret;
 
 	buscore = brcmf_chip_get_core(bus->ci, BCMA_CORE_SDIO_DEV);
-	addr = buscore->base + offsetof(struct sdpcmd_regs, intstatus);
+	addr = buscore->base + __sd_reg(intstatus);
 
 	val = brcmf_sdiod_regrl(bus->sdiodev, addr, &ret);
 	bus->sdcnt.f1regdata++;
@@ -2569,11 +2563,9 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 	 */
 	if (intstatus & I_HMB_FC_CHANGE) {
 		intstatus &= ~I_HMB_FC_CHANGE;
-		err = w_sdreg32(bus, I_HMB_FC_CHANGE,
-				offsetof(struct sdpcmd_regs, intstatus));
+		err = w_sdreg32(bus, I_HMB_FC_CHANGE, __sd_reg(intstatus));
 
-		err = r_sdreg32(bus, &newstatus,
-				offsetof(struct sdpcmd_regs, intstatus));
+		err = r_sdreg32(bus, &newstatus, __sd_reg(intstatus));
 		bus->sdcnt.f1regdata += 2;
 		atomic_set(&bus->fcstate,
 			   !!(newstatus & (I_HMB_FC_STATE | I_HMB_FC_CHANGE)));
@@ -3756,7 +3748,7 @@ static void brcmf_sdio_buscore_activate(void *ctx, struct brcmf_chip *chip,
 
 	/* clear all interrupts */
 	core = brcmf_chip_get_core(chip, BCMA_CORE_SDIO_DEV);
-	reg_addr = core->base + offsetof(struct sdpcmd_regs, intstatus);
+	reg_addr = core->base + __sd_reg(intstatus);
 	brcmf_sdiod_regwl(sdiodev, reg_addr, 0xFFFFFFFF, NULL);
 
 	if (rstvec)
@@ -4033,7 +4025,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 
 	/* Enable function 2 (frame transfers) */
 	w_sdreg32(bus, SDPCM_PROT_VERSION << SMB_DATA_VERSION_SHIFT,
-		  offsetof(struct sdpcmd_regs, tosbmailboxdata));
+		  __sd_reg(tosbmailboxdata));
 	err = sdio_enable_func(sdiodev->func[SDIO_FUNC_2]);
 
 
@@ -4043,8 +4035,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 	if (!err) {
 		/* Set up the interrupt mask and enable interrupts */
 		bus->hostintmask = HOSTINTMASK;
-		w_sdreg32(bus, bus->hostintmask,
-			  offsetof(struct sdpcmd_regs, hostintmask));
+		w_sdreg32(bus, bus->hostintmask, __sd_reg(hostintmask));
 
 		brcm_sdio_func1_wb(sdiodev, SBSDIO_WATERMARK, 8, &err);
 	} else {
-- 
2.11.0

[PATCH 12/21] brcmfmac: Fix awfully named #define and crap multi-stage if...elseif clause.

[Ian Molton]

---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c    | 11 +++++++----
 drivers/net/wireless/broadcom/brcm80211/brcmsmac/aiutils.h |  3 ++-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index 05f22ff81d60..7b8ba7192e1a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -30,7 +30,7 @@
 
 /* SOC Interconnect types (aka chip types) */
 #define SOCI_SB		0
-#define SOCI_AI		1
+#define SOCI_AXI	1
 
 /* PL-368 DMP definitions */
 #define DMP_DESC_TYPE_MSK	0x0000000F
@@ -927,7 +927,8 @@ static int brcmf_chip_recognition(struct brcmf_chip_priv *ci)
 		  socitype == SOCI_SB ? "SB" : "AXI", ci->pub.name,
 		  ci->pub.chiprev);
 
-	if (socitype == SOCI_SB) {
+	switch(socitype) {
+		case SOCI_SB:
 		if (ci->pub.chip != BRCM_CC_4329_CHIP_ID) {
 			brcmf_err("SB chip is not supported\n");
 			return -ENODEV;
@@ -951,13 +952,15 @@ static int brcmf_chip_recognition(struct brcmf_chip_priv *ci)
 
 		core = brcmf_chip_add_core(ci, BCMA_CORE_80211, 0x18001000, 0);
 		brcmf_chip_sb_corerev(ci, core);
-	} else if (socitype == SOCI_AI) {
+		break;
+	case SOCI_AXI:
 		ci->iscoreup = brcmf_chip_ai_iscoreup;
 		ci->coredisable = brcmf_chip_ai_coredisable;
 		ci->resetcore = brcmf_chip_ai_resetcore;
 
 		brcmf_chip_dmp_erom_scan(ci);
-	} else {
+		break;
+	default:
 		brcmf_err("chip backplane type %u is not supported\n",
 			  socitype);
 		return -ENODEV;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/aiutils.h b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/aiutils.h
index 2d08c155c23b..58f2d6e01f13 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/aiutils.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/aiutils.h
@@ -44,7 +44,8 @@
 #define	SI_CC_IDX		0
 
 /* SOC Interconnect types (aka chip types) */
-#define	SOCI_AI			1
+//FIXME - this is multiply defined!
+#define	SOCI_AXI			1
 
 /* A register that is common to all cores to
  * communicate w/PMU regarding clock control.
-- 
2.11.0

[PATCH 13/21] brcmfmac: HACK - stabilise the value of ->sbwad in use for some xfer routines.

[Ian Molton]

We probably need to look up the value in future, but this has to be better for now.
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 5214556465d1..5324068a945d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -59,6 +59,11 @@
 
 #define BRCMF_DEFAULT_RXGLOM_SIZE	32  /* max rx frames in glom chain */
 
+/* This is a huge hack - we should look up this value and refer to that - who knows if it varies from chip to chip...
+ * however this has got to be better than just letting ->sbwad dangle after the last bit of IO during setup
+ */
+#define FIXME_CHIPCOMMON_BASE 0x18000000
+
 struct brcmf_sdiod_freezer {
 	atomic_t freezing;
 	atomic_t thread_count;
@@ -557,7 +562,7 @@ int brcmf_sdiod_recv_buf(struct brcmf_sdio_dev *sdiodev, u8 *buf, uint nbytes)
 
 int brcmf_sdiod_recv_pkt(struct brcmf_sdio_dev *sdiodev, struct sk_buff *pkt)
 {
-	u32 addr = sdiodev->sbwad;
+	u32 addr = FIXME_CHIPCOMMON_BASE;
 	int err = 0;
 
 	brcmf_dbg(SDIO, "addr = 0x%x, size = %d\n", addr, pkt->len);
@@ -577,7 +582,7 @@ int brcmf_sdiod_recv_chain(struct brcmf_sdio_dev *sdiodev,
 {
 	struct sk_buff *glom_skb;
 	struct sk_buff *skb;
-	u32 addr = sdiodev->sbwad;
+	u32 addr = FIXME_CHIPCOMMON_BASE;
 	int err = 0;
 
 	brcmf_dbg(SDIO, "addr = 0x%x, size = %d\n",
@@ -616,7 +621,7 @@ int brcmf_sdiod_recv_chain(struct brcmf_sdio_dev *sdiodev,
 int brcmf_sdiod_send_buf(struct brcmf_sdio_dev *sdiodev, u8 *buf, uint nbytes)
 {
 	struct sk_buff *mypkt;
-	u32 addr = sdiodev->sbwad;
+	u32 addr = FIXME_CHIPCOMMON_BASE;
 	int err;
 
 	mypkt = brcmu_pkt_buf_get_skb(nbytes);
@@ -644,7 +649,7 @@ int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev,
 			 struct sk_buff_head *pktq)
 {
 	struct sk_buff *skb;
-	u32 addr = sdiodev->sbwad;
+	u32 addr = FIXME_CHIPCOMMON_BASE;
 	int err;
 
 	brcmf_dbg(SDIO, "addr = 0x%x, size = %d\n", addr, pktq->qlen);
-- 
2.11.0

[PATCH 14/21] brcmfmac: Get rid of hideous chip_priv and core_priv structs

[Ian Molton]

There is zero need to keep these structures separate, they are *always*
allocated together. All they do is cause painful container_of()
shenanigans throughout the code, whilst offering zero real safety.
---
 .../wireless/broadcom/brcm80211/brcmfmac/chip.c    | 431 +++++++++++----------
 .../wireless/broadcom/brcm80211/brcmfmac/chip.h    |  22 ++
 2 files changed, 239 insertions(+), 214 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index 7b8ba7192e1a..38794d7c1da8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -223,99 +223,89 @@ struct sbsocramregs {
 #define	ARMCR4_BSZ_MASK		0x3f
 #define	ARMCR4_BSZ_MULT		8192
 
-struct brcmf_core_priv {
-	struct brcmf_core pub;
-	u32 wrapbase;
-	struct list_head list;
-	struct brcmf_chip_priv *chip;
-};
-
-struct brcmf_chip_priv {
-	struct brcmf_chip pub;
-	const struct brcmf_buscore_ops *ops;
-	void *ctx;
-	/* assured first core is chipcommon, second core is buscore */
-	struct list_head cores;
-	u16 num_cores;
-
-	bool (*iscoreup)(struct brcmf_core_priv *core);
-	void (*coredisable)(struct brcmf_core_priv *core, u32 prereset,
-			    u32 reset);
-	void (*resetcore)(struct brcmf_core_priv *core, u32 prereset, u32 reset,
-			  u32 postreset);
-};
 
-static void brcmf_chip_sb_corerev(struct brcmf_chip_priv *ci,
+static void brcmf_chip_sb_corerev(struct brcmf_chip *ci,
 				  struct brcmf_core *core)
 {
 	u32 regdata;
 
 	regdata = ci->ops->read32(ci->ctx, CORE_SB(core->base, sbidhigh));
+
 	core->rev = SBCOREREV(regdata);
 }
 
-static bool brcmf_chip_sb_iscoreup(struct brcmf_core_priv *core)
+static bool brcmf_chip_sb_iscoreup(struct brcmf_core *core)
 {
-	struct brcmf_chip_priv *ci;
+	struct brcmf_chip *ci = core->chip;
 	u32 regdata;
 	u32 address;
 
-	ci = core->chip;
-	address = CORE_SB(core->pub.base, sbtmstatelow);
+	address = CORE_SB(core->base, sbtmstatelow);
+
 	regdata = ci->ops->read32(ci->ctx, address);
+
 	regdata &= (SSB_TMSLOW_RESET | SSB_TMSLOW_REJECT |
 		    SSB_IMSTATE_REJECT | SSB_TMSLOW_CLOCK);
+
 	return SSB_TMSLOW_CLOCK == regdata;
 }
 
-static bool brcmf_chip_ai_iscoreup(struct brcmf_core_priv *core)
+static bool brcmf_chip_ai_iscoreup(struct brcmf_core *core)
 {
-	struct brcmf_chip_priv *ci;
+	struct brcmf_chip *ci = core->chip;
 	u32 regdata;
 	bool ret;
 
-	ci = core->chip;
 	regdata = ci->ops->read32(ci->ctx, core->wrapbase + BCMA_IOCTL);
+
 	ret = (regdata & (BCMA_IOCTL_FGC | BCMA_IOCTL_CLK)) == BCMA_IOCTL_CLK;
 
 	regdata = ci->ops->read32(ci->ctx, core->wrapbase + BCMA_RESET_CTL);
+
 	ret = ret && ((regdata & BCMA_RESET_CTL_RESET) == 0);
 
 	return ret;
 }
 
-static void brcmf_chip_sb_coredisable(struct brcmf_core_priv *core,
+static void brcmf_chip_sb_coredisable(struct brcmf_core *core,
 				      u32 prereset, u32 reset)
 {
-	struct brcmf_chip_priv *ci;
+	struct brcmf_chip *ci = core->chip;
 	u32 val, base;
 
-	ci = core->chip;
-	base = core->pub.base;
+	base = core->base;
+
 	val = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+
 	if (val & SSB_TMSLOW_RESET)
 		return;
 
 	val = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+
 	if ((val & SSB_TMSLOW_CLOCK) != 0) {
 		/*
 		 * set target reject and spin until busy is clear
 		 * (preserve core-specific bits)
 		 */
 		val = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+
 		ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatelow),
 					 val | SSB_TMSLOW_REJECT);
 
 		val = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+
 		udelay(1);
+
 		SPINWAIT((ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatehigh))
 			  & SSB_TMSHIGH_BUSY), 100000);
 
 		val = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatehigh));
+
 		if (val & SSB_TMSHIGH_BUSY)
 			brcmf_err("core state still busy\n");
 
 		val = ci->ops->read32(ci->ctx, CORE_SB(base, sbidlow));
+
 		if (val & SSB_IDLOW_INITIATOR) {
 			val = ci->ops->read32(ci->ctx,
 					      CORE_SB(base, sbimstate));
@@ -333,16 +323,22 @@ static void brcmf_chip_sb_coredisable(struct brcmf_core_priv *core,
 		/* set reset and reject while enabling the clocks */
 		val = SSB_TMSLOW_FGC | SSB_TMSLOW_CLOCK |
 		      SSB_TMSLOW_REJECT | SSB_TMSLOW_RESET;
+
 		ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatelow), val);
+
 		val = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+
 		udelay(10);
 
 		/* clear the initiator reject bit */
 		val = ci->ops->read32(ci->ctx, CORE_SB(base, sbidlow));
+
 		if (val & SSB_IDLOW_INITIATOR) {
 			val = ci->ops->read32(ci->ctx,
 					      CORE_SB(base, sbimstate));
+
 			val &= ~SSB_IMSTATE_REJECT;
+
 			ci->ops->write32(ci->ctx,
 					 CORE_SB(base, sbimstate), val);
 		}
@@ -351,30 +347,32 @@ static void brcmf_chip_sb_coredisable(struct brcmf_core_priv *core,
 	/* leave reset and reject asserted */
 	ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatelow),
 			 (SSB_TMSLOW_REJECT | SSB_TMSLOW_RESET));
+
 	udelay(1);
 }
 
-static void brcmf_chip_ai_coredisable(struct brcmf_core_priv *core,
+static void brcmf_chip_ai_coredisable(struct brcmf_core *core,
 				      u32 prereset, u32 reset)
 {
-	struct brcmf_chip_priv *ci;
+	struct brcmf_chip *ci = core->chip;
 	u32 regdata;
 
-	ci = core->chip;
-
 	/* if core is already in reset, skip reset */
 	regdata = ci->ops->read32(ci->ctx, core->wrapbase + BCMA_RESET_CTL);
+
 	if ((regdata & BCMA_RESET_CTL_RESET) != 0)
 		goto in_reset_configure;
 
 	/* configure reset */
 	ci->ops->write32(ci->ctx, core->wrapbase + BCMA_IOCTL,
 			 prereset | BCMA_IOCTL_FGC | BCMA_IOCTL_CLK);
+
 	ci->ops->read32(ci->ctx, core->wrapbase + BCMA_IOCTL);
 
 	/* put in reset */
 	ci->ops->write32(ci->ctx, core->wrapbase + BCMA_RESET_CTL,
 			 BCMA_RESET_CTL_RESET);
+
 	usleep_range(10, 20);
 
 	/* wait till reset is 1 */
@@ -385,18 +383,17 @@ static void brcmf_chip_ai_coredisable(struct brcmf_core_priv *core,
 	/* in-reset configure */
 	ci->ops->write32(ci->ctx, core->wrapbase + BCMA_IOCTL,
 			 reset | BCMA_IOCTL_FGC | BCMA_IOCTL_CLK);
+
 	ci->ops->read32(ci->ctx, core->wrapbase + BCMA_IOCTL);
 }
 
-static void brcmf_chip_sb_resetcore(struct brcmf_core_priv *core, u32 prereset,
+static void brcmf_chip_sb_resetcore(struct brcmf_core *core, u32 prereset,
 				    u32 reset, u32 postreset)
 {
-	struct brcmf_chip_priv *ci;
+	struct brcmf_chip *ci = core->chip;
+	u32 base = core->base;
 	u32 regdata;
-	u32 base;
 
-	ci = core->chip;
-	base = core->pub.base;
 	/*
 	 * Must do the disable sequence first to work for
 	 * arbitrary current core state.
@@ -411,15 +408,19 @@ static void brcmf_chip_sb_resetcore(struct brcmf_core_priv *core, u32 prereset,
 	ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatelow),
 			 SSB_TMSLOW_FGC | SSB_TMSLOW_CLOCK |
 			 SSB_TMSLOW_RESET);
+
 	regdata = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+
 	udelay(1);
 
 	/* clear any serror */
 	regdata = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatehigh));
+
 	if (regdata & SSB_TMSHIGH_SERR)
 		ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatehigh), 0);
 
 	regdata = ci->ops->read32(ci->ctx, CORE_SB(base, sbimstate));
+
 	if (regdata & (SSB_IMSTATE_IBE | SSB_IMSTATE_TO)) {
 		regdata &= ~(SSB_IMSTATE_IBE | SSB_IMSTATE_TO);
 		ci->ops->write32(ci->ctx, CORE_SB(base, sbimstate), regdata);
@@ -428,39 +429,47 @@ static void brcmf_chip_sb_resetcore(struct brcmf_core_priv *core, u32 prereset,
 	/* clear reset and allow it to propagate throughout the core */
 	ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatelow),
 			 SSB_TMSLOW_FGC | SSB_TMSLOW_CLOCK);
+
 	regdata = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+
 	udelay(1);
 
 	/* leave clock enabled */
 	ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatelow),
 			 SSB_TMSLOW_CLOCK);
+
 	regdata = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+
 	udelay(1);
 }
 
-static void brcmf_chip_ai_resetcore(struct brcmf_core_priv *core, u32 prereset,
+static void brcmf_chip_ai_resetcore(struct brcmf_core *core, u32 prereset,
 				    u32 reset, u32 postreset)
 {
-	struct brcmf_chip_priv *ci;
+	struct brcmf_chip *ci = core->chip;
 	int count;
 
-	ci = core->chip;
-
 	/* must disable first to work for arbitrary current core state */
 	brcmf_chip_ai_coredisable(core, prereset, reset);
 
 	count = 0;
+
 	while (ci->ops->read32(ci->ctx, core->wrapbase + BCMA_RESET_CTL) &
 	       BCMA_RESET_CTL_RESET) {
+
 		ci->ops->write32(ci->ctx, core->wrapbase + BCMA_RESET_CTL, 0);
+
 		count++;
+
 		if (count > 50)
 			break;
+
 		usleep_range(40, 60);
 	}
 
 	ci->ops->write32(ci->ctx, core->wrapbase + BCMA_IOCTL,
 			 postreset | BCMA_IOCTL_CLK);
+
 	ci->ops->read32(ci->ctx, core->wrapbase + BCMA_IOCTL);
 }
 
@@ -473,29 +482,30 @@ static char *brcmf_chip_name(uint chipid, char *buf, uint len)
 	return buf;
 }
 
-static struct brcmf_core *brcmf_chip_add_core(struct brcmf_chip_priv *ci,
+static struct brcmf_core *brcmf_chip_add_core(struct brcmf_chip *ci,
 					      u16 coreid, u32 base,
 					      u32 wrapbase)
 {
-	struct brcmf_core_priv *core;
+	struct brcmf_core *core;
 
 	core = kzalloc(sizeof(*core), GFP_KERNEL);
 	if (!core)
 		return ERR_PTR(-ENOMEM);
 
-	core->pub.id = coreid;
-	core->pub.base = base;
+	core->id = coreid;
+	core->base = base;
 	core->chip = ci;
 	core->wrapbase = wrapbase;
 
 	list_add_tail(&core->list, &ci->cores);
-	return &core->pub;
+
+	return core;
 }
 
 /* safety check for chipinfo */
-static int brcmf_chip_cores_check(struct brcmf_chip_priv *ci)
+static int brcmf_chip_cores_check(struct brcmf_chip *ci)
 {
-	struct brcmf_core_priv *core;
+	struct brcmf_core *core;
 	bool need_socram = false;
 	bool has_socram = false;
 	bool cpu_found = false;
@@ -503,10 +513,10 @@ static int brcmf_chip_cores_check(struct brcmf_chip_priv *ci)
 
 	list_for_each_entry(core, &ci->cores, list) {
 		brcmf_dbg(INFO, " [%-2d] core 0x%x:%-2d base 0x%08x wrap 0x%08x\n",
-			  idx++, core->pub.id, core->pub.rev, core->pub.base,
+			  idx++, core->id, core->rev, core->base,
 			  core->wrapbase);
 
-		switch (core->pub.id) {
+		switch (core->id) {
 		case BCMA_CORE_ARM_CM3:
 			cpu_found = true;
 			need_socram = true;
@@ -529,26 +539,28 @@ static int brcmf_chip_cores_check(struct brcmf_chip_priv *ci)
 		brcmf_err("CPU core not detected\n");
 		return -ENXIO;
 	}
+
 	/* check RAM core presence for ARM CM3 core */
 	if (need_socram && !has_socram) {
 		brcmf_err("RAM core not provided with ARM CM3 core\n");
 		return -ENODEV;
 	}
+
 	return 0;
 }
 
-static u32 brcmf_chip_core_read32(struct brcmf_core_priv *core, u16 reg)
+static u32 brcmf_chip_core_read32(struct brcmf_core *core, u16 reg)
 {
-	return core->chip->ops->read32(core->chip->ctx, core->pub.base + reg);
+	return core->chip->ops->read32(core->chip->ctx, core->base + reg);
 }
 
-static void brcmf_chip_core_write32(struct brcmf_core_priv *core,
+static void brcmf_chip_core_write32(struct brcmf_core *core,
 				    u16 reg, u32 val)
 {
-	core->chip->ops->write32(core->chip->ctx, core->pub.base + reg, val);
+	core->chip->ops->write32(core->chip->ctx, core->base + reg, val);
 }
 
-static bool brcmf_chip_socram_banksize(struct brcmf_core_priv *core, u8 idx,
+static bool brcmf_chip_socram_banksize(struct brcmf_core *core, u8 idx,
 				       u32 *banksize)
 {
 	u32 bankinfo;
@@ -562,7 +574,7 @@ static bool brcmf_chip_socram_banksize(struct brcmf_core_priv *core, u8 idx,
 	return !!(bankinfo & SOCRAM_BANKINFO_RETNTRAM_MASK);
 }
 
-static void brcmf_chip_socram_ramsize(struct brcmf_core_priv *sr, u32 *ramsize,
+static void brcmf_chip_socram_ramsize(struct brcmf_core *sr, u32 *ramsize,
 				      u32 *srsize)
 {
 	u32 coreinfo;
@@ -573,17 +585,17 @@ static void brcmf_chip_socram_ramsize(struct brcmf_core_priv *sr, u32 *ramsize,
 	*ramsize = 0;
 	*srsize = 0;
 
-	if (WARN_ON(sr->pub.rev < 4))
+	if (WARN_ON(sr->rev < 4))
 		return;
 
-	if (!brcmf_chip_iscoreup(&sr->pub))
-		brcmf_chip_resetcore(&sr->pub, 0, 0, 0);
+	if (!brcmf_chip_iscoreup(sr))
+		brcmf_chip_resetcore(sr, 0, 0, 0);
 
 	/* Get info for determining size */
 	coreinfo = brcmf_chip_core_read32(sr, SOCRAMREGOFFS(coreinfo));
 	nb = (coreinfo & SRCI_SRNB_MASK) >> SRCI_SRNB_SHIFT;
 
-	if ((sr->pub.rev <= 7) || (sr->pub.rev == 12)) {
+	if ((sr->rev <= 7) || (sr->rev == 12)) {
 		banksize = (coreinfo & SRCI_SRBSZ_MASK);
 		lss = (coreinfo & SRCI_LSS_MASK) >> SRCI_LSS_SHIFT;
 		if (lss != 0)
@@ -602,9 +614,9 @@ static void brcmf_chip_socram_ramsize(struct brcmf_core_priv *sr, u32 *ramsize,
 	}
 
 	/* hardcoded save&restore memory sizes */
-	switch (sr->chip->pub.chip) {
+	switch (sr->chip->chip) {
 	case BRCM_CC_4334_CHIP_ID:
-		if (sr->chip->pub.chiprev < 2)
+		if (sr->chip->chiprev < 2)
 			*srsize = (32 * 1024);
 		break;
 	case BRCM_CC_43430_CHIP_ID:
@@ -619,7 +631,7 @@ static void brcmf_chip_socram_ramsize(struct brcmf_core_priv *sr, u32 *ramsize,
 }
 
 /** Return the SYS MEM size */
-static u32 brcmf_chip_sysmem_ramsize(struct brcmf_core_priv *sysmem)
+static u32 brcmf_chip_sysmem_ramsize(struct brcmf_core *sysmem)
 {
 	u32 memsize = 0;
 	u32 coreinfo;
@@ -627,8 +639,8 @@ static u32 brcmf_chip_sysmem_ramsize(struct brcmf_core_priv *sysmem)
 	u32 nb;
 	u32 banksize;
 
-	if (!brcmf_chip_iscoreup(&sysmem->pub))
-		brcmf_chip_resetcore(&sysmem->pub, 0, 0, 0);
+	if (!brcmf_chip_iscoreup(sysmem))
+		brcmf_chip_resetcore(sysmem, 0, 0, 0);
 
 	coreinfo = brcmf_chip_core_read32(sysmem, SYSMEMREGOFFS(coreinfo));
 	nb = (coreinfo & SRCI_SRNB_MASK) >> SRCI_SRNB_SHIFT;
@@ -642,7 +654,7 @@ static u32 brcmf_chip_sysmem_ramsize(struct brcmf_core_priv *sysmem)
 }
 
 /** Return the TCM-RAM size of the ARMCR4 core. */
-static u32 brcmf_chip_tcm_ramsize(struct brcmf_core_priv *cr4)
+static u32 brcmf_chip_tcm_ramsize(struct brcmf_core *cr4)
 {
 	u32 corecap;
 	u32 memsize = 0;
@@ -667,9 +679,9 @@ static u32 brcmf_chip_tcm_ramsize(struct brcmf_core_priv *cr4)
 	return memsize;
 }
 
-static u32 brcmf_chip_tcm_rambase(struct brcmf_chip_priv *ci)
+static u32 brcmf_chip_tcm_rambase(struct brcmf_chip *ci)
 {
-	switch (ci->pub.chip) {
+	switch (ci->chip) {
 	case BRCM_CC_4345_CHIP_ID:
 		return 0x198000;
 	case BRCM_CC_4335_CHIP_ID:
@@ -691,60 +703,53 @@ static u32 brcmf_chip_tcm_rambase(struct brcmf_chip_priv *ci)
 	case BRCM_CC_4366_CHIP_ID:
 		return 0x200000;
 	default:
-		brcmf_err("unknown chip: %s\n", ci->pub.name);
+		brcmf_err("unknown chip: %s\n", ci->name);
 		break;
 	}
 	return 0;
 }
 
-static int brcmf_chip_get_raminfo(struct brcmf_chip_priv *ci)
+static int brcmf_chip_get_raminfo(struct brcmf_chip *ci)
 {
-	struct brcmf_core_priv *mem_core;
 	struct brcmf_core *mem;
 
-	mem = brcmf_chip_get_core(&ci->pub, BCMA_CORE_ARM_CR4);
+	mem = brcmf_chip_get_core(ci, BCMA_CORE_ARM_CR4);
 	if (mem) {
-		mem_core = container_of(mem, struct brcmf_core_priv, pub);
-		ci->pub.ramsize = brcmf_chip_tcm_ramsize(mem_core);
-		ci->pub.rambase = brcmf_chip_tcm_rambase(ci);
-		if (!ci->pub.rambase) {
+		ci->ramsize = brcmf_chip_tcm_ramsize(mem);
+		ci->rambase = brcmf_chip_tcm_rambase(ci);
+		if (!ci->rambase) {
 			brcmf_err("RAM base not provided with ARM CR4 core\n");
 			return -EINVAL;
 		}
 	} else {
-		mem = brcmf_chip_get_core(&ci->pub, BCMA_CORE_SYS_MEM);
+		mem = brcmf_chip_get_core(ci, BCMA_CORE_SYS_MEM);
 		if (mem) {
-			mem_core = container_of(mem, struct brcmf_core_priv,
-						pub);
-			ci->pub.ramsize = brcmf_chip_sysmem_ramsize(mem_core);
-			ci->pub.rambase = brcmf_chip_tcm_rambase(ci);
-			if (!ci->pub.rambase) {
+			ci->ramsize = brcmf_chip_sysmem_ramsize(mem);
+			ci->rambase = brcmf_chip_tcm_rambase(ci);
+			if (!ci->rambase) {
 				brcmf_err("RAM base not provided with ARM CA7 core\n");
 				return -EINVAL;
 			}
 		} else {
-			mem = brcmf_chip_get_core(&ci->pub,
-						  BCMA_CORE_INTERNAL_MEM);
+			mem = brcmf_chip_get_core(ci, BCMA_CORE_INTERNAL_MEM);
 			if (!mem) {
 				brcmf_err("No memory cores found\n");
 				return -ENOMEM;
 			}
-			mem_core = container_of(mem, struct brcmf_core_priv,
-						pub);
-			brcmf_chip_socram_ramsize(mem_core, &ci->pub.ramsize,
-						  &ci->pub.srsize);
+			brcmf_chip_socram_ramsize(mem, &ci->ramsize,
+						  &ci->srsize);
 		}
 	}
 	brcmf_dbg(INFO, "RAM: base=0x%x size=%d (0x%x) sr=%d (0x%x)\n",
-		  ci->pub.rambase, ci->pub.ramsize, ci->pub.ramsize,
-		  ci->pub.srsize, ci->pub.srsize);
+		  ci->rambase, ci->ramsize, ci->ramsize,
+		  ci->srsize, ci->srsize);
 
-	if (!ci->pub.ramsize) {
+	if (!ci->ramsize) {
 		brcmf_err("RAM size is undetermined\n");
 		return -ENOMEM;
 	}
 
-	if (ci->pub.ramsize > BRCMF_CHIP_MAX_MEMSIZE) {
+	if (ci->ramsize > BRCMF_CHIP_MAX_MEMSIZE) {
 		brcmf_err("RAM size is incorrect\n");
 		return -ENOMEM;
 	}
@@ -752,7 +757,7 @@ static int brcmf_chip_get_raminfo(struct brcmf_chip_priv *ci)
 	return 0;
 }
 
-static u32 brcmf_chip_dmp_get_desc(struct brcmf_chip_priv *ci, u32 *eromaddr,
+static u32 brcmf_chip_dmp_get_desc(struct brcmf_chip *ci, u32 *eromaddr,
 				   u8 *type)
 {
 	u32 val;
@@ -772,7 +777,7 @@ static u32 brcmf_chip_dmp_get_desc(struct brcmf_chip_priv *ci, u32 *eromaddr,
 	return val;
 }
 
-static int brcmf_chip_dmp_get_regaddr(struct brcmf_chip_priv *ci, u32 *eromaddr,
+static int brcmf_chip_dmp_get_regaddr(struct brcmf_chip *ci, u32 *eromaddr,
 				      u32 *regbase, u32 *wrapbase)
 {
 	u8 desc;
@@ -845,7 +850,7 @@ static int brcmf_chip_dmp_get_regaddr(struct brcmf_chip_priv *ci, u32 *eromaddr,
 }
 
 static
-int brcmf_chip_dmp_erom_scan(struct brcmf_chip_priv *ci)
+int brcmf_chip_dmp_erom_scan(struct brcmf_chip *ci)
 {
 	struct brcmf_core *core;
 	u32 eromaddr;
@@ -905,7 +910,7 @@ int brcmf_chip_dmp_erom_scan(struct brcmf_chip_priv *ci)
 	return 0;
 }
 
-static int brcmf_chip_recognition(struct brcmf_chip_priv *ci)
+static int brcmf_chip_recognition(struct brcmf_chip *ci)
 {
 	struct brcmf_core *core;
 	u32 regdata;
@@ -918,21 +923,24 @@ static int brcmf_chip_recognition(struct brcmf_chip_priv *ci)
 	 * other ways of recognition should be added here.
 	 */
 	regdata = ci->ops->read32(ci->ctx, CORE_CC_REG(SI_ENUM_BASE, chipid));
-	ci->pub.chip = regdata & CID_ID_MASK;
-	ci->pub.chiprev = (regdata & CID_REV_MASK) >> CID_REV_SHIFT;
+	ci->chip = regdata & CID_ID_MASK;
+	ci->chiprev = (regdata & CID_REV_MASK) >> CID_REV_SHIFT;
 	socitype = (regdata & CID_TYPE_MASK) >> CID_TYPE_SHIFT;
 
-	brcmf_chip_name(ci->pub.chip, ci->pub.name, sizeof(ci->pub.name));
-	brcmf_dbg(INFO, "found %s chip: BCM%s, rev=%d\n",
-		  socitype == SOCI_SB ? "SB" : "AXI", ci->pub.name,
-		  ci->pub.chiprev);
+	brcmf_chip_name(ci->chip, ci->name, sizeof(ci->name));
+
+	printk("found %s chip: BCM%s, rev=%d\n",
+		  socitype == SOCI_SB ? "SB" : "AXI", ci->name,
+		  ci->chiprev);
 
 	switch(socitype) {
 		case SOCI_SB:
-		if (ci->pub.chip != BRCM_CC_4329_CHIP_ID) {
+
+		if (ci->chip != BRCM_CC_4329_CHIP_ID) {
 			brcmf_err("SB chip is not supported\n");
 			return -ENODEV;
 		}
+
 		ci->iscoreup = brcmf_chip_sb_iscoreup;
 		ci->coredisable = brcmf_chip_sb_coredisable;
 		ci->resetcore = brcmf_chip_sb_resetcore;
@@ -940,25 +948,31 @@ static int brcmf_chip_recognition(struct brcmf_chip_priv *ci)
 		core = brcmf_chip_add_core(ci, BCMA_CORE_CHIPCOMMON,
 					   SI_ENUM_BASE, 0);
 		brcmf_chip_sb_corerev(ci, core);
+
 		core = brcmf_chip_add_core(ci, BCMA_CORE_SDIO_DEV,
 					   BCM4329_CORE_BUS_BASE, 0);
 		brcmf_chip_sb_corerev(ci, core);
+
 		core = brcmf_chip_add_core(ci, BCMA_CORE_INTERNAL_MEM,
 					   BCM4329_CORE_SOCRAM_BASE, 0);
 		brcmf_chip_sb_corerev(ci, core);
+
 		core = brcmf_chip_add_core(ci, BCMA_CORE_ARM_CM3,
 					   BCM4329_CORE_ARM_BASE, 0);
 		brcmf_chip_sb_corerev(ci, core);
 
 		core = brcmf_chip_add_core(ci, BCMA_CORE_80211, 0x18001000, 0);
 		brcmf_chip_sb_corerev(ci, core);
+
 		break;
 	case SOCI_AXI:
+
 		ci->iscoreup = brcmf_chip_ai_iscoreup;
 		ci->coredisable = brcmf_chip_ai_coredisable;
 		ci->resetcore = brcmf_chip_ai_resetcore;
 
 		brcmf_chip_dmp_erom_scan(ci);
+
 		break;
 	default:
 		brcmf_err("chip backplane type %u is not supported\n",
@@ -971,42 +985,40 @@ static int brcmf_chip_recognition(struct brcmf_chip_priv *ci)
 		return ret;
 
 	/* assure chip is passive for core access */
-	brcmf_chip_set_passive(&ci->pub);
+	brcmf_chip_set_passive(ci);
 
 	/* Call bus specific reset function now. Cores have been determined
 	 * but further access may require a chip specific reset at this point.
 	 */
 	if (ci->ops->reset) {
-		ci->ops->reset(ci->ctx, &ci->pub);
-		brcmf_chip_set_passive(&ci->pub);
+		ci->ops->reset(ci->ctx, ci);
+		brcmf_chip_set_passive(ci);
 	}
 
 	return brcmf_chip_get_raminfo(ci);
 }
 
-static void brcmf_chip_disable_arm(struct brcmf_chip_priv *chip, u16 id)
+static void brcmf_chip_disable_arm(struct brcmf_chip *chip, u16 id)
 {
-	struct brcmf_core *core;
-	struct brcmf_core_priv *cpu;
+	struct brcmf_core *cpu;
 	u32 val;
 
 
-	core = brcmf_chip_get_core(&chip->pub, id);
-	if (!core)
+	cpu = brcmf_chip_get_core(chip, id);
+	if (!cpu)
 		return;
 
 	switch (id) {
 	case BCMA_CORE_ARM_CM3:
-		brcmf_chip_coredisable(core, 0, 0);
+		brcmf_chip_coredisable(cpu, 0, 0);
 		break;
 	case BCMA_CORE_ARM_CR4:
 	case BCMA_CORE_ARM_CA7:
-		cpu = container_of(core, struct brcmf_core_priv, pub);
 
 		/* clear all IOCTL bits except HALT bit */
 		val = chip->ops->read32(chip->ctx, cpu->wrapbase + BCMA_IOCTL);
 		val &= ARMCR4_BCMA_IOCTL_CPUHALT;
-		brcmf_chip_resetcore(core, val, ARMCR4_BCMA_IOCTL_CPUHALT,
+		brcmf_chip_resetcore(cpu, val, ARMCR4_BCMA_IOCTL_CPUHALT,
 				     ARMCR4_BCMA_IOCTL_CPUHALT);
 		break;
 	default:
@@ -1015,41 +1027,38 @@ static void brcmf_chip_disable_arm(struct brcmf_chip_priv *chip, u16 id)
 	}
 }
 
-static int brcmf_chip_setup(struct brcmf_chip_priv *chip)
+static int brcmf_chip_setup(struct brcmf_chip *chip)
 {
-	struct brcmf_chip *pub;
-	struct brcmf_core_priv *cc;
-	struct brcmf_core *pmu;
+	struct brcmf_core *cc, *pmu;
 	u32 base;
 	u32 val;
 	int ret = 0;
 
-	pub = &chip->pub;
-	cc = list_first_entry(&chip->cores, struct brcmf_core_priv, list);
-	base = cc->pub.base;
+	cc = list_first_entry(&chip->cores, struct brcmf_core, list);
+	base = cc->base;
 
 	/* get chipcommon capabilites */
-	pub->cc_caps = chip->ops->read32(chip->ctx,
+	chip->cc_caps = chip->ops->read32(chip->ctx,
 					 CORE_CC_REG(base, capabilities));
-	pub->cc_caps_ext = chip->ops->read32(chip->ctx,
+	chip->cc_caps_ext = chip->ops->read32(chip->ctx,
 					     CORE_CC_REG(base,
 							 capabilities_ext));
 
 	/* get pmu caps & rev */
-	pmu = brcmf_chip_get_pmu(pub); /* after reading cc_caps_ext */
-	if (pub->cc_caps & CC_CAP_PMU) {
+	pmu = brcmf_chip_get_pmu(chip); /* after reading cc_caps_ext */
+	if (chip->cc_caps & CC_CAP_PMU) {
 		val = chip->ops->read32(chip->ctx,
 					CORE_CC_REG(pmu->base, pmucapabilities));
-		pub->pmurev = val & PCAP_REV_MASK;
-		pub->pmucaps = val;
+		chip->pmurev = val & PCAP_REV_MASK;
+		chip->pmucaps = val;
 	}
 
 	brcmf_dbg(INFO, "ccrev=%d, pmurev=%d, pmucaps=0x%x\n",
-		  cc->pub.rev, pub->pmurev, pub->pmucaps);
+		  cc->rev, chip->pmurev, chip->pmucaps);
 
 	/* execute bus core specific setup */
 	if (chip->ops->setup)
-		ret = chip->ops->setup(chip->ctx, pub);
+		ret = chip->ops->setup(chip->ctx, chip);
 
 	return ret;
 }
@@ -1057,7 +1066,7 @@ static int brcmf_chip_setup(struct brcmf_chip_priv *chip)
 struct brcmf_chip *brcmf_chip_attach(void *ctx,
 				     const struct brcmf_buscore_ops *ops)
 {
-	struct brcmf_chip_priv *chip;
+	struct brcmf_chip *chip;
 	int err = 0;
 
 	if (WARN_ON(!ops->read32))
@@ -1092,20 +1101,18 @@ struct brcmf_chip *brcmf_chip_attach(void *ctx,
 	if (err < 0)
 		goto fail;
 
-	return &chip->pub;
+	return chip;
 
 fail:
-	brcmf_chip_detach(&chip->pub);
+	brcmf_chip_detach(chip);
 	return ERR_PTR(err);
 }
 
-void brcmf_chip_detach(struct brcmf_chip *pub)
+void brcmf_chip_detach(struct brcmf_chip *chip)
 {
-	struct brcmf_chip_priv *chip;
-	struct brcmf_core_priv *core;
-	struct brcmf_core_priv *tmp;
+	struct brcmf_core *core;
+	struct brcmf_core *tmp;
 
-	chip = container_of(pub, struct brcmf_chip_priv, pub);
 	list_for_each_entry_safe(core, tmp, &chip->cores, list) {
 		list_del(&core->list);
 		kfree(core);
@@ -1113,40 +1120,38 @@ void brcmf_chip_detach(struct brcmf_chip *pub)
 	kfree(chip);
 }
 
-struct brcmf_core *brcmf_chip_get_core(struct brcmf_chip *pub, u16 coreid)
+struct brcmf_core *brcmf_chip_get_core(struct brcmf_chip *chip, u16 coreid)
 {
-	struct brcmf_chip_priv *chip;
-	struct brcmf_core_priv *core;
+	struct brcmf_core *core;
 
-	chip = container_of(pub, struct brcmf_chip_priv, pub);
 	list_for_each_entry(core, &chip->cores, list)
-		if (core->pub.id == coreid)
-			return &core->pub;
+		if (core->id == coreid)
+			return core;
 
 	return NULL;
 }
 
-struct brcmf_core *brcmf_chip_get_chipcommon(struct brcmf_chip *pub)
+struct brcmf_core *brcmf_chip_get_chipcommon(struct brcmf_chip *chip)
 {
-	struct brcmf_chip_priv *chip;
-	struct brcmf_core_priv *cc;
-
-	chip = container_of(pub, struct brcmf_chip_priv, pub);
-	cc = list_first_entry(&chip->cores, struct brcmf_core_priv, list);
-	if (WARN_ON(!cc || cc->pub.id != BCMA_CORE_CHIPCOMMON))
-		return brcmf_chip_get_core(pub, BCMA_CORE_CHIPCOMMON);
-	return &cc->pub;
+	struct brcmf_core *cc;
+
+	cc = list_first_entry(&chip->cores, struct brcmf_core, list);
+
+	if (WARN_ON(!cc || cc->id != BCMA_CORE_CHIPCOMMON))
+		return brcmf_chip_get_core(chip, BCMA_CORE_CHIPCOMMON);
+
+	return cc;
 }
 
-struct brcmf_core *brcmf_chip_get_pmu(struct brcmf_chip *pub)
+struct brcmf_core *brcmf_chip_get_pmu(struct brcmf_chip *chip)
 {
-	struct brcmf_core *cc = brcmf_chip_get_chipcommon(pub);
+	struct brcmf_core *cc = brcmf_chip_get_chipcommon(chip);
 	struct brcmf_core *pmu;
 
 	/* See if there is separated PMU core available */
 	if (cc->rev >= 35 &&
-	    pub->cc_caps_ext & BCMA_CC_CAP_EXT_AOB_PRESENT) {
-		pmu = brcmf_chip_get_core(pub, BCMA_CORE_PMU);
+	    chip->cc_caps_ext & BCMA_CC_CAP_EXT_AOB_PRESENT) {
+		pmu = brcmf_chip_get_core(chip, BCMA_CORE_PMU);
 		if (pmu)
 			return pmu;
 	}
@@ -1155,188 +1160,178 @@ struct brcmf_core *brcmf_chip_get_pmu(struct brcmf_chip *pub)
 	return cc;
 }
 
-bool brcmf_chip_iscoreup(struct brcmf_core *pub)
+bool brcmf_chip_iscoreup(struct brcmf_core *core)
 {
-	struct brcmf_core_priv *core;
-
-	core = container_of(pub, struct brcmf_core_priv, pub);
 	return core->chip->iscoreup(core);
 }
 
-void brcmf_chip_coredisable(struct brcmf_core *pub, u32 prereset, u32 reset)
+void brcmf_chip_coredisable(struct brcmf_core *core, u32 prereset, u32 reset)
 {
-	struct brcmf_core_priv *core;
-
-	core = container_of(pub, struct brcmf_core_priv, pub);
 	core->chip->coredisable(core, prereset, reset);
 }
 
-void brcmf_chip_resetcore(struct brcmf_core *pub, u32 prereset, u32 reset,
+void brcmf_chip_resetcore(struct brcmf_core *core, u32 prereset, u32 reset,
 			  u32 postreset)
 {
-	struct brcmf_core_priv *core;
-
-	core = container_of(pub, struct brcmf_core_priv, pub);
 	core->chip->resetcore(core, prereset, reset, postreset);
 }
 
 static void
-brcmf_chip_cm3_set_passive(struct brcmf_chip_priv *chip)
+brcmf_chip_cm3_set_passive(struct brcmf_chip *chip)
 {
 	struct brcmf_core *core;
-	struct brcmf_core_priv *sr;
 
 	brcmf_chip_disable_arm(chip, BCMA_CORE_ARM_CM3);
-	core = brcmf_chip_get_core(&chip->pub, BCMA_CORE_80211);
+
+	core = brcmf_chip_get_core(chip, BCMA_CORE_80211);
 	brcmf_chip_resetcore(core, D11_BCMA_IOCTL_PHYRESET |
 				   D11_BCMA_IOCTL_PHYCLOCKEN,
 			     D11_BCMA_IOCTL_PHYCLOCKEN,
 			     D11_BCMA_IOCTL_PHYCLOCKEN);
-	core = brcmf_chip_get_core(&chip->pub, BCMA_CORE_INTERNAL_MEM);
+
+	core = brcmf_chip_get_core(chip, BCMA_CORE_INTERNAL_MEM);
 	brcmf_chip_resetcore(core, 0, 0, 0);
 
 	/* disable bank #3 remap for this device */
-	if (chip->pub.chip == BRCM_CC_43430_CHIP_ID) {
-		sr = container_of(core, struct brcmf_core_priv, pub);
-		brcmf_chip_core_write32(sr, SOCRAMREGOFFS(bankidx), 3);
-		brcmf_chip_core_write32(sr, SOCRAMREGOFFS(bankpda), 0);
+	if (chip->chip == BRCM_CC_43430_CHIP_ID) {
+		brcmf_chip_core_write32(core, SOCRAMREGOFFS(bankidx), 3);
+		brcmf_chip_core_write32(core, SOCRAMREGOFFS(bankpda), 0);
 	}
 }
 
-static bool brcmf_chip_cm3_set_active(struct brcmf_chip_priv *chip)
+static bool brcmf_chip_cm3_set_active(struct brcmf_chip *chip)
 {
 	struct brcmf_core *core;
 
-	core = brcmf_chip_get_core(&chip->pub, BCMA_CORE_INTERNAL_MEM);
+	core = brcmf_chip_get_core(chip, BCMA_CORE_INTERNAL_MEM);
+
 	if (!brcmf_chip_iscoreup(core)) {
 		brcmf_err("SOCRAM core is down after reset?\n");
 		return false;
 	}
 
-	chip->ops->activate(chip->ctx, &chip->pub, 0);
+	chip->ops->activate(chip->ctx, chip, 0);
 
-	core = brcmf_chip_get_core(&chip->pub, BCMA_CORE_ARM_CM3);
+	core = brcmf_chip_get_core(chip, BCMA_CORE_ARM_CM3);
 	brcmf_chip_resetcore(core, 0, 0, 0);
 
 	return true;
 }
 
 static inline void
-brcmf_chip_cr4_set_passive(struct brcmf_chip_priv *chip)
+brcmf_chip_cr4_set_passive(struct brcmf_chip *chip)
 {
 	struct brcmf_core *core;
 
 	brcmf_chip_disable_arm(chip, BCMA_CORE_ARM_CR4);
 
-	core = brcmf_chip_get_core(&chip->pub, BCMA_CORE_80211);
+	core = brcmf_chip_get_core(chip, BCMA_CORE_80211);
 	brcmf_chip_resetcore(core, D11_BCMA_IOCTL_PHYRESET |
 				   D11_BCMA_IOCTL_PHYCLOCKEN,
 			     D11_BCMA_IOCTL_PHYCLOCKEN,
 			     D11_BCMA_IOCTL_PHYCLOCKEN);
 }
 
-static bool brcmf_chip_cr4_set_active(struct brcmf_chip_priv *chip, u32 rstvec)
+static bool brcmf_chip_cr4_set_active(struct brcmf_chip *chip, u32 rstvec)
 {
 	struct brcmf_core *core;
 
-	chip->ops->activate(chip->ctx, &chip->pub, rstvec);
+	chip->ops->activate(chip->ctx, chip, rstvec);
 
 	/* restore ARM */
-	core = brcmf_chip_get_core(&chip->pub, BCMA_CORE_ARM_CR4);
+	core = brcmf_chip_get_core(chip, BCMA_CORE_ARM_CR4);
 	brcmf_chip_resetcore(core, ARMCR4_BCMA_IOCTL_CPUHALT, 0, 0);
 
 	return true;
 }
 
 static inline void
-brcmf_chip_ca7_set_passive(struct brcmf_chip_priv *chip)
+brcmf_chip_ca7_set_passive(struct brcmf_chip *chip)
 {
 	struct brcmf_core *core;
 
 	brcmf_chip_disable_arm(chip, BCMA_CORE_ARM_CA7);
 
-	core = brcmf_chip_get_core(&chip->pub, BCMA_CORE_80211);
+	core = brcmf_chip_get_core(chip, BCMA_CORE_80211);
 	brcmf_chip_resetcore(core, D11_BCMA_IOCTL_PHYRESET |
 				   D11_BCMA_IOCTL_PHYCLOCKEN,
 			     D11_BCMA_IOCTL_PHYCLOCKEN,
 			     D11_BCMA_IOCTL_PHYCLOCKEN);
 }
 
-static bool brcmf_chip_ca7_set_active(struct brcmf_chip_priv *chip, u32 rstvec)
+static bool brcmf_chip_ca7_set_active(struct brcmf_chip *chip, u32 rstvec)
 {
 	struct brcmf_core *core;
 
-	chip->ops->activate(chip->ctx, &chip->pub, rstvec);
+	chip->ops->activate(chip->ctx, chip, rstvec);
 
 	/* restore ARM */
-	core = brcmf_chip_get_core(&chip->pub, BCMA_CORE_ARM_CA7);
+	core = brcmf_chip_get_core(chip, BCMA_CORE_ARM_CA7);
 	brcmf_chip_resetcore(core, ARMCR4_BCMA_IOCTL_CPUHALT, 0, 0);
 
 	return true;
 }
 
-void brcmf_chip_set_passive(struct brcmf_chip *pub)
+void brcmf_chip_set_passive(struct brcmf_chip *chip)
 {
-	struct brcmf_chip_priv *chip;
 	struct brcmf_core *arm;
 
 	brcmf_dbg(TRACE, "Enter\n");
 
-	chip = container_of(pub, struct brcmf_chip_priv, pub);
-	arm = brcmf_chip_get_core(pub, BCMA_CORE_ARM_CR4);
+	arm = brcmf_chip_get_core(chip, BCMA_CORE_ARM_CR4);
 	if (arm) {
 		brcmf_chip_cr4_set_passive(chip);
 		return;
 	}
-	arm = brcmf_chip_get_core(pub, BCMA_CORE_ARM_CA7);
+
+	arm = brcmf_chip_get_core(chip, BCMA_CORE_ARM_CA7);
 	if (arm) {
 		brcmf_chip_ca7_set_passive(chip);
 		return;
 	}
-	arm = brcmf_chip_get_core(pub, BCMA_CORE_ARM_CM3);
+
+	arm = brcmf_chip_get_core(chip, BCMA_CORE_ARM_CM3);
 	if (arm) {
 		brcmf_chip_cm3_set_passive(chip);
 		return;
 	}
 }
 
-bool brcmf_chip_set_active(struct brcmf_chip *pub, u32 rstvec)
+bool brcmf_chip_set_active(struct brcmf_chip *chip, u32 rstvec)
 {
-	struct brcmf_chip_priv *chip;
 	struct brcmf_core *arm;
 
 	brcmf_dbg(TRACE, "Enter\n");
 
-	chip = container_of(pub, struct brcmf_chip_priv, pub);
-	arm = brcmf_chip_get_core(pub, BCMA_CORE_ARM_CR4);
+	arm = brcmf_chip_get_core(chip, BCMA_CORE_ARM_CR4);
 	if (arm)
 		return brcmf_chip_cr4_set_active(chip, rstvec);
-	arm = brcmf_chip_get_core(pub, BCMA_CORE_ARM_CA7);
+
+	arm = brcmf_chip_get_core(chip, BCMA_CORE_ARM_CA7);
 	if (arm)
 		return brcmf_chip_ca7_set_active(chip, rstvec);
-	arm = brcmf_chip_get_core(pub, BCMA_CORE_ARM_CM3);
+
+	arm = brcmf_chip_get_core(chip, BCMA_CORE_ARM_CM3);
 	if (arm)
 		return brcmf_chip_cm3_set_active(chip);
 
 	return false;
 }
 
-bool brcmf_chip_sr_capable(struct brcmf_chip *pub)
+bool brcmf_chip_sr_capable(struct brcmf_chip *chip)
 {
+	struct brcmf_core *pmu = brcmf_chip_get_pmu(chip);
 	u32 base, addr, reg, pmu_cc3_mask = ~0;
-	struct brcmf_chip_priv *chip;
-	struct brcmf_core *pmu = brcmf_chip_get_pmu(pub);
 
 	brcmf_dbg(TRACE, "Enter\n");
 
 	/* old chips with PMU version less than 17 don't support save restore */
-	if (pub->pmurev < 17)
+	if (chip->pmurev < 17)
 		return false;
 
-	base = brcmf_chip_get_chipcommon(pub)->base;
-	chip = container_of(pub, struct brcmf_chip_priv, pub);
+	base = brcmf_chip_get_chipcommon(chip)->base;
 
-	switch (pub->chip) {
+	switch (chip->chip) {
 	case BRCM_CC_4354_CHIP_ID:
 	case BRCM_CC_4356_CHIP_ID:
 		/* explicitly check SR engine enable bit */
@@ -1345,24 +1340,32 @@ bool brcmf_chip_sr_capable(struct brcmf_chip *pub)
 	case BRCM_CC_43241_CHIP_ID:
 	case BRCM_CC_4335_CHIP_ID:
 	case BRCM_CC_4339_CHIP_ID:
+
 		/* read PMU chipcontrol register 3 */
 		addr = CORE_CC_REG(pmu->base, chipcontrol_addr);
 		chip->ops->write32(chip->ctx, addr, 3);
+
 		addr = CORE_CC_REG(pmu->base, chipcontrol_data);
 		reg = chip->ops->read32(chip->ctx, addr);
+
 		return (reg & pmu_cc3_mask) != 0;
 	case BRCM_CC_43430_CHIP_ID:
+
 		addr = CORE_CC_REG(base, sr_control1);
 		reg = chip->ops->read32(chip->ctx, addr);
+
 		return reg != 0;
 	default:
+
 		addr = CORE_CC_REG(pmu->base, pmucapabilities_ext);
 		reg = chip->ops->read32(chip->ctx, addr);
+
 		if ((reg & PCAPEXT_SR_SUPPORTED_MASK) == 0)
 			return false;
 
 		addr = CORE_CC_REG(pmu->base, retention_ctl);
 		reg = chip->ops->read32(chip->ctx, addr);
+
 		return (reg & (PMU_RCTL_MACPHY_DISABLE_MASK |
 			       PMU_RCTL_LOGIC_DISABLE_MASK)) == 0;
 	}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h
index dd0ec3eba6a9..0babb328d155 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h
@@ -21,6 +21,8 @@
 #define CORE_CC_REG(base, field) \
 		(base + offsetof(struct chipcregs, field))
 
+struct brcmf_core;
+
 /**
  * struct brcmf_chip - chip level information.
  *
@@ -36,6 +38,20 @@
  * @name: string representation of the chip identifier.
  */
 struct brcmf_chip {
+	const struct brcmf_buscore_ops *ops;
+
+	/* assured first core is chipcommon, second core is buscore */
+	//FIXME: Really? does not appear to hold true for 43430
+	struct list_head cores; /* List of cores in this chip */
+	u16 num_cores;
+
+	bool (*iscoreup)(struct brcmf_core *core);
+	void (*coredisable)(struct brcmf_core *core, u32 prereset, u32 reset);
+	void (*resetcore)(struct brcmf_core *core, u32 prereset, u32 reset,
+				u32 postreset);
+
+	void *ctx;
+
 	u32 chip;
 	u32 chiprev;
 	u32 cc_caps;
@@ -45,6 +61,7 @@ struct brcmf_chip {
 	u32 rambase;
 	u32 ramsize;
 	u32 srsize;
+
 	char name[8];
 };
 
@@ -56,9 +73,14 @@ struct brcmf_chip {
  * @base: base address of core register space.
  */
 struct brcmf_core {
+	struct brcmf_chip *chip; /* Parent chip */
+	struct list_head list;
+
 	u16 id;
 	u16 rev;
 	u32 base;
+	u32 wrapbase;
+
 };
 
 /**
-- 
2.11.0

[PATCH 15/21] brcmfmac: Simplify chip probe routine

[Ian Molton]

Also remove otherwise un-needed chiprev function.
Also rename in line with typical kernel naming.
---
 .../wireless/broadcom/brcm80211/brcmfmac/chip.c    | 130 +++++++++++++--------
 1 file changed, 84 insertions(+), 46 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index 38794d7c1da8..b3b28e78c49d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -223,17 +223,6 @@ struct sbsocramregs {
 #define	ARMCR4_BSZ_MASK		0x3f
 #define	ARMCR4_BSZ_MULT		8192
 
-
-static void brcmf_chip_sb_corerev(struct brcmf_chip *ci,
-				  struct brcmf_core *core)
-{
-	u32 regdata;
-
-	regdata = ci->ops->read32(ci->ctx, CORE_SB(core->base, sbidhigh));
-
-	core->rev = SBCOREREV(regdata);
-}
-
 static bool brcmf_chip_sb_iscoreup(struct brcmf_core *core)
 {
 	struct brcmf_chip *ci = core->chip;
@@ -482,7 +471,7 @@ static char *brcmf_chip_name(uint chipid, char *buf, uint len)
 	return buf;
 }
 
-static struct brcmf_core *brcmf_chip_add_core(struct brcmf_chip *ci,
+static struct brcmf_core *__brcmf_chip_add_core(struct brcmf_chip *ci,
 					      u16 coreid, u32 base,
 					      u32 wrapbase)
 {
@@ -490,7 +479,7 @@ static struct brcmf_core *brcmf_chip_add_core(struct brcmf_chip *ci,
 
 	core = kzalloc(sizeof(*core), GFP_KERNEL);
 	if (!core)
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 
 	core->id = coreid;
 	core->base = base;
@@ -502,6 +491,41 @@ static struct brcmf_core *brcmf_chip_add_core(struct brcmf_chip *ci,
 	return core;
 }
 
+static struct brcmf_core *brcmf_chip_add_sb_core(struct brcmf_chip *ci,
+					      u16 coreid, u32 base,
+					      u32 wrapbase)
+{
+	struct brcmf_core *core;
+	u32 regdata;
+
+	core = __brcmf_chip_add_core(ci, coreid, base, wrapbase);
+
+	if (!core)
+		goto out;
+
+	regdata = ci->ops->read32(ci->ctx, CORE_SB(core->base, sbidhigh));
+
+	core->rev = SBCOREREV(regdata);
+
+out:
+	return core;
+}
+
+static struct brcmf_core *brcmf_chip_add_axi_core(struct brcmf_chip *ci,
+					      u16 coreid, u32 base,
+					      u32 wrapbase, u8 rev)
+{
+	struct brcmf_core *core;
+
+	core = __brcmf_chip_add_core(ci, coreid, base, wrapbase);
+
+	if (core)
+		core->rev = rev;
+
+	return core;
+}
+
+
 /* safety check for chipinfo */
 static int brcmf_chip_cores_check(struct brcmf_chip *ci)
 {
@@ -900,19 +924,42 @@ int brcmf_chip_dmp_erom_scan(struct brcmf_chip *ci)
 			continue;
 
 		/* finally a core to be added */
-		core = brcmf_chip_add_core(ci, id, base, wrap);
-		if (IS_ERR(core))
-			return PTR_ERR(core);
-
-		core->rev = rev;
+		core = brcmf_chip_add_axi_core(ci, id, base, wrap, rev);
+		if (!core)
+			return -ENOMEM; //FIXME - and cleanup the allocated cores?
 	}
 
 	return 0;
 }
 
-static int brcmf_chip_recognition(struct brcmf_chip *ci)
+struct brcmf_chip_desc {
+	u16 id;
+	u32 base;
+};
+
+static struct brcmf_chip_desc brcmf_4329[] = {
+	{ BCMA_CORE_CHIPCOMMON,   SI_ENUM_BASE },
+	{ BCMA_CORE_SDIO_DEV,     BCM4329_CORE_BUS_BASE },
+	{ BCMA_CORE_INTERNAL_MEM, BCM4329_CORE_SOCRAM_BASE },
+	{ BCMA_CORE_ARM_CM3,      BCM4329_CORE_ARM_BASE },
+	{ BCMA_CORE_80211,        0x18001000 },
+	{ 0 , 0},
+};
+
+static int brcmf_chip_add_static(struct brcmf_chip *ci,
+				struct brcmf_chip_desc *desc)
+{
+	for ( ; desc->id ; desc++ )
+		brcmf_chip_add_sb_core(ci, desc->id, desc->base, 0);
+
+	//FIXME: cleanup if we fail to add a core?
+
+	return 0;
+}
+
+
+static int brcmf_chip_probe(struct brcmf_chip *ci)
 {
-	struct brcmf_core *core;
 	u32 regdata;
 	u32 socitype;
 	int ret;
@@ -920,11 +967,13 @@ static int brcmf_chip_recognition(struct brcmf_chip *ci)
 	/* Get CC core rev
 	 * Chipid is assume to be at offset 0 from SI_ENUM_BASE
 	 * For different chiptypes or old sdio hosts w/o chipcommon,
-	 * other ways of recognition should be added here.
+	 * other ways of to probe should be added here.
 	 */
 	regdata = ci->ops->read32(ci->ctx, CORE_CC_REG(SI_ENUM_BASE, chipid));
+
 	ci->chip = regdata & CID_ID_MASK;
 	ci->chiprev = (regdata & CID_REV_MASK) >> CID_REV_SHIFT;
+
 	socitype = (regdata & CID_TYPE_MASK) >> CID_TYPE_SHIFT;
 
 	brcmf_chip_name(ci->chip, ci->name, sizeof(ci->name));
@@ -936,43 +985,32 @@ static int brcmf_chip_recognition(struct brcmf_chip *ci)
 	switch(socitype) {
 		case SOCI_SB:
 
-		if (ci->chip != BRCM_CC_4329_CHIP_ID) {
-			brcmf_err("SB chip is not supported\n");
-			return -ENODEV;
+		switch(ci->chip) {
+			case BRCM_CC_4329_CHIP_ID:
+				ret = brcmf_chip_add_static(ci, brcmf_4329);
+				break;
+			default:
+				brcmf_err("SB chip is not supported\n");
+				return -ENODEV;
 		}
 
+		if(!ret)
+			return -ENODEV;
+
 		ci->iscoreup = brcmf_chip_sb_iscoreup;
 		ci->coredisable = brcmf_chip_sb_coredisable;
 		ci->resetcore = brcmf_chip_sb_resetcore;
 
-		core = brcmf_chip_add_core(ci, BCMA_CORE_CHIPCOMMON,
-					   SI_ENUM_BASE, 0);
-		brcmf_chip_sb_corerev(ci, core);
-
-		core = brcmf_chip_add_core(ci, BCMA_CORE_SDIO_DEV,
-					   BCM4329_CORE_BUS_BASE, 0);
-		brcmf_chip_sb_corerev(ci, core);
-
-		core = brcmf_chip_add_core(ci, BCMA_CORE_INTERNAL_MEM,
-					   BCM4329_CORE_SOCRAM_BASE, 0);
-		brcmf_chip_sb_corerev(ci, core);
-
-		core = brcmf_chip_add_core(ci, BCMA_CORE_ARM_CM3,
-					   BCM4329_CORE_ARM_BASE, 0);
-		brcmf_chip_sb_corerev(ci, core);
-
-		core = brcmf_chip_add_core(ci, BCMA_CORE_80211, 0x18001000, 0);
-		brcmf_chip_sb_corerev(ci, core);
-
 		break;
 	case SOCI_AXI:
 
+		if (brcmf_chip_dmp_erom_scan(ci))
+			return -ENODEV;
+
 		ci->iscoreup = brcmf_chip_ai_iscoreup;
 		ci->coredisable = brcmf_chip_ai_coredisable;
 		ci->resetcore = brcmf_chip_ai_resetcore;
 
-		brcmf_chip_dmp_erom_scan(ci);
-
 		break;
 	default:
 		brcmf_err("chip backplane type %u is not supported\n",
@@ -1093,7 +1131,7 @@ struct brcmf_chip *brcmf_chip_attach(void *ctx,
 	if (err < 0)
 		goto fail;
 
-	err = brcmf_chip_recognition(chip);
+	err = brcmf_chip_probe(chip);
 	if (err < 0)
 		goto fail;
 
-- 
2.11.0

[PATCH 16/21] brcmfmac: rename axi functions for clarity

[Ian Molton]

---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index b3b28e78c49d..1485463794ff 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -239,7 +239,7 @@ static bool brcmf_chip_sb_iscoreup(struct brcmf_core *core)
 	return SSB_TMSLOW_CLOCK == regdata;
 }
 
-static bool brcmf_chip_ai_iscoreup(struct brcmf_core *core)
+static bool brcmf_chip_axi_iscoreup(struct brcmf_core *core)
 {
 	struct brcmf_chip *ci = core->chip;
 	u32 regdata;
@@ -340,7 +340,7 @@ static void brcmf_chip_sb_coredisable(struct brcmf_core *core,
 	udelay(1);
 }
 
-static void brcmf_chip_ai_coredisable(struct brcmf_core *core,
+static void brcmf_chip_axi_coredisable(struct brcmf_core *core,
 				      u32 prereset, u32 reset)
 {
 	struct brcmf_chip *ci = core->chip;
@@ -432,14 +432,14 @@ static void brcmf_chip_sb_resetcore(struct brcmf_core *core, u32 prereset,
 	udelay(1);
 }
 
-static void brcmf_chip_ai_resetcore(struct brcmf_core *core, u32 prereset,
+static void brcmf_chip_axi_resetcore(struct brcmf_core *core, u32 prereset,
 				    u32 reset, u32 postreset)
 {
 	struct brcmf_chip *ci = core->chip;
 	int count;
 
 	/* must disable first to work for arbitrary current core state */
-	brcmf_chip_ai_coredisable(core, prereset, reset);
+	brcmf_chip_axi_coredisable(core, prereset, reset);
 
 	count = 0;
 
@@ -1007,9 +1007,9 @@ static int brcmf_chip_probe(struct brcmf_chip *ci)
 		if (brcmf_chip_dmp_erom_scan(ci))
 			return -ENODEV;
 
-		ci->iscoreup = brcmf_chip_ai_iscoreup;
-		ci->coredisable = brcmf_chip_ai_coredisable;
-		ci->resetcore = brcmf_chip_ai_resetcore;
+		ci->iscoreup = brcmf_chip_axi_iscoreup;
+		ci->coredisable = brcmf_chip_axi_coredisable;
+		ci->resetcore = brcmf_chip_axi_resetcore;
 
 		break;
 	default:
-- 
2.11.0

[PATCH 17/21] brcmfmac: HUGE cleanup of IO access functions.

[Ian Molton]

---
 .../wireless/broadcom/brcm80211/brcmfmac/chip.c    | 474 ++++++++++-----------
 1 file changed, 235 insertions(+), 239 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index 1485463794ff..bab1bb3e99ae 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -28,6 +28,9 @@
 #include "debug.h"
 #include "chip.h"
 
+#undef brcmf_dbg
+#define brcmf_dbg(a, b, ...) printk(b, ##__VA_ARGS__)
+
 /* SOC Interconnect types (aka chip types) */
 #define SOCI_SB		0
 #define SOCI_AXI	1
@@ -106,9 +109,6 @@
 
 #define CORE_SB(base, field) \
 		(base + SBCONFIGOFF + offsetof(struct sbconfig, field))
-#define	SBCOREREV(sbidh) \
-	((((sbidh) & SSB_IDHIGH_RCHI) >> SSB_IDHIGH_RCHI_SHIFT) | \
-	  ((sbidh) & SSB_IDHIGH_RCLO))
 
 struct sbconfig {
 	u32 PAD[2];
@@ -223,40 +223,75 @@ struct sbsocramregs {
 #define	ARMCR4_BSZ_MASK		0x3f
 #define	ARMCR4_BSZ_MULT		8192
 
+static inline int brcmf_readl(struct brcmf_chip *chip, u32 addr)
+{
+	return chip->ops->read32(chip->ctx, addr);
+}
+
+static inline void brcmf_writel(struct brcmf_chip *chip, u32 addr, u32 v)
+{
+	chip->ops->write32(chip->ctx, addr, v);
+}
+
+static inline void brcmf_writelp(struct brcmf_chip *chip, u32 addr, u32 v)
+{
+	chip->ops->write32(chip->ctx, addr, v);
+	chip->ops->read32(chip->ctx, addr);
+}
+
+static void brcmf_clear_bits(struct brcmf_chip *ci, u32 reg, u32 bits)
+{
+	u32 val;
+
+	val = brcmf_readl(ci, reg);
+	val &= ~bits;
+	brcmf_writel(ci, reg, val);
+}
+
+static void brcmf_set_bits_post(struct brcmf_chip *ci, u32 reg, u32 bits)
+{
+	u32 val;
+
+	val = brcmf_readl(ci, reg);
+	val |= bits;
+	brcmf_writelp(ci, reg, val);
+}
+
 static bool brcmf_chip_sb_iscoreup(struct brcmf_core *core)
 {
 	struct brcmf_chip *ci = core->chip;
-	u32 regdata;
-	u32 address;
-
-	address = CORE_SB(core->base, sbtmstatelow);
+	u32 val;
 
-	regdata = ci->ops->read32(ci->ctx, address);
+	val = brcmf_readl(ci, CORE_SB(core->base, sbtmstatelow));
 
-	regdata &= (SSB_TMSLOW_RESET | SSB_TMSLOW_REJECT |
-		    SSB_IMSTATE_REJECT | SSB_TMSLOW_CLOCK);
+	val &= SSB_TMSLOW_RESET |
+		SSB_TMSLOW_REJECT |
+		SSB_IMSTATE_REJECT |
+		SSB_TMSLOW_CLOCK;
 
-	return SSB_TMSLOW_CLOCK == regdata;
+	return SSB_TMSLOW_CLOCK == val;
 }
 
 static bool brcmf_chip_axi_iscoreup(struct brcmf_core *core)
 {
 	struct brcmf_chip *ci = core->chip;
-	u32 regdata;
-	bool ret;
+	u32 val;
 
-	regdata = ci->ops->read32(ci->ctx, core->wrapbase + BCMA_IOCTL);
+	val = brcmf_readl(ci, core->wrapbase + BCMA_IOCTL);
 
-	ret = (regdata & (BCMA_IOCTL_FGC | BCMA_IOCTL_CLK)) == BCMA_IOCTL_CLK;
+	if((val & BCMA_IOCTL_FGC) || !(val & BCMA_IOCTL_CLK))
+		return 0;
 
-	regdata = ci->ops->read32(ci->ctx, core->wrapbase + BCMA_RESET_CTL);
+	/* Is core in reset? */
+	val = brcmf_readl(ci, core->wrapbase + BCMA_RESET_CTL);
 
-	ret = ret && ((regdata & BCMA_RESET_CTL_RESET) == 0);
+	val &= BCMA_RESET_CTL_RESET;
 
-	return ret;
+	return !val;
 }
 
-static void brcmf_chip_sb_coredisable(struct brcmf_core *core,
+/* prereset and reset ignored */
+static void brcmf_chip_sb_core_disable(struct brcmf_core *core,
 				      u32 prereset, u32 reset)
 {
 	struct brcmf_chip *ci = core->chip;
@@ -264,211 +299,176 @@ static void brcmf_chip_sb_coredisable(struct brcmf_core *core,
 
 	base = core->base;
 
-	val = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+	val = brcmf_readl(ci, CORE_SB(base, sbtmstatelow));
 
+	/* If core is already in reset, skip reset */
 	if (val & SSB_TMSLOW_RESET)
 		return;
 
-	val = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
-
-	if ((val & SSB_TMSLOW_CLOCK) != 0) {
-		/*
-		 * set target reject and spin until busy is clear
-		 * (preserve core-specific bits)
-		 */
-		val = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+	if (!(val & SSB_TMSLOW_CLOCK))
+		goto out_clock_off;
 
-		ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatelow),
-					 val | SSB_TMSLOW_REJECT);
+	/* Clock is running, so do other stuff? */
 
-		val = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+	/*
+	 * Set target reject and spin until busy is clear
+	 * (preserve core-specific bits)
+	 */
+	brcmf_set_bits_post(ci, CORE_SB(base, sbtmstatelow), SSB_TMSLOW_REJECT);
+	udelay(1);
 
-		udelay(1);
+	SPINWAIT((brcmf_readl(ci, CORE_SB(base, sbtmstatehigh))
+		  & SSB_TMSHIGH_BUSY), 100000);
 
-		SPINWAIT((ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatehigh))
-			  & SSB_TMSHIGH_BUSY), 100000);
+	val = brcmf_readl(ci, CORE_SB(base, sbtmstatehigh));
 
-		val = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatehigh));
+	if (val & SSB_TMSHIGH_BUSY)
+		brcmf_err("core state still busy\n");
 
-		if (val & SSB_TMSHIGH_BUSY)
-			brcmf_err("core state still busy\n");
 
-		val = ci->ops->read32(ci->ctx, CORE_SB(base, sbidlow));
-
-		if (val & SSB_IDLOW_INITIATOR) {
-			val = ci->ops->read32(ci->ctx,
-					      CORE_SB(base, sbimstate));
-			val |= SSB_IMSTATE_REJECT;
-			ci->ops->write32(ci->ctx,
-					 CORE_SB(base, sbimstate), val);
-			val = ci->ops->read32(ci->ctx,
-					      CORE_SB(base, sbimstate));
-			udelay(1);
-			SPINWAIT((ci->ops->read32(ci->ctx,
-						  CORE_SB(base, sbimstate)) &
-				  SSB_IMSTATE_BUSY), 100000);
-		}
+	val = brcmf_readl(ci, CORE_SB(base, sbidlow));
+	if (val & SSB_IDLOW_INITIATOR)
+	{
+		/* Do something and spin until core acknowledges */
 
-		/* set reset and reject while enabling the clocks */
-		val = SSB_TMSLOW_FGC | SSB_TMSLOW_CLOCK |
-		      SSB_TMSLOW_REJECT | SSB_TMSLOW_RESET;
+		brcmf_set_bits_post(ci, CORE_SB(base, sbimstate),
+				SSB_IMSTATE_REJECT);
+		udelay(1);
 
-		ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatelow), val);
+		SPINWAIT((brcmf_readl(ci, CORE_SB(base, sbimstate)) &
+			  SSB_IMSTATE_BUSY), 100000);
 
-		val = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+		val = brcmf_readl(ci, CORE_SB(base, sbimstate));
 
-		udelay(10);
+		if (val & SSB_IMSTATE_BUSY)
+			brcmf_err("core state still busy\n");
+	}
 
-		/* clear the initiator reject bit */
-		val = ci->ops->read32(ci->ctx, CORE_SB(base, sbidlow));
+	/* Set reset and reject while enabling the clocks */
+	brcmf_writelp(ci, CORE_SB(base, sbtmstatelow),
+			SSB_TMSLOW_FGC | SSB_TMSLOW_CLOCK |
+			SSB_TMSLOW_REJECT | SSB_TMSLOW_RESET);
 
-		if (val & SSB_IDLOW_INITIATOR) {
-			val = ci->ops->read32(ci->ctx,
-					      CORE_SB(base, sbimstate));
+	udelay(10);
 
-			val &= ~SSB_IMSTATE_REJECT;
+	/* Clear the initiator reject bit */
+	/* FIXME: Has this value actually changed since it was read earlier? */
+	val = brcmf_readl(ci, CORE_SB(base, sbidlow));
 
-			ci->ops->write32(ci->ctx,
-					 CORE_SB(base, sbimstate), val);
-		}
-	}
+	if (val & SSB_IDLOW_INITIATOR)
+		brcmf_clear_bits(ci, CORE_SB(base, sbimstate),
+				SSB_IMSTATE_REJECT);
 
+out_clock_off:
 	/* leave reset and reject asserted */
-	ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatelow),
-			 (SSB_TMSLOW_REJECT | SSB_TMSLOW_RESET));
+	brcmf_writel(ci, CORE_SB(base, sbtmstatelow),
+			 SSB_TMSLOW_REJECT | SSB_TMSLOW_RESET);
 
 	udelay(1);
 }
 
-static void brcmf_chip_axi_coredisable(struct brcmf_core *core,
+static void brcmf_chip_axi_core_disable(struct brcmf_core *core,
 				      u32 prereset, u32 reset)
 {
 	struct brcmf_chip *ci = core->chip;
-	u32 regdata;
+	u32 val;
 
-	/* if core is already in reset, skip reset */
-	regdata = ci->ops->read32(ci->ctx, core->wrapbase + BCMA_RESET_CTL);
+	val = brcmf_readl(ci, core->wrapbase + BCMA_RESET_CTL);
 
-	if ((regdata & BCMA_RESET_CTL_RESET) != 0)
+	/* If core is already in reset, skip reset */
+	if ((val & BCMA_RESET_CTL_RESET))
 		goto in_reset_configure;
 
-	/* configure reset */
-	ci->ops->write32(ci->ctx, core->wrapbase + BCMA_IOCTL,
+	/* Configure reset */
+	brcmf_writelp(ci, core->wrapbase + BCMA_IOCTL,
 			 prereset | BCMA_IOCTL_FGC | BCMA_IOCTL_CLK);
 
-	ci->ops->read32(ci->ctx, core->wrapbase + BCMA_IOCTL);
-
-	/* put in reset */
-	ci->ops->write32(ci->ctx, core->wrapbase + BCMA_RESET_CTL,
-			 BCMA_RESET_CTL_RESET);
+	/* Put the core into reset */
+	brcmf_writel(ci, core->wrapbase + BCMA_RESET_CTL, BCMA_RESET_CTL_RESET);
 
 	usleep_range(10, 20);
 
-	/* wait till reset is 1 */
-	SPINWAIT(ci->ops->read32(ci->ctx, core->wrapbase + BCMA_RESET_CTL) !=
+	/*
+	 * Wait till reset is 1
+	 * FIXME: should this test the BCMA_RESET_CTL_RESET bit only?
+	 */
+	SPINWAIT(brcmf_readl(ci, core->wrapbase + BCMA_RESET_CTL) !=
 		 BCMA_RESET_CTL_RESET, 300);
 
 in_reset_configure:
-	/* in-reset configure */
-	ci->ops->write32(ci->ctx, core->wrapbase + BCMA_IOCTL,
+	brcmf_writelp(ci, core->wrapbase + BCMA_IOCTL,
 			 reset | BCMA_IOCTL_FGC | BCMA_IOCTL_CLK);
-
-	ci->ops->read32(ci->ctx, core->wrapbase + BCMA_IOCTL);
 }
 
-static void brcmf_chip_sb_resetcore(struct brcmf_core *core, u32 prereset,
+static void brcmf_chip_sb_reset_core(struct brcmf_core *core, u32 prereset,
 				    u32 reset, u32 postreset)
 {
 	struct brcmf_chip *ci = core->chip;
 	u32 base = core->base;
-	u32 regdata;
+	u32 val;
 
 	/*
 	 * Must do the disable sequence first to work for
 	 * arbitrary current core state.
 	 */
-	brcmf_chip_sb_coredisable(core, 0, 0);
+	brcmf_chip_sb_core_disable(core, 0, 0);
 
 	/*
 	 * Now do the initialization sequence.
 	 * set reset while enabling the clock and
 	 * forcing them on throughout the core
 	 */
-	ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatelow),
+	brcmf_writelp(ci, CORE_SB(base, sbtmstatelow),
 			 SSB_TMSLOW_FGC | SSB_TMSLOW_CLOCK |
 			 SSB_TMSLOW_RESET);
 
-	regdata = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
-
 	udelay(1);
 
-	/* clear any serror */
-	regdata = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatehigh));
+	/* Clear any serror */
+	val = brcmf_readl(ci, CORE_SB(base, sbtmstatehigh));
 
-	if (regdata & SSB_TMSHIGH_SERR)
-		ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatehigh), 0);
+	if (val & SSB_TMSHIGH_SERR)
+		brcmf_writel(ci, CORE_SB(base, sbtmstatehigh), 0);
 
-	regdata = ci->ops->read32(ci->ctx, CORE_SB(base, sbimstate));
+	val = brcmf_readl(ci, CORE_SB(base, sbimstate));
 
-	if (regdata & (SSB_IMSTATE_IBE | SSB_IMSTATE_TO)) {
-		regdata &= ~(SSB_IMSTATE_IBE | SSB_IMSTATE_TO);
-		ci->ops->write32(ci->ctx, CORE_SB(base, sbimstate), regdata);
+	if (val & (SSB_IMSTATE_IBE | SSB_IMSTATE_TO)) {
+		val &= ~(SSB_IMSTATE_IBE | SSB_IMSTATE_TO);
+		brcmf_writel(ci, CORE_SB(base, sbimstate), val);
 	}
 
-	/* clear reset and allow it to propagate throughout the core */
-	ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatelow),
+	/* Clear reset and allow it to propagate throughout the core */
+	brcmf_writelp(ci, CORE_SB(base, sbtmstatelow),
 			 SSB_TMSLOW_FGC | SSB_TMSLOW_CLOCK);
 
-	regdata = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
-
 	udelay(1);
 
-	/* leave clock enabled */
-	ci->ops->write32(ci->ctx, CORE_SB(base, sbtmstatelow),
-			 SSB_TMSLOW_CLOCK);
-
-	regdata = ci->ops->read32(ci->ctx, CORE_SB(base, sbtmstatelow));
+	/* Leave clock enabled */
+	brcmf_writelp(ci, CORE_SB(base, sbtmstatelow), SSB_TMSLOW_CLOCK);
 
 	udelay(1);
 }
 
-static void brcmf_chip_axi_resetcore(struct brcmf_core *core, u32 prereset,
+static void brcmf_chip_axi_reset_core(struct brcmf_core *core, u32 prereset,
 				    u32 reset, u32 postreset)
 {
 	struct brcmf_chip *ci = core->chip;
-	int count;
-
-	/* must disable first to work for arbitrary current core state */
-	brcmf_chip_axi_coredisable(core, prereset, reset);
-
-	count = 0;
+	int count = 0;
 
-	while (ci->ops->read32(ci->ctx, core->wrapbase + BCMA_RESET_CTL) &
-	       BCMA_RESET_CTL_RESET) {
+	/* Must disable first to work for arbitrary current core state */
+	brcmf_chip_axi_core_disable(core, prereset, reset);
 
-		ci->ops->write32(ci->ctx, core->wrapbase + BCMA_RESET_CTL, 0);
+	while ((brcmf_readl(ci, core->wrapbase + BCMA_RESET_CTL) &
+			BCMA_RESET_CTL_RESET) &&
+			count++ <= 50) {
 
-		count++;
-
-		if (count > 50)
-			break;
+		brcmf_writel(ci, core->wrapbase + BCMA_RESET_CTL, 0);
 
 		usleep_range(40, 60);
 	}
 
-	ci->ops->write32(ci->ctx, core->wrapbase + BCMA_IOCTL,
-			 postreset | BCMA_IOCTL_CLK);
-
-	ci->ops->read32(ci->ctx, core->wrapbase + BCMA_IOCTL);
-}
-
-static char *brcmf_chip_name(uint chipid, char *buf, uint len)
-{
-	const char *fmt;
-
-	fmt = ((chipid > 0xa000) || (chipid < 0x4000)) ? "%d" : "%x";
-	snprintf(buf, len, fmt, chipid);
-	return buf;
+	brcmf_writelp(ci, core->wrapbase + BCMA_IOCTL,
+			postreset | BCMA_IOCTL_CLK);
 }
 
 static struct brcmf_core *__brcmf_chip_add_core(struct brcmf_chip *ci,
@@ -481,9 +481,9 @@ static struct brcmf_core *__brcmf_chip_add_core(struct brcmf_chip *ci,
 	if (!core)
 		return NULL;
 
+	core->chip = ci;
 	core->id = coreid;
 	core->base = base;
-	core->chip = ci;
 	core->wrapbase = wrapbase;
 
 	list_add_tail(&core->list, &ci->cores);
@@ -496,16 +496,16 @@ static struct brcmf_core *brcmf_chip_add_sb_core(struct brcmf_chip *ci,
 					      u32 wrapbase)
 {
 	struct brcmf_core *core;
-	u32 regdata;
+	u32 val;
 
 	core = __brcmf_chip_add_core(ci, coreid, base, wrapbase);
 
 	if (!core)
 		goto out;
 
-	regdata = ci->ops->read32(ci->ctx, CORE_SB(core->base, sbidhigh));
+	val = brcmf_readl(ci, CORE_SB(core->base, sbidhigh));
 
-	core->rev = SBCOREREV(regdata);
+	core->rev = (val & 0xf) | ((val & 0x7000) >> 8);
 
 out:
 	return core;
@@ -573,34 +573,29 @@ static int brcmf_chip_cores_check(struct brcmf_chip *ci)
 	return 0;
 }
 
-static u32 brcmf_chip_core_read32(struct brcmf_core *core, u16 reg)
-{
-	return core->chip->ops->read32(core->chip->ctx, core->base + reg);
-}
-
-static void brcmf_chip_core_write32(struct brcmf_core *core,
-				    u16 reg, u32 val)
-{
-	core->chip->ops->write32(core->chip->ctx, core->base + reg, val);
-}
-
 static bool brcmf_chip_socram_banksize(struct brcmf_core *core, u8 idx,
 				       u32 *banksize)
 {
+	struct brcmf_chip *ci = core->chip;
 	u32 bankinfo;
 	u32 bankidx = (SOCRAM_MEMTYPE_RAM << SOCRAM_BANKIDX_MEMTYPE_SHIFT);
 
 	bankidx |= idx;
-	brcmf_chip_core_write32(core, SOCRAMREGOFFS(bankidx), bankidx);
-	bankinfo = brcmf_chip_core_read32(core, SOCRAMREGOFFS(bankinfo));
+
+	brcmf_writel(ci, core->base + SOCRAMREGOFFS(bankidx), bankidx);
+
+	bankinfo = brcmf_readl(ci, core->base + SOCRAMREGOFFS(bankinfo));
+
 	*banksize = (bankinfo & SOCRAM_BANKINFO_SZMASK) + 1;
 	*banksize *= SOCRAM_BANKINFO_SZBASE;
+
 	return !!(bankinfo & SOCRAM_BANKINFO_RETNTRAM_MASK);
 }
 
 static void brcmf_chip_socram_ramsize(struct brcmf_core *sr, u32 *ramsize,
 				      u32 *srsize)
 {
+	struct brcmf_chip *ci = sr->chip;
 	u32 coreinfo;
 	uint nb, banksize, lss;
 	bool retent;
@@ -616,7 +611,8 @@ static void brcmf_chip_socram_ramsize(struct brcmf_core *sr, u32 *ramsize,
 		brcmf_chip_resetcore(sr, 0, 0, 0);
 
 	/* Get info for determining size */
-	coreinfo = brcmf_chip_core_read32(sr, SOCRAMREGOFFS(coreinfo));
+	coreinfo = brcmf_readl(ci, sr->base + SOCRAMREGOFFS(coreinfo));
+
 	nb = (coreinfo & SRCI_SRNB_MASK) >> SRCI_SRNB_SHIFT;
 
 	if ((sr->rev <= 7) || (sr->rev == 12)) {
@@ -657,6 +653,7 @@ static void brcmf_chip_socram_ramsize(struct brcmf_core *sr, u32 *ramsize,
 /** Return the SYS MEM size */
 static u32 brcmf_chip_sysmem_ramsize(struct brcmf_core *sysmem)
 {
+	struct brcmf_chip *ci = sysmem->chip;
 	u32 memsize = 0;
 	u32 coreinfo;
 	u32 idx;
@@ -666,7 +663,7 @@ static u32 brcmf_chip_sysmem_ramsize(struct brcmf_core *sysmem)
 	if (!brcmf_chip_iscoreup(sysmem))
 		brcmf_chip_resetcore(sysmem, 0, 0, 0);
 
-	coreinfo = brcmf_chip_core_read32(sysmem, SYSMEMREGOFFS(coreinfo));
+	coreinfo = brcmf_readl(ci, sysmem->base + SYSMEMREGOFFS(coreinfo));
 	nb = (coreinfo & SRCI_SRNB_MASK) >> SRCI_SRNB_SHIFT;
 
 	for (idx = 0; idx < nb; idx++) {
@@ -680,6 +677,7 @@ static u32 brcmf_chip_sysmem_ramsize(struct brcmf_core *sysmem)
 /** Return the TCM-RAM size of the ARMCR4 core. */
 static u32 brcmf_chip_tcm_ramsize(struct brcmf_core *cr4)
 {
+	struct brcmf_chip *ci = cr4->chip;
 	u32 corecap;
 	u32 memsize = 0;
 	u32 nab;
@@ -688,15 +686,18 @@ static u32 brcmf_chip_tcm_ramsize(struct brcmf_core *cr4)
 	u32 bxinfo;
 	u32 idx;
 
-	corecap = brcmf_chip_core_read32(cr4, ARMCR4_CAP);
+	corecap = brcmf_readl(ci, cr4->base + ARMCR4_CAP);
 
 	nab = (corecap & ARMCR4_TCBANB_MASK) >> ARMCR4_TCBANB_SHIFT;
 	nbb = (corecap & ARMCR4_TCBBNB_MASK) >> ARMCR4_TCBBNB_SHIFT;
+
 	totb = nab + nbb;
 
 	for (idx = 0; idx < totb; idx++) {
-		brcmf_chip_core_write32(cr4, ARMCR4_BANKIDX, idx);
-		bxinfo = brcmf_chip_core_read32(cr4, ARMCR4_BANKINFO);
+		brcmf_writel(ci, cr4->base + ARMCR4_BANKIDX, idx);
+
+		bxinfo = brcmf_readl(ci, cr4->base + ARMCR4_BANKINFO);
+
 		memsize += ((bxinfo & ARMCR4_BSZ_MASK) + 1) * ARMCR4_BSZ_MULT;
 	}
 
@@ -786,15 +787,17 @@ static u32 brcmf_chip_dmp_get_desc(struct brcmf_chip *ci, u32 *eromaddr,
 {
 	u32 val;
 
-	/* read next descriptor */
-	val = ci->ops->read32(ci->ctx, *eromaddr);
+	/* Read next descriptor */
+	val = brcmf_readl(ci, *eromaddr);
+
 	*eromaddr += 4;
 
 	if (!type)
 		return val;
 
-	/* determine descriptor type */
+	/* Determine descriptor type */
 	*type = (val & DMP_DESC_TYPE_MSK);
+
 	if ((*type & ~DMP_DESC_ADDRSIZE_GT32) == DMP_DESC_ADDRESS)
 		*type = DMP_DESC_ADDRESS;
 
@@ -885,7 +888,7 @@ int brcmf_chip_dmp_erom_scan(struct brcmf_chip *ci)
 	u32 base, wrap;
 	int err;
 
-	eromaddr = ci->ops->read32(ci->ctx, CORE_CC_REG(SI_ENUM_BASE, eromptr));
+	eromaddr = brcmf_readl(ci, CORE_CC_REG(SI_ENUM_BASE, eromptr));
 
 	while (desc_type != DMP_DESC_EOT) {
 		val = brcmf_chip_dmp_get_desc(ci, &eromaddr, &desc_type);
@@ -960,80 +963,79 @@ static int brcmf_chip_add_static(struct brcmf_chip *ci,
 
 static int brcmf_chip_probe(struct brcmf_chip *ci)
 {
-	u32 regdata;
-	u32 socitype;
+	u32 val, socitype;
 	int ret;
 
-	/* Get CC core rev
-	 * Chipid is assume to be at offset 0 from SI_ENUM_BASE
-	 * For different chiptypes or old sdio hosts w/o chipcommon,
-	 * other ways of to probe should be added here.
-	 */
-	regdata = ci->ops->read32(ci->ctx, CORE_CC_REG(SI_ENUM_BASE, chipid));
+	val = brcmf_readl(ci, CORE_CC_REG(SI_ENUM_BASE, chipid));
 
-	ci->chip = regdata & CID_ID_MASK;
-	ci->chiprev = (regdata & CID_REV_MASK) >> CID_REV_SHIFT;
 
-	socitype = (regdata & CID_TYPE_MASK) >> CID_TYPE_SHIFT;
+	ci->chip = val & CID_ID_MASK;
+	ci->chiprev = (val & CID_REV_MASK) >> CID_REV_SHIFT;
 
-	brcmf_chip_name(ci->chip, ci->name, sizeof(ci->name));
+	snprintf(ci->name, sizeof(ci->name), 
+		(ci->chip > 0xa000 || ci->chip < 0x4000) ? "%d" : "%x",
+		ci->chip);
 
-	printk("found %s chip: BCM%s, rev=%d\n",
-		  socitype == SOCI_SB ? "SB" : "AXI", ci->name,
-		  ci->chiprev);
+	socitype = (val & CID_TYPE_MASK) >> CID_TYPE_SHIFT;
 
 	switch(socitype) {
 		case SOCI_SB:
 
-		switch(ci->chip) {
-			case BRCM_CC_4329_CHIP_ID:
-				ret = brcmf_chip_add_static(ci, brcmf_4329);
-				break;
-			default:
-				brcmf_err("SB chip is not supported\n");
-				return -ENODEV;
-		}
+			switch(ci->chip) {
+				case BRCM_CC_4329_CHIP_ID:
+					ret = brcmf_chip_add_static(ci, brcmf_4329);
+					break;
+				default:
+					brcmf_err("SB chip is not supported\n");
+					return -ENODEV;
+			}
 
-		if(!ret)
-			return -ENODEV;
+			if(!ret)
+				return -ENODEV;
 
-		ci->iscoreup = brcmf_chip_sb_iscoreup;
-		ci->coredisable = brcmf_chip_sb_coredisable;
-		ci->resetcore = brcmf_chip_sb_resetcore;
+			ci->iscoreup = brcmf_chip_sb_iscoreup;
+			ci->coredisable = brcmf_chip_sb_core_disable;
+			ci->resetcore = brcmf_chip_sb_reset_core;
 
-		break;
-	case SOCI_AXI:
+			break;
+		case SOCI_AXI:
 
-		if (brcmf_chip_dmp_erom_scan(ci))
-			return -ENODEV;
+			if (brcmf_chip_dmp_erom_scan(ci))
+				return -ENODEV;
 
-		ci->iscoreup = brcmf_chip_axi_iscoreup;
-		ci->coredisable = brcmf_chip_axi_coredisable;
-		ci->resetcore = brcmf_chip_axi_resetcore;
+			ci->iscoreup = brcmf_chip_axi_iscoreup;
+			ci->coredisable = brcmf_chip_axi_core_disable;
+			ci->resetcore = brcmf_chip_axi_reset_core;
 
-		break;
-	default:
-		brcmf_err("chip backplane type %u is not supported\n",
-			  socitype);
-		return -ENODEV;
+			break;
+		default:
+			brcmf_err("chip backplane type %u is not supported\n",
+				  socitype);
+			return -ENODEV;
 	}
 
 	ret = brcmf_chip_cores_check(ci);
 	if (ret)
 		return ret;
 
-	/* assure chip is passive for core access */
+	/* Ensure chip is passive for core access */
 	brcmf_chip_set_passive(ci);
 
-	/* Call bus specific reset function now. Cores have been determined
-	 * but further access may require a chip specific reset at this point.
+	/* Cores have been probed, but further access may require a chip
+	 * specific reset at this point.
 	 */
 	if (ci->ops->reset) {
 		ci->ops->reset(ci->ctx, ci);
 		brcmf_chip_set_passive(ci);
 	}
 
-	return brcmf_chip_get_raminfo(ci);
+	ret = brcmf_chip_get_raminfo(ci);
+
+	printk("Found BCM%s, %s backplane, rev=%d\n", ci->name,
+		  socitype == SOCI_SB ? "SB" : "AXI",
+		  ci->chiprev);
+
+	return ret;
 }
 
 static void brcmf_chip_disable_arm(struct brcmf_chip *chip, u16 id)
@@ -1053,9 +1055,11 @@ static void brcmf_chip_disable_arm(struct brcmf_chip *chip, u16 id)
 	case BCMA_CORE_ARM_CR4:
 	case BCMA_CORE_ARM_CA7:
 
-		/* clear all IOCTL bits except HALT bit */
-		val = chip->ops->read32(chip->ctx, cpu->wrapbase + BCMA_IOCTL);
+		/* Clear all IOCTL bits except HALT bit */
+		val = brcmf_readl(chip, cpu->wrapbase + BCMA_IOCTL);
+
 		val &= ARMCR4_BCMA_IOCTL_CPUHALT;
+
 		brcmf_chip_resetcore(cpu, val, ARMCR4_BCMA_IOCTL_CPUHALT,
 				     ARMCR4_BCMA_IOCTL_CPUHALT);
 		break;
@@ -1076,17 +1080,14 @@ static int brcmf_chip_setup(struct brcmf_chip *chip)
 	base = cc->base;
 
 	/* get chipcommon capabilites */
-	chip->cc_caps = chip->ops->read32(chip->ctx,
-					 CORE_CC_REG(base, capabilities));
-	chip->cc_caps_ext = chip->ops->read32(chip->ctx,
-					     CORE_CC_REG(base,
+	chip->cc_caps = brcmf_readl(chip, CORE_CC_REG(base, capabilities));
+	chip->cc_caps_ext = brcmf_readl(chip, CORE_CC_REG(base,
 							 capabilities_ext));
 
 	/* get pmu caps & rev */
 	pmu = brcmf_chip_get_pmu(chip); /* after reading cc_caps_ext */
 	if (chip->cc_caps & CC_CAP_PMU) {
-		val = chip->ops->read32(chip->ctx,
-					CORE_CC_REG(pmu->base, pmucapabilities));
+		val = brcmf_readl(chip,	CORE_CC_REG(pmu->base, pmucapabilities));
 		chip->pmurev = val & PCAP_REV_MASK;
 		chip->pmucaps = val;
 	}
@@ -1232,8 +1233,8 @@ brcmf_chip_cm3_set_passive(struct brcmf_chip *chip)
 
 	/* disable bank #3 remap for this device */
 	if (chip->chip == BRCM_CC_43430_CHIP_ID) {
-		brcmf_chip_core_write32(core, SOCRAMREGOFFS(bankidx), 3);
-		brcmf_chip_core_write32(core, SOCRAMREGOFFS(bankpda), 0);
+		brcmf_writel(chip, core->base + SOCRAMREGOFFS(bankidx), 3);
+		brcmf_writel(chip, core->base + SOCRAMREGOFFS(bankpda), 0);
 	}
 }
 
@@ -1356,53 +1357,48 @@ bool brcmf_chip_set_active(struct brcmf_chip *chip, u32 rstvec)
 	return false;
 }
 
-bool brcmf_chip_sr_capable(struct brcmf_chip *chip)
+bool brcmf_chip_sr_capable(struct brcmf_chip *ci)
 {
-	struct brcmf_core *pmu = brcmf_chip_get_pmu(chip);
-	u32 base, addr, reg, pmu_cc3_mask = ~0;
+	struct brcmf_core *pmu = brcmf_chip_get_pmu(ci);
+	u32 base, reg, pmu_cc3_mask = ~0;
 
 	brcmf_dbg(TRACE, "Enter\n");
 
-	/* old chips with PMU version less than 17 don't support save restore */
-	if (chip->pmurev < 17)
+	/* Old chips with PMU version less than 17 don't support save restore */
+	if (ci->pmurev < 17)
 		return false;
 
-	base = brcmf_chip_get_chipcommon(chip)->base;
-
-	switch (chip->chip) {
+	switch (ci->chip) {
 	case BRCM_CC_4354_CHIP_ID:
 	case BRCM_CC_4356_CHIP_ID:
-		/* explicitly check SR engine enable bit */
+		/* Explicitly check SR engine enable bit */
 		pmu_cc3_mask = BIT(2);
-		/* fall-through */
+		/* Fall-through */
 	case BRCM_CC_43241_CHIP_ID:
 	case BRCM_CC_4335_CHIP_ID:
 	case BRCM_CC_4339_CHIP_ID:
 
-		/* read PMU chipcontrol register 3 */
-		addr = CORE_CC_REG(pmu->base, chipcontrol_addr);
-		chip->ops->write32(chip->ctx, addr, 3);
+		/* Read PMU chipcontrol register 3 */
+		brcmf_writel(ci, CORE_CC_REG(pmu->base, chipcontrol_addr), 3);
 
-		addr = CORE_CC_REG(pmu->base, chipcontrol_data);
-		reg = chip->ops->read32(chip->ctx, addr);
+		reg = brcmf_readl(ci, CORE_CC_REG(pmu->base, chipcontrol_data));
 
 		return (reg & pmu_cc3_mask) != 0;
 	case BRCM_CC_43430_CHIP_ID:
 
-		addr = CORE_CC_REG(base, sr_control1);
-		reg = chip->ops->read32(chip->ctx, addr);
+		base = brcmf_chip_get_chipcommon(ci)->base;
+
+		reg = brcmf_readl(ci, CORE_CC_REG(base, sr_control1));
 
 		return reg != 0;
 	default:
 
-		addr = CORE_CC_REG(pmu->base, pmucapabilities_ext);
-		reg = chip->ops->read32(chip->ctx, addr);
+		reg = brcmf_readl(ci, CORE_CC_REG(pmu->base, pmucapabilities_ext));
 
 		if ((reg & PCAPEXT_SR_SUPPORTED_MASK) == 0)
 			return false;
 
-		addr = CORE_CC_REG(pmu->base, retention_ctl);
-		reg = chip->ops->read32(chip->ctx, addr);
+		reg = brcmf_readl(ci, CORE_CC_REG(pmu->base, retention_ctl));
 
 		return (reg & (PMU_RCTL_MACPHY_DISABLE_MASK |
 			       PMU_RCTL_LOGIC_DISABLE_MASK)) == 0;
-- 
2.11.0

[PATCH 18/21] brcmfmac: rename ctx -> bus_priv

[Ian Molton]

---
 .../wireless/broadcom/brcm80211/brcmfmac/chip.c    | 24 +++++++++++-----------
 .../wireless/broadcom/brcm80211/brcmfmac/chip.h    | 16 +++++++--------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index bab1bb3e99ae..ae21f16b31de 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -225,18 +225,18 @@ struct sbsocramregs {
 
 static inline int brcmf_readl(struct brcmf_chip *chip, u32 addr)
 {
-	return chip->ops->read32(chip->ctx, addr);
+	return chip->ops->read32(chip->bus_priv, addr);
 }
 
 static inline void brcmf_writel(struct brcmf_chip *chip, u32 addr, u32 v)
 {
-	chip->ops->write32(chip->ctx, addr, v);
+	chip->ops->write32(chip->bus_priv, addr, v);
 }
 
 static inline void brcmf_writelp(struct brcmf_chip *chip, u32 addr, u32 v)
 {
-	chip->ops->write32(chip->ctx, addr, v);
-	chip->ops->read32(chip->ctx, addr);
+	chip->ops->write32(chip->bus_priv, addr, v);
+	chip->ops->read32(chip->bus_priv, addr);
 }
 
 static void brcmf_clear_bits(struct brcmf_chip *ci, u32 reg, u32 bits)
@@ -1025,7 +1025,7 @@ static int brcmf_chip_probe(struct brcmf_chip *ci)
 	 * specific reset at this point.
 	 */
 	if (ci->ops->reset) {
-		ci->ops->reset(ci->ctx, ci);
+		ci->ops->reset(ci->bus_priv, ci);
 		brcmf_chip_set_passive(ci);
 	}
 
@@ -1097,12 +1097,12 @@ static int brcmf_chip_setup(struct brcmf_chip *chip)
 
 	/* execute bus core specific setup */
 	if (chip->ops->setup)
-		ret = chip->ops->setup(chip->ctx, chip);
+		ret = chip->ops->setup(chip->bus_priv, chip);
 
 	return ret;
 }
 
-struct brcmf_chip *brcmf_chip_attach(void *ctx,
+struct brcmf_chip *brcmf_chip_attach(void *bus_priv,
 				     const struct brcmf_buscore_ops *ops)
 {
 	struct brcmf_chip *chip;
@@ -1126,9 +1126,9 @@ struct brcmf_chip *brcmf_chip_attach(void *ctx,
 	INIT_LIST_HEAD(&chip->cores);
 	chip->num_cores = 0;
 	chip->ops = ops;
-	chip->ctx = ctx;
+	chip->bus_priv = bus_priv;
 
-	err = ops->prepare(ctx);
+	err = ops->prepare(bus_priv);
 	if (err < 0)
 		goto fail;
 
@@ -1249,7 +1249,7 @@ static bool brcmf_chip_cm3_set_active(struct brcmf_chip *chip)
 		return false;
 	}
 
-	chip->ops->activate(chip->ctx, chip, 0);
+	chip->ops->activate(chip->bus_priv, chip, 0);
 
 	core = brcmf_chip_get_core(chip, BCMA_CORE_ARM_CM3);
 	brcmf_chip_resetcore(core, 0, 0, 0);
@@ -1275,7 +1275,7 @@ static bool brcmf_chip_cr4_set_active(struct brcmf_chip *chip, u32 rstvec)
 {
 	struct brcmf_core *core;
 
-	chip->ops->activate(chip->ctx, chip, rstvec);
+	chip->ops->activate(chip->bus_priv, chip, rstvec);
 
 	/* restore ARM */
 	core = brcmf_chip_get_core(chip, BCMA_CORE_ARM_CR4);
@@ -1302,7 +1302,7 @@ static bool brcmf_chip_ca7_set_active(struct brcmf_chip *chip, u32 rstvec)
 {
 	struct brcmf_core *core;
 
-	chip->ops->activate(chip->ctx, chip, rstvec);
+	chip->ops->activate(chip->bus_priv, chip, rstvec);
 
 	/* restore ARM */
 	core = brcmf_chip_get_core(chip, BCMA_CORE_ARM_CA7);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h
index 0babb328d155..8698abfb806e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.h
@@ -50,7 +50,7 @@ struct brcmf_chip {
 	void (*resetcore)(struct brcmf_core *core, u32 prereset, u32 reset,
 				u32 postreset);
 
-	void *ctx;
+	void *bus_priv;
 
 	u32 chip;
 	u32 chiprev;
@@ -94,15 +94,15 @@ struct brcmf_core {
  *	The callback should use the provided @rstvec when non-zero.
  */
 struct brcmf_buscore_ops {
-	u32 (*read32)(void *ctx, u32 addr);
-	void (*write32)(void *ctx, u32 addr, u32 value);
-	int (*prepare)(void *ctx);
-	int (*reset)(void *ctx, struct brcmf_chip *chip);
-	int (*setup)(void *ctx, struct brcmf_chip *chip);
-	void (*activate)(void *ctx, struct brcmf_chip *chip, u32 rstvec);
+	u32 (*read32)(void *bus_priv, u32 addr);
+	void (*write32)(void *bus_priv, u32 addr, u32 value);
+	int (*prepare)(void *bus_priv);
+	int (*reset)(void *bus_priv, struct brcmf_chip *chip);
+	int (*setup)(void *bus_priv, struct brcmf_chip *chip);
+	void (*activate)(void *bus_priv, struct brcmf_chip *chip, u32 rstvec);
 };
 
-struct brcmf_chip *brcmf_chip_attach(void *ctx,
+struct brcmf_chip *brcmf_chip_attach(void *bus_priv,
 				     const struct brcmf_buscore_ops *ops);
 void brcmf_chip_detach(struct brcmf_chip *chip);
 struct brcmf_core *brcmf_chip_get_core(struct brcmf_chip *chip, u16 coreid);
-- 
2.11.0

[PATCH 19/21] brcmfmac: Remove repeated and annoying calls to brcmf_chip_get_core()

[Ian Molton]

---
 .../wireless/broadcom/brcm80211/brcmfmac/sdio.c    | 23 ++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 2d819d44358a..95d325921ce2 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -435,6 +435,7 @@ struct brcmf_sdio_count {
 struct brcmf_sdio {
 	struct brcmf_sdio_dev *sdiodev;	/* sdio device handler */
 	struct brcmf_chip *ci;	/* Chip info struct */
+	struct brcmf_core *sdio_core; /* sdio core info struct */
 
 	u32 hostintmask;	/* Copy of Host Interrupt Mask */
 	atomic_t intstatus;	/* Intstatus bits (events) pending */
@@ -659,10 +660,9 @@ static bool data_ok(struct brcmf_sdio *bus)
  */
 static int r_sdreg32(struct brcmf_sdio *bus, u32 *regvar, u32 offset)
 {
-	struct brcmf_core *core;
+	struct brcmf_core *core = bus->sdio_core;
 	int ret;
 
-	core = brcmf_chip_get_core(bus->ci, BCMA_CORE_SDIO_DEV);
 	*regvar = brcmf_sdiod_regrl(bus->sdiodev, core->base + offset, &ret);
 
 	return ret;
@@ -670,10 +670,9 @@ static int r_sdreg32(struct brcmf_sdio *bus, u32 *regvar, u32 offset)
 
 static int w_sdreg32(struct brcmf_sdio *bus, u32 regval, u32 reg_offset)
 {
-	struct brcmf_core *core;
+	struct brcmf_core *core = bus->sdio_core;
 	int ret;
 
-	core = brcmf_chip_get_core(bus->ci, BCMA_CORE_SDIO_DEV);
 	brcmf_sdiod_regwl(bus->sdiodev, core->base + reg_offset, regval, &ret);
 
 	return ret;
@@ -2480,12 +2479,11 @@ static inline void brcmf_sdio_clrintr(struct brcmf_sdio *bus)
 
 static int brcmf_sdio_intr_rstatus(struct brcmf_sdio *bus)
 {
-	struct brcmf_core *buscore;
+	struct brcmf_core *buscore = bus->sdio_core;
 	u32 addr;
 	unsigned long val;
 	int ret;
 
-	buscore = brcmf_chip_get_core(bus->ci, BCMA_CORE_SDIO_DEV);
 	addr = buscore->base + __sd_reg(intstatus);
 
 	val = brcmf_sdiod_regrl(bus->sdiodev, addr, &ret);
@@ -3362,13 +3360,14 @@ static void brcmf_sdio_sr_init(struct brcmf_sdio *bus)
 /* enable KSO bit */
 static int brcmf_sdio_kso_init(struct brcmf_sdio *bus)
 {
+	struct brcmf_core *core = bus->sdio_core;
 	u8 val;
 	int err = 0;
 
 	brcmf_dbg(TRACE, "Enter\n");
 
 	/* KSO bit added in SDIO core rev 12 */
-	if (brcmf_chip_get_core(bus->ci, BCMA_CORE_SDIO_DEV)->rev < 12)
+	if (core->rev < 12)
 		return 0;
 
 	val = brcm_sdio_func1_rb(bus->sdiodev, SBSDIO_FUNC1_SLEEPCSR, &err);
@@ -3397,6 +3396,7 @@ static int brcmf_sdio_bus_preinit(struct device *dev)
 	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;
+	struct brcmf_core *core = bus->sdio_core;
 	uint pad_size;
 	u32 value;
 	int err;
@@ -3405,7 +3405,7 @@ static int brcmf_sdio_bus_preinit(struct device *dev)
 	 * a device perspective, ie. bus:txglom affects the
 	 * bus transfers from device to host.
 	 */
-	if (brcmf_chip_get_core(bus->ci, BCMA_CORE_SDIO_DEV)->rev < 12) {
+	if (core->rev < 12) {
 		/* for sdio core rev < 12, disable txgloming */
 		value = 0;
 		err = brcmf_iovar_data_set(dev, "bus:txglom", &value,
@@ -3743,11 +3743,10 @@ static void brcmf_sdio_buscore_activate(void *ctx, struct brcmf_chip *chip,
 					u32 rstvec)
 {
 	struct brcmf_sdio_dev *sdiodev = ctx;
-	struct brcmf_core *core;
+	struct brcmf_core *core = sdiodev->bus->sdio_core;
 	u32 reg_addr;
 
 	/* clear all interrupts */
-	core = brcmf_chip_get_core(chip, BCMA_CORE_SDIO_DEV);
 	reg_addr = core->base + __sd_reg(intstatus);
 	brcmf_sdiod_regwl(sdiodev, reg_addr, 0xFFFFFFFF, NULL);
 
@@ -3828,6 +3827,10 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
 		bus->ci = NULL;
 		goto fail;
 	}
+
+	/* Pick up the SDIO core info struct from chip.c */
+	bus->sdio_core = brcmf_chip_get_core(bus->ci, BCMA_CORE_SDIO_DEV);
+
 	sdiodev->settings = brcmf_get_module_param(sdiodev->dev,
 						   BRCMF_BUSTYPE_SDIO,
 						   bus->ci->chip,
-- 
2.11.0

[PATCH 20/21] brcmfmac: general cleanup

[Ian Molton]

---
 .../wireless/broadcom/brcm80211/brcmfmac/sdio.c    | 24 ++++++++++++++--------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 95d325921ce2..45b8000680c0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -639,11 +639,13 @@ static struct brcmf_firmware_mapping brcmf_sdio_fwnames[] = {
 
 static void pkt_align(struct sk_buff *p, int len, int align)
 {
-	uint datalign;
-	datalign = (unsigned long)(p->data);
+	uint datalign = (unsigned long)(p->data);
+
 	datalign = roundup(datalign, (align)) - datalign;
+
 	if (datalign)
 		skb_pull(p, datalign);
+
 	__skb_trim(p, len);
 }
 
@@ -2463,10 +2465,9 @@ static void brcmf_sdio_bus_stop(struct device *dev)
 
 static inline void brcmf_sdio_clrintr(struct brcmf_sdio *bus)
 {
-	struct brcmf_sdio_dev *sdiodev;
+	struct brcmf_sdio_dev *sdiodev = bus->sdiodev;
 	unsigned long flags;
 
-	sdiodev = bus->sdiodev;
 	if (sdiodev->oob_irq_requested) {
 		spin_lock_irqsave(&sdiodev->irq_en_lock, flags);
 		if (!sdiodev->irq_en && !atomic_read(&bus->ipend)) {
@@ -3762,15 +3763,23 @@ static u32 brcmf_sdio_buscore_read32(void *ctx, u32 addr)
 	u32 val, rev;
 
 	val = brcmf_sdiod_regrl(sdiodev, addr, NULL);
+
+	/* Force 4339 chips over rev2 to use the same ID */
+	/* This is borderline tolerable whilst there is only two exceptions */
+	/* But could be handled better */
 	if ((sdiodev->func[0]->device == SDIO_DEVICE_ID_BROADCOM_4335_4339 ||
-	     sdiodev->func[0]->device == SDIO_DEVICE_ID_BROADCOM_4339) &&
-	    addr == CORE_CC_REG(SI_ENUM_BASE, chipid)) {
+		sdiodev->func[0]->device == SDIO_DEVICE_ID_BROADCOM_4339) &&
+		addr == CORE_CC_REG(SI_ENUM_BASE, chipid)) {
+
 		rev = (val & CID_REV_MASK) >> CID_REV_SHIFT;
+
 		if (rev >= 2) {
 			val &= ~CID_ID_MASK;
 			val |= BRCM_CC_4339_CHIP_ID;
 		}
+
 	}
+
 	return val;
 }
 
@@ -3801,9 +3810,6 @@ brcmf_sdio_probe_attach(struct brcmf_sdio *bus)
 	sdiodev = bus->sdiodev;
 	sdio_claim_host(sdiodev->func[1]);
 
-	pr_debug("F1 signature read @0x18000000=0x%4x\n",
-		 brcmf_sdiod_regrl(sdiodev, SI_ENUM_BASE, NULL));
-
 	/*
 	 * Force PLL off until brcmf_chip_attach()
 	 * programs PLL control regs
-- 
2.11.0

[PATCH 21/21] brcmfmac: rename horridly named IO functions

[Ian Molton]

---
 .../wireless/broadcom/brcm80211/brcmfmac/sdio.c    | 24 +++++++++++-----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 45b8000680c0..b41f2afe4d5a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -660,7 +660,7 @@ static bool data_ok(struct brcmf_sdio *bus)
  * Reads a register in the SDIO hardware block. This block occupies a series of
  * adresses on the 32 bit backplane bus.
  */
-static int r_sdreg32(struct brcmf_sdio *bus, u32 *regvar, u32 offset)
+static int brcm_sdio_readl(struct brcmf_sdio *bus, u32 *regvar, u32 offset)
 {
 	struct brcmf_core *core = bus->sdio_core;
 	int ret;
@@ -670,7 +670,7 @@ static int r_sdreg32(struct brcmf_sdio *bus, u32 *regvar, u32 offset)
 	return ret;
 }
 
-static int w_sdreg32(struct brcmf_sdio *bus, u32 regval, u32 reg_offset)
+static int brcm_sdio_writel(struct brcmf_sdio *bus, u32 regval, u32 reg_offset)
 {
 	struct brcmf_core *core = bus->sdio_core;
 	int ret;
@@ -1082,10 +1082,10 @@ static u32 brcmf_sdio_hostmail(struct brcmf_sdio *bus)
 	brcmf_dbg(SDIO, "Enter\n");
 
 	/* Read mailbox data and ack that we did so */
-	ret = r_sdreg32(bus, &hmb_data,	__sd_reg(tohostmailboxdata));
+	ret = brcm_sdio_readl(bus, &hmb_data, __sd_reg(tohostmailboxdata));
 
 	if (ret == 0)
-		w_sdreg32(bus, SMB_INT_ACK, __sd_reg(tosbmailbox));
+		brcm_sdio_writel(bus, SMB_INT_ACK, __sd_reg(tosbmailbox));
 	bus->sdcnt.f1regdata += 2;
 
 	/* Dongle recomposed rx frames, accept them again */
@@ -1195,7 +1195,7 @@ static void brcmf_sdio_rxfail(struct brcmf_sdio *bus, bool abort, bool rtx)
 
 	if (rtx) {
 		bus->sdcnt.rxrtx++;
-		err = w_sdreg32(bus, SMB_NAK, __sd_reg(tosbmailbox));
+		err = brcm_sdio_writel(bus, SMB_NAK, __sd_reg(tosbmailbox));
 
 		bus->sdcnt.f1regdata++;
 		if (err == 0)
@@ -2316,7 +2316,7 @@ static uint brcmf_sdio_sendfromq(struct brcmf_sdio *bus, uint maxframes)
 		if (!bus->intr) {
 			/* Check device status, signal pending interrupt */
 			sdio_claim_host(bus->sdiodev->func[1]);
-			ret = r_sdreg32(bus, &intstatus, __sd_reg(intstatus));
+			ret = brcm_sdio_readl(bus, &intstatus, __sd_reg(intstatus));
 			sdio_release_host(bus->sdiodev->func[1]);
 			bus->sdcnt.f2txdata++;
 			if (ret != 0)
@@ -2422,7 +2422,7 @@ static void brcmf_sdio_bus_stop(struct device *dev)
 		brcmf_sdio_bus_sleep(bus, false, false);
 
 		/* Disable and clear interrupts at the chip level also */
-		w_sdreg32(bus, 0, __sd_reg(hostintmask));
+		brcm_sdio_writel(bus, 0, __sd_reg(hostintmask));
 		local_hostintmask = bus->hostintmask;
 		bus->hostintmask = 0;
 
@@ -2441,7 +2441,7 @@ static void brcmf_sdio_bus_stop(struct device *dev)
 		sdio_disable_func(sdiodev->func[SDIO_FUNC_2]);
 
 		/* Clear any pending interrupts now that F2 is disabled */
-		w_sdreg32(bus, local_hostintmask, __sd_reg(intstatus));
+		brcm_sdio_writel(bus, local_hostintmask, __sd_reg(intstatus));
 
 		sdio_release_host(sdiodev->func[1]);
 	}
@@ -2562,9 +2562,9 @@ static void brcmf_sdio_dpc(struct brcmf_sdio *bus)
 	 */
 	if (intstatus & I_HMB_FC_CHANGE) {
 		intstatus &= ~I_HMB_FC_CHANGE;
-		err = w_sdreg32(bus, I_HMB_FC_CHANGE, __sd_reg(intstatus));
+		err = brcm_sdio_writel(bus, I_HMB_FC_CHANGE, __sd_reg(intstatus));
 
-		err = r_sdreg32(bus, &newstatus, __sd_reg(intstatus));
+		err = brcm_sdio_readl(bus, &newstatus, __sd_reg(intstatus));
 		bus->sdcnt.f1regdata += 2;
 		atomic_set(&bus->fcstate,
 			   !!(newstatus & (I_HMB_FC_STATE | I_HMB_FC_CHANGE)));
@@ -4033,7 +4033,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 	}
 
 	/* Enable function 2 (frame transfers) */
-	w_sdreg32(bus, SDPCM_PROT_VERSION << SMB_DATA_VERSION_SHIFT,
+	brcm_sdio_writel(bus, SDPCM_PROT_VERSION << SMB_DATA_VERSION_SHIFT,
 		  __sd_reg(tosbmailboxdata));
 	err = sdio_enable_func(sdiodev->func[SDIO_FUNC_2]);
 
@@ -4044,7 +4044,7 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
 	if (!err) {
 		/* Set up the interrupt mask and enable interrupts */
 		bus->hostintmask = HOSTINTMASK;
-		w_sdreg32(bus, bus->hostintmask, __sd_reg(hostintmask));
+		brcm_sdio_writel(bus, bus->hostintmask, __sd_reg(hostintmask));
 
 		brcm_sdio_func1_wb(sdiodev, SBSDIO_WATERMARK, 8, &err);
 	} else {
-- 
2.11.0

[PATCH] brcmfmac: fix boolreturn.cocci warnings

[kbuild test robot]

drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c:283:9-10: WARNING: return of 0/1 in function 'brcmf_chip_axi_iscoreup' with return type bool

 Return statements in functions returning bool should use
 true/false instead of 1/0.
Generated by: scripts/coccinelle/misc/boolreturn.cocci

Fixes: f9996a7ac81f ("brcmfmac: HUGE cleanup of IO access functions.")
CC: Ian Molton <ian@mnementh.co.uk>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 chip.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -280,7 +280,7 @@ static bool brcmf_chip_axi_iscoreup(stru
 	val = brcmf_readl(ci, core->wrapbase + BCMA_IOCTL);
 
 	if((val & BCMA_IOCTL_FGC) || !(val & BCMA_IOCTL_CLK))
-		return 0;
+		return false;
 
 	/* Is core in reset? */
 	val = brcmf_readl(ci, core->wrapbase + BCMA_RESET_CTL);

Re: [PATCH 17/21] brcmfmac: HUGE cleanup of IO access functions.

[kbuild test robot]

Hi Ian,

[auto build test WARNING on wireless-drivers/master]
[also build test WARNING on v4.13-rc1 next-20170714]
[cannot apply to wireless-drivers-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ian-Molton/net-brcmfmac-Write-function-depends-on-size-of-regs-not-types/20170716-195206
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git master


coccinelle warnings: (new ones prefixed by >>)

>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c:283:9-10: WARNING: return of 0/1 in function 'brcmf_chip_axi_iscoreup' with return type bool

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: RFC: Broadcom fmac wireless driver cleanup

[Rafał Miłecki]

On 16 July 2017 at 13:21, Ian Molton <ian@mnementh.co.uk> wrote:
> I've been doing some cleanups in the broadcome brcmfmac driver, trying to
> understand how it works.
>
> I think this makes the driver a *lot* more readable.
>
> Of note:
>
> * Gets rid of the arbitrary and completely 1:1 mapping of
>  struct brcmf_{core,chip}_priv/struct brcmf_{core,chip} structures
>  which add unreadability whilst offering no real seperation.
>
> * The patch titled HACK - stabilise the value of ->sbwad in use
>  highlights an issue that is either bizarre undocumented behaviour or a
>  genuine bug - without datasheets, I dont know. Essentially the value of =
sbwad
>  is taken as the base address in several functions, even though it is sub=
ject
>  to change should other IO occur that moves the window offset
>
> Obviously this is a first cut at this and needs substantial cleanup itsel=
f,
> however I wanted to get some idea of wether I should continue working on =
this.

I looked at 4 random patches and none got any description. Not to
mention their chaotic subjects. In this state I can't even review it.
If you want to have some change accepted, you've to convince us it's
needed. Work on cleaning your patches and resend them. You also need
to signed off your changes.

--=20
Rafa=C5=82

Re: RFC: Broadcom fmac wireless driver cleanup

[James Hughes]

On 17 July 2017 at 05:53, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> wrote:
> On 16 July 2017 at 13:21, Ian Molton <ian@mnementh.co.uk> wrote:
>> I've been doing some cleanups in the broadcome brcmfmac driver, trying t=
o
>> understand how it works.
>>
>> I think this makes the driver a *lot* more readable.
>>
>> Of note:
>>
>> * Gets rid of the arbitrary and completely 1:1 mapping of
>>  struct brcmf_{core,chip}_priv/struct brcmf_{core,chip} structures
>>  which add unreadability whilst offering no real seperation.
>>
>> * The patch titled HACK - stabilise the value of ->sbwad in use
>>  highlights an issue that is either bizarre undocumented behaviour or a
>>  genuine bug - without datasheets, I dont know. Essentially the value of=
 sbwad
>>  is taken as the base address in several functions, even though it is su=
bject
>>  to change should other IO occur that moves the window offset
>>
>> Obviously this is a first cut at this and needs substantial cleanup itse=
lf,
>> however I wanted to get some idea of wether I should continue working on=
 this.
>
> I looked at 4 random patches and none got any description. Not to
> mention their chaotic subjects. In this state I can't even review it.
> If you want to have some change accepted, you've to convince us it's
> needed. Work on cleaning your patches and resend them. You also need
> to signed off your changes.
>
> --
> Rafa=C5=82

As someone who is interested in any bug fixes to this driver (Device
is used on Raspberry Pi3/0W and we have a number of issues reported
which we are actively investigating), it would be very useful to more
clearly split out any actual fixes vs simply tidying up (Yes, I agree
the driver is mostly incomprehensible). Perhaps asking the
list/maintainers for comments on any located issues/bugs fixes would
be a useful starting point, along with ensuring the description gives
a good explanation of what the suspect issue is.

James

Re: RFC: Broadcom fmac wireless driver cleanup

[Ian Molton]

On 17/07/17 10:13, James Hughes wrote:

> As someone who is interested in any bug fixes to this driver (Device
> is used on Raspberry Pi3/0W and we have a number of issues reported
> which we are actively investigating), it would be very useful to more
> clearly split out any actual fixes vs simply tidying up (Yes, I agree
> the driver is mostly incomprehensible).

I don't think there are any actual *fixes* in this RFC series. Its all
cleanup.

The only issue I highlighted in my covering email - the patch titled
"HACK" was the only issue I uncovered thus far in that code. I dont know
the correct solution, although I can *guess* it, which is not good
enough, IMO.

If no-one has the docs for this chip, thats a bad state of affairs. Is
there anything circulating?

> Perhaps asking the
> list/maintainers for comments on any located issues/bugs fixes would
> be a useful starting point,

Thats *literally* what RFC means, is it not?

> along with ensuring the description gives a good explanation of what
> the suspect issue is.

Yes, absolutely - for a fully signed off series, or a particularly
complex bit of code, sure.

I think each of these patches is easily reviewable. None of them are
complex, just huge.

As yet, I have *zero* idea that the maintainers are interested in taking
the driver in the direction I'm going. It looks a LOT like a typical
corporate code-dump, and unless I'm convinced that sending carefully
polished patches is worth it, I don't see why I should put the effort in.

Trust goes both ways in this process. Give me a sign that I should carry
on with this work. If its got no hope of ever going upstream, all I'm
doing is wasting time.

-Ian

Re: RFC: Broadcom fmac wireless driver cleanup

[Rafał Miłecki]

Gmail seems to mark your replies as spam :(

On 17 July 2017 at 11:34, Ian Molton <ian@mnementh.co.uk> wrote:
> On 17/07/17 05:53, Rafa=C5=82 Mi=C5=82ecki wrote:
>> I looked at 4 random patches and none got any description. Not to
>> mention their chaotic subjects. In this state I can't even review it.
>> If you want to have some change accepted, you've to convince us it's
>> needed. Work on cleaning your patches and resend them. You also need
>> to signed off your changes.
>
> This isn't my first rodeo. I know there are only outline descriptions,
> and no Sob.
>
> Thats because this is an RFC. You sign off *finished* work.

Sending signed patches, include RFCs is much more convenient. It
allows e.g. other people to pick your work if you won't manage to get
in accepted for some reason.


> This is a codebase I'm not 100% familiar with, and I don't know the
> maintainers - Im not going to polish patches if they aren't then going
> to get accepted upstream.
>
> I'm looking for comments on the actual *code*. Review requires *reading*
> it. Review is not just "I read the description and it looked ok at the
> time" - Thats clearly how this code got into this state in the first plac=
e.

I don't expect patches to be perfectly polished at RFC phase. I also
never said I'm interested in description only. Don't expect to get
nicely described hack to get accepted for that reason.

Description is supposed to provide a context for the changes. It's
easier to review *code changes* knowing what you are trying to
fix/achieve. It saves a lot of guessing time.


> Honestly, the patch robot has given more useful feedback than the humans
> on here thus far.
>
> But hey, if thats how patch submission works these days... I'll add some
> descriptions. But I'd better not be polishing this stuff for no reason.

Insulting maintainers may not be the best way of getting your stuff
reviewed & accepted.

--=20
Rafa=C5=82

Re: RFC: Broadcom fmac wireless driver cleanup

[Arend van Spriel]

On 16-07-17 13:21, Ian Molton wrote:
> Hi folks,
> 
> I've been doing some cleanups in the broadcome brcmfmac driver, trying to
> understand how it works.
> 
> I think this makes the driver a *lot* more readable.

Everyone is entitled to his own opinion. While skimming the patches
terms like horrid, abomination, brain damaged does not really get me
motivated in reviewing it let alone accepting it. Especially when I just
start my vacation, but you could not have known ;-)

> Of note:
> 
> * Gets rid of the arbitrary and completely 1:1 mapping of
>  struct brcmf_{core,chip}_priv/struct brcmf_{core,chip} structures
>  which add unreadability whilst offering no real seperation.

It is maybe 1:1 but it assures that whatever is in the priv structures
in only accessible in chip.c. Why would that not offer any real separation.

> * The patch titled HACK - stabilise the value of ->sbwad in use
>  highlights an issue that is either bizarre undocumented behaviour or a
>  genuine bug - without datasheets, I dont know. Essentially the value of sbwad
>  is taken as the base address in several functions, even though it is subject
>  to change should other IO occur that moves the window offset

Ok. This needs to be looked at, but as I recall sbwad is only changing
when needed. All IO occurs under claimed host lock so I don't expect
there is a bug here, but will look at it more closely.

> Obviously this is a first cut at this and needs substantial cleanup itself,
> however I wanted to get some idea of wether I should continue working on this.

Now this is where a commit message would really help to explain your
train of thought. What is obvious to you, may not be to others. For
instance changing the order of parameters in a function is simply absurd
although that is my opinion.

> I only have a 43430 SDIO WiFi / BT chip to test on, but other chips *should*
> be unaffected by these changes.

I have a decent amount of SDIO chipsets so count on it that I will run
this patch series when I return from my vacation.

I just noticed some earlier reactions and it seems your are easily
offended starting to use *star* quotes. No need to start shouting. There
is stuff in there that I am happy to acknowledge, but some I think is a
matter of taste. Although mostly there are no arguments given to get the
discussion going. I am sure you have them in mind so don't qualm and
just spew them.

Regards,
Arend

Re: RFC: Broadcom fmac wireless driver cleanup

[Ian Molton]

On 17/07/17 13:41, Arend van Spriel wrote:
>> * Gets rid of the arbitrary and completely 1:1 mapping of
>>  struct brcmf_{core,chip}_priv/struct brcmf_{core,chip} structures
>>  which add unreadability whilst offering no real seperation.
> It is maybe 1:1 but it assures that whatever is in the priv structures
> in only accessible in chip.c. Why would that not offer any real separation.

Ok, I'm going to keep this to technical points from now on, everyones
had a chance to say how they feel about it now :)

To address your specific comment above - "Why would that not offer any
real separation"

The two structures *always* exist together. And yes, preventing access
to the members that we don't want accessed from the wrong layer would
be *possibly* worthwhile, however the driver code as it stands
*constantly* short circuits that with the use of offsetof().

This makes any minimal protection against poor coding practice
completely irrelevant, at the same time as making the code unreadable.

If the separation is to stay, it needs to be re-written such that
offsetof() is not required throughout the code.

I've re-done my patchset, which I'll resubmit in just a moment.

My preferred solution is still removal, as nothing outside this driver
will ever touch these structures anyway, so all they are doing, other
than making the code impossible to maintain, is protecting us from no-one.

-Ian

Re: RFC: Broadcom fmac wireless driver cleanup

[Ian Molton]

On 17/07/17 13:41, Arend van Spriel wrote:
> For
> instance changing the order of parameters in a function is simply absurd
> although that is my opinion.

If one function has its parameters in a different order to all its
compatriots, I would consider this a highly likely scenario for someone
to mess up and use it incorrectly.

-Ian

Re: RFC: Broadcom fmac wireless driver cleanup

[Ian Molton]

On 17/07/17 13:41, Arend van Spriel wrote:

> I am sure you have them in mind so don't qualm and just spew them.

Done. See later patchset.

-Ian

Re: RFC: Broadcom fmac wireless driver cleanup

[Ian Molton]

On 17/07/17 16:56, Ian Molton wrote:
> The two structures *always* exist together. And yes, preventing access
> to the members that we don't want accessed from the wrong layer would
> be *possibly* worthwhile, however the driver code as it stands
> *constantly* short circuits that with the use of offsetof().

inb4: misthought there - it doesnt use offsetof() for that, but my point
stands.

Look, for example, at the core structs:

struct brcmf_core_priv {
	struct brcmf_core pub;
	u32 wrapbase;
	struct list_head list;
	struct brcmf_chip_priv *chip;
};

and

struct brcmf_core {
 	u16 id;
 	u16 rev;
 	u32 base;
}


What is the useful separation being achieved here? both base and
wrapbase are pointers to core address space.

Apart from that, nothing but the core ID and revision are separated from
the other data.

so we have two address space pointers, which are separated for no good
reason, and two derived values which its pretty much irrelevant if
they're changed.

The only other information in there is the list management structures,
which are used in public structures throughout the linux kernel - its
common practice.

A quick grep shows that the struct brcmf_core does not get used outside
chip.c, and struct brcmf_chip is used purely as a pointer in a less than
a handful of places in the bus drivers, which are barely interested in
their internal data (chiprev, etc.).

For these tiny number of cases do we really want to make all of chip.c
and chip.h illegible?

eg:

-	chip->ops->activate(chip->ctx, &chip->pub, rstvec);
+	chip->ops->activate(chip->ctx, chip, rstvec);

Which I was planning to submit a further patch to convert it to simply:

chip->ops->activate(chip, rstvec);

Which is *so* much more readable.

-Ian

Re: RFC: Broadcom fmac wireless driver cleanup

[Kalle Valo]

Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> writes:

> On 16 July 2017 at 13:21, Ian Molton <ian@mnementh.co.uk> wrote:
>> I've been doing some cleanups in the broadcome brcmfmac driver, trying to
>> understand how it works.
>>
>> I think this makes the driver a *lot* more readable.
>>
>> Of note:
>>
>> * Gets rid of the arbitrary and completely 1:1 mapping of
>>  struct brcmf_{core,chip}_priv/struct brcmf_{core,chip} structures
>>  which add unreadability whilst offering no real seperation.
>>
>> * The patch titled HACK - stabilise the value of ->sbwad in use
>>  highlights an issue that is either bizarre undocumented behaviour or a
>>  genuine bug - without datasheets, I dont know. Essentially the value of=
 sbwad
>>  is taken as the base address in several functions, even though it is su=
bject
>>  to change should other IO occur that moves the window offset
>>
>> Obviously this is a first cut at this and needs substantial cleanup itse=
lf,
>> however I wanted to get some idea of wether I should continue working on=
 this.
>
> I looked at 4 random patches and none got any description. Not to
> mention their chaotic subjects. In this state I can't even review it.
> If you want to have some change accepted, you've to convince us it's
> needed. Work on cleaning your patches and resend them. You also need
> to signed off your changes.

And try to stick with max 10-12 patches per patchset rule. More than
that and it gets difficult for everyone (submitter, reviewers and
maintainers). And if you are a new with the subsystem, like you are
here, start with baby steps: send one patch, wait for it to get applied,
send two, wait, send four, wait etc. This way you learn how things work
without putting too much burden on the maintainers and things go forward
smoothly.

--=20
Kalle Valo