2021-09-14 16:38:26

by Jouni Malinen

[permalink] [raw]
Subject: [PATCH 1/3] ath11k: Change number of TCL rings to one for QCA6390

From: Baochen Qiang <[email protected]>

Some targets, QCA6390 for example, use only one TCL ring,
it is better to initialize only one ring and leave others
untouched for such targets.

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1

Signed-off-by: Baochen Qiang <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
drivers/net/wireless/ath/ath11k/core.c | 10 +++++-----
drivers/net/wireless/ath/ath11k/debugfs.c | 2 +-
drivers/net/wireless/ath/ath11k/dp.c | 10 +++++-----
drivers/net/wireless/ath/ath11k/dp.h | 1 +
drivers/net/wireless/ath/ath11k/dp_tx.c | 13 +++++--------
drivers/net/wireless/ath/ath11k/hw.h | 2 +-
drivers/net/wireless/ath/ath11k/mac.c | 2 +-
7 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index a8c6f7cf33d5..af76e37d11ae 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -58,7 +58,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.rx_mac_buf_ring = false,
.vdev_start_delay = false,
.htt_peer_map_v2 = true,
- .tcl_0_only = false,
.spectral = {
.fft_sz = 2,
/* HW bug, expected BIN size is 2 bytes but HW report as 4 bytes.
@@ -82,6 +81,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.hal_desc_sz = sizeof(struct hal_rx_desc_ipq8074),
.fix_l1ss = true,
.sram_dump = NULL,
+ .max_tx_ring = DP_TCL_NUM_RING_MAX,
},
{
.hw_rev = ATH11K_HW_IPQ6018_HW10,
@@ -110,7 +110,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.rx_mac_buf_ring = false,
.vdev_start_delay = false,
.htt_peer_map_v2 = true,
- .tcl_0_only = false,
.spectral = {
.fft_sz = 4,
.fft_pad_sz = 0,
@@ -131,6 +130,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.hal_desc_sz = sizeof(struct hal_rx_desc_ipq8074),
.fix_l1ss = true,
.sram_dump = NULL,
+ .max_tx_ring = DP_TCL_NUM_RING_MAX,
},
{
.name = "qca6390 hw2.0",
@@ -159,7 +159,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.rx_mac_buf_ring = true,
.vdev_start_delay = true,
.htt_peer_map_v2 = false,
- .tcl_0_only = true,
.spectral = {
.fft_sz = 0,
.fft_pad_sz = 0,
@@ -179,6 +178,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.hal_desc_sz = sizeof(struct hal_rx_desc_ipq8074),
.fix_l1ss = true,
.sram_dump = &sram_dump_qca6390,
+ .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
},
{
.name = "qcn9074 hw1.0",
@@ -206,7 +206,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.rx_mac_buf_ring = false,
.vdev_start_delay = false,
.htt_peer_map_v2 = true,
- .tcl_0_only = false,
.spectral = {
.fft_sz = 2,
.fft_pad_sz = 0,
@@ -227,6 +226,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.hal_desc_sz = sizeof(struct hal_rx_desc_qcn9074),
.fix_l1ss = true,
.sram_dump = NULL,
+ .max_tx_ring = DP_TCL_NUM_RING_MAX,
},
{
.name = "wcn6855 hw2.0",
@@ -255,7 +255,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.rx_mac_buf_ring = true,
.vdev_start_delay = true,
.htt_peer_map_v2 = false,
- .tcl_0_only = true,
.spectral = {
.fft_sz = 0,
.fft_pad_sz = 0,
@@ -274,6 +273,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.hal_desc_sz = sizeof(struct hal_rx_desc_wcn6855),
.fix_l1ss = false,
.sram_dump = &sram_dump_wcn6855,
+ .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
},
};

diff --git a/drivers/net/wireless/ath/ath11k/debugfs.c b/drivers/net/wireless/ath/ath11k/debugfs.c
index 871445f4e96a..0b42df65e4dc 100644
--- a/drivers/net/wireless/ath/ath11k/debugfs.c
+++ b/drivers/net/wireless/ath/ath11k/debugfs.c
@@ -807,7 +807,7 @@ static ssize_t ath11k_debugfs_dump_soc_dp_stats(struct file *file,
len += scnprintf(buf + len, size - len, "\nSOC TX STATS:\n");
len += scnprintf(buf + len, size - len, "\nTCL Ring Full Failures:\n");

- for (i = 0; i < DP_TCL_NUM_RING_MAX; i++)
+ for (i = 0; i < ab->hw_params.max_tx_ring; i++)
len += scnprintf(buf + len, size - len, "ring%d: %u\n",
i, soc_stats->tx_err.desc_na[i]);

diff --git a/drivers/net/wireless/ath/ath11k/dp.c b/drivers/net/wireless/ath/ath11k/dp.c
index dd69ba8f7d80..f7049a13973d 100644
--- a/drivers/net/wireless/ath/ath11k/dp.c
+++ b/drivers/net/wireless/ath/ath11k/dp.c
@@ -311,7 +311,7 @@ void ath11k_dp_stop_shadow_timers(struct ath11k_base *ab)
if (!ab->hw_params.supports_shadow_regs)
return;

- for (i = 0; i < DP_TCL_NUM_RING_MAX; i++)
+ for (i = 0; i < ab->hw_params.max_tx_ring; i++)
ath11k_dp_shadow_stop_timer(ab, &ab->dp.tx_ring_timer[i]);

ath11k_dp_shadow_stop_timer(ab, &ab->dp.reo_cmd_timer);
@@ -326,7 +326,7 @@ static void ath11k_dp_srng_common_cleanup(struct ath11k_base *ab)
ath11k_dp_srng_cleanup(ab, &dp->wbm_desc_rel_ring);
ath11k_dp_srng_cleanup(ab, &dp->tcl_cmd_ring);
ath11k_dp_srng_cleanup(ab, &dp->tcl_status_ring);
- for (i = 0; i < DP_TCL_NUM_RING_MAX; i++) {
+ for (i = 0; i < ab->hw_params.max_tx_ring; i++) {
ath11k_dp_srng_cleanup(ab, &dp->tx_ring[i].tcl_data_ring);
ath11k_dp_srng_cleanup(ab, &dp->tx_ring[i].tcl_comp_ring);
}
@@ -366,7 +366,7 @@ static int ath11k_dp_srng_common_setup(struct ath11k_base *ab)
goto err;
}

- for (i = 0; i < DP_TCL_NUM_RING_MAX; i++) {
+ for (i = 0; i < ab->hw_params.max_tx_ring; i++) {
ret = ath11k_dp_srng_setup(ab, &dp->tx_ring[i].tcl_data_ring,
HAL_TCL_DATA, i, 0,
DP_TCL_DATA_RING_SIZE);
@@ -996,7 +996,7 @@ void ath11k_dp_free(struct ath11k_base *ab)

ath11k_dp_reo_cmd_list_cleanup(ab);

- for (i = 0; i < DP_TCL_NUM_RING_MAX; i++) {
+ for (i = 0; i < ab->hw_params.max_tx_ring; i++) {
spin_lock_bh(&dp->tx_ring[i].tx_idr_lock);
idr_for_each(&dp->tx_ring[i].txbuf_idr,
ath11k_dp_tx_pending_cleanup, ab);
@@ -1047,7 +1047,7 @@ int ath11k_dp_alloc(struct ath11k_base *ab)

size = sizeof(struct hal_wbm_release_ring) * DP_TX_COMP_RING_SIZE;

- for (i = 0; i < DP_TCL_NUM_RING_MAX; i++) {
+ for (i = 0; i < ab->hw_params.max_tx_ring; i++) {
idr_init(&dp->tx_ring[i].txbuf_idr);
spin_lock_init(&dp->tx_ring[i].tx_idr_lock);
dp->tx_ring[i].tcl_data_ring_id = i;
diff --git a/drivers/net/wireless/ath/ath11k/dp.h b/drivers/net/wireless/ath/ath11k/dp.h
index 36ef85e9515a..64c90cfd9c38 100644
--- a/drivers/net/wireless/ath/ath11k/dp.h
+++ b/drivers/net/wireless/ath/ath11k/dp.h
@@ -193,6 +193,7 @@ struct ath11k_pdev_dp {
#define DP_BA_WIN_SZ_MAX 256

#define DP_TCL_NUM_RING_MAX 3
+#define DP_TCL_NUM_RING_MAX_QCA6390 1

#define DP_IDLE_SCATTER_BUFS_MAX 16

diff --git a/drivers/net/wireless/ath/ath11k/dp_tx.c b/drivers/net/wireless/ath/ath11k/dp_tx.c
index 125a9a8a5b0a..6ef9b5d0b11a 100644
--- a/drivers/net/wireless/ath/ath11k/dp_tx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
@@ -115,11 +115,8 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,

tcl_ring_sel:
tcl_ring_retry = false;
- /* For some chip, it can only use tcl0 to tx */
- if (ar->ab->hw_params.tcl_0_only)
- ti.ring_id = 0;
- else
- ti.ring_id = ring_selector % DP_TCL_NUM_RING_MAX;
+
+ ti.ring_id = ring_selector % ab->hw_params.max_tx_ring;

ring_map |= BIT(ti.ring_id);

@@ -131,7 +128,7 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
spin_unlock_bh(&tx_ring->tx_idr_lock);

if (ret < 0) {
- if (ring_map == (BIT(DP_TCL_NUM_RING_MAX) - 1)) {
+ if (ring_map == (BIT(ab->hw_params.max_tx_ring) - 1)) {
atomic_inc(&ab->soc_stats.tx_err.misc_fail);
return -ENOSPC;
}
@@ -248,8 +245,8 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
* checking this ring earlier for each pkt tx.
* Restart ring selection if some rings are not checked yet.
*/
- if (ring_map != (BIT(DP_TCL_NUM_RING_MAX) - 1) &&
- !ar->ab->hw_params.tcl_0_only) {
+ if (ring_map != (BIT(ab->hw_params.max_tx_ring) - 1) &&
+ ab->hw_params.max_tx_ring > 1) {
tcl_ring_retry = true;
ring_selector++;
}
diff --git a/drivers/net/wireless/ath/ath11k/hw.h b/drivers/net/wireless/ath/ath11k/hw.h
index 484c0bcec86d..6e68c5b60eaa 100644
--- a/drivers/net/wireless/ath/ath11k/hw.h
+++ b/drivers/net/wireless/ath/ath11k/hw.h
@@ -158,7 +158,6 @@ struct ath11k_hw_params {
bool rx_mac_buf_ring;
bool vdev_start_delay;
bool htt_peer_map_v2;
- bool tcl_0_only;

struct {
u8 fft_sz;
@@ -179,6 +178,7 @@ struct ath11k_hw_params {
u32 hal_desc_sz;
bool fix_l1ss;
const struct ath11k_hw_sram_dump *sram_dump;
+ u8 max_tx_ring;
};

struct ath11k_hw_ops {
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index 155ca6af1b45..1f4765e43546 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -5731,7 +5731,7 @@ static void ath11k_mac_op_remove_interface(struct ieee80211_hw *hw,
idr_for_each(&ar->txmgmt_idr,
ath11k_mac_vif_txmgmt_idr_remove, vif);

- for (i = 0; i < DP_TCL_NUM_RING_MAX; i++) {
+ for (i = 0; i < ab->hw_params.max_tx_ring; i++) {
spin_lock_bh(&ab->dp.tx_ring[i].tx_idr_lock);
idr_for_each(&ab->dp.tx_ring[i].txbuf_idr,
ath11k_mac_vif_unref, vif);
--
2.25.1


2021-09-14 16:38:38

by Jouni Malinen

[permalink] [raw]
Subject: [PATCH 3/3] ath11k: set correct NL80211_FEATURE_DYNAMIC_SMPS for WCN6855

From: Wen Gong <[email protected]>

Commit "ath11k: support SMPS configuration for 6 GHz" changed "if
(ht_cap & WMI_HT_CAP_DYNAMIC_SMPS)" to "if (ht_cap &
WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)" which means
NL80211_FEATURE_DYNAMIC_SMPS is enabled for all chips which support 6
GHz. However, WCN6855 supports 6 GHz but it does not support feature
NL80211_FEATURE_DYNAMIC_SMPS, and this can lead to MU-MIMO test failures
for WCN6855.

Disable NL80211_FEATURE_DYNAMIC_SMPS for WCN6855 since its ht_cap does
not support WMI_HT_CAP_DYNAMIC_SMPS.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1

Signed-off-by: Wen Gong <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
drivers/net/wireless/ath/ath11k/core.c | 1 +
drivers/net/wireless/ath/ath11k/hw.h | 1 +
drivers/net/wireless/ath/ath11k/mac.c | 3 ++-
3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index 265ff225bd81..2bae8c5184d4 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -279,6 +279,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.sram_dump = &sram_dump_wcn6855,
.max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
.hal_params = &ath11k_hal_params_qca6390,
+ .check_dynamic_smps = true,
},
};

diff --git a/drivers/net/wireless/ath/ath11k/hw.h b/drivers/net/wireless/ath/ath11k/hw.h
index c6831fb110ba..7463b96770b7 100644
--- a/drivers/net/wireless/ath/ath11k/hw.h
+++ b/drivers/net/wireless/ath/ath11k/hw.h
@@ -180,6 +180,7 @@ struct ath11k_hw_params {
const struct ath11k_hw_sram_dump *sram_dump;
u8 max_tx_ring;
const struct hal_param *hal_params;
+ bool check_dynamic_smps;
};

struct ath11k_hw_ops {
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index 1f4765e43546..97a2c92b7b9b 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -7570,7 +7570,8 @@ static int __ath11k_mac_register(struct ath11k *ar)
* for each band for a dual band capable radio. It will be tricky to
* handle it when the ht capability different for each band.
*/
- if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)
+ if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS ||
+ (ar->supports_6ghz && !ab->hw_params.check_dynamic_smps))
ar->hw->wiphy->features |= NL80211_FEATURE_DYNAMIC_SMPS;

ar->hw->wiphy->max_scan_ssids = WLAN_SCAN_PARAMS_MAX_SSID;
--
2.25.1

2021-09-14 16:38:50

by Jouni Malinen

[permalink] [raw]
Subject: [PATCH 2/3] ath11k: change return buffer manager for QCA6390

From: Baochen Qiang <[email protected]>

QCA6390 firmware uses HAL_RX_BUF_RBM_SW1_BM, not HAL_RX_BUF_RBM_SW3_BM.

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1

Signed-off-by: Baochen Qiang <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
drivers/net/wireless/ath/ath11k/core.c | 5 +++++
drivers/net/wireless/ath/ath11k/dp.c | 2 +-
drivers/net/wireless/ath/ath11k/dp_rx.c | 22 +++++++++++-----------
drivers/net/wireless/ath/ath11k/hal.c | 8 ++++++++
drivers/net/wireless/ath/ath11k/hal.h | 7 +++++++
drivers/net/wireless/ath/ath11k/hal_rx.c | 2 +-
drivers/net/wireless/ath/ath11k/hw.c | 3 ++-
drivers/net/wireless/ath/ath11k/hw.h | 1 +
8 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index af76e37d11ae..265ff225bd81 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -82,6 +82,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.fix_l1ss = true,
.sram_dump = NULL,
.max_tx_ring = DP_TCL_NUM_RING_MAX,
+ .hal_params = &ath11k_hal_params_ipq8074,
},
{
.hw_rev = ATH11K_HW_IPQ6018_HW10,
@@ -131,6 +132,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.fix_l1ss = true,
.sram_dump = NULL,
.max_tx_ring = DP_TCL_NUM_RING_MAX,
+ .hal_params = &ath11k_hal_params_ipq8074,
},
{
.name = "qca6390 hw2.0",
@@ -179,6 +181,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.fix_l1ss = true,
.sram_dump = &sram_dump_qca6390,
.max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
+ .hal_params = &ath11k_hal_params_qca6390,
},
{
.name = "qcn9074 hw1.0",
@@ -227,6 +230,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.fix_l1ss = true,
.sram_dump = NULL,
.max_tx_ring = DP_TCL_NUM_RING_MAX,
+ .hal_params = &ath11k_hal_params_ipq8074,
},
{
.name = "wcn6855 hw2.0",
@@ -274,6 +278,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.fix_l1ss = false,
.sram_dump = &sram_dump_wcn6855,
.max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
+ .hal_params = &ath11k_hal_params_qca6390,
},
};

diff --git a/drivers/net/wireless/ath/ath11k/dp.c b/drivers/net/wireless/ath/ath11k/dp.c
index f7049a13973d..4a4f30e933a3 100644
--- a/drivers/net/wireless/ath/ath11k/dp.c
+++ b/drivers/net/wireless/ath/ath11k/dp.c
@@ -822,7 +822,7 @@ int ath11k_dp_service_srng(struct ath11k_base *ab,
struct dp_rxdma_ring *rx_ring = &dp->rx_refill_buf_ring;

ath11k_dp_rxbufs_replenish(ab, id, rx_ring, 0,
- HAL_RX_BUF_RBM_SW3_BM);
+ ab->hw_params.hal_params->rx_buf_rbm);
}
}
}
diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index c50f70913583..913e8a9878eb 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -499,7 +499,7 @@ static int ath11k_dp_rxdma_ring_buf_setup(struct ath11k *ar,

rx_ring->bufs_max = num_entries;
ath11k_dp_rxbufs_replenish(ar->ab, dp->mac_id, rx_ring, num_entries,
- HAL_RX_BUF_RBM_SW3_BM);
+ ar->ab->hw_params.hal_params->rx_buf_rbm);
return 0;
}

@@ -2756,7 +2756,7 @@ int ath11k_dp_process_rx(struct ath11k_base *ab, int ring_id,
rx_ring = &ar->dp.rx_refill_buf_ring;

ath11k_dp_rxbufs_replenish(ab, i, rx_ring, num_buffs_reaped[i],
- HAL_RX_BUF_RBM_SW3_BM);
+ ab->hw_params.hal_params->rx_buf_rbm);
}

