Re: [PATCH 1/2] staging: rtl8192e: Remove empty Array Rtl8192PciERadioC_Array

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/6/23 10:12, Dan Carpenter wrote:
On Sun, Mar 05, 2023 at 11:33:05PM +0100, Philipp Hortmann wrote:
Remove empty array Rtl8192PciERadioC_Array and the code where it is used
because it is dead code.

Signed-off-by: Philipp Hortmann <philipp.g.hortmann@xxxxxxxxx>
---
  drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c | 12 ------------
  drivers/staging/rtl8192e/rtl8192e/r8192E_phy.h |  2 --
  drivers/staging/rtl8192e/rtl8192e/table.c      |  3 ---
  drivers/staging/rtl8192e/rtl8192e/table.h      |  2 --
  4 files changed, 19 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c b/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c
index 35ca01ab65ff..fe0ef52c163a 100644
--- a/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c
+++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c
@@ -649,18 +649,6 @@ u8 rtl92e_config_rf_path(struct net_device *dev, enum rf90_radio_path eRFPath)
  					  bMask12Bits,
  					  Rtl819XRadioB_Array[i+1]);
- }
-		break;
-	case RF90_PATH_C:
-		for (i = 0; i < RadioC_ArrayLength; i += 2) {
-			if (Rtl819XRadioC_Array[i] == 0xfe) {
-				msleep(100);
-				continue;
-			}
-			rtl92e_set_rf_reg(dev, eRFPath, Rtl819XRadioC_Array[i],
-					  bMask12Bits,
-					  Rtl819XRadioC_Array[i+1]);
-

Why is this dead code?  So far as I can see "== 0xfe" is always false
so this calls rtl92e_set_rf_reg() on every iteration through the loop.
It only does one iteration through the loop.

Is it dead code because case RF90_PATH_C is always false?  If so then
that needs to be explained in the commit message.

regards,
dan carpenter


Hi Dan,

thanks for the review. Here some answers to your questions:

With patch: "[PATCH] staging: rtl8192e: Change filename r8192E_hwimg.x to table.x" I changed the filename of r8192E_hwimg.c to table.c and r8192E_hwimg.h to table.h to adapt filenames from drivers/net/wireless/realtek/rtlwifi rtl8192ee and rtl8192se. Task is from TODO file.

The explanation from the cover letter of this patch series was:

Rtl8192PciERadioC_Array and Rtl8192PciERadioD_Array contain only two
values set to 0. Reviewing the other Arrays in table.c and looking into
other realtek drivers (rtl8192se and rtl8192ee) this arrays are not
containing valid data.

Here some more examples of my thoughts:

A valid filled array is looking like this:
u32 Rtl8192PciERadioA_Array[RadioA_ArrayLengthPciE] = {
	0x019, 0x00000003,
	0x000, 0x000000bf,
	0x001, 0x00000ee0,
	...
	over 100 lines but no 0x000, 0x00000000,
	...
	0x004, 0x00000975,
	0x007, 0x00000700,
};

u32 Rtl8192PciERadioB_Array[RadioB_ArrayLengthPciE] = {
	0x019, 0x00000003,
	0x000, 0x000000bf,
	0x001, 0x000006e0,
	...
	over 30 lines but no 0x000, 0x00000000,
	...
	0x004, 0x00000975,
	0x007, 0x00000700,
};

The empty (it is not empty but compared to the ones filled with data it is empty) one is looking like this:
u32 Rtl8192PciERadioC_Array[RadioC_ArrayLengthPciE] = {
	0x0,  };

Looking into other cleaned up drivers from the same family:
Example: drivers/net/wireless/realtek/rtlwifi/rtl8192se
Arrays RadioA and RadioB are filled RadioC and RadioD do not exist.
Example: drivers/net/wireless/realtek/rtlwifi/rtl8192ee
Arrays RadioA and RadioB are filled RadioC and RadioD do not exist.
Example: drivers/net/wireless/realtek/rtlwifi/rtl8192de
Arrays RadioA and RadioB are filled RadioC and RadioD do not exist.


In Example: drivers/net/wireless/realtek/rtlwifi/rtl8192cu
I can find a RadioB Array that is just filled with one 0 which is odd:
#define RTL8192CURADIOB_1TARRAYLENGTH	1

u32 RTL8192CU_RADIOB_1TARRAY[RTL8192CURADIOB_1TARRAYLENGTH] = {
	0x0,
};

Here it is written to a variable:

		rtlphy->hwparam_tables[RADIOB_1T].length =
			RTL8192CURADIOB_1TARRAYLENGTH;
		rtlphy->hwparam_tables[RADIOB_1T].pdata =
			RTL8192CU_RADIOB_1TARRAY;

Written to an another variable:

		radiob_arraylen = rtlphy->hwparam_tables[RADIOB_1T].length;
		radiob_array_table = rtlphy->hwparam_tables[RADIOB_1T].pdata;


And then accessed after with i + 1 the element that is random/undefined.


	case RF90_PATH_B:
		for (i = 0; i < radiob_arraylen; i = i + 2) {
			rtl_rfreg_delay(hw, rfpath, radiob_array_table[i],
					RFREG_OFFSET_MASK,
					radiob_array_table[i + 1]);
		}

May be another patch.

I hope this can convince you that Arrays for RadioX that do only contain one or two 0 are not in use.

Please let me know your thoughts.

Thanks

Philipp




























[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux