2023-11-06 03:38:49

by Ping-Ke Shih

[permalink] [raw]
Subject: RE: [PATCH 1/2] wifi: rtlwifi: remove dead code in phy_get_tx_swing_8812A()



> -----Original Message-----
> From: Dmitry Antipov <[email protected]>
> Sent: Wednesday, November 1, 2023 7:35 PM
> To: Ping-Ke Shih <[email protected]>
> Cc: Kalle Valo <[email protected]>; [email protected]; [email protected]; Dmitry
> Antipov <[email protected]>
> Subject: [PATCH 1/2] wifi: rtlwifi: remove dead code in phy_get_tx_swing_8812A()
>
> Since 'reg_swing_2g', 'swing_2g', 'reg_swing_5g', and 'swing_5g'
> are compile-time constants, mark all of them as such and remove
> never executed blocks in 'phy_get_tx_swing_8812A()'. Minor style
> adjustments, compile tested only.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Dmitry Antipov <[email protected]>
> ---
> .../wireless/realtek/rtlwifi/rtl8821ae/phy.c | 77 ++++---------------
> 1 file changed, 16 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
> b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
> index 5323ead30db0..c262770fea29 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
> @@ -437,12 +437,11 @@ u32 phy_get_tx_swing_8812A(struct ieee80211_hw *hw, u8 band,
> struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw));
> struct rtl_dm *rtldm = rtl_dm(rtlpriv);
> struct rtl_efuse *rtlefuse = rtl_efuse(rtl_priv(hw));
> - s8 reg_swing_2g = -1;/* 0xff; */
> - s8 reg_swing_5g = -1;/* 0xff; */
> - s8 swing_2g = -1 * reg_swing_2g;
> - s8 swing_5g = -1 * reg_swing_5g;
> - u32 out = 0x200;
> - const s8 auto_temp = -1;
> + const s8 reg_swing_2g = -1; /* 0xff; */
> + const s8 reg_swing_5g = -1; /* 0xff; */

Looks like these two can be removed, but I don't really understand what it
wants originally.

> + const s8 swing_2g = -1 * reg_swing_2g;
> + const s8 swing_5g = -1 * reg_swing_5g;
> + u32 out = 0x200;
>
> rtl_dbg(rtlpriv, COMP_SCAN, DBG_LOUD,
> "===> PHY_GetTXBBSwing_8812A, bbSwing_2G: %d, bbSwing_5G: %d,autoload_failflag=%d.\n",
> @@ -452,36 +451,16 @@ u32 phy_get_tx_swing_8812A(struct ieee80211_hw *hw, u8 band,
> if (rtlefuse->autoload_failflag) {
> if (band == BAND_ON_2_4G) {
> rtldm->swing_diff_2g = swing_2g;
> - if (swing_2g == 0) {
> - out = 0x200; /* 0 dB */
> - } else if (swing_2g == -3) {
> - out = 0x16A; /* -3 dB */
> - } else if (swing_2g == -6) {
> - out = 0x101; /* -6 dB */
> - } else if (swing_2g == -9) {
> - out = 0x0B6; /* -9 dB */
> - } else {
> - rtldm->swing_diff_2g = 0;
> - out = 0x200;
> - }
> + rtldm->swing_diff_2g = 0;
> + out = 0x200;

This branch becomes duplicate assignments of swing_diff_2g.
rtldm->swing_diff_2g = swing_2g;
rtldm->swing_diff_2g = 0;
out = 0x200;

The original branch chunk might be debug purpose, so maybe add a debugfs entry as
the input of 'swing_2g'. Or, that is used by some boards that don't programmed efuse
but read 'swing_2g' from external file.


It is hard to me to decide if we should take this cleanup.

Ping-Ke