RE: [PATCH] wifi: rtw89: use int type to store negative error codes

[Ping-Ke Shih <pkshih@realtek.com>]

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