diff mbox

[PATCHv2,3/4] cfg80211: Add Support set btcoex priority value

Message ID 1488029873-14600-4-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 Feb. 25, 2017, 1:37 p.m. UTC
From: Tamizh chelvam <c_traja@qti.qualcomm.com>

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 through
btcoex_priority_support.

This will be useful 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       |   10 ++++++++--
 include/uapi/linux/nl80211.h |    7 +++++++
 net/wireless/nl80211.c       |   16 +++++++++++++++-
 net/wireless/rdev-ops.h      |    8 +++++---
 net/wireless/trace.h         |   10 ++++++----
 5 files changed, 41 insertions(+), 10 deletions(-)

Comments

kernel test robot Feb. 25, 2017, 3:12 p.m. UTC | #1
Hi Tamizh,

[auto build test ERROR on mac80211-next/master]
[also build test ERROR on next-20170224]
[cannot apply to v4.10]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/c_traja-qti-qualcomm-com/cfg80211-mac80211-BTCOEX-feature-support/20170225-215733
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

Note: the linux-review/c_traja-qti-qualcomm-com/cfg80211-mac80211-BTCOEX-feature-support/20170225-215733 HEAD 375b0104c717b355c9c35350bd9a7d34cb05f4bb builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

>> net/mac80211/cfg.c:3703:16: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
     .set_btcoex = ieee80211_set_btcoex,
                   ^~~~~~~~~~~~~~~~~~~~
   net/mac80211/cfg.c:3703:16: note: (near initialization for 'mac80211_config_ops.set_btcoex')
   cc1: some warnings being treated as errors

vim +3703 net/mac80211/cfg.c

6d52563f2b net/mac80211/cfg.c           Johannes Berg    2012-04-04  3687  #ifdef CONFIG_PM
6d52563f2b net/mac80211/cfg.c           Johannes Berg    2012-04-04  3688  	.set_wakeup = ieee80211_set_wakeup,
6d52563f2b net/mac80211/cfg.c           Johannes Berg    2012-04-04  3689  #endif
5b7ccaf3fc net/mac80211/cfg.c           Johannes Berg    2012-07-12  3690  	.get_channel = ieee80211_cfg_get_channel,
164eb02d07 net/mac80211/cfg.c           Simon Wunderlich 2013-02-08  3691  	.start_radar_detection = ieee80211_start_radar_detection,
73da7d5bab net/mac80211/cfg.c           Simon Wunderlich 2013-07-11  3692  	.channel_switch = ieee80211_channel_switch,
32db6b54df net/mac80211/cfg.c           Kyeyoon Park     2013-12-16  3693  	.set_qos_map = ieee80211_set_qos_map,
3b1700bde4 net/mac80211/cfg.c           Jouni Malinen    2014-04-28  3694  	.set_ap_chanwidth = ieee80211_set_ap_chanwidth,
02219b3abc net/mac80211/cfg.c           Johannes Berg    2014-10-07  3695  	.add_tx_ts = ieee80211_add_tx_ts,
02219b3abc net/mac80211/cfg.c           Johannes Berg    2014-10-07  3696  	.del_tx_ts = ieee80211_del_tx_ts,
708d50edb1 net/mac80211/cfg.c           Ayala Beker      2016-09-20  3697  	.start_nan = ieee80211_start_nan,
708d50edb1 net/mac80211/cfg.c           Ayala Beker      2016-09-20  3698  	.stop_nan = ieee80211_stop_nan,
5953ff6d6a net/mac80211/cfg.c           Ayala Beker      2016-09-20  3699  	.nan_change_conf = ieee80211_nan_change_conf,
167e33f4f6 net/mac80211/cfg.c           Ayala Beker      2016-09-20  3700  	.add_nan_func = ieee80211_add_nan_func,
167e33f4f6 net/mac80211/cfg.c           Ayala Beker      2016-09-20  3701  	.del_nan_func = ieee80211_del_nan_func,
ebceec860f net/mac80211/cfg.c           Michael Braun    2016-11-22  3702  	.set_multicast_to_unicast = ieee80211_set_multicast_to_unicast,
8eb981c618 net/mac80211/cfg.c           Tamizh chelvam   2017-02-25 @3703  	.set_btcoex = ieee80211_set_btcoex,
f0706e828e net/mac80211/ieee80211_cfg.c Jiri Benc        2007-05-05  3704  };

