Make the connection flow simpler by using only the sta
role (i.e. no need for dev role).
This is possible now, as we know the AP's BSSID even
before authentication.
This means we no longer start dev role when the device
is non-idle, so now we use the .remain_on_channel()
and start device role only then.
Finally, implement the chanctx callbacks in order to
have a clear per-vif channel (instead of the global
hw->channel)
(this patchset depends on
"mac80211: make remain_on_channel() op pass vif param")
Eliad Peller (15):
wlcore: start sta role on CHANGED_BSSID
wlcore: set ssid before starting station role
wlcore: always use sta.hlid
wlcore: workaround start_sta problem in wl12xx fw
wlcore: implement .remain_on_channel() callback
wlcore: use dev_hlid if the tx is offchannel
wlcore: get channel from bss_conf instead of hw->conf
wlcore: add chanctx implementation
wlcore: remove channel handling from op_config
wlcore: initiate ROC/CROC on sta state updates
wlcore: set active psm on association
wlcore: specify correct supported_rates
wlcore: reconfigure rate policy on association
wlcore: refactor CHANGED_HT handling
wlcore: configure the remote rates with our own rates
drivers/net/wireless/ti/wl12xx/main.c | 9 +-
drivers/net/wireless/ti/wlcore/cmd.c | 56 ++--
drivers/net/wireless/ti/wlcore/cmd.h | 6 +-
drivers/net/wireless/ti/wlcore/event.c | 7 +
drivers/net/wireless/ti/wlcore/main.c | 663 ++++++++++++++++++-------------
drivers/net/wireless/ti/wlcore/tx.c | 14 +-
drivers/net/wireless/ti/wlcore/wlcore.h | 6 +
7 files changed, 454 insertions(+), 307 deletions(-)
--
1.7.6.401.g6a319
The default ps mode of the fw is auto, while the default
ps mode of mac80211 is active (ps off).
In order to sync them, configure active ps on association.
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index e1f0606..0dd0ec1 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2534,6 +2534,15 @@ static int wlcore_set_assoc(struct wl1271 *wl, struct wl12xx_vif *wlvif)
ACX_KEEP_ALIVE_TPL_VALID);
if (ret < 0)
goto out;
+
+ /*
+ * The default fw psm configuration is AUTO, while mac80211 default
+ * setting is off (ACTIVE), so sync the fw with the correct value.
+ */
+ ret = wl1271_ps_set_mode(wl, wlvif, STATION_ACTIVE_MODE);
+ if (ret < 0)
+ goto out;
+
out:
return ret;
}
--
1.7.6.401.g6a319
On Tue, Nov 20, 2012 at 1:42 PM, Johannes Berg
<[email protected]> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>
>> +static int wlcore_op_add_chanctx(struct ieee80211_hw *hw,
>> + struct ieee80211_chanctx_conf *ctx)
>> +{
>> + wl1271_debug(DEBUG_MAC80211, "mac80211 add chanctx %d (type %d)",
>> + ieee80211_frequency_to_channel(ctx->channel->center_freq),
>> + ctx->channel_type);
>
> Ok so I just realized there's another small issue with this patch/these
> patches -- my VHT changes, which I'm planning to merge soon, will change
> the chanctx_conf structure contents, which might then be tricky with git
> tree handling?
>
> I'm not too worried about that, but might be something to keep in mind.
> Right now my changes are easy because they only affect hwsim as the only
> driver using channel contexts.
>
yeah, we don't do anything fancy with the chanctx, so i guess the
merge should be fairly easy.
thanks for the heads up. we'll keep it in mind.
Eliad.
On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <[email protected]> wrote:
> implement the reamin_on_channel() callback by starting
> a dev role (already associated with the current vif)
> on the requested channel/band.
>
> This channel is usually different from the channel
> of the sta role, so pass it to wl12xx_roc() as well,
> and notify mac80211 (async) when the fw is ready
> on the new channel.
>
> Signed-off-by: Eliad Peller <[email protected]>
[...]
> @@ -1608,7 +1610,7 @@ static int wl12xx_cmd_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif,
> }
>
> cmd->role_id = role_id;
> - cmd->channel = wlvif->channel;
> + cmd->channel = channel;
> switch (wlvif->band) {
> case IEEE80211_BAND_2GHZ:
> cmd->band = WLCORE_BAND_2_4GHZ;
This seems wrong... What happens if the roc is on another band from
the one the vif is connected on?
The dev role is started on the correct band, but the roc will be on
the band of the wlvif..
[...]
> +static int wlcore_op_remain_on_channel(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif,
> + struct ieee80211_channel *chan,
> + enum nl80211_channel_type channel_type,
> + int duration)
> +{
> + struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
> + struct wl1271 *wl = hw->priv;
> + int channel, ret = 0;
> +
> + channel = ieee80211_frequency_to_channel(chan->center_freq);
> +
> + wl1271_debug(DEBUG_MAC80211, "mac80211 roc %d (%d)",
> + channel, wlvif->role_id);
> +
> + mutex_lock(&wl->mutex);
> +
> + if (unlikely(wl->state != WLCORE_STATE_ON))
> + goto out;
> +
> + /* return EBUSY if we can't ROC right now */
> + if (WARN_ON(wl->roc_vif ||
> + find_first_bit(wl->roc_map,
> + WL12XX_MAX_ROLES) < WL12XX_MAX_ROLES)) {
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + /*
> + * make sure the vif has dev role.
> + * TODO: enable dev role dynamically instead of pre-allocating it.
> + */
Not sure what you mean here. Seems pretty dynamic to me..
> + if (WARN_ON_ONCE(!(wlvif->bss_type == BSS_TYPE_STA_BSS ||
> + wlvif->bss_type == BSS_TYPE_IBSS))) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + ret = wl1271_ps_elp_wakeup(wl);
> + if (ret < 0)
> + goto out;
> +
> + ret = wl12xx_start_dev(wl, wlvif, chan->band, channel);
> + if (ret < 0)
> + goto out_sleep;
> +
> + wl->roc_vif = vif;
> + ieee80211_queue_delayed_work(hw, &wl->roc_complete_work,
> + msecs_to_jiffies(duration));
> +out_sleep:
> + wl1271_ps_elp_sleep(wl);
> +out:
> + mutex_unlock(&wl->mutex);
> + return ret;
> +}
> +
> +static int __wlcore_roc_completed(struct wl1271 *wl)
> +{
> + struct wl12xx_vif *wlvif;
> + int ret;
> +
> + /* already completed */
> + if (unlikely(!wl->roc_vif))
> + return 0;
> +
> + wlvif = wl12xx_vif_to_data(wl->roc_vif);
> +
> + if (!test_bit(WLVIF_FLAG_INITIALIZED, &wlvif->flags))
> + return -EBUSY;
> +
> + ret = wl12xx_stop_dev(wl, wlvif);
> + if (ret < 0)
> + return ret;
> +
> + wl->roc_vif = NULL;
> +
> + return 0;
> +}
> +
> +static int wlcore_roc_completed(struct wl1271 *wl)
> +{
> + int ret;
> +
> + wl1271_debug(DEBUG_MAC80211, "roc complete");
> +
> + mutex_lock(&wl->mutex);
> +
> + if (unlikely(wl->state != WLCORE_STATE_ON)) {
> + ret = -EBUSY;
> + goto out;
> + }
> +
> + ret = wl1271_ps_elp_wakeup(wl);
> + if (ret < 0)
> + goto out;
> +
> + ret = __wlcore_roc_completed(wl);
> +
> + wl1271_ps_elp_sleep(wl);
> +out:
> + mutex_unlock(&wl->mutex);
> +
> + return ret;
> +}
> +
> +static void wlcore_roc_complete_work(struct work_struct *work)
> +{
> + struct delayed_work *dwork;
> + struct wl1271 *wl;
> + int ret;
> +
> + dwork = container_of(work, struct delayed_work, work);
> + wl = container_of(dwork, struct wl1271, roc_complete_work);
> +
> + ret = wlcore_roc_completed(wl);
> + if (!ret)
> + ieee80211_remain_on_channel_expired(wl->hw);
> +}
> +
> +static int wlcore_op_cancel_remain_on_channel(struct ieee80211_hw *hw)
> +{
> + struct wl1271 *wl = hw->priv;
> +
> + wl1271_debug(DEBUG_MAC80211, "mac80211 croc");
> +
> + /* TODO: per-vif */
> + wl1271_tx_flush(wl);
> +
> + /*
> + * we can't just flush_work here, because it might deadlock
> + * (as we might get called from the same workqueue)
> + */
> + cancel_delayed_work_sync(&wl->roc_complete_work);
> + wlcore_roc_completed(wl);
> +
> + return 0;
> +}
> +
> static bool wl1271_tx_frames_pending(struct ieee80211_hw *hw)
> {
> struct wl1271 *wl = hw->priv;
> @@ -4849,6 +4970,8 @@ static const struct ieee80211_ops wl1271_ops = {
> .set_bitrate_mask = wl12xx_set_bitrate_mask,
> .channel_switch = wl12xx_op_channel_switch,
> .flush = wlcore_op_flush,
> + .remain_on_channel = wlcore_op_remain_on_channel,
> + .cancel_remain_on_channel = wlcore_op_cancel_remain_on_channel,
> CFG80211_TESTMODE_CMD(wl1271_tm_cmd)
> };
>
> @@ -5245,6 +5368,8 @@ static int wl1271_init_ieee80211(struct wl1271 *wl)
> wl->hw->wiphy->max_sched_scan_ie_len = WL1271_CMD_TEMPL_MAX_SIZE -
> sizeof(struct ieee80211_header);
>
> + wl->hw->wiphy->max_remain_on_channel_duration = 5000;
Isn't this the default?
> +
> wl->hw->wiphy->flags |= WIPHY_FLAG_AP_UAPSD |
> WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL;
>
> @@ -5343,6 +5468,7 @@ struct ieee80211_hw *wlcore_alloc_hw(size_t priv_size, u32 aggr_buf_size)
> INIT_WORK(&wl->tx_work, wl1271_tx_work);
> INIT_WORK(&wl->recovery_work, wl1271_recovery_work);
> INIT_DELAYED_WORK(&wl->scan_complete_work, wl1271_scan_complete_work);
> + INIT_DELAYED_WORK(&wl->roc_complete_work, wlcore_roc_complete_work);
> INIT_DELAYED_WORK(&wl->tx_watchdog_work, wl12xx_tx_watchdog_work);
> INIT_DELAYED_WORK(&wl->connection_loss_work,
> wl1271_connection_loss_work);
> diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h b/drivers/net/wireless/ti/wlcore/wlcore.h
> index 1030b6b..b636f1d 100644
> --- a/drivers/net/wireless/ti/wlcore/wlcore.h
> +++ b/drivers/net/wireless/ti/wlcore/wlcore.h
> @@ -286,6 +286,9 @@ struct wl1271 {
> /* Connection loss work */
> struct delayed_work connection_loss_work;
>
> + struct ieee80211_vif *roc_vif;
> + struct delayed_work roc_complete_work;
> +
> bool sched_scanning;
>
> /* The current band */
> --
> 1.7.6.401.g6a319
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 20, 2012 at 10:38 AM, Eliad Peller <[email protected]> wrote:
> On Mon, Nov 19, 2012 at 11:23 PM, Arik Nemtsov <[email protected]> wrote:
>> On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <[email protected]> wrote:
>>> Add some basic chanctx implementation.
>>>
>>> Only add debug prints, and save the vif's channel/band/type.
>>>
>>> Signed-off-by: Eliad Peller <[email protected]>
>>> ---
>
>>> +static int wlcore_op_assign_vif_chanctx(struct ieee80211_hw *hw,
>>> + struct ieee80211_vif *vif,
>>> + struct ieee80211_chanctx_conf *ctx)
>>> +{
>>> + struct wl1271 *wl = hw->priv;
>>> + struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
>>> + int channel = ieee80211_frequency_to_channel(
>>> + ctx->channel->center_freq);
>>> +
>>> + wl1271_debug(DEBUG_MAC80211,
>>> + "mac80211 assign chanctx (role %d) %d (type %d)",
>>> + wlvif->role_id, channel, ctx->channel_type);
>>> +
>>> + wlcore_tx_work_locked(wl);
>>
>> is this some kind of a lame attempt at a flush? :)
>>
>> why not use flush here if the channel was set - or better yet, flush
>> stuff at wlcore_op_unassign_vif_chanctx, which is currently unused.
>> we'll need the TODO there for the per-vif-flush. once everything is
>> upstreamed (hw queues) it would be pretty trivial to implement.
>>
> ok, you convinced me. i'll do a flush on unassign instead. :)
btw, it's also a bug to call wlcore_tx_work_locked() since you didn't
take the mutex, which is another bug in your patch. you should take
the mutex before changing members of wlvif :)
>
>> also not sure what happened to the rest of the stuff done on channel
>> config - updating the rate_set according to band etc..
>>
> well, actually you have a pending patch to fix it, and i planned
> sending it in the next patchset. but since you already asked about it
> i'll just squash it into this patch (and add your SOB). :)
any form you prefer.
On Tue, Nov 20, 2012 at 9:55 AM, Luciano Coelho <[email protected]> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> The supported_rates field should contain all our supported
>> rates, even if the remote peer doesn't support them.
>>
>> (use CONF_TX_AP_ENABLED_RATES for bg rates, as the possible
>> rates are the same for ap and sta)
>
> Maybe the macro should be renamed then?
>
sure.
>> @@ -461,7 +462,14 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>> cmd->sta.ssid_len = wlvif->ssid_len;
>> memcpy(cmd->sta.ssid, wlvif->ssid, wlvif->ssid_len);
>> memcpy(cmd->sta.bssid, vif->bss_conf.bssid, ETH_ALEN);
>> - cmd->sta.local_rates = cpu_to_le32(wlvif->rate_set);
>> +
>> + supported_rates = CONF_TX_AP_ENABLED_RATES | CONF_TX_MCS_RATES |
>> + wlcore_hw_sta_get_ap_rate_mask(wl, wlvif);
>> + if (wlvif->p2p)
>> + supported_rates &= ~CONF_TX_CCK_RATES;
>
> Why not do this when creating the vif (init_vif_data)? I think this kind
> of code doesn't belong in the cmd function. Seem better to adjust
> wlvif->rate_set.
>
well, this value is used only here. i don't think adding it to wl
struct and initializing it in a different place is really better.
however, i don't mind doing so if you insist :)
Eliad.
Pass a variable indicating whether HT is enabled,
instead of duplicating the function call with
different arguments.
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 48 ++++++++++++--------------------
1 files changed, 18 insertions(+), 30 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index a1ad326..a81e50b 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -3975,44 +3975,32 @@ sta_not_found:
}
/* Handle new association with HT. Do this after join. */
- if (sta_exists) {
- if ((changed & BSS_CHANGED_HT) &&
- (bss_conf->channel_type != NL80211_CHAN_NO_HT)) {
- ret = wl1271_acx_set_ht_capabilities(wl,
- &sta_ht_cap,
- true,
- wlvif->sta.hlid);
- if (ret < 0) {
- wl1271_warning("Set ht cap true failed %d",
- ret);
- goto out;
- }
+ if (sta_exists &&
+ (changed & BSS_CHANGED_HT)) {
+ bool enabled =
+ bss_conf->channel_type != NL80211_CHAN_NO_HT;
+
+ ret = wl1271_acx_set_ht_capabilities(wl,
+ &sta_ht_cap,
+ enabled,
+ wlvif->sta.hlid);
+ if (ret < 0) {
+ wl1271_warning("Set ht cap failed %d", ret);
+ goto out;
+
}
- /* handle new association without HT and disassociation */
- else if (changed & BSS_CHANGED_ASSOC) {
- ret = wl1271_acx_set_ht_capabilities(wl,
- &sta_ht_cap,
- false,
- wlvif->sta.hlid);
+
+ if (enabled) {
+ ret = wl1271_acx_set_ht_information(wl, wlvif,
+ bss_conf->ht_operation_mode);
if (ret < 0) {
- wl1271_warning("Set ht cap false failed %d",
+ wl1271_warning("Set ht information failed %d",
ret);
goto out;
}
}
}
- /* Handle HT information change. Done after join. */
- if ((changed & BSS_CHANGED_HT) &&
- (bss_conf->channel_type != NL80211_CHAN_NO_HT)) {
- ret = wl1271_acx_set_ht_information(wl, wlvif,
- bss_conf->ht_operation_mode);
- if (ret < 0) {
- wl1271_warning("Set ht information failed %d", ret);
- goto out;
- }
- }
-
/* Handle arp filtering. Done after join. */
if ((changed & BSS_CHANGED_ARP_FILTER) ||
(!is_ibss && (changed & BSS_CHANGED_QOS))) {
--
1.7.6.401.g6a319
After setting the channel on chanctx callbacks, we
no longer need to handle channel change notifications
on op_config.
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 68 +-------------------------------
1 files changed, 3 insertions(+), 65 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index caafd8c..fcdf381 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2569,49 +2569,7 @@ static int wl12xx_config_vif(struct wl1271 *wl, struct wl12xx_vif *wlvif,
struct ieee80211_conf *conf, u32 changed)
{
bool is_ap = (wlvif->bss_type == BSS_TYPE_AP_BSS);
- int channel, ret;
-
- channel = ieee80211_frequency_to_channel(conf->channel->center_freq);
-
- /* if the channel changes while joined, join again */
- if (changed & IEEE80211_CONF_CHANGE_CHANNEL &&
- ((wlvif->band != conf->channel->band) ||
- (wlvif->channel != channel) ||
- (wlvif->channel_type != conf->channel_type))) {
- /* send all pending packets */
- ret = wlcore_tx_work_locked(wl);
- if (ret < 0)
- return ret;
-
- wlvif->band = conf->channel->band;
- wlvif->channel = channel;
- wlvif->channel_type = conf->channel_type;
-
- if (is_ap) {
- wl1271_set_band_rate(wl, wlvif);
- ret = wl1271_init_ap_rates(wl, wlvif);
- if (ret < 0)
- wl1271_error("AP rate policy change failed %d",
- ret);
- } else {
- /*
- * FIXME: the mac80211 should really provide a fixed
- * rate to use here. for now, just use the smallest
- * possible rate for the band as a fixed rate for
- * association frames and other control messages.
- */
- if (!test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags))
- wl1271_set_band_rate(wl, wlvif);
-
- wlvif->basic_rate =
- wl1271_tx_min_rate_get(wl,
- wlvif->basic_rate_set);
- ret = wl1271_acx_sta_rate_policies(wl, wlvif);
- if (ret < 0)
- wl1271_warning("rate policy for channel "
- "failed %d", ret);
- }
- }
+ int ret;
if ((changed & IEEE80211_CONF_CHANGE_PS) && !is_ap) {
@@ -2666,37 +2624,17 @@ static int wl1271_op_config(struct ieee80211_hw *hw, u32 changed)
struct wl1271 *wl = hw->priv;
struct wl12xx_vif *wlvif;
struct ieee80211_conf *conf = &hw->conf;
- int channel, ret = 0;
-
- channel = ieee80211_frequency_to_channel(conf->channel->center_freq);
+ int ret = 0;
- wl1271_debug(DEBUG_MAC80211, "mac80211 config ch %d psm %s power %d %s"
+ wl1271_debug(DEBUG_MAC80211, "mac80211 config psm %s power %d %s"
" changed 0x%x",
- channel,
conf->flags & IEEE80211_CONF_PS ? "on" : "off",
conf->power_level,
conf->flags & IEEE80211_CONF_IDLE ? "idle" : "in use",
changed);
- /*
- * mac80211 will go to idle nearly immediately after transmitting some
- * frames, such as the deauth. To make sure those frames reach the air,
- * wait here until the TX queue is fully flushed.
- */
- if ((changed & IEEE80211_CONF_CHANGE_CHANNEL) ||
- ((changed & IEEE80211_CONF_CHANGE_IDLE) &&
- (conf->flags & IEEE80211_CONF_IDLE)))
- wl1271_tx_flush(wl);
-
mutex_lock(&wl->mutex);
- /* we support configuring the channel and band even while off */
- if (changed & IEEE80211_CONF_CHANGE_CHANNEL) {
- wl->band = conf->channel->band;
- wl->channel = channel;
- wl->channel_type = conf->channel_type;
- }
-
if (changed & IEEE80211_CONF_CHANGE_POWER)
wl->power_level = conf->power_level;
--
1.7.6.401.g6a319
since we no longer use dev role in the connection flow,
we should always use the hlid of the sta role.
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/tx.c | 10 +---------
1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/tx.c b/drivers/net/wireless/ti/wlcore/tx.c
index a90d3cd..a555a70 100644
--- a/drivers/net/wireless/ti/wlcore/tx.c
+++ b/drivers/net/wireless/ti/wlcore/tx.c
@@ -155,21 +155,13 @@ static u8 wl12xx_tx_get_hlid_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif,
u8 wl12xx_tx_get_hlid(struct wl1271 *wl, struct wl12xx_vif *wlvif,
struct sk_buff *skb, struct ieee80211_sta *sta)
{
- struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-
if (!wlvif || wl12xx_is_dummy_packet(wl, skb))
return wl->system_hlid;
if (wlvif->bss_type == BSS_TYPE_AP_BSS)
return wl12xx_tx_get_hlid_ap(wl, wlvif, skb, sta);
- if ((test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags) ||
- test_bit(WLVIF_FLAG_IBSS_JOINED, &wlvif->flags)) &&
- !ieee80211_is_auth(hdr->frame_control) &&
- !ieee80211_is_assoc_req(hdr->frame_control))
- return wlvif->sta.hlid;
- else
- return wlvif->dev_hlid;
+ return wlvif->sta.hlid;
}
unsigned int wlcore_calc_packet_alignment(struct wl1271 *wl,
--
1.7.6.401.g6a319
On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> +static int wlcore_op_add_chanctx(struct ieee80211_hw *hw,
> + struct ieee80211_chanctx_conf *ctx)
> +{
> + wl1271_debug(DEBUG_MAC80211, "mac80211 add chanctx %d (type %d)",
> + ieee80211_frequency_to_channel(ctx->channel->center_freq),
> + ctx->channel_type);
Ok so I just realized there's another small issue with this patch/these
patches -- my VHT changes, which I'm planning to merge soon, will change
the chanctx_conf structure contents, which might then be tricky with git
tree handling?
I'm not too worried about that, but might be something to keep in mind.
Right now my changes are easy because they only affect hwsim as the only
driver using channel contexts.
johannes
start_role(station) takes the ssid as parameter, so set
the correct ssid on BSS_CHANGED_BSSID (just before
calling start_role(sta))
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 41a8502..2cb978e 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -3390,6 +3390,23 @@ static int wl1271_ssid_set(struct ieee80211_vif *vif, struct sk_buff *skb,
return 0;
}
+static int wlcore_set_ssid(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+{
+ struct ieee80211_vif *vif = wl12xx_wlvif_to_vif(wlvif);
+ struct sk_buff *skb;
+ int ieoffset;
+
+ skb = ieee80211_ap_probereq_get(wl->hw, vif);
+ if (!skb)
+ return -EINVAL;
+
+ ieoffset = offsetof(struct ieee80211_mgmt,
+ u.probe_req.variable);
+ wl1271_ssid_set(vif, skb, ieoffset);
+ dev_kfree_skb(skb);
+
+ return 0;
+}
static void wl12xx_remove_ie(struct sk_buff *skb, u8 eid, int ieoffset)
{
int len;
@@ -3875,6 +3892,8 @@ sta_not_found:
if (ret < 0)
goto out;
+ wlcore_set_ssid(wl, wlvif);
+
/* Need to update the BSSID (for filtering etc) */
set_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
do_join = true;
--
1.7.6.401.g6a319
On Tue, Nov 20, 2012 at 10:42 AM, Eliad Peller <[email protected]> wrote:
> On Mon, Nov 19, 2012 at 11:41 PM, Arik Nemtsov <[email protected]> wrote:
>> On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <[email protected]> wrote:
>>> Use the sta_state notifications to ROC when a station
>>> is about to connect, and CROC respectively on
>>> authorization (success) / deletion (failure).
>>>
>>> Change the wl12xx_update_sta_state() flow to bail out
>>> only on error, so multiple code blocks could refer
>>> to the same state.
>>>
>>> Signed-off-by: Eliad Peller <[email protected]>
>> [...]
>>>
>>> + /* clear ROCs on failure or authorization */
>>> + if (is_sta &&
>>> + (new_state == IEEE80211_STA_AUTHORIZED ||
>>> + new_state == IEEE80211_STA_NOTEXIST)) {
>>> + if (test_bit(wlvif->role_id, wl->roc_map))
>>> + wl12xx_croc(wl, wlvif->role_id);
>>> + }
>>> +
>>> + if (is_sta &&
>>> + old_state == IEEE80211_STA_NOTEXIST &&
>>> + new_state == IEEE80211_STA_NONE) {
>>> + if (find_first_bit(wl->roc_map,
>>> + WL12XX_MAX_ROLES) >= WL12XX_MAX_ROLES) {
>>> + WARN_ON(wlvif->role_id == WL12XX_INVALID_ROLE_ID);
>>> + wl12xx_roc(wl, wlvif, wlvif->role_id, wlvif->channel);
>>> + }
>>> + }
>>
>> what about AP mode? we don't have an opportunistic ROC there as well
>> for connecting stations?
>
> yeah, we do. but since in order to really be effective it requires
> supplicant changes that weren't upstreamed yet (adding non-associated
> station), i preferred postponing it to another patchset.
> on the other hand, this might help protecting the EAPOL exchange even
> in its current form, so i guess we can upstream it even before the
> supplicant changes.
yea. since this is opportunistic, it can't really hurt.
On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <[email protected]> wrote:
> After setting the channel on chanctx callbacks, we
> no longer need to handle channel change notifications
> on op_config.
>
> Signed-off-by: Eliad Peller <[email protected]>
[...]
> - /*
> - * mac80211 will go to idle nearly immediately after transmitting some
> - * frames, such as the deauth. To make sure those frames reach the air,
> - * wait here until the TX queue is fully flushed.
> - */
> - if ((changed & IEEE80211_CONF_CHANGE_CHANNEL) ||
> - ((changed & IEEE80211_CONF_CHANGE_IDLE) &&
> - (conf->flags & IEEE80211_CONF_IDLE)))
> - wl1271_tx_flush(wl);
This flush (and the comment above) is also a good reason for flushing
tx during wlcore_op_unassign_vif_chanctx...
Hi Julian,
On Tue, Nov 20, 2012 at 12:39 AM, Julian Calaby <[email protected]> wrote:
> On Tue, Nov 20, 2012 at 3:39 AM, Eliad Peller <[email protected]> wrote:
>> since we no longer use dev role in the connection flow,
>> we should always use the hlid of the sta role.
>
> I don't really know the hardware, but this seems like something
> something that should be combined with the first patch.
>
Thanks.
Johannes had a similar comment regarding another patch.
i split the patches in order to make them easier to review, even if it
temporarily breaks the driver.
As agreed with Luca, i'll send the next version (which will hopefully
be the last one ;), after squashing such patches together.
Eliad.
Use the sta_state notifications to ROC when a station
is about to connect, and CROC respectively on
authorization (success) / deletion (failure).
Change the wl12xx_update_sta_state() flow to bail out
only on error, so multiple code blocks could refer
to the same state.
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 43 ++++++++++++++++++++------------
1 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index fcdf381..e1f0606 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -83,8 +83,6 @@ static int wl12xx_set_authorized(struct wl1271 *wl,
if (ret < 0)
return ret;
- wl12xx_croc(wl, wlvif->role_id);
-
wl1271_info("Association completed.");
return 0;
}
@@ -3944,14 +3942,6 @@ sta_not_found:
wl1271_warning("cmd join failed %d", ret);
goto out;
}
-
- /* ROC until connected (after EAPOL exchange) */
- if (!is_ibss) {
- ret = wl12xx_roc(wl, wlvif, wlvif->role_id,
- wlvif->channel);
- if (ret < 0)
- goto out;
- }
}
if (set_assoc) {
@@ -4355,8 +4345,11 @@ static int wl12xx_update_sta_state(struct wl1271 *wl,
/* Add station (AP mode) */
if (is_ap &&
old_state == IEEE80211_STA_NOTEXIST &&
- new_state == IEEE80211_STA_NONE)
- return wl12xx_sta_add(wl, wlvif, sta);
+ new_state == IEEE80211_STA_NONE) {
+ ret = wl12xx_sta_add(wl, wlvif, sta);
+ if (ret)
+ return ret;
+ }
/* Remove station (AP mode) */
if (is_ap &&
@@ -4364,7 +4357,6 @@ static int wl12xx_update_sta_state(struct wl1271 *wl,
new_state == IEEE80211_STA_NOTEXIST) {
/* must not fail */
wl12xx_sta_remove(wl, wlvif, sta);
- return 0;
}
/* Authorize station (AP mode) */
@@ -4376,14 +4368,17 @@ static int wl12xx_update_sta_state(struct wl1271 *wl,
ret = wl1271_acx_set_ht_capabilities(wl, &sta->ht_cap, true,
hlid);
- return ret;
+ if (ret)
+ return ret;
}
/* Authorize station */
if (is_sta &&
new_state == IEEE80211_STA_AUTHORIZED) {
set_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags);
- return wl12xx_set_authorized(wl, wlvif);
+ ret = wl12xx_set_authorized(wl, wlvif);
+ if (ret)
+ return ret;
}
if (is_sta &&
@@ -4391,9 +4386,25 @@ static int wl12xx_update_sta_state(struct wl1271 *wl,
new_state == IEEE80211_STA_ASSOC) {
clear_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags);
clear_bit(WLVIF_FLAG_STA_STATE_SENT, &wlvif->flags);
- return 0;
}
+ /* clear ROCs on failure or authorization */
+ if (is_sta &&
+ (new_state == IEEE80211_STA_AUTHORIZED ||
+ new_state == IEEE80211_STA_NOTEXIST)) {
+ if (test_bit(wlvif->role_id, wl->roc_map))
+ wl12xx_croc(wl, wlvif->role_id);
+ }
+
+ if (is_sta &&
+ old_state == IEEE80211_STA_NOTEXIST &&
+ new_state == IEEE80211_STA_NONE) {
+ if (find_first_bit(wl->roc_map,
+ WL12XX_MAX_ROLES) >= WL12XX_MAX_ROLES) {
+ WARN_ON(wlvif->role_id == WL12XX_INVALID_ROLE_ID);
+ wl12xx_roc(wl, wlvif, wlvif->role_id, wlvif->channel);
+ }
+ }
return 0;
}
--
1.7.6.401.g6a319
With the new connection flow, start_sta is called before
the remote rates where updated. Use our own supported rates
instead to make sure we don't disable any potential rate
(the rate policies will be updated later, but there is
currently no way to update the remote rates)
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/cmd.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
index 4c2ed25..ec7e8b3 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.c
+++ b/drivers/net/wireless/ti/wlcore/cmd.c
@@ -479,7 +479,12 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
}
cmd->sta.hlid = wlvif->sta.hlid;
cmd->sta.session = wl12xx_get_new_session_id(wl, wlvif);
- cmd->sta.remote_rates = cpu_to_le32(wlvif->rate_set);
+ /*
+ * We don't have the correct remote rates in this stage, and there
+ * is no way to update them later, so use our supported rates instead.
+ * The fw will take the configured rate policies into account anyway.
+ */
+ cmd->sta.remote_rates = cpu_to_le32(supported_rates);
wl1271_debug(DEBUG_CMD, "role start: roleid=%d, hlid=%d, session=%d "
"basic_rate_set: 0x%x, remote_rates: 0x%x",
--
1.7.6.401.g6a319
On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> The supported_rates field should contain all our supported
> rates, even if the remote peer doesn't support them.
>
> (use CONF_TX_AP_ENABLED_RATES for bg rates, as the possible
> rates are the same for ap and sta)
Maybe the macro should be renamed then?
> Signed-off-by: Eliad Peller <[email protected]>
> ---
> drivers/net/wireless/ti/wlcore/cmd.c | 10 +++++++++-
> 1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
> index 28a235f..4c2ed25 100644
> --- a/drivers/net/wireless/ti/wlcore/cmd.c
> +++ b/drivers/net/wireless/ti/wlcore/cmd.c
> @@ -441,6 +441,7 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
> {
> struct ieee80211_vif *vif = wl12xx_wlvif_to_vif(wlvif);
> struct wl12xx_cmd_role_start *cmd;
> + u32 supported_rates;
> int ret;
>
> cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
> @@ -461,7 +462,14 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
> cmd->sta.ssid_len = wlvif->ssid_len;
> memcpy(cmd->sta.ssid, wlvif->ssid, wlvif->ssid_len);
> memcpy(cmd->sta.bssid, vif->bss_conf.bssid, ETH_ALEN);
> - cmd->sta.local_rates = cpu_to_le32(wlvif->rate_set);
> +
> + supported_rates = CONF_TX_AP_ENABLED_RATES | CONF_TX_MCS_RATES |
> + wlcore_hw_sta_get_ap_rate_mask(wl, wlvif);
> + if (wlvif->p2p)
> + supported_rates &= ~CONF_TX_CCK_RATES;
Why not do this when creating the vif (init_vif_data)? I think this kind
of code doesn't belong in the cmd function. Seem better to adjust
wlvif->rate_set.
--
Luca.
When first configuring the rate policy, before auth,
we still don't have the correct rates that were
agreed during association.
Reconfigure the rate policy on association in order
to update them.
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 0dd0ec1..a1ad326 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -3771,7 +3771,8 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
wlvif->rssi_thold = bss_conf->cqm_rssi_thold;
}
- if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT)) {
+ if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT |
+ BSS_CHANGED_ASSOC)) {
rcu_read_lock();
sta = ieee80211_find_sta(vif, bss_conf->bssid);
if (!sta)
@@ -3958,6 +3959,17 @@ sta_not_found:
if (ret < 0)
goto out;
+ if (sta_rate_set) {
+ wlvif->rate_set =
+ wl1271_tx_enabled_rates_get(wl,
+ sta_rate_set,
+ wlvif->band);
+
+ ret = wl1271_acx_sta_rate_policies(wl, wlvif);
+ if (ret < 0)
+ goto out;
+ }
+
if (test_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags))
wl12xx_set_authorized(wl, wlvif);
}
--
1.7.6.401.g6a319
On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <[email protected]> wrote:
> Use the sta_state notifications to ROC when a station
> is about to connect, and CROC respectively on
> authorization (success) / deletion (failure).
>
> Change the wl12xx_update_sta_state() flow to bail out
> only on error, so multiple code blocks could refer
> to the same state.
>
> Signed-off-by: Eliad Peller <[email protected]>
[...]
>
> + /* clear ROCs on failure or authorization */
> + if (is_sta &&
> + (new_state == IEEE80211_STA_AUTHORIZED ||
> + new_state == IEEE80211_STA_NOTEXIST)) {
> + if (test_bit(wlvif->role_id, wl->roc_map))
> + wl12xx_croc(wl, wlvif->role_id);
> + }
> +
> + if (is_sta &&
> + old_state == IEEE80211_STA_NOTEXIST &&
> + new_state == IEEE80211_STA_NONE) {
> + if (find_first_bit(wl->roc_map,
> + WL12XX_MAX_ROLES) >= WL12XX_MAX_ROLES) {
> + WARN_ON(wlvif->role_id == WL12XX_INVALID_ROLE_ID);
> + wl12xx_roc(wl, wlvif, wlvif->role_id, wlvif->channel);
> + }
> + }
what about AP mode? we don't have an opportunistic ROC there as well
for connecting stations?
On Mon, 2012-11-19 at 18:39 +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_disconnect(), 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).
>
> Signed-off-by: Eliad Peller <[email protected]>
> ---
[...]
> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
> index 41ed1d5..41a8502 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -2468,8 +2468,7 @@ static int wl12xx_op_change_interface(struct ieee80211_hw *hw,
[...]
> -static int wl1271_unjoin(struct wl1271 *wl, struct wl12xx_vif *wlvif)
> +static void wlcore_disconnect(struct wl1271 *wl, struct wl12xx_vif *wlvif)
Why not wlcore_unset_assoc()? Or change the wlcore_set_assoc() to
wlcore_connect()?
[...]
> @@ -3921,15 +3850,63 @@ sta_not_found:
> wlvif->basic_rate =
> wl1271_tx_min_rate_get(wl,
> wlvif->basic_rate_set);
> +
> if (sta_rate_set)
> wlvif->rate_set =
> wl1271_tx_enabled_rates_get(wl,
> sta_rate_set,
> wlvif->band);
> +
> + /* we only supports sched_scan while not connected */
s/supports/support/
> + if (wl->sched_scanning) {
> + wl1271_scan_sched_scan_stop(wl, wlvif);
> + ieee80211_sched_scan_stopped(wl->hw);
> + }
> +
> ret = wl1271_acx_sta_rate_policies(wl, wlvif);
> if (ret < 0)
> goto out;
>
> + ret = wl12xx_cmd_build_null_data(wl, wlvif);
> + if (ret < 0)
> + goto out;
> +
> + ret = wl1271_build_qos_null_data(wl, vif);
> + if (ret < 0)
> + goto out;
> +
> + /* Need to update the BSSID (for filtering etc) */
> + set_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
> + do_join = true;
> + } else {
> + /* revert back to minimum rates for the current band */
> + wl1271_set_band_rate(wl, wlvif);
> + wlvif->basic_rate =
> + wl1271_tx_min_rate_get(wl,
> + wlvif->basic_rate_set);
> + ret = wl1271_acx_sta_rate_policies(wl, wlvif);
> + if (ret < 0)
> + goto out;
> +
> + if (!is_ibss &&
> + test_bit(WLVIF_FLAG_IN_USE, &wlvif->flags)) {
> + ret = wl12xx_cmd_role_stop_sta(wl, wlvif);
> + if (ret < 0)
> + goto out;
> + }
> + clear_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
> + }
> + }
It would be nice to keep all this stuff in a separate function instead
of bringing it back into this already-huge wl1271_bss_info_changed_sta()
function.
> + if ((changed & BSS_CHANGED_ASSOC)) {
Same thing for this if, but you already did a great job removing things
from here. ;)
> + if (bss_conf->assoc) {
> + int ieoffset;
> + wlvif->aid = bss_conf->aid;
> + wlvif->channel_type = bss_conf->channel_type;
> + wlvif->beacon_int = bss_conf->beacon_int;
> +
> + set_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags);
Wouldn't it be better to put this in the set_assoc() function?
--
Luca.
On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> When first configuring the rate policy, before auth,
> we still don't have the correct rates that were
> agreed during association.
>
> Reconfigure the rate policy on association in order
> to update them.
>
> Signed-off-by: Eliad Peller <[email protected]>
> ---
> drivers/net/wireless/ti/wlcore/main.c | 14 +++++++++++++-
> 1 files changed, 13 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
> index 0dd0ec1..a1ad326 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -3771,7 +3771,8 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
> wlvif->rssi_thold = bss_conf->cqm_rssi_thold;
> }
>
> - if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT)) {
> + if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT |
> + BSS_CHANGED_ASSOC)) {
> rcu_read_lock();
> sta = ieee80211_find_sta(vif, bss_conf->bssid);
> if (!sta)
> @@ -3958,6 +3959,17 @@ sta_not_found:
> if (ret < 0)
> goto out;
>
> + if (sta_rate_set) {
> + wlvif->rate_set =
> + wl1271_tx_enabled_rates_get(wl,
> + sta_rate_set,
> + wlvif->band);
> +
> + ret = wl1271_acx_sta_rate_policies(wl, wlvif);
> + if (ret < 0)
> + goto out;
> + }
> +
> if (test_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags))
> wl12xx_set_authorized(wl, wlvif);
> }
Yes, it's messy that we use the same value for vif-supported rates and
current association negotiated rates... :(
--
Luca.
We care only about the operational channel, not
about the temporal hw channel (which won't have
any real meaning in multi-channel env anyway)
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index ef299f9..4f4a9e5 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -3833,7 +3833,7 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
goto sta_not_found;
/* save the supp_rates of the ap */
- sta_rate_set = sta->supp_rates[wl->hw->conf.channel->band];
+ sta_rate_set = sta->supp_rates[wlvif->band];
if (sta->ht_cap.ht_supported)
sta_rate_set |=
(sta->ht_cap.mcs.rx_mask[0] << HW_HT_RATES_OFFSET) |
--
1.7.6.401.g6a319
On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> Add some basic chanctx implementation.
This patch will leave the driver in a broken state, and in fact probably
crashing trying to use hw.conf.channel.
johannes
Hi Eliad,
On Tue, Nov 20, 2012 at 3:39 AM, Eliad Peller <[email protected]> wrote:
> since we no longer use dev role in the connection flow,
> we should always use the hlid of the sta role.
I don't really know the hardware, but this seems like something
something that should be combined with the first patch.
> Signed-off-by: Eliad Peller <[email protected]>
> ---
> drivers/net/wireless/ti/wlcore/tx.c | 10 +---------
> 1 files changed, 1 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/tx.c b/drivers/net/wireless/ti/wlcore/tx.c
> index a90d3cd..a555a70 100644
> --- a/drivers/net/wireless/ti/wlcore/tx.c
> +++ b/drivers/net/wireless/ti/wlcore/tx.c
> @@ -155,21 +155,13 @@ static u8 wl12xx_tx_get_hlid_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif,
> u8 wl12xx_tx_get_hlid(struct wl1271 *wl, struct wl12xx_vif *wlvif,
> struct sk_buff *skb, struct ieee80211_sta *sta)
> {
> - struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> -
> if (!wlvif || wl12xx_is_dummy_packet(wl, skb))
> return wl->system_hlid;
>
> if (wlvif->bss_type == BSS_TYPE_AP_BSS)
> return wl12xx_tx_get_hlid_ap(wl, wlvif, skb, sta);
>
> - if ((test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags) ||
> - test_bit(WLVIF_FLAG_IBSS_JOINED, &wlvif->flags)) &&
> - !ieee80211_is_auth(hdr->frame_control) &&
> - !ieee80211_is_assoc_req(hdr->frame_control))
> - return wlvif->sta.hlid;
> - else
> - return wlvif->dev_hlid;
> + return wlvif->sta.hlid;
> }
>
> unsigned int wlcore_calc_packet_alignment(struct wl1271 *wl,
> --
> 1.7.6.401.g6a319
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Julian Calaby
Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
On Mon, Nov 19, 2012 at 11:13 PM, Arik Nemtsov <[email protected]> wrote:
> On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <[email protected]> wrote:
>> implement the reamin_on_channel() callback by starting
>> a dev role (already associated with the current vif)
>> on the requested channel/band.
>>
>> This channel is usually different from the channel
>> of the sta role, so pass it to wl12xx_roc() as well,
>> and notify mac80211 (async) when the fw is ready
>> on the new channel.
>>
>> Signed-off-by: Eliad Peller <[email protected]>
> [...]
>> @@ -1608,7 +1610,7 @@ static int wl12xx_cmd_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif,
>> }
>>
>> cmd->role_id = role_id;
>> - cmd->channel = wlvif->channel;
>> + cmd->channel = channel;
>> switch (wlvif->band) {
>> case IEEE80211_BAND_2GHZ:
>> cmd->band = WLCORE_BAND_2_4GHZ;
>
> This seems wrong... What happens if the roc is on another band from
> the one the vif is connected on?
>
> The dev role is started on the correct band, but the roc will be on
> the band of the wlvif..
>
good catch. i'll fix it.
>> +static int wlcore_op_remain_on_channel(struct ieee80211_hw *hw,
>> + struct ieee80211_vif *vif,
>> + struct ieee80211_channel *chan,
>> + enum nl80211_channel_type channel_type,
>> + int duration)
>> +{
>> + struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
>> + struct wl1271 *wl = hw->priv;
>> + int channel, ret = 0;
>> +
>> + channel = ieee80211_frequency_to_channel(chan->center_freq);
>> +
>> + wl1271_debug(DEBUG_MAC80211, "mac80211 roc %d (%d)",
>> + channel, wlvif->role_id);
>> +
>> + mutex_lock(&wl->mutex);
>> +
>> + if (unlikely(wl->state != WLCORE_STATE_ON))
>> + goto out;
>> +
>> + /* return EBUSY if we can't ROC right now */
>> + if (WARN_ON(wl->roc_vif ||
>> + find_first_bit(wl->roc_map,
>> + WL12XX_MAX_ROLES) < WL12XX_MAX_ROLES)) {
>> + ret = -EBUSY;
>> + goto out;
>> + }
>> +
>> + /*
>> + * make sure the vif has dev role.
>> + * TODO: enable dev role dynamically instead of pre-allocating it.
>> + */
>
> Not sure what you mean here. Seems pretty dynamic to me..
>
yeah... i referred to the lazy-enable of dev role, but forgot that we
already upstreamed it :)
so i'll just drop this check (as we can start dev role along with any
other role).
>> @@ -5245,6 +5368,8 @@ static int wl1271_init_ieee80211(struct wl1271 *wl)
>> wl->hw->wiphy->max_sched_scan_ie_len = WL1271_CMD_TEMPL_MAX_SIZE -
>> sizeof(struct ieee80211_header);
>>
>> + wl->hw->wiphy->max_remain_on_channel_duration = 5000;
>
> Isn't this the default?
>
only for drivers that don't implement .remain_on_channel (i.e.
implemented by mac80211).
Eliad.
On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> The default ps mode of the fw is auto, while the default
> ps mode of mac80211 is active (ps off).
> In order to sync them, configure active ps on association.
>
> Signed-off-by: Eliad Peller <[email protected]>
> ---
> drivers/net/wireless/ti/wlcore/main.c | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
> index e1f0606..0dd0ec1 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -2534,6 +2534,15 @@ static int wlcore_set_assoc(struct wl1271 *wl, struct wl12xx_vif *wlvif)
> ACX_KEEP_ALIVE_TPL_VALID);
> if (ret < 0)
> goto out;
> +
> + /*
> + * The default fw psm configuration is AUTO, while mac80211 default
> + * setting is off (ACTIVE), so sync the fw with the correct value.
> + */
> + ret = wl1271_ps_set_mode(wl, wlvif, STATION_ACTIVE_MODE);
> + if (ret < 0)
> + goto out;
> +
Does mac80211 enable PS soon after this by default?
--
Luca.
On Tue, Nov 20, 2012 at 10:18 AM, Luciano Coelho <[email protected]> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> With the new connection flow, start_sta is called before
>> the remote rates where updated. Use our own supported rates
>> instead to make sure we don't disable any potential rate
>> (the rate policies will be updated later, but there is
>> currently no way to update the remote rates)
>>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
>> drivers/net/wireless/ti/wlcore/cmd.c | 7 ++++++-
>> 1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
>> index 4c2ed25..ec7e8b3 100644
>> --- a/drivers/net/wireless/ti/wlcore/cmd.c
>> +++ b/drivers/net/wireless/ti/wlcore/cmd.c
>> @@ -479,7 +479,12 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>> }
>> cmd->sta.hlid = wlvif->sta.hlid;
>> cmd->sta.session = wl12xx_get_new_session_id(wl, wlvif);
>> - cmd->sta.remote_rates = cpu_to_le32(wlvif->rate_set);
>> + /*
>> + * We don't have the correct remote rates in this stage, and there
>> + * is no way to update them later, so use our supported rates instead.
>> + * The fw will take the configured rate policies into account anyway.
>> + */
>> + cmd->sta.remote_rates = cpu_to_le32(supported_rates);
>
> Why do we even have to pass this value, then? Any hidden reason that we
> may be overlooking?
>
the fw uses an intersection our local_supported_rates, the
remote_rates and the rate_policy in order to determine the rates.
i'm not too familiar with its whole logic, though.
Eliad.
implement the reamin_on_channel() callback by starting
a dev role (already associated with the current vif)
on the requested channel/band.
This channel is usually different from the channel
of the sta role, so pass it to wl12xx_roc() as well,
and notify mac80211 (async) when the fw is ready
on the new channel.
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/cmd.c | 39 +++-----
drivers/net/wireless/ti/wlcore/cmd.h | 6 +-
drivers/net/wireless/ti/wlcore/event.c | 7 ++
drivers/net/wireless/ti/wlcore/main.c | 164 +++++++++++++++++++++++++++----
drivers/net/wireless/ti/wlcore/wlcore.h | 3 +
5 files changed, 174 insertions(+), 45 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
index eaef3f4..28a235f 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.c
+++ b/drivers/net/wireless/ti/wlcore/cmd.c
@@ -345,7 +345,9 @@ static u8 wlcore_get_native_channel_type(u8 nl_channel_type)
}
static int wl12xx_cmd_role_start_dev(struct wl1271 *wl,
- struct wl12xx_vif *wlvif)
+ struct wl12xx_vif *wlvif,
+ enum ieee80211_band band,
+ int channel)
{
struct wl12xx_cmd_role_start *cmd;
int ret;
@@ -359,9 +361,9 @@ static int wl12xx_cmd_role_start_dev(struct wl1271 *wl,
wl1271_debug(DEBUG_CMD, "cmd role start dev %d", wlvif->dev_role_id);
cmd->role_id = wlvif->dev_role_id;
- if (wlvif->band == IEEE80211_BAND_5GHZ)
+ if (band == IEEE80211_BAND_5GHZ)
cmd->band = WLCORE_BAND_5GHZ;
- cmd->channel = wlvif->channel;
+ cmd->channel = channel;
if (wlvif->dev_hlid == WL12XX_INVALID_LINK_ID) {
ret = wl12xx_allocate_link(wl, wlvif, &wlvif->dev_hlid);
@@ -1591,12 +1593,12 @@ out:
}
static int wl12xx_cmd_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif,
- u8 role_id)
+ u8 role_id, u8 channel)
{
struct wl12xx_cmd_roc *cmd;
int ret = 0;
- wl1271_debug(DEBUG_CMD, "cmd roc %d (%d)", wlvif->channel, role_id);
+ wl1271_debug(DEBUG_CMD, "cmd roc %d (%d)", channel, role_id);
if (WARN_ON(role_id == WL12XX_INVALID_ROLE_ID))
return -EINVAL;
@@ -1608,7 +1610,7 @@ static int wl12xx_cmd_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif,
}
cmd->role_id = role_id;
- cmd->channel = wlvif->channel;
+ cmd->channel = channel;
switch (wlvif->band) {
case IEEE80211_BAND_2GHZ:
cmd->band = WLCORE_BAND_2_4GHZ;
@@ -1664,30 +1666,18 @@ out:
return ret;
}
-int wl12xx_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 role_id)
+int wl12xx_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 role_id,
+ u8 channel)
{
int ret = 0;
- bool is_first_roc;
if (WARN_ON(test_bit(role_id, wl->roc_map)))
return 0;
- is_first_roc = (find_first_bit(wl->roc_map, WL12XX_MAX_ROLES) >=
- WL12XX_MAX_ROLES);
-
- ret = wl12xx_cmd_roc(wl, wlvif, role_id);
+ ret = wl12xx_cmd_roc(wl, wlvif, role_id, channel);
if (ret < 0)
goto out;
- if (is_first_roc) {
- ret = wl1271_cmd_wait_for_event(wl,
- REMAIN_ON_CHANNEL_COMPLETE_EVENT_ID);
- if (ret < 0) {
- wl1271_error("cmd roc event completion error");
- goto out;
- }
- }
-
__set_bit(role_id, wl->roc_map);
out:
return ret;
@@ -1780,7 +1770,8 @@ out:
}
/* start dev role and roc on its channel */
-int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif,
+ enum ieee80211_band band, int channel)
{
int ret;
@@ -1795,11 +1786,11 @@ int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif)
if (ret < 0)
goto out;
- ret = wl12xx_cmd_role_start_dev(wl, wlvif);
+ ret = wl12xx_cmd_role_start_dev(wl, wlvif, band, channel);
if (ret < 0)
goto out_disable;
- ret = wl12xx_roc(wl, wlvif, wlvif->dev_role_id);
+ ret = wl12xx_roc(wl, wlvif, wlvif->dev_role_id, channel);
if (ret < 0)
goto out_stop;
diff --git a/drivers/net/wireless/ti/wlcore/cmd.h b/drivers/net/wireless/ti/wlcore/cmd.h
index 2409f3d..2b63759 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.h
+++ b/drivers/net/wireless/ti/wlcore/cmd.h
@@ -39,7 +39,8 @@ int wl12xx_cmd_role_stop_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_start_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_stop_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl12xx_cmd_role_start_ibss(struct wl1271 *wl, struct wl12xx_vif *wlvif);
-int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif);
+int wl12xx_start_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif,
+ enum ieee80211_band band, int channel);
int wl12xx_stop_dev(struct wl1271 *wl, struct wl12xx_vif *wlvif);
int wl1271_cmd_test(struct wl1271 *wl, void *buf, size_t buf_len, u8 answer);
int wl1271_cmd_interrogate(struct wl1271 *wl, u16 id, void *buf, size_t len);
@@ -76,7 +77,8 @@ int wl1271_cmd_set_ap_key(struct wl1271 *wl, struct wl12xx_vif *wlvif,
u8 key_size, const u8 *key, u8 hlid, u32 tx_seq_32,
u16 tx_seq_16);
int wl12xx_cmd_set_peer_state(struct wl1271 *wl, u8 hlid);
-int wl12xx_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 role_id);
+int wl12xx_roc(struct wl1271 *wl, struct wl12xx_vif *wlvif, u8 role_id,
+ u8 channel);
int wl12xx_croc(struct wl1271 *wl, u8 role_id);
int wl12xx_cmd_add_peer(struct wl1271 *wl, struct wl12xx_vif *wlvif,
struct ieee80211_sta *sta, u8 hlid);
diff --git a/drivers/net/wireless/ti/wlcore/event.c b/drivers/net/wireless/ti/wlcore/event.c
index 4890705..cf45aaf 100644
--- a/drivers/net/wireless/ti/wlcore/event.c
+++ b/drivers/net/wireless/ti/wlcore/event.c
@@ -250,6 +250,13 @@ static int wl1271_event_process(struct wl1271 *wl)
disconnect_sta = true;
}
+ if (vector & REMAIN_ON_CHANNEL_COMPLETE_EVENT_ID) {
+ wl1271_debug(DEBUG_EVENT,
+ "REMAIN_ON_CHANNEL_COMPLETE_EVENT_ID");
+ if (wl->roc_vif)
+ ieee80211_ready_on_channel(wl->hw);
+ }
+
if (disconnect_sta) {
u32 num_packets = wl->conf.tx.max_tx_retries;
struct ieee80211_sta *sta;
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index f4017f3..ef299f9 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2610,24 +2610,6 @@ static int wl12xx_config_vif(struct wl1271 *wl, struct wl12xx_vif *wlvif,
if (ret < 0)
wl1271_warning("rate policy for channel "
"failed %d", ret);
-
- /*
- * change the ROC channel. do it only if we are
- * not idle. otherwise, CROC will be called
- * anyway.
- */
- if (!test_bit(WLVIF_FLAG_STA_ASSOCIATED,
- &wlvif->flags) &&
- wl12xx_dev_role_started(wlvif) &&
- !(conf->flags & IEEE80211_CONF_IDLE)) {
- ret = wl12xx_stop_dev(wl, wlvif);
- if (ret < 0)
- return ret;
-
- ret = wl12xx_start_dev(wl, wlvif);
- if (ret < 0)
- return ret;
- }
}
}
@@ -4027,7 +4009,8 @@ sta_not_found:
/* ROC until connected (after EAPOL exchange) */
if (!is_ibss) {
- ret = wl12xx_roc(wl, wlvif, wlvif->role_id);
+ ret = wl12xx_roc(wl, wlvif, wlvif->role_id,
+ wlvif->channel);
if (ret < 0)
goto out;
}
@@ -4658,6 +4641,144 @@ static void wlcore_op_flush(struct ieee80211_hw *hw, bool drop)
wl1271_tx_flush(wl);
}
+static int wlcore_op_remain_on_channel(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct ieee80211_channel *chan,
+ enum nl80211_channel_type channel_type,
+ int duration)
+{
+ struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
+ struct wl1271 *wl = hw->priv;
+ int channel, ret = 0;
+
+ channel = ieee80211_frequency_to_channel(chan->center_freq);
+
+ wl1271_debug(DEBUG_MAC80211, "mac80211 roc %d (%d)",
+ channel, wlvif->role_id);
+
+ mutex_lock(&wl->mutex);
+
+ if (unlikely(wl->state != WLCORE_STATE_ON))
+ goto out;
+
+ /* return EBUSY if we can't ROC right now */
+ if (WARN_ON(wl->roc_vif ||
+ find_first_bit(wl->roc_map,
+ WL12XX_MAX_ROLES) < WL12XX_MAX_ROLES)) {
+ ret = -EBUSY;
+ goto out;
+ }
+
+ /*
+ * make sure the vif has dev role.
+ * TODO: enable dev role dynamically instead of pre-allocating it.
+ */
+ if (WARN_ON_ONCE(!(wlvif->bss_type == BSS_TYPE_STA_BSS ||
+ wlvif->bss_type == BSS_TYPE_IBSS))) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = wl1271_ps_elp_wakeup(wl);
+ if (ret < 0)
+ goto out;
+
+ ret = wl12xx_start_dev(wl, wlvif, chan->band, channel);
+ if (ret < 0)
+ goto out_sleep;
+
+ wl->roc_vif = vif;
+ ieee80211_queue_delayed_work(hw, &wl->roc_complete_work,
+ msecs_to_jiffies(duration));
+out_sleep:
+ wl1271_ps_elp_sleep(wl);
+out:
+ mutex_unlock(&wl->mutex);
+ return ret;
+}
+
+static int __wlcore_roc_completed(struct wl1271 *wl)
+{
+ struct wl12xx_vif *wlvif;
+ int ret;
+
+ /* already completed */
+ if (unlikely(!wl->roc_vif))
+ return 0;
+
+ wlvif = wl12xx_vif_to_data(wl->roc_vif);
+
+ if (!test_bit(WLVIF_FLAG_INITIALIZED, &wlvif->flags))
+ return -EBUSY;
+
+ ret = wl12xx_stop_dev(wl, wlvif);
+ if (ret < 0)
+ return ret;
+
+ wl->roc_vif = NULL;
+
+ return 0;
+}
+
+static int wlcore_roc_completed(struct wl1271 *wl)
+{
+ int ret;
+
+ wl1271_debug(DEBUG_MAC80211, "roc complete");
+
+ mutex_lock(&wl->mutex);
+
+ if (unlikely(wl->state != WLCORE_STATE_ON)) {
+ ret = -EBUSY;
+ goto out;
+ }
+
+ ret = wl1271_ps_elp_wakeup(wl);
+ if (ret < 0)
+ goto out;
+
+ ret = __wlcore_roc_completed(wl);
+
+ wl1271_ps_elp_sleep(wl);
+out:
+ mutex_unlock(&wl->mutex);
+
+ return ret;
+}
+
+static void wlcore_roc_complete_work(struct work_struct *work)
+{
+ struct delayed_work *dwork;
+ struct wl1271 *wl;
+ int ret;
+
+ dwork = container_of(work, struct delayed_work, work);
+ wl = container_of(dwork, struct wl1271, roc_complete_work);
+
+ ret = wlcore_roc_completed(wl);
+ if (!ret)
+ ieee80211_remain_on_channel_expired(wl->hw);
+}
+
+static int wlcore_op_cancel_remain_on_channel(struct ieee80211_hw *hw)
+{
+ struct wl1271 *wl = hw->priv;
+
+ wl1271_debug(DEBUG_MAC80211, "mac80211 croc");
+
+ /* TODO: per-vif */
+ wl1271_tx_flush(wl);
+
+ /*
+ * we can't just flush_work here, because it might deadlock
+ * (as we might get called from the same workqueue)
+ */
+ cancel_delayed_work_sync(&wl->roc_complete_work);
+ wlcore_roc_completed(wl);
+
+ return 0;
+}
+
static bool wl1271_tx_frames_pending(struct ieee80211_hw *hw)
{
struct wl1271 *wl = hw->priv;
@@ -4849,6 +4970,8 @@ static const struct ieee80211_ops wl1271_ops = {
.set_bitrate_mask = wl12xx_set_bitrate_mask,
.channel_switch = wl12xx_op_channel_switch,
.flush = wlcore_op_flush,
+ .remain_on_channel = wlcore_op_remain_on_channel,
+ .cancel_remain_on_channel = wlcore_op_cancel_remain_on_channel,
CFG80211_TESTMODE_CMD(wl1271_tm_cmd)
};
@@ -5245,6 +5368,8 @@ static int wl1271_init_ieee80211(struct wl1271 *wl)
wl->hw->wiphy->max_sched_scan_ie_len = WL1271_CMD_TEMPL_MAX_SIZE -
sizeof(struct ieee80211_header);
+ wl->hw->wiphy->max_remain_on_channel_duration = 5000;
+
wl->hw->wiphy->flags |= WIPHY_FLAG_AP_UAPSD |
WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL;
@@ -5343,6 +5468,7 @@ struct ieee80211_hw *wlcore_alloc_hw(size_t priv_size, u32 aggr_buf_size)
INIT_WORK(&wl->tx_work, wl1271_tx_work);
INIT_WORK(&wl->recovery_work, wl1271_recovery_work);
INIT_DELAYED_WORK(&wl->scan_complete_work, wl1271_scan_complete_work);
+ INIT_DELAYED_WORK(&wl->roc_complete_work, wlcore_roc_complete_work);
INIT_DELAYED_WORK(&wl->tx_watchdog_work, wl12xx_tx_watchdog_work);
INIT_DELAYED_WORK(&wl->connection_loss_work,
wl1271_connection_loss_work);
diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h b/drivers/net/wireless/ti/wlcore/wlcore.h
index 1030b6b..b636f1d 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore.h
@@ -286,6 +286,9 @@ struct wl1271 {
/* Connection loss work */
struct delayed_work connection_loss_work;
+ struct ieee80211_vif *roc_vif;
+ struct delayed_work roc_complete_work;
+
bool sched_scanning;
/* The current band */
--
1.7.6.401.g6a319
On Mon, Nov 19, 2012 at 11:23 PM, Arik Nemtsov <[email protected]> wrote:
> On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <[email protected]> wrote:
>> Add some basic chanctx implementation.
>>
>> Only add debug prints, and save the vif's channel/band/type.
>>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
>> +static int wlcore_op_assign_vif_chanctx(struct ieee80211_hw *hw,
>> + struct ieee80211_vif *vif,
>> + struct ieee80211_chanctx_conf *ctx)
>> +{
>> + struct wl1271 *wl = hw->priv;
>> + struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
>> + int channel = ieee80211_frequency_to_channel(
>> + ctx->channel->center_freq);
>> +
>> + wl1271_debug(DEBUG_MAC80211,
>> + "mac80211 assign chanctx (role %d) %d (type %d)",
>> + wlvif->role_id, channel, ctx->channel_type);
>> +
>> + wlcore_tx_work_locked(wl);
>
> is this some kind of a lame attempt at a flush? :)
>
> why not use flush here if the channel was set - or better yet, flush
> stuff at wlcore_op_unassign_vif_chanctx, which is currently unused.
> we'll need the TODO there for the per-vif-flush. once everything is
> upstreamed (hw queues) it would be pretty trivial to implement.
>
ok, you convinced me. i'll do a flush on unassign instead. :)
> also not sure what happened to the rest of the stuff done on channel
> config - updating the rate_set according to band etc..
>
well, actually you have a pending patch to fix it, and i planned
sending it in the next patchset. but since you already asked about it
i'll just squash it into this patch (and add your SOB). :)
Eliad.
On Mon, Nov 19, 2012 at 7:03 PM, Johannes Berg
<[email protected]> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> Add some basic chanctx implementation.
>
> This patch will leave the driver in a broken state, and in fact probably
> crashing trying to use hw.conf.channel.
>
you are right. i'm taking care of these in the following patches.
i preferred splitting the patches for clarity instead of squashing
them all into one big patch (removing hw.conf.channel references
before implementing chanctx will break the driver as well).
i guess i can squash them in this case, but i'm not sure it will
really help, as other intermediate patches in this patchset might
still break some functionality of the driver.
i don't mind either way. whatever Luca will prefer :)
(maybe keeping it that way for review, and only squashing them on apply?)
Eliad.
On Mon, Nov 19, 2012 at 11:41 PM, Arik Nemtsov <[email protected]> wrote:
> On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <[email protected]> wrote:
>> Use the sta_state notifications to ROC when a station
>> is about to connect, and CROC respectively on
>> authorization (success) / deletion (failure).
>>
>> Change the wl12xx_update_sta_state() flow to bail out
>> only on error, so multiple code blocks could refer
>> to the same state.
>>
>> Signed-off-by: Eliad Peller <[email protected]>
> [...]
>>
>> + /* clear ROCs on failure or authorization */
>> + if (is_sta &&
>> + (new_state == IEEE80211_STA_AUTHORIZED ||
>> + new_state == IEEE80211_STA_NOTEXIST)) {
>> + if (test_bit(wlvif->role_id, wl->roc_map))
>> + wl12xx_croc(wl, wlvif->role_id);
>> + }
>> +
>> + if (is_sta &&
>> + old_state == IEEE80211_STA_NOTEXIST &&
>> + new_state == IEEE80211_STA_NONE) {
>> + if (find_first_bit(wl->roc_map,
>> + WL12XX_MAX_ROLES) >= WL12XX_MAX_ROLES) {
>> + WARN_ON(wlvif->role_id == WL12XX_INVALID_ROLE_ID);
>> + wl12xx_roc(wl, wlvif, wlvif->role_id, wlvif->channel);
>> + }
>> + }
>
> what about AP mode? we don't have an opportunistic ROC there as well
> for connecting stations?
yeah, we do. but since in order to really be effective it requires
supplicant changes that weren't upstreamed yet (adding non-associated
station), i preferred postponing it to another patchset.
on the other hand, this might help protecting the EAPOL exchange even
in its current form, so i guess we can upstream it even before the
supplicant changes.
Eliad.
On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> With the new connection flow, start_sta is called before
> the remote rates where updated. Use our own supported rates
> instead to make sure we don't disable any potential rate
> (the rate policies will be updated later, but there is
> currently no way to update the remote rates)
>
> Signed-off-by: Eliad Peller <[email protected]>
> ---
> drivers/net/wireless/ti/wlcore/cmd.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
> index 4c2ed25..ec7e8b3 100644
> --- a/drivers/net/wireless/ti/wlcore/cmd.c
> +++ b/drivers/net/wireless/ti/wlcore/cmd.c
> @@ -479,7 +479,12 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
> }
> cmd->sta.hlid = wlvif->sta.hlid;
> cmd->sta.session = wl12xx_get_new_session_id(wl, wlvif);
> - cmd->sta.remote_rates = cpu_to_le32(wlvif->rate_set);
> + /*
> + * We don't have the correct remote rates in this stage, and there
> + * is no way to update them later, so use our supported rates instead.
> + * The fw will take the configured rate policies into account anyway.
> + */
> + cmd->sta.remote_rates = cpu_to_le32(supported_rates);
Why do we even have to pass this value, then? Any hidden reason that we
may be overlooking?
--
Luca.
The supported_rates field should contain all our supported
rates, even if the remote peer doesn't support them.
(use CONF_TX_AP_ENABLED_RATES for bg rates, as the possible
rates are the same for ap and sta)
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/cmd.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
index 28a235f..4c2ed25 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.c
+++ b/drivers/net/wireless/ti/wlcore/cmd.c
@@ -441,6 +441,7 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
{
struct ieee80211_vif *vif = wl12xx_wlvif_to_vif(wlvif);
struct wl12xx_cmd_role_start *cmd;
+ u32 supported_rates;
int ret;
cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
@@ -461,7 +462,14 @@ int wl12xx_cmd_role_start_sta(struct wl1271 *wl, struct wl12xx_vif *wlvif)
cmd->sta.ssid_len = wlvif->ssid_len;
memcpy(cmd->sta.ssid, wlvif->ssid, wlvif->ssid_len);
memcpy(cmd->sta.bssid, vif->bss_conf.bssid, ETH_ALEN);
- cmd->sta.local_rates = cpu_to_le32(wlvif->rate_set);
+
+ supported_rates = CONF_TX_AP_ENABLED_RATES | CONF_TX_MCS_RATES |
+ wlcore_hw_sta_get_ap_rate_mask(wl, wlvif);
+ if (wlvif->p2p)
+ supported_rates &= ~CONF_TX_CCK_RATES;
+
+ cmd->sta.local_rates = cpu_to_le32(supported_rates);
+
cmd->channel_type = wlcore_get_native_channel_type(wlvif->channel_type);
if (wlvif->sta.hlid == WL12XX_INVALID_LINK_ID) {
--
1.7.6.401.g6a319
for some reason, the wl12xx fw is not able to rx/tx
on the first start_sta cmd.
Workaround it by issuing a dummy start_sta + stop_sta
before starting the sta for the final time.
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wl12xx/main.c | 9 ++++++---
drivers/net/wireless/ti/wlcore/main.c | 15 ++++++++++++++-
drivers/net/wireless/ti/wlcore/wlcore.h | 3 +++
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ti/wl12xx/main.c b/drivers/net/wireless/ti/wl12xx/main.c
index dadf1db..6c9fba4 100644
--- a/drivers/net/wireless/ti/wl12xx/main.c
+++ b/drivers/net/wireless/ti/wl12xx/main.c
@@ -637,7 +637,8 @@ static int wl12xx_identify_chip(struct wl1271 *wl)
wl->quirks |= WLCORE_QUIRK_LEGACY_NVS |
WLCORE_QUIRK_DUAL_PROBE_TMPL |
- WLCORE_QUIRK_TKIP_HEADER_SPACE;
+ WLCORE_QUIRK_TKIP_HEADER_SPACE |
+ WLCORE_QUIRK_START_STA_FAILS;
wl->sr_fw_name = WL127X_FW_NAME_SINGLE;
wl->mr_fw_name = WL127X_FW_NAME_MULTI;
memcpy(&wl->conf.mem, &wl12xx_default_priv_conf.mem_wl127x,
@@ -657,7 +658,8 @@ static int wl12xx_identify_chip(struct wl1271 *wl)
wl->quirks |= WLCORE_QUIRK_LEGACY_NVS |
WLCORE_QUIRK_DUAL_PROBE_TMPL |
- WLCORE_QUIRK_TKIP_HEADER_SPACE;
+ WLCORE_QUIRK_TKIP_HEADER_SPACE |
+ WLCORE_QUIRK_START_STA_FAILS;
wl->plt_fw_name = WL127X_PLT_FW_NAME;
wl->sr_fw_name = WL127X_FW_NAME_SINGLE;
wl->mr_fw_name = WL127X_FW_NAME_MULTI;
@@ -682,7 +684,8 @@ static int wl12xx_identify_chip(struct wl1271 *wl)
/* wl128x requires TX blocksize alignment */
wl->quirks |= WLCORE_QUIRK_TX_BLOCKSIZE_ALIGN |
WLCORE_QUIRK_DUAL_PROBE_TMPL |
- WLCORE_QUIRK_TKIP_HEADER_SPACE;
+ WLCORE_QUIRK_TKIP_HEADER_SPACE |
+ WLCORE_QUIRK_START_STA_FAILS;
wlcore_set_min_fw_ver(wl, WL128X_CHIP_VER, WL128X_IFTYPE_VER,
WL128X_MAJOR_VER, WL128X_SUBTYPE_VER,
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 2cb978e..f4017f3 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2490,8 +2490,21 @@ static int wlcore_join(struct wl1271 *wl, struct wl12xx_vif *wlvif)
if (is_ibss)
ret = wl12xx_cmd_role_start_ibss(wl, wlvif);
- else
+ else {
+ if (wl->quirks & WLCORE_QUIRK_START_STA_FAILS) {
+ /*
+ * TODO: this is an ugly workaround for wl12xx fw
+ * bug - we are not able to tx/rx after the first
+ * start_sta, so make dummy start+stop calls,
+ * and then call start_sta again.
+ * this should be fixed in the fw.
+ */
+ wl12xx_cmd_role_start_sta(wl, wlvif);
+ wl12xx_cmd_role_stop_sta(wl, wlvif);
+ }
+
ret = wl12xx_cmd_role_start_sta(wl, wlvif);
+ }
return ret;
}
diff --git a/drivers/net/wireless/ti/wlcore/wlcore.h b/drivers/net/wireless/ti/wlcore/wlcore.h
index 68584aa..1030b6b 100644
--- a/drivers/net/wireless/ti/wlcore/wlcore.h
+++ b/drivers/net/wireless/ti/wlcore/wlcore.h
@@ -450,6 +450,9 @@ wlcore_set_min_fw_ver(struct wl1271 *wl, unsigned int chip,
/* Each RX/TX transaction requires an end-of-transaction transfer */
#define WLCORE_QUIRK_END_OF_TRANSACTION BIT(0)
+/* the first start_role(sta) sometimes doesn't work on wl12xx */
+#define WLCORE_QUIRK_START_STA_FAILS BIT(1)
+
/* wl127x and SPI don't support SDIO block size alignment */
#define WLCORE_QUIRK_TX_BLOCKSIZE_ALIGN BIT(2)
--
1.7.6.401.g6a319
In case of offchannel tx, we should use the dev role hlid
instead of the sta hlid.
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/tx.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/tx.c b/drivers/net/wireless/ti/wlcore/tx.c
index a555a70..cf6dbee 100644
--- a/drivers/net/wireless/ti/wlcore/tx.c
+++ b/drivers/net/wireless/ti/wlcore/tx.c
@@ -155,12 +155,20 @@ static u8 wl12xx_tx_get_hlid_ap(struct wl1271 *wl, struct wl12xx_vif *wlvif,
u8 wl12xx_tx_get_hlid(struct wl1271 *wl, struct wl12xx_vif *wlvif,
struct sk_buff *skb, struct ieee80211_sta *sta)
{
+ struct ieee80211_tx_info *control;
+
if (!wlvif || wl12xx_is_dummy_packet(wl, skb))
return wl->system_hlid;
if (wlvif->bss_type == BSS_TYPE_AP_BSS)
return wl12xx_tx_get_hlid_ap(wl, wlvif, skb, sta);
+ control = IEEE80211_SKB_CB(skb);
+ if (control->flags & IEEE80211_TX_CTL_TX_OFFCHAN) {
+ wl1271_debug(DEBUG_TX, "tx offchannel");
+ return wlvif->dev_hlid;
+ }
+
return wlvif->sta.hlid;
}
--
1.7.6.401.g6a319
Add some basic chanctx implementation.
Only add debug prints, and save the vif's channel/band/type.
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 66 +++++++++++++++++++++++++++++++++
1 files changed, 66 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 4f4a9e5..caafd8c 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -4150,6 +4150,67 @@ out:
mutex_unlock(&wl->mutex);
}
+static int wlcore_op_add_chanctx(struct ieee80211_hw *hw,
+ struct ieee80211_chanctx_conf *ctx)
+{
+ wl1271_debug(DEBUG_MAC80211, "mac80211 add chanctx %d (type %d)",
+ ieee80211_frequency_to_channel(ctx->channel->center_freq),
+ ctx->channel_type);
+ return 0;
+}
+
+static void wlcore_op_remove_chanctx(struct ieee80211_hw *hw,
+ struct ieee80211_chanctx_conf *ctx)
+{
+ wl1271_debug(DEBUG_MAC80211, "mac80211 remove chanctx %d (type %d)",
+ ieee80211_frequency_to_channel(ctx->channel->center_freq),
+ ctx->channel_type);
+}
+
+static void wlcore_op_change_chanctx(struct ieee80211_hw *hw,
+ struct ieee80211_chanctx_conf *ctx,
+ u32 changed)
+{
+ wl1271_debug(DEBUG_MAC80211,
+ "mac80211 change chanctx %d (type %d) changed 0x%x",
+ ieee80211_frequency_to_channel(ctx->channel->center_freq),
+ ctx->channel_type, changed);
+}
+
+static int wlcore_op_assign_vif_chanctx(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct ieee80211_chanctx_conf *ctx)
+{
+ struct wl1271 *wl = hw->priv;
+ struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
+ int channel = ieee80211_frequency_to_channel(
+ ctx->channel->center_freq);
+
+ wl1271_debug(DEBUG_MAC80211,
+ "mac80211 assign chanctx (role %d) %d (type %d)",
+ wlvif->role_id, channel, ctx->channel_type);
+
+ wlcore_tx_work_locked(wl);
+ wlvif->band = ctx->channel->band;
+ wlvif->channel = channel;
+ wlvif->channel_type = ctx->channel_type;
+
+ return 0;
+}
+
+static void wlcore_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct ieee80211_chanctx_conf *ctx)
+{
+ struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
+
+ wl1271_debug(DEBUG_MAC80211,
+ "mac80211 unassign chanctx (role %d) %d (type %d)",
+ wlvif->role_id,
+ ieee80211_frequency_to_channel(ctx->channel->center_freq),
+ ctx->channel_type);
+}
+
static int wl1271_op_conf_tx(struct ieee80211_hw *hw,
struct ieee80211_vif *vif, u16 queue,
const struct ieee80211_tx_queue_params *params)
@@ -4972,6 +5033,11 @@ static const struct ieee80211_ops wl1271_ops = {
.flush = wlcore_op_flush,
.remain_on_channel = wlcore_op_remain_on_channel,
.cancel_remain_on_channel = wlcore_op_cancel_remain_on_channel,
+ .add_chanctx = wlcore_op_add_chanctx,
+ .remove_chanctx = wlcore_op_remove_chanctx,
+ .change_chanctx = wlcore_op_change_chanctx,
+ .assign_vif_chanctx = wlcore_op_assign_vif_chanctx,
+ .unassign_vif_chanctx = wlcore_op_unassign_vif_chanctx,
CFG80211_TESTMODE_CMD(wl1271_tm_cmd)
};
--
1.7.6.401.g6a319
On Mon, 2012-11-19 at 19:16 +0200, Eliad Peller wrote:
> On Mon, Nov 19, 2012 at 7:03 PM, Johannes Berg
> <[email protected]> wrote:
> > On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
> >> Add some basic chanctx implementation.
> >
> > This patch will leave the driver in a broken state, and in fact probably
> > crashing trying to use hw.conf.channel.
> >
> you are right. i'm taking care of these in the following patches.
> i preferred splitting the patches for clarity instead of squashing
> them all into one big patch (removing hw.conf.channel references
> before implementing chanctx will break the driver as well).
> i guess i can squash them in this case, but i'm not sure it will
> really help, as other intermediate patches in this patchset might
> still break some functionality of the driver.
>
> i don't mind either way. whatever Luca will prefer :)
> (maybe keeping it that way for review, and only squashing them on apply?)
I prefer if we can do it so that the driver is not broken, to make it
easier to bisect. But I also agree that for review, it's better to have
split up patches.
So, your suggestion sounds good to me. Maybe you could send a final
squashed patchset when they're reviewed and ready to be applied?
--
Luca.
On Mon, Nov 19, 2012 at 6:39 PM, Eliad Peller <[email protected]> wrote:
> Add some basic chanctx implementation.
>
> Only add debug prints, and save the vif's channel/band/type.
>
> Signed-off-by: Eliad Peller <[email protected]>
> ---
> drivers/net/wireless/ti/wlcore/main.c | 66 +++++++++++++++++++++++++++++++++
> 1 files changed, 66 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
> index 4f4a9e5..caafd8c 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -4150,6 +4150,67 @@ out:
> mutex_unlock(&wl->mutex);
> }
>
> +static int wlcore_op_add_chanctx(struct ieee80211_hw *hw,
> + struct ieee80211_chanctx_conf *ctx)
> +{
> + wl1271_debug(DEBUG_MAC80211, "mac80211 add chanctx %d (type %d)",
> + ieee80211_frequency_to_channel(ctx->channel->center_freq),
> + ctx->channel_type);
> + return 0;
> +}
> +
> +static void wlcore_op_remove_chanctx(struct ieee80211_hw *hw,
> + struct ieee80211_chanctx_conf *ctx)
> +{
> + wl1271_debug(DEBUG_MAC80211, "mac80211 remove chanctx %d (type %d)",
> + ieee80211_frequency_to_channel(ctx->channel->center_freq),
> + ctx->channel_type);
> +}
> +
> +static void wlcore_op_change_chanctx(struct ieee80211_hw *hw,
> + struct ieee80211_chanctx_conf *ctx,
> + u32 changed)
> +{
> + wl1271_debug(DEBUG_MAC80211,
> + "mac80211 change chanctx %d (type %d) changed 0x%x",
> + ieee80211_frequency_to_channel(ctx->channel->center_freq),
> + ctx->channel_type, changed);
> +}
> +
> +static int wlcore_op_assign_vif_chanctx(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif,
> + struct ieee80211_chanctx_conf *ctx)
> +{
> + struct wl1271 *wl = hw->priv;
> + struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
> + int channel = ieee80211_frequency_to_channel(
> + ctx->channel->center_freq);
> +
> + wl1271_debug(DEBUG_MAC80211,
> + "mac80211 assign chanctx (role %d) %d (type %d)",
> + wlvif->role_id, channel, ctx->channel_type);
> +
> + wlcore_tx_work_locked(wl);
is this some kind of a lame attempt at a flush? :)
why not use flush here if the channel was set - or better yet, flush
stuff at wlcore_op_unassign_vif_chanctx, which is currently unused.
we'll need the TODO there for the per-vif-flush. once everything is
upstreamed (hw queues) it would be pretty trivial to implement.
also not sure what happened to the rest of the stuff done on channel
config - updating the rate_set according to band etc..
> + wlvif->band = ctx->channel->band;
> + wlvif->channel = channel;
> + wlvif->channel_type = ctx->channel_type;
> +
> + return 0;
> +}
> +
> +static void wlcore_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
> + struct ieee80211_vif *vif,
> + struct ieee80211_chanctx_conf *ctx)
> +{
> + struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif);
> +
> + wl1271_debug(DEBUG_MAC80211,
> + "mac80211 unassign chanctx (role %d) %d (type %d)",
> + wlvif->role_id,
> + ieee80211_frequency_to_channel(ctx->channel->center_freq),
> + ctx->channel_type);
> +}
> +
> static int wl1271_op_conf_tx(struct ieee80211_hw *hw,
> struct ieee80211_vif *vif, u16 queue,
> const struct ieee80211_tx_queue_params *params)
> @@ -4972,6 +5033,11 @@ static const struct ieee80211_ops wl1271_ops = {
> .flush = wlcore_op_flush,
> .remain_on_channel = wlcore_op_remain_on_channel,
> .cancel_remain_on_channel = wlcore_op_cancel_remain_on_channel,
> + .add_chanctx = wlcore_op_add_chanctx,
> + .remove_chanctx = wlcore_op_remove_chanctx,
> + .change_chanctx = wlcore_op_change_chanctx,
> + .assign_vif_chanctx = wlcore_op_assign_vif_chanctx,
> + .unassign_vif_chanctx = wlcore_op_unassign_vif_chanctx,
> CFG80211_TESTMODE_CMD(wl1271_tm_cmd)
> };
>
> --
> 1.7.6.401.g6a319
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 19, 2012 at 7:36 PM, Luciano Coelho <[email protected]> wrote:
> On Mon, 2012-11-19 at 19:16 +0200, Eliad Peller wrote:
>> On Mon, Nov 19, 2012 at 7:03 PM, Johannes Berg
>> <[email protected]> wrote:
>> > On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> >> Add some basic chanctx implementation.
>> >
>> > This patch will leave the driver in a broken state, and in fact probably
>> > crashing trying to use hw.conf.channel.
>> >
>> you are right. i'm taking care of these in the following patches.
>> i preferred splitting the patches for clarity instead of squashing
>> them all into one big patch (removing hw.conf.channel references
>> before implementing chanctx will break the driver as well).
>> i guess i can squash them in this case, but i'm not sure it will
>> really help, as other intermediate patches in this patchset might
>> still break some functionality of the driver.
>>
>> i don't mind either way. whatever Luca will prefer :)
>> (maybe keeping it that way for review, and only squashing them on apply?)
>
> I prefer if we can do it so that the driver is not broken, to make it
> easier to bisect. But I also agree that for review, it's better to have
> split up patches.
>
> So, your suggestion sounds good to me. Maybe you could send a final
> squashed patchset when they're reviewed and ready to be applied?
>
sure.
Eliad.
On Tue, Nov 20, 2012 at 10:13 AM, Luciano Coelho <[email protected]> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> When first configuring the rate policy, before auth,
>> we still don't have the correct rates that were
>> agreed during association.
>>
>> Reconfigure the rate policy on association in order
>> to update them.
>>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
>> drivers/net/wireless/ti/wlcore/main.c | 14 +++++++++++++-
>> 1 files changed, 13 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
>> index 0dd0ec1..a1ad326 100644
>> --- a/drivers/net/wireless/ti/wlcore/main.c
>> +++ b/drivers/net/wireless/ti/wlcore/main.c
>> @@ -3771,7 +3771,8 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
>> wlvif->rssi_thold = bss_conf->cqm_rssi_thold;
>> }
>>
>> - if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT)) {
>> + if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT |
>> + BSS_CHANGED_ASSOC)) {
>> rcu_read_lock();
>> sta = ieee80211_find_sta(vif, bss_conf->bssid);
>> if (!sta)
>> @@ -3958,6 +3959,17 @@ sta_not_found:
>> if (ret < 0)
>> goto out;
>>
>> + if (sta_rate_set) {
>> + wlvif->rate_set =
>> + wl1271_tx_enabled_rates_get(wl,
>> + sta_rate_set,
>> + wlvif->band);
>> +
>> + ret = wl1271_acx_sta_rate_policies(wl, wlvif);
>> + if (ret < 0)
>> + goto out;
>> + }
>> +
>> if (test_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags))
>> wl12xx_set_authorized(wl, wlvif);
>> }
>
> Yes, it's messy that we use the same value for vif-supported rates and
> current association negotiated rates... :(
>
yeah, we have a total mess with all the configured/supported rates.
we'll have to reorganize it at some point.
Eliad.
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_disconnect(), 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).
Signed-off-by: Eliad Peller <[email protected]>
---
drivers/net/wireless/ti/wlcore/main.c | 233 +++++++++++++--------------------
1 files changed, 88 insertions(+), 145 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index 41ed1d5..41a8502 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -2468,8 +2468,7 @@ static int wl12xx_op_change_interface(struct ieee80211_hw *hw,
return ret;
}
-static int wl1271_join(struct wl1271 *wl, struct wl12xx_vif *wlvif,
- bool set_assoc)
+static int wlcore_join(struct wl1271 *wl, struct wl12xx_vif *wlvif)
{
int ret;
bool is_ibss = (wlvif->bss_type == BSS_TYPE_IBSS);
@@ -2489,18 +2488,17 @@ static int wl1271_join(struct wl1271 *wl, struct wl12xx_vif *wlvif,
/* clear encryption type */
wlvif->encryption_type = KEY_NONE;
- if (set_assoc)
- set_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags);
-
if (is_ibss)
ret = wl12xx_cmd_role_start_ibss(wl, wlvif);
else
ret = wl12xx_cmd_role_start_sta(wl, wlvif);
- if (ret < 0)
- goto out;
- if (!test_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags))
- goto out;
+ return ret;
+}
+
+static int wlcore_set_assoc(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+{
+ int ret;
/*
* The join command disable the keep-alive mode, shut down its process,
@@ -2525,15 +2523,12 @@ static int wl1271_join(struct wl1271 *wl, struct wl12xx_vif *wlvif,
ACX_KEEP_ALIVE_TPL_VALID);
if (ret < 0)
goto out;
-
out:
return ret;
}
-static int wl1271_unjoin(struct wl1271 *wl, struct wl12xx_vif *wlvif)
+static void wlcore_disconnect(struct wl1271 *wl, struct wl12xx_vif *wlvif)
{
- int ret;
-
if (test_and_clear_bit(WLVIF_FLAG_CS_PROGRESS, &wlvif->flags)) {
struct ieee80211_vif *vif = wl12xx_wlvif_to_vif(wlvif);
@@ -2546,17 +2541,9 @@ static int wl1271_unjoin(struct wl1271 *wl, struct wl12xx_vif *wlvif)
wlvif->sta.klv_template_id,
ACX_KEEP_ALIVE_TPL_INVALID);
- /* to stop listening to a channel, we disconnect */
- ret = wl12xx_cmd_role_stop_sta(wl, wlvif);
- if (ret < 0)
- goto out;
-
/* reset TX security counters on a clean disconnect */
wlvif->tx_security_last_seq_lsb = 0;
wlvif->tx_security_seq = 0;
-
-out:
- return ret;
}
static void wl1271_set_band_rate(struct wl1271 *wl, struct wl12xx_vif *wlvif)
@@ -2565,45 +2552,6 @@ static void wl1271_set_band_rate(struct wl1271 *wl, struct wl12xx_vif *wlvif)
wlvif->rate_set = wlvif->basic_rate_set;
}
-static int wl1271_sta_handle_idle(struct wl1271 *wl, struct wl12xx_vif *wlvif,
- bool idle)
-{
- int ret;
- bool cur_idle = !test_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
-
- if (idle == cur_idle)
- return 0;
-
- if (idle) {
- /* no need to croc if we weren't busy (e.g. during boot) */
- if (wl12xx_dev_role_started(wlvif)) {
- ret = wl12xx_stop_dev(wl, wlvif);
- if (ret < 0)
- goto out;
- }
- wlvif->rate_set =
- wl1271_tx_min_rate_get(wl, wlvif->basic_rate_set);
- ret = wl1271_acx_sta_rate_policies(wl, wlvif);
- if (ret < 0)
- goto out;
- clear_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
- } else {
- /* The current firmware only supports sched_scan in idle */
- if (wl->sched_scanning) {
- wl1271_scan_sched_scan_stop(wl, wlvif);
- ieee80211_sched_scan_stopped(wl->hw);
- }
-
- ret = wl12xx_start_dev(wl, wlvif);
- if (ret < 0)
- goto out;
- set_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
- }
-
-out:
- return ret;
-}
-
static int wl12xx_config_vif(struct wl1271 *wl, struct wl12xx_vif *wlvif,
struct ieee80211_conf *conf, u32 changed)
{
@@ -3833,8 +3781,10 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
ibss_joined = true;
} else {
if (test_and_clear_bit(WLVIF_FLAG_IBSS_JOINED,
- &wlvif->flags))
- wl1271_unjoin(wl, wlvif);
+ &wlvif->flags)) {
+ wlcore_disconnect(wl, wlvif);
+ wl12xx_cmd_role_stop_sta(wl, wlvif);
+ }
}
}
@@ -3852,12 +3802,6 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
do_join = true;
}
- if (changed & BSS_CHANGED_IDLE && !is_ibss) {
- ret = wl1271_sta_handle_idle(wl, wlvif, bss_conf->idle);
- if (ret < 0)
- wl1271_warning("idle mode change failed %d", ret);
- }
-
if ((changed & BSS_CHANGED_CQM)) {
bool enable = false;
if (bss_conf->cqm_rssi_thold)
@@ -3870,18 +3814,7 @@ static void wl1271_bss_info_changed_sta(struct wl1271 *wl,
wlvif->rssi_thold = bss_conf->cqm_rssi_thold;
}
- if (changed & BSS_CHANGED_BSSID)
- if (!is_zero_ether_addr(bss_conf->bssid)) {
- ret = wl12xx_cmd_build_null_data(wl, wlvif);
- if (ret < 0)
- goto out;
-
- ret = wl1271_build_qos_null_data(wl, vif);
- if (ret < 0)
- goto out;
- }
-
- if (changed & (BSS_CHANGED_ASSOC | BSS_CHANGED_HT)) {
+ if (changed & (BSS_CHANGED_BSSID | BSS_CHANGED_HT)) {
rcu_read_lock();
sta = ieee80211_find_sta(vif, bss_conf->bssid);
if (!sta)
@@ -3900,20 +3833,16 @@ sta_not_found:
rcu_read_unlock();
}
- if ((changed & BSS_CHANGED_ASSOC)) {
- if (bss_conf->assoc) {
+ if (changed & BSS_CHANGED_BSSID) {
+ if (!is_zero_ether_addr(bss_conf->bssid)) {
u32 rates;
- int ieoffset;
- wlvif->aid = bss_conf->aid;
- wlvif->channel_type = bss_conf->channel_type;
- wlvif->beacon_int = bss_conf->beacon_int;
- do_join = true;
- set_assoc = true;
+ wl1271_debug(DEBUG_MAC80211,
+ "changed_bssid: %pM, aid: %d, bcn_int: %d, brates: 0x%x sta_rate_set: 0x%x (%d)",
+ bss_conf->bssid, bss_conf->aid,
+ bss_conf->beacon_int,
+ bss_conf->basic_rates, sta_rate_set, sta_exists);
- /*
- * use basic rates from AP, and determine lowest rate
- * to use with control frames.
- */
+ wlvif->beacon_int = bss_conf->beacon_int;
rates = bss_conf->basic_rates;
wlvif->basic_rate_set =
wl1271_tx_enabled_rates_get(wl, rates,
@@ -3921,15 +3850,63 @@ sta_not_found:
wlvif->basic_rate =
wl1271_tx_min_rate_get(wl,
wlvif->basic_rate_set);
+
if (sta_rate_set)
wlvif->rate_set =
wl1271_tx_enabled_rates_get(wl,
sta_rate_set,
wlvif->band);
+
+ /* we only supports sched_scan while not connected */
+ if (wl->sched_scanning) {
+ wl1271_scan_sched_scan_stop(wl, wlvif);
+ ieee80211_sched_scan_stopped(wl->hw);
+ }
+
ret = wl1271_acx_sta_rate_policies(wl, wlvif);
if (ret < 0)
goto out;
+ ret = wl12xx_cmd_build_null_data(wl, wlvif);
+ if (ret < 0)
+ goto out;
+
+ ret = wl1271_build_qos_null_data(wl, vif);
+ if (ret < 0)
+ goto out;
+
+ /* Need to update the BSSID (for filtering etc) */
+ set_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
+ do_join = true;
+ } else {
+ /* revert back to minimum rates for the current band */
+ wl1271_set_band_rate(wl, wlvif);
+ wlvif->basic_rate =
+ wl1271_tx_min_rate_get(wl,
+ wlvif->basic_rate_set);
+ ret = wl1271_acx_sta_rate_policies(wl, wlvif);
+ if (ret < 0)
+ goto out;
+
+ if (!is_ibss &&
+ test_bit(WLVIF_FLAG_IN_USE, &wlvif->flags)) {
+ ret = wl12xx_cmd_role_stop_sta(wl, wlvif);
+ if (ret < 0)
+ goto out;
+ }
+ clear_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
+ }
+ }
+
+ if ((changed & BSS_CHANGED_ASSOC)) {
+ if (bss_conf->assoc) {
+ int ieoffset;
+ wlvif->aid = bss_conf->aid;
+ wlvif->channel_type = bss_conf->channel_type;
+ wlvif->beacon_int = bss_conf->beacon_int;
+
+ set_bit(WLVIF_FLAG_STA_ASSOCIATED, &wlvif->flags);
+
/*
* with wl1271, we don't need to update the
* beacon_int and dtim_period, because the firmware
@@ -3955,64 +3932,32 @@ sta_not_found:
ret = wl1271_acx_conn_monit_params(wl, wlvif, true);
if (ret < 0)
goto out;
+
+ set_assoc = true;
} else {
- /* use defaults when not associated */
bool was_assoc =
!!test_and_clear_bit(WLVIF_FLAG_STA_ASSOCIATED,
&wlvif->flags);
- bool was_ifup =
- !!test_and_clear_bit(WLVIF_FLAG_STA_STATE_SENT,
- &wlvif->flags);
+
+ /* use defaults when not associated */
wlvif->aid = 0;
/* free probe-request template */
dev_kfree_skb(wlvif->probereq);
wlvif->probereq = NULL;
- /* revert back to minimum rates for the current band */
- wl1271_set_band_rate(wl, wlvif);
- wlvif->basic_rate =
- wl1271_tx_min_rate_get(wl,
- wlvif->basic_rate_set);
- ret = wl1271_acx_sta_rate_policies(wl, wlvif);
- if (ret < 0)
- goto out;
-
/* disable connection monitor features */
ret = wl1271_acx_conn_monit_params(wl, wlvif, false);
+ if (ret < 0)
+ goto out;
/* Disable the keep-alive feature */
ret = wl1271_acx_keep_alive_mode(wl, wlvif, false);
if (ret < 0)
goto out;
- /* restore the bssid filter and go to dummy bssid */
- if (was_assoc) {
- /*
- * we might have to disable roc, if there was
- * no IF_OPER_UP notification.
- */
- if (!was_ifup) {
- ret = wl12xx_croc(wl, wlvif->role_id);
- if (ret < 0)
- goto out;
- }
- /*
- * (we also need to disable roc in case of
- * roaming on the same channel. until we will
- * have a better flow...)
- */
- if (test_bit(wlvif->dev_role_id, wl->roc_map)) {
- ret = wl12xx_croc(wl,
- wlvif->dev_role_id);
- if (ret < 0)
- goto out;
- }
-
- wl1271_unjoin(wl, wlvif);
- if (!bss_conf->idle)
- wl12xx_start_dev(wl, wlvif);
- }
+ if (was_assoc)
+ wlcore_disconnect(wl, wlvif);
}
}
@@ -4042,7 +3987,7 @@ sta_not_found:
goto out;
if (do_join) {
- ret = wl1271_join(wl, wlvif, set_assoc);
+ ret = wlcore_join(wl, wlvif);
if (ret < 0) {
wl1271_warning("cmd join failed %d", ret);
goto out;
@@ -4053,21 +3998,18 @@ sta_not_found:
ret = wl12xx_roc(wl, wlvif, wlvif->role_id);
if (ret < 0)
goto out;
-
- if (test_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags))
- wl12xx_set_authorized(wl, wlvif);
- }
- /*
- * stop device role if started (we might already be in
- * STA/IBSS role).
- */
- if (wl12xx_dev_role_started(wlvif)) {
- ret = wl12xx_stop_dev(wl, wlvif);
- if (ret < 0)
- goto out;
}
}
+ if (set_assoc) {
+ ret = wlcore_set_assoc(wl, wlvif);
+ if (ret < 0)
+ goto out;
+
+ if (test_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags))
+ wl12xx_set_authorized(wl, wlvif);
+ }
+
/* Handle new association with HT. Do this after join. */
if (sta_exists) {
if ((changed & BSS_CHANGED_HT) &&
@@ -4434,6 +4376,7 @@ static int wl12xx_update_sta_state(struct wl1271 *wl,
old_state == IEEE80211_STA_AUTHORIZED &&
new_state == IEEE80211_STA_ASSOC) {
clear_bit(WLVIF_FLAG_STA_AUTHORIZED, &wlvif->flags);
+ clear_bit(WLVIF_FLAG_STA_STATE_SENT, &wlvif->flags);
return 0;
}
--
1.7.6.401.g6a319
On Mon, Nov 19, 2012 at 8:14 PM, Luciano Coelho <[email protected]> wrote:
> On Mon, 2012-11-19 at 18:39 +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_disconnect(), 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).
>>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
>
> [...]
>
>> -static int wl1271_unjoin(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>> +static void wlcore_disconnect(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>
> Why not wlcore_unset_assoc()? Or change the wlcore_set_assoc() to
> wlcore_connect()?
>
i'll pick the first option.
>> + if (wl->sched_scanning) {
>> + wl1271_scan_sched_scan_stop(wl, wlvif);
>> + ieee80211_sched_scan_stopped(wl->hw);
>> + }
>> +
>> ret = wl1271_acx_sta_rate_policies(wl, wlvif);
>> if (ret < 0)
>> goto out;
>>
>> + ret = wl12xx_cmd_build_null_data(wl, wlvif);
>> + if (ret < 0)
>> + goto out;
>> +
>> + ret = wl1271_build_qos_null_data(wl, vif);
>> + if (ret < 0)
>> + goto out;
>> +
>> + /* Need to update the BSSID (for filtering etc) */
>> + set_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
>> + do_join = true;
>> + } else {
>> + /* revert back to minimum rates for the current band */
>> + wl1271_set_band_rate(wl, wlvif);
>> + wlvif->basic_rate =
>> + wl1271_tx_min_rate_get(wl,
>> + wlvif->basic_rate_set);
>> + ret = wl1271_acx_sta_rate_policies(wl, wlvif);
>> + if (ret < 0)
>> + goto out;
>> +
>> + if (!is_ibss &&
>> + test_bit(WLVIF_FLAG_IN_USE, &wlvif->flags)) {
>> + ret = wl12xx_cmd_role_stop_sta(wl, wlvif);
>> + if (ret < 0)
>> + goto out;
>> + }
>> + clear_bit(WLVIF_FLAG_IN_USE, &wlvif->flags);
>> + }
>> + }
>
> It would be nice to keep all this stuff in a separate function instead
> of bringing it back into this already-huge wl1271_bss_info_changed_sta()
> function.
>
sure.
i'll give this function some more overhaul.
this applies also for the other similar comments.
Eliad.
On Tue, Nov 20, 2012 at 9:39 AM, Luciano Coelho <[email protected]> wrote:
> On Mon, 2012-11-19 at 18:39 +0200, Eliad Peller wrote:
>> The default ps mode of the fw is auto, while the default
>> ps mode of mac80211 is active (ps off).
>> In order to sync them, configure active ps on association.
>>
>> Signed-off-by: Eliad Peller <[email protected]>
>> ---
>> drivers/net/wireless/ti/wlcore/main.c | 9 +++++++++
>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
>> index e1f0606..0dd0ec1 100644
>> --- a/drivers/net/wireless/ti/wlcore/main.c
>> +++ b/drivers/net/wireless/ti/wlcore/main.c
>> @@ -2534,6 +2534,15 @@ static int wlcore_set_assoc(struct wl1271 *wl, struct wl12xx_vif *wlvif)
>> ACX_KEEP_ALIVE_TPL_VALID);
>> if (ret < 0)
>> goto out;
>> +
>> + /*
>> + * The default fw psm configuration is AUTO, while mac80211 default
>> + * setting is off (ACTIVE), so sync the fw with the correct value.
>> + */
>> + ret = wl1271_ps_set_mode(wl, wlvif, STATION_ACTIVE_MODE);
>> + if (ret < 0)
>> + goto out;
>> +
>
> Does mac80211 enable PS soon after this by default?
>
usually yes (unless userspace disabled it, etc.).
the problem is when mac80211 doesn't enable it - we won't get any
CHANGED_PSM notification, and then while mac80211 considers it as
active, the fw is configured (by default) to AUTO.
Eliad.