ath11k_dp_rx_process_received_packets(ab, napi, &msdu_list,
@@ -3070,7 +3070,7 @@ static int ath11k_dp_rx_reap_mon_status_ring(struct ath11k_base *ab, int mac_id,

if (!skb) {
ath11k_hal_rx_buf_addr_info_set(rx_mon_status_desc, 0, 0,
- HAL_RX_BUF_RBM_SW3_BM);
+ ab->hw_params.hal_params->rx_buf_rbm);
num_buffs_reaped++;
break;
}
@@ -3080,7 +3080,7 @@ static int ath11k_dp_rx_reap_mon_status_ring(struct ath11k_base *ab, int mac_id,
FIELD_PREP(DP_RXDMA_BUF_COOKIE_BUF_ID, buf_id);

ath11k_hal_rx_buf_addr_info_set(rx_mon_status_desc, rxcb->paddr,
- cookie, HAL_RX_BUF_RBM_SW3_BM);
+ cookie, ab->hw_params.hal_params->rx_buf_rbm);
ath11k_hal_srng_src_get_next_entry(ab, srng);
num_buffs_reaped++;
}
@@ -3469,7 +3469,7 @@ static int ath11k_dp_rx_h_defrag_reo_reinject(struct ath11k *ar, struct dp_rx_ti
cookie = FIELD_PREP(DP_RXDMA_BUF_COOKIE_PDEV_ID, dp->mac_id) |
FIELD_PREP(DP_RXDMA_BUF_COOKIE_BUF_ID, buf_id);

- ath11k_hal_rx_buf_addr_info_set(msdu0, paddr, cookie, HAL_RX_BUF_RBM_SW3_BM);
+ ath11k_hal_rx_buf_addr_info_set(msdu0, paddr, cookie, ab->hw_params.hal_params->rx_buf_rbm);

/* Fill mpdu details into reo entrace ring */
srng = &ab->hal.srng_list[ab->dp.reo_reinject_ring.ring_id];
@@ -3846,7 +3846,7 @@ int ath11k_dp_process_rx_err(struct ath11k_base *ab, struct napi_struct *napi,
ath11k_hal_rx_msdu_link_info_get(link_desc_va, &num_msdus, msdu_cookies,
&rbm);
if (rbm != HAL_RX_BUF_RBM_WBM_IDLE_DESC_LIST &&
- rbm != HAL_RX_BUF_RBM_SW3_BM) {
+ rbm != ab->hw_params.hal_params->rx_buf_rbm) {
ab->soc_stats.invalid_rbm++;
ath11k_warn(ab, "invalid return buffer manager %d\n", rbm);
ath11k_dp_rx_link_desc_return(ab, desc,
@@ -3902,7 +3902,7 @@ int ath11k_dp_process_rx_err(struct ath11k_base *ab, struct napi_struct *napi,
rx_ring = &ar->dp.rx_refill_buf_ring;

ath11k_dp_rxbufs_replenish(ab, i, rx_ring, n_bufs_reaped[i],
- HAL_RX_BUF_RBM_SW3_BM);
+ ab->hw_params.hal_params->rx_buf_rbm);
}

return tot_n_bufs_reaped;
@@ -4198,7 +4198,7 @@ int ath11k_dp_rx_process_wbm_err(struct ath11k_base *ab,
rx_ring = &ar->dp.rx_refill_buf_ring;

ath11k_dp_rxbufs_replenish(ab, i, rx_ring, num_buffs_reaped[i],
- HAL_RX_BUF_RBM_SW3_BM);
+ ab->hw_params.hal_params->rx_buf_rbm);
}

rcu_read_lock();
@@ -4307,7 +4307,7 @@ int ath11k_dp_process_rxdma_err(struct ath11k_base *ab, int mac_id, int budget)

if (num_buf_freed)
ath11k_dp_rxbufs_replenish(ab, mac_id, rx_ring, num_buf_freed,
- HAL_RX_BUF_RBM_SW3_BM);
+ ab->hw_params.hal_params->rx_buf_rbm);

return budget - quota;
}
@@ -5101,12 +5101,12 @@ static void ath11k_dp_rx_mon_dest_process(struct ath11k *ar, int mac_id,
ath11k_dp_rxbufs_replenish(ar->ab, dp->mac_id,
&dp->rxdma_mon_buf_ring,
rx_bufs_used,
- HAL_RX_BUF_RBM_SW3_BM);
+ ar->ab->hw_params.hal_params->rx_buf_rbm);
else
ath11k_dp_rxbufs_replenish(ar->ab, dp->mac_id,
&dp->rx_refill_buf_ring,
rx_bufs_used,
- HAL_RX_BUF_RBM_SW3_BM);
+ ar->ab->hw_params.hal_params->rx_buf_rbm);
}
}

diff --git a/drivers/net/wireless/ath/ath11k/hal.c b/drivers/net/wireless/ath/ath11k/hal.c
index eaa0edca5576..f1492cde84ed 100644
--- a/drivers/net/wireless/ath/ath11k/hal.c
+++ b/drivers/net/wireless/ath/ath11k/hal.c
@@ -189,6 +189,14 @@ static const struct hal_srng_config hw_srng_config_template[] = {
},
};

+const struct hal_param ath11k_hal_params_ipq8074 = {
+ .rx_buf_rbm = HAL_RX_BUF_RBM_SW3_BM,
+};
+
+const struct hal_param ath11k_hal_params_qca6390 = {
+ .rx_buf_rbm = HAL_RX_BUF_RBM_SW1_BM,
+};
+
static int ath11k_hal_alloc_cont_rdp(struct ath11k_base *ab)
{
struct ath11k_hal *hal = &ab->hal;
diff --git a/drivers/net/wireless/ath/ath11k/hal.h b/drivers/net/wireless/ath/ath11k/hal.h
index 35ed3a14e200..1bf6e040120f 100644
--- a/drivers/net/wireless/ath/ath11k/hal.h
+++ b/drivers/net/wireless/ath/ath11k/hal.h
@@ -903,6 +903,13 @@ struct ath11k_hal {
int num_shadow_reg_configured;
};

+struct hal_param {
+ enum hal_rx_buf_return_buf_manager rx_buf_rbm;
+};
+
+extern const struct hal_param ath11k_hal_params_ipq8074;
+extern const struct hal_param ath11k_hal_params_qca6390;
+
u32 ath11k_hal_reo_qdesc_size(u32 ba_window_size, u8 tid);
void ath11k_hal_reo_qdesc_setup(void *vaddr, int tid, u32 ba_window_size,
u32 start_seq, enum hal_pn_type type);
diff --git a/drivers/net/wireless/ath/ath11k/hal_rx.c b/drivers/net/wireless/ath/ath11k/hal_rx.c
index f072c95e7434..d4d1c4a9785f 100644
--- a/drivers/net/wireless/ath/ath11k/hal_rx.c
+++ b/drivers/net/wireless/ath/ath11k/hal_rx.c
@@ -372,7 +372,7 @@ int ath11k_hal_wbm_desc_parse_err(struct ath11k_base *ab, void *desc,
return -EINVAL;

if (FIELD_GET(BUFFER_ADDR_INFO1_RET_BUF_MGR,
- wbm_desc->buf_addr_info.info1) != HAL_RX_BUF_RBM_SW3_BM) {
+ wbm_desc->buf_addr_info.info1) != ab->hw_params.hal_params->rx_buf_rbm) {
ab->soc_stats.invalid_rbm++;
return -EINVAL;
}
diff --git a/drivers/net/wireless/ath/ath11k/hw.c b/drivers/net/wireless/ath/ath11k/hw.c
index 57fab9d085d0..fdfb379b4d08 100644
--- a/drivers/net/wireless/ath/ath11k/hw.c
+++ b/drivers/net/wireless/ath/ath11k/hw.c
@@ -7,10 +7,11 @@
#include <linux/bitops.h>
#include <linux/bitfield.h>

-#include "hw.h"
#include "core.h"
#include "ce.h"
#include "hif.h"
+#include "hal.h"
+#include "hw.h"

/* Map from pdev index to hw mac index */
static u8 ath11k_hw_ipq8074_mac_from_pdev_id(int pdev_idx)
diff --git a/drivers/net/wireless/ath/ath11k/hw.h b/drivers/net/wireless/ath/ath11k/hw.h
index 6e68c5b60eaa..c6831fb110ba 100644
--- a/drivers/net/wireless/ath/ath11k/hw.h
+++ b/drivers/net/wireless/ath/ath11k/hw.h
@@ -179,6 +179,7 @@ struct ath11k_hw_params {
bool fix_l1ss;
const struct ath11k_hw_sram_dump *sram_dump;
u8 max_tx_ring;
+ const struct hal_param *hal_params;
};

struct ath11k_hw_ops {
--
2.25.1

2021-09-16 10:03:17

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/3] ath11k: change return buffer manager for QCA6390

Jouni Malinen <[email protected]> writes:

> From: Baochen Qiang <[email protected]>
>
> QCA6390 firmware uses HAL_RX_BUF_RBM_SW1_BM, not HAL_RX_BUF_RBM_SW3_BM.
>
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>
> Signed-off-by: Baochen Qiang <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>

There were checkpatch warnings, I fixex them in the pending branch:

drivers/net/wireless/ath/ath11k/dp.c:825: line length of 97 exceeds 90 columns
drivers/net/wireless/ath/ath11k/hal_rx.c:375: line length of 95 exceeds 90 columns
drivers/net/wireless/ath/ath11k/dp_rx.c:3073: line length of 94 exceeds 90 columns
drivers/net/wireless/ath/ath11k/dp_rx.c:3083: line length of 94 exceeds 90 columns
drivers/net/wireless/ath/ath11k/dp_rx.c:3472: line length of 100 exceeds 90 columns
drivers/net/wireless/ath/ath11k/dp_rx.c:5104: line length of 93 exceeds 90 columns
drivers/net/wireless/ath/ath11k/dp_rx.c:5109: line length of 93 exceeds 90 columns

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-09-16 10:09:20

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 3/3] ath11k: set correct NL80211_FEATURE_DYNAMIC_SMPS for WCN6855

Jouni Malinen <[email protected]> writes:

> From: Wen Gong <[email protected]>
>
> Commit "ath11k: support SMPS configuration for 6 GHz" changed "if
> (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS)" to "if (ht_cap &
> WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)" which means
> NL80211_FEATURE_DYNAMIC_SMPS is enabled for all chips which support 6
> GHz. However, WCN6855 supports 6 GHz but it does not support feature
> NL80211_FEATURE_DYNAMIC_SMPS, and this can lead to MU-MIMO test failures
> for WCN6855.
>
> Disable NL80211_FEATURE_DYNAMIC_SMPS for WCN6855 since its ht_cap does
> not support WMI_HT_CAP_DYNAMIC_SMPS.
>
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>
> Signed-off-by: Wen Gong <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>

[...]

> --- a/drivers/net/wireless/ath/ath11k/mac.c
> +++ b/drivers/net/wireless/ath/ath11k/mac.c
> @@ -7570,7 +7570,8 @@ static int __ath11k_mac_register(struct ath11k *ar)
> * for each band for a dual band capable radio. It will be tricky to
> * handle it when the ht capability different for each band.
> */
> - if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)
> + if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS ||
> + (ar->supports_6ghz && !ab->hw_params.check_dynamic_smps))
> ar->hw->wiphy->features |= NL80211_FEATURE_DYNAMIC_SMPS;
>
> ar->hw->wiphy->max_scan_ssids = WLAN_SCAN_PARAMS_MAX_SSID;

