[PATCH v2 0/3] staging: rtl8723bs: macro fixes in basic_types.h

[PATCH v2 0/3] staging: rtl8723bs: macro fixes in basic_types.h

[Mashiro Chen]

This series addresses issues in the basic_types.h header of the
rtl8723bs driver.

In v2, patch 1 now removes all unused WRITEEF/READEF macros entirely
rather than fixing the latent bug, as suggested by Dan Carpenter.
The do-while cleanup patch from v1 is dropped since those macros no
longer exist.

Mashiro Chen (3):
  staging: rtl8723bs: remove unused WRITEEF/READEF byte macros
  staging: rtl8723bs: wrap complex macros with parentheses
  staging: rtl8723bs: remove redundant blank lines in basic_types.h

 .../staging/rtl8723bs/include/basic_types.h   | 38 +++----------------
 1 file changed, 6 insertions(+), 32 deletions(-)

-- 
2.53.0

[PATCH v2 1/3] staging: rtl8723bs: remove unused WRITEEF/READEF byte macros

[Mashiro Chen]

The WRITEEF4BYTE, WRITEEF2BYTE, WRITEEF1BYTE, READEF4BYTE,
READEF2BYTE and READEF1BYTE macros are never used in the driver.
Remove them entirely.

Suggested-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Mashiro Chen <mashiro.chen@mailbox.org>
---
 .../staging/rtl8723bs/include/basic_types.h   | 25 -------------------
 1 file changed, 25 deletions(-)

diff --git a/drivers/staging/rtl8723bs/include/basic_types.h b/drivers/staging/rtl8723bs/include/basic_types.h
index 8adb95f9f1e5..69f7402d8e5c 100644
--- a/drivers/staging/rtl8723bs/include/basic_types.h
+++ b/drivers/staging/rtl8723bs/include/basic_types.h
@@ -42,31 +42,6 @@
 #define EF4BYTE(_val)		\
 	(le32_to_cpu(_val))
 
-/* Read data from memory */
-#define READEF1BYTE(_ptr)	\
-	EF1BYTE(*((u8 *)(_ptr)))
-/* Read le16 data from memory and convert to host ordering */
-#define READEF2BYTE(_ptr)	\
-	EF2BYTE(*(_ptr))
-#define READEF4BYTE(_ptr)	\
-	EF4BYTE(*(_ptr))
-
-/* Write data to memory */
-#define WRITEEF1BYTE(_ptr, _val)			\
-	do {						\
-		(*((u8 *)(_ptr))) = EF1BYTE(_val);	\
-	} while (0)
-/* Write le data to memory in host ordering */
-#define WRITEEF2BYTE(_ptr, _val)			\
-	do {						\
-		(*((u16 *)(_ptr))) = EF2BYTE(_val);	\
-	} while (0)
-
-#define WRITEEF4BYTE(_ptr, _val)			\
-	do {						\
-		(*((u32 *)(_ptr))) = EF2BYTE(_val);	\
-	} while (0)
-
 /*
  * Create a bit mask
  * Examples:
-- 
2.53.0

[PATCH v2 2/3] staging: rtl8723bs: wrap complex macros with parentheses

[Mashiro Chen]

Fix "COMPLEX_MACRO" errors reported by checkpatch.pl
by wrapping macro values in parentheses to ensure
correct operator precedence.

Signed-off-by: Mashiro Chen <mashiro.chen@mailbox.org>
---
 drivers/staging/rtl8723bs/include/basic_types.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8723bs/include/basic_types.h b/drivers/staging/rtl8723bs/include/basic_types.h
index 69f7402d8e5c..2cb6d24fdb1b 100644
--- a/drivers/staging/rtl8723bs/include/basic_types.h
+++ b/drivers/staging/rtl8723bs/include/basic_types.h
@@ -129,25 +129,25 @@
  * Set subfield of little-endian 4-byte value to specified value.
  */
 #define SET_BITS_TO_LE_4BYTE(__pstart, __bitoffset, __bitlen, __val) \
-		*((u32 *)(__pstart)) =				\
+		(*((u32 *)(__pstart)) =				\
 		(						\
 		LE_BITS_CLEARED_TO_4BYTE(__pstart, __bitoffset, __bitlen) | \
 		((((u32)__val) & BIT_LEN_MASK_32(__bitlen)) << (__bitoffset)) \
-		)
+		))
 
 #define SET_BITS_TO_LE_2BYTE(__pstart, __bitoffset, __bitlen, __val) \
-		*((u16 *)(__pstart)) =				\
+		(*((u16 *)(__pstart)) =				\
 		(					\
 		LE_BITS_CLEARED_TO_2BYTE(__pstart, __bitoffset, __bitlen) | \
 		((((u16)__val) & BIT_LEN_MASK_16(__bitlen)) << (__bitoffset)) \
-		)
+		))
 
 #define SET_BITS_TO_LE_1BYTE(__pstart, __bitoffset, __bitlen, __val) \
-		*((u8 *)(__pstart)) = EF1BYTE			\
+		(*((u8 *)(__pstart)) = EF1BYTE			\
 		(					\
 		LE_BITS_CLEARED_TO_1BYTE(__pstart, __bitoffset, __bitlen) | \
 		((((u8)__val) & BIT_LEN_MASK_8(__bitlen)) << (__bitoffset)) \
-		)
+		))
 
 #define LE_BITS_CLEARED_TO_1BYTE_8BIT(__pStart, __BitOffset, __BitLen) \
 	(\
-- 
2.53.0

[PATCH v2 3/3] staging: rtl8723bs: remove redundant blank lines in basic_types.h

[Mashiro Chen]

Remove redundant blank lines at the top of the file to improve
code cleanliness.

Signed-off-by: Mashiro Chen <mashiro.chen@mailbox.org>
---
 drivers/staging/rtl8723bs/include/basic_types.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/include/basic_types.h b/drivers/staging/rtl8723bs/include/basic_types.h
index 2cb6d24fdb1b..b6b1afaa9ab8 100644
--- a/drivers/staging/rtl8723bs/include/basic_types.h
+++ b/drivers/staging/rtl8723bs/include/basic_types.h
@@ -7,7 +7,6 @@
 #ifndef __BASIC_TYPES_H__
 #define __BASIC_TYPES_H__
 
-
 #define SUCCESS	0
 #define FAIL	(-1)
 
-- 
2.53.0

Re: [PATCH v2 0/3] staging: rtl8723bs: macro fixes in basic_types.h

[Luka Gejak]

Hi Mashiro,
Thanks for the v2. This series is a solid improvement. Cleaning up the
basic_types.h header is a necessary step toward eventually getting 
qqthis driver out of staging and into the mainline networking tree.
Here is my formal review of the series.

Patch 1/3: Excellent catch by Dan Carpenter here. Removing the 
WRITEEF/READEF macros is far superior to fixing them. Specifically, 
the latent bug in WRITEEF4BYTE (which incorrectly used EF2BYTE) is now
gone without adding any binary bloat.

Patch 2/3: This correctly silences the COMPLEX_MACRO warnings from 
checkpatch.pl. While these macros are candidates for a full refactor 
into <linux/bitfield.h> eventually, your fix ensures operator 
precedence is respected in the meantime.

Patch 3/3: Clean and trivial.

So for the series:
Reviewed-by: Luka Gejak <luka.gejak@linux.dev>

Best regards,
Luka Gejak