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.
Tested with rtl8192e
Transferred this patch over wlan connection of rtl8192e
This patch series requires the patches:
"[PATCH 0/2] staging: rtl8192e: Removing rf_type RF_2T4R from driver"
"[PATCH] staging: rtl8192e: Change filename r8192E_hwimg.x to table.x"
"[PATCH] staging: rtl8192e: Remove call_usermodehelper starting RadioPower.sh"
"[PATCH] staging: rtl8192e: Remove function ..dm_check_ac_dc_power calling a script"
Philipp Hortmann (2):
staging: rtl8192e: Remove empty Array Rtl8192PciERadioC_Array
staging: rtl8192e: Remove empty Array Rtl8192PciERadioD_Array
.../staging/rtl8192e/rtl8192e/r8192E_phy.c | 24 -------------------
.../staging/rtl8192e/rtl8192e/r8192E_phy.h | 4 ----
drivers/staging/rtl8192e/rtl8192e/table.c | 6 -----
drivers/staging/rtl8192e/rtl8192e/table.h | 4 ----
4 files changed, 38 deletions(-)
--
2.39.2
Remove empty array Rtl8192PciERadioC_Array and the code where it is used
because it is dead code.
Signed-off-by: Philipp Hortmann <[email protected]>
---
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]);
-
}
break;
case RF90_PATH_D:
diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.h b/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.h
index 561a4c874fe3..6772ed300591 100644
--- a/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.h
+++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.h
@@ -14,7 +14,6 @@
#define RadioA_ArrayLength RadioA_ArrayLengthPciE
#define RadioB_ArrayLength RadioB_ArrayLengthPciE
#define MACPHY_Array_PGLength MACPHY_Array_PGLengthPciE
-#define RadioC_ArrayLength RadioC_ArrayLengthPciE
#define RadioD_ArrayLength RadioD_ArrayLengthPciE
#define PHY_REG_1T2RArrayLength PHY_REG_1T2RArrayLengthPciE
@@ -22,7 +21,6 @@
#define Rtl819XMACPHY_Array Rtl8192PciEMACPHY_Array
#define Rtl819XRadioA_Array Rtl8192PciERadioA_Array
#define Rtl819XRadioB_Array Rtl8192PciERadioB_Array
-#define Rtl819XRadioC_Array Rtl8192PciERadioC_Array
#define Rtl819XRadioD_Array Rtl8192PciERadioD_Array
#define Rtl819XAGCTAB_Array Rtl8192PciEAGCTAB_Array
#define Rtl819XPHY_REG_1T2RArray Rtl8192PciEPHY_REG_1T2RArray
diff --git a/drivers/staging/rtl8192e/rtl8192e/table.c b/drivers/staging/rtl8192e/rtl8192e/table.c
index d04608d99cbf..4e97ad4b8fd7 100644
--- a/drivers/staging/rtl8192e/rtl8192e/table.c
+++ b/drivers/staging/rtl8192e/rtl8192e/table.c
@@ -325,9 +325,6 @@ u32 Rtl8192PciERadioB_Array[RadioB_ArrayLengthPciE] = {
0x007, 0x00000700,
};
-u32 Rtl8192PciERadioC_Array[RadioC_ArrayLengthPciE] = {
- 0x0, };
-
u32 Rtl8192PciERadioD_Array[RadioD_ArrayLengthPciE] = {
0x0, };
diff --git a/drivers/staging/rtl8192e/rtl8192e/table.h b/drivers/staging/rtl8192e/rtl8192e/table.h
index 177fe2173430..981e8b57d3de 100644
--- a/drivers/staging/rtl8192e/rtl8192e/table.h
+++ b/drivers/staging/rtl8192e/rtl8192e/table.h
@@ -17,8 +17,6 @@ extern u32 Rtl8192PciEPHY_REG_1T2RArray[PHY_REG_1T2RArrayLengthPciE];
extern u32 Rtl8192PciERadioA_Array[RadioA_ArrayLengthPciE];
#define RadioB_ArrayLengthPciE 78
extern u32 Rtl8192PciERadioB_Array[RadioB_ArrayLengthPciE];
-#define RadioC_ArrayLengthPciE 2
-extern u32 Rtl8192PciERadioC_Array[RadioC_ArrayLengthPciE];
#define RadioD_ArrayLengthPciE 2
extern u32 Rtl8192PciERadioD_Array[RadioD_ArrayLengthPciE];
#define MACPHY_ArrayLengthPciE 18
--
2.39.2
Remove empty array Rtl8192PciERadioD_Array and the code where it is used
because it is dead code.
Signed-off-by: Philipp Hortmann <[email protected]>
---
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 fe0ef52c163a..a9e83ba811b0 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_D:
- for (i = 0; i < RadioD_ArrayLength; i += 2) {
- if (Rtl819XRadioD_Array[i] == 0xfe) {
- msleep(100);
- continue;
- }
- rtl92e_set_rf_reg(dev, eRFPath, Rtl819XRadioD_Array[i],
- bMask12Bits,
- Rtl819XRadioD_Array[i+1]);
-
}
break;
default:
diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.h b/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.h
index 6772ed300591..ac640033e843 100644
--- a/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.h
+++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_phy.h
@@ -14,14 +14,12 @@
#define RadioA_ArrayLength RadioA_ArrayLengthPciE
#define RadioB_ArrayLength RadioB_ArrayLengthPciE
#define MACPHY_Array_PGLength MACPHY_Array_PGLengthPciE
-#define RadioD_ArrayLength RadioD_ArrayLengthPciE
#define PHY_REG_1T2RArrayLength PHY_REG_1T2RArrayLengthPciE
#define Rtl819XMACPHY_Array_PG Rtl8192PciEMACPHY_Array_PG
#define Rtl819XMACPHY_Array Rtl8192PciEMACPHY_Array
#define Rtl819XRadioA_Array Rtl8192PciERadioA_Array
#define Rtl819XRadioB_Array Rtl8192PciERadioB_Array
-#define Rtl819XRadioD_Array Rtl8192PciERadioD_Array
#define Rtl819XAGCTAB_Array Rtl8192PciEAGCTAB_Array
#define Rtl819XPHY_REG_1T2RArray Rtl8192PciEPHY_REG_1T2RArray
diff --git a/drivers/staging/rtl8192e/rtl8192e/table.c b/drivers/staging/rtl8192e/rtl8192e/table.c
index 4e97ad4b8fd7..7101061e3519 100644
--- a/drivers/staging/rtl8192e/rtl8192e/table.c
+++ b/drivers/staging/rtl8192e/rtl8192e/table.c
@@ -325,9 +325,6 @@ u32 Rtl8192PciERadioB_Array[RadioB_ArrayLengthPciE] = {
0x007, 0x00000700,
};
-u32 Rtl8192PciERadioD_Array[RadioD_ArrayLengthPciE] = {
- 0x0, };
-
u32 Rtl8192PciEMACPHY_Array[] = {
0x03c, 0xffff0000, 0x00000f0f,
0x340, 0xffffffff, 0x161a1a1a,
diff --git a/drivers/staging/rtl8192e/rtl8192e/table.h b/drivers/staging/rtl8192e/rtl8192e/table.h
index 981e8b57d3de..b6d4e9696452 100644
--- a/drivers/staging/rtl8192e/rtl8192e/table.h
+++ b/drivers/staging/rtl8192e/rtl8192e/table.h
@@ -17,8 +17,6 @@ extern u32 Rtl8192PciEPHY_REG_1T2RArray[PHY_REG_1T2RArrayLengthPciE];
extern u32 Rtl8192PciERadioA_Array[RadioA_ArrayLengthPciE];
#define RadioB_ArrayLengthPciE 78
extern u32 Rtl8192PciERadioB_Array[RadioB_ArrayLengthPciE];
-#define RadioD_ArrayLengthPciE 2
-extern u32 Rtl8192PciERadioD_Array[RadioD_ArrayLengthPciE];
#define MACPHY_ArrayLengthPciE 18
extern u32 Rtl8192PciEMACPHY_Array[MACPHY_ArrayLengthPciE];
#define MACPHY_Array_PGLengthPciE 30
--
2.39.2
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 <[email protected]>
> ---
> 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
On Sun, Mar 05, 2023 at 11:33:18PM +0100, Philipp Hortmann wrote:
> Remove empty array Rtl8192PciERadioD_Array and the code where it is used
> because it is dead code.
>
> Signed-off-by: Philipp Hortmann <[email protected]>
Same comments as for the earlier commit.
regards,
dan carpenter
On Sun, Mar 05, 2023 at 11:33:18PM +0100, Philipp Hortmann wrote:
> Remove empty array Rtl8192PciERadioD_Array and the code where it is used
> because it is dead code.
It's not really empty. It has two zeroes in it.
regards,
dan carpenter
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 <[email protected]>
>> ---
>> 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
On Mon, Mar 06, 2023 at 09:43:50PM +0100, Philipp Hortmann wrote:
> 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:
I never read cover letters. No one is ever going to see the cover
letter when they look through `git log -p` so I don't see the point...
>
> 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]);
You make a really compelling argument that the code in rtl8192cu
should be deleted, but that's a different driver from what we're
discussing here.
> }
>
> 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.
I can easily be convinced of that, but that's not what the commit
message says. The commit message says the array is "empty" and that
you are deleting "dead code". When actually you're deleting a call to
rtl92e_set_rf_reg(). So the commit message is just wrong about the
facts.
What you need to do is figure out what the call to rtl92e_set_rf_reg()
does and why it's not required.
I don't have a problem with this *patch* but the commit message says
things which are false.
regards,
dan carpenter