| Message ID | 20250328055058.1648755-10-shayne.chen@mediatek.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Felix Fietkau |
| Headers | show |
| Series | Add MT7990 support | expand |
Shayne Chen <shayne.chen@mediatek.com> wrote: [...] > + > +bool mt7996_eeprom_has_background_radar(struct mt7996_dev *dev) > +{ > + switch (mt76_chip(&dev->mt76)) { > + case MT7996_DEVICE_ID: > + if (dev->var.type == MT7996_VAR_TYPE_233) > + return false; > + break; > + case MT7992_DEVICE_ID: > + if (dev->var.type == MT7992_VAR_TYPE_23) > + return false; > + break; > + case MT7990_DEVICE_ID: { > + u8 path, rx_path, nss, *eeprom = dev->mt76.eeprom.data; > + > + mt7996_eeprom_parse_stream(eeprom, MT_BAND1, &path, &rx_path, &nss); > + /* Disable background radar capability in 3T3R */ > + if (path == 3 || rx_path == 3) > + return false; > + break; > + } The indentation of close brace looks weird. Since -Wdeclaration-after-statement is dropped, I think compilers will not warn without the braces. But note that it is still not recommended to put declarations in the middle. > + default: > + return false; > + } > + > + return true; > +}
On Mon, 2025-03-31 at 05:55 +0000, Ping-Ke Shih wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > Shayne Chen <shayne.chen@mediatek.com> wrote: > > [...] > > > + > > +bool mt7996_eeprom_has_background_radar(struct mt7996_dev *dev) > > +{ > > + switch (mt76_chip(&dev->mt76)) { > > + case MT7996_DEVICE_ID: > > + if (dev->var.type == MT7996_VAR_TYPE_233) > > + return false; > > + break; > > + case MT7992_DEVICE_ID: > > + if (dev->var.type == MT7992_VAR_TYPE_23) > > + return false; > > + break; > > + case MT7990_DEVICE_ID: { > > + u8 path, rx_path, nss, *eeprom = dev- > > >mt76.eeprom.data; > > + > > + mt7996_eeprom_parse_stream(eeprom, MT_BAND1, &path, > > &rx_path, &nss); > > + /* Disable background radar capability in 3T3R */ > > + if (path == 3 || rx_path == 3) > > + return false; > > + break; > > + } > > The indentation of close brace looks weird. Will fix it, thanks. > > Since -Wdeclaration-after-statement is dropped, I think compilers > will not > warn without the braces. But note that it is still not recommended to > put declarations in the middle. > Since those variables are currently only used by mt7990 case, I think they can be putting there for the moment. > > + default: > > + return false; > > + } > > + > > + return true; > > +} > > >
Shayne Chen <shayne.chen@mediatek.com> wrote: > On Mon, 2025-03-31 at 05:55 +0000, Ping-Ke Shih wrote: > > > > > > Shayne Chen <shayne.chen@mediatek.com> wrote: > > > > [...] > > > > > + > > > +bool mt7996_eeprom_has_background_radar(struct mt7996_dev *dev) > > > +{ > > > + switch (mt76_chip(&dev->mt76)) { > > > + case MT7996_DEVICE_ID: > > > + if (dev->var.type == MT7996_VAR_TYPE_233) > > > + return false; > > > + break; > > > + case MT7992_DEVICE_ID: > > > + if (dev->var.type == MT7992_VAR_TYPE_23) > > > + return false; > > > + break; > > > + case MT7990_DEVICE_ID: { > > > + u8 path, rx_path, nss, *eeprom = dev- > > > >mt76.eeprom.data; > > > + > > > + mt7996_eeprom_parse_stream(eeprom, MT_BAND1, &path, > > > &rx_path, &nss); > > > + /* Disable background radar capability in 3T3R */ > > > + if (path == 3 || rx_path == 3) > > > + return false; > > > + break; > > > + } > > > > The indentation of close brace looks weird. > > Will fix it, thanks. > > > > Since -Wdeclaration-after-statement is dropped, I think compilers > > will not > > warn without the braces. But note that it is still not recommended to > > put declarations in the middle. > > > Since those variables are currently only used by mt7990 case, I think > they can be putting there for the moment. That looks not very natural though... In fact, the declarations either in beginning of this function or at the mt7990 case, the frame size (stack) are the same. > > > > + default: > > > + return false; > > > + } > > > + > > > + return true; > > > +} > > > > > >
On 4/1/25 18:30, Ping-Ke Shih wrote: > Shayne Chen <shayne.chen@mediatek.com> wrote: > >> On Mon, 2025-03-31 at 05:55 +0000, Ping-Ke Shih wrote: >>> >>> >>> Shayne Chen <shayne.chen@mediatek.com> wrote: >>> >>> [...] >>> >>>> + >>>> +bool mt7996_eeprom_has_background_radar(struct mt7996_dev *dev) >>>> +{ >>>> + switch (mt76_chip(&dev->mt76)) { >>>> + case MT7996_DEVICE_ID: >>>> + if (dev->var.type == MT7996_VAR_TYPE_233) >>>> + return false; >>>> + break; >>>> + case MT7992_DEVICE_ID: >>>> + if (dev->var.type == MT7992_VAR_TYPE_23) >>>> + return false; >>>> + break; >>>> + case MT7990_DEVICE_ID: { >>>> + u8 path, rx_path, nss, *eeprom = dev- >>>>> mt76.eeprom.data; >>>> + >>>> + mt7996_eeprom_parse_stream(eeprom, MT_BAND1, &path, >>>> &rx_path, &nss); >>>> + /* Disable background radar capability in 3T3R */ >>>> + if (path == 3 || rx_path == 3) >>>> + return false; >>>> + break; >>>> + } >>> >>> The indentation of close brace looks weird. >> >> Will fix it, thanks. >>> >>> Since -Wdeclaration-after-statement is dropped, I think compilers >>> will not >>> warn without the braces. But note that it is still not recommended to >>> put declarations in the middle. >>> >> Since those variables are currently only used by mt7990 case, I think >> they can be putting there for the moment. > > That looks not very natural though... > > In fact, the declarations either in beginning of this function or at the > mt7990 case, the frame size (stack) are the same. The open parens make it the start of a new code block, so even strict old c compilers should handle this just fine, and I personally like variables kept to a tight local scope if possible. Unless there is a technical reason to change the code, then there are probably more important things to worry about. Thanks, Ben
diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/eeprom.c b/drivers/net/wireless/mediatek/mt76/mt7996/eeprom.c index 6f3eb053ef02..ca67a2c4b6a7 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7996/eeprom.c +++ b/drivers/net/wireless/mediatek/mt76/mt7996/eeprom.c @@ -376,3 +376,30 @@ s8 mt7996_eeprom_get_power_delta(struct mt7996_dev *dev, int band) return val & MT_EE_RATE_DELTA_SIGN ? delta : -delta; } + +bool mt7996_eeprom_has_background_radar(struct mt7996_dev *dev) +{ + switch (mt76_chip(&dev->mt76)) { + case MT7996_DEVICE_ID: + if (dev->var.type == MT7996_VAR_TYPE_233) + return false; + break; + case MT7992_DEVICE_ID: + if (dev->var.type == MT7992_VAR_TYPE_23) + return false; + break; + case MT7990_DEVICE_ID: { + u8 path, rx_path, nss, *eeprom = dev->mt76.eeprom.data; + + mt7996_eeprom_parse_stream(eeprom, MT_BAND1, &path, &rx_path, &nss); + /* Disable background radar capability in 3T3R */ + if (path == 3 || rx_path == 3) + return false; + break; + } + default: + return false; + } + + return true; +} diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/init.c b/drivers/net/wireless/mediatek/mt76/mt7996/init.c index 9192fbdbc7da..3bbf6041c96d 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7996/init.c +++ b/drivers/net/wireless/mediatek/mt76/mt7996/init.c @@ -475,7 +475,7 @@ mt7996_init_wiphy(struct ieee80211_hw *hw, struct mtk_wed_device *wed) wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_CAN_REPLACE_PTK0); wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_MU_MIMO_AIR_SNIFFER); - if (mt7996_has_background_radar(dev) && + if (mt7996_eeprom_has_background_radar(dev) && (!mdev->dev->of_node || !of_property_read_bool(mdev->dev->of_node, "mediatek,disable-radar-background"))) diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h index 92b01ed82e7e..a45cd3ff61a0 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h @@ -488,25 +488,6 @@ mt7996_band_valid(struct mt7996_dev *dev, u8 band) return band <= MT_BAND2; } -static inline bool -mt7996_has_background_radar(struct mt7996_dev *dev) -{ - switch (mt76_chip(&dev->mt76)) { - case MT7996_DEVICE_ID: - if (dev->var.type == MT7996_VAR_TYPE_233) - return false; - break; - case MT7992_DEVICE_ID: - if (dev->var.type == MT7992_VAR_TYPE_23) - return false; - break; - default: - return false; - } - - return true; -} - static inline struct mt7996_phy * mt7996_band_phy(struct mt7996_dev *dev, enum nl80211_band band) { @@ -570,6 +551,7 @@ int mt7996_eeprom_parse_hw_cap(struct mt7996_dev *dev, struct mt7996_phy *phy); int mt7996_eeprom_get_target_power(struct mt7996_dev *dev, struct ieee80211_channel *chan); s8 mt7996_eeprom_get_power_delta(struct mt7996_dev *dev, int band); +bool mt7996_eeprom_has_background_radar(struct mt7996_dev *dev); int mt7996_dma_init(struct mt7996_dev *dev); void mt7996_dma_reset(struct mt7996_dev *dev, bool force); void mt7996_dma_prefetch(struct mt7996_dev *dev);