RE: [PATCH 1/5] wifi: rtl8xxxu: Add central frequency offset tracking

[Ping-Ke Shih <pkshih@realtek.com>]

> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> Sent: Monday, October 17, 2022 1:27 AM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>
> Subject: [PATCH 1/5] wifi: rtl8xxxu: Add central frequency offset tracking
> 
> According to Realtek programmers, "to adjust oscillator to align
> central frequency of connected AP. Then, it can yield better
> performance." From commit fb8517f4fade ("rtw88: 8822c: add CFO
> tracking").
> 
> The RTL8192CU and a version of RTL8723AU apparently don't have the
> ability to adjust the oscillator, so this doesn't apply to them.
> 
> This also doesn't apply to the wifi + bluetooth combo chips (RTL8723AU
> and RTL8723BU) because the CFO tracking should only be done when
> bluetooth is disabled, and determining that looked complicated.
> 
> That leaves only the RTL8192EU and RTL8188FU chips. I tested this with
> the latter.
> 
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> ---
>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  21 ++-
>  .../realtek/rtl8xxxu/rtl8xxxu_8188f.c         |  35 ++++-
>  .../realtek/rtl8xxxu/rtl8xxxu_8192e.c         |   4 +-
>  .../realtek/rtl8xxxu/rtl8xxxu_8723a.c         |  35 ++++-
>  .../realtek/rtl8xxxu/rtl8xxxu_8723b.c         |   4 +-
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 143 ++++++++++++++++--
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h |   1 +
>  7 files changed, 211 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 1b9da71dc38d..14f0b3224553 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h


[...]

> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> index bb88bab7c72a..39ae4d971ff6 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> @@ -703,7 +703,7 @@ static int rtl8188fu_parse_efuse(struct rtl8xxxu_priv *priv)
>  	priv->ofdm_tx_power_diff[0].a = efuse->tx_power_index_A.ht20_ofdm_1s_diff.a;
>  	priv->ht20_tx_power_diff[0].a = efuse->tx_power_index_A.ht20_ofdm_1s_diff.b;
> 
> -	priv->xtalk = efuse->xtal_k & 0x3f;
> +	priv->default_crystal_cap = efuse->xtal_k & 0x3f;
> 
>  	dev_info(&priv->udev->dev, "Vendor: %.7s\n", efuse->vendor_name);
>  	dev_info(&priv->udev->dev, "Product: %.7s\n", efuse->device_name);
> @@ -737,7 +737,6 @@ static void rtl8188fu_init_phy_bb(struct rtl8xxxu_priv *priv)
>  {
>  	u8 val8;
>  	u16 val16;
> -	u32 val32;
> 
>  	/* Enable BB and RF */
>  	val16 = rtl8xxxu_read16(priv, REG_SYS_FUNC);
> @@ -759,12 +758,6 @@ static void rtl8188fu_init_phy_bb(struct rtl8xxxu_priv *priv)
> 
>  	rtl8xxxu_init_phy_regs(priv, rtl8188fu_phy_init_table);
>  	rtl8xxxu_init_phy_regs(priv, rtl8188f_agc_table);
> -
> -	val32 = rtl8xxxu_read32(priv, REG_AFE_XTAL_CTRL);
> -	val8 = priv->xtalk;
> -	val32 &= ~0x007FF800;
> -	val32 |= ((val8 | (val8 << 6)) << 11);
> -	rtl8xxxu_write32(priv, REG_AFE_XTAL_CTRL, val32);
>  }
> 
>  static int rtl8188fu_init_phy_rf(struct rtl8xxxu_priv *priv)
> @@ -1636,6 +1629,31 @@ static void rtl8188f_usb_quirks(struct rtl8xxxu_priv *priv)
>  	rtl8xxxu_write32(priv, REG_TXDMA_OFFSET_CHK, val32);
>  }
> 
> +static void rtl8188f_set_crystal_cap(struct rtl8xxxu_priv *priv, u8 crystal_cap)
> +{
> +	struct rtl8xxxu_cfo_tracking *cfo = &priv->cfo_tracking;
> +	u32 val32;
> +
> +	if (crystal_cap == cfo->crystal_cap)
> +		return;
> +
> +	val32 = rtl8xxxu_read32(priv, REG_AFE_XTAL_CTRL);
> +
> +	dev_dbg(&priv->udev->dev,
> +	        "%s: Adjusting crystal cap from 0x%x (actually 0x%x 0x%x) to 0x%x\n",
> +	        __func__,
> +	        cfo->crystal_cap,
> +	        (val32 & 0x007e0000) >> 17,
> +	        (val32 & 0x0001f800) >> 11,
> +	        crystal_cap);
> +
> +	val32 &= ~0x007ff800;
> +	val32 |= (crystal_cap | (crystal_cap << 6)) << 11;
> +	rtl8xxxu_write32(priv, REG_AFE_XTAL_CTRL, val32);

This could be clear:

#define XTAL1 GENMASK(22, 17)
#define XTAL0 GENMASK(16, 11)

val32 &= ~(XTAL1 | XTAL2)
val32 |= FIELD_PREP(XTAL1, crystal_cap) |
         FIELD_PREP(XTAL0, crystal_cap);

> +
> +	cfo->crystal_cap = crystal_cap;
> +}
> +
>  struct rtl8xxxu_fileops rtl8188fu_fops = {
>  	.parse_efuse = rtl8188fu_parse_efuse,
>  	.load_firmware = rtl8188fu_load_firmware,
> @@ -1660,6 +1678,7 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
>  	.update_rate_mask = rtl8xxxu_gen2_update_rate_mask,
>  	.report_connect = rtl8xxxu_gen2_report_connect,
>  	.fill_txdesc = rtl8xxxu_fill_txdesc_v2,
> +	.set_crystal_cap = rtl8188f_set_crystal_cap,
>  	.writeN_block_size = 128,
>  	.rx_desc_size = sizeof(struct rtl8xxxu_rxdesc24),
>  	.tx_desc_size = sizeof(struct rtl8xxxu_txdesc40),

[...]

> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 33a8ee545113..947916363e3f 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -2286,7 +2286,6 @@ void rtl8xxxu_gen1_init_phy_bb(struct rtl8xxxu_priv *priv)
>   */
>  static int rtl8xxxu_init_phy_bb(struct rtl8xxxu_priv *priv)
>  {
> -	u8 val8;
>  	u32 val32;
> 
>  	priv->fops->init_phy_bb(priv);
> @@ -2351,15 +2350,8 @@ static int rtl8xxxu_init_phy_bb(struct rtl8xxxu_priv *priv)
>  		rtl8xxxu_write32(priv, REG_TX_TO_TX, val32);
>  	}
> 
> -	if (priv->has_xtalk) {
> -		val32 = rtl8xxxu_read32(priv, REG_MAC_PHY_CTRL);
> -
> -		val8 = priv->xtalk;
> -		val32 &= 0xff000fff;
> -		val32 |= ((val8 | (val8 << 6)) << 12);
> -
> -		rtl8xxxu_write32(priv, REG_MAC_PHY_CTRL, val32);
> -	}
> +	if (priv->fops->set_crystal_cap)
> +		priv->fops->set_crystal_cap(priv, priv->default_crystal_cap);
> 
>  	if (priv->rtl_chip == RTL8192E)
>  		rtl8xxxu_write32(priv, REG_AFE_XTAL_CTRL, 0x000f81fb);
> @@ -4334,6 +4326,15 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>  		val32 |= 0x0007e000;
>  		rtl8xxxu_write32(priv, REG_AFE_MISC, val32);
>  	}
> +
> +	/* Initialise the center frequency offset tracking */
> +	if (priv->fops->set_crystal_cap) {
> +		val32 = rtl8xxxu_read32(priv, REG_OFDM1_CFO_TRACKING);
> +		priv->cfo_tracking.atc_status = val32 & CFO_TRACKING_ATC_STATUS;
> +		priv->cfo_tracking.adjust = true;
> +		priv->cfo_tracking.crystal_cap = priv->default_crystal_cap;
> +	}
> +
>  exit:
>  	return ret;
>  }
> @@ -5301,7 +5302,8 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
>  static void rtl8xxxu_rx_parse_phystats(struct rtl8xxxu_priv *priv,
>  				       struct ieee80211_rx_status *rx_status,
>  				       struct rtl8723au_phy_stats *phy_stats,
> -				       u32 rxmcs)
> +				       u32 rxmcs, struct ieee80211_hdr *hdr,
> +				       bool crc_icv_err)
>  {
>  	if (phy_stats->sgi_en)
>  		rx_status->enc_flags |= RX_ENC_FLAG_SHORT_GI;
> @@ -5327,6 +5329,24 @@ static void rtl8xxxu_rx_parse_phystats(struct rtl8xxxu_priv *priv,
>  			break;
>  		}
>  	} else {
> +		bool parse_cfo = priv->fops->set_crystal_cap &&
> +				 priv->vif &&
> +				 priv->vif->type == NL80211_IFTYPE_STATION &&
> +				 priv->vif->cfg.assoc &&
> +				 !crc_icv_err &&
> +				 !ieee80211_is_ctl(hdr->frame_control) &&
> +				 ether_addr_equal(priv->vif->bss_conf.bssid, hdr->addr2);
> +
> +		if (parse_cfo) {
> +			priv->cfo_tracking.cfo_tail[0] = phy_stats->path_cfotail[0];
> +			priv->cfo_tracking.cfo_tail[1] = phy_stats->path_cfotail[1];
> +
> +			if (priv->cfo_tracking.packet_count == 0xffffffff)
> +				priv->cfo_tracking.packet_count = 0;
> +			else
> +				priv->cfo_tracking.packet_count++;

'packet_count' is u32, so 0xffffffff + 1 will become 0. Then, just

   priv->cfo_tracking.packet_count++;

> +		}
> +
>  		rx_status->signal =
>  			(phy_stats->cck_sig_qual_ofdm_pwdb_all >> 1) - 110;
>  	}

[...]

