RE: [PATCH v2 2/2] wifi: rtlwifi: rtl8821ae: phy: fix an undefined bitwise shift behavior

[Ping-Ke Shih]

> -----Original Message-----
> From: Su Hui <suhui@xxxxxxxxxxxx>
> Sent: Friday, November 24, 2023 10:04 AM
> To: dan.carpenter@xxxxxxxxxx; Ping-Ke Shih <pkshih@xxxxxxxxxxx>; kvalo@xxxxxxxxxx; nathan@xxxxxxxxxx;
> ndesaulniers@xxxxxxxxxx; trix@xxxxxxxxxx
> Cc: Su Hui <suhui@xxxxxxxxxxxx>; lizetao1@xxxxxxxxxx; linville@xxxxxxxxxxxxx; Larry.Finger@xxxxxxxxxxxx;
> linux-wireless@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; llvm@xxxxxxxxxxxxxxx;
> kernel-janitors@xxxxxxxxxxxxxxx
> Subject: [PATCH v2 2/2] wifi: rtlwifi: rtl8821ae: phy: fix an undefined bitwise shift behavior
> 

[...]

> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
> b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
> index 6df270e29e66..52ab1b0761c0 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
> @@ -31,7 +31,12 @@ static u32 _rtl8821ae_phy_calculate_bit_shift(u32 bitmask)
>  {
>         u32 i = ffs(bitmask);
> 
> -       return i ? i - 1 : 32;
> +       if (!i) {
> +               WARN_ON_ONCE(1);
> +               return 0;
> +       }
> +
> +       return i - 1;
>  }

Personally, I prefer to use __ffs(), because in normal case no need additional '-1',
and abnormal cases should not happen.