Hi Fabio,
On 6/22/21 11:16 AM, Fabio Aiuto wrote:
> Hello Hans,
>
> On Mon, Jun 21, 2021 at 11:45:39AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 6/19/21 12:47 PM, Fabio Aiuto wrote:
>>> remove VHT dead code, as the device doesn't support
>>> VHT (which is a 802.11ac capability).
>>>
>>> Signed-off-by: Fabio Aiuto <fabioaiuto83@xxxxxxxxx>
>>> ---
>>> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 1 -
>>> drivers/staging/rtl8723bs/hal/hal_com.c | 241 -----------
>>> .../staging/rtl8723bs/hal/hal_com_phycfg.c | 383 +-----------------
>>> drivers/staging/rtl8723bs/include/hal_com.h | 62 +--
>>> .../rtl8723bs/include/hal_com_phycfg.h | 4 -
>>> drivers/staging/rtl8723bs/include/ieee80211.h | 45 --
>>> .../staging/rtl8723bs/include/rtl8723b_xmit.h | 21 -
>>> drivers/staging/rtl8723bs/include/rtw_ht.h | 4 -
>>> 8 files changed, 9 insertions(+), 752 deletions(-)
>>>
>>> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
>>> index 285acd3d843b..c128d462c6c7 100644
>>> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
>>> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
>>> @@ -46,7 +46,6 @@ static struct action_handler OnAction_tbl[] = {
>>> {RTW_WLAN_CATEGORY_UNPROTECTED_WNM, "ACTION_UNPROTECTED_WNM", &DoReserved},
>>> {RTW_WLAN_CATEGORY_SELF_PROTECTED, "ACTION_SELF_PROTECTED", &DoReserved},
>>> {RTW_WLAN_CATEGORY_WMM, "ACTION_WMM", &DoReserved},
>>> - {RTW_WLAN_CATEGORY_VHT, "ACTION_VHT", &DoReserved},
>>> {RTW_WLAN_CATEGORY_P2P, "ACTION_P2P", &DoReserved},
>>> };
>>>
>>> diff --git a/drivers/staging/rtl8723bs/hal/hal_com.c b/drivers/staging/rtl8723bs/hal/hal_com.c
>>> index 7a88447f8294..eebd48438733 100644
>>> --- a/drivers/staging/rtl8723bs/hal/hal_com.c
>>> +++ b/drivers/staging/rtl8723bs/hal/hal_com.c
>>> @@ -295,126 +295,6 @@ u8 MRateToHwRate(u8 rate)
>>> case MGN_MCS31:
>>> ret = DESC_RATEMCS31;
>>> break;
>>> - case MGN_VHT1SS_MCS0:
>>> - ret = DESC_RATEVHTSS1MCS0;
>>> - break;
>>> - case MGN_VHT1SS_MCS1:
>>> - ret = DESC_RATEVHTSS1MCS1;
>>> - break;
>>> - case MGN_VHT1SS_MCS2:
>>> - ret = DESC_RATEVHTSS1MCS2;
>>> - break;
>>> - case MGN_VHT1SS_MCS3:
>>> - ret = DESC_RATEVHTSS1MCS3;
>>> - break;
>>> - case MGN_VHT1SS_MCS4:
>>> - ret = DESC_RATEVHTSS1MCS4;
>>> - break;
>>> - case MGN_VHT1SS_MCS5:
>>> - ret = DESC_RATEVHTSS1MCS5;
>>> - break;
>>> - case MGN_VHT1SS_MCS6:
>>> - ret = DESC_RATEVHTSS1MCS6;
>>> - break;
>>> - case MGN_VHT1SS_MCS7:
>>> - ret = DESC_RATEVHTSS1MCS7;
>>> - break;
>>> - case MGN_VHT1SS_MCS8:
>>> - ret = DESC_RATEVHTSS1MCS8;
>>> - break;
>>> - case MGN_VHT1SS_MCS9:
>>> - ret = DESC_RATEVHTSS1MCS9;
>>> - break;
>>> - case MGN_VHT2SS_MCS0:
>>> - ret = DESC_RATEVHTSS2MCS0;
>>> - break;
>>> - case MGN_VHT2SS_MCS1:
>>> - ret = DESC_RATEVHTSS2MCS1;
>>> - break;
>>> - case MGN_VHT2SS_MCS2:
>>> - ret = DESC_RATEVHTSS2MCS2;
>>> - break;
>>> - case MGN_VHT2SS_MCS3:
>>> - ret = DESC_RATEVHTSS2MCS3;
>>> - break;
>>> - case MGN_VHT2SS_MCS4:
>>> - ret = DESC_RATEVHTSS2MCS4;
>>> - break;
>>> - case MGN_VHT2SS_MCS5:
>>> - ret = DESC_RATEVHTSS2MCS5;
>>> - break;
>>> - case MGN_VHT2SS_MCS6:
>>> - ret = DESC_RATEVHTSS2MCS6;
>>> - break;
>>> - case MGN_VHT2SS_MCS7:
>>> - ret = DESC_RATEVHTSS2MCS7;
>>> - break;
>>> - case MGN_VHT2SS_MCS8:
>>> - ret = DESC_RATEVHTSS2MCS8;
>>> - break;
>>> - case MGN_VHT2SS_MCS9:
>>> - ret = DESC_RATEVHTSS2MCS9;
>>> - break;
>>> - case MGN_VHT3SS_MCS0:
>>> - ret = DESC_RATEVHTSS3MCS0;
>>> - break;
>>> - case MGN_VHT3SS_MCS1:
>>> - ret = DESC_RATEVHTSS3MCS1;
>>> - break;
>>> - case MGN_VHT3SS_MCS2:
>>> - ret = DESC_RATEVHTSS3MCS2;
>>> - break;
>>> - case MGN_VHT3SS_MCS3:
>>> - ret = DESC_RATEVHTSS3MCS3;
>>> - break;
>>> - case MGN_VHT3SS_MCS4:
>>> - ret = DESC_RATEVHTSS3MCS4;
>>> - break;
>>> - case MGN_VHT3SS_MCS5:
>>> - ret = DESC_RATEVHTSS3MCS5;
>>> - break;
>>> - case MGN_VHT3SS_MCS6:
>>> - ret = DESC_RATEVHTSS3MCS6;
>>> - break;
>>> - case MGN_VHT3SS_MCS7:
>>> - ret = DESC_RATEVHTSS3MCS7;
>>> - break;
>>> - case MGN_VHT3SS_MCS8:
>>> - ret = DESC_RATEVHTSS3MCS8;
>>> - break;
>>> - case MGN_VHT3SS_MCS9:
>>> - ret = DESC_RATEVHTSS3MCS9;
>>> - break;
>>> - case MGN_VHT4SS_MCS0:
>>> - ret = DESC_RATEVHTSS4MCS0;
>>> - break;
>>> - case MGN_VHT4SS_MCS1:
>>> - ret = DESC_RATEVHTSS4MCS1;
>>> - break;
>>> - case MGN_VHT4SS_MCS2:
>>> - ret = DESC_RATEVHTSS4MCS2;
>>> - break;
>>> - case MGN_VHT4SS_MCS3:
>>> - ret = DESC_RATEVHTSS4MCS3;
>>> - break;
>>> - case MGN_VHT4SS_MCS4:
>>> - ret = DESC_RATEVHTSS4MCS4;
>>> - break;
>>> - case MGN_VHT4SS_MCS5:
>>> - ret = DESC_RATEVHTSS4MCS5;
>>> - break;
>>> - case MGN_VHT4SS_MCS6:
>>> - ret = DESC_RATEVHTSS4MCS6;
>>> - break;
>>> - case MGN_VHT4SS_MCS7:
>>> - ret = DESC_RATEVHTSS4MCS7;
>>> - break;
>>> - case MGN_VHT4SS_MCS8:
>>> - ret = DESC_RATEVHTSS4MCS8;
>>> - break;
>>> - case MGN_VHT4SS_MCS9:
>>> - ret = DESC_RATEVHTSS4MCS9;
>>> - break;
>>> default:
>>> break;
>>> }
>>> @@ -559,127 +439,6 @@ u8 HwRateToMRate(u8 rate)
>>> case DESC_RATEMCS31:
>>> ret_rate = MGN_MCS31;
>>> break;
>>> - case DESC_RATEVHTSS1MCS0:
>>> - ret_rate = MGN_VHT1SS_MCS0;
>>> - break;
>>> - case DESC_RATEVHTSS1MCS1:
>>> - ret_rate = MGN_VHT1SS_MCS1;
>>> - break;
>>> - case DESC_RATEVHTSS1MCS2:
>>> - ret_rate = MGN_VHT1SS_MCS2;
>>> - break;
>>> - case DESC_RATEVHTSS1MCS3:
>>> - ret_rate = MGN_VHT1SS_MCS3;
>>> - break;
>>> - case DESC_RATEVHTSS1MCS4:
>>> - ret_rate = MGN_VHT1SS_MCS4;
>>> - break;
>>> - case DESC_RATEVHTSS1MCS5:
>>> - ret_rate = MGN_VHT1SS_MCS5;
>>> - break;
>>> - case DESC_RATEVHTSS1MCS6:
>>> - ret_rate = MGN_VHT1SS_MCS6;
>>> - break;
>>> - case DESC_RATEVHTSS1MCS7:
>>> - ret_rate = MGN_VHT1SS_MCS7;
>>> - break;
>>> - case DESC_RATEVHTSS1MCS8:
>>> - ret_rate = MGN_VHT1SS_MCS8;
>>> - break;
>>> - case DESC_RATEVHTSS1MCS9:
>>> - ret_rate = MGN_VHT1SS_MCS9;
>>> - break;
>>> - case DESC_RATEVHTSS2MCS0:
>>> - ret_rate = MGN_VHT2SS_MCS0;
>>> - break;
>>> - case DESC_RATEVHTSS2MCS1:
>>> - ret_rate = MGN_VHT2SS_MCS1;
>>> - break;
>>> - case DESC_RATEVHTSS2MCS2:
>>> - ret_rate = MGN_VHT2SS_MCS2;
>>> - break;
>>> - case DESC_RATEVHTSS2MCS3:
>>> - ret_rate = MGN_VHT2SS_MCS3;
>>> - break;
>>> - case DESC_RATEVHTSS2MCS4:
>>> - ret_rate = MGN_VHT2SS_MCS4;
>>> - break;
>>> - case DESC_RATEVHTSS2MCS5:
>>> - ret_rate = MGN_VHT2SS_MCS5;
>>> - break;
>>> - case DESC_RATEVHTSS2MCS6:
>>> - ret_rate = MGN_VHT2SS_MCS6;
>>> - break;
>>> - case DESC_RATEVHTSS2MCS7:
>>> - ret_rate = MGN_VHT2SS_MCS7;
>>> - break;
>>> - case DESC_RATEVHTSS2MCS8:
>>> - ret_rate = MGN_VHT2SS_MCS8;
>>> - break;
>>> - case DESC_RATEVHTSS2MCS9:
>>> - ret_rate = MGN_VHT2SS_MCS9;
>>> - break;
>>> - case DESC_RATEVHTSS3MCS0:
>>> - ret_rate = MGN_VHT3SS_MCS0;
>>> - break;
>>> - case DESC_RATEVHTSS3MCS1:
>>> - ret_rate = MGN_VHT3SS_MCS1;
>>> - break;
>>> - case DESC_RATEVHTSS3MCS2:
>>> - ret_rate = MGN_VHT3SS_MCS2;
>>> - break;
>>> - case DESC_RATEVHTSS3MCS3:
>>> - ret_rate = MGN_VHT3SS_MCS3;
>>> - break;
>>> - case DESC_RATEVHTSS3MCS4:
>>> - ret_rate = MGN_VHT3SS_MCS4;
>>> - break;
>>> - case DESC_RATEVHTSS3MCS5:
>>> - ret_rate = MGN_VHT3SS_MCS5;
>>> - break;
>>> - case DESC_RATEVHTSS3MCS6:
>>> - ret_rate = MGN_VHT3SS_MCS6;
>>> - break;
>>> - case DESC_RATEVHTSS3MCS7:
>>> - ret_rate = MGN_VHT3SS_MCS7;
>>> - break;
>>> - case DESC_RATEVHTSS3MCS8:
>>> - ret_rate = MGN_VHT3SS_MCS8;
>>> - break;
>>> - case DESC_RATEVHTSS3MCS9:
>>> - ret_rate = MGN_VHT3SS_MCS9;
>>> - break;
>>> - case DESC_RATEVHTSS4MCS0:
>>> - ret_rate = MGN_VHT4SS_MCS0;
>>> - break;
>>> - case DESC_RATEVHTSS4MCS1:
>>> - ret_rate = MGN_VHT4SS_MCS1;
>>> - break;
>>> - case DESC_RATEVHTSS4MCS2:
>>> - ret_rate = MGN_VHT4SS_MCS2;
>>> - break;
>>> - case DESC_RATEVHTSS4MCS3:
>>> - ret_rate = MGN_VHT4SS_MCS3;
>>> - break;
>>> - case DESC_RATEVHTSS4MCS4:
>>> - ret_rate = MGN_VHT4SS_MCS4;
>>> - break;
>>> - case DESC_RATEVHTSS4MCS5:
>>> - ret_rate = MGN_VHT4SS_MCS5;
>>> - break;
>>> - case DESC_RATEVHTSS4MCS6:
>>> - ret_rate = MGN_VHT4SS_MCS6;
>>> - break;
>>> - case DESC_RATEVHTSS4MCS7:
>>> - ret_rate = MGN_VHT4SS_MCS7;
>>> - break;
>>> - case DESC_RATEVHTSS4MCS8:
>>> - ret_rate = MGN_VHT4SS_MCS8;
>>> - break;
>>> - case DESC_RATEVHTSS4MCS9:
>>> - ret_rate = MGN_VHT4SS_MCS9;
>>> - break;
>>> -
>>> default:
>>> break;
>>> }
>>> diff --git a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
>>> index 395eb3b5af71..bb7941aee0c4 100644
>>> --- a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
>>> +++ b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
>>> @@ -39,18 +39,6 @@ u8 PHY_GetTxPowerByRateBase(struct adapter *Adapter, u8 RfPath,
>>> case HT_MCS24_MCS31:
>>> value = pHalData->TxPwrByRateBase2_4G[RfPath][TxNum][5];
>>> break;
>>> - case VHT_1SSMCS0_1SSMCS9:
>>> - value = pHalData->TxPwrByRateBase2_4G[RfPath][TxNum][6];
>>> - break;
>>> - case VHT_2SSMCS0_2SSMCS9:
>>> - value = pHalData->TxPwrByRateBase2_4G[RfPath][TxNum][7];
>>> - break;
>>> - case VHT_3SSMCS0_3SSMCS9:
>>> - value = pHalData->TxPwrByRateBase2_4G[RfPath][TxNum][8];
>>> - break;
>>> - case VHT_4SSMCS0_4SSMCS9:
>>> - value = pHalData->TxPwrByRateBase2_4G[RfPath][TxNum][9];
>>> - break;
>>> default:
>>> break;
>>> }
>>> @@ -91,18 +79,6 @@ phy_SetTxPowerByRateBase(
>>> case HT_MCS24_MCS31:
>>> pHalData->TxPwrByRateBase2_4G[RfPath][TxNum][5] = Value;
>>> break;
>>> - case VHT_1SSMCS0_1SSMCS9:
>>> - pHalData->TxPwrByRateBase2_4G[RfPath][TxNum][6] = Value;
>>> - break;
>>> - case VHT_2SSMCS0_2SSMCS9:
>>> - pHalData->TxPwrByRateBase2_4G[RfPath][TxNum][7] = Value;
>>> - break;
>>> - case VHT_3SSMCS0_3SSMCS9:
>>> - pHalData->TxPwrByRateBase2_4G[RfPath][TxNum][8] = Value;
>>> - break;
>>> - case VHT_4SSMCS0_4SSMCS9:
>>> - pHalData->TxPwrByRateBase2_4G[RfPath][TxNum][9] = Value;
>>> - break;
>>> default:
>>> break;
>>> }
>>> @@ -131,14 +107,6 @@ struct adapter *padapter
>>> base = PHY_GetTxPowerByRate(padapter, path, RF_3TX, MGN_MCS23);
>>> phy_SetTxPowerByRateBase(padapter, path, HT_MCS16_MCS23, RF_3TX, base);
>>>
>>> - base = PHY_GetTxPowerByRate(padapter, path, RF_1TX, MGN_VHT1SS_MCS7);
>>> - phy_SetTxPowerByRateBase(padapter, path, VHT_1SSMCS0_1SSMCS9, RF_1TX, base);
>>> -
>>> - base = PHY_GetTxPowerByRate(padapter, path, RF_2TX, MGN_VHT2SS_MCS7);
>>> - phy_SetTxPowerByRateBase(padapter, path, VHT_2SSMCS0_2SSMCS9, RF_2TX, base);
>>> -
>>> - base = PHY_GetTxPowerByRate(padapter, path, RF_3TX, MGN_VHT3SS_MCS7);
>>> - phy_SetTxPowerByRateBase(padapter, path, VHT_3SSMCS0_3SSMCS9, RF_3TX, base);
>>
>> You are removing a bunch of register writes from a block here, while keeping others,
>> so this does not seem like it is just dead-code removal it feels like you are
>> actually making a functional change.
>
> I looked deeply at what I deed, and I just removed some functions updating tables
> in memory, no register write here. Tables are used to load Tx Power Limit. So
> removing these function calls according with the removal of the VHT enum items
> grants that wrong Power Limits will never be fetched.
>
> Moreover I have been quite conservative, for I left untouched HT indexes above
> 7 which rtl8723bs doesn't support.
>
> So IMO I think this patch is fine as is...
>> Perhaps this entire block can never be executed ?
>
> the block is executed but there's no register write happening. Just
> updating of values which will never be fetched.
Ack, my bad I was under the impression that phy_SetTxPowerByRateBase()
would actually do a register write, but I checked and it just updates
some unused table values, so dropping this code is fine and you can
keep this patch for v2 of the patch set.
Regards,
Hans
