On Tue October 12 2010 11:10:43 Ben Greear wrote: > On 10/11/2010 06:44 PM, Bruno Randolf wrote: > > On Sat October 9 2010 04:01:15 greearb@xxxxxxxxxxxxxxx wrote: > >> + > >> + /* Calculate combined mode - when APs are active, operate in AP mode. > >> + * Otherwise use the mode of the new interface. This can currently > >> + * only deal with combinations of APs and STAs. Only one ad-hoc > >> + * interfaces is allowed above. > > > > the comment reference to "above" does not make sense here any more. > > This patch is already in..but I can send a followup cleanup patch. just for the comment it's probably not worth it, but... > >> ieee80211_vif *vif) +static void ath_do_set_opmode(struct ath5k_softc > >> *sc) > >> +{ > >> + struct ath5k_hw *ah = sc->ah; > >> + ath5k_hw_set_opmode(ah, sc->opmode); > >> + ATH5K_DBG(sc, ATH5K_DEBUG_MODE, "mode setup opmode %d (%s)\n", > >> + sc->opmode, > >> + ath_opmode_to_string(sc->opmode) ? > >> + ath_opmode_to_string(sc->opmode) : "UKNOWN"); > >> +} > > > > what's the point of this function? just to add the debug print? > > Yes. At one point, I had it being called from several places..but now it's > called from only one place currently, I think. I could make it explicitly > inline code if someone cared. i think i do care. could you please just open-code it at the one place where you call this function? > > i think it makes sense to set the opmode here. and also do the same in > > ath5k_remove_interface. that way we figure out the opmode when interfaces > > are added/removed. > > It calls ath5k_mode_setup(sc, vif), which sets rx filter, updates bssid > mask, and sets the opmode. > > >> @@ -2905,7 +2919,7 @@ ath5k_remove_interface(struct ieee80211_hw *hw, > >> > >> else if (avf->opmode == NL80211_IFTYPE_ADHOC) > >> > >> sc->num_adhoc_vifs--; > >> > >> - ath5k_update_bssid_mask(sc, NULL); > >> + ath5k_update_bssid_mask_and_opmode(sc, NULL); > >> > >> mutex_unlock(&sc->lock); > > And that recalculates opmode and bssid mask on removal. > > Or maybe I'm mis-understanding your suggestion? i guess my suggestion was to solve it directly in add/remove_interface, as i think that would me a more obvious place to do it, but as your patch already got already merged: nevermind. your solution has advantages too. thanks, bruno -- 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