Skip to contentSkip to conversation

staging: rtl8723bs: removal of 5G code

DISCUSSION Fabio Aiuto 2 participants 3 messages May 20, 2021 – May 25, 2021 Staging

lore
Share & tools
lore
#0 lore raw

Hi all,

I'm stick with removal of 5Ghz code from rtl8723bs wireless card driver (in staging subsystem).

I think that this task comprehend deletion of all code managing 80Mhz bandwidth and upper bandwidth (160 and 80+80). For the latter it's simple, there's quite no code (unused enums and obsolete comments).

The former seems to be trickier, there are handlers like this:

        /* 3 Set Reg483 */
        SubChnlNum = phy_GetSecondaryChnl_8723B(Adapter);
        rtw_write8(Adapter, REG_DATA_SC_8723B, SubChnlNum);

phy_GetSecondaryChnl_8723B() contains code like:

        } else if (pHalData->CurrentChannelBW == CHANNEL_WIDTH_40) {
                if (pHalData->nCur40MhzPrimeSC == HAL_PRIME_CHNL_OFFSET_UPPER)
                        SCSettingOf20 = VHT_DATA_SC_20_UPPER_OF_80MHZ;
                else if (pHalData->nCur40MhzPrimeSC == HAL_PRIME_CHNL_OFFSET_LOWER)
                        SCSettingOf20 = VHT_DATA_SC_20_LOWER_OF_80MHZ;
        }

so if we are on a 40M channel some settings involving 80M are made and the whole is then written on card registers.

May I get rid of the whole? Are there some implications I should be aware of? Is secondary channel needed if we are on 40M bandwidth?

Thank you in advance,

fabio

#1 lore raw

Hi,

Quoted 24 lines · Fabio Aiuto

Hi all,

I'm stick with removal of 5Ghz code from rtl8723bs wireless card driver (in staging subsystem).

I think that this task comprehend deletion of all code managing 80Mhz bandwidth and upper bandwidth (160 and 80+80). For the latter it's simple, there's quite no code (unused enums and obsolete comments).

The former seems to be trickier, there are handlers like this:

        /* 3 Set Reg483 */
        SubChnlNum = phy_GetSecondaryChnl_8723B(Adapter);
        rtw_write8(Adapter, REG_DATA_SC_8723B, SubChnlNum);

phy_GetSecondaryChnl_8723B() contains code like:

        } else if (pHalData->CurrentChannelBW == CHANNEL_WIDTH_40) {
                if (pHalData->nCur40MhzPrimeSC == HAL_PRIME_CHNL_OFFSET_UPPER)
                        SCSettingOf20 = VHT_DATA_SC_20_UPPER_OF_80MHZ;
                else if (pHalData->nCur40MhzPrimeSC == HAL_PRIME_CHNL_OFFSET_LOWER)
                        SCSettingOf20 = VHT_DATA_SC_20_LOWER_OF_80MHZ;
        }
Quoted 2 lines

so if we are on a 40M channel some settings involving 80M are made and the whole is then written on card registers.

I'm no wifi expert, so I was hoping someone else would respond...

With that said I believe you should keep this else block, this part of the function seems to select the order of the bonded channels when bonding multiple 20MHz channels together.

The "if (pHalData->CurrentChannelBW == CHANNEL_WIDTH_80) {}" part can be removed because on 2.4G only devices 80 MHz width is not supported, but the 40MHz bit should stay, the constants for the register bits may be named after the 80MHz option, but I believe these same register bits will impact the 40Mhz case too.

It might be a good idea to rename the constants to VHT_DATA_SC_20_*_OF_40MHZ in a separate patch.

Regards,

Hans

#2 lore raw

Hi Hans and everyone,

Quoted 36 lines · Hans de Goede

Hi,

Quoted 24 lines · Fabio Aiuto

Hi all,

I'm stick with removal of 5Ghz code from rtl8723bs wireless card driver (in staging subsystem).

I think that this task comprehend deletion of all code managing 80Mhz bandwidth and upper bandwidth (160 and 80+80). For the latter it's simple, there's quite no code (unused enums and obsolete comments).

The former seems to be trickier, there are handlers like this:

        /* 3 Set Reg483 */
        SubChnlNum = phy_GetSecondaryChnl_8723B(Adapter);
        rtw_write8(Adapter, REG_DATA_SC_8723B, SubChnlNum);

phy_GetSecondaryChnl_8723B() contains code like:

        } else if (pHalData->CurrentChannelBW == CHANNEL_WIDTH_40) {
                if (pHalData->nCur40MhzPrimeSC == HAL_PRIME_CHNL_OFFSET_UPPER)
                        SCSettingOf20 = VHT_DATA_SC_20_UPPER_OF_80MHZ;
                else if (pHalData->nCur40MhzPrimeSC == HAL_PRIME_CHNL_OFFSET_LOWER)
                        SCSettingOf20 = VHT_DATA_SC_20_LOWER_OF_80MHZ;
        }
Quoted 2 lines

so if we are on a 40M channel some settings involving 80M are made and the whole is then written on card registers.

I'm no wifi expert, so I was hoping someone else would respond...

With that said I believe you should keep this else block, this part of the function seems to select the order of the bonded channels when bonding multiple 20MHz channels together.

Quoted 5 lines

The "if (pHalData->CurrentChannelBW == CHANNEL_WIDTH_80) {}" part can be removed because on 2.4G only devices 80 MHz width is not supported, but the 40MHz bit should stay, the constants for the register bits may be named after the 80MHz option, but I believe these same register bits will impact the 40Mhz case too.

agreed, just remove the CHANNEL_WIDTH_80 branch is good

Quoted 3 lines

It might be a good idea to rename the constants to VHT_DATA_SC_20_*_OF_40MHZ in a separate patch.

good idea, I was misguided by the trailing *_80MHZ in constant name

Quoted 5 lines

Regards,

Hans

thank you Hans,

fabio