> @@ -6495,6 +6517,97 @@ static void rtl8xxxu_refresh_rate_mask(struct rtl8xxxu_priv *priv,
>  	}
>  }
> 
> +static void rtl8xxxu_set_atc_status(struct rtl8xxxu_priv *priv, bool atc_status)
> +{
> +	struct rtl8xxxu_cfo_tracking *cfo = &priv->cfo_tracking;
> +	u32 val32;
> +
> +	if (atc_status == cfo->atc_status)
> +		return;
> +
> +	cfo->atc_status = atc_status;
> +
> +	val32 = rtl8xxxu_read32(priv, REG_OFDM1_CFO_TRACKING);
> +	if (atc_status)
> +		val32 |= CFO_TRACKING_ATC_STATUS;
> +	else
> +		val32 &= ~CFO_TRACKING_ATC_STATUS;
> +	rtl8xxxu_write32(priv, REG_OFDM1_CFO_TRACKING, val32);
> +}
> +
> +/* Central frequency offset correction */
> +static void rtl8xxxu_track_cfo(struct rtl8xxxu_priv *priv)
> +{
> +	struct rtl8xxxu_cfo_tracking *cfo = &priv->cfo_tracking;
> +	int cfo_khz_a, cfo_khz_b, cfo_average;
> +	int crystal_cap;
> +
> +	if (!priv->vif || !priv->vif->cfg.assoc) {
> +		/* Reset */
> +		cfo->adjust = true;
> +
> +		if (cfo->crystal_cap > priv->default_crystal_cap)
> +			priv->fops->set_crystal_cap(priv, cfo->crystal_cap - 1);
> +		else if (cfo->crystal_cap < priv->default_crystal_cap)
> +			priv->fops->set_crystal_cap(priv, cfo->crystal_cap + 1);
> +
> +		rtl8xxxu_set_atc_status(priv, true);
> +
> +		return;
> +	}
> +
> +	if (cfo->packet_count == cfo->packet_count_pre)
> +		/* No new information. */
> +		return;
> +
> +	cfo->packet_count_pre = cfo->packet_count;
> +
> +	/* CFO_tail[1:0] is S(8,7), (num_subcarrier>>7) x 312.5K = CFO value(K Hz) */
> +	cfo_khz_a = (int)((cfo->cfo_tail[0] * 3125) / 10) >> 7;
> +	cfo_khz_b = (int)((cfo->cfo_tail[1] * 3125) / 10) >> 7;
> +
> +	if (priv->tx_paths == 1)
> +		cfo_average = cfo_khz_a;
> +	else
> +		cfo_average = (cfo_khz_a + cfo_khz_b) / 2;
> +
> +	dev_dbg(&priv->udev->dev, "cfo_average: %d\n", cfo_average);
> +
> +	if (cfo->adjust) {
> +		if (abs(cfo_average) < CFO_TH_XTAL_LOW)
> +			cfo->adjust = false;
> +	} else {
> +		if (abs(cfo_average) > CFO_TH_XTAL_HIGH)
> +			cfo->adjust = true;
> +	}
> +
> +	/*
> +	 * TODO: We should return here only if bluetooth is enabled.
> +	 * See the vendor drivers for how to determine that.
> +	 */
> +	if (priv->has_bluetooth)
> +		return;
> +
> +	if (!cfo->adjust)
> +		return;
> +
> +	crystal_cap = cfo->crystal_cap;
> +
> +	if (cfo_average > CFO_TH_XTAL_LOW)
> +		crystal_cap++;
> +	else if (cfo_average < -CFO_TH_XTAL_LOW)
> +		crystal_cap--;
> +
> +	if (crystal_cap > 0x3f)
> +		crystal_cap = 0x3f;
> +	else if (crystal_cap < 0)
> +		crystal_cap = 0;

crystal_cap = clamp(crystal_cap, 0, 0x3f);

> +
> +	priv->fops->set_crystal_cap(priv, crystal_cap);
> +
> +	rtl8xxxu_set_atc_status(priv, abs(cfo_average) >= CFO_TH_ATC);
> +}
> +
>  static void rtl8xxxu_watchdog_callback(struct work_struct *work)
>  {
>  	struct ieee80211_vif *vif;
> @@ -6519,6 +6632,10 @@ static void rtl8xxxu_watchdog_callback(struct work_struct *work)
>  		rcu_read_unlock();
> 
>  		signal = ieee80211_ave_rssi(vif);
> +
> +		if (priv->fops->set_crystal_cap)
> +			rtl8xxxu_track_cfo(priv);
> +
>  		rtl8xxxu_refresh_rate_mask(priv, signal, sta);
>  	}
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
> index 35bde1404793..190bc0e8dc33 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_regs.h
> @@ -1027,6 +1027,7 @@
> 
>  #define REG_OFDM1_TRX_PATH_ENABLE	0x0d04
>  #define REG_OFDM1_CFO_TRACKING		0x0d2c
> +#define  CFO_TRACKING_ATC_STATUS	BIT(11)
>  #define REG_OFDM1_CSI_FIX_MASK1		0x0d40
>  #define REG_OFDM1_CSI_FIX_MASK2		0x0d44
> 
> --
> 2.38.0
> 
> ------Please consider the environment before printing this e-mail.