| Message ID | 20170822112550.60311-27-ian@mnementh.co.uk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On 22-08-17 13:25, Ian Molton wrote: > Its more efficient to test the register we're interested in first, > potentially avoiding two more comparisons, and therefore always avoiding > one comparison per call on all other chips. Efficiency is really not a big issue. The buscore callbacks are really only used during chip recognition so this feels like change for sake of change. Not my favorite. > Signed-off-by: Ian Molton <ian@mnementh.co.uk> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > index 7ebe6460cb5c..8a730133db77 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c > @@ -3766,15 +3766,18 @@ static u32 brcmf_sdio_buscore_read32(void *ctx, u32 addr) > /* Force 4339 chips over rev2 to use the same ID */ > /* This is borderline tolerable whilst there is only two exceptions */ > /* But could be handled better */ This is not correct way to do multi-line comment and the story is not quite correct or maybe I do not understand what is written here. > - if ((sdiodev->func[1]->device == SDIO_DEVICE_ID_BROADCOM_4335_4339 || > - sdiodev->func[1]->device == SDIO_DEVICE_ID_BROADCOM_4339) && > - addr == CORE_CC_REG(SI_ENUM_BASE, chipid)) { > + if (addr == CORE_CC_REG(SI_ENUM_BASE, chipid) && > + (sdiodev->func[1]->device == SDIO_DEVICE_ID_BROADCOM_4339 || > + sdiodev->func[1]->device == SDIO_DEVICE_ID_BROADCOM_4335_4339)) { > + > rev = (val & CID_REV_MASK) >> CID_REV_SHIFT; > + > if (rev >= 2) { > val &= ~CID_ID_MASK; > val |= BRCM_CC_4339_CHIP_ID; > } > } > + > return val; > } > >
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index 7ebe6460cb5c..8a730133db77 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -3766,15 +3766,18 @@ static u32 brcmf_sdio_buscore_read32(void *ctx, u32 addr) /* Force 4339 chips over rev2 to use the same ID */ /* This is borderline tolerable whilst there is only two exceptions */ /* But could be handled better */ - if ((sdiodev->func[1]->device == SDIO_DEVICE_ID_BROADCOM_4335_4339 || - sdiodev->func[1]->device == SDIO_DEVICE_ID_BROADCOM_4339) && - addr == CORE_CC_REG(SI_ENUM_BASE, chipid)) { + if (addr == CORE_CC_REG(SI_ENUM_BASE, chipid) && + (sdiodev->func[1]->device == SDIO_DEVICE_ID_BROADCOM_4339 || + sdiodev->func[1]->device == SDIO_DEVICE_ID_BROADCOM_4335_4339)) { + rev = (val & CID_REV_MASK) >> CID_REV_SHIFT; + if (rev >= 2) { val &= ~CID_ID_MASK; val |= BRCM_CC_4339_CHIP_ID; } } + return val; }
Its more efficient to test the register we're interested in first, potentially avoiding two more comparisons, and therefore always avoiding one comparison per call on all other chips. Signed-off-by: Ian Molton <ian@mnementh.co.uk> --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)