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

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

[Tamizh chelvam]

From: Tamizh chelvam <tamizhr@codeaurora.org>

This patch introduces NL80211_CMD_SET_BTCOEX command and
NL80211_ATTR_BTCOEX_OP attribute to enable or disable btcoex.

Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org>
---
 include/net/cfg80211.h       |    4 ++++
 include/uapi/linux/nl80211.h |   10 ++++++++++
 net/wireless/nl80211.c       |   28 ++++++++++++++++++++++++++++
 net/wireless/rdev-ops.h      |   11 +++++++++++
 net/wireless/trace.h         |   15 +++++++++++++++
 5 files changed, 68 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index fc40843..b0e8bf6 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2960,6 +2960,9 @@ struct cfg80211_external_auth_params {
  *
  * @external_auth: indicates result of offloaded authentication processing from
  *     user space
+ *
+ * @set_btcoex: This callback used to Enable/disable btcoex
+ *
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -3255,6 +3258,7 @@ struct cfg80211_ops {
 			   const u8 *aa);
 	int     (*external_auth)(struct wiphy *wiphy, struct net_device *dev,
 				 struct cfg80211_external_auth_params *params);
+	int     (*set_btcoex)(struct wiphy *wiphy, bool enabled);
 };
 
 /*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index c13c843..3fb45b2 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1018,6 +1018,8 @@
  *	&NL80211_ATTR_CHANNEL_WIDTH,&NL80211_ATTR_NSS attributes with its
  *	address(specified in &NL80211_ATTR_MAC).
  *
+ * @NL80211_CMD_SET_BTCOEX: Enable/Disable btcoex using %NL80211_ATTR_BTCOEX_OP.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -1228,6 +1230,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_STA_OPMODE_CHANGED,
 
+	NL80211_CMD_SET_BTCOEX,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -2196,6 +2200,10 @@ enum nl80211_commands {
  * @NL80211_ATTR_NSS: Station's New/updated  RX_NSS value notified using this
  *	u8 attribute. This is used with %NL80211_CMD_STA_OPMODE_CHANGED.
  *
+ * @NL80211_ATTR_BTCOEX_OP: u8 attribute for driver supporting
+ *	the btcoex feature. This will be used with %NL80211_CMD_SET_BTCOEX
+ *	to enable/disable btcoex.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2628,6 +2636,8 @@ enum nl80211_attrs {
 	NL80211_ATTR_NSS,
 	NL80211_ATTR_ACK_SIGNAL,
 
+	NL80211_ATTR_BTCOEX_OP,
+
 	/* 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 a910150..ebd119b 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -422,6 +422,7 @@ enum nl80211_multicast_groups {
 	[NL80211_ATTR_PMK] = { .type = NLA_BINARY, .len = PMK_MAX_LEN },
 	[NL80211_ATTR_SCHED_SCAN_MULTI] = { .type = NLA_FLAG },
 	[NL80211_ATTR_EXTERNAL_AUTH_SUPPORT] = { .type = NLA_FLAG },
+	[NL80211_ATTR_BTCOEX_OP] = { .type = NLA_U8 },
 };
 
 /* policy for the key attributes */
@@ -12517,6 +12518,25 @@ static int nl80211_external_auth(struct sk_buff *skb, struct genl_info *info)
 	return rdev_external_auth(rdev, dev, &params);
 }
 
+static int nl80211_set_btcoex(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	u8 val = 0;
+
+	if (!rdev->ops->set_btcoex)
+		return -ENOTSUPP;
+
+	if (!info->attrs[NL80211_ATTR_BTCOEX_OP])
+		return -EINVAL;
+
+	val = nla_get_u8(info->attrs[NL80211_ATTR_BTCOEX_OP]);
+
+	if (val > 1)
+		return -EINVAL;
+
+	return rdev_set_btcoex(rdev, val);
+}
+
 #define NL80211_FLAG_NEED_WIPHY		0x01
 #define NL80211_FLAG_NEED_NETDEV	0x02
 #define NL80211_FLAG_NEED_RTNL		0x04
