| Message ID | 20170726202557.15632-10-ian@mnementh.co.uk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On 7/26/2017 10:25 PM, Ian Molton wrote: > If you need debugging this low level, you're doing something wrong. > Remove these noisy debug statements so the code is more readable. Needing this debugging does not necessarily means you are doing something wrong. You may be dealing with hardware that is doing something wrong and when that happens this debug can be useful. I frankly hardly ever enable SDIO debug level unless I am in that scenario. Maybe adding a debug level for low-level access would be useful to reduce the noise for SDIO debug level. Regards, Arend > Signed-off-by: Ian Molton <ian@mnementh.co.uk> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-)
On 07/08/17 12:26, Arend van Spriel wrote: > > Needing this debugging does not necessarily means you are doing > something wrong. You may be dealing with hardware that is doing > something wrong and when that happens this debug can be useful. I > frankly hardly ever enable SDIO debug level unless I am in that > scenario. Maybe adding a debug level for low-level access would be > useful to reduce the noise for SDIO debug level. Perhaps, but its only actually called from a half dozen or so places in the code + the buscore_*32 entrypoints. All it actually does now is ensure the address window is right and call the Linux SDIO core readl method. if we can't trust that, we're kinda screwed anyway. If we later bring the SDIO code in line with the PCIe code, it wont even do the address window checking (it'll just assume its already correct). We can always enable the SDIO core lowlevel debug if we really want to see register level acceses. -Ian
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c index 73f2194a854f..049086f6db97 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c @@ -370,9 +370,7 @@ u8 brcmf_sdiod_regrb(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret) u8 data = 0; 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; @@ -385,8 +383,6 @@ 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); - brcmf_dbg(SDIO, "data:0x%08x\n", data); retval = brcmf_sdiod_reg_read(sdiodev, addr, 4, &data); if (ret) @@ -400,7 +396,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) @@ -412,7 +407,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_reg_write(sdiodev, addr, 4, &data); if (ret) *ret = retval;
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> --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)