[PATCH] ath10k: improve vdev map handling.
Kalle Valo
kvalo at qca.qualcomm.com
Fri May 16 07:06:05 PDT 2014
Ben Greear <greearb at candelatech.com> writes:
> 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)
Oh, I didn't know that. Too bad, but then removing it makes sense.
> 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.
All Linux engineers should know what BIT() does. If not, they should
learn that ;)
>>> - 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.
--
Kalle Valo
More information about the ath10k
mailing list