:::::: The code at line 3703 was first introduced by commit
:::::: 8eb981c618fea1c068e64a61adc32c2149f5cfd8 mac80211: Add support to enable or disable btcoex

:::::: TO: Tamizh chelvam <c_traja@qti.qualcomm.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Johannes Berg March 2, 2017, 8:50 a.m. UTC | #2
Is there much point in having 4 rather than just 2 patches?


> +	int     (*set_btcoex)(struct wiphy *wiphy, bool enabled,
> +			      int btcoex_priority);

Shouldn't that be u32 as a bitmap?

> +	bool btcoex_priority_support;

Why not use an extended nl80211 feature flag directly?

> + * @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.

I think you need to define the bits somewhere in an enum - i.e. which
one is VO, VI, ...

> +	int btcoex_priority = -1;

That -1 is pretty useless, if the driver doesn't support it, hopefully
it won't look at the value at all?

johannes
Johannes Berg March 2, 2017, 8:51 a.m. UTC | #3
This patch will also (and correctly so) cause compilation warnings in
mac80211, so you really should have just two patches.

johannes
c_traja@qti.qualcomm.com March 2, 2017, 11:48 a.m. UTC | #4
> Is there much point in having 4 rather than just 2 patches?

>

[Tamizh] Yes, I agree to it. 
> 

> > +	int     (*set_btcoex)(struct wiphy *wiphy, bool enabled,

> > +			      int btcoex_priority);

> 

> Shouldn't that be u32 as a bitmap?

> 

[Tamizh] Yes.

> > +	bool btcoex_priority_support;

> 

> Why not use an extended nl80211 feature flag directly?

> 

[Tamizh] Ok sure. I'll use extended nl80211 feature flag.

> > + * @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.

> 

> I think you need to define the bits somewhere in an enum - i.e. which one is VO,

> VI, ...

>

[Tamizh] ok sure. Is this for documentation purpose or do you want me to use that 
enum for something else?
 
> > +	int btcoex_priority = -1;

> 

> That -1 is pretty useless, if the driver doesn't support it, hopefully it won't look

> at the value at all?

> 

[Tamizh] Ok sure
Johannes Berg March 2, 2017, 12:15 p.m. UTC | #5
> > I think you need to define the bits somewhere in an enum - i.e.
> > which one is VO,
> > VI, ...
> > 
> 
> [Tamizh] ok sure. Is this for documentation purpose or do you want me
> to use that  enum for something else?

Well I think you should check in cfg80211 that no bits not defined in
the enum are used, and then in the driver you need to use that enum to
translate to the firmware API, right?

johannes
c_traja@qti.qualcomm.com March 2, 2017, 1:08 p.m. UTC | #6
> 

> > > I think you need to define the bits somewhere in an enum - i.e.

> > > which one is VO,

> > > VI, ...

> > >

> >

> > [Tamizh] ok sure. Is this for documentation purpose or do you want me

> > to use that  enum for something else?

> 

> Well I think you should check in cfg80211 that no bits not defined in the enum

> are used, 

[Tamizh] I'm planning to do this check in iw itself and converting into a value, 

iw command will looks like below

iw btcoex enable [<be> <bk> <vo> <vi> <mgmt><beacon>]

If anything other than this given by user will be rejected and will not forward to cfg80211.
Do you have any concern on this?

>and then in the driver you need to use that enum to translate to the

> firmware API, right?

> 

[Tamizh] Yes, we can have this for checking invalid or the value is greater the driver support value
Johannes Berg March 2, 2017, 1:43 p.m. UTC | #7
> 
> [Tamizh] I'm planning to do this check in iw itself and converting
> into a value, 
> 
> iw command will looks like below
> 
> iw btcoex enable [<be> <bk> <vo> <vi> <mgmt><beacon>]
> 
> If anything other than this given by user will be rejected and will
> not forward to cfg80211.
> Do you have any concern on this?

Yes, you really need to check it in cfg80211 and define the bits that
are valid in nl80211.

johannes
diff mbox

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index a9aae03..3f44aac 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2817,7 +2817,10 @@  struct cfg80211_nan_func {
  *
  * @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.
+ *	enable/disable btcoex and use this callback to set wlan high priority over
+ *	Bluetooth. This capability will be exposed by the driver using
+ *	btcoex_priority_support boolean flag. 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);
@@ -3102,7 +3105,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);
+	int     (*set_btcoex)(struct wiphy *wiphy, bool enabled,
+			      int btcoex_priority);
 };
 
 /*
@@ -3738,6 +3742,8 @@  struct wiphy {
 
 	u8 nan_supported_bands;
 
+	bool btcoex_priority_support;
+
 	char priv[0] __aligned(NETDEV_ALIGN);
 };
 
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 30d691f..94b6eff 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2019,6 +2019,12 @@  enum nl80211_commands {
  *     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
@@ -2431,6 +2437,7 @@  enum nl80211_attrs {
 	NL80211_ATTR_TIMEOUT_REASON,
 
 	NL80211_ATTR_BTCOEX_OP,
+	NL80211_ATTR_BTCOEX_PRIORITY,
 
 	/* add attributes here, update the policy in nl80211.c */
 
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index bd203c2..56518c4 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -411,6 +411,7 @@  enum nl80211_multicast_groups {
 	},
 	[NL80211_ATTR_TIMEOUT_REASON] = { .type = NLA_U32 },
 	[NL80211_ATTR_BTCOEX_OP] = { .type = NLA_U8 },
+	[NL80211_ATTR_BTCOEX_PRIORITY] = { .type = NLA_U32 },
 };
 
 /* policy for the key attributes */
@@ -11970,7 +11971,9 @@  static int nl80211_set_multicast_to_unicast(struct sk_buff *skb,
 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;
+	int btcoex_priority = -1;
 
 	if (!rdev->ops->set_btcoex)
 		return -ENOTSUPP;
@@ -11984,9 +11987,20 @@  static int nl80211_set_btcoex(struct sk_buff *skb, struct genl_info *info)
 	if (val > 1)
 		return -EINVAL;
 
+	if (wiphy->btcoex_priority_support)
+		btcoex_priority = 0;
+
+	if (info->attrs[NL80211_ATTR_BTCOEX_PRIORITY]) {
+		if (!wiphy->btcoex_priority_support)
+			return -EOPNOTSUPP;
+
+		btcoex_priority =
+		nla_get_u32(info->attrs[NL80211_ATTR_BTCOEX_PRIORITY]);
+
+	}
 
 set_btcoex:
-	return rdev_set_btcoex(rdev, val);
+	return rdev_set_btcoex(rdev, val, btcoex_priority);
 }
 
 #define NL80211_FLAG_NEED_WIPHY		0x01
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 6592f14..7c0c139 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -1155,13 +1155,15 @@  static inline int rdev_set_qos_map(struct cfg80211_registered_device *rdev,
 }
 
 static inline int
-rdev_set_btcoex(struct cfg80211_registered_device *rdev, bool enabled)
+rdev_set_btcoex(struct cfg80211_registered_device *rdev, bool enabled,
+		int btcoex_priority)
 {
 	int ret = -ENOTSUPP;
 
-	trace_rdev_set_btcoex(&rdev->wiphy, enabled);
-	ret = rdev->ops->set_btcoex(&rdev->wiphy, enabled);
+	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 c3970b1..4f88f50 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -3048,18 +3048,20 @@ 
 );
 
 TRACE_EVENT(rdev_set_btcoex,
-	TP_PROTO(struct wiphy *wiphy, bool enabled),
-	TP_ARGS(wiphy, enabled),
+	TP_PROTO(struct wiphy *wiphy, bool enabled, int btcoex_priority),
+	TP_ARGS(wiphy, enabled, btcoex_priority),
 	TP_STRUCT__entry(
 		WIPHY_ENTRY
 		__field(bool, enabled)
+		__field(int, btcoex_priority)
 	),
 	TP_fast_assign(
 		WIPHY_ASSIGN;
 		__entry->enabled = enabled;
+		__entry->btcoex_priority = btcoex_priority;
 	),
-	TP_printk(WIPHY_PR_FMT, ", enabled=%d",
-		  WIPHY_PR_ARG, __entry->enabled)
+	TP_printk(WIPHY_PR_FMT, ", enabled=%d btcoex_priority :%d",
+		  WIPHY_PR_ARG, __entry->enabled, __entry->btcoex_priority)
 );
 
 DEFINE_EVENT(wiphy_wdev_evt, rdev_abort_scan,