| Message ID | 20170726202557.15632-9-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: > Unlikely to be a problem, but brcmf_sdiod_regrb() is > not symmetric with brcmf_sdiod_regrl() in this regard. You are pretty keen on symmetry, but you could also remove the initialization in brcmf_sdiod_regrl(). As long as no -Wunitialized pops up that would have my preference. Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Ian Molton <ian@mnementh.co.uk> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-)
On 07/08/17 12:26, Arend van Spriel wrote: > On 7/26/2017 10:25 PM, Ian Molton wrote: >> Unlikely to be a problem, but brcmf_sdiod_regrb() is >> not symmetric with brcmf_sdiod_regrl() in this regard. > > You are pretty keen on symmetry, but you could also remove the > initialization in brcmf_sdiod_regrl(). As long as no -Wunitialized pops > up that would have my preference. Whilst the compiler does not complain, there are paths through brcmf_sdiod_reg_read() that do not set *data, so I think it best to initialise the value, but its not really very important. Code that isn't checking the return value is asking for it anyway :) In the later patches we could remove the initialisation, as IIRC the Linux MMC IO functions do it for us and we don't avoid calling them in that version of the code. -Ian
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c index f703d7be6a85..73f2194a854f 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c @@ -367,7 +367,7 @@ static int brcmf_sdiod_reg_read(struct brcmf_sdio_dev *sdiodev, u32 addr, u8 brcmf_sdiod_regrb(struct brcmf_sdio_dev *sdiodev, u32 addr, int *ret) { - u8 data; + u8 data = 0; int retval; brcmf_dbg(SDIO, "addr:0x%08x\n", addr);
Unlikely to be a problem, but brcmf_sdiod_regrb() is not symmetric with brcmf_sdiod_regrl() in this regard. Signed-off-by: Ian Molton <ian@mnementh.co.uk> --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)