diff mbox

[PATCHv4,1/2] cfg80211: Add support to enable or disable btcoex and set btcoex_priority

Message ID 1491905730-16392-2-git-send-email-c_traja@qti.qualcomm.com (mailing list archive)
State Not Applicable
Delegated to: Kalle Valo
Headers show

Commit Message

c_traja@qti.qualcomm.com April 11, 2017, 10:15 a.m. UTC
From: Tamizh chelvam <c_traja@qti.qualcomm.com>

This patch introduces NL80211_CMD_SET_BTCOEX command and
NL80211_ATTR_BTCOEX_OP attribute to enable or disable btcoex.
And this change enables user to set btcoex priority by using
NL80211_ATTR_BTCOEX_PRIORITY attribute for the driver which
has the btcoex priority capability. Driver should expose such
capability and make use of this priority to decide whom to share the radio
(either bluetooth or WLAN). When the high priority wlan frames are queued
driver or firmware will signal to block BT activity.
Capable drivers should advertise this support by setting
NL80211_EXT_FEATURE_BTCOEX_PRIORITY nl80211 extended feature.

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

Comments

Johannes Berg May 19, 2017, 11:47 a.m. UTC | #1
Hi,

Sorry for the long delay.

> +/**
> + * enum nl80211_btcoex_priority - btcoex priority supported frame
> types and
> + *	its bitmap values.
> + * @NL80211_BTCOEX_SUPPORTS_BE_PREF - wlan Best effort frame takes
> more

You should drop the _SUPPORTS part from these names, it's not just for
that now.

> + */
> +
> +enum nl80211_btcoex_priority {

There's an extra blank line here.

> +	NL80211_BTCOEX_SUPPORTS_BE_PREF		= 1 << 0,
> +	NL80211_BTCOEX_SUPPORTS_BK_PREF		= 1 << 1,
> +	NL80211_BTCOEX_SUPPORTS_VI_PREF		= 1 << 2,
> +	NL80211_BTCOEX_SUPPORTS_VO_PREF		= 1 << 3,
> +	NL80211_BTCOEX_SUPPORTS_BEACON_PREF	= 1 << 4,
> +	NL80211_BTCOEX_SUPPORTS_MGMT_PREF	= 1 << 5,
> +	NL80211_BTCOEX_SUPPORTS_MAX_PREF	= (1 << 6) - 1,

I'd prefer to formulate this as

 NL80211_BTCOEX_PREF_MASK = (NL80211_BTCOEX_SUPPORTS_BE_PREF |
			     ...)

and then later use

if (value & ~NL80211_BTCOEX_PREF_MASK)
	return -EINVAL;

The way it is now isn't really all that obvious IMHO.

> +	u8 val = 0;
> +	u32 btcoex_priority = 0;

No need to initialize those values.

+	if (!rdev->ops->set_btcoex)
> +		return -ENOTSUPP;
> +
> +	if (!(info->attrs[NL80211_ATTR_BTCOEX_OP]))
> +		goto set_btcoex;

Don't really need those extra parentheses.

> +	if (info->attrs[NL80211_ATTR_BTCOEX_OP])
> +		val = nla_get_u8(info-
> >attrs[NL80211_ATTR_BTCOEX_OP]);

Ok, actually, here you do need to initialize val but it makes no sense
- why is this even a U8 attribute rather than a FLAG?

So there are two ways you can play this:

1) either you make it a U8 attribute as you have, and allow *not
changing* it, by making val default to -1 and documenting in the
cfg80211 API that -1 means no changes. This would allow userspace to
set the BT coex priority parameters without changing whether or not BT
coex is turned on or off entirely, but the drivers would have to be
storing the priority values all the time etc. This seems somewhat error
prone.

- or -

2) You simply disallow changing the BT coex priority parameters by
themselves, i.e. allow only setting those if the FLAG attribute for
enabling BT coex is also present. IMHO this is less error prone.

The way it is now makes no sense - you could set the BT coex parameters
and leave out the BTCOEX_OP attribute entirely, but then if you leave
it out that would mean it actually gets disabled and the new priority
values don't take any effect.

I strongly suggest you go for option 2) unless you can provide a really
good reason for option 1). If you do go for 2) you should rename the
BTCOEX_OP to BTCOEX_ENABLE and make it a flag.

> +	if (val > 1)
> +		return -EINVAL;
> +
> +	if (info->attrs[NL80211_ATTR_BTCOEX_PRIORITY]) {
> +		if (!wiphy_ext_feature_isset(wiphy,
> +					NL80211_EXT_FEATURE_BTCOEX_P
> RIORITY))
> +			return -EOPNOTSUPP;
> +
> +		btcoex_priority =
> +		nla_get_u32(info-
> >attrs[NL80211_ATTR_BTCOEX_PRIORITY]);
> +
> +		if (btcoex_priority >
> NL80211_BTCOEX_SUPPORTS_MAX_PREF)
> +			return -E2BIG;
> +	}

-EINVAL, even if that's not nice, but E2BIG means "argument list too
long" which really isn't the right thing here.

