* Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
From: Kalle Valo @ 2016-10-26 11:11 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Arend van Spriel, Linus Torvalds, linux-kernel, Hante Meuleman,
Franky Lin, Pieter-Paul Giesberts, Franky (Zhenhui) Lin,
Rafał Miłecki, linux-wireless, brcm80211-dev-list.pdl,
netdev
In-Reply-To: <3406231.8mt2808XDi@wuerfel>
Arnd Bergmann <arnd@arndb.de> writes:
> On Wednesday, October 26, 2016 9:49:58 AM CEST Kalle Valo wrote:
>> Arnd Bergmann <arnd@arndb.de> writes:
>>=20
>> > A bugfix added a sanity check around the assignment and use of the
>> > 'is_11d' variable, which looks correct to me, but as the function is
>> > rather complex already, this confuses the compiler to the point where
>> > it can no longer figure out if the variable is always initialized
>> > correctly:
>> >
>> > brcm80211/brcmfmac/cfg80211.c: In function =E2=80=98brcmf_cfg80211_sta=
rt_ap=E2=80=99:
>> > brcm80211/brcmfmac/cfg80211.c:4586:10: error: =E2=80=98is_11d=E2=80=99=
may be used uninitialized in this function [-Werror=3Dmaybe-uninitialized]
>> >
>> > This adds an initialization for the newly introduced case in which
>> > the variable should not really be used, in order to make the warning
>> > go away.
>> >
>> > Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
>> > Cc: Hante Meuleman <hante.meuleman@broadcom.com>
>> > Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
>> > Cc: Kalle Valo <kvalo@codeaurora.org>
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>=20
>> Via which tree are you planning to submit this? Should I take it?
>
> I'd prefer if you can take it and forward it along with your other
> bugfixes. I'll try to take care of the ones that nobody else
> picked up.
Ok, I'll take it. I'm planning to push this to 4.9.
--=20
Kalle Valo
^ permalink raw reply
* Re: [PATCH] cfg80211: add key management offload feature
From: Johannes Berg @ 2016-10-26 12:11 UTC (permalink / raw)
To: Amitkumar Karwar, linux-wireless, hostap, Jouni Malinen
Cc: yangzy, Cathy Luo, Nishant Sarmukadam, lihz
In-Reply-To: <1474973796-1873-1-git-send-email-akarwar@marvell.com>
Getting back to this ... as I was preparing my patch.
> @@ -3687,6 +3692,9 @@ enum nl80211_key_attributes {
> NL80211_KEY_DEFAULT_MGMT,
> NL80211_KEY_TYPE,
> NL80211_KEY_DEFAULT_TYPES,
> + NL80211_KEY_REPLAY_CTR,
> + NL80211_KEY_KCK,
> + NL80211_KEY_KEK,
You made those key attributes, but ...
> nla_put(msg, NL80211_ATTR_RESP_IE, resp_ie_len,
> resp_ie)))
> goto nla_put_failure;
>
> + if (wiphy_ext_feature_isset(&rdev->wiphy,
> + NL80211_EXT_FEATURE_KEY_MGMT_OFF
> LOAD) &&
> + (nla_put_u8(msg, NL80211_ATTR_AUTHORIZED, authorized) ||
> + (key_replay_ctr && nla_put(msg, NL80211_KEY_REPLAY_CTR,
> + NL80211_REPLAY_CTR_LEN, key_replay_ctr)) ||
> + (key_kck &&
> + nla_put(msg, NL80211_KEY_KCK, NL80211_KCK_LEN,
> key_kck)) ||
> + (key_kek &&
> + nla_put(msg, NL80211_KEY_KEK, NL80211_KEK_LEN,
> key_kek))))
> + goto nla_put_failure;
Used them at a top level here! That can't possibly have worked.
Anyway, I checked and we can transport these without adding new
attributes, but adding the NL80211_ATTR_REKEY_DATA attribute with its
nested KEK, KCK and REPLAY_CTR.
That leaves the authorized attribute, I guess nesting a whole bunch of
station info etc. doesn't make a lot of sense.
I also fail to see how the data is actually configured down, since you
just pass it through. I'll send our patch for configuring the PMK/PSK
via the PMKSA cache separately in a few minutes.
johannes
^ permalink raw reply
* [RFC] cfg80211: support 4-way-handshake offload with PSK and 802.1X
From: Johannes Berg @ 2016-10-26 12:26 UTC (permalink / raw)
To: linux-wireless
Cc: avraham.stern, akarwar, j, yangzy, cluo, nishants, lihz,
ilan.peer
In-Reply-To: <1477483882.4059.34.camel@sipsolutions.net>
From: Avraham Stern <avraham.stern@intel.com>
TODO:
* add a separate capability flag? and explain how the offload
is supposed to work in 802.1X, EAPOL-Key messages are going
to be processed by the supplicant, but then the 4-way-HS is
done by the device after getting the PMK in the PMKSA cache
entry - explain that mechanism
* does anyone still want EAP-LEAP 16-byte PMK?
Signed-off-by: Avraham Stern <avraham.stern@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
include/linux/ieee80211.h | 3 +++
include/net/cfg80211.h | 8 +++++++-
include/uapi/linux/nl80211.h | 14 +++++++++++++-
net/wireless/nl80211.c | 15 +++++++++++++++
4 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index a80516fd65c8..40206b2a6e6d 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -2326,6 +2326,9 @@ enum ieee80211_sa_query_action {
#define WLAN_PMKID_LEN 16
+#define WLAN_PMK_LEN 32
+#define WLAN_PMK_LEN_SUITE_B_192 48
+
#define WLAN_OUI_WFA 0x506f9a
#define WLAN_OUI_TYPE_WFA_P2P 9
#define WLAN_OUI_MICROSOFT 0x0050f2
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d1ffbc3a8e55..3bb407c57177 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -615,6 +615,7 @@ struct survey_info {
* @wep_keys: static WEP keys, if not NULL points to an array of
* CFG80211_MAX_WEP_KEYS WEP keys
* @wep_tx_key: key index (0..3) of the default TX static WEP key
+ * @psk: PSK (for devices supporting 4-way-handshake offload, 32 bytes)
*/
struct cfg80211_crypto_settings {
u32 wpa_versions;
@@ -628,6 +629,7 @@ struct cfg80211_crypto_settings {
bool control_port_no_encrypt;
struct key_params *wep_keys;
int wep_tx_key;
+ const u8 *psk;
};
/**
@@ -2064,11 +2066,15 @@ enum wiphy_params_flags {
* caching.
*
* @bssid: The AP's BSSID.
- * @pmkid: The PMK material itself.
+ * @pmkid: The PMK identifier.
+ * @pmk: The PMK material itself.
+ * @pmk_len: The PMK length in bytes.
*/
struct cfg80211_pmksa {
const u8 *bssid;
const u8 *pmkid;
+ const u8 *pmk;
+ u8 pmk_len;
};
/**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 1362d24957b5..40b003cc07bd 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -371,7 +371,8 @@
* NL80211_CMD_GET_SURVEY and on the "scan" multicast group)
*
* @NL80211_CMD_SET_PMKSA: Add a PMKSA cache entry, using %NL80211_ATTR_MAC
- * (for the BSSID) and %NL80211_ATTR_PMKID.
+ * (for the BSSID) and %NL80211_ATTR_PMKID. Optionally, %NL80211_ATTR_PMK
+ * can be used to specify the PMK.
* @NL80211_CMD_DEL_PMKSA: Delete a PMKSA cache entry, using %NL80211_ATTR_MAC
* (for the BSSID) and %NL80211_ATTR_PMKID.
* @NL80211_CMD_FLUSH_PMKSA: Flush all PMKSA cache entries.
@@ -1937,6 +1938,11 @@ enum nl80211_commands {
* @NL80211_ATTR_NAN_MATCH: used to report a match. This is a nested attribute.
* See &enum nl80211_nan_match_attributes.
*
+ * @NL80211_ATTR_PMK: PMK for offloaded 4-Way Handshake. Relevant with
+ * %NL80211_CMD_CONNECT (for WPA/WPA2-PSK networks) when PSK is used, or
+ * with %NL80211_CMD_SET_PMKSA when 802.1X authentication is used and for
+ * PMKSA caching.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2336,6 +2342,8 @@ enum nl80211_attrs {
NL80211_ATTR_NAN_FUNC,
NL80211_ATTR_NAN_MATCH,
+ NL80211_ATTR_PMK,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -4638,6 +4646,9 @@ enum nl80211_feature_flags {
* configuration (AP/mesh) with HT rates.
* @NL80211_EXT_FEATURE_BEACON_RATE_VHT: Driver supports beacon rate
* configuration (AP/mesh) with VHT rates.
+ * @NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA: Device supports
+ * doing 4-way handshake in station mode (PSK is passed as part
+ * of the connect command).
*
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -4652,6 +4663,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_BEACON_RATE_LEGACY,
NL80211_EXT_FEATURE_BEACON_RATE_HT,
NL80211_EXT_FEATURE_BEACON_RATE_VHT,
+ NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA,
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b0440de82171..6720c7bf3ed1 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -414,6 +414,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_NAN_MASTER_PREF] = { .type = NLA_U8 },
[NL80211_ATTR_NAN_DUAL] = { .type = NLA_U8 },
[NL80211_ATTR_NAN_FUNC] = { .type = NLA_NESTED },
+ [NL80211_ATTR_PMK] = { .len = WLAN_PMK_LEN },
};
/* policy for the key attributes */
@@ -7922,6 +7923,13 @@ static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev,
memcpy(settings->akm_suites, data, len);
}
+ if (info->attrs[NL80211_ATTR_PMK]) {
+ if (!wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA))
+ return -EINVAL;
+ settings->psk = nla_data(info->attrs[NL80211_ATTR_PMK]);
+ }
+
return 0;
}
@@ -8824,6 +8832,13 @@ static int nl80211_setdel_pmksa(struct sk_buff *skb, struct genl_info *info)
pmksa.pmkid = nla_data(info->attrs[NL80211_ATTR_PMKID]);
pmksa.bssid = nla_data(info->attrs[NL80211_ATTR_MAC]);
+ if (info->attrs[NL80211_ATTR_PMK]) {
+ pmksa.pmk = nla_data(info->attrs[NL80211_ATTR_PMK]);
+ pmksa.pmk_len = nla_len(info->attrs[NL80211_ATTR_PMK]);
+ if (pmksa.pmk_len != WLAN_PMK_LEN &&
+ pmksa.pmk_len != WLAN_PMK_LEN_SUITE_B_192)
+ return -EINVAL;
+ }
if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_STATION &&
dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_CLIENT)
--
2.8.1
^ permalink raw reply related
* [PATCH] nl80211: use nla_parse_nested() instead of nla_parse()
From: Johannes Berg @ 2016-10-26 12:45 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
It's just an inline doing the same thing, but the code
is nicer with it.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/wireless/nl80211.c | 85 ++++++++++++++++++++++----------------------------
1 file changed, 37 insertions(+), 48 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b0440de82171..ad49a4c43e01 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2318,10 +2318,9 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
nla_for_each_nested(nl_txq_params,
info->attrs[NL80211_ATTR_WIPHY_TXQ_PARAMS],
rem_txq_params) {
- result = nla_parse(tb, NL80211_TXQ_ATTR_MAX,
- nla_data(nl_txq_params),
- nla_len(nl_txq_params),
- txq_params_policy);
+ result = nla_parse_nested(tb, NL80211_TXQ_ATTR_MAX,
+ nl_txq_params,
+ txq_params_policy);
if (result)
return result;
result = parse_txq_params(tb, &txq_params);
@@ -3571,8 +3570,8 @@ static int nl80211_parse_tx_bitrate_mask(struct genl_info *info,
sband = rdev->wiphy.bands[band];
if (sband == NULL)
return -EINVAL;
- err = nla_parse(tb, NL80211_TXRATE_MAX, nla_data(tx_rates),
- nla_len(tx_rates), nl80211_txattr_policy);
+ err = nla_parse_nested(tb, NL80211_TXRATE_MAX, tx_rates,
+ nl80211_txattr_policy);
if (err)
return err;
if (tb[NL80211_TXRATE_LEGACY]) {
@@ -6328,9 +6327,8 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
nla_for_each_nested(nl_reg_rule, info->attrs[NL80211_ATTR_REG_RULES],
rem_reg_rules) {
- r = nla_parse(tb, NL80211_REG_RULE_ATTR_MAX,
- nla_data(nl_reg_rule), nla_len(nl_reg_rule),
- reg_rule_policy);
+ r = nla_parse_nested(tb, NL80211_REG_RULE_ATTR_MAX,
+ nl_reg_rule, reg_rule_policy);
if (r)
goto bad_reg;
r = parse_reg_rule(tb, &rd->reg_rules[rule_idx]);
@@ -6397,8 +6395,8 @@ static int parse_bss_select(struct nlattr *nla, struct wiphy *wiphy,
if (!nla_ok(nest, nla_len(nest)))
return -EINVAL;
- err = nla_parse(attr, NL80211_BSS_SELECT_ATTR_MAX, nla_data(nest),
- nla_len(nest), nl80211_bss_select_policy);
+ err = nla_parse_nested(attr, NL80211_BSS_SELECT_ATTR_MAX, nest,
+ nl80211_bss_select_policy);
if (err)
return err;
@@ -6788,9 +6786,8 @@ nl80211_parse_sched_scan_plans(struct wiphy *wiphy, int n_plans,
if (WARN_ON(i >= n_plans))
return -EINVAL;
- err = nla_parse(plan, NL80211_SCHED_SCAN_PLAN_MAX,
- nla_data(attr), nla_len(attr),
- nl80211_plan_policy);
+ err = nla_parse_nested(plan, NL80211_SCHED_SCAN_PLAN_MAX,
+ attr, nl80211_plan_policy);
if (err)
return err;
@@ -6879,9 +6876,9 @@ nl80211_parse_sched_scan(struct wiphy *wiphy, struct wireless_dev *wdev,
tmp) {
struct nlattr *rssi;
- err = nla_parse(tb, NL80211_SCHED_SCAN_MATCH_ATTR_MAX,
- nla_data(attr), nla_len(attr),
- nl80211_match_policy);
+ err = nla_parse_nested(tb,
+ NL80211_SCHED_SCAN_MATCH_ATTR_MAX,
+ attr, nl80211_match_policy);
if (err)
return ERR_PTR(err);
/* add other standalone attributes here */
@@ -7052,9 +7049,9 @@ nl80211_parse_sched_scan(struct wiphy *wiphy, struct wireless_dev *wdev,
tmp) {
struct nlattr *ssid, *rssi;
- err = nla_parse(tb, NL80211_SCHED_SCAN_MATCH_ATTR_MAX,
- nla_data(attr), nla_len(attr),
- nl80211_match_policy);
+ err = nla_parse_nested(tb,
+ NL80211_SCHED_SCAN_MATCH_ATTR_MAX,
+ attr, nl80211_match_policy);
if (err)
goto out_free;
ssid = tb[NL80211_SCHED_SCAN_MATCH_ATTR_SSID];
@@ -9754,9 +9751,8 @@ static int nl80211_parse_wowlan_tcp(struct cfg80211_registered_device *rdev,
if (!rdev->wiphy.wowlan->tcp)
return -EINVAL;
- err = nla_parse(tb, MAX_NL80211_WOWLAN_TCP,
- nla_data(attr), nla_len(attr),
- nl80211_wowlan_tcp_policy);
+ err = nla_parse_nested(tb, MAX_NL80211_WOWLAN_TCP, attr,
+ nl80211_wowlan_tcp_policy);
if (err)
return err;
@@ -9901,9 +9897,7 @@ static int nl80211_parse_wowlan_nd(struct cfg80211_registered_device *rdev,
goto out;
}
- err = nla_parse(tb, NL80211_ATTR_MAX,
- nla_data(attr), nla_len(attr),
- nl80211_policy);
+ err = nla_parse_nested(tb, NL80211_ATTR_MAX, attr, nl80211_policy);
if (err)
goto out;
@@ -9937,10 +9931,9 @@ static int nl80211_set_wowlan(struct sk_buff *skb, struct genl_info *info)
goto set_wakeup;
}
- err = nla_parse(tb, MAX_NL80211_WOWLAN_TRIG,
- nla_data(info->attrs[NL80211_ATTR_WOWLAN_TRIGGERS]),
- nla_len(info->attrs[NL80211_ATTR_WOWLAN_TRIGGERS]),
- nl80211_wowlan_policy);
+ err = nla_parse_nested(tb, MAX_NL80211_WOWLAN_TRIG,
+ info->attrs[NL80211_ATTR_WOWLAN_TRIGGERS],
+ nl80211_wowlan_policy);
if (err)
return err;
@@ -10022,8 +10015,8 @@ static int nl80211_set_wowlan(struct sk_buff *skb, struct genl_info *info)
rem) {
u8 *mask_pat;
- nla_parse(pat_tb, MAX_NL80211_PKTPAT, nla_data(pat),
- nla_len(pat), NULL);
+ nla_parse_nested(pat_tb, MAX_NL80211_PKTPAT, pat,
+ NULL);
err = -EINVAL;
if (!pat_tb[NL80211_PKTPAT_MASK] ||
!pat_tb[NL80211_PKTPAT_PATTERN])
@@ -10233,8 +10226,8 @@ static int nl80211_parse_coalesce_rule(struct cfg80211_registered_device *rdev,
int rem, pat_len, mask_len, pkt_offset, n_patterns = 0;
struct nlattr *pat_tb[NUM_NL80211_PKTPAT];
- err = nla_parse(tb, NL80211_ATTR_COALESCE_RULE_MAX, nla_data(rule),
- nla_len(rule), nl80211_coalesce_policy);
+ err = nla_parse_nested(tb, NL80211_ATTR_COALESCE_RULE_MAX, rule,
+ nl80211_coalesce_policy);
if (err)
return err;
@@ -10272,8 +10265,7 @@ static int nl80211_parse_coalesce_rule(struct cfg80211_registered_device *rdev,
rem) {
u8 *mask_pat;
- nla_parse(pat_tb, MAX_NL80211_PKTPAT, nla_data(pat),
- nla_len(pat), NULL);
+ nla_parse_nested(pat_tb, MAX_NL80211_PKTPAT, pat, NULL);
if (!pat_tb[NL80211_PKTPAT_MASK] ||
!pat_tb[NL80211_PKTPAT_PATTERN])
return -EINVAL;
@@ -10392,10 +10384,9 @@ static int nl80211_set_rekey_data(struct sk_buff *skb, struct genl_info *info)
if (!info->attrs[NL80211_ATTR_REKEY_DATA])
return -EINVAL;
- err = nla_parse(tb, MAX_NL80211_REKEY_DATA,
- nla_data(info->attrs[NL80211_ATTR_REKEY_DATA]),
- nla_len(info->attrs[NL80211_ATTR_REKEY_DATA]),
- nl80211_rekey_policy);
+ err = nla_parse_nested(tb, MAX_NL80211_REKEY_DATA,
+ info->attrs[NL80211_ATTR_REKEY_DATA],
+ nl80211_rekey_policy);
if (err)
return err;
@@ -10704,10 +10695,9 @@ static int nl80211_nan_add_func(struct sk_buff *skb,
wdev->owner_nlportid != info->snd_portid)
return -ENOTCONN;
- err = nla_parse(tb, NL80211_NAN_FUNC_ATTR_MAX,
- nla_data(info->attrs[NL80211_ATTR_NAN_FUNC]),
- nla_len(info->attrs[NL80211_ATTR_NAN_FUNC]),
- nl80211_nan_func_policy);
+ err = nla_parse_nested(tb, NL80211_NAN_FUNC_ATTR_MAX,
+ info->attrs[NL80211_ATTR_NAN_FUNC],
+ nl80211_nan_func_policy);
if (err)
return err;
@@ -10802,10 +10792,9 @@ static int nl80211_nan_add_func(struct sk_buff *skb,
if (tb[NL80211_NAN_FUNC_SRF]) {
struct nlattr *srf_tb[NUM_NL80211_NAN_SRF_ATTR];
- err = nla_parse(srf_tb, NL80211_NAN_SRF_ATTR_MAX,
- nla_data(tb[NL80211_NAN_FUNC_SRF]),
- nla_len(tb[NL80211_NAN_FUNC_SRF]),
- nl80211_nan_srf_policy);
+ err = nla_parse_nested(srf_tb, NL80211_NAN_SRF_ATTR_MAX,
+ tb[NL80211_NAN_FUNC_SRF],
+ nl80211_nan_srf_policy);
if (err)
goto out;
--
2.8.1
^ permalink raw reply related
* Re: [PATCH v2 2/2] mac80211: passively scan DFS channels if requested
From: Johannes Berg @ 2016-10-26 12:58 UTC (permalink / raw)
To: Simon Wunderlich; +Cc: Antonio Quartulli, linux-wireless
In-Reply-To: <3164753.QVH0CDzpbV@prime>
On Mon, 2016-10-24 at 16:53 +0200, Simon Wunderlich wrote:
> [snip]
>
> Again, no explicit "on installation" here, but there is also nothing
> saying that we can not check/operate on other channels in the
> meantime.
Yeah, ok.
> (NOTE: going off-channel while operating is a different topic).
Why do you think it's different?
Seems exactly the same to me, since you come back to the channel and
start sending without any new checking?
johannes
^ permalink raw reply
* Re: [PATCH][RFC] nl80211/mac80211: Rounded RSSI reporting
From: Johannes Berg @ 2016-10-26 13:11 UTC (permalink / raw)
To: Denis Kenzior, Andrew Zaborowski, linux-wireless
In-Reply-To: <580A2438.7030106@gmail.com>
On Fri, 2016-10-21 at 09:20 -0500, Denis Kenzior wrote:
> > It's actually not clear to me that this is really how it should be.
> > There's a point to be made that taking a more holistic "link
> > quality" would be a better choice. That's related, but maybe can be
> > a separate discussion.
> Can you elaborate on this 'link quality' idea?
Well, I didn't really want to - getting 3 system folks into a room will
result in 4 different ways of doing it - but you can take into account
not just the RSSI, but also the bitrate you can reasonably use on the
channel/with the AP, the noise you can perhaps detect (if you can), the
amount of packet loss or retransmissions you experience, etc.
I think that some systems (Android, maybe Windows) already do something
more complex than pure RSSI indicators, but I don't really know for
sure.
> > Yes, this would be ideal.
> >
> > [...]
see my other email
> This sounds really brittle. Furthermore, we also need a facility to
> know when signal strength is getting low to trigger roaming
> logic. This would mean sharing CQM facility between roaming & signal
> strength notifications. As you wrote above, things become quite
> impractical.
This would likely go through the supplicant anyway, so it could manage
proper range overlaps etc. for this.
It does seem brittle if we just have a single value, but if we add
low/high thresholds (with hysteresis) then I think we can do this, and
gain more flexibility in the process. But let's discuss more details
over in the other email I just sent :)
johannes
^ permalink raw reply
* Re: [PATCH][RFC] nl80211/mac80211: Rounded RSSI reporting
From: Johannes Berg @ 2016-10-26 13:05 UTC (permalink / raw)
To: Zaborowski, Andrew, linux-wireless@vger.kernel.org
In-Reply-To: <F1E72F916E15444E976109681700BEE23F6AF7D9@IRSMSX102.ger.corp.intel.com>
On Fri, 2016-10-21 at 19:03 +0000, Zaborowski, Andrew wrote:
> > The problem is that you really want this to be offloaded to the
> > device, *especially* if you care about low power usage, because you
> > absolutely don't want to be processing each beacon (which is
> > typically what we derive the data from today) in the host CPU - you
> > want those to be filtered by the device so you don't wake up the
> > host CPU every ~102ms.
>
> Right, that would be a separate optimisation but it probably won't
> arrive until there's an API that the drivers can implement, so I
> think this is a prerequisite.
Huh? No, beacon filtering is implemented by a lot of drivers today.
> The userspace switches count too and are the main motivation for this
> patch. Eventually, as you say, things will depend on specific
> drivers and you will want to optimise whatever you can assuming the
> hardware you're constrained to.
Yes and no. I think we can probably define a reasonable subset that
you'd expect more devices to implement. I don't really see the
requirement to do the "banding" that you did here offloaded - doing it
in mac80211 won't help much, and won't work in cases where you have
beacon filtering already anyway.
Drivers like iwlwifi would be able to implement the banding in
software, since they get a notification on every change above a
configurable threshold, but other drivers like one I looked at actually
do have a low and high threshold today, and program them to be the same
value with our current CQM API definitions.
> It seems like any mechanism you choose can be implemented on top of
> hardware that supports a low and a high threshold by setting the next
> values while handling the event related to the previous threshold
> crossing event. If the thresholds are specified by a middle value
> and a hysteresis or distance, that would work equally well.
> The API I added in the patch also allows offloading to such hardware.
Well, ok, technically the API can be implemented on top of drivers with
low/high thresholds, by doing the configuration according to the
current range you're in.
I would argue though that it makes more sense to expose a simpler
capability (e.g. two instead of the current single threshold) and do
the reprogramming from higher layers. That ends up being more flexible,
since you can then, for example, also have ranges that aren't all
identical - which makes some sense because above a certain level you
don't really care at all.
> In short a nl80211 change would be needed regardless of the mechanism
> chosen.
Agree. I'm just not convinced that the banding mechanism you propose is
the most reasonable choice for new API.
johannes
^ permalink raw reply
* Re: [PATCH v2 2/2] mac80211: passively scan DFS channels if requested
From: Simon Wunderlich @ 2016-10-26 13:30 UTC (permalink / raw)
To: Johannes Berg; +Cc: Antonio Quartulli, linux-wireless
In-Reply-To: <1477486736.4059.36.camel@sipsolutions.net>
[-- Attachment #1: Type: text/plain, Size: 1987 bytes --]
On Wednesday, October 26, 2016 2:58:56 PM CEST Johannes Berg wrote:
> > (NOTE: going off-channel while operating is a different topic).
>
> Why do you think it's different?
>
> Seems exactly the same to me, since you come back to the channel and
> start sending without any new checking?
There are two ways to leave a channel:
1.) Leave the operating channel "permanently". Then the ETSI 301 893 says:
"If no radar was detected on an Operating Channel but the RLAN stops operating
on that channel, then the channel becomes an Available Channel." (4.7.1.4
Master Devices (c))
Then we can select a new operating channel and re-start the process.
2.) Off-Channel CAC: This is an optional feature. Simply spoken, we keep the
channel operating, but try to do CAC-checking on a different channel by
shortly switching to that new channel from time to time. The new channel must
be checked for a longer time than the standard CAC time (6 minutes for non-
weather radar channels in ETSI).
The main difference is that we keep the current channel as "operating channel"
and are also required to be able to detect radars with the same probability.
Also note that this feature is not implemented in Linux as far as I know, and
personally I haven't seen it in the wild yet.
From 4.7.2.3.1:
"Off-Channel CAC is defined as an optional mechanism by which an RLAN device
monitors channel(s), different from the Operating Channel(s), for the presence
of radar signals. The Off-Channel CAC may be used in addition to the Channel
Availability Check defined in clause 4.7.2.2, for identifying Available
Channels.
Off-Channel CAC is performed by a number of non-continuous checks spread over
a period in time. This period, which is required to determine the presence of
radar signals, is defined as the Off-Channel CAC Time.
If no radars have been detected in a channel, then that channel becomes an
Available Channel."
Cheers,
Simon
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [NOT FOR MERGE] ath9k: work around key cache corruption
From: Kalle Valo @ 2016-10-26 14:05 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: ath9k-devel, linux-wireless, sw, Antonio Quartulli
In-Reply-To: <20161018083552.28592-1-a@unstable.cc>
Antonio Quartulli <a@unstable.cc> writes:
> From: Antonio Quartulli <antonio@open-mesh.com>
>
> This patch was crafted long time ago to work around a key cache
> corruption problem on ath9k chipsets.
>
> The workaround consists in periodically triggering a worker that
> uploads all the keys to the HW cache. The worker is triggered also
> when the vif detects 21 undecryptable packets.
>
>
> This patch is based on top compat-wireless-2015-10-26.
>
>
> I was asked to release this code to the public and, since it is
> GPL, I am now doing it.
GPL? AFAICS ath9k is under ISC. I'm not sure what you mean with the
sentence above, but it's possible to interpret it so that this patch is
not under ISC license, which is problematic.
--
Kalle Valo
^ permalink raw reply
* Re: [NOT FOR MERGE] ath9k: work around key cache corruption
From: Antonio Quartulli @ 2016-10-26 14:10 UTC (permalink / raw)
To: Kalle Valo; +Cc: ath9k-devel, linux-wireless, sw, Antonio Quartulli
In-Reply-To: <87lgxbxl2d.fsf@purkki.adurom.net>
[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]
On Wed, Oct 26, 2016 at 05:05:14PM +0300, Kalle Valo wrote:
> Antonio Quartulli <a@unstable.cc> writes:
>
> > From: Antonio Quartulli <antonio@open-mesh.com>
> >
> > This patch was crafted long time ago to work around a key cache
> > corruption problem on ath9k chipsets.
> >
> > The workaround consists in periodically triggering a worker that
> > uploads all the keys to the HW cache. The worker is triggered also
> > when the vif detects 21 undecryptable packets.
> >
> >
> > This patch is based on top compat-wireless-2015-10-26.
> >
> >
> > I was asked to release this code to the public and, since it is
> > GPL, I am now doing it.
>
> GPL? AFAICS ath9k is under ISC. I'm not sure what you mean with the
> sentence above, but it's possible to interpret it so that this patch is
> not under ISC license, which is problematic.
Honestly, my sentence was just a way to say "it makes sense to release this
patch to the public".
If it needs to be ISC in order to be merged, then it can be released under ISC.
I don't want to enter a legal case :) I just to make the patch public
Cheers,
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [ath9k-devel] [NOT FOR MERGE] ath9k: work around key cache corruption
From: Ferry Huberts @ 2016-10-26 14:43 UTC (permalink / raw)
To: Kalle Valo, Antonio Quartulli
Cc: ath9k-devel, linux-wireless, Antonio Quartulli
In-Reply-To: <87lgxbxl2d.fsf@purkki.adurom.net>
do a grep in the kernel: dual license bsd/gpl
On 26/10/16 16:05, Kalle Valo wrote:
> Antonio Quartulli <a@unstable.cc> writes:
>
>> From: Antonio Quartulli <antonio@open-mesh.com>
>>
>> This patch was crafted long time ago to work around a key cache
>> corruption problem on ath9k chipsets.
>>
>> The workaround consists in periodically triggering a worker that
>> uploads all the keys to the HW cache. The worker is triggered also
>> when the vif detects 21 undecryptable packets.
>>
>>
>> This patch is based on top compat-wireless-2015-10-26.
>>
>>
>> I was asked to release this code to the public and, since it is
>> GPL, I am now doing it.
>
> GPL? AFAICS ath9k is under ISC. I'm not sure what you mean with the
> sentence above, but it's possible to interpret it so that this patch is
> not under ISC license, which is problematic.
>
--
Ferry Huberts
^ permalink raw reply
* RE: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv
From: Amitkumar Karwar @ 2016-10-26 15:23 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Brian Norris, linux-wireless@vger.kernel.org, Cathy Luo,
Nishant Sarmukadam
In-Reply-To: <20161025163520.GA10979@dtor-ws>
Hi Dmitry,
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Tuesday, October 25, 2016 10:05 PM
> To: Amitkumar Karwar
> Cc: Brian Norris; linux-wireless@vger.kernel.org; Cathy Luo; Nishant
> Sarmukadam
> Subject: Re: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing'
> in shutdown_drv
>
> On Tue, Oct 25, 2016 at 04:11:14PM +0000, Amitkumar Karwar wrote:
> > Hi Dmitry,
> >
> > > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> > > Sent: Tuesday, October 25, 2016 5:28 AM
> > > To: Brian Norris
> > > Cc: Amitkumar Karwar; linux-wireless@vger.kernel.org; Cathy Luo;
> > > Nishant Sarmukadam
> > > Subject: Re: [PATCH 2/5] mwifiex: use spinlock for
> 'mwifiex_processing'
> > > in shutdown_drv
> > >
> > > On Mon, Oct 24, 2016 at 12:19:15PM -0700, Brian Norris wrote:
> > > > On Mon, Oct 24, 2016 at 07:51:29PM +0530, Amitkumar Karwar wrote:
> > > > > This variable is guarded by spinlock at all other places. This
> > > > > patch takes care of missing spinlock usage in
> mwifiex_shutdown_drv().
> > > > >
> > > > > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > > > > ---
> > > > > drivers/net/wireless/marvell/mwifiex/init.c | 3 +++
> > > > > 1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/init.c
> > > > > b/drivers/net/wireless/marvell/mwifiex/init.c
> > > > > index 82839d9..8e5e424 100644
> > > > > --- a/drivers/net/wireless/marvell/mwifiex/init.c
> > > > > +++ b/drivers/net/wireless/marvell/mwifiex/init.c
> > > > > @@ -670,11 +670,14 @@ mwifiex_shutdown_drv(struct
> > > > > mwifiex_adapter
> > > > > *adapter)
> > > > >
> > > > > adapter->hw_status = MWIFIEX_HW_STATUS_CLOSING;
> > > > > /* wait for mwifiex_process to complete */
> > > > > + spin_lock_irqsave(&adapter->main_proc_lock, flags);
> > > > > if (adapter->mwifiex_processing) {
> > > >
> > > > I'm not quite sure why we have this check in the first place; if
> > > > the main process is still running, then don't we have bigger
> > > > problems here anyway? But this is definitely the "right" way to
> check this flag, so:
> > >
> > > No, this is definitely not right way to check it. What exactly does
> > > this spinlock protect? It seems that the intent is to make sure we
> > > do not call mwifiex_shutdown_drv() while mwifiex_main_process() is
> executing.
> > > It even says above that we are "waiting" for it (but we do not, we
> > > may bail out or we may not, depends on luck).
> > >
> > > To implement this properly you not only need to take a lock and
> > > check the flag, but keep the lock until mwifiex_shutdown_drv() is
> > > done, or use some other way to let mwifiex_main_process() know it
> > > should not access the adapter while mwifiex_shutdown_drv() is
> running.
> > >
> > > NACK.
> > >
> >
> > As I explained in other email, here is the sequence.
> > 1) We find mwifiex_processing is true in mwifiex_shitdown_drv(). "-
> EINPROGRESS" is returned by the function and hw_status state is changed
> to MWIFIEX_HW_STATUS_CLOSING.
> > 2) We wait until main_process is completed.
> >
> > if (mwifiex_shutdown_drv(adapter) == -EINPROGRESS)
> > wait_event_interruptible(adapter->init_wait_q,
> >
> > adapter->init_wait_q_woken);
> >
> > 3) We have following check at the end of main_process()
> >
> > exit_main_proc:
> > if (adapter->hw_status == MWIFIEX_HW_STATUS_CLOSING)
> > mwifiex_shutdown_drv(adapter);
> >
> > 4) Here at the end of mwifiex_shutdown_drv(), we are setting
> > "adapter->init_wait_q_woken" and waking up the thread in point (2)
>
> 1. We do not find mwifiex_processing to be true
> 2. We proceed to try and shut down the driver
> 3. Interrupt is raised in the meantime, kicking work item
> 4. mwifiex_main_process() sees that adapter->hw_status is
> MWIFIEX_HW_STATUS_CLOSING and jumps to exit_main_proc
> 5.mwifiex_main_process() calls into mwifiex_shutdown_drv() that is now
> racing with another copy of the same.
This race won't occur. At this point of time(i.e while calling mwifiex_shutdown_drv() in deinit), following things are completed. We don't expect mwifiex_main_process() to be scheduled.
1) Connection to peer device is terminated at the beginning of teardown thread. So we don't receive any Tx data from kernel.
2) Last command SHUTDOWN is exchanged with firmware. So there won't be any activity/interrupt from firmware.
3) Interrupts are disabled.
4) "adapter->surprise_removed" flag is set. It will skip mwifiex_main_process() calls.
-----------
static void mwifiex_main_work_queue(struct work_struct *work)
{
struct mwifiex_adapter *adapter =
container_of(work, struct mwifiex_adapter, main_work);
if (adapter->surprise_removed)
return;
mwifiex_main_process(adapter);
}
----------
5) We have "mwifiex_terminate_workqueue(adapter)" call to flush and destroy workqueue.
>
> It seems to me that mwifiex_main_process() is [almost] always used from
> a workqueue. Can you instead of sprinkling spinlocks ensure that
> mwifiex_shutdown_drv():
>
> 1. Inhibits scheduling of mwifiex_main_process()
> 2. Does cancel_work_sync(...) to ensure that mwifiex_main_process() does not run
> 3. Continues shutting down the driver.
I think, this is already taken care of in current implementation.
>
> Alternatively, do these have to be spinlocks? Can you make them mutexes
> and take them for the duration of mwifiex_main_process() and
> mwifiex_shutdown_drv() and others, as needed?
>
As I explained above, there won't be a race between mwifiex_main_process() and mwifiex_shutdown_drv().
Let me know if you think otherwise and have any suggestions.
Regards,
Amitkumar
^ permalink raw reply
* Re: [PATCH 2/8] mac80211: Allow AUTH_DATA to be used for FILS
From: Malinen, Jouni @ 2016-10-26 15:36 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1477459800.4059.1.camel@sipsolutions.net>
T24gV2VkLCBPY3QgMjYsIDIwMTYgYXQgMDc6MzA6MDBBTSArMDIwMCwgSm9oYW5uZXMgQmVyZyB3
cm90ZToNCj4gDQo+ID4gwqAJaWYgKHJlcS0+YXV0aF9kYXRhX2xlbiA+PSA0KSB7DQo+ID4gLQkJ
X19sZTE2ICpwb3MgPSAoX19sZTE2ICopIHJlcS0+YXV0aF9kYXRhOw0KPiA+IC0JCWF1dGhfZGF0
YS0+c2FlX3RyYW5zID0gbGUxNl90b19jcHUocG9zWzBdKTsNCj4gPiAtCQlhdXRoX2RhdGEtPnNh
ZV9zdGF0dXMgPSBsZTE2X3RvX2NwdShwb3NbMV0pOw0KPiA+ICsJCWlmIChyZXEtPmF1dGhfdHlw
ZSA9PSBOTDgwMjExX0FVVEhUWVBFX1NBRSkgew0KPiA+ICsJCQlfX2xlMTYgKnBvcyA9IChfX2xl
MTYgKikgcmVxLT5hdXRoX2RhdGE7DQo+ID4gKwkJCWF1dGhfZGF0YS0+c2FlX3RyYW5zID0gbGUx
Nl90b19jcHUocG9zWzBdKTsNCj4gPiArCQkJYXV0aF9kYXRhLT5zYWVfc3RhdHVzID0gbGUxNl90
b19jcHUocG9zWzFdKTsNCj4gPiArCQl9DQo+ID4gwqAJCW1lbWNweShhdXRoX2RhdGEtPmRhdGEs
IHJlcS0+YXV0aF9kYXRhICsgNCwNCj4gPiDCoAkJwqDCoMKgwqDCoMKgwqByZXEtPmF1dGhfZGF0
YV9sZW4gLSA0KTsNCj4gPiDCoAkJYXV0aF9kYXRhLT5kYXRhX2xlbiArPSByZXEtPmF1dGhfZGF0
YV9sZW4gLSA0Ow0KPiANCj4gSG1tLiBEbyB3ZSByZWFsbHkgd2FudCB0byBzdGlsbCBza2lwIHRo
ZSBmaXJzdCBmb3VyIGJ5dGVzIG9mIHRoZSBkYXRhDQo+IHVzZXJzcGFjZSBwYXNzZWQ/IFRoYXQg
c2VlbXMgYSBiaXQgc3RyYW5nZSB0byBtZS4gVGhlIGRvY3MgaW4gbmw4MDIxMS5oDQo+IGRvIHNh
eSBpdCB0aGF0IHdheSBub3csIGJ1dCBzaG91bGQgd2UgcmVhbGx5IGluY2x1ZGUgYSBkdW1teQ0K
PiBBdXRoZW50aWNhdGlvbiB0cmFuc2FjdGlvbiBzZXF1ZW5jZSBudW1iZXIgZmllbGQ/DQoNClRo
aXMgaXMgYWRtaXR0ZWRseSBhIGJpdCBzdHJhbmdlIGRlc2lnbiB3aXRoIHRoYXQgc3BlY2lhbCBj
YXNlIG5lZWRlZA0KZm9yIFNBRS4gSWYgd2Ugd2VyZSB0byBkZXNpZ24gdGhlIFNBRSBjYXNlIG5v
dyBpbiBjb21iaW5hdGlvbiB3aXRoIEZJTFMsDQpJIGd1ZXNzIHRoaXMgd291bGQgYmUgcXVpdGUg
ZGlmZmVyZW50IChlLmcuLCBzZXBhcmF0ZSBhdHRyaWJ1dGVzIGZvcg0KQXV0aGVudGljYXRpb24g
dHJhbnNhY3Rpb24gc2VxdWVuY2UgbnVtYmVyIGFuZCBTdGF0dXMgY29kZSkuIFVubGlrZSB0aGUN
Cm1lc2ggdXNlIGNhc2Ugd2l0aCBTQUUsIEZJTFMgaXMgb25seSBiZXR3ZWVuIGFuIEFQIGFuZCBh
IHN0YXRpb24gYW5kIGFzDQpzdWNoLCB0aGVyZSB3b3VsZCBub3QgcmVhbGx5IGJlIGEgY2FzZSB3
aGVyZSB0aGUgc3RhdGlvbiB3b3VsZCBzZW5kIGFuDQpBdXRoZW50aWNhdGlvbiBmcmFtZSB3aXRo
IG5vbi16ZXJvIFN0YXR1cyBjb2RlLg0KDQpBIGZ1dHVyZSBhbWVuZG1lbnQgbWlnaHQgZGVmaW5l
IGEgbmV3IGF1dGhlbnRpY2F0aW9uIGFsZ29yaXRobSB0aGF0IGVuZHMNCnVwIHVzaW5nIG1vcmUg
dGhhbiBhIHNpbmdsZSBBdXRoZW50aWNhdGlvbiBmcmFtZSBleGNoYW5nZS4gSW4gc3VjaCBhDQpj
YXNlLCB3ZSB3b3VsZCBhY3R1YWxseSBoYXZlIG5lZWQgZm9yIEF1dGhlbnRpY2F0aW9uIHRyYW5z
YWN0aW9uDQpzZXF1ZW5jZSBudW1iZXIgZXZlbiB0aG91Z2ggRklMUyBkb2Vzbid0IG5lZWQgaXQu
DQoNCkkgdGhpbmsgSSdkIHJhdGhlciBtYWludGFpbiBhIGNvbnNpc3RlbnQgYXR0cmlidXRlIGRl
c2lnbiBmb3IgYWxsDQphdXRoZW50aWNhdGlvbiBhbGdvcml0aG1zIGFuZCBsZWF2ZSB0aGlzIGFz
LWlzIG5vdy4gQW5vdGhlciBvcHRpb24gd291bGQNCmJlIHRvIG5vdCBhcHBseSB0aGUgcmVuYW1l
IFNBRSBhdHRyaWJ1dGVzIHBhdGNoIGFuZCBkZWZpbmUgc29tZXRoaW5nIG5ldw0KYXMgYSBtb3Jl
IGdlbmVyaWMgc29sdXRpb24sIGJ1dCBJJ20gbm90IHN1cmUgdGhlcmUgaXMgc3VmZmljaWVudA0K
anVzdGlmaWNhdGlvbiBmb3IgdGhlIGFkZGVkIGNvbXBsZXhpdHkgc2luY2Ugd2UgY2Fubm90IHJl
YWxseSBnZXQgcmlkIG9mDQp0aGUgY3VycmVudCBTQUUgZGVzaWduIGFueSB0aW1lIHNvb24uDQoN
Ci0tIA0KSm91bmkgTWFsaW5lbiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgUEdQIGlkIEVGQzg5NUZB
^ permalink raw reply
* Re: [PATCH] rtl8xxxu: Add D-Link DWA-131 rev E1
From: Jes Sorensen @ 2016-10-26 15:55 UTC (permalink / raw)
To: Barry Day; +Cc: linux-wireless, Kalle Valo
In-Reply-To: <20161025202928.GA10934@box64.home.org>
Barry Day <briselec@gmail.com> writes:
> This is a rtl8192eu dongle and has been tested
>
> Signed-off-by: Barry Day <briselec@gmail.com>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Thanks, I'll add it to my queue, unless Kalle grabs it first. Glad to
know it's working for you!
Jes
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 04141e5..d426836 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -6009,7 +6009,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
> untested = 0;
> break;
> case 0x2001:
> - if (id->idProduct == 0x3308)
> + if (id->idProduct == 0x3308 || id->idProduct == 0x3319)
> untested = 0;
> break;
> case 0x2357:
> @@ -6188,6 +6188,8 @@ static struct usb_device_id dev_table[] = {
> .driver_info = (unsigned long)&rtl8723au_fops},
> {USB_DEVICE_AND_INTERFACE_INFO(USB_VENDOR_ID_REALTEK, 0x818b, 0xff, 0xff, 0xff),
> .driver_info = (unsigned long)&rtl8192eu_fops},
> +{USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x3319, 0xff, 0xff, 0xff),
> + .driver_info = (unsigned long)&rtl8192eu_fops},
> /* Tested by Myckel Habets */
> {USB_DEVICE_AND_INTERFACE_INFO(0x2357, 0x0109, 0xff, 0xff, 0xff),
> .driver_info = (unsigned long)&rtl8192eu_fops},
^ permalink raw reply
* Re: [PATCH 2/8] mac80211: Allow AUTH_DATA to be used for FILS
From: Johannes Berg @ 2016-10-26 16:10 UTC (permalink / raw)
To: Malinen, Jouni; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <20161026153559.GA13254@jouni.qca.qualcomm.com>
> This is admittedly a bit strange design with that special case needed
> for SAE. If we were to design the SAE case now in combination with
> FILS, I guess this would be quite different (e.g., separate
> attributes for Authentication transaction sequence number and Status
> code). Unlike the mesh use case with SAE, FILS is only between an AP
> and a station and as such, there would not really be a case where the
> station would send an Authentication frame with non-zero Status code.
>
> A future amendment might define a new authentication algorithm that
> ends up using more than a single Authentication frame exchange. In
> such a case, we would actually have need for Authentication
> transaction sequence number even though FILS doesn't need it.
>
> I think I'd rather maintain a consistent attribute design for all
> authentication algorithms and leave this as-is now. Another option
> would be to not apply the rename SAE attributes patch and define
> something new as a more generic solution, but I'm not sure there is
> sufficient justification for the added complexity since we cannot
> really get rid of the current SAE design any time soon.
Yes, fair point.
Maybe you can clarify the nl80211 attribute documentation wrt. this? It
just states that it starts with the Authentication transaction sequence
field, but afaict that's not true, it also has the status code field,
which is also ignored here.
johannes
^ permalink raw reply
* Re: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv
From: Dmitry Torokhov @ 2016-10-26 16:36 UTC (permalink / raw)
To: Amitkumar Karwar
Cc: Brian Norris, linux-wireless@vger.kernel.org, Cathy Luo,
Nishant Sarmukadam
In-Reply-To: <24731897638e42ba8b05acd6afafef3b@SC-EXCH04.marvell.com>
Hi Amit,
On Wed, Oct 26, 2016 at 03:23:08PM +0000, Amitkumar Karwar wrote:
>
> This race won't occur. At this point of time(i.e while calling mwifiex_shutdown_drv() in deinit), following things are completed. We don't expect mwifiex_main_process() to be scheduled.
> 1) Connection to peer device is terminated at the beginning of teardown thread. So we don't receive any Tx data from kernel.
> 2) Last command SHUTDOWN is exchanged with firmware. So there won't be any activity/interrupt from firmware.
> 3) Interrupts are disabled.
> 4) "adapter->surprise_removed" flag is set. It will skip mwifiex_main_process() calls.
>
> -----------
> static void mwifiex_main_work_queue(struct work_struct *work)
> {
> struct mwifiex_adapter *adapter =
> container_of(work, struct mwifiex_adapter, main_work);
>
> if (adapter->surprise_removed)
> return;
> mwifiex_main_process(adapter);
> }
> ----------
> 5) We have "mwifiex_terminate_workqueue(adapter)" call to flush and destroy workqueue.
OK, but if interrupts are disabled and you ensure that work is flushed
or completed before you call mwifiex_shutdown_drv() then I do not
understand why you need all of this at all? Why do you need to check
status in mwifiex_shutdown_drv() and why do you want
mwifiex_main_process() to call mwifiex_shutdown_drv() in certain cases?
Can you simply remove all this stuff?
Thanks.
--
Dmitry
^ permalink raw reply
* RE: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv
From: Amitkumar Karwar @ 2016-10-26 16:59 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Brian Norris, linux-wireless@vger.kernel.org, Cathy Luo,
Nishant Sarmukadam
In-Reply-To: <20161026163607.GA3989@dtor-ws>
Hi Dmitry,
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Wednesday, October 26, 2016 10:06 PM
> To: Amitkumar Karwar
> Cc: Brian Norris; linux-wireless@vger.kernel.org; Cathy Luo; Nishant
> Sarmukadam
> Subject: Re: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing'
> in shutdown_drv
>
> Hi Amit,
>
> On Wed, Oct 26, 2016 at 03:23:08PM +0000, Amitkumar Karwar wrote:
> >
> > This race won't occur. At this point of time(i.e while calling
> mwifiex_shutdown_drv() in deinit), following things are completed. We
> don't expect mwifiex_main_process() to be scheduled.
> > 1) Connection to peer device is terminated at the beginning of
> teardown thread. So we don't receive any Tx data from kernel.
> > 2) Last command SHUTDOWN is exchanged with firmware. So there won't be
> any activity/interrupt from firmware.
> > 3) Interrupts are disabled.
> > 4) "adapter->surprise_removed" flag is set. It will skip
> mwifiex_main_process() calls.
> >
> > -----------
> > static void mwifiex_main_work_queue(struct work_struct *work) {
> > struct mwifiex_adapter *adapter =
> > container_of(work, struct mwifiex_adapter, main_work);
> >
> > if (adapter->surprise_removed)
> > return;
> > mwifiex_main_process(adapter); }
> > ----------
> > 5) We have "mwifiex_terminate_workqueue(adapter)" call to flush and
> destroy workqueue.
>
> OK, but if interrupts are disabled and you ensure that work is flushed
> or completed before you call mwifiex_shutdown_drv() then I do not
> understand why you need all of this at all? Why do you need to check
> status in mwifiex_shutdown_drv() and why do you want
> mwifiex_main_process() to call mwifiex_shutdown_drv() in certain cases?
> Can you simply remove all this stuff?
>
I agree. This code is there for long time. I will prepare a patch for this cleanup work.
Regards,
Amitkumar
^ permalink raw reply
* Re: [PATCH v6] mwifiex: parse device tree node for PCIe
From: Brian Norris @ 2016-10-26 20:17 UTC (permalink / raw)
To: Rajat Jain
Cc: linux-wireless, devicetree, Xinming Hu, Amitkumar Karwar,
Brian Norris, Kalle Valo, Rob Herring, rajatxjain,
Dmitry Torokhov
In-Reply-To: <1477084869-15612-1-git-send-email-rajatja@google.com>
Hi Rajat,
On Fri, Oct 21, 2016 at 02:21:09PM -0700, Rajat Jain wrote:
> From: Xinming Hu <huxm@marvell.com>
>
> This patch derives device tree node from pcie bus layer framework, and
> fixes a minor memory leak in mwifiex_pcie_probe() (in failure path).
> Device tree bindings file has been renamed(marvell-sd8xxx.txt ->
> marvell-8xxx.txt) to accommodate PCIe changes.
>
> Signed-off-by: Xinming Hu <huxm@marvell.com>
> Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> Signed-off-by: Rajat Jain <rajatja@google.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> ---
> v2: Included vendor and product IDs in compatible strings for PCIe
> chipsets(Rob Herring)
> v3: Patch is created using -M option so that it will only include diff of
> original and renamed files(Rob Herring)
> Resend v3: Resending the patch because I missed to include device tree mailing
> while sending v3.
> v4: Fix error handling, also move-on even if no device tree node is present.
> v5: Update commit log to include memory leak, return -EINVAL instead of -1.
I've been working on reworking some bugfixes for this driver, and I
noticed we have some problems w.r.t. memory leaks, and the "memory leak"
fix is not actually a fix. See below.
> v6: Remove an unnecessary error print, fix typo in commit log
>
> .../{marvell-sd8xxx.txt => marvell-8xxx.txt} | 8 +++--
> drivers/net/wireless/marvell/mwifiex/pcie.c | 36 +++++++++++++++++++---
> drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 3 +-
> 3 files changed, 39 insertions(+), 8 deletions(-)
> rename Documentation/devicetree/bindings/net/wireless/{marvell-sd8xxx.txt => marvell-8xxx.txt} (91%)
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> similarity index 91%
> rename from Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
> rename to Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> index c421aba..dfe5f8e 100644
> --- a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> @@ -1,8 +1,8 @@
> -Marvell 8897/8997 (sd8897/sd8997) SDIO devices
> +Marvell 8897/8997 (sd8897/sd8997/pcie8997) SDIO/PCIE devices
> ------
>
> -This node provides properties for controlling the marvell sdio wireless device.
> -The node is expected to be specified as a child node to the SDIO controller that
> +This node provides properties for controlling the marvell sdio/pcie wireless device.
> +The node is expected to be specified as a child node to the SDIO/PCIE controller that
> connects the device to the system.
>
> Required properties:
> @@ -10,6 +10,8 @@ Required properties:
> - compatible : should be one of the following:
> * "marvell,sd8897"
> * "marvell,sd8997"
> + * "pci11ab,2b42"
> + * "pci1b4b,2b42"
>
> Optional properties:
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index 3c3c4f1..f7c84d3 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -37,6 +37,22 @@ static struct mwifiex_if_ops pcie_ops;
>
> static struct semaphore add_remove_card_sem;
>
> +static const struct of_device_id mwifiex_pcie_of_match_table[] = {
> + { .compatible = "pci11ab,2b42" },
> + { .compatible = "pci1b4b,2b42" },
> + { }
> +};
> +
> +static int mwifiex_pcie_probe_of(struct device *dev)
> +{
> + if (!of_match_node(mwifiex_pcie_of_match_table, dev->of_node)) {
> + dev_err(dev, "required compatible string missing\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int
> mwifiex_map_pci_memory(struct mwifiex_adapter *adapter, struct sk_buff *skb,
> size_t size, int flags)
> @@ -178,6 +194,7 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
> const struct pci_device_id *ent)
> {
> struct pcie_service_card *card;
> + int ret;
>
> pr_debug("info: vendor=0x%4.04X device=0x%4.04X rev=%d\n",
> pdev->vendor, pdev->device, pdev->revision);
> @@ -199,13 +216,24 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
> card->pcie.can_ext_scan = data->can_ext_scan;
> }
>
> - if (mwifiex_add_card(card, &add_remove_card_sem, &pcie_ops,
> - MWIFIEX_PCIE)) {
> - pr_err("%s failed\n", __func__);
> - return -1;
> + /* device tree node parsing and platform specific configuration*/
> + if (pdev->dev.of_node) {
> + ret = mwifiex_pcie_probe_of(&pdev->dev);
> + if (ret)
> + goto err_free;
> }
>
> + ret = mwifiex_add_card(card, &add_remove_card_sem, &pcie_ops,
> + MWIFIEX_PCIE);
> + if (ret) {
> + pr_err("%s failed\n", __func__);
> + goto err_free;
For most error cases in mwifiex_add_card(), we call cleanup_if(), which
currently calls kfree(card). It's currently unbalanced, so there are
*some* cases that leak. But...
> + }
> return 0;
> +
> +err_free:
> + kfree(card);
That means that most of the time, this is actually a double-free. I'd
rather have the leak than the double-free :)
Anyway, I have a patch in the works (as part of some device
init/teardown bugfixes) that will convert the allocation to
devm_kzalloc() and drop the kfree()'ing. So that'll fix the
aforementioned bug.
In your next revision (sorry), please just drop this "leak" fix.
Regards,
Brian
> + return ret;
> }
>
> /*
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> index 2a162c3..c8dccf5 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> @@ -2218,7 +2218,8 @@ int mwifiex_sta_init_cmd(struct mwifiex_private *priv, u8 first_sta, bool init)
> * The cal-data can be read from device tree and/or
> * a configuration file and downloaded to firmware.
> */
> - if (priv->adapter->iface_type == MWIFIEX_SDIO &&
> + if ((priv->adapter->iface_type == MWIFIEX_SDIO ||
> + priv->adapter->iface_type == MWIFIEX_PCIE) &&
> adapter->dev->of_node) {
> adapter->dt_node = adapter->dev->of_node;
> if (of_property_read_u32(adapter->dt_node,
> --
> 2.8.0.rc3.226.g39d4020
>
^ permalink raw reply
* Re: compex wle900vx (ath10k) problem on 4.4.24 / armv7
From: Oliver Zemann @ 2016-10-26 20:25 UTC (permalink / raw)
To: Matthias Klein, Michal Kazior, Jonas Gorski; +Cc: linux-wireless
In-Reply-To: <em1c0906d6-d30a-4f2c-bafb-bfec0d18a58d@nb-mak>
>
>> I (naively) went through pci/pm git log and found the following was
>> applied on 4.7-rc2 (i.e. prior to 4.7 release):
>>
>> commit 006d44e49a259b39947366728d65a873a19aadc0
>> Author: Mika Westerberg <mika.westerberg@linux.intel.com>
>> Date: Thu Jun 2 11:17:15 2016 +0300
>>
>> PCI: Add runtime PM support for PCIe ports
>>
>> From reading the commit log it seems to me like it could be it.
>>
>> ath10k tries to wake up the device during probing before it starts
>> talking to it and it does so through MMIO/PCI config space. If it's
>> not mapped properly then driver will not be able to wake it up and
>> will timeout waiting for it.
>>
>> Can you try cherry-picking it into your 4.4.24 and see if it helps?
>>
>>
> Thanks for the help!
>
> Until now I did not compile a kernel for the board.
> I will give it a try and come back with the results ...
>
>
> Best regards,
> Matthias
>
Any news on that? I am in contact with the support of SolidRun
(clearfog). Unfortunately they will stay at 4.4.x - no upgrade to 4.8
nor 4.9 planned. Unfortunately the commit (006d44e) is incompatible to
4.4. because pci_dev does not know bridge_d3
^ permalink raw reply
* Re: [PATCH v6] mwifiex: parse device tree node for PCIe
From: Dmitry Torokhov @ 2016-10-26 20:46 UTC (permalink / raw)
To: Brian Norris
Cc: Rajat Jain, linux-wireless, devicetree, Xinming Hu,
Amitkumar Karwar, Brian Norris, Kalle Valo, Rob Herring,
rajatxjain
In-Reply-To: <20161026201735.GA10192@localhost>
On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
> Hi Rajat,
>
> On Fri, Oct 21, 2016 at 02:21:09PM -0700, Rajat Jain wrote:
> > From: Xinming Hu <huxm@marvell.com>
> >
> > This patch derives device tree node from pcie bus layer framework, and
> > fixes a minor memory leak in mwifiex_pcie_probe() (in failure path).
> > Device tree bindings file has been renamed(marvell-sd8xxx.txt ->
> > marvell-8xxx.txt) to accommodate PCIe changes.
> >
> > Signed-off-by: Xinming Hu <huxm@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > Signed-off-by: Rajat Jain <rajatja@google.com>
> > Reviewed-by: Brian Norris <briannorris@chromium.org>
> > ---
> > v2: Included vendor and product IDs in compatible strings for PCIe
> > chipsets(Rob Herring)
> > v3: Patch is created using -M option so that it will only include diff of
> > original and renamed files(Rob Herring)
> > Resend v3: Resending the patch because I missed to include device tree mailing
> > while sending v3.
> > v4: Fix error handling, also move-on even if no device tree node is present.
> > v5: Update commit log to include memory leak, return -EINVAL instead of -1.
>
> I've been working on reworking some bugfixes for this driver, and I
> noticed we have some problems w.r.t. memory leaks, and the "memory leak"
> fix is not actually a fix. See below.
Sorry, I just saw this... Why do we need devicetree data for
discoverable bus (PCI)? How does the driver work on systems that do not
use DT? Why do we need them to behave differently?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v6] mwifiex: parse device tree node for PCIe
From: Brian Norris @ 2016-10-26 20:56 UTC (permalink / raw)
To: Rajat Jain
Cc: Dmitry Torokhov, linux-wireless, devicetree, Xinming Hu,
Amitkumar Karwar, Brian Norris, Kalle Valo, Rob Herring,
Rajat Jain
In-Reply-To: <CACK8Z6H1ACBo7nDxaXR3Vn9cPdyRqxxzdgYz7vNmojiSvjbW5A@mail.gmail.com>
On Wed, Oct 26, 2016 at 01:51:48PM -0700, Rajat Jain wrote:
> On Wed, Oct 26, 2016 at 1:46 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
> Sorry, I just saw this... Why do we need devicetree data for
> discoverable bus (PCI)? How does the driver work on systems that do not
> use DT? Why do we need them to behave differently?
>
> There are a couple of out-of-band GPIO pins from Marvell chip that can
> serve as wake-up pins (wake up the CPU when asserted). The Marvell chip
> has to be told which GPIO pin is to be used as the wake-up pin. The pin to
> be used is system / platform dependent. (On some systems it could be
> GPIO13, on others it could be GPIO14 etc depending on how the marvell chip
> is wired up to the CPU).
There's also calibration data. See "marvell,caldata*" and
"marvell,wakeup-pin" properties. Currently only for SDIO, in
Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt, but
we're adding support for PCIe.
Brian
^ permalink raw reply
* Re: [PATCH 7/8] mac80211: FILS AEAD protection for station mode association frames
From: Malinen, Jouni @ 2016-10-26 21:04 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <1477460999.4059.13.camel@sipsolutions.net>
On Wed, Oct 26, 2016 at 07:49:59AM +0200, Johannes Berg wrote:
> > +static u8 *fils_find_session(u8 *pos, u8 *end)
> Hmm. I think we should try to write this in terms of cfg80211_find_ie,
> or perhaps cfg80211_find_ie_match, maybe we need to extend those but
> this won't be the only one using the 255 escape.
>=20
> Perhaps just making the eid passed there be a u16, and extending the
> EID definitions appropriately?
Will do that based on our discussion on the details (a new wrapper
function).
> > + if (!session) {
> > + sdata_info(sdata,
> Should we really print the message this prominently? It seems like an
> error case that we may not want to log at all, in case somebody tries
> to flood us with such frames?
...
> Likewise here. Perhaps make these mlme_dbg() or so instead?
These are pretty useful messages for debugging at least now that FILS is
still so new.. Once it gets more mature and has documented
interoperability between independent implementations, we may consider
removing the messages since they would not really show up during normal
operations and would not help much an actual end user. Anyway, I'll
replace them with mlme_dbg() for now.
> > + if (req->fils_nonces)
> > + assoc_data_len +=3D 2 * FILS_NONCE_LEN;
>=20
> Is this really correct? It seems like a bit of an artifact of initially
> having had the nonces in a variable part of the struct?
>=20
> Or perhaps they have to go into the frame in some place that I missed?
> Please add a comment if so.
Ah, looks like I forgot that there and req->fils_kek_len for that
matter. I had initially thought of adding these as dynamically allocated
components at the end of the buffer, but after seeing how req->ie[] was
used, gave up on that extra complexity and simply used fixed size arrays
since both the KEK and FILS nonces do have a known fixed size and we can
easily "waste" that memory in struct ieee80211_mgd_assoc_data for
non-FILS cases without causing any noticeable impact.
> Also, you're never checking req->fils_nonces_set, so you can get rid of
> that.
Indeed. I'll make the cfg80211 patch enforce that both the KEK and
nonces are set since they are both needed for all FILS cases and remove
fils_nonces_set from mac80211.
--=20
Jouni Malinen PGP id EFC895FA=
^ permalink raw reply
* Re: [PATCH v6] mwifiex: parse device tree node for PCIe
From: Dmitry Torokhov @ 2016-10-26 21:06 UTC (permalink / raw)
To: Brian Norris
Cc: Rajat Jain, linux-wireless, devicetree, Xinming Hu,
Amitkumar Karwar, Brian Norris, Kalle Valo, Rob Herring,
Rajat Jain
In-Reply-To: <20161026205634.GA13170@localhost>
On Wed, Oct 26, 2016 at 01:56:34PM -0700, Brian Norris wrote:
> On Wed, Oct 26, 2016 at 01:51:48PM -0700, Rajat Jain wrote:
> > On Wed, Oct 26, 2016 at 1:46 PM, Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> > On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
> > Sorry, I just saw this... Why do we need devicetree data for
> > discoverable bus (PCI)? How does the driver work on systems that do not
> > use DT? Why do we need them to behave differently?
> >
> > There are a couple of out-of-band GPIO pins from Marvell chip that can
> > serve as wake-up pins (wake up the CPU when asserted). The Marvell chip
> > has to be told which GPIO pin is to be used as the wake-up pin. The pin to
> > be used is system / platform dependent. (On some systems it could be
> > GPIO13, on others it could be GPIO14 etc depending on how the marvell chip
> > is wired up to the CPU).
So wakeup pin is not wired to PCIe WAKE?
> There's also calibration data. See "marvell,caldata*" and
> "marvell,wakeup-pin" properties. Currently only for SDIO, in
> Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt, but
> we're adding support for PCIe.
How would it all work if I moved the PCIe module from one device to
another?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v6] mwifiex: parse device tree node for PCIe
From: Brian Norris @ 2016-10-26 21:08 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rajat Jain, linux-wireless, devicetree, Xinming Hu,
Amitkumar Karwar, Brian Norris, Kalle Valo, Rob Herring,
Rajat Jain
In-Reply-To: <20161026210648.GE3989@dtor-ws>
On Wed, Oct 26, 2016 at 02:06:48PM -0700, Dmitry Torokhov wrote:
> On Wed, Oct 26, 2016 at 01:56:34PM -0700, Brian Norris wrote:
> > On Wed, Oct 26, 2016 at 01:51:48PM -0700, Rajat Jain wrote:
> > > On Wed, Oct 26, 2016 at 1:46 PM, Dmitry Torokhov
> > > <dmitry.torokhov@gmail.com> wrote:
> > > On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
> > > Sorry, I just saw this... Why do we need devicetree data for
> > > discoverable bus (PCI)? How does the driver work on systems that do not
> > > use DT? Why do we need them to behave differently?
> > >
> > > There are a couple of out-of-band GPIO pins from Marvell chip that can
> > > serve as wake-up pins (wake up the CPU when asserted). The Marvell chip
> > > has to be told which GPIO pin is to be used as the wake-up pin. The pin to
> > > be used is system / platform dependent. (On some systems it could be
> > > GPIO13, on others it could be GPIO14 etc depending on how the marvell chip
> > > is wired up to the CPU).
>
> So wakeup pin is not wired to PCIe WAKE?
Not in our case.
> > There's also calibration data. See "marvell,caldata*" and
> > "marvell,wakeup-pin" properties. Currently only for SDIO, in
> > Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt, but
> > we're adding support for PCIe.
>
> How would it all work if I moved the PCIe module from one device to
> another?
These boards are soldered down, at least in the case I care about.
Brian
^ permalink raw reply
* Re: [PATCH v6] mwifiex: parse device tree node for PCIe
From: Rajat Jain @ 2016-10-26 21:16 UTC (permalink / raw)
To: Brian Norris
Cc: Dmitry Torokhov, linux-wireless, devicetree, Xinming Hu,
Amitkumar Karwar, Brian Norris, Kalle Valo, Rob Herring,
Rajat Jain
In-Reply-To: <20161026210805.GA14423@localhost>
On Wed, Oct 26, 2016 at 2:08 PM, Brian Norris <briannorris@chromium.org> wrote:
> On Wed, Oct 26, 2016 at 02:06:48PM -0700, Dmitry Torokhov wrote:
>> On Wed, Oct 26, 2016 at 01:56:34PM -0700, Brian Norris wrote:
>> > On Wed, Oct 26, 2016 at 01:51:48PM -0700, Rajat Jain wrote:
>> > > On Wed, Oct 26, 2016 at 1:46 PM, Dmitry Torokhov
>> > > <dmitry.torokhov@gmail.com> wrote:
>> > > On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
>> > > Sorry, I just saw this... Why do we need devicetree data for
>> > > discoverable bus (PCI)? How does the driver work on systems that do not
>> > > use DT? Why do we need them to behave differently?
>> > >
>> > > There are a couple of out-of-band GPIO pins from Marvell chip that can
>> > > serve as wake-up pins (wake up the CPU when asserted). The Marvell chip
>> > > has to be told which GPIO pin is to be used as the wake-up pin. The pin to
>> > > be used is system / platform dependent. (On some systems it could be
>> > > GPIO13, on others it could be GPIO14 etc depending on how the marvell chip
>> > > is wired up to the CPU).
>>
>> So wakeup pin is not wired to PCIe WAKE?
>
> Not in our case.
>
>> > There's also calibration data. See "marvell,caldata*" and
>> > "marvell,wakeup-pin" properties. Currently only for SDIO, in
>> > Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt, but
>> > we're adding support for PCIe.
>>
>> How would it all work if I moved the PCIe module from one device to
>> another?
>
> These boards are soldered down, at least in the case I care about.
That is right. Since the out of band wake-up pin is not standard on
the PCIe connector - this feature is for soldered chips only.
>
> Brian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox