Re: [PATCH v2] staging: rtl8723bs: fix constant on left side of test checkpatch warnings

[Prithvi]

On Mon, Mar 23, 2026 at 06:42:49PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 23, 2026 at 09:59:01PM +0530, Prithvi Tambewagh wrote:
> > In all types of comparison/test (using ==, !=, >=, <=, >, <
> > operators) ensure that the constant lies to the right side of the test,
> > thus fixing checkpatch warnings : comparisons should place the constant
> > on the right side of the test, throughout the rtl8723bs driver.
> 
> ...
> 
> >  	do {
> >  		valid = rtw_read8(padapter, REG_HMETFR) & BIT(msgbox_num);
> > -		if (0 == valid) {
> > +		if (!valid)
> >  			read_down = true;
> > -		}
> >  	} while ((!read_down) && (retry_cnts--));
> >  
> >  	return read_down;
> 
> This entire piece can be replaced with something from iopoll.h.
> 

Yes...I checked out and found we can use read_poll_timeout_atomic():

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_cmd.c b/drivers/staging/rtl8723bs/hal/rtl8723b_cmd.c
index 25bc11ac874c..6fa694ce27ec 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723b_cmd.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723b_cmd.c
@@ -9,6 +9,7 @@
 #include <rtl8723b_hal.h>
 #include <linux/etherdevice.h>
 #include "hal_com_h2c.h"
+#include <linux/iopoll.h>
 
 #define MAX_H2C_BOX_NUMS       4
 #define MESSAGE_BOX_SIZE       4
@@ -18,18 +19,17 @@
 
 static u8 _is_fw_read_cmd_down(struct adapter *padapter, u8 msgbox_num)
 {
-       u8 read_down = false;
-       int retry_cnts = 100;
-
        u8 valid;
-
-       do {
-               valid = rtw_read8(padapter, REG_HMETFR) & BIT(msgbox_num);
-               if (!valid)
-                       read_down = true;
-       } while ((!read_down) && (retry_cnts--));
-
-       return read_down;
+       int ret;
+
+       ret = read_poll_timeout_atomic(rtw_read8,
+                         valid,
+                         !(valid & BIT(msgbox_num)),
+                         0,
+                         500,
+                         false,
+                         padapter, REG_HMETFR);
+       return !ret;
 
 }

Is this approach correct?

> ...
> 
> > -	if (WIFI_CTRL_TYPE == GetFrameType(pframe))
> > +	if (GetFrameType(pframe) == WIFI_CTRL_TYPE)
> >  		return true;
> >  	else
> >  		return false;
> 
> Just
> 
> 	return GetFrameType(pframe) == WIFI_CTRL_TYPE;

Sure...I will incorporate this in the next version of patch.

Thanks,
Prithvi