johannes
diff mbox

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 273b1dc..c280c7d 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2856,6 +2856,10 @@  struct cfg80211_nan_func {
  *	All other parameters must be ignored.
  *
  * @set_multicast_to_unicast: configure multicast to unicast conversion for BSS
+ * @set_btcoex: Use this callback to call driver API when user wants to
+ *	enable/disable btcoex and use this callback to set wlan high priority
+ *	over bluetooth. 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);
@@ -3144,6 +3148,8 @@  struct cfg80211_ops {
 	int	(*set_multicast_to_unicast)(struct wiphy *wiphy,
 					    struct net_device *dev,
 					    const bool enabled);
+	int     (*set_btcoex)(struct wiphy *wiphy, bool enabled,
+			      u32 btcoex_priority);
 };
 
 /*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 6095a6c..404b74e 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -944,6 +944,8 @@ 
  *	BSS selection. This command can be issued only while connected and it
  *	does not result in a change for the current association. Currently,
  *	only the %NL80211_ATTR_IE data is used and updated with this command.
+ * @NL80211_CMD_SET_BTCOEX: Enable/Disable btcoex using %NL80211_ATTR_BTCOEX_OP
+ *	and set/modify btcoex priority using %NL80211_ATTR_BTCOEX_PRIORITY.
  *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
@@ -1144,6 +1146,8 @@  enum nl80211_commands {
 
 	NL80211_CMD_UPDATE_CONNECT_PARAMS,
 
+	NL80211_CMD_SET_BTCOEX,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -2081,6 +2085,16 @@  enum nl80211_commands {
  * @NL80211_ATTR_PMK: PMK for the PMKSA identified by %NL80211_ATTR_PMKID.
  *	This is used with @NL80211_CMD_SET_PMKSA.
  *
+ * @NL80211_ATTR_BTCOEX_OP: u8 attribute for driver supporting
+ *	the btcoex feature. When used with %NL80211_CMD_SET_BTCOEX it contains
+ *	either 0 for disable or 1 for enable btcoex.
+ *
+ * @NL80211_ATTR_BTCOEX_PRIORITY: This is for the driver which support
+ *	btcoex priority feature. It used with %NL80211_CMD_SET_BTCOEX.
+ *	This will have u32 BITMAP value which represents
+ *	frame(bk, be, vi, vo, mgmt, beacon) type and that will have more
+ *	priority than a BT traffic.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2500,6 +2514,9 @@  enum nl80211_attrs {
 
 	NL80211_ATTR_PMK,
 
+	NL80211_ATTR_BTCOEX_OP,
+	NL80211_ATTR_BTCOEX_PRIORITY,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -4838,6 +4855,8 @@  enum nl80211_feature_flags {
  *	RSSI threshold values to monitor rather than exactly one threshold.
  * @NL80211_EXT_FEATURE_FILS_SK_OFFLOAD: Driver SME supports FILS shared key
  *	authentication with %NL80211_CMD_CONNECT.
+ * @NL80211_EXT_FEATURE_BTCOEX_PRIORITY: Driver supports btcoex priority
+ *	feature with %NL80211_ATTR_BTCOEX_PRIORITY.
  *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -4858,6 +4877,7 @@  enum nl80211_ext_feature_index {
 	NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI,
 	NL80211_EXT_FEATURE_CQM_RSSI_LIST,
 	NL80211_EXT_FEATURE_FILS_SK_OFFLOAD,
+	NL80211_EXT_FEATURE_BTCOEX_PRIORITY,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
@@ -5339,4 +5359,34 @@  enum nl80211_nan_match_attributes {
 	NL80211_NAN_MATCH_ATTR_MAX = NUM_NL80211_NAN_MATCH_ATTR - 1
 };
 
+/**
+ * enum nl80211_btcoex_priority - btcoex priority supported frame types and
+ *	its bitmap values.
+ * @NL80211_BTCOEX_SUPPORTS_BE_PREF - wlan Best effort frame takes more
+ *	priority than BT traffic.
+ * @NL80211_BTCOEX_SUPPORTS_BK_PREF - wlan Background frame takes more
+ *	priority than BT traffic.
+ * @NL80211_BTCOEX_SUPPORTS_VI_PREF - wlan Video frame takes more priority
+ *	than BT traffic.
+ * @NL80211_BTCOEX_SUPPORTS_VO_PREF - wlan Voice frame takes more priority
+ *	than BT traffic.
+ * @NL80211_BTCOEX_SUPPORTS_BEACON_PREF - wlan BEACON frame takes more
+ *	priority than  BT traffic.
+ * @NL80211_BTCOEX_SUPPORTS_MGMT_PREF - wlan Management frame takes more
+ *	priority than BT traffic.
+ * @NL80211_BTCOEX_SUPPORTS_MAX_PREF - MAX supported value for btcoex
+ *	priority feature.
+ */
+
+enum nl80211_btcoex_priority {
+	NL80211_BTCOEX_SUPPORTS_BE_PREF		= 1 << 0,
+	NL80211_BTCOEX_SUPPORTS_BK_PREF		= 1 << 1,
+	NL80211_BTCOEX_SUPPORTS_VI_PREF		= 1 << 2,
+	NL80211_BTCOEX_SUPPORTS_VO_PREF		= 1 << 3,
+	NL80211_BTCOEX_SUPPORTS_BEACON_PREF	= 1 << 4,
+	NL80211_BTCOEX_SUPPORTS_MGMT_PREF	= 1 << 5,
+	NL80211_BTCOEX_SUPPORTS_MAX_PREF	= (1 << 6) - 1,
+};
+
+
 #endif /* __LINUX_NL80211_H */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 9910aae..55b9ac7 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -419,6 +419,8 @@  enum nl80211_multicast_groups {
 					.len = FILS_ERP_MAX_RRK_LEN },
 	[NL80211_ATTR_FILS_CACHE_ID] = { .len = 2 },
 	[NL80211_ATTR_PMK] = { .type = NLA_BINARY, .len = PMK_MAX_LEN },
