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
From: Ian Molton <[email protected]>
Trivial cleanup of nasty variable name
Signed-off-by: Ian Molton <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
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
From: Ian Molton <[email protected]>
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 <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
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
From: Ian Molton <[email protected]>
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 <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
.../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
From: Ian Molton <[email protected]>
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 <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
.../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
From: Ian Molton <[email protected]>
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 <[email protected]>
[arend: reword the commit message a bit]
Signed-off-by: Arend van Spriel <[email protected]>
---
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
From: Ian Molton <[email protected]>
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 <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
.../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
From: Ian Molton <[email protected]>
All the other IO functions are the other way round in this
driver. Make this one match.
Signed-off-by: Ian Molton <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
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
From: Ian Molton <[email protected]>
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 <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
.../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
From: Ian Molton <[email protected]>
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 <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
.../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
From: Ian Molton <[email protected]>
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 <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
.../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
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
Arend Van Spriel <[email protected]> wrote:
> From: Ian Molton <[email protected]>
>
> All the other IO functions are the other way round in this
> driver. Make this one match.
>
> Signed-off-by: Ian Molton <[email protected]>
> Signed-off-by: Arend van Spriel <[email protected]>
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
Arend van Spriel <[email protected]> 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
On 12/7/2017 9:49 AM, Kalle Valo wrote:
> Arend van Spriel <[email protected]> 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
Arend van Spriel <[email protected]> writes:
> On 12/7/2017 9:49 AM, Kalle Valo wrote:
>> Arend van Spriel <[email protected]> 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