* [PATCH 1/2] ath5k: Print out opmode in debugfs. @ 2010-10-08 16:43 greearb 2010-10-08 16:43 ` [PATCH 2/2] ath5k: Adjust opmode when interfaces are removed greearb 0 siblings, 1 reply; 7+ messages in thread From: greearb @ 2010-10-08 16:43 UTC (permalink / raw) To: linux-wireless; +Cc: Ben Greear From: Ben Greear <greearb@candelatech.com> Helps debug multi-VIF scenarios. Signed-off-by: Ben Greear <greearb@candelatech.com> --- :100644 100644 c2d549f... a342a9d... M drivers/net/wireless/ath/ath5k/debug.c :100644 100644 53e77bd... a9eb787... M drivers/net/wireless/ath/debug.c :100644 100644 fd3a020... a3a5a62... M drivers/net/wireless/ath/debug.h drivers/net/wireless/ath/ath5k/debug.c | 10 ++++++++++ drivers/net/wireless/ath/debug.c | 29 +++++++++++++++++++++++++++++ drivers/net/wireless/ath/debug.h | 3 +++ 3 files changed, 42 insertions(+), 0 deletions(-) diff --git a/drivers/net/wireless/ath/ath5k/debug.c b/drivers/net/wireless/ath/ath5k/debug.c index c2d549f..a342a9d 100644 --- a/drivers/net/wireless/ath/ath5k/debug.c +++ b/drivers/net/wireless/ath/ath5k/debug.c @@ -60,6 +60,7 @@ #include "base.h" #include "debug.h" +#include "../debug.h" static unsigned int ath5k_debug; module_param_named(debug, ath5k_debug, uint, 0); @@ -492,6 +493,7 @@ static ssize_t read_file_misc(struct file *file, char __user *user_buf, char buf[700]; unsigned int len = 0; u32 filt = ath5k_hw_get_rx_filter(sc->ah); + const char *tmp; len += snprintf(buf+len, sizeof(buf)-len, "bssid-mask: %pM\n", sc->bssidmask); @@ -524,6 +526,14 @@ static ssize_t read_file_misc(struct file *file, char __user *user_buf, else len += snprintf(buf+len, sizeof(buf)-len, "\n"); + tmp = ath_opmode_to_string(sc->opmode); + if (tmp) + len += snprintf(buf+len, sizeof(buf)-len, "opmode: %s\n", + tmp); + else + len += snprintf(buf+len, sizeof(buf)-len, + "opmode: UNKNOWN-%i\n", sc->opmode); + if (len > sizeof(buf)) len = sizeof(buf); diff --git a/drivers/net/wireless/ath/debug.c b/drivers/net/wireless/ath/debug.c index 53e77bd..a9eb787 100644 --- a/drivers/net/wireless/ath/debug.c +++ b/drivers/net/wireless/ath/debug.c @@ -30,3 +30,32 @@ void ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...) va_end(args); } EXPORT_SYMBOL(ath_print); + +const char *ath_opmode_to_string(enum nl80211_iftype opmode) +{ + switch (opmode) { + case NL80211_IFTYPE_UNSPECIFIED: + return "UNSPEC"; + case NL80211_IFTYPE_ADHOC: + return "ADHOC"; + case NL80211_IFTYPE_STATION: + return "STATION"; + case NL80211_IFTYPE_AP: + return "AP"; + case NL80211_IFTYPE_AP_VLAN: + return "AP-VLAN"; + case NL80211_IFTYPE_WDS: + return "WDS"; + case NL80211_IFTYPE_MONITOR: + return "MONITOR"; + case NL80211_IFTYPE_MESH_POINT: + return "MESH"; + case NL80211_IFTYPE_P2P_CLIENT: + return "P2P-CLIENT"; + case NL80211_IFTYPE_P2P_GO: + return "P2P-GO"; + default: + return NULL; + } +} +EXPORT_SYMBOL(ath_opmode_to_string); diff --git a/drivers/net/wireless/ath/debug.h b/drivers/net/wireless/ath/debug.h index fd3a020..a3a5a62 100644 --- a/drivers/net/wireless/ath/debug.h +++ b/drivers/net/wireless/ath/debug.h @@ -77,4 +77,7 @@ ath_print(struct ath_common *common, int dbg_mask, const char *fmt, ...) } #endif /* CONFIG_ATH_DEBUG */ +/** Returns string describing opmode, or NULL if unknown mode. */ +const char *ath_opmode_to_string(enum nl80211_iftype opmode); + #endif /* ATH_DEBUG_H */ -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] ath5k: Adjust opmode when interfaces are removed. 2010-10-08 16:43 [PATCH 1/2] ath5k: Print out opmode in debugfs greearb @ 2010-10-08 16:43 ` greearb 2010-10-08 18:03 ` Bob Copeland 0 siblings, 1 reply; 7+ messages in thread From: greearb @ 2010-10-08 16:43 UTC (permalink / raw) To: linux-wireless; +Cc: Ben Greear From: Ben Greear <greearb@candelatech.com> Otherwise, if there is an AP and a STATION, and AP is removed, the NIC will not revert back to STATION mode. Reported-by: Eliad Peller <eliad@wizery.com> Signed-off-by: Ben Greear <greearb@candelatech.com> --- :100644 100644 dad7265... 813425d... M drivers/net/wireless/ath/ath5k/base.c drivers/net/wireless/ath/ath5k/base.c | 40 +++++++++++++++++++++++++------- 1 files changed, 31 insertions(+), 9 deletions(-) diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c index dad7265..813425d 100644 --- a/drivers/net/wireless/ath/ath5k/base.c +++ b/drivers/net/wireless/ath/ath5k/base.c @@ -62,6 +62,7 @@ #include "reg.h" #include "debug.h" #include "ani.h" +#include "../debug.h" static int modparam_nohwcrypt; module_param_named(nohwcrypt, modparam_nohwcrypt, bool, S_IRUGO); @@ -517,12 +518,14 @@ struct ath_vif_iter_data { bool need_set_hw_addr; bool found_active; bool any_assoc; + enum nl80211_iftype opmode; }; static void ath_vif_iter(void *data, u8 *mac, struct ieee80211_vif *vif) { struct ath_vif_iter_data *iter_data = data; int i; + struct ath5k_vif *avf = (void *)vif->drv_priv; if (iter_data->hw_macaddr) for (i = 0; i < ETH_ALEN; i++) @@ -539,13 +542,29 @@ static void ath_vif_iter(void *data, u8 *mac, struct ieee80211_vif *vif) iter_data->need_set_hw_addr = false; if (!iter_data->any_assoc) { - struct ath5k_vif *avf = (void *)vif->drv_priv; if (avf->assoc) iter_data->any_assoc = true; } + + if (avf->opmode == NL80211_IFTYPE_AP) + iter_data->opmode = NL80211_IFTYPE_AP; + else + if (iter_data->opmode == NL80211_IFTYPE_UNSPECIFIED) + iter_data->opmode = avf->opmode; } -void ath5k_update_bssid_mask(struct ath5k_softc *sc, struct 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"); +} + +void ath5k_update_bssid_mask(struct ath5k_softc *sc, struct ieee80211_vif *vif, + bool update_opmode) { struct ath_common *common = ath5k_hw_common(sc->ah); struct ath_vif_iter_data iter_data; @@ -558,6 +577,7 @@ void ath5k_update_bssid_mask(struct ath5k_softc *sc, struct ieee80211_vif *vif) memset(&iter_data.mask, 0xff, ETH_ALEN); iter_data.found_active = false; iter_data.need_set_hw_addr = true; + iter_data.opmode = NL80211_IFTYPE_UNSPECIFIED; if (vif) ath_vif_iter(&iter_data, vif->addr, vif); @@ -567,6 +587,11 @@ void ath5k_update_bssid_mask(struct ath5k_softc *sc, struct ieee80211_vif *vif) &iter_data); memcpy(sc->bssidmask, iter_data.mask, ETH_ALEN); + if (update_opmode && sc->opmode != iter_data.opmode) { + sc->opmode = iter_data.opmode; + ath_do_set_opmode(sc); + } + if (iter_data.need_set_hw_addr && iter_data.found_active) ath5k_hw_set_lladdr(sc->ah, iter_data.active_mac); @@ -584,12 +609,9 @@ ath5k_mode_setup(struct ath5k_softc *sc, struct ieee80211_vif *vif) ath5k_hw_set_rx_filter(ah, rfilt); if (ath5k_hw_hasbssidmask(ah)) - ath5k_update_bssid_mask(sc, vif); - - /* configure operational mode */ - ath5k_hw_set_opmode(ah, sc->opmode); + ath5k_update_bssid_mask(sc, vif, false); - ATH5K_DBG(sc, ATH5K_DEBUG_MODE, "mode setup opmode %d\n", sc->opmode); + ath_do_set_opmode(sc); ATH5K_DBG(sc, ATH5K_DEBUG_MODE, "RX filter 0x%x\n", rfilt); } @@ -2688,7 +2710,7 @@ ath5k_attach(struct pci_dev *pdev, struct ieee80211_hw *hw) SET_IEEE80211_PERM_ADDR(hw, mac); memcpy(&sc->lladdr, mac, ETH_ALEN); /* All MAC address bits matter for ACKs */ - ath5k_update_bssid_mask(sc, NULL); + ath5k_update_bssid_mask(sc, NULL, false); regulatory->current_rd = ah->ah_capabilities.cap_eeprom.ee_regdomain; ret = ath_regd_init(regulatory, hw->wiphy, ath5k_reg_notifier); @@ -2905,7 +2927,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(sc, NULL, true); mutex_unlock(&sc->lock); } -- 1.7.2.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ath5k: Adjust opmode when interfaces are removed. 2010-10-08 16:43 ` [PATCH 2/2] ath5k: Adjust opmode when interfaces are removed greearb @ 2010-10-08 18:03 ` Bob Copeland 2010-10-08 18:06 ` Ben Greear 0 siblings, 1 reply; 7+ messages in thread From: Bob Copeland @ 2010-10-08 18:03 UTC (permalink / raw) To: greearb; +Cc: linux-wireless On Fri, Oct 8, 2010 at 12:43 PM, <greearb@candelatech.com> wrote: > @@ -558,6 +577,7 @@ void ath5k_update_bssid_mask(struct ath5k_softc *sc, struct ieee80211_vif *vif) > memset(&iter_data.mask, 0xff, ETH_ALEN); > iter_data.found_active = false; > iter_data.need_set_hw_addr = true; > + iter_data.opmode = NL80211_IFTYPE_UNSPECIFIED; > > if (vif) > ath_vif_iter(&iter_data, vif->addr, vif); > @@ -567,6 +587,11 @@ void ath5k_update_bssid_mask(struct ath5k_softc *sc, struct ieee80211_vif *vif) > &iter_data); > memcpy(sc->bssidmask, iter_data.mask, ETH_ALEN); > > + if (update_opmode && sc->opmode != iter_data.opmode) { > + sc->opmode = iter_data.opmode; > + ath_do_set_opmode(sc); > + } > + Should we really couple updating bssid mask and configuring the opmode? Generally, I dislike adding boolean flags to functions because it's hard to figure out from the callsite what is happening (you have to go back to the prototype), and it usually indicates that the abstraction is a little broken. -- Bob Copeland %% www.bobcopeland.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ath5k: Adjust opmode when interfaces are removed. 2010-10-08 18:03 ` Bob Copeland @ 2010-10-08 18:06 ` Ben Greear 2010-10-08 18:13 ` Bob Copeland 0 siblings, 1 reply; 7+ messages in thread From: Ben Greear @ 2010-10-08 18:06 UTC (permalink / raw) To: Bob Copeland; +Cc: linux-wireless On 10/08/2010 11:03 AM, Bob Copeland wrote: > On Fri, Oct 8, 2010 at 12:43 PM,<greearb@candelatech.com> wrote: >> @@ -558,6 +577,7 @@ void ath5k_update_bssid_mask(struct ath5k_softc *sc, struct ieee80211_vif *vif) >> memset(&iter_data.mask, 0xff, ETH_ALEN); >> iter_data.found_active = false; >> iter_data.need_set_hw_addr = true; >> + iter_data.opmode = NL80211_IFTYPE_UNSPECIFIED; >> >> if (vif) >> ath_vif_iter(&iter_data, vif->addr, vif); >> @@ -567,6 +587,11 @@ void ath5k_update_bssid_mask(struct ath5k_softc *sc, struct ieee80211_vif *vif) >> &iter_data); >> memcpy(sc->bssidmask, iter_data.mask, ETH_ALEN); >> >> + if (update_opmode&& sc->opmode != iter_data.opmode) { >> + sc->opmode = iter_data.opmode; >> + ath_do_set_opmode(sc); >> + } >> + > > Should we really couple updating bssid mask and configuring > the opmode? Generally, I dislike adding boolean flags to > functions because it's hard to figure out from the callsite > what is happening (you have to go back to the prototype), and > it usually indicates that the abstraction is a little broken. Well, we need to do the iteration over all VIFS to figure out what to set this too. Seems doing one iteration v/s doing two is worth the extra flag? Thanks, Ben -- Ben Greear <greearb@candelatech.com> Candela Technologies Inc http://www.candelatech.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ath5k: Adjust opmode when interfaces are removed. 2010-10-08 18:06 ` Ben Greear @ 2010-10-08 18:13 ` Bob Copeland 2010-10-08 18:18 ` Ben Greear 0 siblings, 1 reply; 7+ messages in thread From: Bob Copeland @ 2010-10-08 18:13 UTC (permalink / raw) To: Ben Greear; +Cc: linux-wireless On Fri, Oct 8, 2010 at 2:06 PM, Ben Greear <greearb@candelatech.com> wrote: > On 10/08/2010 11:03 AM, Bob Copeland wrote: >> >> On Fri, Oct 8, 2010 at 12:43 PM,<greearb@candelatech.com> wrote: >>> >>> @@ -558,6 +577,7 @@ void ath5k_update_bssid_mask(struct ath5k_softc *sc, >>> struct ieee80211_vif *vif) >>> memset(&iter_data.mask, 0xff, ETH_ALEN); >>> iter_data.found_active = false; >>> iter_data.need_set_hw_addr = true; >>> + iter_data.opmode = NL80211_IFTYPE_UNSPECIFIED; >>> >>> if (vif) >>> ath_vif_iter(&iter_data, vif->addr, vif); >>> @@ -567,6 +587,11 @@ void ath5k_update_bssid_mask(struct ath5k_softc *sc, >>> struct ieee80211_vif *vif) >>> &iter_data); >>> memcpy(sc->bssidmask, iter_data.mask, ETH_ALEN); >>> >>> + if (update_opmode&& sc->opmode != iter_data.opmode) { >>> + sc->opmode = iter_data.opmode; >>> + ath_do_set_opmode(sc); >>> + } >>> + >> >> Should we really couple updating bssid mask and configuring >> the opmode? Generally, I dislike adding boolean flags to >> functions because it's hard to figure out from the callsite >> what is happening (you have to go back to the prototype), and >> it usually indicates that the abstraction is a little broken. > > Well, we need to do the iteration over all VIFS to figure out what to set > this > too. Seems doing one iteration v/s doing two is worth the > extra flag? I admit I haven't looked at the context, so I'm not sure how ugly it is, but you can often do this kind of thing by inverting the loop: update_vif_data() { for all vifs: compute new bssid & opmode update_bssid(new_bssid) update_opmode(neW_opmode) } and call that instead (even in the single vif case). -- Bob Copeland %% www.bobcopeland.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ath5k: Adjust opmode when interfaces are removed. 2010-10-08 18:13 ` Bob Copeland @ 2010-10-08 18:18 ` Ben Greear 2010-10-08 18:25 ` Bob Copeland 0 siblings, 1 reply; 7+ messages in thread From: Ben Greear @ 2010-10-08 18:18 UTC (permalink / raw) To: Bob Copeland; +Cc: linux-wireless On 10/08/2010 11:13 AM, Bob Copeland wrote: > On Fri, Oct 8, 2010 at 2:06 PM, Ben Greear<greearb@candelatech.com> wrote: >> On 10/08/2010 11:03 AM, Bob Copeland wrote: >>> >>> On Fri, Oct 8, 2010 at 12:43 PM,<greearb@candelatech.com> wrote: >>>> >>>> @@ -558,6 +577,7 @@ void ath5k_update_bssid_mask(struct ath5k_softc *sc, >>>> struct ieee80211_vif *vif) >>>> memset(&iter_data.mask, 0xff, ETH_ALEN); >>>> iter_data.found_active = false; >>>> iter_data.need_set_hw_addr = true; >>>> + iter_data.opmode = NL80211_IFTYPE_UNSPECIFIED; >>>> >>>> if (vif) >>>> ath_vif_iter(&iter_data, vif->addr, vif); >>>> @@ -567,6 +587,11 @@ void ath5k_update_bssid_mask(struct ath5k_softc *sc, >>>> struct ieee80211_vif *vif) >>>> &iter_data); >>>> memcpy(sc->bssidmask, iter_data.mask, ETH_ALEN); >>>> >>>> + if (update_opmode&& sc->opmode != iter_data.opmode) { >>>> + sc->opmode = iter_data.opmode; >>>> + ath_do_set_opmode(sc); >>>> + } >>>> + >>> >>> Should we really couple updating bssid mask and configuring >>> the opmode? Generally, I dislike adding boolean flags to >>> functions because it's hard to figure out from the callsite >>> what is happening (you have to go back to the prototype), and >>> it usually indicates that the abstraction is a little broken. >> >> Well, we need to do the iteration over all VIFS to figure out what to set >> this >> too. Seems doing one iteration v/s doing two is worth the >> extra flag? > > I admit I haven't looked at the context, so I'm not sure how ugly it > is, but you can often do this kind of thing by inverting the loop: > > update_vif_data() > { > for all vifs: > compute new bssid& opmode > > update_bssid(new_bssid) > update_opmode(neW_opmode) > } > > and call that instead (even in the single vif case). I can just always set the opmode if you prefer that. Maybe change the name of that method. I'll work on a patch. Thanks, Ben > -- Ben Greear <greearb@candelatech.com> Candela Technologies Inc http://www.candelatech.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ath5k: Adjust opmode when interfaces are removed. 2010-10-08 18:18 ` Ben Greear @ 2010-10-08 18:25 ` Bob Copeland 0 siblings, 0 replies; 7+ messages in thread From: Bob Copeland @ 2010-10-08 18:25 UTC (permalink / raw) To: Ben Greear; +Cc: linux-wireless On Fri, Oct 08, 2010 at 11:18:34AM -0700, Ben Greear wrote: > I can just always set the opmode if you prefer that. Maybe > change the name of that method. > > I'll work on a patch. Yeah I think that would be better from a long-term maintenance standpoint. Thanks! -- Bob Copeland %% www.bobcopeland.com ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-10-08 18:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-10-08 16:43 [PATCH 1/2] ath5k: Print out opmode in debugfs greearb 2010-10-08 16:43 ` [PATCH 2/2] ath5k: Adjust opmode when interfaces are removed greearb 2010-10-08 18:03 ` Bob Copeland 2010-10-08 18:06 ` Ben Greear 2010-10-08 18:13 ` Bob Copeland 2010-10-08 18:18 ` Ben Greear 2010-10-08 18:25 ` Bob Copeland
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).