linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] brcmfmac: restructuring sdio access functions
@ 2017-11-13 20:35 Arend van Spriel
  2017-11-13 20:35 ` [PATCH 01/10] brcmfmac: Fix parameter order in brcmf_sdiod_f0_writeb() Arend van Spriel
                   ` (10 more replies)
  0 siblings, 11 replies; 16+ messages in thread
From: Arend van Spriel @ 2017-11-13 20:35 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel

These patches were originally submitted by Ian Molton a while ago so decided
to resubmit them. This is a first series of 10 patches. Tested using BCM4354
device which showed no regression.

It is intended for 4.16 (no rush ;-) ) and applies to the master branch of
the wireless-drivers-next repository.

Ian Molton (10):
  brcmfmac: Fix parameter order in brcmf_sdiod_f0_writeb()
  brcmfmac: Register sizes on hardware are not dependent on compiler
    types
  brcmfmac: Split brcmf_sdiod_regrw_helper() up.
  brcmfmac: Clean up brcmf_sdiod_set_sbaddr_window()
  brcmfmac: Remove dead IO code
  brcmfmac: Remove bandaid for SleepCSR
  brcmfmac: Remove brcmf_sdiod_request_data()
  brcmfmac: Fix asymmetric IO functions.
  brcmfmac: Remove noisy debugging.
  brcmfmac: Rename bcmerror to err

 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 280 +++++++++------------
 .../wireless/broadcom/brcm80211/brcmfmac/sdio.h    |   5 +-
 2 files changed, 114 insertions(+), 171 deletions(-)

--
1.9.1

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

* [PATCH 01/10] brcmfmac: Fix parameter order in brcmf_sdiod_f0_writeb()
  2017-11-13 20:35 [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
@ 2017-11-13 20:35 ` Arend van Spriel
  2017-12-07 13:12   ` [01/10] " Kalle Valo
  2017-11-13 20:35 ` [PATCH 02/10] brcmfmac: Register sizes on hardware are not dependent on compiler types Arend van Spriel
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Arend van Spriel @ 2017-11-13 20:35 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Ian Molton, Arend van Spriel

From: Ian Molton <ian@mnementh.co.uk>

All the other IO functions are the other way round in this
driver. Make this one match.

Signed-off-by: Ian Molton <ian@mnementh.co.uk>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index cd58732..a8976a76 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -230,8 +230,8 @@ 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;
 
@@ -269,8 +269,8 @@ 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);
-- 
1.9.1

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

* [PATCH 02/10] brcmfmac: Register sizes on hardware are not dependent on compiler types
  2017-11-13 20:35 [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
  2017-11-13 20:35 ` [PATCH 01/10] brcmfmac: Fix parameter order in brcmf_sdiod_f0_writeb() Arend van Spriel
@ 2017-11-13 20:35 ` Arend van Spriel
  2017-11-13 20:35 ` [PATCH 03/10] brcmfmac: Split brcmf_sdiod_regrw_helper() up Arend van Spriel
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Arend van Spriel @ 2017-11-13 20:35 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Ian Molton, Arend van Spriel

From: Ian Molton <ian@mnementh.co.uk>

The 4 IO functions in this patch are incorrect as they use compiler types
to determine how many bytes to send to the hardware.

Signed-off-by: Ian Molton <ian@mnementh.co.uk>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index a8976a76..25b5e9b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -264,7 +264,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);
@@ -278,13 +278,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
@@ -368,7 +368,7 @@ static int brcmf_sdiod_regrw_helper(struct brcmf_sdio_dev *sdiodev, u32 addr,
 	for (i = 0; i < 3; i++) {
 		err = brcmf_sdiod_regrw_helper(sdiodev,
 					       SBSDIO_FUNC1_SBADDRLOW + i,
-					       sizeof(u8), &addr[i], true);
+					       1, &addr[i], true);
 		if (err) {
 			brcmf_err("failed at addr: 0x%0x\n",
 				  SBSDIO_FUNC1_SBADDRLOW + i);
@@ -407,7 +407,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,
+	retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 1, &data,
 					  false);
 	brcmf_dbg(SDIO, "data:0x%02x\n", data);
 
@@ -423,10 +423,10 @@ 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, 4, &addr);
 	if (retval)
 		goto done;
