| Message ID | 20170726202557.15632-7-ian@mnementh.co.uk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On 26-07-17 22:25, Ian Molton wrote: > 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. So lets make it a SDIO debug message for all register accesses getting rid of the error message. Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Ian Molton <ian@mnementh.co.uk> > > # Conflicts: > # drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > --- > .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 28 +--------------------- > 1 file changed, 1 insertion(+), 27 deletions(-)
On 07/08/17 12:25, Arend van Spriel wrote: > On 26-07-17 22:25, Ian Molton wrote: >> 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. > > So lets make it a SDIO debug message for all register accesses getting > rid of the error message. Not quite sure I follow here - but as the code is completely gone in later patches in the series, does it matter? Perhaps address this if it in future, if it crops up as a problem, since it wont be fatal even if it does... -Ian
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c index 609a934c1658..d217b1281e0d 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; }
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> # Conflicts: # drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c --- .../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 28 +--------------------- 1 file changed, 1 insertion(+), 27 deletions(-)