wifi: mt76: inititalize sband.band to correct value

PATCH In review Bert Karwatzki 3 participants 4 messages Feb 7, 2024

Subsystems (3)

Arm MediaTek mac80211 MediaTek MT76 wireless LAN driver

lore

Share & tools

lore

Bert Karwatzki

author

2024-02-07 00:02 UTC #0 lore raw

PATCH 1/1 wifi: mt76: inititalize sband.band to correct value

Set phy->sband_{2,5,6}g.sband.band to the correct enum value, otherwise the ieee80211_register_hw() will fail to register the device.

Fixes https://bugzilla.kernel.org/show_bug.cgi?id=218466

Signed-off-by Bert Karwatzki <[email protected]>

---
 drivers/net/wireless/mediatek/mt76/mac80211.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/net/wireless/mediatek/mt76/mac80211.c +3 -0

index 8a3a90d1bfac..6cf08446f445 100644
@@ -363,6 +363,7 @@ mt76_init_sband_2g(struct mt76_phy *phy, struct ieee80211_rate *rates,
 		   int n_rates)
 {
 	phy->hw->wiphy->bands[NL80211_BAND_2GHZ] = &phy->sband_2g.sband;
+	phy->sband_2g.sband.band = NL80211_BAND_2GHZ;
 
 	return mt76_init_sband(phy, &phy->sband_2g, mt76_channels_2ghz,
 			       ARRAY_SIZE(mt76_channels_2ghz), rates,

@@ -374,6 +375,7 @@ mt76_init_sband_5g(struct mt76_phy *phy, struct ieee80211_rate *rates,
 		   int n_rates, bool vht)
 {
 	phy->hw->wiphy->bands[NL80211_BAND_5GHZ] = &phy->sband_5g.sband;
+	phy->sband_5g.sband.band = NL80211_BAND_5GHZ;
 
 	return mt76_init_sband(phy, &phy->sband_5g, mt76_channels_5ghz,
 			       ARRAY_SIZE(mt76_channels_5ghz), rates,

@@ -385,6 +387,7 @@ mt76_init_sband_6g(struct mt76_phy *phy, struct ieee80211_rate *rates,
 		   int n_rates)
 {
 	phy->hw->wiphy->bands[NL80211_BAND_6GHZ] = &phy->sband_6g.sband;
+	phy->sband_6g.sband.band = NL80211_BAND_6GHZ;
 
 	return mt76_init_sband(phy, &phy->sband_6g, mt76_channels_6ghz,
 			       ARRAY_SIZE(mt76_channels_6ghz), rates,

--
2.39.2

When using linux-next-20240205 my mediatek wifi device failed to start. A bisection led to commit f04d2c247e0407 as the problem:
https://bugzilla.kernel.org/show_bug.cgi?id=218466
After closer examination of the mt76 code it the problem was to to be the incomplete initialization of phy->sband_{2,5,6}g. The band field has to be set to not confuse the new mac80211 code. (the sband_2g structure was correctly initialized accidendly as NL80211_BAND_2GHZ is 0).

Bert Karwatzki

Kalle Valo

2024-02-07 05:46 UTC #1 lore raw

Quoted 4 lines · Bert Karwatzki

Set phy->sband_{2,5,6}g.sband.band to the correct enum value, otherwise the ieee80211_register_hw() will fail to register the device.
Fixes https://bugzilla.kernel.org/show_bug.cgi?id=218466

s/Fixes/Closes/

Signature · 3 lines

https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Bert Karwatzki

author

2024-02-07 07:23 UTC #2 lore raw

PATCH 1/1 wifi: mt76: inititalize sband.band to correct value

Set phy->sband_{2,5,6}g.sband.band to the correct enum value, otherwise the ieee80211_register_hw() will fail to register the device.

Closes https://bugzilla.kernel.org/show_bug.cgi?id=218466

Signed-off-by Bert Karwatzki <[email protected]>

---
 drivers/net/wireless/mediatek/mt76/mac80211.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/net/wireless/mediatek/mt76/mac80211.c +3 -0

index 8a3a90d1bfac..6cf08446f445 100644
@@ -363,6 +363,7 @@ mt76_init_sband_2g(struct mt76_phy *phy, struct ieee80211_rate *rates,
 		   int n_rates)
 {
 	phy->hw->wiphy->bands[NL80211_BAND_2GHZ] = &phy->sband_2g.sband;
+	phy->sband_2g.sband.band = NL80211_BAND_2GHZ;
 
 	return mt76_init_sband(phy, &phy->sband_2g, mt76_channels_2ghz,
 			       ARRAY_SIZE(mt76_channels_2ghz), rates,

@@ -374,6 +375,7 @@ mt76_init_sband_5g(struct mt76_phy *phy, struct ieee80211_rate *rates,
 		   int n_rates, bool vht)
 {
 	phy->hw->wiphy->bands[NL80211_BAND_5GHZ] = &phy->sband_5g.sband;
+	phy->sband_5g.sband.band = NL80211_BAND_5GHZ;
 
 	return mt76_init_sband(phy, &phy->sband_5g, mt76_channels_5ghz,
 			       ARRAY_SIZE(mt76_channels_5ghz), rates,

@@ -385,6 +387,7 @@ mt76_init_sband_6g(struct mt76_phy *phy, struct ieee80211_rate *rates,
 		   int n_rates)
 {
 	phy->hw->wiphy->bands[NL80211_BAND_6GHZ] = &phy->sband_6g.sband;
+	phy->sband_6g.sband.band = NL80211_BAND_6GHZ;
 
 	return mt76_init_sband(phy, &phy->sband_6g, mt76_channels_6ghz,
 			       ARRAY_SIZE(mt76_channels_6ghz), rates,

--
2.39.2

It's "Fixes" for commits and "Closes" for bugs, ain't it?

Bert Karwatzki

Johannes Berg

maintainer

2024-02-07 10:52 UTC #3 lore raw

Quoted 2 lines · Bert Karwatzki

Set phy->sband_{2,5,6}g.sband.band to the correct enum value, otherwise the ieee80211_register_hw() will fail to register the device.

So turns out that technically, it wasn't needed before because cfg80211 *will* set sband->band, but *mac80211* now requires it to have been set before ...

We could do a trivial fix in mac80211 as well:

net/mac80211/main.c +1 -1

@@ -1124,7 +1124,7 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 			supp_he = supp_he || iftd->he_cap.has_he;
 			supp_eht = supp_eht || iftd->eht_cap.has_eht;
 
-			if (sband->band == NL80211_BAND_2GHZ)
+			if (band == NL80211_BAND_2GHZ)
 				he_40_mhz_cap =
 					IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_IN_2G;
 			else

but seems like no other driver even likely needed this, and it's probably less reliable in the long term?

Or do both?

johannes