| Message ID | 20231218143645.433356-8-martin.kaistra@linutronix.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | wifi: rtl8xxxu: Add concurrent mode for 8188f | expand |
> -----Original Message----- > From: Martin Kaistra <martin.kaistra@linutronix.de> > Sent: Monday, December 18, 2023 10:37 PM > To: linux-wireless@vger.kernel.org > Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih > <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior > <bigeasy@linutronix.de> > Subject: [PATCH 07/20] wifi: rtl8xxxu: extend check for matching bssid to both interfaces > > The driver will support two interfaces soon, which both can be in > station mode, so extend the check, whether cfo information should be > parsed, to cover both interfaces. > > For better code readability put the lines with priv->vifs[port_num] in a > separate function. > > Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de> > --- > .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 22 ++++++++++++------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > index fd6b6e2eba038..c3039049e9f5b 100644 > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > @@ -5706,6 +5706,16 @@ static void rtl8xxxu_update_beacon_work_callback(struct work_struct *work) > rtl8xxxu_send_beacon_frame(hw, vif); > } > > +static inline bool rtl8xxxu_is_packet_match_bssid(struct rtl8xxxu_priv *priv, > + struct ieee80211_hdr *hdr, > + int port_num) > +{ > + return priv->vifs[port_num] && > + priv->vifs[port_num]->type == NL80211_IFTYPE_STATION && > + priv->vifs[port_num]->cfg.assoc && > + ether_addr_equal(priv->vifs[port_num]->bss_conf.bssid, hdr->addr2); nit: coding style: align "priv->vifs", like return priv->vifs[...] priv->vifs[port_num].... > +} > + > void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv, > struct ieee80211_rx_status *rx_status, > struct rtl8723au_phy_stats *phy_stats, > @@ -5722,12 +5732,10 @@ void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv, > rx_status->signal = priv->fops->cck_rssi(priv, phy_stats); > } 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); > + (rtl8xxxu_is_packet_match_bssid(priv, hdr, 0) || > + rtl8xxxu_is_packet_match_bssid(priv, hdr, 1)); I feel that driver can only track single one CFO (carrier frequency offset) from AP. Considering STA+STA case with two different APs, it would cause ping-pong CFO values between two APs. A simple way is just to ignore CFO for STA+STA case. Another way is to reference the methods implemented by rtw89 where function name is rtw89_phy_multi_sta_cfo_calc(). One method is to record CFO tail for each mac_id and use the average as target CFO value to hardware. Ping-Ke
Am 20.12.23 um 06:51 schrieb Ping-Ke Shih: > > >> -----Original Message----- >> From: Martin Kaistra <martin.kaistra@linutronix.de> >> Sent: Monday, December 18, 2023 10:37 PM >> To: linux-wireless@vger.kernel.org >> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ping-Ke Shih >> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebastian Andrzej Siewior >> <bigeasy@linutronix.de> >> Subject: [PATCH 07/20] wifi: rtl8xxxu: extend check for matching bssid to both interfaces >> >> The driver will support two interfaces soon, which both can be in >> station mode, so extend the check, whether cfo information should be >> parsed, to cover both interfaces. >> >> For better code readability put the lines with priv->vifs[port_num] in a >> separate function. >> >> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de> >> --- >> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 22 ++++++++++++------- >> 1 file changed, 14 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c >> index fd6b6e2eba038..c3039049e9f5b 100644 >> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c >> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c >> @@ -5706,6 +5706,16 @@ static void rtl8xxxu_update_beacon_work_callback(struct work_struct *work) >> rtl8xxxu_send_beacon_frame(hw, vif); >> } >> >> +static inline bool rtl8xxxu_is_packet_match_bssid(struct rtl8xxxu_priv *priv, >> + struct ieee80211_hdr *hdr, >> + int port_num) >> +{ >> + return priv->vifs[port_num] && >> + priv->vifs[port_num]->type == NL80211_IFTYPE_STATION && >> + priv->vifs[port_num]->cfg.assoc && >> + ether_addr_equal(priv->vifs[port_num]->bss_conf.bssid, hdr->addr2); > > nit: coding style: align "priv->vifs", like > > return priv->vifs[...] > priv->vifs[port_num].... > > >> +} >> + >> void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv, >> struct ieee80211_rx_status *rx_status, >> struct rtl8723au_phy_stats *phy_stats, >> @@ -5722,12 +5732,10 @@ void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv, >> rx_status->signal = priv->fops->cck_rssi(priv, phy_stats); >> } 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); >> + (rtl8xxxu_is_packet_match_bssid(priv, hdr, 0) || >> + rtl8xxxu_is_packet_match_bssid(priv, hdr, 1)); > > I feel that driver can only track single one CFO (carrier frequency offset) > from AP. Considering STA+STA case with two different APs, it would cause > ping-pong CFO values between two APs. > > A simple way is just to ignore CFO for STA+STA case. Another way is to > reference the methods implemented by rtw89 where function name is > rtw89_phy_multi_sta_cfo_calc(). One method is to record CFO tail for each > mac_id and use the average as target CFO value to hardware. This is only a problem, if both interfaces are in STA mode and are actually connected, right? Then, I will add a check and ignore CFO in that case. > > Ping-Ke >
On Wed, 2023-12-20 at 17:43 +0100, Martin Kaistra wrote: > > Am 20.12.23 um 06:51 schrieb Ping-Ke Shih: > > > > I feel that driver can only track single one CFO (carrier frequency offset) > > from AP. Considering STA+STA case with two different APs, it would cause > > ping-pong CFO values between two APs. > > > > A simple way is just to ignore CFO for STA+STA case. Another way is to > > reference the methods implemented by rtw89 where function name is > > rtw89_phy_multi_sta_cfo_calc(). One method is to record CFO tail for each > > mac_id and use the average as target CFO value to hardware. > > This is only a problem, if both interfaces are in STA mode and are actually > connected, right? Yes.
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c index fd6b6e2eba038..c3039049e9f5b 100644 --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c @@ -5706,6 +5706,16 @@ static void rtl8xxxu_update_beacon_work_callback(struct work_struct *work) rtl8xxxu_send_beacon_frame(hw, vif); } +static inline bool rtl8xxxu_is_packet_match_bssid(struct rtl8xxxu_priv *priv, + struct ieee80211_hdr *hdr, + int port_num) +{ + return priv->vifs[port_num] && + priv->vifs[port_num]->type == NL80211_IFTYPE_STATION && + priv->vifs[port_num]->cfg.assoc && + ether_addr_equal(priv->vifs[port_num]->bss_conf.bssid, hdr->addr2); +} + void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv, struct ieee80211_rx_status *rx_status, struct rtl8723au_phy_stats *phy_stats, @@ -5722,12 +5732,10 @@ void rtl8723au_rx_parse_phystats(struct rtl8xxxu_priv *priv, rx_status->signal = priv->fops->cck_rssi(priv, phy_stats); } 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); + (rtl8xxxu_is_packet_match_bssid(priv, hdr, 0) || + rtl8xxxu_is_packet_match_bssid(priv, hdr, 1)); if (parse_cfo) { priv->cfo_tracking.cfo_tail[0] = phy_stats->path_cfotail[0]; @@ -5762,12 +5770,10 @@ static void jaguar2_rx_parse_phystats_type1(struct rtl8xxxu_priv *priv, bool crc_icv_err) { 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); + (rtl8xxxu_is_packet_match_bssid(priv, hdr, 0) || + rtl8xxxu_is_packet_match_bssid(priv, hdr, 1)); u8 pwdb_max = 0; int rx_path;
The driver will support two interfaces soon, which both can be in station mode, so extend the check, whether cfo information should be parsed, to cover both interfaces. For better code readability put the lines with priv->vifs[port_num] in a separate function. Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de> --- .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-)