This hunk failed, in the pending branch as I don't have that
ar->supports_6ghz check. I'll drop this patch 3 for now, let's revisit
after my queue of ath11k patches is smaller.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-09-16 14:58:20

by Wen Gong

[permalink] [raw]
Subject: Re: [PATCH 3/3] ath11k: set correct NL80211_FEATURE_DYNAMIC_SMPS for WCN6855

On 2021-09-16 18:08, Kalle Valo wrote:
> Jouni Malinen <[email protected]> writes:
>
>> From: Wen Gong <[email protected]>
>>
>> Commit "ath11k: support SMPS configuration for 6 GHz" changed "if
>> (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS)" to "if (ht_cap &
>> WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)" which means
>> NL80211_FEATURE_DYNAMIC_SMPS is enabled for all chips which support 6
>> GHz. However, WCN6855 supports 6 GHz but it does not support feature
>> NL80211_FEATURE_DYNAMIC_SMPS, and this can lead to MU-MIMO test
>> failures
>> for WCN6855.
>>
>> Disable NL80211_FEATURE_DYNAMIC_SMPS for WCN6855 since its ht_cap does
>> not support WMI_HT_CAP_DYNAMIC_SMPS.
>>
>> Tested-on: WCN6855 hw2.0 PCI
>> WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>>
>> Signed-off-by: Wen Gong <[email protected]>
>> Signed-off-by: Jouni Malinen <[email protected]>
>
> [...]
>
>> --- a/drivers/net/wireless/ath/ath11k/mac.c
>> +++ b/drivers/net/wireless/ath/ath11k/mac.c
>> @@ -7570,7 +7570,8 @@ static int __ath11k_mac_register(struct ath11k
>> *ar)
>> * for each band for a dual band capable radio. It will be tricky to
>> * handle it when the ht capability different for each band.
>> */
>> - if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)
>> + if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS ||
>> + (ar->supports_6ghz && !ab->hw_params.check_dynamic_smps))
>> ar->hw->wiphy->features |= NL80211_FEATURE_DYNAMIC_SMPS;
>>
>> ar->hw->wiphy->max_scan_ssids = WLAN_SCAN_PARAMS_MAX_SSID;
>
> This hunk failed, in the pending branch as I don't have that
> ar->supports_6ghz check. I'll drop this patch 3 for now, let's revisit
> after my queue of ath11k patches is smaller.
Hi Kalle,

ar->supports_6ghz is introduced by this patch:
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

@@ -7559,7 +7570,7 @@ static int __ath11k_mac_register(struct ath11k
*ar)
* for each band for a dual band capable radio. It will be tricky to
* handle it when the ht capability different for each band.
*/
- if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS)
+ if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)
ar->hw->wiphy->features |= NL80211_FEATURE_DYNAMIC_SMPS;

