[PATCH wireless-next v2] wifi: cfg80211: init S1G properly when creating chandef

[PATCH wireless-next v2] wifi: cfg80211: init S1G properly when creating chandef

[Lachlan Hodges]

When creating a chandef, the initial width is determined by the
chan_type argument leading to a default width of 20/20_NOHT/40 which
does not make sense for S1G channels. Fix this by ensuring we default
S1G chandefs to a width of 1MHz.

Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
v1 -> v2:

- Remove the mac80211-based wording in the commit message and keep it
  simple.
---
 net/wireless/chan.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 68221b1ab45e..c0c0151fe55f 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -32,6 +32,12 @@ void cfg80211_chandef_create(struct cfg80211_chan_def *chandef,
 		.freq1_offset = chan->freq_offset,
 	};
 
+	if (cfg80211_chandef_is_s1g(chandef)) {
+		chandef->width = NL80211_CHAN_WIDTH_1;
+		chandef->center_freq1 = chan->center_freq;
+		return;
+	}
+
 	switch (chan_type) {
 	case NL80211_CHAN_NO_HT:
 		chandef->width = NL80211_CHAN_WIDTH_20_NOHT;
-- 
2.43.0

Re: [PATCH wireless-next v2] wifi: cfg80211: init S1G properly when creating chandef

[Johannes Berg]

On Wed, 2026-03-11 at 17:43 +1100, Lachlan Hodges wrote:
> When creating a chandef, the initial width is determined by the
> chan_type argument leading to a default width of 20/20_NOHT/40 which
> does not make sense for S1G channels. Fix this by ensuring we default
> S1G chandefs to a width of 1MHz.
> 
> Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
> ---
> v1 -> v2:
> 
> - Remove the mac80211-based wording in the commit message and keep it
>   simple.

Thanks :)

Now with more tea and all that, do we really want it to completely
ignore the width? Should that be documented? Almost feels like it should
WARN if you use it wrong, with an S1G channel but a regular width, and
mac80211 should be fixed?

johannes

Re: [PATCH wireless-next v2] wifi: cfg80211: init S1G properly when creating chandef

[Lachlan Hodges]

On Wed, Mar 11, 2026 at 10:55:46AM +0100, Johannes Berg wrote:
> On Wed, 2026-03-11 at 17:43 +1100, Lachlan Hodges wrote:
> > When creating a chandef, the initial width is determined by the
> > chan_type argument leading to a default width of 20/20_NOHT/40 which
> > does not make sense for S1G channels. Fix this by ensuring we default
> > S1G chandefs to a width of 1MHz.
> > 
> > Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
> > ---
> > v1 -> v2:
> > 
> > - Remove the mac80211-based wording in the commit message and keep it
> >   simple.
> 
> Thanks :)
> 
> Now with more tea and all that, do we really want it to completely
> ignore the width? Should that be documented? Almost feels like it should
> WARN if you use it wrong, with an S1G channel but a regular width, and
> mac80211 should be fixed?
 
When you say "ignore the width" do you mean nothing checks it? If
that's the case we probably need to add something within
cfg80211_chandef_valid() to ensure if we have an S1G chan we don't
have a non-S1G width i.e > 16MHz. That would fix mac80211 from
giving it to the driver in the first place.

lachlan

Re: [PATCH wireless-next v2] wifi: cfg80211: init S1G properly when creating chandef

[Johannes Berg]

On Wed, 2026-03-11 at 21:21 +1100, Lachlan Hodges wrote:
> On Wed, Mar 11, 2026 at 10:55:46AM +0100, Johannes Berg wrote:
> > On Wed, 2026-03-11 at 17:43 +1100, Lachlan Hodges wrote:
> > > When creating a chandef, the initial width is determined by the
> > > chan_type argument leading to a default width of 20/20_NOHT/40 which
> > > does not make sense for S1G channels. Fix this by ensuring we default
> > > S1G chandefs to a width of 1MHz.
> > > 
> > > Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
> > > ---
> > > v1 -> v2:
> > > 
> > > - Remove the mac80211-based wording in the commit message and keep it
> > >   simple.
> > 
> > Thanks :)
> > 
> > Now with more tea and all that, do we really want it to completely
> > ignore the width? Should that be documented? Almost feels like it should
> > WARN if you use it wrong, with an S1G channel but a regular width, and
> > mac80211 should be fixed?
>  
> When you say "ignore the width" 

Sorry, I meant the channel type (chantype argument).

> do you mean nothing checks it?

I meant for s1g the chantype argument would be ignored now, not that it
wasn't checked.

