Re: [PATCH 02/34] brcmfmac: Register sizes on hardware are not dependent on compiler types

[Arend van Spriel]

On 26-07-17 22:25, Ian Molton wrote:

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


I see no incorrectness here. It is validating the regsz value and those
compiler types are pretty well defined I would say. Anyway, I find the
use of sizeof() here pretty useless so thanks for the change.

Reviewed-by: Arend van Spriel 

Signed-off-by: Ian Molton 
---
 .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c  | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

[PATCH 02/34] brcmfmac: Register sizes on hardware are not dependent on compiler types

[Ian Molton]

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 
---
 .../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 f585dfd89453..2b441ce91d5f 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, );
@@ -278,13 +278,13 @@ static int brcmf_sdiod_request_data(struct brcmf_sdio_dev 
*sdiodev, u8 fn,
*(u8 *)data = sdio_f0_readb(func, addr, );
}
break;
-   case sizeof(u16):
+   case 2:
if (write)
sdio_writew(func, *(u16 *)data, addr, );
else
*(u16 *)data = sdio_readw(func, addr, );
break;
-   case sizeof(u32):
+   case 4:
if (write)
sdio_writel(func, *(u32 *)data, addr, );
else
@@ -368,7 +368,7 @@ brcmf_sdiod_set_sbaddr_window(struct brcmf_sdio_dev 
*sdiodev, u32 address)
for (i = 0; i < 3; i++) {
err = brcmf_sdiod_regrw_helper(sdiodev,
   SBSDIO_FUNC1_SBADDRLOW + i,
-  sizeof(u8), [i], true);
+  1, [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), ,
+   retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 1, ,
  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), );
+   retval = brcmf_sdiod_addrprep(sdiodev, 4, );
if (retval)
goto done;
-   retval = brcmf_sdiod_regrw_helper(sdiodev, addr, sizeof(data), ,
+   retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 4, ,
  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), ,
+   retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 1, ,
  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), );
+   retval = brcmf_sdiod_addrprep(sdiodev, 4, );
if (retval)
goto done;
-   retval = brcmf_sdiod_regrw_helper(sdiodev, addr, sizeof(data), ,
+   retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 4, ,
  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), _func, true);
+1, _func, true);
 
brcmf_dbg(SDIO, "Exit\n");
return 0;
-- 
2.11.0

[PATCH 02/34] brcmfmac: Register sizes on hardware are not dependent on compiler types

[Ian Molton]

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 
---
 .../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 1acbaaf0f5c9..db983168ceba 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -263,7 +263,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, );
@@ -276,13 +276,13 @@ static int brcmf_sdiod_request_data(struct brcmf_sdio_dev 
*sdiodev, u8 fn,
*(u8 *)data = sdio_f0_readb(func, addr, );
}
break;
-   case sizeof(u16):
+   case 2:
if (write)
sdio_writew(func, *(u16 *)data, addr, );
else
*(u16 *)data = sdio_readw(func, addr, );
break;
-   case sizeof(u32):
+   case 4:
if (write)
sdio_writel(func, *(u32 *)data, addr, );
else
@@ -366,7 +366,7 @@ brcmf_sdiod_set_sbaddr_window(struct brcmf_sdio_dev 
*sdiodev, u32 address)
for (i = 0; i < 3; i++) {
err = brcmf_sdiod_regrw_helper(sdiodev,
   SBSDIO_FUNC1_SBADDRLOW + i,
-  sizeof(u8), [i], true);
+  1, [i], true);
if (err) {
brcmf_err("failed at addr: 0x%0x\n",
  SBSDIO_FUNC1_SBADDRLOW + i);
@@ -405,7 +405,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), ,
+   retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 1, ,
  false);
brcmf_dbg(SDIO, "data:0x%02x\n", data);
 
@@ -421,10 +421,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), );
+   retval = brcmf_sdiod_addrprep(sdiodev, 4, );
if (retval)
goto done;
-   retval = brcmf_sdiod_regrw_helper(sdiodev, addr, sizeof(data), ,
+   retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 4, ,
  false);
brcmf_dbg(SDIO, "data:0x%08x\n", data);
 
@@ -441,7 +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_regrw_helper(sdiodev, addr, sizeof(data), ,
+   retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 1, ,
  true);
if (ret)
*ret = retval;
@@ -453,10 +453,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), );
+   retval = brcmf_sdiod_addrprep(sdiodev, 4, );
if (retval)
goto done;
-   retval = brcmf_sdiod_regrw_helper(sdiodev, addr, sizeof(data), ,
+   retval = brcmf_sdiod_regrw_helper(sdiodev, addr, 4, ,
  true);
 
 done:
@@ -874,7 +874,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), _func, true);
+1, _func, true);
 
brcmf_dbg(SDIO, "Exit\n");
return 0;
-- 
2.11.0