@@ -13420,6 +13440,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,
+		.doit = nl80211_set_btcoex,
+		.policy = nl80211_policy,
+		.flags = GENL_UNS_ADMIN_PERM,
+		.internal_flags = NL80211_FLAG_NEED_WIPHY |
+				  NL80211_FLAG_NEED_RTNL,
+	},
 
 };
 
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 84f23ae..4002f2c 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -1205,4 +1205,15 @@ static inline int rdev_del_pmk(struct cfg80211_registered_device *rdev,
 	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 5152938..838ec11 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -3173,6 +3173,21 @@
 		  WIPHY_PR_ARG, __entry->n_rules)
 );
 
+TRACE_EVENT(rdev_set_btcoex,
+	TP_PROTO(struct wiphy *wiphy, bool enabled),
+	TP_ARGS(wiphy, enabled),
+	TP_STRUCT__entry(
+		WIPHY_ENTRY
+		__field(bool, enabled)
+	),
+	TP_fast_assign(
+		WIPHY_ASSIGN;
+		__entry->enabled = enabled;
+	),
+	TP_printk(WIPHY_PR_FMT ", enabled=%d ",
+		  WIPHY_PR_ARG, __entry->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

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

[Tamizh chelvam]

From: Tamizh chelvam <tamizhr@codeaurora.org>

This patch introduces a new driver call back drv_set_btcoex
to enable/disable btcoex.

Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org>
---
 include/net/mac80211.h    |    2 ++
 net/mac80211/cfg.c        |    8 ++++++++
 net/mac80211/driver-ops.h |   13 +++++++++++++
 net/mac80211/trace.h      |   17 +++++++++++++++++
 4 files changed, 40 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 2fd59ed..45d4281 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3487,6 +3487,7 @@ 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: To enable/disable btcoex. Returns 0 on success.
  */
 struct ieee80211_ops {
 	void (*tx)(struct ieee80211_hw *hw,
@@ -3768,6 +3769,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 fd68f6f..1a939c1 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3694,6 +3694,13 @@ static int ieee80211_set_multicast_to_unicast(struct wiphy *wiphy,
 	return 0;
 }
 
+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,
@@ -3786,4 +3793,5 @@ static int ieee80211_set_multicast_to_unicast(struct wiphy *wiphy,
 	.add_nan_func = ieee80211_add_nan_func,
 	.del_nan_func = ieee80211_del_nan_func,
 	.set_multicast_to_unicast = ieee80211_set_multicast_to_unicast,
+	.set_btcoex = ieee80211_set_btcoex,
 };
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 4d82fe7..c424e17 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1250,4 +1250,17 @@ 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 591ad02..e30e0b1 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -746,6 +746,23 @@
 	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

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

[Arend van Spriel]

On 3/1/2018 6:59 PM, Tamizh chelvam wrote:
> From: Tamizh chelvam <tamizhr@codeaurora.org>
>
> This patch introduces NL80211_CMD_SET_BTCOEX command and
> NL80211_ATTR_BTCOEX_OP attribute to enable or disable btcoex.

What kind of btcoex are we talking about here? Is it signalling between 
wlan and bt to get access to the shared RF. Why would it require 
user-space interaction? Are there no options for wlan to detect bt is in 
use, ie. bt hci is setup, and vice versa. Can it be indicated in 
platform data or device tree. Trying to understand the use-case here.

> Signed-off-by: Tamizh chelvam <tamizhr@codeaurora.org>
> ---
>   include/net/cfg80211.h       |    4 ++++
>   include/uapi/linux/nl80211.h |   10 ++++++++++
>   net/wireless/nl80211.c       |   28 ++++++++++++++++++++++++++++
>   net/wireless/rdev-ops.h      |   11 +++++++++++
>   net/wireless/trace.h         |   15 +++++++++++++++
>   5 files changed, 68 insertions(+)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index fc40843..b0e8bf6 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -2960,6 +2960,9 @@ struct cfg80211_external_auth_params {
>    *
>    * @external_auth: indicates result of offloaded authentication processing from
>    *     user space
> + *
> + * @set_btcoex: This callback used to Enable/disable btcoex

Bit strange to capitalize Enable here.

> + *
>    */
>   struct cfg80211_ops {
>   	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
> @@ -3255,6 +3258,7 @@ struct cfg80211_ops {
>   			   const u8 *aa);
>   	int     (*external_auth)(struct wiphy *wiphy, struct net_device *dev,
>   				 struct cfg80211_external_auth_params *params);
> +	int     (*set_btcoex)(struct wiphy *wiphy, bool enabled);
>   };
>
>   /*
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index c13c843..3fb45b2 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -1018,6 +1018,8 @@
>    *	&NL80211_ATTR_CHANNEL_WIDTH,&NL80211_ATTR_NSS attributes with its
>    *	address(specified in &NL80211_ATTR_MAC).
>    *
> + * @NL80211_CMD_SET_BTCOEX: Enable/Disable btcoex using %NL80211_ATTR_BTCOEX_OP.
> + *
>    * @NL80211_CMD_MAX: highest used command number
>    * @__NL80211_CMD_AFTER_LAST: internal use
>    */
> @@ -1228,6 +1230,8 @@ enum nl80211_commands {
>
>   	NL80211_CMD_STA_OPMODE_CHANGED,
>
> +	NL80211_CMD_SET_BTCOEX,
> +
>   	/* add new commands above here */
>
>   	/* used to define NL80211_CMD_MAX below */
> @@ -2196,6 +2200,10 @@ enum nl80211_commands {
>    * @NL80211_ATTR_NSS: Station's New/updated  RX_NSS value notified using this
>    *	u8 attribute. This is used with %NL80211_CMD_STA_OPMODE_CHANGED.
>    *
> + * @NL80211_ATTR_BTCOEX_OP: u8 attribute for driver supporting

How is user-space supposed to know the driver is supporting btcoex. I do 
not see any advertisement in this patch. Should probably add ext_feature 
flag for this.

> + *	the btcoex feature. This will be used with %NL80211_CMD_SET_BTCOEX
> + *	to enable/disable btcoex.

What does 'OP' stand for. Why not just call it 'ENABLE'. Also no need to 
make this u8. A flag attribute type seems sufficient here.

> + *
>    * @NUM_NL80211_ATTR: total number of nl80211_attrs available
>    * @NL80211_ATTR_MAX: highest attribute number currently defined
>    * @__NL80211_ATTR_AFTER_LAST: internal use
> @@ -2628,6 +2636,8 @@ enum nl80211_attrs {
>   	NL80211_ATTR_NSS,
>   	NL80211_ATTR_ACK_SIGNAL,
>
> +	NL80211_ATTR_BTCOEX_OP,
> +
>   	/* 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 a910150..ebd119b 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -422,6 +422,7 @@ enum nl80211_multicast_groups {
>   	[NL80211_ATTR_PMK] = { .type = NLA_BINARY, .len = PMK_MAX_LEN },
>   	[NL80211_ATTR_SCHED_SCAN_MULTI] = { .type = NLA_FLAG },
>   	[NL80211_ATTR_EXTERNAL_AUTH_SUPPORT] = { .type = NLA_FLAG },
> +	[NL80211_ATTR_BTCOEX_OP] = { .type = NLA_U8 },

so:	[NL80211_ATTR_BTCOEX_ENABLE] = { .type = NLA_FLAG },

>   };
>
>   /* policy for the key attributes */
> @@ -12517,6 +12518,25 @@ static int nl80211_external_auth(struct sk_buff *skb, struct genl_info *info)
>   	return rdev_external_auth(rdev, dev, &params);
>   }
>
> +static int nl80211_set_btcoex(struct sk_buff *skb, struct genl_info *info)
> +{
> +	struct cfg80211_registered_device *rdev = info->user_ptr[0];
> +	u8 val = 0;
> +
> +	if (!rdev->ops->set_btcoex)
> +		return -ENOTSUPP;

For missing callback ops we generally use -EOPNOTSUPP.

> +	if (!info->attrs[NL80211_ATTR_BTCOEX_OP])
> +		return -EINVAL;
> +
> +	val = nla_get_u8(info->attrs[NL80211_ATTR_BTCOEX_OP]);
> +
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	return rdev_set_btcoex(rdev, val);

When using NLA_FLAG you can just say:

   +	return rdev_set_btcoex(rdev,
   +			       info->attrs[NL80211_ATTR_BTCOEX_ENABLE]);

and get rid of the EINVAL checks. Of course, you need to document the 
attribute needs to be added to enable and omitted to disable btcoex.

> +}
> +
>   #define NL80211_FLAG_NEED_WIPHY		0x01
>   #define NL80211_FLAG_NEED_NETDEV	0x02
>   #define NL80211_FLAG_NEED_RTNL		0x04

Regards,
Arend

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

[Kalle Valo]

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 3/1/2018 6:59 PM, Tamizh chelvam wrote:
>> From: Tamizh chelvam <tamizhr@codeaurora.org>
>>
>> This patch introduces NL80211_CMD_SET_BTCOEX command and
>> NL80211_ATTR_BTCOEX_OP attribute to enable or disable btcoex.
>
> What kind of btcoex are we talking about here? Is it signalling
> between wlan and bt to get access to the shared RF.

Yes, at least that's how I understand this.

> Why would it require user-space interaction? Are there no options for
> wlan to detect bt is in use, ie. bt hci is setup, and vice versa. Can
> it be indicated in platform data or device tree. Trying to understand
> the use-case here.

One use case is being able to disable btcoex in case of problems or to
test if it's btcoex related. I think during the last five years the need
for this interface has come every once in a while.

-- 
Kalle Valo

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

[Arend van Spriel]

+ Marcel

On 3/2/2018 6:14 AM, Kalle Valo wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> On 3/1/2018 6:59 PM, Tamizh chelvam wrote:
>>> From: Tamizh chelvam <tamizhr@codeaurora.org>
>>>
>>> This patch introduces NL80211_CMD_SET_BTCOEX command and
>>> NL80211_ATTR_BTCOEX_OP attribute to enable or disable btcoex.
>>
>> What kind of btcoex are we talking about here? Is it signalling
>> between wlan and bt to get access to the shared RF.
>
> Yes, at least that's how I understand this.
>
>> Why would it require user-space interaction? Are there no options for
>> wlan to detect bt is in use, ie. bt hci is setup, and vice versa. Can
>> it be indicated in platform data or device tree. Trying to understand
>> the use-case here.
>
> One use case is being able to disable btcoex in case of problems or to
> test if it's btcoex related. I think during the last five years the need
> for this interface has come every once in a while.

Well, you would want to disable btcoex *and* bt to verify wlan is 
working properly on its own. And similarly disable btcoex *and* wlan to 
verify bt works properly.

Now I do recall a thread between you and Marcel. Looked it up and it was 
this thread [1], but did not see a follow-up on it. I suspect it 
involves more than just an enable/disable state. That may be fine for 
devices in which BT and WLAN are integrated and coordination of RF use 
is done on the device. The "btcoex subsystem" thread seems to aim for 
more like providing the coordination logic so independent BT device and 
WLAN device can still use the same RF. So before adopting the api in 
nl80211 it would be good to revive that thread.

Regards,
Arend

[1] https://www.spinics.net/lists/linux-wireless/msg133333.html

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

[Marcel Holtmann]

Hi Arend,

>>>> This patch introduces NL80211_CMD_SET_BTCOEX command and
>>>> NL80211_ATTR_BTCOEX_OP attribute to enable or disable btcoex.
>>> 
>>> What kind of btcoex are we talking about here? Is it signalling
>>> between wlan and bt to get access to the shared RF.
>> 
>> Yes, at least that's how I understand this.
>> 
>>> Why would it require user-space interaction? Are there no options for
>>> wlan to detect bt is in use, ie. bt hci is setup, and vice versa. Can
>>> it be indicated in platform data or device tree. Trying to understand
>>> the use-case here.
>> 
>> One use case is being able to disable btcoex in case of problems or to
>> test if it's btcoex related. I think during the last five years the need
>> for this interface has come every once in a while.
> 
> Well, you would want to disable btcoex *and* bt to verify wlan is working properly on its own. And similarly disable btcoex *and* wlan to verify bt works properly.
> 
> Now I do recall a thread between you and Marcel. Looked it up and it was this thread [1], but did not see a follow-up on it. I suspect it involves more than just an enable/disable state. That may be fine for devices in which BT and WLAN are integrated and coordination of RF use is done on the device. The "btcoex subsystem" thread seems to aim for more like providing the coordination logic so independent BT device and WLAN device can still use the same RF. So before adopting the api in nl80211 it would be good to revive that thread.

actually we can solve any number of WiFi devices vs any number of Bluetooth devices on the same host. It goes mainly in the direction of WiFi having a notifier about its currently used channels and Bluetooth subsystem can subscribe to these and issue AFH channel map classification updates as needed.

The other way around for the Bluetooth audio cases and a shared antenna, you need a tighter integration. If the antenna is not shared, then it makes no difference anyway since you are not sharing the transceiver. Normally these are just handled by the firmware internally and right priorities for RF scheduling are done. The Bluetooth controller normally then does a bit too much, but nobody wants to implement that in the host cleanly, and so they all hack it into the controller. I think that Realtek has some older chips where it would be really needed in the host. And Intel controllers can also let the host do it, but by default it is magically done by the controller.

Regards

Marcel

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

[Arend van Spriel]

On 3/2/2018 10:59 AM, Marcel Holtmann wrote:
> Hi Arend,
>
>>>>> This patch introduces NL80211_CMD_SET_BTCOEX command and
>>>>> NL80211_ATTR_BTCOEX_OP attribute to enable or disable btcoex.
>>>>
>>>> What kind of btcoex are we talking about here? Is it signalling
>>>> between wlan and bt to get access to the shared RF.
>>>
>>> Yes, at least that's how I understand this.
>>>
>>>> Why would it require user-space interaction? Are there no options for
>>>> wlan to detect bt is in use, ie. bt hci is setup, and vice versa. Can
>>>> it be indicated in platform data or device tree. Trying to understand
>>>> the use-case here.
>>>
>>> One use case is being able to disable btcoex in case of problems or to
>>> test if it's btcoex related. I think during the last five years the need
>>> for this interface has come every once in a while.
>>
>> Well, you would want to disable btcoex *and* bt to verify wlan is working properly on its own. And similarly disable btcoex *and* wlan to verify bt works properly.
>>
>> Now I do recall a thread between you and Marcel. Looked it up and it was this thread [1], but did not see a follow-up on it. I suspect it involves more than just an enable/disable state. That may be fine for devices in which BT and WLAN are integrated and coordination of RF use is done on the device. The "btcoex subsystem" thread seems to aim for more like providing the coordination logic so independent BT device and WLAN device can still use the same RF. So before adopting the api in nl80211 it would be good to revive that thread.
>
> actually we can solve any number of WiFi devices vs any number of Bluetooth devices on the same host. It goes mainly in the direction of WiFi having a notifier about its currently used channels and Bluetooth subsystem can subscribe to these and issue AFH channel map classification updates as needed.

I see. From this I would say the notifier functionality would sit nicely 
in cfg80211.

> The other way around for the Bluetooth audio cases and a shared antenna, you need a tighter integration. If the antenna is not shared, then it makes no difference anyway since you are not sharing the transceiver. Normally these are just handled by the firmware internally and right priorities for RF scheduling are done. The Bluetooth controller normally then does a bit too much, but nobody wants to implement that in the host cleanly, and so they all hack it into the controller. I think that Realtek has some older chips where it would be really needed in the host. And Intel controllers can also let the host do it, but by default it is magically done by the controller.

Not entirely sure what you mean by "does a bit too much", but this is 
what I considered as being btcoex. Indeed most do the whole coordination 
in the device and not on the host. Pretty sure about Broadcom ;-)

Regards,
Arend

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

[Kalle Valo]

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> + Marcel
>
> On 3/2/2018 6:14 AM, Kalle Valo wrote:
>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>>> On 3/1/2018 6:59 PM, Tamizh chelvam wrote:
>>>> From: Tamizh chelvam <tamizhr@codeaurora.org>
>>>>
>>>> This patch introduces NL80211_CMD_SET_BTCOEX command and
>>>> NL80211_ATTR_BTCOEX_OP attribute to enable or disable btcoex.
>>>
>>> What kind of btcoex are we talking about here? Is it signalling
>>> between wlan and bt to get access to the shared RF.
>>
>> Yes, at least that's how I understand this.
>>
>>> Why would it require user-space interaction? Are there no options for
>>> wlan to detect bt is in use, ie. bt hci is setup, and vice versa. Can
>>> it be indicated in platform data or device tree. Trying to understand
>>> the use-case here.
>>
>> One use case is being able to disable btcoex in case of problems or to
>> test if it's btcoex related. I think during the last five years the need
>> for this interface has come every once in a while.
>
> Well, you would want to disable btcoex *and* bt to verify wlan is
> working properly on its own. And similarly disable btcoex *and* wlan
> to verify bt works properly.

Sure. But my main motiviation with this is to replace the module
parameters we already have:

drivers/net/wireless/ath/ath9k/htc_drv_init.c:module_param_named(btcoex_enable, ath9k_htc_btcoex_enable, int, 0444);
drivers/net/wireless/ath/ath9k/htc_drv_init.c:MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");

drivers/net/wireless/ath/ath9k/init.c:module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
drivers/net/wireless/ath/ath9k/init.c:MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");

drivers/net/wireless/broadcom/b43/main.c:module_param_named(btcoex, modparam_btcoex, int, 0444);
drivers/net/wireless/broadcom/b43/main.c:MODULE_PARM_DESC(btcoex, "Enable Bluetooth coexistence (default on)");

But looking closely in ath9k cases it doesn't actually sound doable as
IIRC the ath9k module parameters are off by default. In b43 it's enabled
by default, though.

> Now I do recall a thread between you and Marcel. Looked it up and it
> was this thread [1], but did not see a follow-up on it. I suspect it
> involves more than just an enable/disable state. That may be fine for
> devices in which BT and WLAN are integrated and coordination of RF use
> is done on the device. The "btcoex subsystem" thread seems to aim for
> more like providing the coordination logic so independent BT device
> and WLAN device can still use the same RF. So before adopting the api
> in nl80211 it would be good to revive that thread.

I think that "btcoex subsystem" is a holy grail which will be never
implemented :) But who knows, miracles can happen...

-- 
Kalle Valo

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

[Kalle Valo]

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> + Marcel
>
> On 3/2/2018 6:14 AM, Kalle Valo wrote:
>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>>> On 3/1/2018 6:59 PM, Tamizh chelvam wrote:
>>>> From: Tamizh chelvam <tamizhr@codeaurora.org>
>>>>
>>>> This patch introduces NL80211_CMD_SET_BTCOEX command and
>>>> NL80211_ATTR_BTCOEX_OP attribute to enable or disable btcoex.
>>>
>>> What kind of btcoex are we talking about here? Is it signalling
>>> between wlan and bt to get access to the shared RF.
>>
>> Yes, at least that's how I understand this.
>>
>>> Why would it require user-space interaction? Are there no options for
>>> wlan to detect bt is in use, ie. bt hci is setup, and vice versa. Can
>>> it be indicated in platform data or device tree. Trying to understand
>>> the use-case here.
>>
>> One use case is being able to disable btcoex in case of problems or to
>> test if it's btcoex related. I think during the last five years the need
>> for this interface has come every once in a while.
>
> Well, you would want to disable btcoex *and* bt to verify wlan is
> working properly on its own. And similarly disable btcoex *and* wlan
> to verify bt works properly.

Sure. But my main motiviation with this is to replace the module
parameters we already have:

drivers/net/wireless/ath/ath9k/htc_drv_init.c:module_param_named(btcoex_enable, ath9k_htc_btcoex_enable, int, 0444);
drivers/net/wireless/ath/ath9k/htc_drv_init.c:MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");

drivers/net/wireless/ath/ath9k/init.c:module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
drivers/net/wireless/ath/ath9k/init.c:MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");

drivers/net/wireless/broadcom/b43/main.c:module_param_named(btcoex, modparam_btcoex, int, 0444);
drivers/net/wireless/broadcom/b43/main.c:MODULE_PARM_DESC(btcoex, "Enable Bluetooth coexistence (default on)");

But looking closely in ath9k cases it doesn't actually sound doable as
IIRC the ath9k module parameters are off by default. In b43 it's enabled
by default, though.

> Now I do recall a thread between you and Marcel. Looked it up and it
> was this thread [1], but did not see a follow-up on it. I suspect it
> involves more than just an enable/disable state. That may be fine for
> devices in which BT and WLAN are integrated and coordination of RF use
> is done on the device. The "btcoex subsystem" thread seems to aim for
> more like providing the coordination logic so independent BT device
> and WLAN device can still use the same RF. So before adopting the api
> in nl80211 it would be good to revive that thread.

I think that "btcoex subsystem" is a holy grail which will be never
implemented :) But who knows, miracles can happen...

-- 
Kalle Valo