> If
> that's the case we probably need to add something within
> cfg80211_chandef_valid() to ensure if we have an S1G chan we don't
> have a non-S1G width i.e > 16MHz. That would fix mac80211 from
> giving it to the driver in the first place.

We don't have that? Seems like an oversight too, but I was thinking like
this (overall changes, not split to commits), since
cfg80211_chandef_create() really doesn't work for S1G since the chantype
is 20MHz-non-HT, 20MHz-HT, HT40+ or HT40-.


diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index b0451f1c8e79..9a055dfb445a 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1118,6 +1118,19 @@ ieee80211_ifcomb_check(const struct ieee80211_iface_combination *c, int n_comb)
 	return true;
 }
 
+static void ieee80211_create_default_chandef(struct cfg80211_chan_def *chandef,
+					     struct ieee80211_channel *chan)
+{
+	*chandef = (struct cfg80211_chan_def) {
+		.chan = chan,
+		.width = chan->band == NL80211_BAND_S1GHZ ?
+			NL80211_CHAN_WIDTH_1 :
+			NL80211_CHAN_WIDTH_20_NOHT,
+		.center_freq1 = chan->center_freq,
+		.freq1_offset = chan->freq_offset,
+	}
+}
+
 int ieee80211_register_hw(struct ieee80211_hw *hw)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
@@ -1261,9 +1274,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 			/* if none found then use the first anyway */
 			if (i == sband->n_channels)
 				i = 0;
