Qianfeng Rong <rongqianfeng@xxxxxxxx> 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@xxxxxxxx>
> ---
> 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.
