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@xxxxxxxxxxxxxxx> Candela Technologies Inc http://www.candelatech.com -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