-	retval = brcmf_sdiod_regrw_helper(sdiodev, addr, sizeof(data), &data,
+	retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 4, &data,
 					  false);
 	brcmf_dbg(SDIO, "data:0x%08x\n", data);
 
@@ -443,7 +443,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_regrw_helper(sdiodev, addr, sizeof(data), &data,
+	retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 1, &data,
 					  true);
 	if (ret)
 		*ret = retval;
@@ -455,10 +455,10 @@ 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, 4, &addr);
 	if (retval)
 		goto done;
-	retval = brcmf_sdiod_regrw_helper(sdiodev, addr, sizeof(data), &data,
+	retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 4, &data,
 					  true);
 
 done:
@@ -876,7 +876,7 @@ int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, uint fn)
 
 	/* issue abort cmd52 command through F0 */
 	brcmf_sdiod_request_data(sdiodev, SDIO_FUNC_0, SDIO_CCCR_ABORT,
-				 sizeof(t_func), &t_func, true);
+				 1, &t_func, true);
 
 	brcmf_dbg(SDIO, "Exit\n");
 	return 0;
-- 
1.9.1

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

* [PATCH 03/10] brcmfmac: Split brcmf_sdiod_regrw_helper() up.
  2017-11-13 20:35 [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
  2017-11-13 20:35 ` [PATCH 01/10] brcmfmac: Fix parameter order in brcmf_sdiod_f0_writeb() Arend van Spriel
  2017-11-13 20:35 ` [PATCH 02/10] brcmfmac: Register sizes on hardware are not dependent on compiler types Arend van Spriel
@ 2017-11-13 20:35 ` Arend van Spriel
  2017-11-13 20:35 ` [PATCH 04/10] brcmfmac: Clean up brcmf_sdiod_set_sbaddr_window() Arend van Spriel
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Arend van Spriel @ 2017-11-13 20:35 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Ian Molton, Arend van Spriel

From: Ian Molton <ian@mnementh.co.uk>

This large function is concealing a LOT of obscure logic about
how the hardware functions. Time to split it up.

This first patch splits the function into two pieces - read and write,
doing away with the rw flag in the process.

Signed-off-by: Ian Molton <ian@mnementh.co.uk>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 94 +++++++++++++++++-----
 1 file changed, 73 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 25b5e9b..3acc0ff 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;
 }
 
@@ -366,13 +420,11 @@ static int brcmf_sdiod_regrw_helper(struct brcmf_sdio_dev *sdiodev, u32 addr,
 	addr[2] = (address >> 24) & SBSDIO_SBADDRHIGH_MASK;
 
 	for (i = 0; i < 3; i++) {
-		err = brcmf_sdiod_regrw_helper(sdiodev,
-					       SBSDIO_FUNC1_SBADDRLOW + i,
-					       1, &addr[i], true);
+		brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_SBADDRLOW + i, addr[i],
+				  &err);
 		if (err) {
 			brcmf_err("failed at addr: 0x%0x\n",
 				  SBSDIO_FUNC1_SBADDRLOW + i);
-			break;
 		}
 	}
 
@@ -407,8 +459,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, 1, &data,
-					  false);
+	retval = brcmf_sdiod_reg_read(sdiodev, addr, 1, &data);
 	brcmf_dbg(SDIO, "data:0x%02x\n", data);
 
 	if (ret)
@@ -426,8 +477,9 @@ u32 brcmf_sdiod_regrl(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
 	retval = brcmf_sdiod_addrprep(sdiodev, 4, &addr);
 	if (retval)
 		goto done;
-	retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 4, &data,
-					  false);
+
+	retval = brcmf_sdiod_reg_read(sdiodev, addr, 4, &data);
+
 	brcmf_dbg(SDIO, "data:0x%08x\n", data);
 
 done:
@@ -443,8 +495,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, 1, &data,
-					  true);
+	retval = brcmf_sdiod_reg_write(sdiodev, addr, 1, &data);
+
 	if (ret)
 		*ret = retval;
 }
@@ -458,8 +510,8 @@ void brcmf_sdiod_regwl(struct brcmf_sdio_dev *sdiodev, u32 addr,
 	retval = brcmf_sdiod_addrprep(sdiodev, 4, &addr);
 	if (retval)
 		goto done;
-	retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 4, &data,
-					  true);
+
+	retval = brcmf_sdiod_reg_write(sdiodev, addr, 4, &data);
 
 done:
 	if (ret)
-- 
1.9.1

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

* [PATCH 04/10] brcmfmac: Clean up brcmf_sdiod_set_sbaddr_window()
  2017-11-13 20:35 [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
                   ` (2 preceding siblings ...)
  2017-11-13 20:35 ` [PATCH 03/10] brcmfmac: Split brcmf_sdiod_regrw_helper() up Arend van Spriel
@ 2017-11-13 20:35 ` Arend van Spriel
  2017-11-13 20:35 ` [PATCH 05/10] brcmfmac: Remove dead IO code Arend van Spriel
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Arend van Spriel @ 2017-11-13 20:35 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Ian Molton, Arend van Spriel

From: Ian Molton <ian@mnementh.co.uk>

This function sets the address of the IO window used for
SDIO accesses onto the backplane of the chip.

It currently uses 3 separate masks despite the full mask being
defined in the code already. Remove the separate masks and clean up.

Signed-off-by: Ian Molton <ian@mnementh.co.uk>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c   | 17 +++++------------
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h |  3 ---
 2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 3acc0ff..1ea42c3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -410,23 +410,16 @@ static int brcmf_sdiod_reg_read(struct brcmf_sdio_dev *sdiodev, u32 addr,
 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++) {
-		brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_SBADDRLOW + i, addr[i],
-				  &err);
-		if (err) {
-			brcmf_err("failed at addr: 0x%0x\n",
-				  SBSDIO_FUNC1_SBADDRLOW + i);
-		}
-	}
+	for (i = 0 ; i < 3 && !err ; i++, addr >>= 8)
+		brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_SBADDRLOW + i,
+				  addr & 0xff, &err);
 
 	return err;
 }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