-			cfg80211_chandef_create(&dflt_chandef,
-						&sband->channels[i],
-						NL80211_CHAN_NO_HT);
+			ieee80211_create_default_chandef(&dflt_chandef,
+							 &sband->channels[i]);
 			/* init channel we're on */
 			local->monitor_chanreq.oper = dflt_chandef;
 			if (local->emulate_chanctx) {
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index d9d4e043bb39..4ae2f6ca0378 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -29,9 +29,10 @@ void cfg80211_chandef_create(struct cfg80211_chan_def *chandef,
 
 	*chandef = (struct cfg80211_chan_def) {
 		.chan = chan,
-		.freq1_offset = chan->freq_offset,
 	};
 
+	WARN_ON(chan->band == NL80211_BAND_S1GHZ);
+
 	switch (chan_type) {
 	case NL80211_CHAN_NO_HT:
 		chandef->width = NL80211_CHAN_WIDTH_20_NOHT;


johannes

Re: [PATCH wireless-next v2] wifi: cfg80211: init S1G properly when creating chandef

[Johannes Berg]

On Wed, 2026-03-11 at 11:56 +0100, Johannes Berg wrote:
> 
> +++ b/net/wireless/chan.c
> @@ -29,9 +29,10 @@ void cfg80211_chandef_create(struct cfg80211_chan_def *chandef,
>  
>  	*chandef = (struct cfg80211_chan_def) {
>  		.chan = chan,
> -		.freq1_offset = chan->freq_offset,
>  	};
>  
> +	WARN_ON(chan->band == NL80211_BAND_S1GHZ);
> 

Hm, no, I guess that really should only allow 2.4, 5 and 6 GHz.

johannes

Re: [PATCH wireless-next v2] wifi: cfg80211: init S1G properly when creating chandef

[Lachlan Hodges]

On Wed, Mar 11, 2026 at 11:56:37AM +0100, Johannes Berg wrote:
> On Wed, 2026-03-11 at 21:21 +1100, Lachlan Hodges wrote:
> > On Wed, Mar 11, 2026 at 10:55:46AM +0100, Johannes Berg wrote:
> > > On Wed, 2026-03-11 at 17:43 +1100, Lachlan Hodges wrote:
> > > > When creating a chandef, the initial width is determined by the
> > > > chan_type argument leading to a default width of 20/20_NOHT/40 which
> > > > does not make sense for S1G channels. Fix this by ensuring we default
> > > > S1G chandefs to a width of 1MHz.
> > > > 
> > > > Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
> > > > ---
> > > > v1 -> v2:
> > > > 
> > > > - Remove the mac80211-based wording in the commit message and keep it
> > > >   simple.
> > > 
> > > Thanks :)
> > > 
> > > Now with more tea and all that, do we really want it to completely
> > > ignore the width? Should that be documented? Almost feels like it should
> > > WARN if you use it wrong, with an S1G channel but a regular width, and
> > > mac80211 should be fixed?
> >  
> > When you say "ignore the width" 
> 
> Sorry, I meant the channel type (chantype argument).
> 
> > do you mean nothing checks it?
> 
> I meant for s1g the chantype argument would be ignored now, not that it
> wasn't checked.
> 
> > If
> > that's the case we probably need to add something within
> > cfg80211_chandef_valid() to ensure if we have an S1G chan we don't
> > have a non-S1G width i.e > 16MHz. That would fix mac80211 from
> > giving it to the driver in the first place.
> 
> We don't have that? Seems like an oversight too,

Yea we have one way not the other, I will send a fix for that at
some point.

> but I was thinking like
> this (overall changes, not split to commits), since
> cfg80211_chandef_create() really doesn't work for S1G since the chantype
> is 20MHz-non-HT, 20MHz-HT, HT40+ or HT40-.
> 
> 
> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> index b0451f1c8e79..9a055dfb445a 100644
> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c
> @@ -1118,6 +1118,19 @@ ieee80211_ifcomb_check(const struct ieee80211_iface_combination *c, int n_comb)
>  	return true;
>  }
>  
> +static void ieee80211_create_default_chandef(struct cfg80211_chan_def *chandef,
> +					     struct ieee80211_channel *chan)
> +{
> +	*chandef = (struct cfg80211_chan_def) {
> +		.chan = chan,
> +		.width = chan->band == NL80211_BAND_S1GHZ ?
> +			NL80211_CHAN_WIDTH_1 :
> +			NL80211_CHAN_WIDTH_20_NOHT,
> +		.center_freq1 = chan->center_freq,
> +		.freq1_offset = chan->freq_offset,
> +	}
> +}
> +
>  int ieee80211_register_hw(struct ieee80211_hw *hw)
>  {
>  	struct ieee80211_local *local = hw_to_local(hw);
> @@ -1261,9 +1274,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
>  			/* if none found then use the first anyway */
>  			if (i == sband->n_channels)
>  				i = 0;
> -			cfg80211_chandef_create(&dflt_chandef,
> -						&sband->channels[i],
> -						NL80211_CHAN_NO_HT);
> +			ieee80211_create_default_chandef(&dflt_chandef,
> +							 &sband->channels[i]);
>  			/* init channel we're on */
>  			local->monitor_chanreq.oper = dflt_chandef;
>  			if (local->emulate_chanctx) {
> diff --git a/net/wireless/chan.c b/net/wireless/chan.c
> index d9d4e043bb39..4ae2f6ca0378 100644
> --- a/net/wireless/chan.c
> +++ b/net/wireless/chan.c
> @@ -29,9 +29,10 @@ void cfg80211_chandef_create(struct cfg80211_chan_def *chandef,
>  
>  	*chandef = (struct cfg80211_chan_def) {
>  		.chan = chan,
> -		.freq1_offset = chan->freq_offset,
>  	};
>  
> +	WARN_ON(chan->band == NL80211_BAND_S1GHZ);
> +
>  	switch (chan_type) {
>  	case NL80211_CHAN_NO_HT:
>  		chandef->width = NL80211_CHAN_WIDTH_20_NOHT;
> 

Initially reading this I was a bit concerned because it means any
path calling cfg80211_chandef_create() wouldn't work for S1G; But it's
actually only a couple of places now that call it and - being forward
looking - for example with channel switching we would probably have
an S1G specific path similar to processing a S1G connection anyway. No
important paths (will confirm this) call this function anyway they
seem to build the chandef locally.

I'll do some testing, if it seems OK will send a new patchset.

lachlan

Re: [PATCH wireless-next v2] wifi: cfg80211: init S1G properly when creating chandef

[Johannes Berg]

On Wed, 2026-03-11 at 22:48 +1100, Lachlan Hodges wrote:
> Initially reading this I was a bit concerned because it means any
> path calling cfg80211_chandef_create() wouldn't work for S1G

Yes, I agree, but I tend to think it fundamentally doesn't work for S1G
anyway the way it's defined with 20 or 40 MHz width. Now you had
somewhat made it work by just ignoring the chantype, but that seems
confusing too, at least in the long run.

> But it's
> actually only a couple of places now that call it and - being forward
> looking - for example with channel switching we would probably have
> an S1G specific path similar to processing a S1G connection anyway.

Right, for mac80211: I don't know if you will have IBSS on S1G, if not
that's right out, TDLS might be an issue, mesh support is probably not
there, HT/HE operation elements (util.c) are irrelevant, which basically
leaves the one I patched, and CSA that you mentioned.

For cfg80211 it's a similar story, but maybe we need to look - but
anything that can hit 60 GHz today would already be invalid too...


> No
> important paths (will confirm this) call this function anyway they
> seem to build the chandef locally.

There are a lot of places calling it in drivers so I don't think we can
remove the function, but all existing drivers without S1G anyway. In
mac80211 there might be paths, but then with the warning I guess syzbot
can find them ;)

johannes