Re: [PATCH V2 1/2] {cfg,mac}80211: utility function for calculating mandatory rates

[Ashok Nagarajan <ashok@xxxxxxxxxxx>]

On Tue, Jul 17, 2012 at 11:55 PM, Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> wrote:
> On Tue, 2012-07-17 at 21:42 -0700, Ashok Nagarajan wrote:
>
>> + */
>> +
>> +u32 ieee80211_mandatory_rates(struct ieee80211_supported_band *sband,
>
> Please remove the blank line and document the return value.
>
Noted.
>
>> +u32 ieee80211_mandatory_rates(struct ieee80211_supported_band *sband,
>> +                           enum ieee80211_band band)
>
> Passing both the band enum and the sband doesn't make a lot of sense.
>
Noted.
>> +{
>> +     struct ieee80211_rate *bitrates;
>> +     u32 mandatory_rates = 0;
>> +     enum ieee80211_rate_flags mandatory_flag;
>> +     int i;
>> +
>> +     if (WARN_ON(!sband))
>> +             return 1;
>> +
>> +     bitrates = sband->bitrates;
>> +     if (band == IEEE80211_BAND_5GHZ)
>> +             mandatory_flag = IEEE80211_RATE_MANDATORY_A;
>> +     else {
>> +             mandatory_flag = IEEE80211_RATE_MANDATORY_B;
>> +             for (i = 0; i < sband->n_bitrates; i++)
>> +                     if (bitrates[i].bitrate > 110) {
>> +                             mandatory_flag =
>> +                                     IEEE80211_RATE_MANDATORY_G;
>> +                             break;
>> +                     }
>> +     }
>
> You're not just moving the function but also changing the logic in it,
> it'd be better to do in separate patches I think.
>
I will split the patch into two, one for moving the function and
another patch for changing the logic in v3.

Thanks,
Ashok
> johannes
>
--
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