+	[NL80211_ATTR_BTCOEX_OP] = { .type = NLA_U8 },
+	[NL80211_ATTR_BTCOEX_PRIORITY] = { .type = NLA_U32 },
 };
 
 /* policy for the key attributes */
@@ -12184,6 +12186,41 @@  static int nl80211_set_multicast_to_unicast(struct sk_buff *skb,
 	return rdev_set_multicast_to_unicast(rdev, dev, enabled);
 }
 
+static int nl80211_set_btcoex(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct wiphy *wiphy = &rdev->wiphy;
+	u8 val = 0;
+	u32 btcoex_priority = 0;
+
+	if (!rdev->ops->set_btcoex)
+		return -ENOTSUPP;
+
+	if (!(info->attrs[NL80211_ATTR_BTCOEX_OP]))
+		goto set_btcoex;
+
+	if (info->attrs[NL80211_ATTR_BTCOEX_OP])
+		val = nla_get_u8(info->attrs[NL80211_ATTR_BTCOEX_OP]);
+
+	if (val > 1)
+		return -EINVAL;
+
+	if (info->attrs[NL80211_ATTR_BTCOEX_PRIORITY]) {
+		if (!wiphy_ext_feature_isset(wiphy,
+					NL80211_EXT_FEATURE_BTCOEX_PRIORITY))
+			return -EOPNOTSUPP;
+
+		btcoex_priority =
+		nla_get_u32(info->attrs[NL80211_ATTR_BTCOEX_PRIORITY]);
+
+		if (btcoex_priority > NL80211_BTCOEX_SUPPORTS_MAX_PREF)
+			return -E2BIG;
+	}
+
+set_btcoex:
+	return rdev_set_btcoex(rdev, val, btcoex_priority);
+}
+
 #define NL80211_FLAG_NEED_WIPHY		0x01
 #define NL80211_FLAG_NEED_NETDEV	0x02
 #define NL80211_FLAG_NEED_RTNL		0x04
@@ -13059,6 +13096,14 @@  static void nl80211_post_doit(const struct genl_ops *ops, struct sk_buff *skb,
 		.internal_flags = NL80211_FLAG_NEED_NETDEV |
 				  NL80211_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL80211_CMD_SET_BTCOEX,
+		.doit = nl80211_set_btcoex,
+		.policy = nl80211_policy,
+		.flags = GENL_UNS_ADMIN_PERM,
+		.internal_flags = NL80211_FLAG_NEED_WIPHY |
+				  NL80211_FLAG_NEED_RTNL,
+	},
 };
 
 static struct genl_family nl80211_fam __ro_after_init = {
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index f2baf59..d0dec56 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -1165,4 +1165,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(struct cfg80211_registered_device *rdev, bool enabled,
+		u32 btcoex_priority)
+{
+	int ret = -ENOTSUPP;
+
+	trace_rdev_set_btcoex(&rdev->wiphy, enabled, btcoex_priority);
+	ret = rdev->ops->set_btcoex(&rdev->wiphy, enabled, 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 fd55786..1947011 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -3069,6 +3069,23 @@ 
 		  WIPHY_PR_ARG, __entry->n_rules)
 );
 
+TRACE_EVENT(rdev_set_btcoex,
+	TP_PROTO(struct wiphy *wiphy, bool enabled, u32 btcoex_priority),
+	TP_ARGS(wiphy, enabled, btcoex_priority),
+	TP_STRUCT__entry(
+		WIPHY_ENTRY
+		__field(bool, enabled)
+		__field(u32, btcoex_priority)
+	),
+	TP_fast_assign(
+		WIPHY_ASSIGN;
+		__entry->enabled = enabled;
+		__entry->btcoex_priority = btcoex_priority;
+	),
+	TP_printk(WIPHY_PR_FMT ", enabled=%d btcoex_priority :%u",
+		  WIPHY_PR_ARG, __entry->enabled, __entry->btcoex_priority)
+);
+
 DEFINE_EVENT(wiphy_wdev_evt, rdev_abort_scan,
 	TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev),
 	TP_ARGS(wiphy, wdev)