Thread (32 messages) 32 messages, 3 authors, 2013-08-21

Re: [PATCH 01/16] wcn36xx: Add main.c

From: Johannes Berg <johannes@sipsolutions.net>
Date: 2013-08-20 21:00:04

Just a cursory review ...

+		.cap = IEEE80211_HT_CAP_GRN_FLD
+			| IEEE80211_HT_CAP_SGI_20
wouldn't that usually be written as

GRN_FLD |
SGI_20 |
...

(multiple similar places)
+#ifdef CONFIG_PM
+
+static const struct wiphy_wowlan_support wowlan_support = {
+	.flags = WIPHY_WOWLAN_ANY,
+	.n_patterns = 0,
that n_patterns is pretty useless.

+#define WCN36XX_SUPPORTED_FILTERS (0)
+
+static void wcn36xx_configure_filter(struct ieee80211_hw *hw,
+				       unsigned int changed,
+				       unsigned int *total, u64 multicast)
+{
+	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac configure filter");
+
+	changed &= WCN36XX_SUPPORTED_FILTERS;
That's pointless
+		if (IEEE80211_KEY_FLAG_PAIRWISE & key_conf->flags) {
+			sta_priv->is_data_encrypted = true;
+			/* Reconfigure bss with encrypt_type */
+			if (NL80211_IFTYPE_STATION == vif->type)
+				wcn36xx_smd_config_bss(wcn,
+						       vif,
+						       sta,
+						       sta->addr,
+						       true);
It seems to me this should not be here but you should have mac80211 set
something in e.g. bss_conf that indicates encryption?

+	/* Not supported so far*/
+	case IEEE80211_AMPDU_TX_STOP_CONT:
+		ieee80211_stop_tx_ba_cb_irqsafe(vif, sta->addr, tid);
+		break;
+	case IEEE80211_AMPDU_TX_STOP_FLUSH:
+	case IEEE80211_AMPDU_TX_STOP_FLUSH_CONT:
+		break;
You can't just "not support" them - you have to at least stop the
aggregation session, see the commit that introduced this.
+	static const u32 cipher_suites[] = {
+		WLAN_CIPHER_SUITE_TKIP,
+		WLAN_CIPHER_SUITE_CCMP,
+	};
You actually don't want to support WEP, not even in software? Otherwise
just leave this out and mac80211 will add it.
+	wcn->hw->wiphy->iface_combinations = &if_comb;
+	wcn->hw->wiphy->n_iface_combinations = 1;
Your code with "wcn->current_vif = " etc. *really* doesn't look like you
support combinations. Are you positive this is OK?
+	wcn->hw->wiphy->max_scan_ssids = 1;
Really? You don't even have hardware scan, so why?

johannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help