Search Linux Wireless

Re: [PATCH] rtw89: -Wenum-compare-conditional warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 18, 2024 at 03:23:07PM +0000, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@xxxxxxxx>
> 
> 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@xxxxxxxx>

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@xxxxxxxxxx>

> ---
>  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
> 




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux