On 2020-04-29 17:56:51 [+0800], yhchuang@xxxxxxxxxxx wrote:
> diff --git a/drivers/net/wireless/realtek/rtw88/rtw8723d.c b/drivers/net/wireless/realtek/rtw88/rtw8723d.c
> index 653cfa9445fc..4e6ee00697be 100644
> --- a/drivers/net/wireless/realtek/rtw88/rtw8723d.c
> +++ b/drivers/net/wireless/realtek/rtw88/rtw8723d.c
> @@ -287,6 +287,168 @@ static void rtw8723d_query_rx_desc(struct rtw_dev *rtwdev, u8 *rx_desc,
…
> +static void rtw8723d_cfg_notch(struct rtw_dev *rtwdev, u8 channel, bool notch)
> +{
> + if (!notch)
Would it make sense in pull in the code from the no_notch label up here
and avoid the goto?
> + goto no_notch;
> +
> + switch (channel) {
> + case 13:
> + rtw_write32_mask(rtwdev, REG_OFDM0_RXDSP, BIT_MASK_RXDSP, 0xB);
> + rtw_write32_mask(rtwdev, REG_OFDM0_RXDSP, BIT_EN_RXDSP, 0x1);
> + rtw_write32(rtwdev, REG_OFDM1_CSI1, 0x04000000);
> + rtw_write32(rtwdev, REG_OFDM1_CSI2, 0x00000000);
> + rtw_write32(rtwdev, REG_OFDM1_CSI3, 0x00000000);
> + rtw_write32(rtwdev, REG_OFDM1_CSI4, 0x00000000);
> + rtw_write32_mask(rtwdev, REG_OFDM1_CFOTRK, BIT_EN_CFOTRK, 0x1);
> + break;
> + case 14:
> + rtw_write32_mask(rtwdev, REG_OFDM0_RXDSP, BIT_MASK_RXDSP, 0x5);
> + rtw_write32_mask(rtwdev, REG_OFDM0_RXDSP, BIT_EN_RXDSP, 0x1);
> + rtw_write32(rtwdev, REG_OFDM1_CSI1, 0x00000000);
> + rtw_write32(rtwdev, REG_OFDM1_CSI2, 0x00000000);
> + rtw_write32(rtwdev, REG_OFDM1_CSI3, 0x00000000);
> + rtw_write32(rtwdev, REG_OFDM1_CSI4, 0x00080000);
> + rtw_write32_mask(rtwdev, REG_OFDM1_CFOTRK, BIT_EN_CFOTRK, 0x1);
> + break;
> + default:
> + rtw_write32_mask(rtwdev, REG_OFDM0_RXDSP, BIT_EN_RXDSP, 0x0);
> + rtw_write32_mask(rtwdev, REG_OFDM1_CFOTRK, BIT_EN_CFOTRK, 0x0);
> + break;
> + }
> +
> + return;
> +
> +no_notch:
> + rtw_write32_mask(rtwdev, REG_OFDM0_RXDSP, BIT_MASK_RXDSP, 0x1f);
> + rtw_write32_mask(rtwdev, REG_OFDM0_RXDSP, BIT_EN_RXDSP, 0x0);
> + rtw_write32(rtwdev, REG_OFDM1_CSI1, 0x00000000);
> + rtw_write32(rtwdev, REG_OFDM1_CSI2, 0x00000000);
> + rtw_write32(rtwdev, REG_OFDM1_CSI3, 0x00000000);
> + rtw_write32(rtwdev, REG_OFDM1_CSI4, 0x00000000);
> + rtw_write32_mask(rtwdev, REG_OFDM1_CFOTRK, BIT_EN_CFOTRK, 0x0);
> +}
> +
> +static void rtw8723d_spur_cal(struct rtw_dev *rtwdev, u8 channel)
> +{
> + bool notch = false;
> +
> + if (channel < 13)
> + goto do_notch;
if you reverse the if statement, then you could avoid the goto.
> +
> + notch = rtw8723d_check_spur_ov_thres(rtwdev, channel, SPUR_THRES);
> +
> +do_notch:
> + rtw8723d_cfg_notch(rtwdev, channel, notch);
> +}
> +
> +static void rtw8723d_set_channel_rf(struct rtw_dev *rtwdev, u8 channel, u8 bw)
> +{
> + u32 rf_cfgch[2];
Would it make sense to use rf_cfgch_A rf_cfgch_B instead the array?
> + rf_cfgch[0] = rtw_read_rf(rtwdev, RF_PATH_A, RF_CFGCH, RFREG_MASK);
> + rf_cfgch[1] = rtw_read_rf(rtwdev, RF_PATH_B, RF_CFGCH, RFREG_MASK);
> +
> + rf_cfgch[0] &= ~RFCFGCH_CHANNEL_MASK;
> + rf_cfgch[1] &= ~RFCFGCH_CHANNEL_MASK;
> + rf_cfgch[0] |= (channel & RFCFGCH_CHANNEL_MASK);
> + rf_cfgch[1] |= (channel & RFCFGCH_CHANNEL_MASK);
> +
> + rf_cfgch[0] &= ~RFCFGCH_BW_MASK;
> + switch (bw) {
> + case RTW_CHANNEL_WIDTH_20:
> + rf_cfgch[0] |= RFCFGCH_BW_20M;
> + break;
> + case RTW_CHANNEL_WIDTH_40:
> + rf_cfgch[0] |= RFCFGCH_BW_40M;
> + break;
> + default:
> + break;
> + }
> +
> + rtw_write_rf(rtwdev, RF_PATH_A, RF_CFGCH, RFREG_MASK, rf_cfgch[0]);
> + rtw_write_rf(rtwdev, RF_PATH_B, RF_CFGCH, RFREG_MASK, rf_cfgch[1]);
> +
> + rtw8723d_spur_cal(rtwdev, channel);
> +}
…
> +static void rtw8723d_set_channel_bb(struct rtw_dev *rtwdev, u8 channel, u8 bw,
> + u8 primary_ch_idx)
> +{
> + const struct rtw_backup_info *cck_dfir =
> + channel <= 13 ? cck_dfir_cfg[0] : cck_dfir_cfg[1];
> + int i;
If you move the assignment of `cck_dfir' here the definition block
would look a nicer.
> +
> + for (i = 0; i < CCK_DFIR_NR; i++, cck_dfir++)
> + rtw_write32(rtwdev, cck_dfir->reg, cck_dfir->val);
> +
> + switch (bw) {
> + case RTW_CHANNEL_WIDTH_20:
> + rtw_write32_mask(rtwdev, REG_FPGA0_RFMOD, BIT_MASK_RFMOD, 0x0);
> + rtw_write32_mask(rtwdev, REG_FPGA1_RFMOD, BIT_MASK_RFMOD, 0x0);
> + rtw_write32_mask(rtwdev, REG_BBRX_DFIR, BIT_RXBB_DFIR_EN, 1);
> + rtw_write32_mask(rtwdev, REG_BBRX_DFIR, BIT_MASK_RXBB_DFIR, 0xa);
> + break;
> + case RTW_CHANNEL_WIDTH_40:
> + rtw_write32_mask(rtwdev, REG_FPGA0_RFMOD, BIT_MASK_RFMOD, 0x1);
> + rtw_write32_mask(rtwdev, REG_FPGA1_RFMOD, BIT_MASK_RFMOD, 0x1);
> + rtw_write32_mask(rtwdev, REG_BBRX_DFIR, BIT_RXBB_DFIR_EN, 0);
> + rtw_write32_mask(rtwdev, REG_CCK0_SYS, BIT_CCK_SIDE_BAND,
> + (primary_ch_idx == RTW_SC_20_UPPER ? 1 : 0));
> + break;
> + default:
> + break;
> + }
> +}
Sebastian
