Received-SPF: pass (google.com: domain of linux-wireless+bounces-47-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <cb551005-eff0-1391-92a0-d956b3d2b930@nfschina.com>
Date: Fri, 24 Nov 2023 18:06:11 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH v2 2/2] wifi: rtlwifi: rtl8821ae: phy: fix an undefined
 bitwise shift behavior
Content-Language: en-US
To: Ping-Ke Shih <pkshih@realtek.com>,
 "dan.carpenter@linaro.org" <dan.carpenter@linaro.org>,
 "kvalo@kernel.org" <kvalo@kernel.org>, "nathan@kernel.org"
 <nathan@kernel.org>, "ndesaulniers@google.com" <ndesaulniers@google.com>,
 "trix@redhat.com" <trix@redhat.com>
Cc: "lizetao1@huawei.com" <lizetao1@huawei.com>,
 "linville@tuxdriver.com" <linville@tuxdriver.com>,
 "Larry.Finger@lwfinger.net" <Larry.Finger@lwfinger.net>,
 "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
 "llvm@lists.linux.dev" <llvm@lists.linux.dev>,
 "kernel-janitors@vger.kernel.org" <kernel-janitors@vger.kernel.org>
From: Su Hui <suhui@nfschina.com>
In-Reply-To: <167fbc7a77db447d90f696666f6f0a9b@realtek.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 2023/11/24 16:51, Ping-Ke Shih wrote:
> 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.

Hi,  Ping-Ke

Replace _rtl8821ae_phy_calculate_bit_shift() by __ffs(bitmask) is better,
but I'm not sure what callers should do when callers check bitmask is 0 before calling.
Maybe this check is useless?

I can send a v3 patch if using  __ffs(bitmask) and no check for bitmask is fine.
Or could you send this patch if you have a better idea?
Thanks for your suggestion!

Su Hui