Re: [PATCH 6/7] mac80211: wait for the first beacon on the new channel after CSA
Sorry for the delay in replying. On Mon, 2014-09-29 at 11:02 +0200, Michal Kazior wrote: On 26 September 2014 21:35, Luca Coelho l...@coelho.fi wrote: From: Luciano Coelho luciano.coe...@intel.com Instead of immediately reopening the queues (in case of block_tx), calling the post_channel_switch operation and sending the notification, wait for the first beacon on the new channel. This makes sure that we don't lose packets if the AP/GO is not on the new channel yet. Signed-off-by: Luciano Coelho luciano.coe...@intel.com [...] Just a few nitpicks from me: +static void ieee80211_chswitch_post_beacon(struct ieee80211_sub_if_data *sdata) +{ + struct ieee80211_local *local = sdata-local; + struct ieee80211_if_managed *ifmgd = sdata-u.mgd; + int ret; + + sdata_assert_lock(sdata); I was thinking about WARN_ON(!sdata-vif.csa_active) here. csa_active should be set in all cases if csa_waiting_bcn is set, no? Good idea, csa_active must still be true otherwise we may get into trouble. I'll add the WARN_ON. - /* XXX: wait for a beacon first? */ if (sdata-csa_block_tx) { ieee80211_wake_vif_queues(local, sdata, IEEE80211_QUEUE_STOP_REASON_CSA); sdata-csa_block_tx = false; } + sdata-vif.csa_active = false; + ifmgd-csa_waiting_bcn = false; + ret = drv_post_channel_switch(sdata); if (ret) { sdata_info(sdata, driver post channel switch failed, disconnecting\n); ieee80211_queue_work(local-hw, ifmgd-csa_connection_drop_work); - goto out; + return; } here - ieee80211_sta_reset_beacon_monitor(sdata); - ieee80211_sta_reset_conn_monitor(sdata); - -out: - mutex_unlock(local-chanctx_mtx); - mutex_unlock(local-mtx); - sdata_unlock(sdata); } Is that an empty line I before final `}`? Yep, good catch, I'll fix it. -- Luca. -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/7] mac80211: add pre_channel_switch driver operation
From: Luciano Coelho luciano.coe...@intel.com Some drivers may need to prepare for a channel switch also when it is initiated from the remote side (eg. station, P2P client). To make this possible, add a generic callback that can be called for all interface types. Signed-off-by: Luciano Coelho luciano.coe...@intel.com --- include/net/mac80211.h| 7 +++ net/mac80211/cfg.c| 11 +++ net/mac80211/driver-ops.h | 18 ++ net/mac80211/mlme.c | 25 + net/mac80211/trace.h | 33 + 5 files changed, 86 insertions(+), 8 deletions(-) diff --git a/include/net/mac80211.h b/include/net/mac80211.h index ec0a5b0..19e4159 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -2832,6 +2832,10 @@ enum ieee80211_roc_type { * transmitted and then call ieee80211_csa_finish(). * If the CSA count starts as zero or 1, this function will not be called, * since there won't be any time to beacon before the switch anyway. + * @pre_channel_switch: This is an optional callback that is called + * before a channel switch procedure is started (ie. when a STA + * gets a CSA or an userspace initiated channel-switch), allowing + * the driver to prepare for the channel switch. * * @join_ibss: Join an IBSS (on an IBSS interface); this is called after all * information in bss_conf is set up and the beacon can be retrieved. A @@ -3038,6 +3042,9 @@ struct ieee80211_ops { void (*channel_switch_beacon)(struct ieee80211_hw *hw, struct ieee80211_vif *vif, struct cfg80211_chan_def *chandef); + int (*pre_channel_switch)(struct ieee80211_hw *hw, + struct ieee80211_vif *vif, + struct ieee80211_channel_switch *ch_switch); int (*join_ibss)(struct ieee80211_hw *hw, struct ieee80211_vif *vif); void (*leave_ibss)(struct ieee80211_hw *hw, struct ieee80211_vif *vif); diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index fb6a150..2caa7f0 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -3053,6 +3053,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev, { struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev); struct ieee80211_local *local = sdata-local; + struct ieee80211_channel_switch ch_switch; struct ieee80211_chanctx_conf *conf; struct ieee80211_chanctx *chanctx; int err, changed = 0; @@ -3088,6 +3089,10 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev, goto out; } + err = drv_pre_channel_switch(sdata, ch_switch); + if (err) + goto out; + err = ieee80211_vif_reserve_chanctx(sdata, params-chandef, chanctx-mode, params-radar_required); @@ -3101,6 +3106,12 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev, goto out; } + ch_switch.timestamp = 0; + ch_switch.device_timestamp = 0; + ch_switch.block_tx = params-block_tx; + ch_switch.chandef = params-chandef; + ch_switch.count = params-count; + err = ieee80211_set_csa_beacon(sdata, params, changed); if (err) { ieee80211_vif_unreserve_chanctx(sdata); diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h index 196d48c..5522672 100644 --- a/net/mac80211/driver-ops.h +++ b/net/mac80211/driver-ops.h @@ -1196,6 +1196,24 @@ drv_channel_switch_beacon(struct ieee80211_sub_if_data *sdata, } } +static inline int +drv_pre_channel_switch(struct ieee80211_sub_if_data *sdata, + struct ieee80211_channel_switch *ch_switch) +{ + struct ieee80211_local *local = sdata-local; + int ret = 0; + + if (!check_sdata_in_driver(sdata)) + return -EIO; + + trace_drv_pre_channel_switch(local, sdata, ch_switch); + if (local-ops-pre_channel_switch) + ret = local-ops-pre_channel_switch(local-hw, sdata-vif, +ch_switch); + trace_drv_return_int(local, ret); + return ret; +} + static inline int drv_join_ibss(struct ieee80211_local *local, struct ieee80211_sub_if_data *sdata) { diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c index cb4d074..c5fa20f 100644 --- a/net/mac80211/mlme.c +++ b/net/mac80211/mlme.c @@ -1057,6 +1057,7 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata, struct ieee80211_chanctx *chanctx; enum ieee80211_band current_band; struct ieee80211_csa_ie csa_ie; + struct ieee80211_channel_switch ch_switch; int res; sdata_assert_lock(sdata); @@ -1128,6 +1129,22
[PATCH v2 0/7] mac80211: some more channel switch work
From: Luciano Coelho luciano.coe...@intel.com Hi, A few minor changes in v2: * add WARN_ON to make sure csa_active is set in ieee80211_chswitch_post_beacon() [Michal]; * remove stray empty line [Michal]; * s/vifos/vif/ in a mac80211.h declaration (no clue where the os came from :P); ---8--- These patches contain some more channel switch work. They have been in use in our internal tree for some time now and are mostly adding small details needed by the iwlmvm driver. There's also a nice improvement to a long-time TODO, where we wait for a beacon on the new channel before we start transmitting again (in BSS station/P2P client mode). One slightly controversial change is the one that allows channel switches with multiple channel contexts. The problem is not with mult-context itself, but the handling of drivers that offload the channel switch and have, so far, not received any information about the vif that is switching (so the assumption was that all vifs would have to switch). Please review. -- Cheers, Luca. Luciano Coelho (7): nl80211: sanity check the channel switch counter value mac80211: add device_timestamp to the ieee80211_channel_switch struct mac80211: add extended channel switching capability if the driver supports CSA mac80211: add pre_channel_switch driver operation mac80211: add post_channel_switch driver operation mac80211: wait for the first beacon on the new channel after CSA mac80211: allow channel switch with multiple channel contexts drivers/net/wireless/iwlegacy/4965-mac.c| 2 +- drivers/net/wireless/iwlegacy/4965.h| 5 +- drivers/net/wireless/iwlwifi/dvm/mac80211.c | 1 + drivers/net/wireless/ti/wlcore/main.c | 23 ++ include/linux/ieee80211.h | 5 ++ include/net/mac80211.h | 17 + net/mac80211/cfg.c | 18 - net/mac80211/driver-ops.h | 41 ++- net/mac80211/ieee80211_i.h | 5 ++ net/mac80211/iface.c| 2 + net/mac80211/main.c | 20 ++--- net/mac80211/mlme.c | 110 ++-- net/mac80211/trace.h| 50 - net/wireless/nl80211.c | 10 ++- 14 files changed, 237 insertions(+), 72 deletions(-) -- 2.1.1 -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/7] mac80211: add device_timestamp to the ieee80211_channel_switch struct
From: Luciano Coelho luciano.coe...@intel.com Some devices may need the device timestamp in order to synchronize the channel switch. To pass this value back to the driver, add it to the channel switch structure and copy the device_timestamp value received in the rx info structure into it. Signed-off-by: Luciano Coelho luciano.coe...@intel.com --- include/net/mac80211.h | 3 +++ net/mac80211/mlme.c| 15 ++- net/mac80211/trace.h | 2 ++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 0ad1f47..ec0a5b0 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -1117,6 +1117,8 @@ struct ieee80211_conf { * Function (TSF) timer when the frame containing the channel switch * announcement was received. This is simply the rx.mactime parameter * the driver passed into mac80211. + * @device_timestamp: arbitrary timestamp for the device, this is the + * rx.device_timestamp parameter the driver passed to mac80211. * @block_tx: Indicates whether transmission must be blocked before the * scheduled channel switch, as indicated by the AP. * @chandef: the new channel to switch to @@ -1124,6 +1126,7 @@ struct ieee80211_conf { */ struct ieee80211_channel_switch { u64 timestamp; + u32 device_timestamp; bool block_tx; struct cfg80211_chan_def chandef; u8 count; diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c index 9d4ccb2..cb4d074 100644 --- a/net/mac80211/mlme.c +++ b/net/mac80211/mlme.c @@ -1046,7 +1046,8 @@ static void ieee80211_chswitch_timer(unsigned long data) static void ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata, -u64 timestamp, struct ieee802_11_elems *elems, +u64 timestamp, u32 device_timestamp, +struct ieee802_11_elems *elems, bool beacon) { struct ieee80211_local *local = sdata-local; @@ -1154,6 +1155,7 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata, /* use driver's channel switch callback */ struct ieee80211_channel_switch ch_switch = { .timestamp = timestamp, + .device_timestamp = device_timestamp, .block_tx = csa_ie.mode, .chandef = csa_ie.chandef, .count = csa_ie.count, @@ -3203,6 +3205,7 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata, ieee80211_rx_bss_info(sdata, mgmt, len, rx_status, elems); ieee80211_sta_process_chanswitch(sdata, rx_status-mactime, +rx_status-device_timestamp, elems, true); if (!(ifmgd-flags IEEE80211_STA_DISABLE_WMM) @@ -3334,8 +3337,9 @@ void ieee80211_sta_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata, break; ieee80211_sta_process_chanswitch(sdata, -rx_status-mactime, -elems, false); +rx_status-mactime, +rx_status-device_timestamp, +elems, false); } else if (mgmt-u.action.category == WLAN_CATEGORY_PUBLIC) { ies_len = skb-len - offsetof(struct ieee80211_mgmt, @@ -3356,8 +3360,9 @@ void ieee80211_sta_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata, mgmt-u.action.u.ext_chan_switch.data; ieee80211_sta_process_chanswitch(sdata, -rx_status-mactime, -elems, false); +rx_status-mactime, +rx_status-device_timestamp, +elems, false); } break; } diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h index 38fae7e..853c440 100644 --- a/net/mac80211/trace.h +++ b/net/mac80211/trace.h @@ -995,6 +995,7 @@ TRACE_EVENT(drv_channel_switch, LOCAL_ENTRY CHANDEF_ENTRY __field(u64, timestamp) + __field(u32, device_timestamp) __field(bool, block_tx) __field(u8, count) ), @@ -1003,6 +1004,7 @@ TRACE_EVENT(drv_channel_switch, LOCAL_ASSIGN; CHANDEF_ASSIGN(ch_switch-chandef) __entry-timestamp = ch_switch-timestamp; + __entry-device_timestamp =
[PATCH v2 1/7] nl80211: sanity check the channel switch counter value
From: Luciano Coelho luciano.coe...@intel.com The nl80211 channel switch count attribute (NL80211_ATTR_CH_SWITCH_COUNT) is specified as u32, but the specification uses u8 for the counter. To make sure strange things don't happen without informing the user, sanity check the value and return -EINVAL if it doesn't fit in u8. Signed-off-by: Luciano Coelho luciano.coe...@intel.com --- net/wireless/nl80211.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 4cce3e1..9e29053 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -5927,6 +5927,7 @@ static int nl80211_channel_switch(struct sk_buff *skb, struct genl_info *info) int err; bool need_new_beacon = false; int len, i; + u32 cs_count; if (!rdev-ops-channel_switch || !(rdev-wiphy.flags WIPHY_FLAG_HAS_CHANNEL_SWITCH)) @@ -5963,7 +5964,14 @@ static int nl80211_channel_switch(struct sk_buff *skb, struct genl_info *info) if (need_new_beacon !info-attrs[NL80211_ATTR_CSA_IES]) return -EINVAL; - params.count = nla_get_u32(info-attrs[NL80211_ATTR_CH_SWITCH_COUNT]); + /* Even though the attribute is u32, the specification says +* u8, so let's make sure we don't overflow. +*/ + cs_count = nla_get_u32(info-attrs[NL80211_ATTR_CH_SWITCH_COUNT]); + if (cs_count 255) + return -EINVAL; + + params.count = cs_count; if (!need_new_beacon) goto skip_beacons; -- 2.1.1 -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/7] mac80211: add post_channel_switch driver operation
From: Luciano Coelho luciano.coe...@intel.com As a counterpart to the pre_channel_switch operation, add a post_channel_switch operation. This allows the drivers to go back to a normal configuration after the channel switch is completed. Signed-off-by: Luciano Coelho luciano.coe...@intel.com --- include/net/mac80211.h| 6 ++ net/mac80211/cfg.c| 7 ++- net/mac80211/driver-ops.h | 16 net/mac80211/mlme.c | 9 + net/mac80211/trace.h | 6 ++ 5 files changed, 43 insertions(+), 1 deletion(-) diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 19e4159..7861ed8 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -2836,6 +2836,9 @@ enum ieee80211_roc_type { * before a channel switch procedure is started (ie. when a STA * gets a CSA or an userspace initiated channel-switch), allowing * the driver to prepare for the channel switch. + * @post_channel_switch: This is an optional callback that is called + * after a channel switch procedure is completed, allowing the + * driver to go back to a normal configuration. * * @join_ibss: Join an IBSS (on an IBSS interface); this is called after all * information in bss_conf is set up and the beacon can be retrieved. A @@ -3046,6 +3049,9 @@ struct ieee80211_ops { struct ieee80211_vif *vif, struct ieee80211_channel_switch *ch_switch); + int (*post_channel_switch)(struct ieee80211_hw *hw, + struct ieee80211_vif *vif); + int (*join_ibss)(struct ieee80211_hw *hw, struct ieee80211_vif *vif); void (*leave_ibss)(struct ieee80211_hw *hw, struct ieee80211_vif *vif); u32 (*get_expected_throughput)(struct ieee80211_sta *sta); diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index 2caa7f0..ce81ff2 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -2868,7 +2868,6 @@ static int __ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata) return err; ieee80211_bss_info_change_notify(sdata, changed); - cfg80211_ch_switch_notify(sdata-dev, sdata-csa_chandef); if (sdata-csa_block_tx) { ieee80211_wake_vif_queues(local, sdata, @@ -2876,6 +2875,12 @@ static int __ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata) sdata-csa_block_tx = false; } + err = drv_post_channel_switch(sdata); + if (err) + return err; + + cfg80211_ch_switch_notify(sdata-dev, sdata-csa_chandef); + return 0; } diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h index 5522672..0a60906 100644 --- a/net/mac80211/driver-ops.h +++ b/net/mac80211/driver-ops.h @@ -1214,6 +1214,22 @@ drv_pre_channel_switch(struct ieee80211_sub_if_data *sdata, return ret; } +static inline int +drv_post_channel_switch(struct ieee80211_sub_if_data *sdata) +{ + struct ieee80211_local *local = sdata-local; + int ret = 0; + + if (!check_sdata_in_driver(sdata)) + return -EIO; + + trace_drv_post_channel_switch(local, sdata); + if (local-ops-post_channel_switch) + ret = local-ops-post_channel_switch(local-hw, sdata-vif); + trace_drv_return_int(local, ret); + return ret; +} + static inline int drv_join_ibss(struct ieee80211_local *local, struct ieee80211_sub_if_data *sdata) { diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c index c5fa20f..2e29cc3 100644 --- a/net/mac80211/mlme.c +++ b/net/mac80211/mlme.c @@ -1010,6 +1010,15 @@ static void ieee80211_chswitch_work(struct work_struct *work) sdata-csa_block_tx = false; } + ret = drv_post_channel_switch(sdata); + if (ret) { + sdata_info(sdata, + driver post channel switch failed, disconnecting\n); + ieee80211_queue_work(local-hw, +ifmgd-csa_connection_drop_work); + goto out; + } + ieee80211_sta_reset_beacon_monitor(sdata); ieee80211_sta_reset_conn_monitor(sdata); diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h index 30476d2..ca0e12d 100644 --- a/net/mac80211/trace.h +++ b/net/mac80211/trace.h @@ -2141,6 +2141,12 @@ TRACE_EVENT(drv_pre_channel_switch, ) ); +DEFINE_EVENT(local_sdata_evt, drv_post_channel_switch, +TP_PROTO(struct ieee80211_local *local, + struct ieee80211_sub_if_data *sdata), +TP_ARGS(local, sdata) +); + #ifdef CONFIG_MAC80211_MESSAGE_TRACING #undef TRACE_SYSTEM -- 2.1.1 -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/7] mac80211: allow channel switch with multiple channel contexts
From: Luciano Coelho luciano.coe...@intel.com Channel switch with multiple channel contexts should now work fine. Remove check that disallows switches when multiple contexts are in use. Signed-off-by: Luciano Coelho luciano.coe...@intel.com --- Note: the driver changes are only compile-tested. --- drivers/net/wireless/iwlegacy/4965-mac.c| 2 +- drivers/net/wireless/iwlegacy/4965.h| 5 +++-- drivers/net/wireless/iwlwifi/dvm/mac80211.c | 1 + drivers/net/wireless/ti/wlcore/main.c | 23 --- include/net/mac80211.h | 1 + net/mac80211/driver-ops.h | 7 --- net/mac80211/mlme.c | 26 ++ net/mac80211/trace.h| 9 ++--- 8 files changed, 34 insertions(+), 40 deletions(-) diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c index 9f930a0..42c4e39 100644 --- a/drivers/net/wireless/iwlegacy/4965-mac.c +++ b/drivers/net/wireless/iwlegacy/4965-mac.c @@ -6063,7 +6063,7 @@ il4965_mac_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif, } void -il4965_mac_channel_switch(struct ieee80211_hw *hw, +il4965_mac_channel_switch(struct ieee80211_hw *hw, struct ieee80211_vif *vif, struct ieee80211_channel_switch *ch_switch) { struct il_priv *il = hw-priv; diff --git a/drivers/net/wireless/iwlegacy/4965.h b/drivers/net/wireless/iwlegacy/4965.h index 337dfcf..3a57f71 100644 --- a/drivers/net/wireless/iwlegacy/4965.h +++ b/drivers/net/wireless/iwlegacy/4965.h @@ -187,8 +187,9 @@ int il4965_mac_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif, u8 buf_size); int il4965_mac_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif, struct ieee80211_sta *sta); -void il4965_mac_channel_switch(struct ieee80211_hw *hw, - struct ieee80211_channel_switch *ch_switch); +void +il4965_mac_channel_switch(struct ieee80211_hw *hw, struct ieee80211_vif *vif, + struct ieee80211_channel_switch *ch_switch); void il4965_led_enable(struct il_priv *il); diff --git a/drivers/net/wireless/iwlwifi/dvm/mac80211.c b/drivers/net/wireless/iwlwifi/dvm/mac80211.c index 2364a3c..a967bf8 100644 --- a/drivers/net/wireless/iwlwifi/dvm/mac80211.c +++ b/drivers/net/wireless/iwlwifi/dvm/mac80211.c @@ -941,6 +941,7 @@ static int iwlagn_mac_sta_state(struct ieee80211_hw *hw, } static void iwlagn_mac_channel_switch(struct ieee80211_hw *hw, + struct ieee80211_vif *vif, struct ieee80211_channel_switch *ch_switch) { struct iwl_priv *priv = IWL_MAC80211_GET_DVM(hw); diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c index 575c8f6..6ad3fce 100644 --- a/drivers/net/wireless/ti/wlcore/main.c +++ b/drivers/net/wireless/ti/wlcore/main.c @@ -5177,10 +5177,11 @@ out: } static void wl12xx_op_channel_switch(struct ieee80211_hw *hw, +struct ieee80211_vif *vif, struct ieee80211_channel_switch *ch_switch) { struct wl1271 *wl = hw-priv; - struct wl12xx_vif *wlvif; + struct wl12xx_vif *wlvif = wl12xx_vif_to_data(vif); int ret; wl1271_debug(DEBUG_MAC80211, mac80211 channel switch); @@ -5190,14 +5191,8 @@ static void wl12xx_op_channel_switch(struct ieee80211_hw *hw, mutex_lock(wl-mutex); if (unlikely(wl-state == WLCORE_STATE_OFF)) { - wl12xx_for_each_wlvif_sta(wl, wlvif) { - struct ieee80211_vif *vif = wl12xx_wlvif_to_vif(wlvif); - - if (!test_bit(WLVIF_FLAG_STA_ASSOCIATED, wlvif-flags)) - continue; - + if (test_bit(WLVIF_FLAG_STA_ASSOCIATED, wlvif-flags)) ieee80211_chswitch_done(vif, false); - } goto out; } else if (unlikely(wl-state != WLCORE_STATE_ON)) { goto out; @@ -5208,11 +5203,9 @@ static void wl12xx_op_channel_switch(struct ieee80211_hw *hw, goto out; /* TODO: change mac80211 to pass vif as param */ - wl12xx_for_each_wlvif_sta(wl, wlvif) { - unsigned long delay_usec; - if (!test_bit(WLVIF_FLAG_STA_ASSOCIATED, wlvif-flags)) - continue; + if (test_bit(WLVIF_FLAG_STA_ASSOCIATED, wlvif-flags)) { + unsigned long delay_usec; ret = wl-ops-channel_switch(wl, wlvif, ch_switch); if (ret) @@ -5222,10 +5215,10 @@ static void wl12xx_op_channel_switch(struct ieee80211_hw *hw, /* indicate failure 5 seconds after channel switch time */ delay_usec = ieee80211_tu_to_usec(wlvif-beacon_int) * -
[PATCH v2 3/7] mac80211: add extended channel switching capability if the driver supports CSA
From: Luciano Coelho luciano.coe...@intel.com The Extended Channel Switching capability bit in the extended capabilities element must be set if the driver supports CSA on non-beaconing interfaces. Since this capability needs to be set during driver registration, the extended_capabiliities global variable needs to be moved to the local structure so that it can be modified. Signed-off-by: Luciano Coelho luciano.coe...@intel.com --- include/linux/ieee80211.h | 5 + net/mac80211/ieee80211_i.h | 3 +++ net/mac80211/main.c| 20 +++- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h index b1be39c..5fab17b 100644 --- a/include/linux/ieee80211.h +++ b/include/linux/ieee80211.h @@ -1998,6 +1998,11 @@ enum ieee80211_tdls_actioncode { WLAN_TDLS_DISCOVERY_REQUEST = 10, }; +/* Extended Channel Switching capability to be set in the 1st byte of + * the @WLAN_EID_EXT_CAPABILITY information element + */ +#define WLAN_EXT_CAPA1_EXT_CHANNEL_SWITCHING BIT(2) + /* Interworking capabilities are set in 7th bit of 4th byte of the * @WLAN_EID_EXT_CAPABILITY information element */ diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index c2aaec4..a9cc491 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -1307,6 +1307,9 @@ struct ieee80211_local { /* virtual monitor interface */ struct ieee80211_sub_if_data __rcu *monitor_sdata; struct cfg80211_chan_def monitor_chandef; + + /* extended capabilities provided by mac80211 */ + u8 ext_capa[8]; }; static inline struct ieee80211_sub_if_data * diff --git a/net/mac80211/main.c b/net/mac80211/main.c index 0de7c93..107d1c8 100644 --- a/net/mac80211/main.c +++ b/net/mac80211/main.c @@ -478,11 +478,6 @@ static const struct ieee80211_vht_cap mac80211_vht_capa_mod_mask = { }, }; -static const u8 extended_capabilities[] = { - 0, 0, 0, 0, 0, 0, 0, - WLAN_EXT_CAPA8_OPMODE_NOTIF, -}; - struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len, const struct ieee80211_ops *ops) { @@ -539,10 +534,6 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len, WIPHY_FLAG_REPORTS_OBSS | WIPHY_FLAG_OFFCHAN_TX; - wiphy-extended_capabilities = extended_capabilities; - wiphy-extended_capabilities_mask = extended_capabilities; - wiphy-extended_capabilities_len = ARRAY_SIZE(extended_capabilities); - if (ops-remain_on_channel) wiphy-flags |= WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL; @@ -591,6 +582,13 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len, wiphy-ht_capa_mod_mask = mac80211_ht_capa_mod_mask; wiphy-vht_capa_mod_mask = mac80211_vht_capa_mod_mask; + local-ext_capa[7] = WLAN_EXT_CAPA8_OPMODE_NOTIF; + + wiphy-extended_capabilities = local-ext_capa; + wiphy-extended_capabilities_mask = local-ext_capa; + wiphy-extended_capabilities_len = + ARRAY_SIZE(local-ext_capa); + INIT_LIST_HEAD(local-interfaces); __hw_addr_init(local-mc_list); @@ -958,6 +956,10 @@ int ieee80211_register_hw(struct ieee80211_hw *hw) if (local-hw.wiphy-flags WIPHY_FLAG_SUPPORTS_TDLS) local-hw.wiphy-flags |= WIPHY_FLAG_TDLS_EXTERNAL_SETUP; + /* mac80211 supports eCSA, if the driver supports STA CSA at all */ + if (local-hw.flags IEEE80211_HW_CHANCTX_STA_CSA) + local-ext_capa[0] |= WLAN_EXT_CAPA1_EXT_CHANNEL_SWITCHING; + local-hw.wiphy-max_num_csa_counters = IEEE80211_MAX_CSA_COUNTERS_NUM; result = wiphy_register(local-hw.wiphy); -- 2.1.1 -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rt2x00: rt2x00queue: avoid using more headroom then driver requested
Mark Asselstine asse...@gmail.com schrieb: On Wed, Oct 1, 2014 at 9:42 PM, Mark Asselstine asse...@gmail.com wrote: Damn, you are right. I thought I had it licked. Unfortunately with the overloaded variable name it was easy to get turned around. The comments in the code didn't prevent me knotting myself up either. If you look in in rt2x00.h, struct rt2x00_dev, the comment above the extra_tx_headroom member says Extra TX headroom required for alignment purposes. I would say this is incorrect. This variable is initialized via rt2x00dev_extra_tx_headroom() with a combination of winfo_size and desc_size and has nothing to do with alignment. At any rate, keeping in mind that rt2x00_dev.hw.extra_tx_headroom which is used to set the amount of available headroom includes room for alignment and padding as well as winfo and desc space, and that rt2x00_dev.extra_tx_headroom doesn't include padding or alignment, you are correct in that we aren't over spending our headroom. Back to the drawing board. Mark On Wed, Oct 1, 2014 at 5:12 AM, Stanislaw Gruszka sgrus...@redhat.com wrote: On Tue, Sep 30, 2014 at 11:45:57PM -0400, Mark Asselstine wrote: 'struct ieee80211_hw' contains 'extra_tx_headroom' which it defines as headroom to reserve in each transmit skb for use by the driver. This value is properly setup during rt2x00lib_probe_hw() to account for all the rt2x00lib's purposes, including DMA alignment and L2 pad if needed. As such under all circumstances the proper amount of headroom is allocated to a skb such that under any usage we would not ever use more headroom then is allotted. However rt2x00queue_write_tx_frame() uses up the headroom (via calls to skb_push) allotted for L2 padding (with a potential call to rt2x00queue_insert_l2pad()) and uses up the headroom allotted to DMA alignment (with a potential call to rt2x00queue_align_frame()) and then proceeds to use up 'extra_tx_headroom' (in a call to rt2x00queue_write_tx_data()) So the driver has requested just 'extra_tx_headroom' worth of headroom and we have used up 'extra_tx_headroom' + DMA + L2PAD worth. As such it is possible to hit a 'skb_under_panic', where we have used up all the available headroom. Since extra_tx_headroom was calculated as a function of winfo_size, desc_size, RT2X00_L2PAD_SIZE and RT2X00_ALIGN_SIZE we can simply remove the part for alignment and padding and we will know how much is left to use up (for txdesc) and only use that much. Keeping the driver's use of headroom to what it requested via extra_tx_headroom. Signed-off-by: Mark Asselstine asse...@gmail.com --- drivers/net/wireless/rt2x00/rt2x00queue.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c index 8e68f87..2a48bf5 100644 --- a/drivers/net/wireless/rt2x00/rt2x00queue.c +++ b/drivers/net/wireless/rt2x00/rt2x00queue.c @@ -522,6 +522,7 @@ static int rt2x00queue_write_tx_data(struct queue_entry *entry, struct txentry_desc *txdesc) { struct rt2x00_dev *rt2x00dev = entry-queue-rt2x00dev; + unsigned int avail_extra_tx_headroom = rt2x00dev-extra_tx_headroom; /* * This should not happen, we already checked the entry @@ -538,10 +539,18 @@ static int rt2x00queue_write_tx_data(struct queue_entry *entry, } /* - * Add the requested extra tx headroom in front of the skb. + * Add room for data at the front of the buffer for txdesc. The space + * needed for this was accounted for in extra_tx_headroom, we just + * need to remove the amount allocated for padding and alignment + * to get what is left for txdesc. */ - skb_push(entry-skb, rt2x00dev-extra_tx_headroom); - memset(entry-skb-data, 0, rt2x00dev-extra_tx_headroom); + if (test_bit(REQUIRE_L2PAD, rt2x00dev-cap_flags)) + avail_extra_tx_headroom -= RT2X00_L2PAD_SIZE; + else if (test_bit(REQUIRE_DMA, rt2x00dev-cap_flags)) + avail_extra_tx_headroom -= RT2X00_ALIGN_SIZE; + + skb_push(entry-skb, avail_extra_tx_headroom); + memset(entry-skb-data, 0, avail_extra_tx_headroom); I don't think patch is correct. We have rt2x00-extra_tx_headroom and rt2x00-hw-extra_tx_headroom. Second value, which we provide as maximum needed headroom to mac80211 is rt2x00-extra_tx_headrom + RT2X00_L2PAD_SIZE + RT2X00_ALIGN_SIZE. We really need to reserve rt2x00dev-extra_tx_headroom on TX skb, as this is room for metadata needed by H/W and if needed we should reserve RT2x00_L2PAD_SIZE and RTX00_ALIGN_SIZE. There should be room for that, since we provide bigger rt2x00-hw-extra_tx_headroom to mac80211. The only possibility to skb_under_panic I can see, is that we retransmit frame and try to align it many times, but alignment should
[PATCH 4/5] net: rfkill: gpio: Add OBDA8723 ACPI ID
Add OBDA8723 (rtl8723) ACPI id. rtl8723 bluetooth chip has the following interface: HOST ---UART--- CONTROLLER HOST --- WAKE--- CONTROLLER (gpio res 0) HOST ---ENABLE-- CONTROLLER (gpio res 1) HOST ---WAKE CONTROLLER (gpio res 2) Signed-off-by: Loic Poulain loic.poul...@intel.com --- net/rfkill/rfkill-gpio.c | 9 + 1 file changed, 9 insertions(+) diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c index 2f6aad1..28d1cbf 100644 --- a/net/rfkill/rfkill-gpio.c +++ b/net/rfkill/rfkill-gpio.c @@ -301,11 +301,20 @@ static struct rfkill_gpio_desc acpi_default_gps = { .host_wake_idx = -1, }; +static struct rfkill_gpio_desc acpi_bluetooth_wake = { + .type = RFKILL_TYPE_BLUETOOTH, + .reset_idx = -1, + .shutdown_idx = 1, + .wake_idx = 0, + .host_wake_idx = 2, +}; + static const struct acpi_device_id rfkill_acpi_match[] = { { BCM2E1A, (kernel_ulong_t)acpi_default_bluetooth }, { BCM2E39, (kernel_ulong_t)acpi_default_bluetooth }, { BCM2E3D, (kernel_ulong_t)acpi_default_bluetooth }, { BCM2E64, (kernel_ulong_t)acpi_default_bluetooth }, + { OBDA8723, (kernel_ulong_t)acpi_bluetooth_wake }, { BCM4752, (kernel_ulong_t)acpi_default_gps }, { LNV4752, (kernel_ulong_t)acpi_default_gps }, { }, -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] net: rfkill: gpio: Implement host wake up support
Some GPIO based rfkill devices can wake their host up from suspend by toggling an input (from the host perspective) GPIO. This patch adds a generic support for that feature by registering a threaded interrupt routine and thus setting the corresponding GPIO as a wake up source. Signed-off-by: Loic Poulain loic.poul...@intel.com --- net/rfkill/rfkill-gpio.c | 49 1 file changed, 49 insertions(+) diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c index 29369d6..2f6aad1 100644 --- a/net/rfkill/rfkill-gpio.c +++ b/net/rfkill/rfkill-gpio.c @@ -26,6 +26,7 @@ #include linux/slab.h #include linux/acpi.h #include linux/gpio/consumer.h +#include linux/interrupt.h #include linux/rfkill-gpio.h @@ -36,6 +37,8 @@ struct rfkill_gpio_data { struct gpio_desc*shutdown_gpio; struct gpio_desc*wake_gpio; + int host_wake_irq; + struct rfkill *rfkill_dev; struct clk *clk; @@ -48,6 +51,7 @@ struct rfkill_gpio_desc { int reset_idx; int shutdown_idx; int wake_idx; + int host_wake_idx; }; static int rfkill_gpio_set_power(void *data, bool blocked) @@ -73,6 +77,15 @@ static const struct rfkill_ops rfkill_gpio_ops = { .set_block = rfkill_gpio_set_power, }; +static irqreturn_t rfkill_gpio_host_wake(int irq, void *dev) +{ + struct rfkill_gpio_data *rfkill = dev; + + pr_info(Host wake IRQ for %s\n, rfkill-name); + + return IRQ_HANDLED; +} + static int rfkill_gpio_init(struct device *dev, struct rfkill_gpio_desc *desc) { int ret; @@ -125,6 +138,34 @@ static int rfkill_gpio_init(struct device *dev, struct rfkill_gpio_desc *desc) } } + rfkill-host_wake_irq = -1; + + if (desc (desc-host_wake_idx = 0)) { + gpio = devm_gpiod_get_index(dev, host_wake, + desc-host_wake_idx); + if (!IS_ERR(gpio)) { + int irqf = IRQF_TRIGGER_RISING | IRQF_NO_SUSPEND | + IRQF_ONESHOT; + + ret = gpiod_direction_input(gpio); + if (ret) + return ret; + + rfkill-host_wake_irq = gpiod_to_irq(gpio); + ret = devm_request_threaded_irq(dev, + rfkill-host_wake_irq, + NULL, + rfkill_gpio_host_wake, + irqf, host_wake, + rfkill); + if (ret) + return ret; + + /* Wakeup IRQ, only used in suspend */ + disable_irq(rfkill-host_wake_irq); + } + } + /* Make sure at-least one of the GPIO is defined */ if (!rfkill-reset_gpio !rfkill-shutdown_gpio) { dev_err(dev, invalid platform data\n); @@ -205,6 +246,9 @@ static int rfkill_gpio_suspend(struct device *dev) if (!rfkill-clk_enabled) return 0; + if (rfkill-host_wake_irq = 0) + enable_irq(rfkill-host_wake_irq); + gpiod_set_value_cansleep(rfkill-wake_gpio, 0); return 0; @@ -219,6 +263,9 @@ static int rfkill_gpio_resume(struct device *dev) if (!rfkill-clk_enabled) return 0; + if (rfkill-host_wake_irq = 0) + disable_irq(rfkill-host_wake_irq); + gpiod_set_value_cansleep(rfkill-wake_gpio, 1); return 0; @@ -243,6 +290,7 @@ static struct rfkill_gpio_desc acpi_default_bluetooth = { .reset_idx = 0, .shutdown_idx = 1, .wake_idx = -1, + .host_wake_idx = -1, }; static struct rfkill_gpio_desc acpi_default_gps = { @@ -250,6 +298,7 @@ static struct rfkill_gpio_desc acpi_default_gps = { .reset_idx = 0, .shutdown_idx = 1, .wake_idx = -1, + .host_wake_idx = -1, }; static const struct acpi_device_id rfkill_acpi_match[] = { -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] net: rfkill: gpio: Implement PM hooks
GPIO based rfkill devices should also be put to sleep mode when the system enters suspend. This patch introduces a wake gpio. Signed-off-by: Loic Poulain loic.poul...@intel.com --- net/rfkill/rfkill-gpio.c | 48 1 file changed, 48 insertions(+) diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c index 7982841..29369d6 100644 --- a/net/rfkill/rfkill-gpio.c +++ b/net/rfkill/rfkill-gpio.c @@ -34,6 +34,7 @@ struct rfkill_gpio_data { enum rfkill_typetype; struct gpio_desc*reset_gpio; struct gpio_desc*shutdown_gpio; + struct gpio_desc*wake_gpio; struct rfkill *rfkill_dev; struct clk *clk; @@ -46,6 +47,7 @@ struct rfkill_gpio_desc { int reset_idx; int shutdown_idx; + int wake_idx; }; static int rfkill_gpio_set_power(void *data, bool blocked) @@ -57,6 +59,7 @@ static int rfkill_gpio_set_power(void *data, bool blocked) gpiod_set_value_cansleep(rfkill-shutdown_gpio, !blocked); gpiod_set_value_cansleep(rfkill-reset_gpio, !blocked); + gpiod_set_value_cansleep(rfkill-wake_gpio, !blocked); if (blocked !IS_ERR(rfkill-clk) rfkill-clk_enabled) clk_disable(rfkill-clk); @@ -112,6 +115,16 @@ static int rfkill_gpio_init(struct device *dev, struct rfkill_gpio_desc *desc) rfkill-shutdown_gpio = gpio; } + if (desc (desc-wake_idx = 0)) { + gpio = devm_gpiod_get_index(dev, wake, desc-wake_idx); + if (!IS_ERR(gpio)) { + ret = gpiod_direction_output(gpio, 0); + if (ret) + return ret; + rfkill-wake_gpio = gpio; + } + } + /* Make sure at-least one of the GPIO is defined */ if (!rfkill-reset_gpio !rfkill-shutdown_gpio) { dev_err(dev, invalid platform data\n); @@ -182,6 +195,38 @@ static int rfkill_gpio_probe(struct platform_device *pdev) return 0; } +#ifdef CONFIG_PM +static int rfkill_gpio_suspend(struct device *dev) +{ + struct rfkill_gpio_data *rfkill = dev_get_drvdata(dev); + + dev_dbg(dev, Suspend\n); + + if (!rfkill-clk_enabled) + return 0; + + gpiod_set_value_cansleep(rfkill-wake_gpio, 0); + + return 0; +} + +static int rfkill_gpio_resume(struct device *dev) +{ + struct rfkill_gpio_data *rfkill = dev_get_drvdata(dev); + + dev_dbg(dev, Resume\n); + + if (!rfkill-clk_enabled) + return 0; + + gpiod_set_value_cansleep(rfkill-wake_gpio, 1); + + return 0; +} +#endif +static SIMPLE_DEV_PM_OPS(rfkill_gpio_pm, rfkill_gpio_suspend, +rfkill_gpio_resume); + static int rfkill_gpio_remove(struct platform_device *pdev) { struct rfkill_gpio_data *rfkill = platform_get_drvdata(pdev); @@ -197,12 +242,14 @@ static struct rfkill_gpio_desc acpi_default_bluetooth = { .type = RFKILL_TYPE_BLUETOOTH, .reset_idx = 0, .shutdown_idx = 1, + .wake_idx = -1, }; static struct rfkill_gpio_desc acpi_default_gps = { .type = RFKILL_TYPE_GPS, .reset_idx = 0, .shutdown_idx = 1, + .wake_idx = -1, }; static const struct acpi_device_id rfkill_acpi_match[] = { @@ -223,6 +270,7 @@ static struct platform_driver rfkill_gpio_driver = { .driver = { .name = rfkill_gpio, .owner = THIS_MODULE, + .pm = rfkill_gpio_pm, .acpi_match_table = ACPI_PTR(rfkill_acpi_match), }, }; -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] net: rfkill: gpio: Add BCM47521 ACPI ID
Add BCM47521 (GPS) to the ACPI table. BCM47521 supports host wake. HOST ---UART--- CONTROLLER HOST -- WAKE CONTROLLER (gpio res 0) HOST ---ENABLE-- CONTROLLER (gpio res 1) Signed-off-by: Loic Poulain loic.poul...@intel.com --- net/rfkill/rfkill-gpio.c | 9 + 1 file changed, 9 insertions(+) diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c index 28d1cbf..da239ef 100644 --- a/net/rfkill/rfkill-gpio.c +++ b/net/rfkill/rfkill-gpio.c @@ -309,6 +309,14 @@ static struct rfkill_gpio_desc acpi_bluetooth_wake = { .host_wake_idx = 2, }; +static struct rfkill_gpio_desc acpi_gps_wake = { + .type = RFKILL_TYPE_GPS, + .reset_idx = -1, + .shutdown_idx = 1, + .wake_idx = -1, + .host_wake_idx = 0, +}; + static const struct acpi_device_id rfkill_acpi_match[] = { { BCM2E1A, (kernel_ulong_t)acpi_default_bluetooth }, { BCM2E39, (kernel_ulong_t)acpi_default_bluetooth }, @@ -317,6 +325,7 @@ static const struct acpi_device_id rfkill_acpi_match[] = { { OBDA8723, (kernel_ulong_t)acpi_bluetooth_wake }, { BCM4752, (kernel_ulong_t)acpi_default_gps }, { LNV4752, (kernel_ulong_t)acpi_default_gps }, + { BCM47521, (kernel_ulong_t)acpi_gps_wake }, { }, }; MODULE_DEVICE_TABLE(acpi, rfkill_acpi_match); -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] net: rfkill: gpio: Configurable GPIO idx
On Wed, 2014-10-08 at 10:34 +0200, Loic Poulain wrote: + { BCM2E1A, (kernel_ulong_t)acpi_default_bluetooth }, Shouldn't that be uintptr_t? johannes -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ath10k: fix kernel panic while shutting down AP
The commit ath10k: workaround fw beaconing bug is freeing DMA-coherent memory in irq context which is hitting BUG ON in ARM platforms. Fix this by moving dma_free out of spin lock. kernel BUG at mm/vmalloc.c:1512! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM CPU: 0 PID: 722 Comm: hostapd Not tainted 3.14.0 #3 task: dd58b840 ti: da6a6000 task.ti: da6a6000 PC is at vunmap+0x24/0x34 LR is at __arm_dma_free.isra.21+0x12c/0x190 [c02a97d0] (vunmap) from [c021f81c] (__arm_dma_free.isra.21+0x12c/0x190) [c021f81c] (__arm_dma_free.isra.21) from [bf3b2440] (ath10k_mac_vif_beacon_free+0xf4/0x100 [ath10k_core]) [bf3b2440] (ath10k_mac_vif_beacon_free [ath10k_core]) from [bf3b2490] (ath10k_remove_interface+0x44/0x1ec [ath10k_core]) [bf3b2490] (ath10k_remove_interface [ath10k_core]) from [bf3352e4] (ieee80211_add_virtual_monitor+0x9d8/0x9f0 [mac80211]) [bf3352e4] (ieee80211_add_virtual_monitor [mac80211]) from [bf33530c] (ieee80211_stop+0x10/0x18 [mac80211]) [bf33530c] (ieee80211_stop [mac80211]) from [c040d144] (__dev_close_many+0x9c/0xcc) Cc: Michal Kazior michal.kaz...@tieto.com Signed-off-by: Rajkumar Manoharan rmano...@qti.qualcomm.com --- drivers/net/wireless/ath/ath10k/mac.c | 32 +++- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index bf8333c..dd4f56a 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -498,21 +498,6 @@ void ath10k_mac_vif_beacon_free(struct ath10k_vif *arvif) arvif-beacon_sent = false; } -static void ath10k_mac_vif_beacon_cleanup(struct ath10k_vif *arvif) -{ - struct ath10k *ar = arvif-ar; - - lockdep_assert_held(ar-data_lock); - - ath10k_mac_vif_beacon_free(arvif); - - if (arvif-beacon_buf) { - dma_free_coherent(ar-dev, IEEE80211_MAX_FRAME_LEN, - arvif-beacon_buf, arvif-beacon_paddr); - arvif-beacon_buf = NULL; - } -} - static inline int ath10k_vdev_setup_sync(struct ath10k *ar) { int ret; @@ -2404,8 +2389,15 @@ void ath10k_halt(struct ath10k *ar) spin_lock_bh(ar-data_lock); list_for_each_entry(arvif, ar-arvifs, list) - ath10k_mac_vif_beacon_cleanup(arvif); + ath10k_mac_vif_beacon_free(arvif); spin_unlock_bh(ar-data_lock); + list_for_each_entry(arvif, ar-arvifs, list) { + if (!arvif-beacon_buf) + continue; + dma_free_coherent(ar-dev, IEEE80211_MAX_FRAME_LEN, + arvif-beacon_buf, arvif-beacon_paddr); + arvif-beacon_buf = NULL; + } } static int ath10k_get_antenna(struct ieee80211_hw *hw, u32 *tx_ant, u32 *rx_ant) @@ -2988,9 +2980,15 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw, cancel_work_sync(arvif-wep_key_work); spin_lock_bh(ar-data_lock); - ath10k_mac_vif_beacon_cleanup(arvif); + ath10k_mac_vif_beacon_free(arvif); spin_unlock_bh(ar-data_lock); + if (arvif-beacon_buf) { + dma_free_coherent(ar-dev, IEEE80211_MAX_FRAME_LEN, + arvif-beacon_buf, arvif-beacon_paddr); + arvif-beacon_buf = NULL; + } + ret = ath10k_spectral_vif_stop(arvif); if (ret) ath10k_warn(ar, failed to stop spectral for vdev %i: %d\n, -- 2.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] net: rfkill: gpio: Configurable GPIO idx
Hi Johannes, acpi_device_id driver_data field is defined as a kernel_ulong_t in mod_devicetable.h. Regards, Loic On 08/10/2014 10:53, Johannes Berg wrote: On Wed, 2014-10-08 at 10:34 +0200, Loic Poulain wrote: + { BCM2E1A, (kernel_ulong_t)acpi_default_bluetooth }, Shouldn't that be uintptr_t? johannes -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Intel Open Source Technology Center http://oss.intel.com/ -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: fix kernel panic while shutting down AP
On 8 October 2014 11:16, Rajkumar Manoharan rmano...@qti.qualcomm.com wrote: The commit ath10k: workaround fw beaconing bug is freeing DMA-coherent memory in irq context which is hitting BUG ON in ARM platforms. Fix this by moving dma_free out of spin lock. I hardly see how moving the freeing outside the spinlock is a fix. kernel BUG at mm/vmalloc.c:1512! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM CPU: 0 PID: 722 Comm: hostapd Not tainted 3.14.0 #3 task: dd58b840 ti: da6a6000 task.ti: da6a6000 PC is at vunmap+0x24/0x34 LR is at __arm_dma_free.isra.21+0x12c/0x190 [c02a97d0] (vunmap) from [c021f81c] (__arm_dma_free.isra.21+0x12c/0x190) [c021f81c] (__arm_dma_free.isra.21) from [bf3b2440] (ath10k_mac_vif_beacon_free+0xf4/0x100 [ath10k_core]) [bf3b2440] (ath10k_mac_vif_beacon_free [ath10k_core]) from [bf3b2490] (ath10k_remove_interface+0x44/0x1ec [ath10k_core]) [bf3b2490] (ath10k_remove_interface [ath10k_core]) from [bf3352e4] (ieee80211_add_virtual_monitor+0x9d8/0x9f0 [mac80211]) [bf3352e4] (ieee80211_add_virtual_monitor [mac80211]) from [bf33530c] (ieee80211_stop+0x10/0x18 [mac80211]) [bf33530c] (ieee80211_stop [mac80211]) from [c040d144] (__dev_close_many+0x9c/0xcc) 1. How can even ieee80211_add_virtual_monitor() call ath10k_remove_interface()? Upstream ath10k doesn't advertise IEEE80211_HW_WANT_MONITOR_VIF. This call trace is either invalid, you're not using upstream ath10k and/or have custom patches applied to ath10k. 2. How can ieee80211_stop() be called from an interrupt context anyway? ieee80211_stop() calls ieee80211_do_stop() which calls ieee80211_roc_purge() which tries to get a hold of local-mtx. This implies ieee80211_stop() isn't design to be run in an interrupt context to begin with so I don't see why ath10k should even care if ath10k_remove_interface() is called in an interrupt context at this point. Michał -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: fix kernel panic while shutting down AP
Rajkumar Manoharan rmano...@qti.qualcomm.com writes: The commit ath10k: workaround fw beaconing bug is freeing DMA-coherent memory in irq context which is hitting BUG ON in ARM platforms. Fix this by moving dma_free out of spin lock. kernel BUG at mm/vmalloc.c:1512! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM CPU: 0 PID: 722 Comm: hostapd Not tainted 3.14.0 #3 task: dd58b840 ti: da6a6000 task.ti: da6a6000 PC is at vunmap+0x24/0x34 LR is at __arm_dma_free.isra.21+0x12c/0x190 [c02a97d0] (vunmap) from [c021f81c] (__arm_dma_free.isra.21+0x12c/0x190) [c021f81c] (__arm_dma_free.isra.21) from [bf3b2440] (ath10k_mac_vif_beacon_free+0xf4/0x100 [ath10k_core]) [bf3b2440] (ath10k_mac_vif_beacon_free [ath10k_core]) from [bf3b2490] (ath10k_remove_interface+0x44/0x1ec [ath10k_core]) [bf3b2490] (ath10k_remove_interface [ath10k_core]) from [bf3352e4] (ieee80211_add_virtual_monitor+0x9d8/0x9f0 [mac80211]) [bf3352e4] (ieee80211_add_virtual_monitor [mac80211]) from [bf33530c] (ieee80211_stop+0x10/0x18 [mac80211]) [bf33530c] (ieee80211_stop [mac80211]) from [c040d144] (__dev_close_many+0x9c/0xcc) Cc: Michal Kazior michal.kaz...@tieto.com Signed-off-by: Rajkumar Manoharan rmano...@qti.qualcomm.com [...] @@ -2404,8 +2389,15 @@ void ath10k_halt(struct ath10k *ar) spin_lock_bh(ar-data_lock); list_for_each_entry(arvif, ar-arvifs, list) - ath10k_mac_vif_beacon_cleanup(arvif); + ath10k_mac_vif_beacon_free(arvif); spin_unlock_bh(ar-data_lock); + list_for_each_entry(arvif, ar-arvifs, list) { + if (!arvif-beacon_buf) + continue; + dma_free_coherent(ar-dev, IEEE80211_MAX_FRAME_LEN, + arvif-beacon_buf, arvif-beacon_paddr); + arvif-beacon_buf = NULL; + } } Until now we have protected arvif-beacon_buf with data_lock. How do we know that this is safe to do without taking data_lock? -- Kalle Valo -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [rt2x00-users] MediaTek Inc. MT7601U Wireless Adapter
On Tue, Oct 07, 2014 at 05:12:47PM +0200, poma wrote: BTW, pán Gruszka, when will these devices be supported as a part of the upstream Linux kernel? I'm not an oracle :-) Larry declared to work on some mt76xx chip support, I'm cc him. I can also work on new chips for rt2x00, but wanted to finish some other work before. Stanislaw -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ath10k: add tracing for frame transmission
Rajkumar Manoharan rmano...@qti.qualcomm.com writes: + trace_ath10k_htt_rx_pop_msdu(ar, msdu-data, msdu-len + + skb_tailroom(msdu)); Why add skb_tailroom() to the length? I think that deserves a comment. It is used for rx crypto length. skb_tailroom is used in most of the places even while dumping rx pkt by ath10k_dbg_dump. Ok, thanks for clarifying that. -- Kalle Valo -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: warn on unhandled htt events
Michal Kazior michal.kaz...@tieto.com writes: It makes a lot more sense to print these kinds of problems as a warning instead of a debug. Signed-off-by: Michal Kazior michal.kaz...@tieto.com Thanks, applied. -- Kalle Valo -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: don't create bssid peer for ibss
Janusz Dziedzic janusz.dzied...@tieto.com writes: It's not really necessary to create bssid peer for bssid. Self-address peer is sufficient. This prevents some firmware revisions from crashing. Signed-off-by: Janusz Dziedzic janusz.dzied...@tieto.com Thanks, applied. -- Kalle Valo -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: advertise all possible firmware(-api) files
Bartosz Markowski bartosz.markow...@tieto.com writes: This is required if we take into account possibility to load the driver from initrd (RAM disk), so in other words: very early in the boot process, before the file system is visible. In such case we need to have the firmware files accessible from ram disk too, and this patch guarantee this. Signed-off-by: Bartosz Markowski bartosz.markow...@tieto.com Thanks, applied. -- Kalle Valo -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: fix WMI scan command length
Janusz Dziedzic janusz.dzied...@tieto.com writes: Fix WMI scan command length we setup when scan request. This fix issue with 636 firmware when scan always failed with message: ath10k_pci :02:00.0: wmi start scan ath10k_pci :02:00.0: wmi stop scan reqid 1 req_type 0 vdev/scan_id 0 ath10k_pci :02:00.0: failed to stop wmi scan: -11 ath10k_pci :02:00.0: failed to stop scan: -11 ath10k_pci :02:00.0: failed to start hw scan: -110 Signed-off-by: Janusz Dziedzic janusz.dzied...@tieto.com Thanks, applied. -- Kalle Valo -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] net: rfkill: gpio: Configurable GPIO idx
On Wed, Oct 08, 2014 at 10:34:36AM +0200, Loic Poulain wrote: Some devices don't match the current static gpio mapping: -BCM4752 (GPS) uses gpio res 0 as host wake irq. -OBDA8723 (Bluetooth) uses gpio res 0 as controller wake gpio and res 2 as host wake irq. To allow more flexibility, this patch introduces an index map description. By default, legacy config still used (reset: idx 0; shutdown: idx 1). Signed-off-by: Loic Poulain loic.poul...@intel.com --- net/rfkill/rfkill-gpio.c | 125 +-- 1 file changed, 88 insertions(+), 37 deletions(-) NAK I'm afraid the order of the GPIOs is platform specific. This would break the systems already out in the market. You need to fix your firmware to generate ACPI DSDT according to ACPI spec. v5.1 [1], which introduces the Device Specific Data (_DSD) and dt like device properties. We need to be able to request the GPIO based on the label, not the index, with all new platforms. Here is an example of the type of_ASL you need to have for the device.. ... Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings { Name (BBUF, ResourceTemplate () { UartSerialBus (0x0001C200, DataBitsEight, StopBitsOne, 0xFC, LittleEndian, ParityTypeNone, FlowControlNone, 0x0020, 0x0020, \\_SB.PCI0.URT1, 0x00, ResourceConsumer, , ) GpioInt (Level, ActiveLow, Exclusive, PullNone, 0x, \\_SB.GPO3, 0x00, ResourceConsumer, , ) { // Pin list 0x003C } GpioIo (Exclusive, PullDefault, 0x, 0x, IoRestrictionOutputOnly, \\_SB.GPO3, 0x00, ResourceConsumer, , ) { // Pin list 0x003E } GpioIo (Exclusive, PullDefault, 0x, 0x, IoRestrictionOutputOnly, \\_SB.GPO3, 0x00, ResourceConsumer, , ) { // Pin list 0x0040 } }) Return (BBUF) /* \_SB_.PCI0.URT1.BTH1._CRS.BBUF */ } Name (_DSD, Package () { ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301), Package () { Package () {reset-gpio, Package () {^BTH1, 1, 0, 0}}, Package () {shutdown-gpio, Package () {^BTH1, 2, 0, 0}}, } }) ... That will make sure you get the correct GPIO the moment we get support for the ACPI properties [2], regardless of the index. So there is no need for any changes to the driver. [1] http://www.uefi.org/sites/default/files/resources/ACPI_5_1release.pdf [2] https://lkml.org/lkml/2014/9/16/229 -- heikki -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: fix kernel panic while shutting down AP
On Wed, Oct 08, 2014 at 11:45:38AM +0200, Michal Kazior wrote: On 8 October 2014 11:16, Rajkumar Manoharan rmano...@qti.qualcomm.com wrote: The commit ath10k: workaround fw beaconing bug is freeing DMA-coherent memory in irq context which is hitting BUG ON in ARM platforms. Fix this by moving dma_free out of spin lock. I hardly see how moving the freeing outside the spinlock is a fix. kernel BUG at mm/vmalloc.c:1512! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM CPU: 0 PID: 722 Comm: hostapd Not tainted 3.14.0 #3 task: dd58b840 ti: da6a6000 task.ti: da6a6000 PC is at vunmap+0x24/0x34 LR is at __arm_dma_free.isra.21+0x12c/0x190 [c02a97d0] (vunmap) from [c021f81c] (__arm_dma_free.isra.21+0x12c/0x190) [c021f81c] (__arm_dma_free.isra.21) from [bf3b2440] (ath10k_mac_vif_beacon_free+0xf4/0x100 [ath10k_core]) [bf3b2440] (ath10k_mac_vif_beacon_free [ath10k_core]) from [bf3b2490] (ath10k_remove_interface+0x44/0x1ec [ath10k_core]) [bf3b2490] (ath10k_remove_interface [ath10k_core]) from [bf3352e4] (ieee80211_add_virtual_monitor+0x9d8/0x9f0 [mac80211]) [bf3352e4] (ieee80211_add_virtual_monitor [mac80211]) from [bf33530c] (ieee80211_stop+0x10/0x18 [mac80211]) [bf33530c] (ieee80211_stop [mac80211]) from [c040d144] (__dev_close_many+0x9c/0xcc) 1. How can even ieee80211_add_virtual_monitor() call ath10k_remove_interface()? Upstream ath10k doesn't advertise IEEE80211_HW_WANT_MONITOR_VIF. This call trace is either invalid, you're not using upstream ath10k and/or have custom patches applied to ath10k. This is the backtrace captured on panic and we are getting the same backtrace consistently. I confirmed that add_virtual_monitor is not called for ath10k as it is not advertising. ath10k_remove_interface is called for master mode. 2. How can ieee80211_stop() be called from an interrupt context anyway? ieee80211_stop() calls ieee80211_do_stop() which calls ieee80211_roc_purge() which tries to get a hold of local-mtx. This implies ieee80211_stop() isn't design to be run in an interrupt context to begin with so I don't see why ath10k should even care if ath10k_remove_interface() is called in an interrupt context at this point. in_interrupt is counting soft and hard irqs. ieee80211_stop is not called from interrupt context. In ath10k, by aquiring spin_lock in ath10k_mac_vif_beacon_free is increasing soft irq count. In ARM arch, __arm_dma_free is calling vunmap which might sleep. So it can not be called within spin_lock. Similar to dma_alloc_coherent, dma_free_coherent can not be called under soft irq context. PS: I am using upstream ath10k driver without private changes. -Rajkumar -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] staging: rtl8723au: incorrect use of ether_addr_copy()
The return from myid() isn't aligned correctly for ether_addr_copy(). Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c index 3eb77de..c8f7890 100644 --- a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c +++ b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c @@ -2402,7 +2402,7 @@ void issue_beacon23a(struct rtw_adapter *padapter, int timeout_ms) mgmt-seq_ctrl = 0; ether_addr_copy(mgmt-da, bc_addr); - ether_addr_copy(mgmt-sa, myid(padapter-eeprompriv)); + memcpy(mgmt-sa, myid(padapter-eeprompriv), ETH_ALEN); ether_addr_copy(mgmt-bssid, get_my_bssid23a(cur_network)); /* timestamp will be inserted by hardware */ @@ -2556,7 +2556,7 @@ static void issue_probersp(struct rtw_adapter *padapter, unsigned char *da, cpu_to_le16(IEEE80211_FTYPE_MGMT | IEEE80211_STYPE_PROBE_RESP); ether_addr_copy(mgmt-da, da); - ether_addr_copy(mgmt-sa, mac); + memcpy(mgmt-sa, mac, ETH_ALEN); ether_addr_copy(mgmt-bssid, bssid); mgmt-seq_ctrl = cpu_to_le16(IEEE80211_SN_TO_SEQ(pmlmeext-mgnt_seq)); @@ -2718,7 +2718,7 @@ static int _issue_probereq(struct rtw_adapter *padapter, ether_addr_copy(pwlanhdr-addr3, bc_addr); } - ether_addr_copy(pwlanhdr-addr2, mac); + memcpy(pwlanhdr-addr2, mac, ETH_ALEN); pwlanhdr-seq_ctrl = cpu_to_le16(IEEE80211_SN_TO_SEQ(pmlmeext-mgnt_seq)); @@ -2863,8 +2863,8 @@ static void issue_auth(struct rtw_adapter *padapter, struct sta_info *psta, #ifdef CONFIG_8723AU_AP_MODE unsigned short val16; ether_addr_copy(mgmt-da, psta-hwaddr); - ether_addr_copy(mgmt-sa, myid(padapter-eeprompriv)); - ether_addr_copy(mgmt-bssid, myid(padapter-eeprompriv)); + memcpy(mgmt-sa, myid(padapter-eeprompriv), ETH_ALEN); + memcpy(mgmt-bssid, myid(padapter-eeprompriv), ETH_ALEN); /* setting auth algo number */ val16 = (u16)psta-authalg; @@ -2895,7 +2895,7 @@ static void issue_auth(struct rtw_adapter *padapter, struct sta_info *psta, struct ieee80211_mgmt *iv_mgmt; ether_addr_copy(mgmt-da, get_my_bssid23a(pmlmeinfo-network)); - ether_addr_copy(mgmt-sa, myid(padapter-eeprompriv)); + memcpy(mgmt-sa, myid(padapter-eeprompriv), ETH_ALEN); ether_addr_copy(mgmt-bssid, get_my_bssid23a(pmlmeinfo-network)); @@ -3006,7 +3006,7 @@ static void issue_assocrsp(struct rtw_adapter *padapter, unsigned short status, mgmt-frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT | pkt_type); ether_addr_copy(mgmt-da, pstat-hwaddr); - ether_addr_copy(mgmt-sa, myid(padapter-eeprompriv)); + memcpy(mgmt-sa, myid(padapter-eeprompriv), ETH_ALEN); ether_addr_copy(mgmt-bssid, get_my_bssid23a(pmlmeinfo-network)); mgmt-seq_ctrl = cpu_to_le16(IEEE80211_SN_TO_SEQ(pmlmeext-mgnt_seq)); @@ -3129,7 +3129,7 @@ static void issue_assocreq(struct rtw_adapter *padapter) cpu_to_le16(IEEE80211_FTYPE_MGMT | IEEE80211_STYPE_ASSOC_REQ); ether_addr_copy(mgmt-da, get_my_bssid23a(pmlmeinfo-network)); - ether_addr_copy(mgmt-sa, myid(padapter-eeprompriv)); + memcpy(mgmt-sa, myid(padapter-eeprompriv), ETH_ALEN); ether_addr_copy(mgmt-bssid, get_my_bssid23a(pmlmeinfo-network)); mgmt-seq_ctrl = cpu_to_le16(IEEE80211_SN_TO_SEQ(pmlmeext-mgnt_seq)); @@ -3400,7 +3400,7 @@ static int _issue_nulldata23a(struct rtw_adapter *padapter, unsigned char *da, pwlanhdr-frame_control |= cpu_to_le16(IEEE80211_FCTL_PM); ether_addr_copy(pwlanhdr-addr1, da); - ether_addr_copy(pwlanhdr-addr2, myid(padapter-eeprompriv)); + memcpy(pwlanhdr-addr2, myid(padapter-eeprompriv), ETH_ALEN); ether_addr_copy(pwlanhdr-addr3, get_my_bssid23a(pmlmeinfo-network)); pwlanhdr-seq_ctrl = @@ -3528,7 +3528,7 @@ static int _issue_qos_nulldata23a(struct rtw_adapter *padapter, pwlanhdr-qos_ctrl |= cpu_to_le16(IEEE80211_QOS_CTL_EOSP); ether_addr_copy(pwlanhdr-addr1, da); - ether_addr_copy(pwlanhdr-addr2, myid(padapter-eeprompriv)); + memcpy(pwlanhdr-addr2, myid(padapter-eeprompriv), ETH_ALEN); ether_addr_copy(pwlanhdr-addr3, get_my_bssid23a(pmlmeinfo-network)); pwlanhdr-seq_ctrl = @@ -3633,7 +3633,7 @@ static int _issue_deauth(struct rtw_adapter *padapter, unsigned char *da, cpu_to_le16(IEEE80211_FTYPE_MGMT | IEEE80211_STYPE_DEAUTH); ether_addr_copy(mgmt-da, da); - ether_addr_copy(mgmt-sa, myid(padapter-eeprompriv)); + memcpy(mgmt-sa, myid(padapter-eeprompriv), ETH_ALEN); ether_addr_copy(mgmt-bssid, get_my_bssid23a(pmlmeinfo-network)); mgmt-seq_ctrl =
Re: [PATCH] ath10k: fix kernel panic while shutting down AP
On Wed, Oct 08, 2014 at 12:52:04PM +0300, Kalle Valo wrote: Rajkumar Manoharan rmano...@qti.qualcomm.com writes: The commit ath10k: workaround fw beaconing bug is freeing DMA-coherent memory in irq context which is hitting BUG ON in ARM platforms. Fix this by moving dma_free out of spin lock. [...] @@ -2404,8 +2389,15 @@ void ath10k_halt(struct ath10k *ar) spin_lock_bh(ar-data_lock); list_for_each_entry(arvif, ar-arvifs, list) - ath10k_mac_vif_beacon_cleanup(arvif); + ath10k_mac_vif_beacon_free(arvif); spin_unlock_bh(ar-data_lock); + list_for_each_entry(arvif, ar-arvifs, list) { + if (!arvif-beacon_buf) + continue; + dma_free_coherent(ar-dev, IEEE80211_MAX_FRAME_LEN, + arvif-beacon_buf, arvif-beacon_paddr); + arvif-beacon_buf = NULL; + } } Until now we have protected arvif-beacon_buf with data_lock. How do we know that this is safe to do without taking data_lock? As said, spin_lock can not be used for dma_free_coherent. arvif-beacon_buf is already protected by conf_mutex. At this state in ath10k_halt path, no one can access beacon_buf. So mutex lock itself is sufficient. -Rajkumar -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: fix kernel panic while shutting down AP
On Wed, Oct 08, 2014 at 12:50:28PM +0200, Michal Kazior wrote: On 8 October 2014 12:33, Rajkumar Manoharan rmano...@qti.qualcomm.com wrote: On Wed, Oct 08, 2014 at 11:45:38AM +0200, Michal Kazior wrote: On 8 October 2014 11:16, Rajkumar Manoharan rmano...@qti.qualcomm.com wrote: The commit ath10k: workaround fw beaconing bug is freeing DMA-coherent memory in irq context which is hitting BUG ON in ARM platforms. Fix this by moving dma_free out of spin lock. I hardly see how moving the freeing outside the spinlock is a fix. kernel BUG at mm/vmalloc.c:1512! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM CPU: 0 PID: 722 Comm: hostapd Not tainted 3.14.0 #3 task: dd58b840 ti: da6a6000 task.ti: da6a6000 PC is at vunmap+0x24/0x34 LR is at __arm_dma_free.isra.21+0x12c/0x190 [c02a97d0] (vunmap) from [c021f81c] (__arm_dma_free.isra.21+0x12c/0x190) [c021f81c] (__arm_dma_free.isra.21) from [bf3b2440] (ath10k_mac_vif_beacon_free+0xf4/0x100 [ath10k_core]) [bf3b2440] (ath10k_mac_vif_beacon_free [ath10k_core]) from [bf3b2490] (ath10k_remove_interface+0x44/0x1ec [ath10k_core]) [bf3b2490] (ath10k_remove_interface [ath10k_core]) from [bf3352e4] (ieee80211_add_virtual_monitor+0x9d8/0x9f0 [mac80211]) [bf3352e4] (ieee80211_add_virtual_monitor [mac80211]) from [bf33530c] (ieee80211_stop+0x10/0x18 [mac80211]) [bf33530c] (ieee80211_stop [mac80211]) from [c040d144] (__dev_close_many+0x9c/0xcc) 1. How can even ieee80211_add_virtual_monitor() call ath10k_remove_interface()? Upstream ath10k doesn't advertise IEEE80211_HW_WANT_MONITOR_VIF. This call trace is either invalid, you're not using upstream ath10k and/or have custom patches applied to ath10k. This is the backtrace captured on panic and we are getting the same backtrace consistently. I confirmed that add_virtual_monitor is not called for ath10k as it is not advertising. ath10k_remove_interface is called for master mode. 2. How can ieee80211_stop() be called from an interrupt context anyway? ieee80211_stop() calls ieee80211_do_stop() which calls ieee80211_roc_purge() which tries to get a hold of local-mtx. This implies ieee80211_stop() isn't design to be run in an interrupt context to begin with so I don't see why ath10k should even care if ath10k_remove_interface() is called in an interrupt context at this point. in_interrupt is counting soft and hard irqs. ieee80211_stop is not called from interrupt context. In ath10k, by aquiring spin_lock in ath10k_mac_vif_beacon_free is increasing soft irq count. In ARM arch, __arm_dma_free is calling vunmap which might sleep. So it can not be called within spin_lock. Did you try using GFP_ATOMIC in the dma_alloc_coherent instead of moving the spinlock? Nope. The problem is while freeing dma memory not during allocation. dma_free_coherent won't take any GFP_* flags. Similar to dma_alloc_coherent, dma_free_coherent can not be called under soft irq context. The call trace points to ath10k_mac_vif_beacon_free() which doesn't use dma_free_coherent() so why are you blaming it for the BUG_ON? I agree the calltrace is a bogus. ath10k_mac_vif_beacon_cleanup is calling dma_free_coherent. If anything the offender should be dma_unmap_single() but the thing is beacon_buf is always allocated for AP/IBSS now which means dma_unmap_single() is never called. For non-AP/IBSS both arvif-beacon and arvif-beacon_buf are always NULL so neither dma_alloc/free_coherent nor dma_map/unmap_single are called. Agree. We need one more check in ath10k_mac_vif_beacon_free. But here the problem is dma_free_coherent is being called from soft irq context which is not allowed (BUG_ON noted in ARM arch). After moving dma_free_coherent out of spinlock, the kernel BUG at mm/vmalloc.c:1512 is never hit. -Rajkumar -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: fix kernel panic while shutting down AP
On Wed, Oct 08, 2014 at 04:38:44PM +0530, Rajkumar Manoharan wrote: On Wed, Oct 08, 2014 at 12:50:28PM +0200, Michal Kazior wrote: On 8 October 2014 12:33, Rajkumar Manoharan rmano...@qti.qualcomm.com wrote: On Wed, Oct 08, 2014 at 11:45:38AM +0200, Michal Kazior wrote: On 8 October 2014 11:16, Rajkumar Manoharan rmano...@qti.qualcomm.com wrote: The commit ath10k: workaround fw beaconing bug is freeing DMA-coherent memory in irq context which is hitting BUG ON in ARM platforms. Fix this by moving dma_free out of spin lock. I hardly see how moving the freeing outside the spinlock is a fix. kernel BUG at mm/vmalloc.c:1512! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM CPU: 0 PID: 722 Comm: hostapd Not tainted 3.14.0 #3 task: dd58b840 ti: da6a6000 task.ti: da6a6000 PC is at vunmap+0x24/0x34 LR is at __arm_dma_free.isra.21+0x12c/0x190 [c02a97d0] (vunmap) from [c021f81c] (__arm_dma_free.isra.21+0x12c/0x190) [c021f81c] (__arm_dma_free.isra.21) from [bf3b2440] (ath10k_mac_vif_beacon_free+0xf4/0x100 [ath10k_core]) [bf3b2440] (ath10k_mac_vif_beacon_free [ath10k_core]) from [bf3b2490] (ath10k_remove_interface+0x44/0x1ec [ath10k_core]) [bf3b2490] (ath10k_remove_interface [ath10k_core]) from [bf3352e4] (ieee80211_add_virtual_monitor+0x9d8/0x9f0 [mac80211]) [bf3352e4] (ieee80211_add_virtual_monitor [mac80211]) from [bf33530c] (ieee80211_stop+0x10/0x18 [mac80211]) [bf33530c] (ieee80211_stop [mac80211]) from [c040d144] (__dev_close_many+0x9c/0xcc) 1. How can even ieee80211_add_virtual_monitor() call ath10k_remove_interface()? Upstream ath10k doesn't advertise IEEE80211_HW_WANT_MONITOR_VIF. This call trace is either invalid, you're not using upstream ath10k and/or have custom patches applied to ath10k. This is the backtrace captured on panic and we are getting the same backtrace consistently. I confirmed that add_virtual_monitor is not called for ath10k as it is not advertising. ath10k_remove_interface is called for master mode. 2. How can ieee80211_stop() be called from an interrupt context anyway? ieee80211_stop() calls ieee80211_do_stop() which calls ieee80211_roc_purge() which tries to get a hold of local-mtx. This implies ieee80211_stop() isn't design to be run in an interrupt context to begin with so I don't see why ath10k should even care if ath10k_remove_interface() is called in an interrupt context at this point. in_interrupt is counting soft and hard irqs. ieee80211_stop is not called from interrupt context. In ath10k, by aquiring spin_lock in ath10k_mac_vif_beacon_free is increasing soft irq count. In ARM arch, __arm_dma_free is calling vunmap which might sleep. So it can not be called within spin_lock. Did you try using GFP_ATOMIC in the dma_alloc_coherent instead of moving the spinlock? Nope. The problem is while freeing dma memory not during allocation. dma_free_coherent won't take any GFP_* flags. Similar to dma_alloc_coherent, dma_free_coherent can not be called under soft irq context. The call trace points to ath10k_mac_vif_beacon_free() which doesn't use dma_free_coherent() so why are you blaming it for the BUG_ON? I agree the calltrace is a bogus. ath10k_mac_vif_beacon_cleanup is calling dma_free_coherent. If anything the offender should be dma_unmap_single() but the thing is beacon_buf is always allocated for AP/IBSS now which means dma_unmap_single() is never called. For non-AP/IBSS both arvif-beacon and arvif-beacon_buf are always NULL so neither dma_alloc/free_coherent nor dma_map/unmap_single are called. Agree. We need one more check in ath10k_mac_vif_beacon_free. No additional check is needed. For non beaconing mode, arvif-beacon should be false. isn't it? -Rajkumar -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rt2x00: rt2x00queue: avoid using more headroom then driver requested
On Sun, Oct 5, 2014 at 4:39 AM, Helmut Schaa helmut.sc...@googlemail.com wrote: Mark Asselstine asse...@gmail.com schrieb: On Wed, Oct 1, 2014 at 9:42 PM, Mark Asselstine asse...@gmail.com wrote: Damn, you are right. I thought I had it licked. Unfortunately with the overloaded variable name it was easy to get turned around. The comments in the code didn't prevent me knotting myself up either. If you look in in rt2x00.h, struct rt2x00_dev, the comment above the extra_tx_headroom member says Extra TX headroom required for alignment purposes. I would say this is incorrect. This variable is initialized via rt2x00dev_extra_tx_headroom() with a combination of winfo_size and desc_size and has nothing to do with alignment. At any rate, keeping in mind that rt2x00_dev.hw.extra_tx_headroom which is used to set the amount of available headroom includes room for alignment and padding as well as winfo and desc space, and that rt2x00_dev.extra_tx_headroom doesn't include padding or alignment, you are correct in that we aren't over spending our headroom. Back to the drawing board. Mark On Wed, Oct 1, 2014 at 5:12 AM, Stanislaw Gruszka sgrus...@redhat.com wrote: On Tue, Sep 30, 2014 at 11:45:57PM -0400, Mark Asselstine wrote: 'struct ieee80211_hw' contains 'extra_tx_headroom' which it defines as headroom to reserve in each transmit skb for use by the driver. This value is properly setup during rt2x00lib_probe_hw() to account for all the rt2x00lib's purposes, including DMA alignment and L2 pad if needed. As such under all circumstances the proper amount of headroom is allocated to a skb such that under any usage we would not ever use more headroom then is allotted. However rt2x00queue_write_tx_frame() uses up the headroom (via calls to skb_push) allotted for L2 padding (with a potential call to rt2x00queue_insert_l2pad()) and uses up the headroom allotted to DMA alignment (with a potential call to rt2x00queue_align_frame()) and then proceeds to use up 'extra_tx_headroom' (in a call to rt2x00queue_write_tx_data()) So the driver has requested just 'extra_tx_headroom' worth of headroom and we have used up 'extra_tx_headroom' + DMA + L2PAD worth. As such it is possible to hit a 'skb_under_panic', where we have used up all the available headroom. Since extra_tx_headroom was calculated as a function of winfo_size, desc_size, RT2X00_L2PAD_SIZE and RT2X00_ALIGN_SIZE we can simply remove the part for alignment and padding and we will know how much is left to use up (for txdesc) and only use that much. Keeping the driver's use of headroom to what it requested via extra_tx_headroom. Signed-off-by: Mark Asselstine asse...@gmail.com --- drivers/net/wireless/rt2x00/rt2x00queue.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c index 8e68f87..2a48bf5 100644 --- a/drivers/net/wireless/rt2x00/rt2x00queue.c +++ b/drivers/net/wireless/rt2x00/rt2x00queue.c @@ -522,6 +522,7 @@ static int rt2x00queue_write_tx_data(struct queue_entry *entry, struct txentry_desc *txdesc) { struct rt2x00_dev *rt2x00dev = entry-queue-rt2x00dev; + unsigned int avail_extra_tx_headroom = rt2x00dev-extra_tx_headroom; /* * This should not happen, we already checked the entry @@ -538,10 +539,18 @@ static int rt2x00queue_write_tx_data(struct queue_entry *entry, } /* - * Add the requested extra tx headroom in front of the skb. + * Add room for data at the front of the buffer for txdesc. The space + * needed for this was accounted for in extra_tx_headroom, we just + * need to remove the amount allocated for padding and alignment + * to get what is left for txdesc. */ - skb_push(entry-skb, rt2x00dev-extra_tx_headroom); - memset(entry-skb-data, 0, rt2x00dev-extra_tx_headroom); + if (test_bit(REQUIRE_L2PAD, rt2x00dev-cap_flags)) + avail_extra_tx_headroom -= RT2X00_L2PAD_SIZE; + else if (test_bit(REQUIRE_DMA, rt2x00dev-cap_flags)) + avail_extra_tx_headroom -= RT2X00_ALIGN_SIZE; + + skb_push(entry-skb, avail_extra_tx_headroom); + memset(entry-skb-data, 0, avail_extra_tx_headroom); I don't think patch is correct. We have rt2x00-extra_tx_headroom and rt2x00-hw-extra_tx_headroom. Second value, which we provide as maximum needed headroom to mac80211 is rt2x00-extra_tx_headrom + RT2X00_L2PAD_SIZE + RT2X00_ALIGN_SIZE. We really need to reserve rt2x00dev-extra_tx_headroom on TX skb, as this is room for metadata needed by H/W and if needed we should reserve RT2x00_L2PAD_SIZE and RTX00_ALIGN_SIZE. There should be room for that, since we provide bigger rt2x00-hw-extra_tx_headroom to mac80211. The only possibility to skb_under_panic I can see, is
Re: [PATCH 3/5] net: rfkill: gpio: Implement host wake up support
On Wed, Oct 08, 2014 at 10:34:38AM +0200, Loic Poulain wrote: Some GPIO based rfkill devices can wake their host up from suspend by toggling an input (from the host perspective) GPIO. This patch adds a generic support for that feature by registering a threaded interrupt routine and thus setting the corresponding GPIO as a wake up source. Signed-off-by: Loic Poulain loic.poul...@intel.com --- net/rfkill/rfkill-gpio.c | 49 1 file changed, 49 insertions(+) To continue my previous answer, for this you could have the following _DSD.. Name (_DSD, Package () { ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301), Package () { Package () {host_wake-gpio, Package () {^BTH1, 0, 0, 0}}, Package () {reset-gpio, Package () {^BTH1, 1, 0, 0}}, Package () {shutdown-gpio, Package () {^BTH1, 2, 0, 0}}, } }) And in the driver you can then request it without caring about the index.. ... gpio = devm_gpiod_get_index(dev, host_wake, 3); if (!IS_ERR(gpio)) { ret = gpiod_direction_input(gpio); if (ret) return ret; rfkill-irq = gpiod_to_irq(gpio); ... Actually, we could just use devm_gpiod_get instead of devm_gpiod_get_index in this case. Cheers, -- heikki -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: fix kernel panic while shutting down AP
On 8 October 2014 13:08, Rajkumar Manoharan rmano...@qti.qualcomm.com wrote: On Wed, Oct 08, 2014 at 12:50:28PM +0200, Michal Kazior wrote: On 8 October 2014 12:33, Rajkumar Manoharan rmano...@qti.qualcomm.com wrote: On Wed, Oct 08, 2014 at 11:45:38AM +0200, Michal Kazior wrote: On 8 October 2014 11:16, Rajkumar Manoharan rmano...@qti.qualcomm.com wrote: The commit ath10k: workaround fw beaconing bug is freeing DMA-coherent memory in irq context which is hitting BUG ON in ARM platforms. Fix this by moving dma_free out of spin lock. I hardly see how moving the freeing outside the spinlock is a fix. kernel BUG at mm/vmalloc.c:1512! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM CPU: 0 PID: 722 Comm: hostapd Not tainted 3.14.0 #3 task: dd58b840 ti: da6a6000 task.ti: da6a6000 PC is at vunmap+0x24/0x34 LR is at __arm_dma_free.isra.21+0x12c/0x190 [c02a97d0] (vunmap) from [c021f81c] (__arm_dma_free.isra.21+0x12c/0x190) [c021f81c] (__arm_dma_free.isra.21) from [bf3b2440] (ath10k_mac_vif_beacon_free+0xf4/0x100 [ath10k_core]) [bf3b2440] (ath10k_mac_vif_beacon_free [ath10k_core]) from [bf3b2490] (ath10k_remove_interface+0x44/0x1ec [ath10k_core]) [bf3b2490] (ath10k_remove_interface [ath10k_core]) from [bf3352e4] (ieee80211_add_virtual_monitor+0x9d8/0x9f0 [mac80211]) [bf3352e4] (ieee80211_add_virtual_monitor [mac80211]) from [bf33530c] (ieee80211_stop+0x10/0x18 [mac80211]) [bf33530c] (ieee80211_stop [mac80211]) from [c040d144] (__dev_close_many+0x9c/0xcc) 1. How can even ieee80211_add_virtual_monitor() call ath10k_remove_interface()? Upstream ath10k doesn't advertise IEEE80211_HW_WANT_MONITOR_VIF. This call trace is either invalid, you're not using upstream ath10k and/or have custom patches applied to ath10k. This is the backtrace captured on panic and we are getting the same backtrace consistently. I confirmed that add_virtual_monitor is not called for ath10k as it is not advertising. ath10k_remove_interface is called for master mode. 2. How can ieee80211_stop() be called from an interrupt context anyway? ieee80211_stop() calls ieee80211_do_stop() which calls ieee80211_roc_purge() which tries to get a hold of local-mtx. This implies ieee80211_stop() isn't design to be run in an interrupt context to begin with so I don't see why ath10k should even care if ath10k_remove_interface() is called in an interrupt context at this point. in_interrupt is counting soft and hard irqs. ieee80211_stop is not called from interrupt context. In ath10k, by aquiring spin_lock in ath10k_mac_vif_beacon_free is increasing soft irq count. In ARM arch, __arm_dma_free is calling vunmap which might sleep. So it can not be called within spin_lock. Did you try using GFP_ATOMIC in the dma_alloc_coherent instead of moving the spinlock? Nope. The problem is while freeing dma memory not during allocation. dma_free_coherent won't take any GFP_* flags. My thinking was GFP_ given to dma_alloc_coherent could impact behaviour of dma_free_coherent (e.g. different kind of pages is allocated so different kind of freeing will be involved) but I may be wrong. Can you give it a try, please? Similar to dma_alloc_coherent, dma_free_coherent can not be called under soft irq context. The call trace points to ath10k_mac_vif_beacon_free() which doesn't use dma_free_coherent() so why are you blaming it for the BUG_ON? I agree the calltrace is a bogus. ath10k_mac_vif_beacon_cleanup is calling dma_free_coherent. The call trace without additional comment in the commit log (stating the fact that the call trace is inaccurate for some reason) is confusing. If anything the offender should be dma_unmap_single() but the thing is beacon_buf is always allocated for AP/IBSS now which means dma_unmap_single() is never called. For non-AP/IBSS both arvif-beacon and arvif-beacon_buf are always NULL so neither dma_alloc/free_coherent nor dma_map/unmap_single are called. Agree. We need one more check in ath10k_mac_vif_beacon_free. But here the problem is dma_free_coherent is being called from soft irq context which is not allowed (BUG_ON noted in ARM arch). After moving dma_free_coherent out of spinlock, the kernel BUG at mm/vmalloc.c:1512 is never hit. It seems there's a bit of branching in the arm dma code. Maybe it's possible to fix this problem in ath10k differently (vide the gfp idea). Michał -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ath10k: fix kernel panic while shutting down AP
On 8 October 2014 13:13, Rajkumar Manoharan rmano...@qti.qualcomm.com wrote: On Wed, Oct 08, 2014 at 04:38:44PM +0530, Rajkumar Manoharan wrote: On Wed, Oct 08, 2014 at 12:50:28PM +0200, Michal Kazior wrote: If anything the offender should be dma_unmap_single() but the thing is beacon_buf is always allocated for AP/IBSS now which means dma_unmap_single() is never called. For non-AP/IBSS both arvif-beacon and arvif-beacon_buf are always NULL so neither dma_alloc/free_coherent nor dma_map/unmap_single are called. Agree. We need one more check in ath10k_mac_vif_beacon_free. No additional check is needed. For non beaconing mode, arvif-beacon should be false. isn't it? Correct. No extra checks are necessary. Michał -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] rt2x00: tune multi-registers I/O timeout
We provide timeout value to rt2x00usb_vendor_request_buff() based on number of registers to process. That value is passed down to rt2x00usb_vendor_req_buff_lock() and ends in usb_control_msg(). But we do not read/write all registers in rt2x00usb_vendor_req_buff_lock() at once. We read/write them in chunks of 64 bytes in the loop, hence passed timeout value to low level is too big. Patch removes timeout argument from rt2x00usb_vendor_request_buff() and use short REGISTER_TIMEOUT in rt2x00usb_vendor_req_buff_lock(). That timeout value should be fine for 64 bytes and smaller requests. For EEPROM read we introduced new timeout value equal to 2 seconds. Patch fixes process uninterruptible sleep stalls for long period, when USB bus has problem to satisfy a request and we wait very long time on usb_start_wait_urb(). Reported-and-tested-by: Sakari Ailus sakari.ai...@iki.fi Signed-off-by: Stanislaw Gruszka sgrus...@redhat.com --- drivers/net/wireless/rt2x00/rt2500usb.c | 10 - drivers/net/wireless/rt2x00/rt2x00usb.c | 4 ++-- drivers/net/wireless/rt2x00/rt2x00usb.h | 39 ++--- 3 files changed, 17 insertions(+), 36 deletions(-) diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c index d849d59..c878e3f 100644 --- a/drivers/net/wireless/rt2x00/rt2500usb.c +++ b/drivers/net/wireless/rt2x00/rt2500usb.c @@ -62,7 +62,7 @@ static inline void rt2500usb_register_read(struct rt2x00_dev *rt2x00dev, __le16 reg; rt2x00usb_vendor_request_buff(rt2x00dev, USB_MULTI_READ, USB_VENDOR_REQUEST_IN, offset, - reg, sizeof(reg), REGISTER_TIMEOUT); + reg, sizeof(reg)); *value = le16_to_cpu(reg); } @@ -83,8 +83,7 @@ static inline void rt2500usb_register_multiread(struct rt2x00_dev *rt2x00dev, { rt2x00usb_vendor_request_buff(rt2x00dev, USB_MULTI_READ, USB_VENDOR_REQUEST_IN, offset, - value, length, - REGISTER_TIMEOUT16(length)); + value, length); } static inline void rt2500usb_register_write(struct rt2x00_dev *rt2x00dev, @@ -94,7 +93,7 @@ static inline void rt2500usb_register_write(struct rt2x00_dev *rt2x00dev, __le16 reg = cpu_to_le16(value); rt2x00usb_vendor_request_buff(rt2x00dev, USB_MULTI_WRITE, USB_VENDOR_REQUEST_OUT, offset, - reg, sizeof(reg), REGISTER_TIMEOUT); + reg, sizeof(reg)); } static inline void rt2500usb_register_write_lock(struct rt2x00_dev *rt2x00dev, @@ -113,8 +112,7 @@ static inline void rt2500usb_register_multiwrite(struct rt2x00_dev *rt2x00dev, { rt2x00usb_vendor_request_buff(rt2x00dev, USB_MULTI_WRITE, USB_VENDOR_REQUEST_OUT, offset, - value, length, - REGISTER_TIMEOUT16(length)); + value, length); } static int rt2500usb_regbusy_read(struct rt2x00_dev *rt2x00dev, diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c index 86c43d1..dc85d3e 100644 --- a/drivers/net/wireless/rt2x00/rt2x00usb.c +++ b/drivers/net/wireless/rt2x00/rt2x00usb.c @@ -116,7 +116,7 @@ EXPORT_SYMBOL_GPL(rt2x00usb_vendor_req_buff_lock); int rt2x00usb_vendor_request_buff(struct rt2x00_dev *rt2x00dev, const u8 request, const u8 requesttype, const u16 offset, void *buffer, - const u16 buffer_length, const int timeout) + const u16 buffer_length) { int status = 0; unsigned char *tb; @@ -131,7 +131,7 @@ int rt2x00usb_vendor_request_buff(struct rt2x00_dev *rt2x00dev, bsize = min_t(u16, CSR_CACHE_SIZE, len); status = rt2x00usb_vendor_req_buff_lock(rt2x00dev, request, requesttype, off, tb, - bsize, timeout); + bsize, REGISTER_TIMEOUT); tb += bsize; len -= bsize; diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.h b/drivers/net/wireless/rt2x00/rt2x00usb.h index 831b65f..819690e 100644 --- a/drivers/net/wireless/rt2x00/rt2x00usb.h +++ b/drivers/net/wireless/rt2x00/rt2x00usb.h @@ -33,27 +33,14 @@ }) /* - * For USB vendor requests we need to pass a timeout - * time in ms, for this we use the REGISTER_TIMEOUT, - * however when loading firmware a higher value is - * required. In that case we use the REGISTER_TIMEOUT_FIRMWARE. + * For USB vendor requests we need to pass a timeout
Re: [PATCH] ath10k: fix kernel panic while shutting down AP
On 8 October 2014 12:48, Rajkumar Manoharan rmano...@qti.qualcomm.com wrote: On Wed, Oct 08, 2014 at 12:52:04PM +0300, Kalle Valo wrote: Rajkumar Manoharan rmano...@qti.qualcomm.com writes: The commit ath10k: workaround fw beaconing bug is freeing DMA-coherent memory in irq context which is hitting BUG ON in ARM platforms. Fix this by moving dma_free out of spin lock. [...] @@ -2404,8 +2389,15 @@ void ath10k_halt(struct ath10k *ar) spin_lock_bh(ar-data_lock); list_for_each_entry(arvif, ar-arvifs, list) - ath10k_mac_vif_beacon_cleanup(arvif); + ath10k_mac_vif_beacon_free(arvif); spin_unlock_bh(ar-data_lock); + list_for_each_entry(arvif, ar-arvifs, list) { + if (!arvif-beacon_buf) + continue; + dma_free_coherent(ar-dev, IEEE80211_MAX_FRAME_LEN, + arvif-beacon_buf, arvif-beacon_paddr); + arvif-beacon_buf = NULL; + } } Until now we have protected arvif-beacon_buf with data_lock. How do we know that this is safe to do without taking data_lock? As said, spin_lock can not be used for dma_free_coherent. arvif-beacon_buf is already protected by conf_mutex. At this state in ath10k_halt path, no one can access beacon_buf. So mutex lock itself is sufficient. beacon_buf is protected by conf_mutex implicitly. It wasn't the main intent. It is protected with data_lock spinlock. Do not trust the device - if there's a spurious SWBA event while ath10k_remove_interface() is running you could end up with invalid memory access. It might be acceptable to drop the spinlock for ath10k_halt() since the device is guaranteed to be stopped at that point (effectively reset) though. Anyway I'm hoping this bug can be fixed with the gfp flag. Michał -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] staging: rtl8723au: incorrect use of ether_addr_copy()
On Wed, 2014-10-08 at 13:40 +0300, Dan Carpenter wrote: The return from myid() isn't aligned correctly for ether_addr_copy(). Hey Dan. Actual evidence showing ether_addr_copy conversions may not always be wise. How did you find them? Is there a new alignment capability in smatch? -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] staging: rtl8723au: incorrect use of ether_addr_copy()
On Wed, 8 Oct 2014, Dan Carpenter wrote: On Wed, Oct 08, 2014 at 05:26:11AM -0700, Joe Perches wrote: On Wed, 2014-10-08 at 13:40 +0300, Dan Carpenter wrote: The return from myid() isn't aligned correctly for ether_addr_copy(). Hey Dan. Actual evidence showing ether_addr_copy conversions may not always be wise. How did you find them? I was just trying to see how common these kinds of bugs are. It didn't take long to find, but my impression is that they are rare and I got lucky. These kinds of bugs are tricky to find and we don't have any tools for it. Couldn't you just use your favorite matching tool, collect the file names, compile them, run pahole, and process the output in some way? It doesn't give a complete analysis (you don't find all problems), but if you find a problem it is a real problem. julia -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rt2x00: rt2x00queue: avoid using more headroom then driver requested
On Wed, Oct 08, 2014 at 07:46:33AM -0400, Mark Asselstine wrote: If rt2x00 does not remove the alignment from the frame before giving it back to mac80211 and the same frame comes into rt2x00 again it should be correctly aligned and no additional header space is required. So this should be fine. Then I would say this definitely hints at a design flaw in rt2x00queue_insert_l2pad(). Take the following scenario. * skb's first arrival in rt2x00queue_insert_l2pad(), 3 bytes needed for frame alignment, 2 bytes for l2pad results in 3 bytes of headroom taken. Not quite realistic assumption - header length will have to be odd then. But if such situation would happen we will have: header_align=2, payload_align=3, l2pad=3 Since payload_align will be bigger than header_align, header_align will be increased to 6. Header will be moved by 6 bytes, frame will be moved by 3 bytes, between header and frame there will be l2pad equal to 3. * rt2x00lib_txdone() returns 2 bytes of headroom Return 3 bytes. * skb's second arrival in rt2x00queue_insert_l2pad(), 0 bytes needed for frame alignment, 2 bytes for l2pad results in 4 bytes of headroom taken. Header will be moved by 3 bytes. * rt2x00lib_txdone() returns 2 bytes of headroom Return 3 bytes. Basically as long as any bytes are required for l2pad the headroom will lose 4 bytes again and again, never being returned by rt2x00lib_txdone(). I think that's not true - you made a few mistakes in your scenario, but perhaps I'm wrong :-) Stanislaw -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] staging: rtl8723au: incorrect use of ether_addr_copy()
On Wed, Oct 08, 2014 at 02:50:50PM +0200, Julia Lawall wrote: Couldn't you just use your favorite matching tool, collect the file names, compile them, run pahole, and process the output in some way? It doesn't give a complete analysis (you don't find all problems), but if you find a problem it is a real problem. Well... You would be better off using Smatch than pahole. And obviously I did try something like this, but it's fairly tricky. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
wl1271 driver on Linux 3.10 for SabreSD
Hi all, I need to activate the wl12xx driver (from drivers/net/wireless/ti/) on a (Android) Linux 3.10.31 kernel on an i.MX6Q SabreSD board. The wl1271 is to communicate with the board through an SDIO interface (SD2 on the board). I have built the driver modules (cfg80211 and mac80211; wlcore, wlcore_sdio and wl12xx), and insmod'ed them. They are listed by lsmod : wlcore_sdio 3820 0 - Live 0x wl12xx 51103 0 - Live 0x wlcore 161008 1 wl12xx, Live 0x mac80211 253798 2 wl12xx,wlcore, Live 0x cfg80211 201451 2 wlcore,mac80211, Live 0x dmesg shows only a message from cfg80211 (no errors for other modules, when insmod'ed in the right order) : cfg80211: Calling CRDA to update world regulatory domain 1) Can I use /sys/modules/{wlcore,wlcore_sdio,wl12xx}/ to test it ? 2) The board uses the device tree to enumerate platform devices. To correctly activate the driver, do I need to add it to the board DT (and a binding to the driver) ? 3) The driver is supposed to use a firmware (according to http://wireless.kernel.org/en/users/Drivers/wl12xx), how do I know whether it found it (no kernel log message) (I've put the firmware in /system/etc/firmware/) ? Thanks in advance. Regards, Hubert -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] net: rfkill: gpio: Implement host wake up support
Thanks, it sounds good to use a label instead of index. However: - DSD is only compatible with BIOS ACPI 5.1 - gpiod_get_index does not take care of con_id (label) parameter in case of ACPI (acpi_find_gpio). - gpiod_get calls gpiod_get_index with index 0. Any patch ongoing to support con_id parameter with ACPI? Regards, Loic On 08/10/2014 13:38, Heikki Krogerus wrote: On Wed, Oct 08, 2014 at 10:34:38AM +0200, Loic Poulain wrote: Some GPIO based rfkill devices can wake their host up from suspend by toggling an input (from the host perspective) GPIO. This patch adds a generic support for that feature by registering a threaded interrupt routine and thus setting the corresponding GPIO as a wake up source. Signed-off-by: Loic Poulain loic.poul...@intel.com --- net/rfkill/rfkill-gpio.c | 49 1 file changed, 49 insertions(+) To continue my previous answer, for this you could have the following _DSD.. Name (_DSD, Package () { ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301), Package () { Package () {host_wake-gpio, Package () {^BTH1, 0, 0, 0}}, Package () {reset-gpio, Package () {^BTH1, 1, 0, 0}}, Package () {shutdown-gpio, Package () {^BTH1, 2, 0, 0}}, } }) And in the driver you can then request it without caring about the index.. ... gpio = devm_gpiod_get_index(dev, host_wake, 3); if (!IS_ERR(gpio)) { ret = gpiod_direction_input(gpio); if (ret) return ret; rfkill-irq = gpiod_to_irq(gpio); ... Actually, we could just use devm_gpiod_get instead of devm_gpiod_get_index in this case. Cheers, -- Intel Open Source Technology Center http://oss.intel.com/ -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to know the information about RTS reception in mac80211 or device driver
+ath...@lists.infradead.org On 7 October 2014 13:30, Okhwan Lee oh...@mwnl.snu.ac.kr wrote: Hi, We are trying to develop an algorithm by using the information of RTS reception at the receiver side. (actually, we need the preceding frame of A-MPDU). As far as we know, RTS/CTS exchange is done by firmware of NIC. However, in monitor mode, we quite sure that the RTS/CTS frame reception should be reported to upper layer. As an evidence, Wireshark can capture and display the RTS/CTS reception. However, mac80211 and device driver (e.g., ath10k) can not recognize the reception of RTS even if we put the WiFi NIC in monitor mode. We check the type and subtype of frame_control field for all the received frames at the “ieee80211_rx_monitor” function of mac80211. We can find the reception of data frames amd mgmt frames, but there is no RTS/CTS frame. (We confirm that there is the RTS/CTS frame by using wireshark.) Is there any way to know the information about RTS reception in mac80211 or device driver? I think you've just found an ath10k bug. I've been running through Rx code lately. I was looking at htt_rx_mpdu_status and noticed it was a bit greedy. I then recalled someone was complaining about RTS reception. I'll try to fix this soon. I'll put you in Cc in case you'd want to play with it before it's merged. Michał -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] staging: rtl8723au: incorrect use of ether_addr_copy()
Dan Carpenter dan.carpen...@oracle.com writes: The return from myid() isn't aligned correctly for ether_addr_copy(). Signed-off-by: Dan Carpenter dan.carpen...@oracle.com Sorry, this makes no sense, just fix it properly! drivers/staging/rtl8723au/include/rtw_eeprom.h: struct eeprom_priv { u8 bautoload_fail_flag; u8 bloadfile_fail_flag; u8 bloadmac_fail_flag; /* u8 bempty; */ /* u8 sys_config; */ u8 mac_addr[6];/* PermanentAddress */ /* u8 config0; */ Move mac_addr[6] to the top of the struct and be done with it. NACK Jes diff --git a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c index 3eb77de..c8f7890 100644 --- a/drivers/staging/rtl8723au/core/rtw_mlme_ext.c +++ b/drivers/staging/rtl8723au/core/rtw_mlme_ext.c @@ -2402,7 +2402,7 @@ void issue_beacon23a(struct rtw_adapter *padapter, int timeout_ms) mgmt-seq_ctrl = 0; ether_addr_copy(mgmt-da, bc_addr); - ether_addr_copy(mgmt-sa, myid(padapter-eeprompriv)); + memcpy(mgmt-sa, myid(padapter-eeprompriv), ETH_ALEN); ether_addr_copy(mgmt-bssid, get_my_bssid23a(cur_network)); /* timestamp will be inserted by hardware */ @@ -2556,7 +2556,7 @@ static void issue_probersp(struct rtw_adapter *padapter, unsigned char *da, cpu_to_le16(IEEE80211_FTYPE_MGMT | IEEE80211_STYPE_PROBE_RESP); ether_addr_copy(mgmt-da, da); - ether_addr_copy(mgmt-sa, mac); + memcpy(mgmt-sa, mac, ETH_ALEN); ether_addr_copy(mgmt-bssid, bssid); mgmt-seq_ctrl = cpu_to_le16(IEEE80211_SN_TO_SEQ(pmlmeext-mgnt_seq)); @@ -2718,7 +2718,7 @@ static int _issue_probereq(struct rtw_adapter *padapter, ether_addr_copy(pwlanhdr-addr3, bc_addr); } - ether_addr_copy(pwlanhdr-addr2, mac); + memcpy(pwlanhdr-addr2, mac, ETH_ALEN); pwlanhdr-seq_ctrl = cpu_to_le16(IEEE80211_SN_TO_SEQ(pmlmeext-mgnt_seq)); @@ -2863,8 +2863,8 @@ static void issue_auth(struct rtw_adapter *padapter, struct sta_info *psta, #ifdef CONFIG_8723AU_AP_MODE unsigned short val16; ether_addr_copy(mgmt-da, psta-hwaddr); - ether_addr_copy(mgmt-sa, myid(padapter-eeprompriv)); - ether_addr_copy(mgmt-bssid, myid(padapter-eeprompriv)); + memcpy(mgmt-sa, myid(padapter-eeprompriv), ETH_ALEN); + memcpy(mgmt-bssid, myid(padapter-eeprompriv), ETH_ALEN); /* setting auth algo number */ val16 = (u16)psta-authalg; @@ -2895,7 +2895,7 @@ static void issue_auth(struct rtw_adapter *padapter, struct sta_info *psta, struct ieee80211_mgmt *iv_mgmt; ether_addr_copy(mgmt-da, get_my_bssid23a(pmlmeinfo-network)); - ether_addr_copy(mgmt-sa, myid(padapter-eeprompriv)); + memcpy(mgmt-sa, myid(padapter-eeprompriv), ETH_ALEN); ether_addr_copy(mgmt-bssid, get_my_bssid23a(pmlmeinfo-network)); @@ -3006,7 +3006,7 @@ static void issue_assocrsp(struct rtw_adapter *padapter, unsigned short status, mgmt-frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT | pkt_type); ether_addr_copy(mgmt-da, pstat-hwaddr); - ether_addr_copy(mgmt-sa, myid(padapter-eeprompriv)); + memcpy(mgmt-sa, myid(padapter-eeprompriv), ETH_ALEN); ether_addr_copy(mgmt-bssid, get_my_bssid23a(pmlmeinfo-network)); mgmt-seq_ctrl = cpu_to_le16(IEEE80211_SN_TO_SEQ(pmlmeext-mgnt_seq)); @@ -3129,7 +3129,7 @@ static void issue_assocreq(struct rtw_adapter *padapter) cpu_to_le16(IEEE80211_FTYPE_MGMT | IEEE80211_STYPE_ASSOC_REQ); ether_addr_copy(mgmt-da, get_my_bssid23a(pmlmeinfo-network)); - ether_addr_copy(mgmt-sa, myid(padapter-eeprompriv)); + memcpy(mgmt-sa, myid(padapter-eeprompriv), ETH_ALEN); ether_addr_copy(mgmt-bssid, get_my_bssid23a(pmlmeinfo-network)); mgmt-seq_ctrl = cpu_to_le16(IEEE80211_SN_TO_SEQ(pmlmeext-mgnt_seq)); @@ -3400,7 +3400,7 @@ static int _issue_nulldata23a(struct rtw_adapter *padapter, unsigned char *da, pwlanhdr-frame_control |= cpu_to_le16(IEEE80211_FCTL_PM); ether_addr_copy(pwlanhdr-addr1, da); - ether_addr_copy(pwlanhdr-addr2, myid(padapter-eeprompriv)); + memcpy(pwlanhdr-addr2, myid(padapter-eeprompriv), ETH_ALEN); ether_addr_copy(pwlanhdr-addr3, get_my_bssid23a(pmlmeinfo-network)); pwlanhdr-seq_ctrl = @@ -3528,7 +3528,7 @@ static int _issue_qos_nulldata23a(struct rtw_adapter *padapter, pwlanhdr-qos_ctrl |= cpu_to_le16(IEEE80211_QOS_CTL_EOSP); ether_addr_copy(pwlanhdr-addr1, da); - ether_addr_copy(pwlanhdr-addr2, myid(padapter-eeprompriv)); + memcpy(pwlanhdr-addr2, myid(padapter-eeprompriv), ETH_ALEN);
Re: [patch] staging: rtl8723au: incorrect use of ether_addr_copy()
On Wed, 2014-10-08 at 15:46 +0300, Dan Carpenter wrote: On Wed, Oct 08, 2014 at 05:26:11AM -0700, Joe Perches wrote: On Wed, 2014-10-08 at 13:40 +0300, Dan Carpenter wrote: The return from myid() isn't aligned correctly for ether_addr_copy(). Hey Dan. Actual evidence showing ether_addr_copy conversions may not always be wise. How did you find them? I was just trying to see how common these kinds of bugs are. It didn't take long to find, but my impression is that they are rare and I got lucky. These kinds of bugs are tricky to find and we don't have any tools for it. As far as I know, that's true too. Jes, was the mac_addr field in this struct ever __aligned(2)? struct eeprom_priv { u8 bautoload_fail_flag; u8 bloadfile_fail_flag; u8 bloadmac_fail_flag; /* u8 bempty; */ /* u8 sys_config; */ u8 mac_addr[6];/* PermanentAddress */ ... } As far as I can tell from git history, it was that way at the first check-in. Dan, did you also look for any other alignment defects in uses of any is_foo_ether_addr calls? -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] staging: rtl8723au: incorrect use of ether_addr_copy()
Dan Carpenter dan.carpen...@oracle.com writes: On Wed, Oct 08, 2014 at 03:59:33PM +0200, Jes Sorensen wrote: Dan Carpenter dan.carpen...@oracle.com writes: The return from myid() isn't aligned correctly for ether_addr_copy(). Signed-off-by: Dan Carpenter dan.carpen...@oracle.com Sorry, this makes no sense, just fix it properly! drivers/staging/rtl8723au/include/rtw_eeprom.h: struct eeprom_priv { u8 bautoload_fail_flag; u8 bloadfile_fail_flag; u8 bloadmac_fail_flag; /* u8 bempty; */ /* u8 sys_config; */ u8 mac_addr[6];/* PermanentAddress */ /* u8 config0; */ Move mac_addr[6] to the top of the struct and be done with it. NACK Oops. I thought it was something from the hardware. Actually can you fix it and give me a reported-by tag? That stuff is just copied into memory from the eeprom, so we can pretty much do with it as we like. I'll put it on my list. Jes -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to know the information about RTS reception in mac80211 or device driver
Michal Kazior wrote: I think you've just found an ath10k bug. I've been running through Rx code lately. I was looking at htt_rx_mpdu_status and noticed it was a bit greedy. I then recalled someone was complaining about RTS reception. Maybe the RX filter to receive control frames is not set properly in the FW ? ath10k doesn't seem to use WMI_10X_PDEV_PARAM_RX_FILTER. Sujith -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] staging: rtl8723au: incorrect use of ether_addr_copy()
Joe Perches j...@perches.com writes: On Wed, 2014-10-08 at 16:33 +0200, Jes Sorensen wrote: Joe Perches j...@perches.com writes: On Wed, 2014-10-08 at 15:46 +0300, Dan Carpenter wrote: On Wed, Oct 08, 2014 at 05:26:11AM -0700, Joe Perches wrote: On Wed, 2014-10-08 at 13:40 +0300, Dan Carpenter wrote: The return from myid() isn't aligned correctly for ether_addr_copy(). Hey Dan. Actual evidence showing ether_addr_copy conversions may not always be wise. How did you find them? I was just trying to see how common these kinds of bugs are. It didn't take long to find, but my impression is that they are rare and I got lucky. These kinds of bugs are tricky to find and we don't have any tools for it. As far as I know, that's true too. Jes, was the mac_addr field in this struct ever __aligned(2)? struct eeprom_priv { u8 bautoload_fail_flag; u8 bloadfile_fail_flag; u8 bloadmac_fail_flag; /* u8 bempty; */ /* u8 sys_config; */ u8 mac_addr[6];/* PermanentAddress */ ... } As far as I can tell from git history, it was that way at the first check-in. I may have removed other entries that were unused, and that caused it to become mis-aligned. I can't say for sure - the fix is straight forward though. One option is to add __aligned(2) to the mac_addr field and make no other change. As I said in another mail, just move it to the front of the struct and be done with it. No point in wasting alignment bytes if we don't have to. Jes -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V3.18] rtlwifi: Fix possible unaligned array in ether_addr_copy()
Two macros used to copy BSSID information use ether_addr_copy(), thus the arrays must be 2-byte aligned. In one case, the array could become unaligned if the struct containing it were changed. Use the __unaligned(2) attribute to retain the necessary alignment. In addition, the magic number used to specify the size of the array is replaced by ETH_ALEN. Signed-off-by: Larry Finger larry.fin...@lwfinger.net --- drivers/net/wireless/rtlwifi/wifi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/rtlwifi/wifi.h b/drivers/net/wireless/rtlwifi/wifi.h index 976667a..6866dcf 100644 --- a/drivers/net/wireless/rtlwifi/wifi.h +++ b/drivers/net/wireless/rtlwifi/wifi.h @@ -1370,7 +1370,7 @@ struct rtl_mac { bool rdg_en; /*AP*/ - u8 bssid[6]; + u8 bssid[ETH_ALEN] __aligned(2); u32 vendor; u8 mcs[16]; /* 16 bytes mcs for HT rates. */ u32 basic_rates; /* b/g rates */ -- 1.8.4.5 -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: wl1271 driver on Linux 3.10 for SabreSD
On Wed, 2014-10-08 at 15:37 +0200, CHAUMETTE Hubert wrote: Hi all, I need to activate the wl12xx driver (from drivers/net/wireless/ti/) on a (Android) Linux 3.10.31 kernel on an i.MX6Q SabreSD board. The wl1271 is to communicate with the board through an SDIO interface (SD2 on the board). I have built the driver modules (cfg80211 and mac80211; wlcore, wlcore_sdio and wl12xx), and insmod'ed them. They are listed by lsmod : wlcore_sdio 3820 0 - Live 0x wl12xx 51103 0 - Live 0x wlcore 161008 1 wl12xx, Live 0x mac80211 253798 2 wl12xx,wlcore, Live 0x cfg80211 201451 2 wlcore,mac80211, Live 0x dmesg shows only a message from cfg80211 (no errors for other modules, when insmod'ed in the right order) : cfg80211: Calling CRDA to update world regulatory domain Does the SDHCI driver recognize the device? You should see something like this in 'dmesg': [78504.888006] sdhci-pci :44:06.1: Will use DMA mode even though HW doesn't fully claim to support it. [78536.520725] mmc0: new SDIO card at address 0001 [78536.540309] libertas_sdio: Libertas SDIO driver [78536.540313] libertas_sdio: Copyright Pierre Ossman If you don't see mmc0: new SDIO card... somewhere, then the wl1271 driver isn't even involved and you need to find out why the SD/MMC layer can't even see the card. Once you've got it to the point of the SD/MMC stack finding the driver, find out the VID/PID of the card (it'll be somewhere in /sys/bus/sdio/devices/), and make sure that matches what's in the wl1271 SDIO driver (VID 0x0097 PID 0x4076). If your device doesn't match that, add it to wlcore/sdio.c's wl1271_devices array. Dan 1) Can I use /sys/modules/{wlcore,wlcore_sdio,wl12xx}/ to test it ? 2) The board uses the device tree to enumerate platform devices. To correctly activate the driver, do I need to add it to the board DT (and a binding to the driver) ? 3) The driver is supposed to use a firmware (according to http://wireless.kernel.org/en/users/Drivers/wl12xx), how do I know whether it found it (no kernel log message) (I've put the firmware in /system/etc/firmware/) ? Thanks in advance. Regards, Hubert -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: wl1271 driver on Linux 3.10 for SabreSD
On Wed, 2014-10-08 at 12:52 -0500, Dan Williams wrote: On Wed, 2014-10-08 at 15:37 +0200, CHAUMETTE Hubert wrote: Hi all, I need to activate the wl12xx driver (from drivers/net/wireless/ti/) on a (Android) Linux 3.10.31 kernel on an i.MX6Q SabreSD board. The wl1271 is to communicate with the board through an SDIO interface (SD2 on the board). I have built the driver modules (cfg80211 and mac80211; wlcore, wlcore_sdio and wl12xx), and insmod'ed them. They are listed by lsmod : wlcore_sdio 3820 0 - Live 0x wl12xx 51103 0 - Live 0x wlcore 161008 1 wl12xx, Live 0x mac80211 253798 2 wl12xx,wlcore, Live 0x cfg80211 201451 2 wlcore,mac80211, Live 0x dmesg shows only a message from cfg80211 (no errors for other modules, when insmod'ed in the right order) : cfg80211: Calling CRDA to update world regulatory domain Does the SDHCI driver recognize the device? You should see something like this in 'dmesg': [78504.888006] sdhci-pci :44:06.1: Will use DMA mode even though HW doesn't fully claim to support it. [78536.520725] mmc0: new SDIO card at address 0001 [78536.540309] libertas_sdio: Libertas SDIO driver [78536.540313] libertas_sdio: Copyright Pierre Ossman If you don't see mmc0: new SDIO card... somewhere, then the wl1271 driver isn't even involved and you need to find out why the SD/MMC layer can't even see the card. Once you've got it to the point of the SD/MMC stack finding the driver, I meant finding the card here, not driver. find out the VID/PID of the card (it'll be somewhere in /sys/bus/sdio/devices/), and make sure that matches what's in the wl1271 SDIO driver (VID 0x0097 PID 0x4076). If your device doesn't match that, add it to wlcore/sdio.c's wl1271_devices array. Dan 1) Can I use /sys/modules/{wlcore,wlcore_sdio,wl12xx}/ to test it ? 2) The board uses the device tree to enumerate platform devices. To correctly activate the driver, do I need to add it to the board DT (and a binding to the driver) ? 3) The driver is supposed to use a firmware (according to http://wireless.kernel.org/en/users/Drivers/wl12xx), how do I know whether it found it (no kernel log message) (I've put the firmware in /system/etc/firmware/) ? Thanks in advance. Regards, Hubert -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3.18] rtlwifi: Fix possible unaligned array in ether_addr_copy()
From: Larry Finger larry.fin...@lwfinger.net Date: Wed, 8 Oct 2014 12:44:55 -0500 Two macros used to copy BSSID information use ether_addr_copy(), thus the arrays must be 2-byte aligned. In one case, the array could become unaligned if the struct containing it were changed. Use the __unaligned(2) attribute to retain the necessary alignment. In addition, the magic number used to specify the size of the array is replaced by ETH_ALEN. Signed-off-by: Larry Finger larry.fin...@lwfinger.net Acked-by: David S. Miller da...@davemloft.net -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rt2x00: rt2x00queue: avoid using more headroom then driver requested
On Wed, Oct 8, 2014 at 9:00 AM, Stanislaw Gruszka sgrus...@redhat.com wrote: On Wed, Oct 08, 2014 at 07:46:33AM -0400, Mark Asselstine wrote: If rt2x00 does not remove the alignment from the frame before giving it back to mac80211 and the same frame comes into rt2x00 again it should be correctly aligned and no additional header space is required. So this should be fine. Then I would say this definitely hints at a design flaw in rt2x00queue_insert_l2pad(). Take the following scenario. * skb's first arrival in rt2x00queue_insert_l2pad(), 3 bytes needed for frame alignment, 2 bytes for l2pad results in 3 bytes of headroom taken. Not quite realistic assumption - header length will have to be odd then. But if such situation would happen we will have: header_align=2, payload_align=3, l2pad=3 Since payload_align will be bigger than header_align, header_align will be increased to 6. Header will be moved by 6 bytes, frame will be moved by 3 bytes, between header and frame there will be l2pad equal to 3. * rt2x00lib_txdone() returns 2 bytes of headroom Return 3 bytes. * skb's second arrival in rt2x00queue_insert_l2pad(), 0 bytes needed for frame alignment, 2 bytes for l2pad results in 4 bytes of headroom taken. Header will be moved by 3 bytes. * rt2x00lib_txdone() returns 2 bytes of headroom Return 3 bytes. Basically as long as any bytes are required for l2pad the headroom will lose 4 bytes again and again, never being returned by rt2x00lib_txdone(). I think that's not true - you made a few mistakes in your scenario, but perhaps I'm wrong :-) No just me being an idiot. I had thought frame == header + l2pad + payload not frame == payload With this straight your numbers makes sense and my scenario is incorrect. We don't continue to eat in to the headroom but rather take and return a small bit. Thanks for your patience, it has been a while since I have been working on this stuff. I am still motivated to hunt this issue down, you might just need to correct me along the way. Mark Stanislaw -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BCM4313 brcmsmac 3.12: only semi-working?
On Tuesday 30 September 2014 12:06:10 Arend van Spriel wrote: On 09/29/14 21:40, Maximilian Engelhardt wrote: On Monday 29 September 2014 15:44:03 Arend van Spriel wrote: On 09/26/14 17:20, Michael Tokarev wrote: I can send it your way, -- guess it will be quite a bit costly, but I don't have any use for it anyway (short of throwing it away), and since I already spent significantly more money due to all this (whole thinkpad plus ssds and several wifi adaptors), this additional cost is just a small noize. But since that's 2nd card in a row, maybe there's something else in there, the prob is not in the card? Could be. Maybe some BIOS issue. Can you make some hi-res pictures of the card and email them to me? If it is identical to what I already have over here there is not much sense in sending it. Regards, Arend Hi Arend, I just saw this thread on linux-wireless and wanted to answer as it might be of interest to you. I also own a BCM4313 wireless network card. About a year ago I reported some problems with reception strength which were then fixed after some time, debugging and testing passed. At that time I also did some throughput testing, but only had a 802.11g access-point to test. The results were not ideal, but also not too bad. So at that time I thought the issues were all more or less fixed and mostly fine. I also don't use wireless very much, so as long as things do work somehow acceptable I probably don't notice any problems immediately. So it comes that only about some month ago I noticed that the throughput I measured with my 11g access-point (about half the rate than with an atheros card on the same ap) is about the same on a 11n access-point where it should be much faster. I didn't experience any stalls, but that may also be that I didn't use the card enough to really notice them. I always wanted to report a bug because of the low throughout, but never got to it because of lack of time. I didn't want to provide a report saying just it doesn't work or it's slow without any data about it and a description how to reproduce it, as I think without this information a bug report is mostly useless. I also had a look at the kernel changelog of the brcmsm driver and notices there was little to no activity lately. Because of this I also wasn't sure if there is still someone interesting in fixing bugs for this device. As I was annoyed by the bad support for this card I decided it would be more easy and much less time consuming to simply buy another card than trying to get this fixed. So I bought a BCM43228 card, because it also supports 5 GHz. Only after it arrived I noticed that it was only supported by the 3.17+ kernel (not so much of a problem) and that it only seems to work in 802.11g mode (only slow speeds and no 5 GHz). At least I could use it in full 11g speeds, so it was a improvement. So I still don't have a card that does simply work. As I hope the missing support for my BCM43228 will hopefully be added some time in the future it probably would still be worth fixing the BCM4313 card as other users will also benefit from it. A friend of mine also has the same laptop than me and the same (or at least very similar) wireless card. He has told me he has also problem with stalls like Michael reported (if I remember the history of the thread correctly). So I'm not really sure where I should go from here. I can try to provide some debugging information as time permits, but I don't know how much time I will have for this in the future. Of course ideally I want to use the BCM43228 card with full support, as it can work on 5 GHz. Currently the BCM43228 card is plugged into my laptop, but I want to avoid swapping the cards more that very few times because the antenna connectors are only designed for very few (un)plug cycles. If it's of any information, my card is labeled BCM-BCM94313HMGB on the sticker, the laptop where it was originally is a ThinkPad Edge E135. Thanks for taking time to chime in. This chipset is a pain in the The label info does help. You have a 4313 with internal PA for which support was added later. The card that Michael has seems to have an external PA. The initial iPA support patch broke things for everyone with external PA so it was reverted. In the second round it was better, but it seems Michael still had issues. As he mentioned BT issues and his card shares the external PA between WLAN and BT I believe that there is a BT-coex issue. What is causing your 4313 to seemingly do 11g rates is hard to tell without any debug info. I have that card over here, but in my cabled setup it is doing 72Mbps, ie. 11n rate. I can run a rate-vs-range test to see if there is an issue. Thanks again and Regards, Arend Hi Arend, Today I changed back to the BC4313 card to verify the speed is still slow. At first it
Re: [PATCH] rt2x00: rt2x00queue: avoid using more headroom then driver requested
On Wed, Oct 8, 2014 at 3:52 PM, Mark Asselstine asse...@gmail.com wrote: On Wed, Oct 8, 2014 at 9:00 AM, Stanislaw Gruszka sgrus...@redhat.com wrote: On Wed, Oct 08, 2014 at 07:46:33AM -0400, Mark Asselstine wrote: If rt2x00 does not remove the alignment from the frame before giving it back to mac80211 and the same frame comes into rt2x00 again it should be correctly aligned and no additional header space is required. So this should be fine. Then I would say this definitely hints at a design flaw in rt2x00queue_insert_l2pad(). Take the following scenario. * skb's first arrival in rt2x00queue_insert_l2pad(), 3 bytes needed for frame alignment, 2 bytes for l2pad results in 3 bytes of headroom taken. Not quite realistic assumption - header length will have to be odd then. But if such situation would happen we will have: header_align=2, payload_align=3, l2pad=3 Since payload_align will be bigger than header_align, header_align will be increased to 6. Header will be moved by 6 bytes, frame will be moved by 3 bytes, between header and frame there will be l2pad equal to 3. * rt2x00lib_txdone() returns 2 bytes of headroom Return 3 bytes. * skb's second arrival in rt2x00queue_insert_l2pad(), 0 bytes needed for frame alignment, 2 bytes for l2pad results in 4 bytes of headroom taken. Header will be moved by 3 bytes. * rt2x00lib_txdone() returns 2 bytes of headroom Return 3 bytes. Basically as long as any bytes are required for l2pad the headroom will lose 4 bytes again and again, never being returned by rt2x00lib_txdone(). I think that's not true - you made a few mistakes in your scenario, but perhaps I'm wrong :-) No just me being an idiot. I had thought frame == header + l2pad + payload not frame == payload By the way, I assumed that this due to the name and contents of rt2x00queue_align_frame(). Where all the data (header and payload) are aligned to a 4-byte boundary and the function is name 'align_frame'. I assume your interpretation is the correct one, can you confirm? Mark With this straight your numbers makes sense and my scenario is incorrect. We don't continue to eat in to the headroom but rather take and return a small bit. Thanks for your patience, it has been a while since I have been working on this stuff. I am still motivated to hunt this issue down, you might just need to correct me along the way. Mark Stanislaw -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to know the information about RTS reception in mac80211 or device driver
On 8 October 2014 16:50, Sujith Manoharan suj...@msujith.org wrote: Michal Kazior wrote: I think you've just found an ath10k bug. I've been running through Rx code lately. I was looking at htt_rx_mpdu_status and noticed it was a bit greedy. I then recalled someone was complaining about RTS reception. Maybe the RX filter to receive control frames is not set properly in the FW ? ath10k doesn't seem to use WMI_10X_PDEV_PARAM_RX_FILTER. I suspect ath10k is dropping RTS frames in ath10k_htt_rx_amsdu_allowed() now because RX_ATTENTION_FLAGS_MGMT_TYPE includes management, control and null function frames. I'm yet to confirm what ath10k really ends up dropping now though. Michał -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html