[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <3e38b927-ea3a-409c-93b6-32f86fe68110@vivo.com>
Date: Wed, 27 Aug 2025 10:24:57 +0800
From: Qianfeng Rong <rongqianfeng@...o.com>
To: Ping-Ke Shih <pkshih@...ltek.com>,
"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] wifi: rtw89: use int type to store negative error codes
在 2025/8/27 8:42, Ping-Ke Shih 写道:
> Qianfeng Rong <rongqianfeng@...o.com> wrote:
>> The 'ret' variable stores returns from other functions, which return
>> either zero on success or negative error codes on failure. Storing
>> error codes in u32 (an unsigned type) causes no runtime issues but is
>> stylistically inconsistent and very ugly. Change 'ret' from u32 to
>> int - this has no runtime impact.
>>
>> Signed-off-by: Qianfeng Rong <rongqianfeng@...o.com>
>> ---
>> drivers/net/wireless/realtek/rtw89/fw.c | 7 ++++---
>> drivers/net/wireless/realtek/rtw89/mac.c | 16 ++++++++--------
>> drivers/net/wireless/realtek/rtw89/pci.c | 4 ++--
>> 3 files changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
>> index 16e59a4a486e..01d53f7c142d 100644
>> --- a/drivers/net/wireless/realtek/rtw89/fw.c
>> +++ b/drivers/net/wireless/realtek/rtw89/fw.c
>> @@ -1537,7 +1537,7 @@ static int __rtw89_fw_download_hdr(struct rtw89_dev *rtwdev,
>> struct rtw89_fw_hdr *fw_hdr;
>> struct sk_buff *skb;
>> u32 truncated;
>> - u32 ret = 0;
>> + int ret = 0;
> Initializer is not necessary, by the way.
>
>> skb = rtw89_fw_h2c_alloc_skb_with_hdr(rtwdev, len);
>> if (!skb) {
>> @@ -6826,7 +6826,8 @@ static int rtw89_fw_read_c2h_reg(struct rtw89_dev *rtwdev,
>> const struct rtw89_chip_info *chip = rtwdev->chip;
>> struct rtw89_fw_info *fw_info = &rtwdev->fw;
>> const u32 *c2h_reg = chip->c2h_regs;
>> - u32 ret, timeout;
>> + u32 timeout;
>> + int ret;
>> u8 i, val;
> Keep it in reverse X'mas tree order.
>
>> info->id = RTW89_FWCMD_C2HREG_FUNC_NULL;
>> @@ -6865,7 +6866,7 @@ int rtw89_fw_msg_reg(struct rtw89_dev *rtwdev,
>> struct rtw89_mac_h2c_info *h2c_info,
>> struct rtw89_mac_c2h_info *c2h_info)
>> {
>> - u32 ret;
>> + int ret;
>>
>> if (h2c_info && h2c_info->id != RTW89_FWCMD_H2CREG_FUNC_GET_FEATURE)
>> lockdep_assert_wiphy(rtwdev->hw->wiphy);
> [...]
>
>> @@ -3105,7 +3105,7 @@ int rtw89_mac_setup_phycap(struct rtw89_dev *rtwdev)
>> static int rtw89_hw_sch_tx_en_h2c(struct rtw89_dev *rtwdev, u8 band,
>> u16 tx_en_u16, u16 mask_u16)
>> {
>> - u32 ret;
>> + int ret;
> Please move below to be reverse X'mas tree order.
>
>> struct rtw89_mac_c2h_info c2h_info = {0};
>> struct rtw89_mac_h2c_info h2c_info = {0};
>> struct rtw89_h2creg_sch_tx_en *sch_tx_en = &h2c_info.u.sch_tx_en;
> (move here)
>
> [...]
>
>> @@ -4158,7 +4158,7 @@ static int rtw89_pci_lv1rst_stop_dma_ax(struct rtw89_dev *rtwdev)
>>
>> static int rtw89_pci_lv1rst_start_dma_ax(struct rtw89_dev *rtwdev)
>> {
>> - u32 ret;
>> + int ret;
>>
>> if (rtwdev->chip->chip_id == RTL8852C)
>> return 0;
>
> The last statement of this function is 'return ret;', but actually it can
> just be 'return 0;'. Please change it by the way.
Thanks for taking the time to reply. All your suggestions will be
modified in v2.
Best regards, Qianfeng
Powered by blists - more mailing lists