[PATCH] ath10k: improve vdev map handling.
Ben Greear
greearb at candelatech.com
Fri May 16 07:01:25 PDT 2014
On 05/16/2014 06:37 AM, Kalle Valo wrote:
>> - ar->free_vdev_map &= ~BIT(arvif->vdev_id);
>> + ar->free_vdev_map &= ~(1 << arvif->vdev_id);
>
> Why remove the BIT()? Not that it matters much, I just think it's easier
> to read when BIT() macro is used. Would be good to convert all cases to
> use BIT anyway, but that's for a separate patch.
BIT doesn't work on 64-bit numbers (ie, if vdev_id > 31), and it takes a long time to
figure out exactly what it does (try grepping for BIT). Open-coding means much
easier to fully understand the code.
>> err_vdev_delete:
>> ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
>> - ar->free_vdev_map &= ~BIT(arvif->vdev_id);
>> + ar->free_vdev_map |= (1 << arvif->vdev_id);
>
> Again why remove BIT()?
>
>> @@ -2792,7 +2787,7 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
>> }
>> spin_unlock_bh(&ar->data_lock);
>>
>> - ar->free_vdev_map |= 1 << (arvif->vdev_id);
>> + ar->free_vdev_map |= (1 << arvif->vdev_id);
>
> Do we need the parenthesis?
No, though I like them visually. It's at least more useful than
the previous placement.
I can respin the patch w/out them and with the == 0 and such.
Thanks,
Ben
--
Ben Greear <greearb at candelatech.com>
Candela Technologies Inc http://www.candelatech.com
More information about the ath10k
mailing list