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 <[email protected]>
---
.../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);
+}
+
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;
--
2.39.2
> -----Original Message-----
> From: Martin Kaistra <[email protected]>
> Sent: Monday, December 18, 2023 10:37 PM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
> <[email protected]>
> 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 <[email protected]>
> ---
> .../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 <[email protected]>
>> Sent: Monday, December 18, 2023 10:37 PM
>> To: [email protected]
>> Cc: Jes Sorensen <[email protected]>; Kalle Valo <[email protected]>; Ping-Ke Shih
>> <[email protected]>; Bitterblue Smith <[email protected]>; Sebastian Andrzej Siewior
>> <[email protected]>
>> 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 <[email protected]>
>> ---
>> .../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.