2019-08-29 21:56:25

by Amar Singhal

[permalink] [raw]
Subject: [PATCH] cfg80211: Add new helper function for channels

Add new helper function to convert (chan_number, oper_class) pair to
frequency. Call this function ieee80211_channel_op_class_to_frequency.
This function would be very useful in the context of 6 GHz channels,
where channel number is not unique.

Signed-off-by: Amar Singhal <[email protected]>
---
include/net/cfg80211.h | 10 ++++++++++
net/wireless/util.c | 23 +++++++++++++++++++++++
2 files changed, 33 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 6467b60..decafba 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4914,1 +4914,1 @@ static inline void *wdev_priv(struct wireless_dev *wdev)
int ieee80211_channel_to_frequency(int chan, enum nl80211_band band);

/**
+ * ieee80211_channel_op_class_to_frequency - convert
+ * (channel, operating class) to frequency
+ * @chan_num: channel number
+ * @global_op_class: global operating class
+ *
+ * Return: The corresponding frequency, or 0 if the conversion failed.
+ */
+int ieee80211_channel_op_class_to_frequency(u8 chan_num, u8 global_op_class);
+
+/**
* ieee80211_frequency_to_channel - convert frequency to channel number
* @freq: center frequency
* Return: The corresponding channel, or 0 if the conversion failed.
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 9aba8d54..7f64b4a 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -144,6 +144,29 @@ struct ieee80211_channel *ieee80211_get_channel(struct wiphy *wiphy, int freq)
}
EXPORT_SYMBOL(ieee80211_get_channel);

+int ieee80211_channel_op_class_to_frequency(u8 chan_num, u8 global_op_class)
+{
+ if (global_op_class >= 131 && global_op_class <= 135)
+ return (5940 + 5 * chan_num);
+ else if (global_op_class >= 115 && global_op_class <= 130)
+ return (5000 + 5 * chan_num);
+ else if (global_op_class >= 112 && global_op_class <= 113)
+ return (5000 + 5 * chan_num);
+ else if (global_op_class >= 109 && global_op_class <= 110)
+ return (4000 + 5 * chan_num);
+ else if (global_op_class >= 83 && global_op_class <= 84)
+ return (2407 + 5 * chan_num);
+ else if (global_op_class == 81)
+ return (2407 + 5 * chan_num);
+ else if (global_op_class == 82)
+ return (2414 + 5 * chan_num);
+ else if (global_op_class == 180)
+ return (56160 + 5 * chan_num);
+ else
+ return 0;
+}
+EXPORT_SYMBOL(ieee80211_channel_op_class_to_frequency);
+
static void set_mandatory_flags_band(struct ieee80211_supported_band *sband)
{
int i, want;
--
1.9.1


2019-08-30 07:17:32

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: Add new helper function for channels

On Thu, 2019-08-29 at 14:49 -0700, Amar Singhal wrote:
> Add new helper function to convert (chan_number, oper_class) pair to
> frequency. Call this function ieee80211_channel_op_class_to_frequency.
> This function would be very useful in the context of 6 GHz channels,
> where channel number is not unique.

Nit: it is unique within 6 GHz, just not within the overall channel
number space, and that was actually already not unique before, it just
didn't matter much to us :-)

I may reword that when I apply it.

> Signed-off-by: Amar Singhal <[email protected]>
> ---
> include/net/cfg80211.h | 10 ++++++++++
> net/wireless/util.c | 23 +++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 6467b60..decafba 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -4914,1 +4914,1 @@ static inline void *wdev_priv(struct wireless_dev *wdev)
> int ieee80211_channel_to_frequency(int chan, enum nl80211_band band);
>
> /**
> + * ieee80211_channel_op_class_to_frequency - convert
> + * (channel, operating class) to frequency

That's formatted badly, the short description must fit on one line.

> + if (global_op_class >= 131 && global_op_class <= 135)
> + return (5940 + 5 * chan_num);
> + else if (global_op_class >= 115 && global_op_class <= 130)
> + return (5000 + 5 * chan_num);
> + else if (global_op_class >= 112 && global_op_class <= 113)
> + return (5000 + 5 * chan_num);
> + else if (global_op_class >= 109 && global_op_class <= 110)
> + return (4000 + 5 * chan_num);
> + else if (global_op_class >= 83 && global_op_class <= 84)
> + return (2407 + 5 * chan_num);
> + else if (global_op_class == 81)
> + return (2407 + 5 * chan_num);
> + else if (global_op_class == 82)
> + return (2414 + 5 * chan_num);
> + else if (global_op_class == 180)
> + return (56160 + 5 * chan_num);

I think it would be good to have a list of valid channel numbers for
them as well. I was wondering about 82 for a second there for example,
until I looked up again that it is just for channel 14.

I think this is also missing 83/84 and various other operating classes
for wider channels. Perhaps in those we don't really need to check the
channel numbers precisely, i.e. we could probably treat 81/83/84 all the
same.

But depending on what you feed to the function, it's possible that you
could encounter those other operating classes.

johannes

2019-08-30 10:40:58

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: Add new helper function for channels

On 8/29/2019 11:49 PM, Amar Singhal wrote:
> Add new helper function to convert (chan_number, oper_class) pair to
> frequency. Call this function ieee80211_channel_op_class_to_frequency.
> This function would be very useful in the context of 6 GHz channels,
> where channel number is not unique.

That 'unique' statement does not apply to 6GHz by itself. The addition
of 6GHz channels makes channel numbers across bands not unique.

The funcion
> Signed-off-by: Amar Singhal <[email protected]>
> ---
> include/net/cfg80211.h | 10 ++++++++++
> net/wireless/util.c | 23 +++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 6467b60..decafba 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -4914,1 +4914,1 @@ static inline void *wdev_priv(struct wireless_dev *wdev)
> int ieee80211_channel_to_frequency(int chan, enum nl80211_band band);
>
> /**
> + * ieee80211_channel_op_class_to_frequency - convert
> + * (channel, operating class) to frequency
> + * @chan_num: channel number
> + * @global_op_class: global operating class
> + *
> + * Return: The corresponding frequency, or 0 if the conversion failed.
> + */
> +int ieee80211_channel_op_class_to_frequency(u8 chan_num, u8 global_op_class);
> +
> +/**
> * ieee80211_frequency_to_channel - convert frequency to channel number
> * @freq: center frequency
> * Return: The corresponding channel, or 0 if the conversion failed.
> diff --git a/net/wireless/util.c b/net/wireless/util.c
> index 9aba8d54..7f64b4a 100644
> --- a/net/wireless/util.c
> +++ b/net/wireless/util.c
> @@ -144,6 +144,29 @@ struct ieee80211_channel *ieee80211_get_channel(struct wiphy *wiphy, int freq)
> }
> EXPORT_SYMBOL(ieee80211_get_channel);
>
> +int ieee80211_channel_op_class_to_frequency(u8 chan_num, u8 global_op_class)
> +{
> + if (global_op_class >= 131 && global_op_class <= 135)
> + return (5940 + 5 * chan_num);
> + else if (global_op_class >= 115 && global_op_class <= 130)
> + return (5000 + 5 * chan_num);
> + else if (global_op_class >= 112 && global_op_class <= 113)
> + return (5000 + 5 * chan_num);
> + else if (global_op_class >= 109 && global_op_class <= 110)
> + return (4000 + 5 * chan_num);
> + else if (global_op_class >= 83 && global_op_class <= 84)
> + return (2407 + 5 * chan_num);
> + else if (global_op_class == 81)
> + return (2407 + 5 * chan_num);
> + else if (global_op_class == 82)
> + return (2414 + 5 * chan_num);
> + else if (global_op_class == 180)
> + return (56160 + 5 * chan_num);
> + else
> + return 0;
> +}
> +EXPORT_SYMBOL(ieee80211_channel_op_class_to_frequency);

The function ieee80211_operating_class_to_band() uses ranges within
switch statement, eg.:

case 128 ... 130:
*band = NL80211_BAND_5GHZ;
return true;

For consistency it might be good to do the same here.

Regards,
Arend

2019-08-30 10:43:07

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: Add new helper function for channels

On Fri, 2019-08-30 at 12:40 +0200, Arend Van Spriel wrote:

> > +EXPORT_SYMBOL(ieee80211_channel_op_class_to_frequency);
>
> The function ieee80211_operating_class_to_band() uses ranges within
> switch statement, eg.:
>
> case 128 ... 130:
> *band = NL80211_BAND_5GHZ;
> return true;

No that you remind me - how is this new function not just a composition
of the existing ones?

i.e. just convert the op_class to band first, and then (band, channel)
to freq?

johannes

2019-08-30 11:03:45

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: Add new helper function for channels

On 8/30/2019 12:41 PM, Johannes Berg wrote:
> On Fri, 2019-08-30 at 12:40 +0200, Arend Van Spriel wrote:
>
>>> +EXPORT_SYMBOL(ieee80211_channel_op_class_to_frequency);
>>
>> The function ieee80211_operating_class_to_band() uses ranges within
>> switch statement, eg.:
>>
>> case 128 ... 130:
>> *band = NL80211_BAND_5GHZ;
>> return true;
>
> No that you remind me - how is this new function not just a composition
> of the existing ones?
>
> i.e. just convert the op_class to band first, and then (band, channel)
> to freq?

yup. that would have my preference actually.

Regards,
Arend

2019-08-30 14:10:48

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: Add new helper function for channels

On 2019-08-30 04:03, Arend Van Spriel wrote:
> On 8/30/2019 12:41 PM, Johannes Berg wrote:
>> On Fri, 2019-08-30 at 12:40 +0200, Arend Van Spriel wrote:
>>
>>>> +EXPORT_SYMBOL(ieee80211_channel_op_class_to_frequency);
>>>
>>> The function ieee80211_operating_class_to_band() uses ranges within
>>> switch statement, eg.:
>>>
>>> case 128 ... 130:
>>> *band = NL80211_BAND_5GHZ;
>>> return true;
>>
>> No that you remind me - how is this new function not just a
>> composition
>> of the existing ones?
>>
>> i.e. just convert the op_class to band first, and then (band, channel)
>> to freq?
>
> yup. that would have my preference actually.

Sigh. I had the same guidance in pre-review:

we already have ieee80211_operating_class_to_band() and
ieee80211_channel_to_frequency() so all this function should be is

return ieee80211_channel_to_frequency(chan,
ieee80211_operating_class_to_band(op_class))

but then again if anybody needs this functionality they can simply call
those same functions