| Message ID | 20241018152311.4023979-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Ping-Ke Shih |
| Headers | show |
| Series | rtw89: -Wenum-compare-conditional warnings | expand |
On Fri, Oct 18, 2024 at 03:23:07PM +0000, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > This is one of three drivers that trigger -Wenum-compare-conditional warnings > with clang: > > drivers/net/wireless/realtek/rtw89/core.c:1806:14: error: conditional expression between different enumeration types ('enum nl80211_eht_gi' and 'enum nl80211_he_gi') [-Werror,-Wenum-compare-conditional] > 1806 | return eht ? NL80211_RATE_INFO_EHT_GI_0_8 : > | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 1807 | NL80211_RATE_INFO_HE_GI_0_8; > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/net/wireless/realtek/rtw89/core.c:1810:14: error: conditional expression between different enumeration types ('enum nl80211_eht_gi' and 'enum nl80211_he_gi') [-Werror,-Wenum-compare-conditional] > 1810 | return eht ? NL80211_RATE_INFO_EHT_GI_1_6 : > | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 1811 | NL80211_RATE_INFO_HE_GI_1_6; > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/net/wireless/realtek/rtw89/core.c:1813:14: error: conditional expression between different enumeration types ('enum nl80211_eht_gi' and 'enum nl80211_he_gi') [-Werror,-Wenum-compare-conditional] > 1813 | return eht ? NL80211_RATE_INFO_EHT_GI_3_2 : > | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 1814 | NL80211_RATE_INFO_HE_GI_3_2; > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/net/wireless/realtek/rtw89/core.c:1818:15: error: conditional expression between different enumeration types ('enum nl80211_eht_gi' and 'enum nl80211_he_gi') [-Werror,-Wenum-compare-conditional] > 1818 | return eht ? NL80211_RATE_INFO_EHT_GI_3_2 : > | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 1819 | NL80211_RATE_INFO_HE_GI_3_2; > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > In this case, all four warnings can be easily avoided by splitting the > function into two separate ones, in a way that helps readability as well, > at the expense of a few extra source lines. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> I agree this is slightly less compact but I think it is more obvious, especially in the face of the warning. Reviewed-by: Nathan Chancellor <nathan@kernel.org> > --- > drivers/net/wireless/realtek/rtw89/core.c | 48 +++++++++++++++++------ > 1 file changed, 37 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c > index bba5bde95bb4..62e873fa1659 100644 > --- a/drivers/net/wireless/realtek/rtw89/core.c > +++ b/drivers/net/wireless/realtek/rtw89/core.c > @@ -1858,32 +1858,58 @@ static void rtw89_core_rx_process_phy_sts(struct rtw89_dev *rtwdev, > phy_ppdu); > } > > -static u8 rtw89_rxdesc_to_nl_he_eht_gi(struct rtw89_dev *rtwdev, > - u8 desc_info_gi, > - bool rx_status, bool eht) > +static u8 rtw89_rxdesc_to_nl_he_gi(struct rtw89_dev *rtwdev, > + u8 desc_info_gi, > + bool rx_status) > +{ > + switch (desc_info_gi) { > + case RTW89_GILTF_SGI_4XHE08: > + case RTW89_GILTF_2XHE08: > + case RTW89_GILTF_1XHE08: > + return NL80211_RATE_INFO_HE_GI_0_8; > + case RTW89_GILTF_2XHE16: > + case RTW89_GILTF_1XHE16: > + return NL80211_RATE_INFO_HE_GI_1_6; > + case RTW89_GILTF_LGI_4XHE32: > + return NL80211_RATE_INFO_HE_GI_3_2; > + default: > + rtw89_warn(rtwdev, "invalid gi_ltf=%d", desc_info_gi); > + if (rx_status) > + return NL80211_RATE_INFO_HE_GI_3_2; > + return U8_MAX; > + } > +} > + > +static u8 rtw89_rxdesc_to_nl_eht_gi(struct rtw89_dev *rtwdev, > + u8 desc_info_gi, > + bool rx_status) > { > switch (desc_info_gi) { > case RTW89_GILTF_SGI_4XHE08: > case RTW89_GILTF_2XHE08: > case RTW89_GILTF_1XHE08: > - return eht ? NL80211_RATE_INFO_EHT_GI_0_8 : > - NL80211_RATE_INFO_HE_GI_0_8; > + return NL80211_RATE_INFO_EHT_GI_0_8; > case RTW89_GILTF_2XHE16: > case RTW89_GILTF_1XHE16: > - return eht ? NL80211_RATE_INFO_EHT_GI_1_6 : > - NL80211_RATE_INFO_HE_GI_1_6; > + return NL80211_RATE_INFO_EHT_GI_1_6; > case RTW89_GILTF_LGI_4XHE32: > - return eht ? NL80211_RATE_INFO_EHT_GI_3_2 : > - NL80211_RATE_INFO_HE_GI_3_2; > + return NL80211_RATE_INFO_EHT_GI_3_2; > default: > rtw89_warn(rtwdev, "invalid gi_ltf=%d", desc_info_gi); > if (rx_status) > - return eht ? NL80211_RATE_INFO_EHT_GI_3_2 : > - NL80211_RATE_INFO_HE_GI_3_2; > + return NL80211_RATE_INFO_EHT_GI_3_2; > return U8_MAX; > } > } > > +static u8 rtw89_rxdesc_to_nl_he_eht_gi(struct rtw89_dev *rtwdev, > + u8 desc_info_gi, > + bool rx_status, bool eht) > +{ > + return eht ? rtw89_rxdesc_to_nl_eht_gi(rtwdev, desc_info_gi, rx_status) : > + rtw89_rxdesc_to_nl_he_gi(rtwdev, desc_info_gi, rx_status); > +} > + > static > bool rtw89_check_rx_statu_gi_match(struct ieee80211_rx_status *status, u8 gi_ltf, > bool eht) > -- > 2.39.5 >
Arnd Bergmann <arnd@kernel.org> wrote: > From: Arnd Bergmann <arnd@arndb.de> > > This is one of three drivers that trigger -Wenum-compare-conditional warnings > with clang: > > drivers/net/wireless/realtek/rtw89/core.c:1806:14: error: conditional expression between different enumeration types ('enum nl80211_eht_gi' and 'enum nl80211_he_gi') [-Werror,-Wenum-compare-conditional] > 1806 | return eht ? NL80211_RATE_INFO_EHT_GI_0_8 : > | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 1807 | NL80211_RATE_INFO_HE_GI_0_8; > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/net/wireless/realtek/rtw89/core.c:1810:14: error: conditional expression between different enumeration types ('enum nl80211_eht_gi' and 'enum nl80211_he_gi') [-Werror,-Wenum-compare-conditional] > 1810 | return eht ? NL80211_RATE_INFO_EHT_GI_1_6 : > | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 1811 | NL80211_RATE_INFO_HE_GI_1_6; > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/net/wireless/realtek/rtw89/core.c:1813:14: error: conditional expression between different enumeration types ('enum nl80211_eht_gi' and 'enum nl80211_he_gi') [-Werror,-Wenum-compare-conditional] > 1813 | return eht ? NL80211_RATE_INFO_EHT_GI_3_2 : > | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 1814 | NL80211_RATE_INFO_HE_GI_3_2; > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/net/wireless/realtek/rtw89/core.c:1818:15: error: conditional expression between different enumeration types ('enum nl80211_eht_gi' and 'enum nl80211_he_gi') [-Werror,-Wenum-compare-conditional] > 1818 | return eht ? NL80211_RATE_INFO_EHT_GI_3_2 : > | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 1819 | NL80211_RATE_INFO_HE_GI_3_2; > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > In this case, all four warnings can be easily avoided by splitting the > function into two separate ones, in a way that helps readability as well, > at the expense of a few extra source lines. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Reviewed-by: Nathan Chancellor <nathan@kernel.org> 1 patch(es) applied to rtw-next branch of rtw.git, thanks. c67d7c7f4151 wifi: rtw89: fix -Wenum-compare-conditional warnings --- https://github.com/pkshih/rtw.git
diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c index bba5bde95bb4..62e873fa1659 100644 --- a/drivers/net/wireless/realtek/rtw89/core.c +++ b/drivers/net/wireless/realtek/rtw89/core.c @@ -1858,32 +1858,58 @@ static void rtw89_core_rx_process_phy_sts(struct rtw89_dev *rtwdev, phy_ppdu); } -static u8 rtw89_rxdesc_to_nl_he_eht_gi(struct rtw89_dev *rtwdev, - u8 desc_info_gi, - bool rx_status, bool eht) +static u8 rtw89_rxdesc_to_nl_he_gi(struct rtw89_dev *rtwdev, + u8 desc_info_gi, + bool rx_status) +{ + switch (desc_info_gi) { + case RTW89_GILTF_SGI_4XHE08: + case RTW89_GILTF_2XHE08: + case RTW89_GILTF_1XHE08: + return NL80211_RATE_INFO_HE_GI_0_8; + case RTW89_GILTF_2XHE16: + case RTW89_GILTF_1XHE16: + return NL80211_RATE_INFO_HE_GI_1_6; + case RTW89_GILTF_LGI_4XHE32: + return NL80211_RATE_INFO_HE_GI_3_2; + default: + rtw89_warn(rtwdev, "invalid gi_ltf=%d", desc_info_gi); + if (rx_status) + return NL80211_RATE_INFO_HE_GI_3_2; + return U8_MAX; + } +} + +static u8 rtw89_rxdesc_to_nl_eht_gi(struct rtw89_dev *rtwdev, + u8 desc_info_gi, + bool rx_status) { switch (desc_info_gi) { case RTW89_GILTF_SGI_4XHE08: case RTW89_GILTF_2XHE08: case RTW89_GILTF_1XHE08: - return eht ? NL80211_RATE_INFO_EHT_GI_0_8 : - NL80211_RATE_INFO_HE_GI_0_8; + return NL80211_RATE_INFO_EHT_GI_0_8; case RTW89_GILTF_2XHE16: case RTW89_GILTF_1XHE16: - return eht ? NL80211_RATE_INFO_EHT_GI_1_6 : - NL80211_RATE_INFO_HE_GI_1_6; + return NL80211_RATE_INFO_EHT_GI_1_6; case RTW89_GILTF_LGI_4XHE32: - return eht ? NL80211_RATE_INFO_EHT_GI_3_2 : - NL80211_RATE_INFO_HE_GI_3_2; + return NL80211_RATE_INFO_EHT_GI_3_2; default: rtw89_warn(rtwdev, "invalid gi_ltf=%d", desc_info_gi); if (rx_status) - return eht ? NL80211_RATE_INFO_EHT_GI_3_2 : - NL80211_RATE_INFO_HE_GI_3_2; + return NL80211_RATE_INFO_EHT_GI_3_2; return U8_MAX; } } +static u8 rtw89_rxdesc_to_nl_he_eht_gi(struct rtw89_dev *rtwdev, + u8 desc_info_gi, + bool rx_status, bool eht) +{ + return eht ? rtw89_rxdesc_to_nl_eht_gi(rtwdev, desc_info_gi, rx_status) : + rtw89_rxdesc_to_nl_he_gi(rtwdev, desc_info_gi, rx_status); +} + static bool rtw89_check_rx_statu_gi_match(struct ieee80211_rx_status *status, u8 gi_ltf, bool eht)