ar->hw->wiphy->max_scan_ssids = WLAN_SCAN_PARAMS_MAX_SSID;

2021-09-16 18:27:29

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 3/3] ath11k: set correct NL80211_FEATURE_DYNAMIC_SMPS for WCN6855

Wen Gong <[email protected]> writes:

> On 2021-09-16 18:08, Kalle Valo wrote:
>> Jouni Malinen <[email protected]> writes:
>>
>>> From: Wen Gong <[email protected]>
>>>
>>> Commit "ath11k: support SMPS configuration for 6 GHz" changed "if
>>> (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS)" to "if (ht_cap &
>>> WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)" which means
>>> NL80211_FEATURE_DYNAMIC_SMPS is enabled for all chips which support 6
>>> GHz. However, WCN6855 supports 6 GHz but it does not support feature
>>> NL80211_FEATURE_DYNAMIC_SMPS, and this can lead to MU-MIMO test
>>> failures
>>> for WCN6855.
>>>
>>> Disable NL80211_FEATURE_DYNAMIC_SMPS for WCN6855 since its ht_cap does
>>> not support WMI_HT_CAP_DYNAMIC_SMPS.
>>>
>>> Tested-on: WCN6855 hw2.0 PCI
>>> WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>>>
>>> Signed-off-by: Wen Gong <[email protected]>
>>> Signed-off-by: Jouni Malinen <[email protected]>
>>
>> [...]
>>
>>> --- a/drivers/net/wireless/ath/ath11k/mac.c
>>> +++ b/drivers/net/wireless/ath/ath11k/mac.c
>>> @@ -7570,7 +7570,8 @@ static int __ath11k_mac_register(struct
>>> ath11k *ar)
>>> * for each band for a dual band capable radio. It will be tricky to
>>> * handle it when the ht capability different for each band.
>>> */
>>> - if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)
>>> + if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS ||
>>> + (ar->supports_6ghz && !ab->hw_params.check_dynamic_smps))
>>> ar->hw->wiphy->features |= NL80211_FEATURE_DYNAMIC_SMPS;
>>>
>>> ar->hw->wiphy->max_scan_ssids = WLAN_SCAN_PARAMS_MAX_SSID;
>>
>> This hunk failed, in the pending branch as I don't have that
>> ar->supports_6ghz check. I'll drop this patch 3 for now, let's revisit
>> after my queue of ath11k patches is smaller.
>
> ar->supports_6ghz is introduced by this patch:
> https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

Ah, and that was in Awaiting Upstream state and not yet in my pending
branch. I'll take that patchset first to pending branch and then apply
this patch 3, so no need to resend anything (at least for the moment).

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-09-28 15:16:56

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/3] ath11k: change return buffer manager for QCA6390

Jouni Malinen <[email protected]> writes:

> From: Baochen Qiang <[email protected]>
>
> QCA6390 firmware uses HAL_RX_BUF_RBM_SW1_BM, not HAL_RX_BUF_RBM_SW3_BM.
>
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>
> Signed-off-by: Baochen Qiang <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>

Same question as in patch 1, does this fix a bug or is just a
theoretical issue found during code review?

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-09-28 15:18:32

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/3] ath11k: change return buffer manager for QCA6390

Jouni Malinen <[email protected]> writes:

> From: Baochen Qiang <[email protected]>
>
> QCA6390 firmware uses HAL_RX_BUF_RBM_SW1_BM, not HAL_RX_BUF_RBM_SW3_BM.
>
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>
> Signed-off-by: Baochen Qiang <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>

[...]

> --- a/drivers/net/wireless/ath/ath11k/hal.c
> +++ b/drivers/net/wireless/ath/ath11k/hal.c
> @@ -189,6 +189,14 @@ static const struct hal_srng_config hw_srng_config_template[] = {
> },
> };
>
> +const struct hal_param ath11k_hal_params_ipq8074 = {
> + .rx_buf_rbm = HAL_RX_BUF_RBM_SW3_BM,
> +};
> +
> +const struct hal_param ath11k_hal_params_qca6390 = {
> + .rx_buf_rbm = HAL_RX_BUF_RBM_SW1_BM,
> +};
> +
> static int ath11k_hal_alloc_cont_rdp(struct ath11k_base *ab)
> {
> struct ath11k_hal *hal = &ab->hal;
> diff --git a/drivers/net/wireless/ath/ath11k/hal.h b/drivers/net/wireless/ath/ath11k/hal.h
> index 35ed3a14e200..1bf6e040120f 100644
> --- a/drivers/net/wireless/ath/ath11k/hal.h
> +++ b/drivers/net/wireless/ath/ath11k/hal.h
> @@ -903,6 +903,13 @@ struct ath11k_hal {
> int num_shadow_reg_configured;
> };
>
> +struct hal_param {
> + enum hal_rx_buf_return_buf_manager rx_buf_rbm;
> +};
> +
> +extern const struct hal_param ath11k_hal_params_ipq8074;
> +extern const struct hal_param ath11k_hal_params_qca6390;

In the pending branch I renamed these to struct ath11k_hw_hal_params and
moved to hw.c and hw.h, respectively.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-09-28 15:19:01

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/3] ath11k: Change number of TCL rings to one for QCA6390

Jouni Malinen <[email protected]> writes:

> From: Baochen Qiang <[email protected]>
>
> Some targets, QCA6390 for example, use only one TCL ring,
> it is better to initialize only one ring and leave others
> untouched for such targets.
>
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>
> Signed-off-by: Baochen Qiang <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>

It's better? Please be more specific. Does this fix a bug or is this
just a theoretical fix you found during code review?

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-09-29 02:18:17

by Baochen Qiang

[permalink] [raw]
Subject: Re: [PATCH 1/3] ath11k: Change number of TCL rings to one for QCA6390

On 2021-09-28 23:12, Kalle Valo wrote:
> Jouni Malinen <[email protected]> writes:
>
>> From: Baochen Qiang <[email protected]>
>>
>> Some targets, QCA6390 for example, use only one TCL ring,
>> it is better to initialize only one ring and leave others
>> untouched for such targets.
>>
>> Tested-on: QCA6390 hw2.0 PCI
>> WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>>
>> Signed-off-by: Baochen Qiang <[email protected]>
>> Signed-off-by: Jouni Malinen <[email protected]>
>
> It's better? Please be more specific. Does this fix a bug or is this
> just a theoretical fix you found during code review?

Yes, this is just a theoretical fix. By "better" I mean there is no need
to initialize
the other two TCL rings for QCA6390 since they are not used.

2021-09-29 02:56:35

by Baochen Qiang

