Skip to contentSkip to conversation

staging: r8188eu: clean up dump_chip_info()

PATCH In review Michael Straube 2 participants 4 messages Jul 30, 2022 Staging

lore
Share & tools
lore

Michael Straube

author
#0 lore raw

cover staging: r8188eu: clean up dump_chip_info()

These two patches clean up the function dump_chip_info() as suggested by Greg Kroah-Hartman and Joe Perches.

Michael Straube (2):
staging: r8188eu: convert dump_chip_info() to use netdev_dbg()
staging: r8188eu: refactor dump_chip_info()

 .../staging/r8188eu/hal/rtl8188e_hal_init.c   | 37 ++++++++-----------
 1 file changed, 16 insertions(+), 21 deletions(-)
Signature · 1 line
2.37.1
author
#1 lore raw

PATCH 1/2 staging: r8188eu: convert dump_chip_info() to use netdev_dbg()

Drivers should not spam the kernel log if they work properly. Convert the dump_chip_info() function to use netdev_dbg() instead of pr_info() so that developers can still enable it if they want to see this information.

Suggested-by Greg Kroah-Hartman <[email protected]>
Signed-off-by Michael Straube <[email protected]>
---
 drivers/staging/r8188eu/hal/rtl8188e_hal_init.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/staging/r8188eu/hal/rtl8188e_hal_init.c +5 -4
index 5b8f1a912bbb..012828a05e6c 100644
@@ -526,8 +526,9 @@ void rtl8188e_ReadEFuse(struct adapter *Adapter, u16 _size_byte, u8 *pbuf)
 	Hal_EfuseReadEFuse88E(Adapter, 0, _size_byte, pbuf);
 }
 
-static void dump_chip_info(struct HAL_VERSION chip_vers)
+static void dump_chip_info(struct adapter *adapter, struct HAL_VERSION chip_vers)
 {
+	struct net_device *netdev = adapter->pnetdev;
 	uint cnt = 0;
 	char buf[128];
 

@@ -560,9 +561,9 @@ static void dump_chip_info(struct HAL_VERSION chip_vers)
 
 	cnt += sprintf((buf + cnt), "1T1R_");
 
-	cnt += sprintf((buf + cnt), "RomVer(%d)\n", 0);
+	cnt += sprintf((buf + cnt), "RomVer(%d)", 0);
 
-	pr_info("%s", buf);
+	netdev_dbg(netdev, "%s\n", buf);
 }
 
 void rtl8188e_read_chip_version(struct adapter *padapter)

@@ -581,7 +582,7 @@ void rtl8188e_read_chip_version(struct adapter *padapter)
 	ChipVersion.VendorType = ((value32 & VENDOR_ID) ? CHIP_VENDOR_UMC : CHIP_VENDOR_TSMC);
 	ChipVersion.CUTVersion = (value32 & CHIP_VER_RTL_MASK) >> CHIP_VER_RTL_SHIFT; /*  IC version (CUT) */
 
-	dump_chip_info(ChipVersion);
+	dump_chip_info(padapter, ChipVersion);
 
 	pHalData->VersionID = ChipVersion;
 }
Signature · 1 line
2.37.1
author
#2 lore raw

PATCH 2/2 staging: r8188eu: refactor dump_chip_info()

In reply to Michael Straube

Refactor the function dump_chip_info() to make the code cleaner and reduce the driver object file size. Instead of using sprintf() to print all the information to a buffer use a char pointer for the cut version string and print the other strings directly by netdev_dbg(). For the unknown cut string we can use a smaller buffer and print to that buffer with snprintf() to be safe.

These changes avoid the possible buffer overflow that the original code had and reduces the driver object file size by 1029 bytes.

Suggested-by Joe Perches <[email protected]>
Signed-off-by Michael Straube <[email protected]>
---
 .../staging/r8188eu/hal/rtl8188e_hal_init.c   | 32 ++++++++-----------
 1 file changed, 13 insertions(+), 19 deletions(-)
drivers/staging/r8188eu/hal/rtl8188e_hal_init.c +13 -19
index 012828a05e6c..1fc4ba45bf31 100644
@@ -529,41 +529,35 @@ void rtl8188e_ReadEFuse(struct adapter *Adapter, u16 _size_byte, u8 *pbuf)
 static void dump_chip_info(struct adapter *adapter, struct HAL_VERSION chip_vers)
 {
 	struct net_device *netdev = adapter->pnetdev;
-	uint cnt = 0;
-	char buf[128];
-
-	cnt += sprintf((buf + cnt), "Chip Version Info: CHIP_8188E_");
-	cnt += sprintf((buf + cnt), "%s_", IS_NORMAL_CHIP(chip_vers) ?
-		       "Normal_Chip" : "Test_Chip");
-	cnt += sprintf((buf + cnt), "%s_", IS_CHIP_VENDOR_TSMC(chip_vers) ?
-		       "TSMC" : "UMC");
+	char *cut = NULL;
+	char buf[25];
 
 	switch (chip_vers.CUTVersion) {
 	case A_CUT_VERSION:
-		cnt += sprintf((buf + cnt), "A_CUT_");
+		cut = "A_CUT";
 		break;
 	case B_CUT_VERSION:
-		cnt += sprintf((buf + cnt), "B_CUT_");
+		cut = "B_CUT";
 		break;
 	case C_CUT_VERSION:
-		cnt += sprintf((buf + cnt), "C_CUT_");
+		cut = "C_CUT";
 		break;
 	case D_CUT_VERSION:
-		cnt += sprintf((buf + cnt), "D_CUT_");
+		cut = "D_CUT";
 		break;
 	case E_CUT_VERSION:
-		cnt += sprintf((buf + cnt), "E_CUT_");
+		cut = "E_CUT";
 		break;
 	default:
-		cnt += sprintf((buf + cnt), "UNKNOWN_CUT(%d)_", chip_vers.CUTVersion);
+		snprintf(buf, sizeof(buf), "UNKNOWN_CUT(%d)", chip_vers.CUTVersion);
+		cut = buf;
 		break;
 	}
 
-	cnt += sprintf((buf + cnt), "1T1R_");
-
-	cnt += sprintf((buf + cnt), "RomVer(%d)", 0);
-
-	netdev_dbg(netdev, "%s\n", buf);
+	netdev_dbg(netdev, "Chip Version Info: CHIP_8188E_%s_%s_%s_1T1R_RomVer(%d)\n",
+		   IS_NORMAL_CHIP(chip_vers) ? "Normal_Chip" : "Test_Chip",
+		   IS_CHIP_VENDOR_TSMC(chip_vers) ? "TSMC" : "UMC",
+		   cut, 0);
 }
 
 void rtl8188e_read_chip_version(struct adapter *padapter)
Signature · 1 line
2.37.1
Tested-by on the v1 series
#3 lore raw
In reply to Michael Straube
Quoted 10 lines · Michael Straube

These two patches clean up the function dump_chip_info() as suggested by Greg Kroah-Hartman and Joe Perches.

Michael Straube (2):
staging: r8188eu: convert dump_chip_info() to use netdev_dbg()
staging: r8188eu: refactor dump_chip_info()

  .../staging/r8188eu/hal/rtl8188e_hal_init.c   | 37 ++++++++-----------
  1 file changed, 16 insertions(+), 21 deletions(-)
Tested-by Philipp Hortmann <[email protected]> # Edimax N150