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