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 |
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
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
This patch will also (and correctly so) cause compilation warnings in mac80211, so you really should have just two patches. johannes
> 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
> > 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
> > > > 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
> > [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 --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,