[PATCH] cfg80211: Add new helper function for channels

[PATCH] cfg80211: Add new helper function for channels

[Amar Singhal]

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 <asinghal@codeaurora.org>
---
 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

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

[Johannes Berg]

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 <asinghal@codeaurora.org>
> ---
>  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

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

[Arend Van Spriel]

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 <asinghal@codeaurora.org>
> ---
>   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

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

[Johannes Berg]

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

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

[Arend Van Spriel]

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

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

[Jeff Johnson]

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