Re: [PATCH 26/30] brcmfmac: More efficient and slightly easier to read fixup for 4339 chips

[Arend van Spriel]

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 <i...@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;
  }