From: Tamizh chelvam <[email protected]>
This patchset add support for BTCOEX feature to enable/disable and
modifying btcoex priority value via nl80211
Tamizh chelvam (2):
cfg80211: Add support to enable or disable btcoex and set
btcoex_priority
mac80211: Add support to enable or disable btcoex and set btcoex
priority value
v4 :
* Moved btcoex_priority_support_flag enum to nl80211.h and renamed it.
* fixed typo.
v3 :
* Introduced NL80211_EXT_FEATURE_BTCOEX_PRIORITY to expose
btcoex priority support and removed bool variable.
v2 :
* Introduced NL80211_CMD_SET_BTCOEX to enable/disable btcoex and
to set/modify btcoex_priority.
* Added bool variable in wiphy structure to advertise btcoex_priority
feature and removed BITMAP calculation for btcoex_priority value.
include/net/cfg80211.h | 6 +++++
include/net/mac80211.h | 5 +++++
include/uapi/linux/nl80211.h | 50 ++++++++++++++++++++++++++++++++++++++++++
net/mac80211/cfg.c | 9 ++++++++
net/mac80211/driver-ops.h | 15 +++++++++++++
net/mac80211/trace.h | 20 +++++++++++++++++
net/wireless/nl80211.c | 45 +++++++++++++++++++++++++++++++++++++
net/wireless/rdev-ops.h | 13 +++++++++++
net/wireless/trace.h | 17 ++++++++++++++
9 files changed, 180 insertions(+)
--
1.7.9.5
On Wed, 2017-04-12 at 07:08 +0000, Tamizh Chelvam Raja wrote:
> >
> > So you make a distinction between WMM ACs, but what about the
> > different
> > types/profiles of BT traffic?
> >
>
> [Tamizh] There will be BT high and BT low traffic. It will be decided
> by BT module. Firmware internally checks BT low traffic with wlan
> traffic. If we enable some of wlan frames as high priority, those
> frames will have more priority than BT low traffic.
That ("firmware internally..." etc.) really sounds more like an
argument *not* to apply this patch ...
Everyone has their favourite BT coex control. We could possibly
implement this in our driver, but I'm not sure we'd *want* to, since
it's so far from what we actually do today.
Do we really need more than toggling it on/off?
Actually, I probably should've asked this much earlier - but what use
cases do you see for this? What can a user, or userspace application
like NM, really try to set here? It seems the use cases for this would
be rather constrained?
johannes
+ linux-bluetooth, Marcel
On 11-4-2017 12:15, [email protected] wrote:
> From: Tamizh chelvam <[email protected]>
>
> This patchset add support for BTCOEX feature to enable/disable and
> modifying btcoex priority value via nl80211
So you make a distinction between WMM ACs, but what about the different
types/profiles of BT traffic?
Regards,
Arend
> Tamizh chelvam (2):
> cfg80211: Add support to enable or disable btcoex and set
> btcoex_priority
> mac80211: Add support to enable or disable btcoex and set btcoex
> priority value
>
> v4 :
> * Moved btcoex_priority_support_flag enum to nl80211.h and renamed it.
> * fixed typo.
>
> v3 :
> * Introduced NL80211_EXT_FEATURE_BTCOEX_PRIORITY to expose
> btcoex priority support and removed bool variable.
>
> v2 :
> * Introduced NL80211_CMD_SET_BTCOEX to enable/disable btcoex and
> to set/modify btcoex_priority.
> * Added bool variable in wiphy structure to advertise btcoex_priority
> feature and removed BITMAP calculation for btcoex_priority value.
>
> include/net/cfg80211.h | 6 +++++
> include/net/mac80211.h | 5 +++++
> include/uapi/linux/nl80211.h | 50 ++++++++++++++++++++++++++++++++++++++++++
> net/mac80211/cfg.c | 9 ++++++++
> net/mac80211/driver-ops.h | 15 +++++++++++++
> net/mac80211/trace.h | 20 +++++++++++++++++
> net/wireless/nl80211.c | 45 +++++++++++++++++++++++++++++++++++++
> net/wireless/rdev-ops.h | 13 +++++++++++
> net/wireless/trace.h | 17 ++++++++++++++
> 9 files changed, 180 insertions(+)
>
From: Tamizh chelvam <[email protected]>
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 <[email protected]>
---
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(+)
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)
--
1.7.9.5
From: Tamizh chelvam <[email protected]>
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 and set or modify the btcoex priority value.
Signed-off-by: Tamizh chelvam <[email protected]>
---
include/net/mac80211.h | 5 +++++
net/mac80211/cfg.c | 9 +++++++++
net/mac80211/driver-ops.h | 15 +++++++++++++++
net/mac80211/trace.h | 20 ++++++++++++++++++++
4 files changed, 49 insertions(+)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index b1ac872..865eb7f 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: Called when BTCOEX is enabled/disabled, use
+ * this callback to enable or disable btcoex and use this callback to
+ * set btcoex_priority. BTCOEX should be enabled to set this priority.
*/
struct ieee80211_ops {
void (*tx)(struct ieee80211_hw *hw,
@@ -3734,6 +3737,8 @@ 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,
+ u32 btcoex_priority);
};
/**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 8bc3d366..512e5f7 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3617,6 +3617,14 @@ static int ieee80211_set_multicast_to_unicast(struct wiphy *wiphy,
return 0;
}
+static int ieee80211_set_btcoex(struct wiphy *wiphy, bool enabled,
+ u32 btcoex_priority)
+{
+ struct ieee80211_local *local = wiphy_priv(wiphy);
+
+ return drv_set_btcoex(local, enabled, btcoex_priority);
+}
+
const struct cfg80211_ops mac80211_config_ops = {
.add_virtual_intf = ieee80211_add_iface,
.del_virtual_intf = ieee80211_del_iface,
@@ -3709,4 +3717,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 09f77e4..c420da5 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1248,4 +1248,19 @@ 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, u32 btcoex_priority)
+{
+ int ret = -EOPNOTSUPP;
+
+ trace_drv_set_btcoex(local, enabled, btcoex_priority);
+ if (local->ops->set_btcoex)
+ ret = local->ops->set_btcoex(&local->hw, enabled,
+ btcoex_priority);
+
+ 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 0d645bc..3a4621e 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -742,6 +742,26 @@
TP_ARGS(local, value)
);
+TRACE_EVENT(drv_set_btcoex,
+ TP_PROTO(struct ieee80211_local *local, bool enabled,
+ u32 btcoex_priority),
+ TP_ARGS(local, enabled, btcoex_priority),
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ __field(bool, enabled)
+ __field(u32, btcoex_priority)
+ ),
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ __entry->enabled = enabled;
+ __entry->btcoex_priority = btcoex_priority;
+ ),
+ TP_printk(
+ LOCAL_PR_FMT " enabled:%d btcoex_priority :%u",
+ LOCAL_PR_ARG, __entry->enabled, __entry->btcoex_priority
+ )
+);
+
TRACE_EVENT(drv_set_coverage_class,
TP_PROTO(struct ieee80211_local *local, s16 value),
--
1.7.9.5
> + linux-bluetooth, Marcel
>=20
> On 11-4-2017 12:15, [email protected] wrote:
> > From: Tamizh chelvam <[email protected]>
> >
> > This patchset add support for BTCOEX feature to enable/disable and
> > modifying btcoex priority value via nl80211
>=20
> So you make a distinction between WMM ACs, but what about the different
> types/profiles of BT traffic?
>
[Tamizh] There will be BT high and BT low traffic. It will be decided by BT=
module. Firmware internally checks BT low traffic with wlan traffic. If we=
enable some of
wlan frames as high priority, those frames will have more priority than BT =
low traffic.
=20
> Regards,
> Arend
>=20
> > Tamizh chelvam (2):
> > cfg80211: Add support to enable or disable btcoex and set
> > btcoex_priority
> > mac80211: Add support to enable or disable btcoex and set btcoex
> > priority value
> >
> > v4 :
> > * Moved btcoex_priority_support_flag enum to nl80211.h and renamed it=
.
> > * fixed typo.
> >
> > v3 :
> > * Introduced NL80211_EXT_FEATURE_BTCOEX_PRIORITY to expose
> > btcoex priority support and removed bool variable.
> >
> > v2 :
> > * Introduced NL80211_CMD_SET_BTCOEX to enable/disable btcoex and
> > to set/modify btcoex_priority.
> > * Added bool variable in wiphy structure to advertise btcoex_priority
> > feature and removed BITMAP calculation for btcoex_priority value.
> >
> > include/net/cfg80211.h | 6 +++++
> > include/net/mac80211.h | 5 +++++
> > include/uapi/linux/nl80211.h | 50
> ++++++++++++++++++++++++++++++++++++++++++
> > net/mac80211/cfg.c | 9 ++++++++
> > net/mac80211/driver-ops.h | 15 +++++++++++++
> > net/mac80211/trace.h | 20 +++++++++++++++++
> > net/wireless/nl80211.c | 45
> +++++++++++++++++++++++++++++++++++++
> > net/wireless/rdev-ops.h | 13 +++++++++++
> > net/wireless/trace.h | 17 ++++++++++++++
> > 9 files changed, 180 insertions(+)
> >
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