[PATCH] staging: brcm80211: return false if not a broadcom board
Greg Dietsche
gregory.dietsche at cuw.edu
Sat Jun 18 15:52:30 UTC 2011
On 06/17/2011 05:01 PM, Henry Ptasinski wrote:
> On Thu, Jun 16, 2011 at 04:45:21PM -0700, Julian Calaby wrote:
>
>> Henry,
>>
>> On Fri, Jun 17, 2011 at 09:37, Henry Ptasinski<henryp at broadcom.com> wrote:
>>
>>> How's this for a somewhat clearer implementation:
>>>
>>> static bool brcms_c_validboardtype(struct brcms_c_hw_info *wlc_hw)
>>> {
>>> bool goodboard = true;
>>> uint boardrev = wlc_hw->boardrev;
>>>
>>> if (wlc_hw->sih->boardvendor == PCI_VENDOR_ID_BROADCOM) {
>>>
>> You could reduce indentation by having this be:
>>
>> if (wlc_hw->sih->boardvendor != PCI_VENDOR_ID_BROADCOM)
>> return true;
>>
>>
>>> /* validate boardrev */
>>> if (boardrev == 0)
>>> goodboard = false;
>>>
>> and remove the goodboard variable by having this return false immediately.
>>
>>
>>> else if (boardrev> 0xff) {
>>>
>> You could also drop the else and have this as
>>
>> if (boardrev<= 0xff)
>> return true;
>>
>>
>>> /* 4 bits each for board type, major, minor, and tiny
>>> version numbers */
>>> uint brt = (boardrev& 0xf000)>> 12;
>>> uint b0 = (boardrev& 0xf00)>> 8;
>>> uint b1 = (boardrev& 0xf0)>> 4;
>>> uint b2 = boardrev& 0xf;
>>>
>>> if ((brt> 2) || (brt == 0) || (b0> 9) || (b0 == 0)
>>> || (b1> 9) || (b2> 9))
>>> goodboard = false;
>>>
>> and return false here too.
>>
>>
>>> }
>>> }
>>>
>>> return goodboard;
>>>
>> then just return true here.
>>
>>
>>> }
>>>
>> Thanks,
>>
>> --
>> Julian Calaby
>>
> Yea, it's a lot flatter with those changes. Look for it in a patch set
> relatively soon ...
>
> - Henry
>
>
>
All good points above - also, I might add a short comment to explain why
the code checks the vendor id so it's clear to people who read the code
in the future too.
Greg
More information about the devel
mailing list