index f3da32f..e3b78db 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
@@ -133,9 +133,6 @@
 
 /* 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
 
-- 
1.9.1

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

* [PATCH 05/10] brcmfmac: Remove dead IO code
  2017-11-13 20:35 [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
                   ` (3 preceding siblings ...)
  2017-11-13 20:35 ` [PATCH 04/10] brcmfmac: Clean up brcmf_sdiod_set_sbaddr_window() Arend van Spriel
@ 2017-11-13 20:35 ` Arend van Spriel
  2017-11-13 20:35 ` [PATCH 06/10] brcmfmac: Remove bandaid for SleepCSR Arend van Spriel
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Arend van Spriel @ 2017-11-13 20:35 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Ian Molton, Arend van Spriel

From: Ian Molton <ian@mnementh.co.uk>

The value passed to brcmf_sdiod_addrprep() is *always* 4
remove this parameter and the unused code to handle it.

Signed-off-by: Ian Molton <ian@mnementh.co.uk>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 1ea42c3..ad81ea4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -425,7 +425,7 @@ static int brcmf_sdiod_reg_read(struct brcmf_sdio_dev *sdiodev, u32 addr,
 }
 
 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;
@@ -439,9 +439,7 @@ static int brcmf_sdiod_reg_read(struct brcmf_sdio_dev *sdiodev, 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;
 }
@@ -467,7 +465,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, 4, &addr);
+	retval = brcmf_sdiod_addrprep(sdiodev, &addr);
 	if (retval)
 		goto done;
 
@@ -500,7 +498,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, 4, &addr);
+	retval = brcmf_sdiod_addrprep(sdiodev, &addr);
 	if (retval)
 		goto done;
 
@@ -736,7 +734,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;
 
@@ -757,7 +755,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;
 
@@ -801,7 +799,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,
@@ -821,7 +819,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;
 
-- 
1.9.1

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

* [PATCH 06/10] brcmfmac: Remove bandaid for SleepCSR
  2017-11-13 20:35 [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
                   ` (4 preceding siblings ...)
  2017-11-13 20:35 ` [PATCH 05/10] brcmfmac: Remove dead IO code Arend van Spriel
@ 2017-11-13 20:35 ` Arend van Spriel
  2017-11-13 20:35 ` [PATCH 07/10] brcmfmac: Remove brcmf_sdiod_request_data() Arend van Spriel
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Arend van Spriel @ 2017-11-13 20:35 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Ian Molton, Arend van Spriel

From: Ian Molton <ian@mnementh.co.uk>

Register access code is not the place for band-aid fixes like this.
If this is a genuine problem, it should be fixed further up in the driver
stack.

Signed-off-by: Ian Molton <ian@mnementh.co.uk>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 28 +---------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index ad81ea4..355aebd 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -334,21 +334,8 @@ static int brcmf_sdiod_reg_write(struct brcmf_sdio_dev *sdiodev, u32 addr,
 	} while (ret != 0 && ret != -ENOMEDIUM &&
 		 retry++ < SDIOH_API_ACCESS_RETRY_LIMIT);
 
-	if (ret == -ENOMEDIUM) {
+	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;
 }
@@ -389,19 +376,6 @@ static int brcmf_sdiod_reg_read(struct brcmf_sdio_dev *sdiodev, u32 addr,
 
 	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 read data F%d@0x%05x, err: %d\n",
-				  func, addr, ret);
-		else
-			brcmf_dbg(SDIO, "failed to read data F%d@0x%05x, err: %d\n",
-				  func, addr, ret);
-	}
 
 	return ret;
 }
-- 
1.9.1

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

* [PATCH 07/10] brcmfmac: Remove brcmf_sdiod_request_data()
  2017-11-13 20:35 [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
                   ` (5 preceding siblings ...)
  2017-11-13 20:35 ` [PATCH 06/10] brcmfmac: Remove bandaid for SleepCSR Arend van Spriel
@ 2017-11-13 20:35 ` Arend van Spriel
  2017-11-13 20:35 ` [PATCH 08/10] brcmfmac: Fix asymmetric IO functions Arend van Spriel
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Arend van Spriel @ 2017-11-13 20:35 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Ian Molton, Arend van Spriel

From: Ian Molton <ian@mnementh.co.uk>

This function is obfuscating how IO works on this chip. Remove it
and push its logic into brcmf_sdiod_reg_{read,write}().

Handling of -ENOMEDIUM is altered, but as that's pretty much broken anyway
we can ignore that.

Signed-off-by: Ian Molton <ian@mnementh.co.uk>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 237 ++++++++-------------
 .../wireless/broadcom/brcm80211/brcmfmac/sdio.h    |   2 +-
 2 files changed, 87 insertions(+), 152 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 355aebd..bb2ebe6 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -230,6 +230,43 @@ 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;
+
+	if (sdiodev->state == BRCMF_SDIOD_NOMEDIUM)
+		return -ENOMEDIUM;
+
+	addr = (address & SBSDIO_SBWINDOW_MASK) >> 8;
+
+	for (i = 0 ; i < 3 && !err ; i++, addr >>= 8)
+		brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_SBADDRLOW + i,
+				  addr & 0xff, &err);
+
+	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;
+}
+
 static inline int brcmf_sdiod_f0_writeb(struct sdio_func *func, u8 byte,
 					uint regaddr)
 {
@@ -249,173 +286,84 @@ static inline int brcmf_sdiod_f0_writeb(struct sdio_func *func, u8 byte,
 	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);
-
-	} while (ret != 0 && ret != -ENOMEDIUM &&
-		 retry++ < SDIOH_API_ACCESS_RETRY_LIMIT);
+	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;
 
-	if (ret == -ENOMEDIUM)
-		brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_NOMEDIUM);
+			sdio_writel(sdiodev->func[1], *(u32 *)data, addr, &ret);
+			break;
+		default:
+			WARN(1, "Invalid reg size\n");
+			ret = -EINVAL;
+			break;
+		}
+	}
 
+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);
-
-	} while (ret != 0 && ret != -ENOMEDIUM &&
-		 retry++ < SDIOH_API_ACCESS_RETRY_LIMIT);
-
-	if (ret == -ENOMEDIUM)
-		brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_NOMEDIUM);
-
-	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 && !err ; i++, addr >>= 8)
-		brcmf_sdiod_regwb(sdiodev, SBSDIO_FUNC1_SBADDRLOW + i,
-				  addr & 0xff, &err);
-
-	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;
+	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;
 
-		sdiodev->sbwad = bar0;
+			*(u32 *)data = sdio_readl(sdiodev->func[1], addr, &ret);
+			break;
+		default:
+			WARN(1, "Invalid reg size\n");
+			ret = -EINVAL;
+			break;
+		}
 	}
 
-	*addr &= SBSDIO_SB_OFT_ADDR_MASK;
-	*addr |= SBSDIO_SB_ACCESS_2_4B_FLAG;
-
-	return 0;
+done:
+	return ret;
 }
 
 u8 brcmf_sdiod_regrb(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
@@ -439,15 +387,9 @@ 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, &addr);
-	if (retval)
-		goto done;
-
 	retval = brcmf_sdiod_reg_read(sdiodev, addr, 4, &data);
-
 	brcmf_dbg(SDIO, "data:0x%08x\n", data);
 
-done:
 	if (ret)
 		*ret = retval;
 
@@ -472,13 +414,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, 4, &data);
 
-done:
 	if (ret)
 		*ret = retval;
 }
@@ -886,14 +823,12 @@ int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev,
 	return bcmerror;
 }
 
-int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, uint fn)
+int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, u8 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,
-				 1, &t_func, true);
+	brcmf_sdiod_reg_write(sdiodev, SDIO_CCCR_ABORT, 1, &fn);
 
 	brcmf_dbg(SDIO, "Exit\n");
 	return 0;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
index e3b78db..1e4b476 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
@@ -339,7 +339,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);
-- 
1.9.1

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

* [PATCH 08/10] brcmfmac: Fix asymmetric IO functions.
  2017-11-13 20:35 [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
                   ` (6 preceding siblings ...)
  2017-11-13 20:35 ` [PATCH 07/10] brcmfmac: Remove brcmf_sdiod_request_data() Arend van Spriel
@ 2017-11-13 20:35 ` Arend van Spriel
  2017-11-13 20:35 ` [PATCH 09/10] brcmfmac: Remove noisy debugging Arend van Spriel
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Arend van Spriel @ 2017-11-13 20:35 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Ian Molton, Arend van Spriel

From: Ian Molton <ian@mnementh.co.uk>

Unlikely to be a problem, but brcmf_sdiod_regrl() is
not symmetric with brcmf_sdiod_regrb() in initializing
the data value on stack. Fix that.

Signed-off-by: Ian Molton <ian@mnementh.co.uk>
[arend: reword the commit message a bit]
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index bb2ebe6..e89e242 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -383,7 +383,7 @@ u8 brcmf_sdiod_regrb(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)

 u32 brcmf_sdiod_regrl(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
 {
-	u32 data = 0;
+	u32 data;
 	int retval;

 	brcmf_dbg(SDIO, "addr:0x%08x\n", addr);
--
1.9.1

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

* [PATCH 09/10] brcmfmac: Remove noisy debugging.
  2017-11-13 20:35 [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
                   ` (7 preceding siblings ...)
  2017-11-13 20:35 ` [PATCH 08/10] brcmfmac: Fix asymmetric IO functions Arend van Spriel
@ 2017-11-13 20:35 ` Arend van Spriel
  2017-11-13 20:35 ` [PATCH 10/10] brcmfmac: Rename bcmerror to err Arend van Spriel
  2017-12-07  8:47 ` [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
  10 siblings, 0 replies; 16+ messages in thread
From: Arend van Spriel @ 2017-11-13 20:35 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Ian Molton, Arend van Spriel

From: Ian Molton <ian@mnementh.co.uk>

If you need debugging this low level, you're doing something wrong.
Remove these noisy debug statements so the code is more readable.

Signed-off-by: Ian Molton <ian@mnementh.co.uk>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index e89e242..9a3f903 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -371,9 +371,7 @@ u8 brcmf_sdiod_regrb(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
 	u8 data;
 	int retval;
 
-	brcmf_dbg(SDIO, "addr:0x%08x\n", addr);
 	retval = brcmf_sdiod_reg_read(sdiodev, addr, 1, &data);
-	brcmf_dbg(SDIO, "data:0x%02x\n", data);
 
 	if (ret)
 		*ret = retval;
@@ -386,9 +384,7 @@ u32 brcmf_sdiod_regrl(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret)
 	u32 data;
 	int retval;
 
-	brcmf_dbg(SDIO, "addr:0x%08x\n", addr);
 	retval = brcmf_sdiod_reg_read(sdiodev, addr, 4, &data);
-	brcmf_dbg(SDIO, "data:0x%08x\n", data);
 
 	if (ret)
 		*ret = retval;
@@ -401,7 +397,6 @@ 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, 1, &data);
 
 	if (ret)
@@ -413,7 +408,6 @@ 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_reg_write(sdiodev, addr, 4, &data);
 
 	if (ret)
-- 
1.9.1

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

* [PATCH 10/10] brcmfmac: Rename bcmerror to err
  2017-11-13 20:35 [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
                   ` (8 preceding siblings ...)
  2017-11-13 20:35 ` [PATCH 09/10] brcmfmac: Remove noisy debugging Arend van Spriel
@ 2017-11-13 20:35 ` Arend van Spriel
  2017-12-07  8:47 ` [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
  10 siblings, 0 replies; 16+ messages in thread
From: Arend van Spriel @ 2017-11-13 20:35 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Ian Molton, Arend van Spriel

From: Ian Molton <ian@mnementh.co.uk>

Trivial cleanup of nasty variable name

Signed-off-by: Ian Molton <ian@mnementh.co.uk>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 9a3f903..30ab0aa 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -746,7 +746,7 @@ int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev,
 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;
@@ -771,8 +771,8 @@ int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev,
 	/* 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",
@@ -785,9 +785,9 @@ int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev,
 		skb_put(pkt, dsize);
 		if (write)
 			memcpy(pkt->data, data, dsize);
-		bcmerror = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_1, write,
-					      sdaddr, pkt);
-		if (bcmerror) {
+		err = brcmf_sdiod_buffrw(sdiodev, SDIO_FUNC_1, write, sdaddr,
+					 pkt);
+		if (err) {
 			brcmf_err("membytes transfer failed\n");
 			break;
 		}
@@ -814,7 +814,7 @@ int brcmf_sdiod_send_pkt(struct brcmf_sdio_dev *sdiodev,
 
 	sdio_release_host(sdiodev->func[1]);
 
-	return bcmerror;
+	return err;
 }
 
 int brcmf_sdiod_abort(struct brcmf_sdio_dev *sdiodev, u8 fn)
-- 
1.9.1

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

* Re: [PATCH 00/10] brcmfmac: restructuring sdio access functions
  2017-11-13 20:35 [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
                   ` (9 preceding siblings ...)
  2017-11-13 20:35 ` [PATCH 10/10] brcmfmac: Rename bcmerror to err Arend van Spriel
@ 2017-12-07  8:47 ` Arend van Spriel
  2017-12-07  8:49   ` Kalle Valo
  10 siblings, 1 reply; 16+ messages in thread
From: Arend van Spriel @ 2017-12-07  8:47 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless

On 11/13/2017 9:35 PM, Arend van Spriel wrote:
> These patches were originally submitted by Ian Molton a while ago so decided
> to resubmit them. This is a first series of 10 patches. Tested using BCM4354
> device which showed no regression.
>
> It is intended for 4.16 (no rush ;-) ) and applies to the master branch of
> the wireless-drivers-next repository.

Hi Kalle,

I have follow-up series on this one. If you plan to take it today or 
tomorrow, I can hold the follow-up a bit more.

Regards,
Arend

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

* Re: [PATCH 00/10] brcmfmac: restructuring sdio access functions
  2017-12-07  8:47 ` [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
@ 2017-12-07  8:49   ` Kalle Valo
  2017-12-07  8:59     ` Arend van Spriel
  0 siblings, 1 reply; 16+ messages in thread
From: Kalle Valo @ 2017-12-07  8:49 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: linux-wireless

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 11/13/2017 9:35 PM, Arend van Spriel wrote:
>> These patches were originally submitted by Ian Molton a while ago so decided
>> to resubmit them. This is a first series of 10 patches. Tested using BCM4354
>> device which showed no regression.
>>
>> It is intended for 4.16 (no rush ;-) ) and applies to the master branch of
>> the wireless-drivers-next repository.
>
> Hi Kalle,
>
> I have follow-up series on this one. If you plan to take it today or
> tomorrow, I can hold the follow-up a bit more.

I'm hoping to apply these today.

-- 
Kalle Valo

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

* Re: [PATCH 00/10] brcmfmac: restructuring sdio access functions
  2017-12-07  8:49   ` Kalle Valo
@ 2017-12-07  8:59     ` Arend van Spriel
  2017-12-07 10:49       ` Kalle Valo
  0 siblings, 1 reply; 16+ messages in thread
From: Arend van Spriel @ 2017-12-07  8:59 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless

On 12/7/2017 9:49 AM, Kalle Valo wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> On 11/13/2017 9:35 PM, Arend van Spriel wrote:
>>> These patches were originally submitted by Ian Molton a while ago so decided
>>> to resubmit them. This is a first series of 10 patches. Tested using BCM4354
>>> device which showed no regression.
>>>
>>> It is intended for 4.16 (no rush ;-) ) and applies to the master branch of
>>> the wireless-drivers-next repository.
>>
>> Hi Kalle,
>>
>> I have follow-up series on this one. If you plan to take it today or
>> tomorrow, I can hold the follow-up a bit more.
>
> I'm hoping to apply these today.
>

btw. my neighbors put out the Finnish flag. Was that about your 
independence? Not a yearly event, or is it?

Gr. AvS

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

* Re: [PATCH 00/10] brcmfmac: restructuring sdio access functions
  2017-12-07  8:59     ` Arend van Spriel
@ 2017-12-07 10:49       ` Kalle Valo
  0 siblings, 0 replies; 16+ messages in thread
From: Kalle Valo @ 2017-12-07 10:49 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: linux-wireless

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 12/7/2017 9:49 AM, Kalle Valo wrote:
>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>>> On 11/13/2017 9:35 PM, Arend van Spriel wrote:
>>>> These patches were originally submitted by Ian Molton a while ago so decided
>>>> to resubmit them. This is a first series of 10 patches. Tested using BCM4354
>>>> device which showed no regression.
>>>>
>>>> It is intended for 4.16 (no rush ;-) ) and applies to the master branch of
>>>> the wireless-drivers-next repository.
>>>
>>> Hi Kalle,
>>>
>>> I have follow-up series on this one. If you plan to take it today or
>>> tomorrow, I can hold the follow-up a bit more.
>>
>> I'm hoping to apply these today.
>>
>
> btw. my neighbors put out the Finnish flag.

Cool!

> Was that about your independence? Not a yearly event, or is it?

December 6th is the independence day in Finland and that's a yearly
event, a public holiday etc. But this year was special as it was 100
years from gaining our independence.

So yesterday, instead of applying patches, I was looking at people
shaking hands[1] on TV. Yes, we are weird ;)

[1] https://en.wikipedia.org/wiki/Independence_Day_Reception_(in_Finland)

-- 
Kalle Valo

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

* Re: [01/10] brcmfmac: Fix parameter order in brcmf_sdiod_f0_writeb()
  2017-11-13 20:35 ` [PATCH 01/10] brcmfmac: Fix parameter order in brcmf_sdiod_f0_writeb() Arend van Spriel
@ 2017-12-07 13:12   ` Kalle Valo
  0 siblings, 0 replies; 16+ messages in thread
From: Kalle Valo @ 2017-12-07 13:12 UTC (permalink / raw)
  To: Arend Van Spriel; +Cc: linux-wireless, Ian Molton, Arend van Spriel

Arend Van Spriel <arend.vanspriel@broadcom.com> wrote:

> From: Ian Molton <ian@mnementh.co.uk>
> 
> All the other IO functions are the other way round in this
> driver. Make this one match.
> 
> Signed-off-by: Ian Molton <ian@mnementh.co.uk>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>

10 patches applied to wireless-drivers-next.git, thanks.

1fd3ae124d5e brcmfmac: Fix parameter order in brcmf_sdiod_f0_writeb()
1e6f676f43aa brcmfmac: Register sizes on hardware are not dependent on compiler types
0fcc9fe00484 brcmfmac: Split brcmf_sdiod_regrw_helper() up.
b9b0d290bc0c brcmfmac: Clean up brcmf_sdiod_set_sbaddr_window()
ea243e9077b3 brcmfmac: Remove dead IO code
4a3338ba2a74 brcmfmac: Remove bandaid for SleepCSR
993a98a42e6e brcmfmac: Remove brcmf_sdiod_request_data()
3508a056a1f4 brcmfmac: Fix asymmetric IO functions.
12e3e74e2820 brcmfmac: Remove noisy debugging.
dd8a2d49e4ed brcmfmac: Rename bcmerror to err

-- 
https://patchwork.kernel.org/patch/10056667/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

end of thread, other threads:[~2017-12-07 13:12 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-13 20:35 [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
2017-11-13 20:35 ` [PATCH 01/10] brcmfmac: Fix parameter order in brcmf_sdiod_f0_writeb() Arend van Spriel
2017-12-07 13:12   ` [01/10] " Kalle Valo
2017-11-13 20:35 ` [PATCH 02/10] brcmfmac: Register sizes on hardware are not dependent on compiler types Arend van Spriel
2017-11-13 20:35 ` [PATCH 03/10] brcmfmac: Split brcmf_sdiod_regrw_helper() up Arend van Spriel
2017-11-13 20:35 ` [PATCH 04/10] brcmfmac: Clean up brcmf_sdiod_set_sbaddr_window() Arend van Spriel
2017-11-13 20:35 ` [PATCH 05/10] brcmfmac: Remove dead IO code Arend van Spriel
2017-11-13 20:35 ` [PATCH 06/10] brcmfmac: Remove bandaid for SleepCSR Arend van Spriel
2017-11-13 20:35 ` [PATCH 07/10] brcmfmac: Remove brcmf_sdiod_request_data() Arend van Spriel
2017-11-13 20:35 ` [PATCH 08/10] brcmfmac: Fix asymmetric IO functions Arend van Spriel
2017-11-13 20:35 ` [PATCH 09/10] brcmfmac: Remove noisy debugging Arend van Spriel
2017-11-13 20:35 ` [PATCH 10/10] brcmfmac: Rename bcmerror to err Arend van Spriel
2017-12-07  8:47 ` [PATCH 00/10] brcmfmac: restructuring sdio access functions Arend van Spriel
2017-12-07  8:49   ` Kalle Valo
2017-12-07  8:59     ` Arend van Spriel
2017-12-07 10:49       ` Kalle Valo

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