[PATCH 0/4] cfg80211: mac80211: BTCOEX feature support

[PATCH 0/4] cfg80211: mac80211: BTCOEX feature support

[c_traja]

From: Tamizh chelvam <c_traja@qti.qualcomm.com>

This patchset add support for BTCOEX feature to enable/disable btcoex
and modifying btcoex priority value via nl80211.

Tamizh chelvam (4):
  cfg80211: Add support to enable or disable btcoex
  cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX
  mac80211: Add support to enable or disable btcoex
  mac80211: Add support to update btcoex priority value

 include/net/cfg80211.h       |   68 ++++++++++++++++++++++++
 include/net/mac80211.h       |   11 ++++
 include/uapi/linux/nl80211.h |   43 +++++++++++++++
 net/mac80211/cfg.c           |   17 ++++++
 net/mac80211/driver-ops.h    |   26 +++++++++
 net/mac80211/trace.h         |   14 +++++
 net/wireless/nl80211.c       |  119 ++++++++++++++++++++++++++++++++++++++++++
 net/wireless/rdev-ops.h      |   24 +++++++++
 net/wireless/trace.h         |   48 +++++++++++++++++
 9 files changed, 370 insertions(+)

-- 
1.7.9.5

[PATCH 1/4] cfg80211: Add support to enable or disable btcoex

[c_traja]

From: Tamizh chelvam <c_traja@qti.qualcomm.com>

This patch adds support to enable or disable btcoex by
adding NL80211_ATTR_WIPHY_BTCOEX_ENABLE attribute in
NL80211_CMD_SET_WIPHY command. By default BTCOEX disabled in driver.

Signed-off-by: Tamizh chelvam <c_traja@qti.qualcomm.com>
---
 include/net/cfg80211.h       |    3 +++
 include/uapi/linux/nl80211.h |    6 ++++++
 net/wireless/nl80211.c       |   18 ++++++++++++++++++
 net/wireless/rdev-ops.h      |   11 +++++++++++
 net/wireless/trace.h         |    5 +++++
 5 files changed, 43 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9390365..919ed1d 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2735,6 +2735,8 @@ struct cfg80211_nan_func {
  * @nan_change_conf: changes NAN configuration. The changed parameters must
  *	be specified in @changes (using &enum cfg80211_nan_conf_changes);
  *	All other parameters must be ignored.
+ * @set_btcoex: Use this callback to call driver API when user wants to
+ *     enable/disable btcoex.
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -3011,6 +3013,7 @@ struct cfg80211_ops {
 				   struct wireless_dev *wdev,
 				   struct cfg80211_nan_conf *conf,
 				   u32 changes);
+	int     (*set_btcoex)(struct wiphy *wiphy, bool enabled);
 };
 
 /*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 1362d24..c47fe6c8 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1937,6 +1937,10 @@ 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_WIPHY_BTCOEX_ENABLE: u8 attribute for driver supporting
+ *	the btcoex feature. When used with %NL80211_CMD_SET_WIPHY it contains
+ *	either 0 for disable or 1 for enable btcoex.
+ *
  * @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 +2340,8 @@ enum nl80211_attrs {
 	NL80211_ATTR_NAN_FUNC,
 	NL80211_ATTR_NAN_MATCH,
 
+	NL80211_ATTR_WIPHY_BTCOEX_ENABLE,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 46cd489..5b77a41 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -414,6 +414,7 @@ enum nl80211_multicast_groups {
 	[NL80211_ATTR_NAN_MASTER_PREF] = { .type = NLA_U8 },
 	[NL80211_ATTR_NAN_DUAL] = { .type = NLA_U8 },
 	[NL80211_ATTR_NAN_FUNC] = { .type = NLA_NESTED },
+	[NL80211_ATTR_WIPHY_BTCOEX_ENABLE] = { .type = NLA_U8 },
 };
 
 /* policy for the key attributes */
@@ -2356,6 +2357,23 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 			return result;
 	}
 
+	if (info->attrs[NL80211_ATTR_WIPHY_BTCOEX_ENABLE]) {
+		u8 val;
+
+		if (!rdev->ops->set_btcoex)
+			return -ENOTSUPP;
+
+		val = nla_get_u8(info->attrs[NL80211_ATTR_WIPHY_BTCOEX_ENABLE]);
+
+		if (val > 1)
+			return -EINVAL;
+
+		result = rdev_set_btcoex(rdev, val);
+
+		if (result)
+			return result;
+	}
+
 	if (info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX] &&
 	    info->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]) {
 		u32 tx_ant, rx_ant;
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 11cf83c..2e547c3 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -1129,4 +1129,15 @@ static inline int rdev_set_qos_map(struct cfg80211_registered_device *rdev,
 	trace_rdev_return_int(&rdev->wiphy, ret);
 	return ret;
 }
+
+static inline int
+rdev_set_btcoex(struct cfg80211_registered_device *rdev, bool enabled)
+{
+	int ret;
+
+	trace_rdev_set_btcoex(&rdev->wiphy, enabled);
+	ret = rdev->ops->set_btcoex(&rdev->wiphy, enabled);
+	trace_rdev_return_int(&rdev->wiphy, ret);
+	return ret;
+}
 #endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index a3d0a91b..c9c6579 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -3026,6 +3026,11 @@
 		  WIPHY_PR_ARG, __entry->n_rules)
 );
 
+DEFINE_EVENT(wiphy_enabled_evt, rdev_set_btcoex,
+	TP_PROTO(struct wiphy *wiphy, bool enabled),
+	TP_ARGS(wiphy, enabled)
+);
+
 DEFINE_EVENT(wiphy_wdev_evt, rdev_abort_scan,
 	TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev),
 	TP_ARGS(wiphy, wdev)
-- 
1.7.9.5

Re: [PATCH 1/4] cfg80211: Add support to enable or disable btcoex

[Johannes Berg]

On Tue, 2016-11-08 at 18:45 +0530, c_traja@qti.qualcomm.com wrote:
> From: Tamizh chelvam <c_traja@qti.qualcomm.com>
> 
> This patch adds support to enable or disable btcoex by
> adding NL80211_ATTR_WIPHY_BTCOEX_ENABLE attribute in
> NL80211_CMD_SET_WIPHY command. By default BTCOEX disabled in driver.

I think overloading SET_WIPHY even more is a mistake - why not group
this with the new command you're introducing in the second patch?

Also, can have a flag attribute to enable/disable, or better even
disable when you're not setting any other parameters.

johannes

Re: [PATCH 1/4] cfg80211: Add support to enable or disable btcoex

[Tamizh chelvam]

Hi Johannes,

Thanks for the comments.

On 2016-12-05 20:16, Johannes Berg wrote:
> On Tue, 2016-11-08 at 18:45 +0530, c_traja@qti.qualcomm.com wrote:
>> From: Tamizh chelvam <c_traja@qti.qualcomm.com>
>> 
>> This patch adds support to enable or disable btcoex by
>> adding NL80211_ATTR_WIPHY_BTCOEX_ENABLE attribute in
>> NL80211_CMD_SET_WIPHY command. By default BTCOEX disabled in driver.
> 
> I think overloading SET_WIPHY even more is a mistake - why not group
> this with the new command you're introducing in the second patch?
> 
Sure.

