Skip to contentSkip to conversation

rtw89: -Wenum-compare-conditional warnings

PATCH In review Arnd Bergmann 3 participants 3 messages Oct 18, 2024 – Oct 25, 2024 Realtek wireless driver (rtw89)

lore
Share & tools
lore

Arnd Bergmann

author
#0 lore raw

PATCH 1/1 rtw89: -Wenum-compare-conditional warnings

From: Arnd Bergmann <[email protected]>

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 <[email protected]>
---
 drivers/net/wireless/realtek/rtw89/core.c | 48 +++++++++++++++++------
 1 file changed, 37 insertions(+), 11 deletions(-)
drivers/net/wireless/realtek/rtw89/core.c +37 -11
index bba5bde95bb4..62e873fa1659 100644
@@ -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)
Signature · 1 line
2.39.5
Reviewed-by on v1 1/1
#1 lore raw
Quoted 26 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

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;
      |                                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~
Quoted 5 lines

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 <[email protected]>

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 <[email protected]>
Quoted 81 lines
---
 drivers/net/wireless/realtek/rtw89/core.c | 48 +++++++++++++++++------
 1 file changed, 37 insertions(+), 11 deletions(-)
drivers/net/wireless/realtek/rtw89/core.c +37 -11
index bba5bde95bb4..62e873fa1659 100644
@@ -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)
Signature · 1 line
2.39.5
maintainer
#2 lore raw
In reply to Arnd Bergmann
Quoted 26 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

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;
      |                                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~
Quoted 6 lines

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 <[email protected]>
Reviewed-by Nathan Chancellor <[email protected]>

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