On Tue, Nov 20, 2012 at 1:42 PM, Luciano Coelho <coelho@xxxxxx> wrote: > On Tue, 2012-11-20 at 13:20 +0200, Eliad Peller wrote: >> Make the connection flow simpler by starting >> sta role on bssid change. >> >> Currently, we start dev role when going idle-off, >> and start the sta role only after association >> indication. This complicates the connection >> flow with some possible intermediate states. >> >> Make it simpler by starting sta role on bssid change, >> which now happens *before* auth req get sent. >> >> Update the handling of mac80211's notifications >> and change wl1271_join/unjoin accordingly - >> * Split wl1271_join() into wlcore_join (tuning on >> a channel/bssid) and wlcore_set_assoc (configure >> sta after association). >> * Rename wl1271_unjoin() to wlcore_unset_assoc(), as >> it is no longer the inversion of wl1271_join() >> (now it's only used to disconnect associated sta / >> joined ibss, without stopping the role). >> * Set ssid before starting station role (needed for >> start_role(sta) >> >> While on it, split wl1271_bss_info_changed_sta() into >> some sub-functions. >> >> since we no longer use dev role in the connection flow, >> we now always use the hlid of the sta role. >> >> Signed-off-by: Eliad Peller <eliad@xxxxxxxxxx> >> --- >> v2: take some more code out of wl1271_bss_info_changed_sta (thanks Luca!) >> squash patches [2/15, 3/15] into this one (thanks Julian!) >> > > [...] > >> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c >> index 41ed1d5..2690fe9 100644 >> --- a/drivers/net/wireless/ti/wlcore/main.c >> +++ b/drivers/net/wireless/ti/wlcore/main.c > > [...] > >> @@ -2510,29 +2587,58 @@ static int wl1271_join(struct wl1271 *wl, struct wl12xx_vif *wlvif, >> */ >> ret = wl1271_acx_keep_alive_mode(wl, wlvif, true); >> if (ret < 0) >> - goto out; >> + return ret; >> >> ret = wl1271_acx_aid(wl, wlvif, wlvif->aid); >> if (ret < 0) >> - goto out; >> + return ret; >> >> ret = wl12xx_cmd_build_klv_null_data(wl, wlvif); >> if (ret < 0) >> - goto out; >> + return ret; >> >> ret = wl1271_acx_keep_alive_config(wl, wlvif, >> wlvif->sta.klv_template_id, >> ACX_KEEP_ALIVE_TPL_VALID); >> if (ret < 0) >> - goto out; >> + return ret; >> >> -out: >> return ret; >> } > > This is, of course, functionally okay, but seems like an unrelated > change. Also, this is different than the general style we use in the > driver (ie. use goto out in most error cases). > > Same thing for the other functions you added. It would be nice to be > consistent with the existing style, at least in error paths. > AFAICT, a major part of the functions in the driver use the "goto out" only when there's cleanup work to do. this is also the convention in mac80211. but then again, i don't mind changing it if that's your preference. Eliad. -- 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