[permalink] [raw]
Subject: Re: [PATCH 2/3] ath11k: change return buffer manager for QCA6390

On 2021-09-28 23:14, Kalle Valo wrote:
> Jouni Malinen <[email protected]> writes:
>
>> From: Baochen Qiang <[email protected]>
>>
>> QCA6390 firmware uses HAL_RX_BUF_RBM_SW1_BM, not
>> HAL_RX_BUF_RBM_SW3_BM.
>>
>> Tested-on: QCA6390 hw2.0 PCI
>> WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>>
>> Signed-off-by: Baochen Qiang <[email protected]>
>> Signed-off-by: Jouni Malinen <[email protected]>
>
> Same question as in patch 1, does this fix a bug or is just a
> theoretical issue found during code review?

Yes, this patch did fix a bug.
QCA6390 firmware expects some specific packets from WBM2SW1 ring, which,
however, will
not happen because they are routed directly to host through WBM2SW3 ring
due to wrong
configuration of RBM.

2021-10-01 06:29:46

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/3] ath11k: change return buffer manager for QCA6390

[email protected] writes:

> On 2021-09-28 23:14, Kalle Valo wrote:
>> Jouni Malinen <[email protected]> writes:
>>
>>> From: Baochen Qiang <[email protected]>
>>>
>>> QCA6390 firmware uses HAL_RX_BUF_RBM_SW1_BM, not
>>> HAL_RX_BUF_RBM_SW3_BM.
>>>
>>> Tested-on: QCA6390 hw2.0 PCI
>>> WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>>>
>>> Signed-off-by: Baochen Qiang <[email protected]>
>>> Signed-off-by: Jouni Malinen <[email protected]>
>>
>> Same question as in patch 1, does this fix a bug or is just a
>> theoretical issue found during code review?
>
> Yes, this patch did fix a bug.

Ok, please always describe the bug you are fixing in the commit log.
This is documented in the wiki pages which you definitely should read
very carefully, links below.

> QCA6390 firmware expects some specific packets from WBM2SW1 ring,
> which, however, will not happen because they are routed directly to
> host through WBM2SW3 ring due to wrong configuration of RBM.

Can you give higher level description of the bug (from user's
perspective)? For example, what test case was failing or how did you
notice this? I'll then add it to the commit log.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-10-05 14:09:21

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/3] ath11k: Change number of TCL rings to one for QCA6390

[email protected] writes:

> On 2021-09-28 23:12, Kalle Valo wrote:
>> Jouni Malinen <[email protected]> writes:
>>
>>> From: Baochen Qiang <[email protected]>
>>>
>>> Some targets, QCA6390 for example, use only one TCL ring,
>>> it is better to initialize only one ring and leave others
>>> untouched for such targets.
>>>
>>> Tested-on: QCA6390 hw2.0 PCI
>>> WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>>>
>>> Signed-off-by: Baochen Qiang <[email protected]>
>>> Signed-off-by: Jouni Malinen <[email protected]>
>>
>> It's better? Please be more specific. Does this fix a bug or is this
>> just a theoretical fix you found during code review?
>
> Yes, this is just a theoretical fix. By "better" I mean there is no
> need to initialize
> the other two TCL rings for QCA6390 since they are not used.

Thanks, I changed the commit log now to this:

ath11k: Change number of TCL rings to one for QCA6390

Some targets, QCA6390 for example, use only one TCL ring, it is better to
initialize only one ring and leave others untouched for such targets.

This is a theoretical fix found during code review, no visible impact.

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-10-05 14:12:17

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/3] ath11k: change return buffer manager for QCA6390

[email protected] writes:

> On 2021-09-28 23:14, Kalle Valo wrote:
>> Jouni Malinen <[email protected]> writes:
>>
>>> From: Baochen Qiang <[email protected]>
>>>
>>> QCA6390 firmware uses HAL_RX_BUF_RBM_SW1_BM, not
>>> HAL_RX_BUF_RBM_SW3_BM.
>>>
>>> Tested-on: QCA6390 hw2.0 PCI
>>> WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>>>
>>> Signed-off-by: Baochen Qiang <[email protected]>
>>> Signed-off-by: Jouni Malinen <[email protected]>
>>
>> Same question as in patch 1, does this fix a bug or is just a
>> theoretical issue found during code review?
>
> Yes, this patch did fix a bug.
>
> QCA6390 firmware expects some specific packets from WBM2SW1 ring,
> which, however, will not happen because they are routed directly to
> host through WBM2SW3 ring due to wrong configuration of RBM.

What specific packets exactly?

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-10-11 15:14:30

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/3] ath11k: Change number of TCL rings to one for QCA6390

Jouni Malinen <[email protected]> wrote:

> Some targets, QCA6390 for example, use only one TCL ring, it is better to
> initialize only one ring and leave others untouched for such targets.
>
> This is a theoretical fix found during code review, no visible impact.
>
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>
> Signed-off-by: Baochen Qiang <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

31582373a4a8 ath11k: Change number of TCL rings to one for QCA6390

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-10-25 13:05:51

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/3] ath11k: change return buffer manager for QCA6390

Jouni Malinen <[email protected]> wrote:

> QCA6390 firmware uses HAL_RX_BUF_RBM_SW1_BM, not HAL_RX_BUF_RBM_SW3_BM. This is
> needed to fix a case where an A-MSDU has an unexpected LLC/SNAP header in the
> first subframe (CVE-2020-24588).
>
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>
> Signed-off-by: Baochen Qiang <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

734223d78428 ath11k: change return buffer manager for QCA6390

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-10-25 13:06:25

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/3] ath11k: change return buffer manager for QCA6390

Kalle Valo <[email protected]> writes:

> [email protected] writes:
>
>> On 2021-09-28 23:14, Kalle Valo wrote:
>>> Jouni Malinen <[email protected]> writes:
>>>
>>>> From: Baochen Qiang <[email protected]>
>>>>
>>>> QCA6390 firmware uses HAL_RX_BUF_RBM_SW1_BM, not
>>>> HAL_RX_BUF_RBM_SW3_BM.
>>>>
>>>> Tested-on: QCA6390 hw2.0 PCI
>>>> WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>>>>
>>>> Signed-off-by: Baochen Qiang <[email protected]>
>>>> Signed-off-by: Jouni Malinen <[email protected]>
>>>
>>> Same question as in patch 1, does this fix a bug or is just a
>>> theoretical issue found during code review?
>>
>> Yes, this patch did fix a bug.
>>
>> QCA6390 firmware expects some specific packets from WBM2SW1 ring,
>> which, however, will not happen because they are routed directly to
>> host through WBM2SW3 ring due to wrong configuration of RBM.
>
> What specific packets exactly?