> Also, can have a flag attribute to enable/disable, or better even
> disable when you're not setting any other parameters.

BTCOEX will be disabled by default. I will use 
NL80211_ATTR_WIPHY_BTCOEX_ENABLE
attribute to enable/disable BTCOEX.

[PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[c_traja]

From: Tamizh chelvam <c_traja@qti.qualcomm.com>

This change enables user to set high priority for driver supported wlan
frames when BTCOEX enabled. The drivers that expose such capability make
use of this priority table to decide to whom the radio should be shared
(either bluetooth or WLAN). When the high priority frames are queued
driver or firmware will signal to block BT activity.
Capable drivers should advertise the frame type for which it supports
BTCOEX priority configuration through btcoex_support_flags.
This will be useful when wlan needs to transfer packet to avoid
connection lost or packet drop issue when BT is active on long time.

Signed-off-by: Tamizh chelvam <c_traja@qti.qualcomm.com>
---
 include/net/cfg80211.h       |   65 +++++++++++++++++++++++++++
 include/uapi/linux/nl80211.h |   37 ++++++++++++++++
 net/wireless/nl80211.c       |  101 ++++++++++++++++++++++++++++++++++++++++++
 net/wireless/rdev-ops.h      |   13 ++++++
 net/wireless/trace.h         |   43 ++++++++++++++++++
 5 files changed, 259 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 919ed1d..d52d76b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2434,6 +2434,27 @@ struct cfg80211_nan_func {
 };
 
 /**
+ * struct cfg80211_btcoex_priority - BTCOEX support frame type
+ *
+ * This structure defines the driver supporting frame types for BTCOEX
+ *
+ * @wlan_be_preferred: best effort frames preferred over bt traffic
+ * @wlan_bk_preferred: background frames preferred over bt traffic
+ * @wlan_vi_preferred: video frames preferred over bt traffic
+ * @wlan_vo_preferred: voice frames preferred over bt traffic
+ * @wlan_beacon_preferred: beacon preferred over bt traffic
+ * @wlan_mgmt_preferred: management frames preferred ovet bt traffic
+ */
+struct cfg80211_btcoex_priority {
+	bool wlan_be_preferred;
+	bool wlan_bk_preferred;
+	bool wlan_vi_preferred;
+	bool wlan_vo_preferred;
+	bool wlan_beacon_preferred;
+	bool wlan_mgmt_preferred;
+};
+
+/**
  * struct cfg80211_ops - backend description for wireless configuration
  *
  * This struct is registered by fullmac card drivers and/or wireless stacks
@@ -2737,6 +2758,11 @@ struct cfg80211_nan_func {
  *	All other parameters must be ignored.
  * @set_btcoex: Use this callback to call driver API when user wants to
  *     enable/disable btcoex.
+ * @set_btcoex_priority: Use this callback to set wlan high
+ *     priority frames over bluetooth. Driver supported wlan frames
+ *     for the BTCOEX is exposed by btcoex_support_flags.
+ *     When BTCOEX enabled, the high priority wlan frames will have
+ *     more priority than BT.
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -3014,6 +3040,8 @@ struct cfg80211_ops {
 				   struct cfg80211_nan_conf *conf,
 				   u32 changes);
 	int     (*set_btcoex)(struct wiphy *wiphy, bool enabled);
+	int     (*set_btcoex_priority)(struct wiphy *wiphy,
+			struct cfg80211_btcoex_priority *btcoex_priority);
 };
 
 /*
@@ -3253,6 +3281,38 @@ struct wiphy_wowlan_support {
 };
 
 /**
+ * wiphy_btcoex_support_flags
+ *	This enum has the driver supported frame types for BTCOEX.
+ * @WIPHY_WLAN_BE_PREFERRED - Supports Best Effort frame for BTCOEX
+ * @WIPHY_WLAN_BK_PREFERRED - supports Background frame for BTCOEX
+ * @WIPHY_WLAN_VI_PREFERRED - supports Video frame for BTCOEX
+ * @WIPHY_WLAN_VO_PREFERRED - supports Voice frame for BTCOEX
+ * @WIPHY_WLAN_BEACON_PREFERRED - supports Beacon frame for BTCOEX
+ * @WIPHY_WLAN_MGMT_PREFERRED - supports Management frames for BTCOEX.
+ */
+
+enum wiphy_btcoex_support_flags {
+	WIPHY_WLAN_BE_PREFERRED		= BIT(0),
+	WIPHY_WLAN_BK_PREFERRED		= BIT(1),
+	WIPHY_WLAN_VI_PREFERRED		= BIT(2),
+	WIPHY_WLAN_VO_PREFERRED		= BIT(3),
+	WIPHY_WLAN_BEACON_PREFERRED	= BIT(4),
+	WIPHY_WLAN_MGMT_PREFERRED	= BIT(5),
+};
+
+/**
+ * enum wiphy_btcoex_priority - BTCOEX priority level
+ *	This enum defines priority level for BTCOEX
+ * WIPHY_WLAN_PREFERRED_LOW - low priority frames over BT traffic
+ * WIPHY_WLAN_PREFERRED_HIGH - high priority frames over BT traffic
+ */
+
+enum wiphy_btcoex_priority {
+	WIPHY_WLAN_PREFERRED_LOW = false,
+	WIPHY_WLAN_PREFERRED_HIGH = true,
+};
+
+/**
  * struct wiphy_coalesce_support - coalesce support data
  * @n_rules: maximum number of coalesce rules
  * @max_delay: maximum supported coalescing delay in msecs
@@ -3430,6 +3490,10 @@ struct wiphy_iftype_ext_capab {
  *	used since access to it is necessarily racy, use the parameter passed
  *	to the suspend() operation instead.
  *
+ * @btcoex_support_flags: This will have the driver supported
+ *	frame types for BTCOEX. This value filled by using
+ *	%enum wiphy_btcoex_support_flags while driver
+ *	initialization.
  * @ap_sme_capa: AP SME capabilities, flags from &enum nl80211_ap_sme_features.
  * @ht_capa_mod_mask:  Specify what ht_cap values can be over-ridden.
  *	If null, then none can be over-ridden.
@@ -3538,6 +3602,7 @@ struct wiphy {
 	struct cfg80211_wowlan *wowlan_config;
 #endif
 
+	u32 btcoex_support_flags;
 	u16 max_remain_on_channel_duration;
 
 	u8 max_num_pmkids;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index c47fe6c8..8d15321 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -874,6 +874,10 @@
  *	This will contain a %NL80211_ATTR_NAN_MATCH nested attribute and
  *	%NL80211_ATTR_COOKIE.
  *
+ * @NL80211_CMD_SET_BTCOEX_PRIORITY: Set high priority for driver supported
+ *	wlan frames for BTCOEX over bluetooth. High priority frame type
+ *	identified by %NL80211_ATTR_SET_BTCOEX_PRIORITY parameters.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -1069,6 +1073,8 @@ enum nl80211_commands {
 	NL80211_CMD_CHANGE_NAN_CONFIG,
 	NL80211_CMD_NAN_MATCH,
 
+	NL80211_CMD_SET_BTCOEX_PRIORITY,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -1941,6 +1947,11 @@ enum nl80211_commands {
  *	the btcoex feature. When used with %NL80211_CMD_SET_WIPHY it contains
  *	either 0 for disable or 1 for enable btcoex.
  *
+ * @NL80211_ATTR_SET_BTCOEX_PRIORITY: nested attribute for driver supporting
+ *	the BTCOEX. When used with %NL80211_CMD_SET_BTCOEX_PRIORITY it contains
+ *	attributes according &enum nl80211_btcoex_priority to indicate
+ *	which frame has high priority over BT.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2341,6 +2352,7 @@ enum nl80211_attrs {
 	NL80211_ATTR_NAN_MATCH,
 
 	NL80211_ATTR_WIPHY_BTCOEX_ENABLE,
+	NL80211_ATTR_SET_BTCOEX_PRIORITY,
 
 	/* add attributes here, update the policy in nl80211.c */
 
@@ -3551,6 +3563,31 @@ enum nl80211_chan_width {
 };
 
 /**
+ * enum nl80211_btcoex_priority - BTCOEX parameter attributes
+ *	This strcuture has enum values for driver supported wlan
+ *	frame type for BTCOEX.
+ * @NL80211_WLAN_BE_PREFERRED - Best Effort frame
+ * @NL80211_WLAN_BK_PREFERRED - Background frame
+ * @NL80211_WLAN_VI_PREFERRED - Video frame
+ * @NL80211_WLAN_VO_PREFERRED - Voice frame
+ * @NL80211_WLAN_BEACON_PREFERRED - BEACON frame
+ * @NL80211_WLAN_MGMT_PREFERRED - MGMT frame
+ */
+
+enum nl80211_btcoex_priority {
+	__NL80211_WLAN_PREFERRED_INVALID,
+	NL80211_WLAN_BE_PREFERRED,
+	NL80211_WLAN_BK_PREFERRED,
+	NL80211_WLAN_VI_PREFERRED,
+	NL80211_WLAN_VO_PREFERRED,
+	NL80211_WLAN_BEACON_PREFERRED,
+	NL80211_WLAN_MGMT_PREFERRED,
+	__NL80211_WLAN_PREFERRED_LAST,
+	NL80211_WLAN_PREFERRED_MAX =
+			__NL80211_WLAN_PREFERRED_LAST - 1,
+};
+
+/**
  * enum nl80211_bss_scan_width - control channel width for a BSS
  *
  * These values are used with the %NL80211_BSS_CHAN_WIDTH attribute.
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5b77a41..2d89919 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -415,6 +415,7 @@ enum nl80211_multicast_groups {
 	[NL80211_ATTR_NAN_DUAL] = { .type = NLA_U8 },
 	[NL80211_ATTR_NAN_FUNC] = { .type = NLA_NESTED },
 	[NL80211_ATTR_WIPHY_BTCOEX_ENABLE] = { .type = NLA_U8 },
+	[NL80211_ATTR_SET_BTCOEX_PRIORITY] = { .type = NLA_NESTED },
 };
 
 /* policy for the key attributes */
@@ -2017,6 +2018,16 @@ static int nl80211_get_wiphy(struct sk_buff *skb, struct genl_info *info)
 	return genlmsg_reply(msg, info);
 }
 
+static const struct nla_policy
+wlan_preferred_policy[NL80211_WLAN_PREFERRED_MAX + 1] = {
+	[NL80211_WLAN_BE_PREFERRED]	= { .type = NLA_FLAG },
+	[NL80211_WLAN_BK_PREFERRED]	= { .type = NLA_FLAG },
+	[NL80211_WLAN_VI_PREFERRED]	= { .type = NLA_FLAG },
+	[NL80211_WLAN_VO_PREFERRED]	= { .type = NLA_FLAG },
+	[NL80211_WLAN_BEACON_PREFERRED]	= { .type = NLA_FLAG },
+	[NL80211_WLAN_MGMT_PREFERRED]	= { .type = NLA_FLAG },
+};
+
 static const struct nla_policy txq_params_policy[NL80211_TXQ_ATTR_MAX + 1] = {
 	[NL80211_TXQ_ATTR_QUEUE]		= { .type = NLA_U8 },
 	[NL80211_TXQ_ATTR_TXOP]			= { .type = NLA_U16 },
@@ -11749,6 +11760,88 @@ static int nl80211_tdls_cancel_channel_switch(struct sk_buff *skb,
 	return 0;
 }
 
+static int
+parse_btcoex_priority(struct nlattr *tb[], struct wiphy *wiphy,
+		      struct cfg80211_btcoex_priority *btcoex_priority)
+{
+	memset(btcoex_priority, false, sizeof(*btcoex_priority));
+
+	if (tb[NL80211_WLAN_BE_PREFERRED]) {
+		if (!(wiphy->btcoex_support_flags &
+		      WIPHY_WLAN_BE_PREFERRED))
+			return -EINVAL;
+		btcoex_priority->wlan_be_preferred =
+				WIPHY_WLAN_PREFERRED_HIGH;
+	}
+	if (tb[NL80211_WLAN_BK_PREFERRED]) {
+		if (!(wiphy->btcoex_support_flags &
+		      WIPHY_WLAN_BK_PREFERRED))
+			return -EINVAL;
+		btcoex_priority->wlan_bk_preferred =
+				WIPHY_WLAN_PREFERRED_HIGH;
+	}
+	if (tb[NL80211_WLAN_VI_PREFERRED]) {
+		if (!(wiphy->btcoex_support_flags &
+		      WIPHY_WLAN_VI_PREFERRED))
+			return -EINVAL;
+		btcoex_priority->wlan_vi_preferred =
+				WIPHY_WLAN_PREFERRED_HIGH;
+	}
+	if (tb[NL80211_WLAN_VO_PREFERRED]) {
+		if (!(wiphy->btcoex_support_flags &
+		      WIPHY_WLAN_VO_PREFERRED))
+			return -EINVAL;
+		btcoex_priority->wlan_vo_preferred =
+				WIPHY_WLAN_PREFERRED_HIGH;
+	}
+	if (tb[NL80211_WLAN_BEACON_PREFERRED]) {
+		if (!(wiphy->btcoex_support_flags &
+		      WIPHY_WLAN_BEACON_PREFERRED))
+			return -EINVAL;
+		btcoex_priority->wlan_beacon_preferred =
+				WIPHY_WLAN_PREFERRED_HIGH;
+	}
+	if (tb[NL80211_WLAN_MGMT_PREFERRED]) {
+		if (!(wiphy->btcoex_support_flags &
+		      WIPHY_WLAN_MGMT_PREFERRED))
+			return -EINVAL;
+		btcoex_priority->wlan_mgmt_preferred =
+				WIPHY_WLAN_PREFERRED_HIGH;
+	}
+	return 0;
+}
+
+static int nl80211_btcoex_priority(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct net_device *dev = info->user_ptr[1];
+	struct cfg80211_btcoex_priority btcoex_priority = {};
+	struct wiphy wiphy = rdev->wiphy;
+	struct nlattr *tb[NL80211_WLAN_PREFERRED_MAX + 1];
+	int err;
+
+	if (!rdev->ops->set_btcoex_priority)
+		return -EOPNOTSUPP;
+
+	if (!(info->attrs[NL80211_ATTR_SET_BTCOEX_PRIORITY]))
+		return -EINVAL;
+
+	err = nla_parse(tb, NL80211_WLAN_PREFERRED_MAX,
+			nla_data(info->attrs[NL80211_ATTR_SET_BTCOEX_PRIORITY]),
+			nla_len(info->attrs[NL80211_ATTR_SET_BTCOEX_PRIORITY]),
+			wlan_preferred_policy);
+
+	if (err)
+		return err;
+
+	err = parse_btcoex_priority(tb, &wiphy, &btcoex_priority);
+
+	if (err)
+		return -EINVAL;
+
+	return rdev_set_btcoex_priority(rdev, dev, &btcoex_priority);
+}
+
 #define NL80211_FLAG_NEED_WIPHY		0x01
 #define NL80211_FLAG_NEED_NETDEV	0x02
 #define NL80211_FLAG_NEED_RTNL		0x04
@@ -12608,6 +12701,14 @@ static void nl80211_post_doit(const struct genl_ops *ops, struct sk_buff *skb,
 		.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
 				  NL80211_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL80211_CMD_SET_BTCOEX_PRIORITY,
+		.doit = nl80211_btcoex_priority,
+		.policy = nl80211_policy,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL80211_FLAG_NEED_WIPHY |
+				  NL80211_FLAG_NEED_RTNL,
+	},
 };
 
 /* notification functions */
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 2e547c3..f41168e 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -1140,4 +1140,17 @@ static inline int rdev_set_qos_map(struct cfg80211_registered_device *rdev,
 	trace_rdev_return_int(&rdev->wiphy, ret);
 	return ret;
 }
+
+static inline int
+rdev_set_btcoex_priority(struct cfg80211_registered_device *rdev,
+			 struct net_device *dev,
+			 struct cfg80211_btcoex_priority *btcoex_priority)
+{
+	int ret;
+
+	trace_rdev_set_btcoex_priority(&rdev->wiphy, btcoex_priority);
+	ret = rdev->ops->set_btcoex_priority(&rdev->wiphy, btcoex_priority);
+	trace_rdev_return_int(&rdev->wiphy, ret);
+	return ret;
+}
 #endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index c9c6579..5bde0cf 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -3031,6 +3031,49 @@
 	TP_ARGS(wiphy, enabled)
 );
 
+TRACE_EVENT(rdev_set_btcoex_priority,
+	TP_PROTO(struct wiphy *wiphy,
+		 struct cfg80211_btcoex_priority *btcoex_priority),
+	TP_ARGS(wiphy, btcoex_priority),
+	TP_STRUCT__entry(
+		WIPHY_ENTRY
+		__field(bool, wlan_be_preferred)
+		__field(bool, wlan_bk_preferred)
+		__field(bool, wlan_vi_preferred)
+		__field(bool, wlan_vo_preferred)
+		__field(bool, wlan_beacon_preferred)
+		__field(bool, wlan_mgmt_preferred)
+	),
+	TP_fast_assign(
+		WIPHY_ASSIGN;
+		if (btcoex_priority) {
+			__entry->wlan_be_preferred =
+				btcoex_priority->wlan_be_preferred;
+			__entry->wlan_bk_preferred =
+				btcoex_priority->wlan_bk_preferred;
+			__entry->wlan_vi_preferred =
+				btcoex_priority->wlan_vi_preferred;
+			__entry->wlan_vo_preferred =
+				btcoex_priority->wlan_vo_preferred;
+			__entry->wlan_beacon_preferred =
+				btcoex_priority->wlan_beacon_preferred;
+			__entry->wlan_mgmt_preferred =
+				btcoex_priority->wlan_mgmt_preferred;
+		}
+	),
+	TP_printk(WIPHY_PR_FMT ", wlan_be_preferred: %d, "
+		  "wlan_bk_preferred: %d, "
+		  "wlan_vi_preferred: %d,"
+		  "wlan_vo_preferred: %d, "
+		  "wlan_beacon_preferred: %d "
+		  "wlan_mgmt_preferred: %d.",
+		  WIPHY_PR_ARG, __entry->wlan_be_preferred,
+		  __entry->wlan_bk_preferred,
+		  __entry->wlan_vi_preferred,
+		  __entry->wlan_vo_preferred,
+		  __entry->wlan_beacon_preferred,
+		  __entry->wlan_mgmt_preferred)
+);
 DEFINE_EVENT(wiphy_wdev_evt, rdev_abort_scan,
 	TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev),
 	TP_ARGS(wiphy, wdev)
-- 
1.7.9.5

Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[Johannes Berg]

On Tue, 2016-11-08 at 18:45 +0530, c_traja@qti.qualcomm.com wrote:
> 
> + * struct cfg80211_btcoex_priority - BTCOEX support frame type
> + *
> + * This structure defines the driver supporting frame types for
> BTCOEX
> + *
> + * @wlan_be_preferred: best effort frames preferred over bt traffic
> + * @wlan_bk_preferred: background frames preferred over bt traffic
> + * @wlan_vi_preferred: video frames preferred over bt traffic
> + * @wlan_vo_preferred: voice frames preferred over bt traffic
> + * @wlan_beacon_preferred: beacon preferred over bt traffic
> + * @wlan_mgmt_preferred: management frames preferred ovet bt traffic

typo: over

>  
>  /**
> + * wiphy_btcoex_support_flags
> + *	This enum has the driver supported frame types for BTCOEX.
> + * @WIPHY_WLAN_BE_PREFERRED - Supports Best Effort frame for BTCOEX
> + * @WIPHY_WLAN_BK_PREFERRED - supports Background frame for BTCOEX
> + * @WIPHY_WLAN_VI_PREFERRED - supports Video frame for BTCOEX
> + * @WIPHY_WLAN_VO_PREFERRED - supports Voice frame for BTCOEX
> + * @WIPHY_WLAN_BEACON_PREFERRED - supports Beacon frame for BTCOEX
> + * @WIPHY_WLAN_MGMT_PREFERRED - supports Management frames for
> BTCOEX.
> + */

That's not making much sense to me?

> +/**
> + * enum wiphy_btcoex_priority - BTCOEX priority level
> + *	This enum defines priority level for BTCOEX
> + * WIPHY_WLAN_PREFERRED_LOW - low priority frames over BT traffic
> + * WIPHY_WLAN_PREFERRED_HIGH - high priority frames over BT traffic
> + */
> +
> +enum wiphy_btcoex_priority {
> +	WIPHY_WLAN_PREFERRED_LOW = false,
> +	WIPHY_WLAN_PREFERRED_HIGH = true,
> +};

That false/true seems just strange.

> + * @btcoex_support_flags: This will have the driver supported
> + *	frame types for BTCOEX. This value filled by using
> + *	%enum wiphy_btcoex_support_flags while driver
> + *	initialization.

The whole "will have" isn't really clear.

> + * @NL80211_ATTR_SET_BTCOEX_PRIORITY: nested attribute for driver
> supporting
> + *	the BTCOEX. When used with
> %NL80211_CMD_SET_BTCOEX_PRIORITY it contains
> + *	attributes according &enum nl80211_btcoex_priority to
> indicate
> + *	which frame has high priority over BT.

There should be no "SET" in there.

>  /**
> + * enum nl80211_btcoex_priority - BTCOEX parameter attributes
> + *	This strcuture has enum values for driver supported wlan
> + *	frame type for BTCOEX.
> + * @NL80211_WLAN_BE_PREFERRED - Best Effort frame
> + * @NL80211_WLAN_BK_PREFERRED - Background frame
> + * @NL80211_WLAN_VI_PREFERRED - Video frame
> + * @NL80211_WLAN_VO_PREFERRED - Voice frame
> + * @NL80211_WLAN_BEACON_PREFERRED - BEACON frame
> + * @NL80211_WLAN_MGMT_PREFERRED - MGMT frame
> + */
> +
> +enum nl80211_btcoex_priority {
> +	__NL80211_WLAN_PREFERRED_INVALID,
> +	NL80211_WLAN_BE_PREFERRED,
> +	NL80211_WLAN_BK_PREFERRED,
> +	NL80211_WLAN_VI_PREFERRED,
> +	NL80211_WLAN_VO_PREFERRED,
> +	NL80211_WLAN_BEACON_PREFERRED,
> +	NL80211_WLAN_MGMT_PREFERRED,
> +	__NL80211_WLAN_PREFERRED_LAST,
> +	NL80211_WLAN_PREFERRED_MAX =
> +			__NL80211_WLAN_PREFERRED_LAST - 1,
> +};

Wouldn't a bitmap be easier?

johannes

Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[Tamizh chelvam]

Hi Johannes,

Thanks for the comments.

On 2016-12-05 20:19, Johannes Berg wrote:
> On Tue, 2016-11-08 at 18:45 +0530, c_traja@qti.qualcomm.com wrote:
>>  
>> + * struct cfg80211_btcoex_priority - BTCOEX support frame type
>> + *
>> + * This structure defines the driver supporting frame types for
>> BTCOEX
>> + *
>> + * @wlan_be_preferred: best effort frames preferred over bt traffic
>> + * @wlan_bk_preferred: background frames preferred over bt traffic
>> + * @wlan_vi_preferred: video frames preferred over bt traffic
>> + * @wlan_vo_preferred: voice frames preferred over bt traffic
>> + * @wlan_beacon_preferred: beacon preferred over bt traffic
>> + * @wlan_mgmt_preferred: management frames preferred ovet bt traffic
> 
> typo: over
> 
Okay
>>  
>>  /**
>> + * wiphy_btcoex_support_flags
>> + *	This enum has the driver supported frame types for BTCOEX.
>> + * @WIPHY_WLAN_BE_PREFERRED - Supports Best Effort frame for BTCOEX
>> + * @WIPHY_WLAN_BK_PREFERRED - supports Background frame for BTCOEX
>> + * @WIPHY_WLAN_VI_PREFERRED - supports Video frame for BTCOEX
>> + * @WIPHY_WLAN_VO_PREFERRED - supports Voice frame for BTCOEX
>> + * @WIPHY_WLAN_BEACON_PREFERRED - supports Beacon frame for BTCOEX
>> + * @WIPHY_WLAN_MGMT_PREFERRED - supports Management frames for
>> BTCOEX.
>> + */
> 
> That's not making much sense to me?
> 

is it fine to have as WIPHY_BTCOEX_BE_PREFERRED ?

>> +/**
>> + * enum wiphy_btcoex_priority - BTCOEX priority level
>> + *	This enum defines priority level for BTCOEX
>> + * WIPHY_WLAN_PREFERRED_LOW - low priority frames over BT traffic
>> + * WIPHY_WLAN_PREFERRED_HIGH - high priority frames over BT traffic
>> + */
>> +
>> +enum wiphy_btcoex_priority {
>> +	WIPHY_WLAN_PREFERRED_LOW = false,
>> +	WIPHY_WLAN_PREFERRED_HIGH = true,
>> +};
> 
> That false/true seems just strange.
> 

I will just use as a enum without assigning false/true.

>> + * @btcoex_support_flags: This will have the driver supported
>> + *	frame types for BTCOEX. This value filled by using
>> + *	%enum wiphy_btcoex_support_flags while driver
>> + *	initialization.
> 
> The whole "will have" isn't really clear.
> 
>> + * @NL80211_ATTR_SET_BTCOEX_PRIORITY: nested attribute for driver
>> supporting
>> + *	the BTCOEX. When used with
>> %NL80211_CMD_SET_BTCOEX_PRIORITY it contains
>> + *	attributes according &enum nl80211_btcoex_priority to
>> indicate
>> + *	which frame has high priority over BT.
> 
> There should be no "SET" in there.
> 
Okay sure.

>>  /**
>> + * enum nl80211_btcoex_priority - BTCOEX parameter attributes
>> + *	This strcuture has enum values for driver supported wlan
>> + *	frame type for BTCOEX.
>> + * @NL80211_WLAN_BE_PREFERRED - Best Effort frame
>> + * @NL80211_WLAN_BK_PREFERRED - Background frame
>> + * @NL80211_WLAN_VI_PREFERRED - Video frame
>> + * @NL80211_WLAN_VO_PREFERRED - Voice frame
>> + * @NL80211_WLAN_BEACON_PREFERRED - BEACON frame
>> + * @NL80211_WLAN_MGMT_PREFERRED - MGMT frame
>> + */
>> +
>> +enum nl80211_btcoex_priority {
>> +	__NL80211_WLAN_PREFERRED_INVALID,
>> +	NL80211_WLAN_BE_PREFERRED,
>> +	NL80211_WLAN_BK_PREFERRED,
>> +	NL80211_WLAN_VI_PREFERRED,
>> +	NL80211_WLAN_VO_PREFERRED,
>> +	NL80211_WLAN_BEACON_PREFERRED,
>> +	NL80211_WLAN_MGMT_PREFERRED,
>> +	__NL80211_WLAN_PREFERRED_LAST,
>> +	NL80211_WLAN_PREFERRED_MAX =
>> +			__NL80211_WLAN_PREFERRED_LAST - 1,
>> +};
> 
> Wouldn't a bitmap be easier?
> 
since this is to distinguish between different btcoex priorities and we 
are not going to do any manipulations on these parameters.
It is just used as flag attribute.

Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[Johannes Berg]

> > >  /**
> > > + * wiphy_btcoex_support_flags
> > > + *	This enum has the driver supported frame types for
> > > BTCOEX.
> > > + * @WIPHY_WLAN_BE_PREFERRED - Supports Best Effort frame for
> > > BTCOEX
> > > + * @WIPHY_WLAN_BK_PREFERRED - supports Background frame for
> > > BTCOEX
> > > + * @WIPHY_WLAN_VI_PREFERRED - supports Video frame for BTCOEX
> > > + * @WIPHY_WLAN_VO_PREFERRED - supports Voice frame for BTCOEX
> > > + * @WIPHY_WLAN_BEACON_PREFERRED - supports Beacon frame for
> > > BTCOEX
> > > + * @WIPHY_WLAN_MGMT_PREFERRED - supports Management frames for
> > > BTCOEX.
> > > + */
> > 
> > That's not making much sense to me?
> > 
> 
> is it fine to have as WIPHY_BTCOEX_BE_PREFERRED ?

It's not really clear to me what you intend to do this - if it's really
support flags then you really should name those better.

> > > +/**
> > > + * enum wiphy_btcoex_priority - BTCOEX priority level
> > > + *	This enum defines priority level for BTCOEX
> > > + * WIPHY_WLAN_PREFERRED_LOW - low priority frames over BT
> > > traffic
> > > + * WIPHY_WLAN_PREFERRED_HIGH - high priority frames over BT
> > > traffic
> > > + */
> > > +
> > > +enum wiphy_btcoex_priority {
> > > +	WIPHY_WLAN_PREFERRED_LOW = false,
> > > +	WIPHY_WLAN_PREFERRED_HIGH = true,
> > > +};
> > 
> > That false/true seems just strange.
> > 
> 
> I will just use as a enum without assigning false/true.

What do you even need this enum for though?

> > > +enum nl80211_btcoex_priority {
> > > +	__NL80211_WLAN_PREFERRED_INVALID,
> > > +	NL80211_WLAN_BE_PREFERRED,
> > > +	NL80211_WLAN_BK_PREFERRED,
> > > +	NL80211_WLAN_VI_PREFERRED,
> > > +	NL80211_WLAN_VO_PREFERRED,
> > > +	NL80211_WLAN_BEACON_PREFERRED,
> > > +	NL80211_WLAN_MGMT_PREFERRED,
> > > +	__NL80211_WLAN_PREFERRED_LAST,
> > > +	NL80211_WLAN_PREFERRED_MAX =
> > > +			__NL80211_WLAN_PREFERRED_LAST - 1,
> > > +};
> > 
> > Wouldn't a bitmap be easier?
> > 
> since this is to distinguish between different btcoex priorities and
> we 
> are not going to do any manipulations on these parameters.
> It is just used as flag attribute.

But why the (parsing) complexity, when a single bitmap would do?

johannes

Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[Tamizh chelvam]

Hi Johannes,

Thanks for the comments

On 2016-12-13 21:39, Johannes Berg wrote:
>> > >  /**
>> > > + * wiphy_btcoex_support_flags
>> > > + *	This enum has the driver supported frame types for
>> > > BTCOEX.
>> > > + * @WIPHY_WLAN_BE_PREFERRED - Supports Best Effort frame for
>> > > BTCOEX
>> > > + * @WIPHY_WLAN_BK_PREFERRED - supports Background frame for
>> > > BTCOEX
>> > > + * @WIPHY_WLAN_VI_PREFERRED - supports Video frame for BTCOEX
>> > > + * @WIPHY_WLAN_VO_PREFERRED - supports Voice frame for BTCOEX
>> > > + * @WIPHY_WLAN_BEACON_PREFERRED - supports Beacon frame for
>> > > BTCOEX
>> > > + * @WIPHY_WLAN_MGMT_PREFERRED - supports Management frames for
>> > > BTCOEX.
>> > > + */
>> >
>> > That's not making much sense to me?
>> >
>> 
>> is it fine to have as WIPHY_BTCOEX_BE_PREFERRED ?
> 
> It's not really clear to me what you intend to do this - if it's really
> support flags then you really should name those better.
> 
This is support flags and it used by the driver to intimate driver 
supported frame type
for the BTCOEX to cfg like "wiphy_wowlan_support_flags" implementation.
Please suggest if this is ok ? I will be thankful if you can suggest a 
better one if this is not ok
"WIPHY_BTCOEX_SUPPORTS_BE"

>> > > +/**
>> > > + * enum wiphy_btcoex_priority - BTCOEX priority level
>> > > + *	This enum defines priority level for BTCOEX
>> > > + * WIPHY_WLAN_PREFERRED_LOW - low priority frames over BT
>> > > traffic
>> > > + * WIPHY_WLAN_PREFERRED_HIGH - high priority frames over BT
>> > > traffic
>> > > + */
>> > > +
>> > > +enum wiphy_btcoex_priority {
>> > > +	WIPHY_WLAN_PREFERRED_LOW = false,
>> > > +	WIPHY_WLAN_PREFERRED_HIGH = true,
>> > > +};
>> >
>> > That false/true seems just strange.
>> >
>> 
>> I will just use as a enum without assigning false/true.
> 
> What do you even need this enum for though?
> 
Ok. I will directly assign true for the flag.

>> > > +enum nl80211_btcoex_priority {
>> > > +	__NL80211_WLAN_PREFERRED_INVALID,
>> > > +	NL80211_WLAN_BE_PREFERRED,
>> > > +	NL80211_WLAN_BK_PREFERRED,
>> > > +	NL80211_WLAN_VI_PREFERRED,
>> > > +	NL80211_WLAN_VO_PREFERRED,
>> > > +	NL80211_WLAN_BEACON_PREFERRED,
>> > > +	NL80211_WLAN_MGMT_PREFERRED,
>> > > +	__NL80211_WLAN_PREFERRED_LAST,
>> > > +	NL80211_WLAN_PREFERRED_MAX =
>> > > +			__NL80211_WLAN_PREFERRED_LAST - 1,
>> > > +};
>> >
>> > Wouldn't a bitmap be easier?
>> >
>> since this is to distinguish between different btcoex priorities and
>> we 
>> are not going to do any manipulations on these parameters.
>> It is just used as flag attribute.
> 
> But why the (parsing) complexity, when a single bitmap would do?
> 
Do you mean to say, sending a value from user space and parse that in 
the driver?

Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[Johannes Berg]

> > > is it fine to have as WIPHY_BTCOEX_BE_PREFERRED ?
> > 
> > It's not really clear to me what you intend to do this - if it's
> > really support flags then you really should name those better.
> > 
> 
> This is support flags and it used by the driver to intimate driver 
> supported frame type for the BTCOEX to cfg like
> "wiphy_wowlan_support_flags" implementation. Please suggest if this
> is ok ? I will be thankful if you can suggest a  better one if this
> is not ok "WIPHY_BTCOEX_SUPPORTS_BE"

Well, I see a few things here:

1) does it even make sense to split it out per AC? wouldn't it be weird
if you supported this only for VO and BK, and not the others, or
something like that?

2) Wouldn't it make more sense to define this in nl80211 and just pass
the bitmap through to userspace? That would save quite a bit of netlink
mangling complexity.

3) I think the naming is confusing - "WIPHY_BTCOEX_SUPPORTS_BE_PREF" or
so might be more appropriate?

> Do you mean to say, sending a value from user space and parse that
> in  the driver?

I was more thinking of the capability advertisement, but yeah, both
ways seems reasonable.

johannes

Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[Tamizh chelvam]

Hi Johannes,

Thanks for your comments.

On 2016-12-16 15:07, Johannes Berg wrote:
>> > > is it fine to have as WIPHY_BTCOEX_BE_PREFERRED ?
>> >
>> > It's not really clear to me what you intend to do this - if it's
>> > really support flags then you really should name those better.
>> >
>> 
>> This is support flags and it used by the driver to intimate driver 
>> supported frame type for the BTCOEX to cfg like
>> "wiphy_wowlan_support_flags" implementation. Please suggest if this
>> is ok ? I will be thankful if you can suggest a  better one if this
>> is not ok "WIPHY_BTCOEX_SUPPORTS_BE"
> 
> Well, I see a few things here:
> 
> 1) does it even make sense to split it out per AC? wouldn't it be weird
> if you supported this only for VO and BK, and not the others, or
> something like that?
> 
It has support for BE, VI, management and beacon frames also.
Or do you meant to say like support only for VO and BK?

> 2) Wouldn't it make more sense to define this in nl80211 and just pass
> the bitmap through to userspace? That would save quite a bit of netlink
> mangling complexity.
> 
Please let me know if the below design/thought is fine to you.

iw phyX set btcoex_priority <[vi, vo, be, bk, mgmt, beacon]>

By this command user should give one or more than one frame types for 
this btcoex priority,
we will parse that in "iw" and send as a single bitmap(less than 0x64) 
to
the driver?

> 3) I think the naming is confusing - "WIPHY_BTCOEX_SUPPORTS_BE_PREF" or
> so might be more appropriate?
> 
If the above suggestion is fine, we may not need these flags.

>> Do you mean to say, sending a value from user space and parse that
>> in  the driver?
> 
> I was more thinking of the capability advertisement, but yeah, both
> ways seems reasonable.
> 
Okay.

Thanks,
Tamizh.

Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[Johannes Berg]

> > 1) does it even make sense to split it out per AC? wouldn't it be
> > weird
> > if you supported this only for VO and BK, and not the others, or
> > something like that?
> > 
> 
> It has support for BE, VI, management and beacon frames also.
> Or do you meant to say like support only for VO and BK?

I mean - does it make sense for a piece of hardware to support only
VO/BK, without the others? I don't really see how that would make
sense, but maybe I'm missing something?

IOW - why have all these bits rather than just one?

> > 2) Wouldn't it make more sense to define this in nl80211 and just
> > pass the bitmap through to userspace? That would save quite a bit
> > of netlink mangling complexity.
> > 
> 
> Please let me know if the below design/thought is fine to you.
> 
> iw phyX set btcoex_priority <[vi, vo, be, bk, mgmt, beacon]>

That seems fine, but I don't see how the iw command line is relevant to
the question of whether we pass flag attributes or a bitmap??

> By this command user should give one or more than one frame types
> for 
> this btcoex priority,
> we will parse that in "iw" and send as a single bitmap(less than
> 0x64)  to the driver?

Right, and also to nl80211. Why not?

johannes

Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[Tamizh chelvam]

On 2017-01-02 16:18, Johannes Berg wrote:
>> > 1) does it even make sense to split it out per AC? wouldn't it be
>> > weird
>> > if you supported this only for VO and BK, and not the others, or
>> > something like that?
>> >
>> 
>> It has support for BE, VI, management and beacon frames also.
>> Or do you meant to say like support only for VO and BK?
> 
> I mean - does it make sense for a piece of hardware to support only
> VO/BK, without the others? I don't really see how that would make
> sense, but maybe I'm missing something?
> 
> IOW - why have all these bits rather than just one?

Hardware supports data across all the access categories, this is just 
meant for prioritising the traffic.
f.e, If the fw/target has both wlan and bt traffic queued and if VO is 
set as priority, then wlan VO packets
will be pushed out of the radio first and then the bt traffic.

Here we do not block traffic either from wlan or BT, packets will 
definitely go out of the radio but traffic
arbitration will happen based on the priorities set.

> 
>> > 2) Wouldn't it make more sense to define this in nl80211 and just
>> > pass the bitmap through to userspace? That would save quite a bit
>> > of netlink mangling complexity.
>> >
>> 
>> Please let me know if the below design/thought is fine to you.
>> 
>> iw phyX set btcoex_priority <[vi, vo, be, bk, mgmt, beacon]>
> 
> That seems fine, but I don't see how the iw command line is relevant to
> the question of whether we pass flag attributes or a bitmap??
> 
>> By this command user should give one or more than one frame types
>> for 
>> this btcoex priority,
>> we will parse that in "iw" and send as a single bitmap(less than
>> 0x64)  to the driver?
> 
> Right, and also to nl80211. Why not?

Yes user space to nl80211 and to driver

> 
> johannes

Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[Johannes Berg]

> > IOW - why have all these bits rather than just one?
> 
> Hardware supports data across all the access categories, this is
> just  meant for prioritising the traffic. f.e, If the fw/target has
> both wlan and bt traffic queued and if VO is  set as priority, then
> wlan VO packets will be pushed out of the radio first and then the bt
> traffic.

Exactly. So as far as *capabilities* are concerned, why do we need so
many bits?

johannes

Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[Tamizh chelvam]

Hi Johannes,

Thank you for the reply.

On 2017-01-05 19:08, Johannes Berg wrote:
>> > IOW - why have all these bits rather than just one?
>> 
>> Hardware supports data across all the access categories, this is
>> just  meant for prioritising the traffic. f.e, If the fw/target has
>> both wlan and bt traffic queued and if VO is  set as priority, then
>> wlan VO packets will be pushed out of the radio first and then the bt
>> traffic.
> 
> Exactly. So as far as *capabilities* are concerned, why do we need so
> many bits?
> 
Is it fine to have something like this

1) We can have this btcoex_priority value as a optional value in btcoex 
enable command like below

iw phyX btcoex_state <enable| disable> [prirority(vendor spcific value)]

2) Or we can have seperate command for btcoex_priority as below

iw phyX set btcoex_priority <priority (vendor spcific value)>

Hopefully this will get rid off all the nl80211 bits.

Kindly suggest your thoughts.

Thanks,
Tamizh.

Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[Johannes Berg]

> Is it fine to have something like this
> 
> 1) We can have this btcoex_priority value as a optional value in
> btcoex enable command like below
> 
> iw phyX btcoex_state <enable| disable> [prirority(vendor spcific
> value)]
> 
> 2) Or we can have seperate command for btcoex_priority as below
> 
> iw phyX set btcoex_priority <priority (vendor spcific value)>
> 
> Hopefully this will get rid off all the nl80211 bits.

That makes no sense.

If the bits are vendor specific, then there's no value in having this
as an nl80211 command (rather than a vendor command) to start with.

You need to understand that I'm differentiating between *capability*
bits and actual *priority setting* bits - please re-read the thread
with that in mind.

johannes

Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX

[Tamizh chelvam]

Hi Johannes,

Sorry for the late response:(

On 2017-01-09 16:06, Johannes Berg wrote:
>> Is it fine to have something like this
>> 
>> 1) We can have this btcoex_priority value as a optional value in
>> btcoex enable command like below
>> 
>> iw phyX btcoex_state <enable| disable> [prirority(vendor spcific
>> value)]
>> 
>> 2) Or we can have seperate command for btcoex_priority as below
>> 
>> iw phyX set btcoex_priority <priority (vendor spcific value)>
>> 
>> Hopefully this will get rid off all the nl80211 bits.
> 
> That makes no sense.
> 
> If the bits are vendor specific, then there's no value in having this
> as an nl80211 command (rather than a vendor command) to start with.
> 
> You need to understand that I'm differentiating between *capability*
> bits and actual *priority setting* bits - please re-read the thread
> with that in mind.
> 
Those are for priority and not for capability purpose. Hardware is 
capable of
transmitting all the frames when there is a bt traffic. This is just 
setting priority for the wlan
frame, when there is a contention with BT traffic which one has to go 
out first.
Other than that hardware has the capability of sending all the frames.

This priority field is optional, if users does not want to set any 
priority then driver will force the
default priorities when bt_coex is enabled

iw phyX btcoex_state <enable| disable> [prirority]

[PATCH 3/4] mac80211: Add support to enable or disable btcoex

[c_traja]

From: Tamizh chelvam <c_traja@qti.qualcomm.com>

This patch introduces a new driver call back drv_set_btcoex
This API will pass user space value to driver to
enable or disabe btcoex.

Signed-off-by: Tamizh chelvam <c_traja@qti.qualcomm.com>
---
 include/net/mac80211.h    |    4 ++++
 net/mac80211/cfg.c        |    8 ++++++++
 net/mac80211/driver-ops.h |   14 ++++++++++++++
 net/mac80211/trace.h      |   14 ++++++++++++++
 4 files changed, 40 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index b9b24ab..e7db763 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3453,6 +3453,9 @@ enum ieee80211_reconfig_type {
  * @del_nan_func: Remove a NAN function. The driver must call
  *	ieee80211_nan_func_terminated() with
  *	NL80211_NAN_FUNC_TERM_REASON_USER_REQUEST reason code upon removal.
+ *
+ * @set_btcoex_: set_btcoex: Called when BTCOEX is enabled/disabled, use
+ *	this callback to enable or disable btcoex.
  */
 struct ieee80211_ops {
 	void (*tx)(struct ieee80211_hw *hw,
@@ -3705,6 +3708,7 @@ struct ieee80211_ops {
 	void (*del_nan_func)(struct ieee80211_hw *hw,
 			    struct ieee80211_vif *vif,
 			    u8 instance_id);
+	int (*set_btcoex)(struct ieee80211_hw *hw, bool enabled);
 };
 
 /**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index eb68add..a687056 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3562,6 +3562,13 @@ void ieee80211_nan_func_match(struct ieee80211_vif *vif,
 }
 EXPORT_SYMBOL(ieee80211_nan_func_match);
 
+static int ieee80211_set_btcoex(struct wiphy *wiphy, bool enabled)
+{
+	struct ieee80211_local *local = wiphy_priv(wiphy);
+
+	return drv_set_btcoex(local, enabled);
+}
+
 const struct cfg80211_ops mac80211_config_ops = {
 	.add_virtual_intf = ieee80211_add_iface,
 	.del_virtual_intf = ieee80211_del_iface,
@@ -3652,4 +3659,5 @@ void ieee80211_nan_func_match(struct ieee80211_vif *vif,
 	.nan_change_conf = ieee80211_nan_change_conf,
 	.add_nan_func = ieee80211_add_nan_func,
 	.del_nan_func = ieee80211_del_nan_func,
+	.set_btcoex = ieee80211_set_btcoex,
 };
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 09f77e4..16d9c98 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1248,4 +1248,18 @@ static inline void drv_del_nan_func(struct ieee80211_local *local,
 	trace_drv_return_void(local);
 }
 
+static inline int drv_set_btcoex(struct ieee80211_local *local,
+				 bool enabled)
+{
+	int ret = -EOPNOTSUPP;
+
+	trace_drv_set_btcoex(local, enabled);
+	if (local->ops->set_btcoex)
+		ret = local->ops->set_btcoex(&local->hw, enabled);
+
+	trace_drv_return_int(local, ret);
+
+	return ret;
+}
+
 #endif /* __MAC80211_DRIVER_OPS */
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 92a47af..760dfb6 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -742,6 +742,20 @@
 	TP_ARGS(local, value)
 );
 
+TRACE_EVENT(drv_set_btcoex,
+	TP_PROTO(struct ieee80211_local *local, bool enabled),
+	TP_ARGS(local, enabled),
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+		__field(bool, enabled)
+	),
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+		__entry->enabled = enabled;
+	),
+	TP_printk(LOCAL_PR_FMT " enabled:%d", LOCAL_PR_ARG, __entry->enabled)
+);
+
 TRACE_EVENT(drv_set_coverage_class,
 	TP_PROTO(struct ieee80211_local *local, s16 value),
 
-- 
1.7.9.5

[PATCH 4/4] mac80211: Add support to update btcoex priority value

[c_traja]

From: Tamizh chelvam <c_traja@qti.qualcomm.com>

This patch introduces a new driver callback drv_set_btcoex_priority
to pass the priority value to driver.

Signed-off-by: Tamizh chelvam <c_traja@qti.qualcomm.com>
---
 include/net/mac80211.h    |    7 +++++++
 net/mac80211/cfg.c        |    9 +++++++++
 net/mac80211/driver-ops.h |   12 ++++++++++++
 3 files changed, 28 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index e7db763..3c61015 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3456,6 +3456,11 @@ enum ieee80211_reconfig_type {
  *
  * @set_btcoex_: set_btcoex: Called when BTCOEX is enabled/disabled, use
  *	this callback to enable or disable btcoex.
+ * @set_btcoex_priority: Use this callback to set wlan high
+ *	priority frames over bluetooth. Driver supported wlan frames
+ *	for the BTCOEX is exposed by btcoex_support_flags.
+ *	When BTCOEX enabled, the high priority wlan frames will have
+ *	more priority than BT.
  */
 struct ieee80211_ops {
 	void (*tx)(struct ieee80211_hw *hw,
@@ -3709,6 +3714,8 @@ struct ieee80211_ops {
 			    struct ieee80211_vif *vif,
 			    u8 instance_id);
 	int (*set_btcoex)(struct ieee80211_hw *hw, bool enabled);
+	int (*set_btcoex_priority)(struct ieee80211_hw *hw,
+		struct cfg80211_btcoex_priority *btcoex_priority);
 };
 
 /**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a687056..d75cf97 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3569,6 +3569,14 @@ static int ieee80211_set_btcoex(struct wiphy *wiphy, bool enabled)
 	return drv_set_btcoex(local, enabled);
 }
 
+static int ieee80211_set_btcoex_priority(struct wiphy *wiphy,
+			struct cfg80211_btcoex_priority *btcoex_priority)
+{
+	struct ieee80211_local *local = wiphy_priv(wiphy);
+
+	return drv_set_btcoex_priority(local, btcoex_priority);
+}
+
 const struct cfg80211_ops mac80211_config_ops = {
 	.add_virtual_intf = ieee80211_add_iface,
 	.del_virtual_intf = ieee80211_del_iface,
@@ -3660,4 +3668,5 @@ static int ieee80211_set_btcoex(struct wiphy *wiphy, bool enabled)
 	.add_nan_func = ieee80211_add_nan_func,
 	.del_nan_func = ieee80211_del_nan_func,
 	.set_btcoex = ieee80211_set_btcoex,
+	.set_btcoex_priority = ieee80211_set_btcoex_priority,
 };
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 16d9c98..75ac3ed 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1262,4 +1262,16 @@ static inline int drv_set_btcoex(struct ieee80211_local *local,
 	return ret;
 }
 
+static inline int drv_set_btcoex_priority(struct ieee80211_local *local,
+			struct cfg80211_btcoex_priority *btcoex_priority)
+{
+	int ret = -EOPNOTSUPP;
+
+	if (local->ops->set_btcoex_priority)
+		ret = local->ops->set_btcoex_priority(&local->hw,
+						      btcoex_priority);
+
+	return ret;
+}
+
 #endif /* __MAC80211_DRIVER_OPS */
-- 
1.7.9.5