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

[Johannes Berg <johannes@sipsolutions.net>]

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