We discussed this internally and I now changed the commit log to:

ath11k: change return buffer manager for QCA6390

QCA6390 firmware uses HAL_RX_BUF_RBM_SW1_BM, not HAL_RX_BUF_RBM_SW3_BM. This is
needed to fix a case where an A-MSDU has an unexpected LLC/SNAP header in the
first subframe (CVE-2020-24588).

Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1

Signed-off-by: Baochen Qiang <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
Signed-off-by: Kalle Valo <[email protected]>

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-10-28 10:08:07

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 3/3] ath11k: set correct NL80211_FEATURE_DYNAMIC_SMPS for WCN6855

Jouni Malinen <[email protected]> writes:

> From: Wen Gong <[email protected]>
>
> Commit "ath11k: support SMPS configuration for 6 GHz" changed "if
> (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS)" to "if (ht_cap &
> WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)" which means
> NL80211_FEATURE_DYNAMIC_SMPS is enabled for all chips which support 6
> GHz. However, WCN6855 supports 6 GHz but it does not support feature
> NL80211_FEATURE_DYNAMIC_SMPS, and this can lead to MU-MIMO test failures
> for WCN6855.
>
> Disable NL80211_FEATURE_DYNAMIC_SMPS for WCN6855 since its ht_cap does
> not support WMI_HT_CAP_DYNAMIC_SMPS.
>
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>
> Signed-off-by: Wen Gong <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>
> ---
> drivers/net/wireless/ath/ath11k/core.c | 1 +
> drivers/net/wireless/ath/ath11k/hw.h | 1 +
> drivers/net/wireless/ath/ath11k/mac.c | 3 ++-
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
> index 265ff225bd81..2bae8c5184d4 100644
> --- a/drivers/net/wireless/ath/ath11k/core.c
> +++ b/drivers/net/wireless/ath/ath11k/core.c
> @@ -279,6 +279,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .sram_dump = &sram_dump_wcn6855,
> .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
> .hal_params = &ath11k_hal_params_qca6390,
> + .check_dynamic_smps = true,
> },
> };
>
> diff --git a/drivers/net/wireless/ath/ath11k/hw.h b/drivers/net/wireless/ath/ath11k/hw.h
> index c6831fb110ba..7463b96770b7 100644
> --- a/drivers/net/wireless/ath/ath11k/hw.h
> +++ b/drivers/net/wireless/ath/ath11k/hw.h
> @@ -180,6 +180,7 @@ struct ath11k_hw_params {
> const struct ath11k_hw_sram_dump *sram_dump;
> u8 max_tx_ring;
> const struct hal_param *hal_params;
> + bool check_dynamic_smps;
> };
>
> struct ath11k_hw_ops {
> diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
> index 1f4765e43546..97a2c92b7b9b 100644
> --- a/drivers/net/wireless/ath/ath11k/mac.c
> +++ b/drivers/net/wireless/ath/ath11k/mac.c
> @@ -7570,7 +7570,8 @@ static int __ath11k_mac_register(struct ath11k *ar)
> * for each band for a dual band capable radio. It will be tricky to
> * handle it when the ht capability different for each band.
> */
> - if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)
> + if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS ||
> + (ar->supports_6ghz && !ab->hw_params.check_dynamic_smps))
> ar->hw->wiphy->features |= NL80211_FEATURE_DYNAMIC_SMPS;

Instead of a "negative" flag I reverted the test and renamed the flag to
supports_dynamic_smps_6ghz. AFAIK QCN9074 is the only device supporting
6 GHz band so I enabled the flag only for it.

Please review my changes in the pending branch:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=cc692cfb9f2981691b39b601b37e4544ecf01136

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-10-29 02:31:19

by Wen Gong

[permalink] [raw]
Subject: Re: [PATCH 3/3] ath11k: set correct NL80211_FEATURE_DYNAMIC_SMPS for WCN6855

On 2021-10-28 18:07, Kalle Valo wrote:
> Jouni Malinen <[email protected]> writes:
>
>> From: Wen Gong <[email protected]>
>>
...
>> diff --git a/drivers/net/wireless/ath/ath11k/mac.c
>> b/drivers/net/wireless/ath/ath11k/mac.c
>> index 1f4765e43546..97a2c92b7b9b 100644
>> --- a/drivers/net/wireless/ath/ath11k/mac.c
>> +++ b/drivers/net/wireless/ath/ath11k/mac.c
>> @@ -7570,7 +7570,8 @@ static int __ath11k_mac_register(struct ath11k
>> *ar)
>> * for each band for a dual band capable radio. It will be tricky to
>> * handle it when the ht capability different for each band.
>> */
>> - if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)
>> + if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS ||
>> + (ar->supports_6ghz && !ab->hw_params.check_dynamic_smps))
>> ar->hw->wiphy->features |= NL80211_FEATURE_DYNAMIC_SMPS;
>
> Instead of a "negative" flag I reverted the test and renamed the flag
> to
> supports_dynamic_smps_6ghz. AFAIK QCN9074 is the only device supporting
> 6 GHz band so I enabled the flag only for it.
>
> Please review my changes in the pending branch:
Thanks.
the change is OK for WCN6855/QCA6390.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=cc692cfb9f2981691b39b601b37e4544ecf01136

2021-11-01 14:16:18

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 3/3] ath11k: set correct NL80211_FEATURE_DYNAMIC_SMPS for WCN6855

Jouni Malinen <[email protected]> wrote:

> Commit 6f4d70308e5e ("ath11k: support SMPS configuration for 6 GHz") changed
> "if (ht_cap & WMI_HT_CAP_DYNAMIC_SMPS)" to "if (ht_cap &
> WMI_HT_CAP_DYNAMIC_SMPS || ar->supports_6ghz)" which means
> NL80211_FEATURE_DYNAMIC_SMPS is enabled for all chips which support 6 GHz.
> However, WCN6855 supports 6 GHz but it does not support feature
> NL80211_FEATURE_DYNAMIC_SMPS, and this can lead to MU-MIMO test failures for
> WCN6855.
>
> Disable NL80211_FEATURE_DYNAMIC_SMPS for WCN6855 since its ht_cap does not
> support WMI_HT_CAP_DYNAMIC_SMPS. Enable the feature only on QCN9074 as that's
> the only other device supporting 6 GHz band.
>
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>
> Signed-off-by: Wen Gong <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

82c434c10340 ath11k: set correct NL80211_FEATURE_DYNAMIC_SMPS for WCN6855

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches