linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] wifi: rtw89: use int type to store negative error codes
@ 2025-08-26 13:29 Qianfeng Rong
  2025-08-27  0:42 ` Ping-Ke Shih
  0 siblings, 1 reply; 3+ messages in thread
From: Qianfeng Rong @ 2025-08-26 13:29 UTC (permalink / raw)
  To: Ping-Ke Shih, linux-wireless, linux-kernel; +Cc: Qianfeng Rong

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;
 
 	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;
 
 	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);
diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c
index 5a5da9d9c0c5..9e4d666f15f7 100644
--- a/drivers/net/wireless/realtek/rtw89/mac.c
+++ b/drivers/net/wireless/realtek/rtw89/mac.c
@@ -177,7 +177,7 @@ int rtw89_mac_dle_dfi_qempty_cfg(struct rtw89_dev *rtwdev,
 				 struct rtw89_mac_dle_dfi_qempty *qempty)
 {
 	struct rtw89_mac_dle_dfi_ctrl ctrl;
-	u32 ret;
+	int ret;
 
 	ctrl.type = qempty->dle_type;
 	ctrl.target = DLE_DFI_TYPE_QEMPTY;
@@ -985,7 +985,7 @@ static int hfc_upd_ch_info(struct rtw89_dev *rtwdev, u8 ch)
 	struct rtw89_hfc_ch_info *info = param->ch_info;
 	const struct rtw89_hfc_ch_cfg *cfg = param->ch_cfg;
 	u32 val;
-	u32 ret;
+	int ret;
 
 	ret = rtw89_mac_check_mac_en(rtwdev, RTW89_MAC_0, RTW89_DMAC_SEL);
 	if (ret)
@@ -1177,7 +1177,7 @@ int rtw89_mac_hfc_init(struct rtw89_dev *rtwdev, bool reset, bool en, bool h2c_e
 	const struct rtw89_chip_info *chip = rtwdev->chip;
 	u32 dma_ch_mask = chip->dma_ch_mask;
 	u8 ch;
-	u32 ret = 0;
+	int ret = 0;
 
 	if (reset)
 		ret = hfc_reset_param(rtwdev);
@@ -2413,7 +2413,7 @@ static int addr_cam_init_ax(struct rtw89_dev *rtwdev, u8 mac_idx)
 
 static int scheduler_init_ax(struct rtw89_dev *rtwdev, u8 mac_idx)
 {
-	u32 ret;
+	int ret;
 	u32 reg;
 	u32 val;
 
@@ -2954,7 +2954,7 @@ static int rtw89_mac_read_phycap(struct rtw89_dev *rtwdev,
 	struct rtw89_mac_h2c_info h2c_info = {};
 	enum rtw89_mac_c2h_type c2h_type;
 	u8 content_len;
-	u32 ret;
+	int ret;
 
 	if (chip->chip_gen == RTW89_CHIP_AX)
 		content_len = 0;
@@ -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;
 	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;
@@ -6720,7 +6720,7 @@ int rtw89_mac_set_hw_muedca_ctrl(struct rtw89_dev *rtwdev,
 	u8 mac_idx = rtwvif_link->mac_idx;
 	u16 set = mac->muedca_ctrl.mask;
 	u32 reg;
-	u32 ret;
+	int ret;
 
 	ret = rtw89_mac_check_mac_en(rtwdev, mac_idx, RTW89_CMAC_SEL);
 	if (ret)
@@ -6862,7 +6862,7 @@ int rtw89_mac_cpu_io_rx(struct rtw89_dev *rtwdev, bool wow_enable)
 {
 	struct rtw89_mac_h2c_info h2c_info = {};
 	struct rtw89_mac_c2h_info c2h_info = {};
-	u32 ret;
+	int ret;
 
 	if (RTW89_CHK_FW_FEATURE(NO_WOW_CPU_IO_RX, &rtwdev->fw))
 		return 0;
diff --git a/drivers/net/wireless/realtek/rtw89/pci.c b/drivers/net/wireless/realtek/rtw89/pci.c
index a669f2f843aa..5f58a954ccc7 100644
--- a/drivers/net/wireless/realtek/rtw89/pci.c
+++ b/drivers/net/wireless/realtek/rtw89/pci.c
@@ -2725,7 +2725,7 @@ static int rtw89_pci_poll_rxdma_ch_idle_ax(struct rtw89_dev *rtwdev)
 
 static int rtw89_pci_poll_dma_all_idle(struct rtw89_dev *rtwdev)
 {
-	u32 ret;
+	int ret;
 
 	ret = rtw89_pci_poll_txdma_ch_idle_ax(rtwdev);
 	if (ret) {
@@ -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;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* RE: [PATCH] wifi: rtw89: use int type to store negative error codes
  2025-08-26 13:29 [PATCH] wifi: rtw89: use int type to store negative error codes Qianfeng Rong
@ 2025-08-27  0:42 ` Ping-Ke Shih
  2025-08-27  2:24   ` Qianfeng Rong
  0 siblings, 1 reply; 3+ messages in thread
From: Ping-Ke Shih @ 2025-08-27  0:42 UTC (permalink / raw)
  To: Qianfeng Rong, linux-wireless@vger.kernel.org,
	linux-kernel@vger.kernel.org

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. 



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] wifi: rtw89: use int type to store negative error codes
  2025-08-27  0:42 ` Ping-Ke Shih
@ 2025-08-27  2:24   ` Qianfeng Rong
  0 siblings, 0 replies; 3+ messages in thread
From: Qianfeng Rong @ 2025-08-27  2:24 UTC (permalink / raw)
  To: Ping-Ke Shih, linux-wireless@vger.kernel.org,
	linux-kernel@vger.kernel.org


在 2025/8/27 8:42, Ping-Ke Shih 写道:
> 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.

Thanks for taking the time to reply. All your suggestions will be 
modified in v2.

Best regards, Qianfeng


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-08-27  2:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-26 13:29 [PATCH] wifi: rtw89: use int type to store negative error codes Qianfeng Rong
2025-08-27  0:42 ` Ping-Ke Shih
2025-08-27  2:24   ` Qianfeng Rong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).