| Message ID | 1395409651-26120-2-git-send-email-michal.kazior@tieto.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Fri, 2014-03-21 at 14:47 +0100, Michal Kazior wrote: > +int cfg80211_iter_combinations(struct wiphy *wiphy, > + const int num_different_channels, > + const u8 radar_detect, > + const int iftype_num[NUM_NL80211_IFTYPES], > + void (*iter)(const struct ieee80211_iface_combination *c, > + void *data), > + void *data); Maybe *iter should have a non-void return value and allow aborting the loop somehow? > +static void > +cfg80211_iter_sum_ifcombs(const struct ieee80211_iface_combination *c, > + void *data) > +{ > + int *num = data; > + (*num)++; > +} > + > +int cfg80211_check_combinations(struct wiphy *wiphy, > + const int num_different_channels, > + const u8 radar_detect, > + const int iftype_num[NUM_NL80211_IFTYPES]) > +{ > + int err, num = 0; > + > + err = cfg80211_iter_combinations(wiphy, num_different_channels, > + radar_detect, iftype_num, > + cfg80211_iter_sum_ifcombs, &num); > + if (err) > + return err; > + if (num == 0) > + return -EBUSY; > + > + return 0; > } This also seems a bit pointless - why not just have an int instead of num and set it to -EBUSY outside, and to 0 inside, and then just return it? You don't need the number of available combinations. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28 March 2014 14:05, Johannes Berg <johannes@sipsolutions.net> wrote: > On Fri, 2014-03-21 at 14:47 +0100, Michal Kazior wrote: > >> +int cfg80211_iter_combinations(struct wiphy *wiphy, >> + const int num_different_channels, >> + const u8 radar_detect, >> + const int iftype_num[NUM_NL80211_IFTYPES], >> + void (*iter)(const struct ieee80211_iface_combination *c, >> + void *data), >> + void *data); > > Maybe *iter should have a non-void return value and allow aborting the > loop somehow? I don't see much of use for that. You can still use *data for that. >> +static void >> +cfg80211_iter_sum_ifcombs(const struct ieee80211_iface_combination *c, >> + void *data) >> +{ >> + int *num = data; >> + (*num)++; >> +} >> + >> +int cfg80211_check_combinations(struct wiphy *wiphy, >> + const int num_different_channels, >> + const u8 radar_detect, >> + const int iftype_num[NUM_NL80211_IFTYPES]) >> +{ >> + int err, num = 0; >> + >> + err = cfg80211_iter_combinations(wiphy, num_different_channels, >> + radar_detect, iftype_num, >> + cfg80211_iter_sum_ifcombs, &num); >> + if (err) >> + return err; >> + if (num == 0) >> + return -EBUSY; >> + >> + return 0; >> } > > This also seems a bit pointless - why not just have an int instead of > num and set it to -EBUSY outside, and to 0 inside, and then just return > it? You don't need the number of available combinations. Hmm.. Well, that works too I guess. Micha? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2014-03-28 at 14:21 +0100, Michal Kazior wrote: > >> +int cfg80211_iter_combinations(struct wiphy *wiphy, > >> + const int num_different_channels, > >> + const u8 radar_detect, > >> + const int iftype_num[NUM_NL80211_IFTYPES], > >> + void (*iter)(const struct ieee80211_iface_combination *c, > >> + void *data), > >> + void *data); > > > > Maybe *iter should have a non-void return value and allow aborting the > > loop somehow? > > I don't see much of use for that. You can still use *data for that. No, I meant to abort the iteration loop, say when the function returns true (or false, whichever way you want to look at it). That way you can stop iteration at the first combination that's sufficient. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28 March 2014 14:22, Johannes Berg <johannes@sipsolutions.net> wrote: > On Fri, 2014-03-28 at 14:21 +0100, Michal Kazior wrote: > >> >> +int cfg80211_iter_combinations(struct wiphy *wiphy, >> >> + const int num_different_channels, >> >> + const u8 radar_detect, >> >> + const int iftype_num[NUM_NL80211_IFTYPES], >> >> + void (*iter)(const struct ieee80211_iface_combination *c, >> >> + void *data), >> >> + void *data); >> > >> > Maybe *iter should have a non-void return value and allow aborting the >> > loop somehow? >> >> I don't see much of use for that. You can still use *data for that. > > No, I meant to abort the iteration loop, say when the function returns > true (or false, whichever way you want to look at it). That way you can > stop iteration at the first combination that's sufficient. I know. It's just that it bothers me - if you use true/false you end up with "do I use true for continue or break" (and need to put comments in the code) and having an enum for such a trivial thing seems silly too. On the other hand using *data to guard *iter is also silly. So yeah. Micha? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2014-03-28 at 14:42 +0100, Michal Kazior wrote: > On 28 March 2014 14:22, Johannes Berg <johannes@sipsolutions.net> wrote: > > On Fri, 2014-03-28 at 14:21 +0100, Michal Kazior wrote: > > > >> >> +int cfg80211_iter_combinations(struct wiphy *wiphy, > >> >> + const int num_different_channels, > >> >> + const u8 radar_detect, > >> >> + const int iftype_num[NUM_NL80211_IFTYPES], > >> >> + void (*iter)(const struct ieee80211_iface_combination *c, > >> >> + void *data), > >> >> + void *data); > >> > > >> > Maybe *iter should have a non-void return value and allow aborting the > >> > loop somehow? > >> > >> I don't see much of use for that. You can still use *data for that. > > > > No, I meant to abort the iteration loop, say when the function returns > > true (or false, whichever way you want to look at it). That way you can > > stop iteration at the first combination that's sufficient. > > I know. It's just that it bothers me - if you use true/false you end > up with "do I use true for continue or break" (and need to put > comments in the code) and having an enum for such a trivial thing > seems silly too. On the other hand using *data to guard *iter is also > silly. Ok, let's leave it. It's not like we have a million combinations and iterating them is expensive ... johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 7f0cbfb..21766b9 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -4720,6 +4720,33 @@ int cfg80211_check_combinations(struct wiphy *wiphy, */ void cfg80211_stop_iface(struct wiphy *wiphy, struct wireless_dev *wdev); +/** + * cfg80211_iter_combinations - iterate over matching combinations + * + * @wiphy: the wiphy + * @num_different_channels: the number of different channels we want + * to use for verification + * @radar_detect: a bitmap where each bit corresponds to a channel + * width where radar detection is needed, as in the definition of + * &struct ieee80211_iface_combination.@radar_detect_widths + * @iftype_num: array with the numbers of interfaces of each interface + * type. The index is the interface type as specified in &enum + * nl80211_iftype. + * @iter: function to call for each matching combination + * @data: pointer to pass to iter function + * + * This function can be called by the driver to check what possible + * combinations it fits in at a given moment, e.g. for channel switching + * purposes. + */ +int cfg80211_iter_combinations(struct wiphy *wiphy, + const int num_different_channels, + const u8 radar_detect, + const int iftype_num[NUM_NL80211_IFTYPES], + void (*iter)(const struct ieee80211_iface_combination *c, + void *data), + void *data); + /* Logging, debugging and troubleshooting/diagnostic helpers. */ /* wiphy_printk helpers, similar to dev_printk */ diff --git a/net/wireless/util.c b/net/wireless/util.c index 241f0ad..7fa3503 100644 --- a/net/wireless/util.c +++ b/net/wireless/util.c @@ -1252,10 +1252,13 @@ int cfg80211_validate_beacon_int(struct cfg80211_registered_device *rdev, return res; } -int cfg80211_check_combinations(struct wiphy *wiphy, - const int num_different_channels, - const u8 radar_detect, - const int iftype_num[NUM_NL80211_IFTYPES]) +int cfg80211_iter_combinations(struct wiphy *wiphy, + const int num_different_channels, + const u8 radar_detect, + const int iftype_num[NUM_NL80211_IFTYPES], + void (*iter)(const struct ieee80211_iface_combination *c, + void *data), + void *data) { int i, j, iftype; int num_interfaces = 0; @@ -1312,13 +1315,40 @@ int cfg80211_check_combinations(struct wiphy *wiphy, /* This combination covered all interface types and * supported the requested numbers, so we're good. */ - kfree(limits); - return 0; + + (*iter)(c, data); cont: kfree(limits); } - return -EBUSY; + return 0; +} +EXPORT_SYMBOL(cfg80211_iter_combinations); + +static void +cfg80211_iter_sum_ifcombs(const struct ieee80211_iface_combination *c, + void *data) +{ + int *num = data; + (*num)++; +} + +int cfg80211_check_combinations(struct wiphy *wiphy, + const int num_different_channels, + const u8 radar_detect, + const int iftype_num[NUM_NL80211_IFTYPES]) +{ + int err, num = 0; + + err = cfg80211_iter_combinations(wiphy, num_different_channels, + radar_detect, iftype_num, + cfg80211_iter_sum_ifcombs, &num); + if (err) + return err; + if (num == 0) + return -EBUSY; + + return 0; } EXPORT_SYMBOL(cfg80211_check_combinations);
The patch splits cfg80211_check_combinations() into an iterator function and a simple iteration user. This makes it possible for drivers to asses how many channels can use given iftype setup. This in turn can be used for future multi-interface/multi-channel channel switching. Signed-off-by: Michal Kazior <michal.kazior@tieto.com> --- include/net/cfg80211.h | 27 +++++++++++++++++++++++++++ net/wireless/util.c | 44 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 64 insertions(+), 7 deletions(-)