* [PATCH v3] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params
@ 2025-12-28 15:14 Alexandru Gagniuc
2025-12-29 3:14 ` Baochen Qiang
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Alexandru Gagniuc @ 2025-12-28 15:14 UTC (permalink / raw)
To: jjohnson, ath11k
Cc: baochen.qiang, Alexandru Gagniuc, linux-wireless, linux-kernel
".max_tx_ring" is an upper bounds to indexing ".tcl2wbm_rbm_map". It
is initialized in, core.c, a different file than the array. This
spaghetti-like relation is fragile and not obvious. Accidentally
setting ".max_tx_ring" too high leads to a hard to track out-of-
bounds access and memory corruption.
There is a small ambiguity on the meaning of "max_tx_ring":
- The highest ring, max=3 implies there are 4 rings (0, 1, 2, 3)
- The highest number to use for array indexing (there are 3 rings)
Clarify this dependency by moving ".max_tx_ring" adjacent to the array
".tcl2wbm_rbm_map", and name it "num_tx_rings". Use ARRAY_SIZE()
instead of #defines to initialize the length field.
The ath11k_hw_hal_params_qca6390 uses fewer num_tx_rings than its map,
so use a constant to express the correct value. Add a static_assert()
to fail compilation if the constant is accidentally set too high.
The intent is to make the code easier to understand rather than fix
an existing bug.
Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
I am trying to make ath11k work on IPQ9574. My device uses a IPQ9570
with a QCN5024 as the 2.4 GHz wifi.
I spent a few days tracking a memory corruption bug caused by
erroneously setting ".max_tx_ring" too high. I think I would not have
made this mistake if the initializations of .max_tx_ring and
.tcl2wbm_rbm_map were right next to each other.
Changes since v2:
- Use ipq5018 rbm_map for ath11k_hw_hal_params_qca6390
- Drop unecessarry static_assert
Changes since v1:
- use "num_tx_rings" name instead of "map_len"
- make sure debugfs.c is correctly updated
- add a static_assert for ath11k_hw_hal_params_qca6390 special case
---
drivers/net/wireless/ath/ath11k/core.c | 12 +-----------
drivers/net/wireless/ath/ath11k/debugfs.c | 2 +-
drivers/net/wireless/ath/ath11k/dp.c | 12 ++++++------
drivers/net/wireless/ath/ath11k/dp.h | 1 -
drivers/net/wireless/ath/ath11k/dp_tx.c | 9 +++++----
drivers/net/wireless/ath/ath11k/hw.c | 19 ++++++++++++++++++-
drivers/net/wireless/ath/ath11k/hw.h | 3 ++-
drivers/net/wireless/ath/ath11k/mac.c | 2 +-
8 files changed, 34 insertions(+), 26 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index 812686173ac8a..07199ceecbeb4 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -100,7 +100,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.supports_regdb = false,
.fix_l1ss = true,
.credit_flow = false,
- .max_tx_ring = DP_TCL_NUM_RING_MAX,
.hal_params = &ath11k_hw_hal_params_ipq8074,
.supports_dynamic_smps_6ghz = false,
.alloc_cacheable_memory = true,
@@ -184,7 +183,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.supports_regdb = false,
.fix_l1ss = true,
.credit_flow = false,
- .max_tx_ring = DP_TCL_NUM_RING_MAX,
.hal_params = &ath11k_hw_hal_params_ipq8074,
.supports_dynamic_smps_6ghz = false,
.alloc_cacheable_memory = true,
@@ -271,7 +269,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.supports_regdb = false,
.fix_l1ss = true,
.credit_flow = true,
- .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
.hal_params = &ath11k_hw_hal_params_qca6390,
.supports_dynamic_smps_6ghz = false,
.alloc_cacheable_memory = false,
@@ -358,7 +355,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.supports_regdb = false,
.fix_l1ss = true,
.credit_flow = false,
- .max_tx_ring = DP_TCL_NUM_RING_MAX,
.hal_params = &ath11k_hw_hal_params_ipq8074,
.supports_dynamic_smps_6ghz = true,
.alloc_cacheable_memory = true,
@@ -445,7 +441,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.supports_regdb = true,
.fix_l1ss = false,
.credit_flow = true,
- .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
.hal_params = &ath11k_hw_hal_params_qca6390,
.supports_dynamic_smps_6ghz = false,
.alloc_cacheable_memory = false,
@@ -533,7 +528,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.supports_regdb = true,
.fix_l1ss = false,
.credit_flow = true,
- .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
.hal_params = &ath11k_hw_hal_params_qca6390,
.supports_dynamic_smps_6ghz = false,
.alloc_cacheable_memory = false,
@@ -619,7 +613,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.supports_regdb = true,
.fix_l1ss = false,
.credit_flow = true,
- .max_tx_ring = DP_TCL_NUM_RING_MAX,
.hal_params = &ath11k_hw_hal_params_wcn6750,
.supports_dynamic_smps_6ghz = false,
.alloc_cacheable_memory = false,
@@ -662,7 +655,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.qmi_service_ins_id = ATH11K_QMI_WLFW_SERVICE_INS_ID_V01_IPQ8074,
.ring_mask = &ath11k_hw_ring_mask_ipq8074,
.credit_flow = false,
- .max_tx_ring = 1,
.spectral = {
.fft_sz = 2,
.fft_pad_sz = 0,
@@ -698,7 +690,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.supports_regdb = false,
.idle_ps = false,
.supports_suspend = false,
- .hal_params = &ath11k_hw_hal_params_ipq8074,
+ .hal_params = &ath11k_hw_hal_params_ipq5018,
.single_pdev_only = false,
.coldboot_cal_mm = true,
.coldboot_cal_ftm = true,
@@ -789,7 +781,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.supports_regdb = true,
.fix_l1ss = false,
.credit_flow = true,
- .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
.hal_params = &ath11k_hw_hal_params_qca6390,
.supports_dynamic_smps_6ghz = false,
.alloc_cacheable_memory = false,
@@ -876,7 +867,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.supports_regdb = true,
.fix_l1ss = false,
.credit_flow = true,
- .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
.hal_params = &ath11k_hw_hal_params_qca6390,
.supports_dynamic_smps_6ghz = false,
.alloc_cacheable_memory = false,
diff --git a/drivers/net/wireless/ath/ath11k/debugfs.c b/drivers/net/wireless/ath/ath11k/debugfs.c
index 977f945b6e669..50f344803e8fd 100644
--- a/drivers/net/wireless/ath/ath11k/debugfs.c
+++ b/drivers/net/wireless/ath/ath11k/debugfs.c
@@ -707,7 +707,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 < ab->hw_params.max_tx_ring; i++)
+ for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; 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 56b1a657e0b0f..c940de285276d 100644
--- a/drivers/net/wireless/ath/ath11k/dp.c
+++ b/drivers/net/wireless/ath/ath11k/dp.c
@@ -344,7 +344,7 @@ void ath11k_dp_stop_shadow_timers(struct ath11k_base *ab)
if (!ab->hw_params.supports_shadow_regs)
return;
- for (i = 0; i < ab->hw_params.max_tx_ring; i++)
+ for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; i++)
ath11k_dp_shadow_stop_timer(ab, &ab->dp.tx_ring_timer[i]);
ath11k_dp_shadow_stop_timer(ab, &ab->dp.reo_cmd_timer);
@@ -359,7 +359,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 < ab->hw_params.max_tx_ring; i++) {
+ for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; 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);
}
@@ -400,7 +400,7 @@ static int ath11k_dp_srng_common_setup(struct ath11k_base *ab)
goto err;
}
- for (i = 0; i < ab->hw_params.max_tx_ring; i++) {
+ for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; i++) {
tcl_num = ab->hw_params.hal_params->tcl2wbm_rbm_map[i].tcl_ring_num;
wbm_num = ab->hw_params.hal_params->tcl2wbm_rbm_map[i].wbm_ring_num;
@@ -782,7 +782,7 @@ int ath11k_dp_service_srng(struct ath11k_base *ab,
int i, j;
int tot_work_done = 0;
- for (i = 0; i < ab->hw_params.max_tx_ring; i++) {
+ for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; i++) {
if (BIT(ab->hw_params.hal_params->tcl2wbm_rbm_map[i].wbm_ring_num) &
ab->hw_params.ring_mask->tx[grp_id])
ath11k_dp_tx_completion_handler(ab, i);
@@ -1035,7 +1035,7 @@ void ath11k_dp_free(struct ath11k_base *ab)
ath11k_dp_reo_cmd_list_cleanup(ab);
- for (i = 0; i < ab->hw_params.max_tx_ring; i++) {
+ for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; 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);
@@ -1086,7 +1086,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 < ab->hw_params.max_tx_ring; i++) {
+ for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; 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 7a55afd33be82..1bd513f68a3c3 100644
--- a/drivers/net/wireless/ath/ath11k/dp.h
+++ b/drivers/net/wireless/ath/ath11k/dp.h
@@ -199,7 +199,6 @@ 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 562aba66582f3..86e1e6c27b36c 100644
--- a/drivers/net/wireless/ath/ath11k/dp_tx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
@@ -91,6 +91,7 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
struct hal_srng *tcl_ring;
struct ieee80211_hdr *hdr = (void *)skb->data;
struct dp_tx_ring *tx_ring;
+ size_t num_tx_rings = ab->hw_params.hal_params->num_tx_rings;
void *hal_tcl_desc;
u8 pool_id;
u8 hal_ring_id;
@@ -113,7 +114,7 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
tcl_ring_sel:
tcl_ring_retry = false;
- ti.ring_id = ring_selector % ab->hw_params.max_tx_ring;
+ ti.ring_id = ring_selector % num_tx_rings;
ti.rbm_id = ab->hw_params.hal_params->tcl2wbm_rbm_map[ti.ring_id].rbm_id;
ring_map |= BIT(ti.ring_id);
@@ -126,7 +127,7 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
spin_unlock_bh(&tx_ring->tx_idr_lock);
if (unlikely(ret < 0)) {
- if (ring_map == (BIT(ab->hw_params.max_tx_ring) - 1) ||
+ if (ring_map == (BIT(num_tx_rings) - 1) ||
!ab->hw_params.tcl_ring_retry) {
atomic_inc(&ab->soc_stats.tx_err.misc_fail);
return -ENOSPC;
@@ -244,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 (unlikely(ring_map != (BIT(ab->hw_params.max_tx_ring)) - 1) &&
- ab->hw_params.tcl_ring_retry && ab->hw_params.max_tx_ring > 1) {
+ if (unlikely(ring_map != (BIT(num_tx_rings)) - 1) &&
+ ab->hw_params.tcl_ring_retry && num_tx_rings > 1) {
tcl_ring_retry = true;
ring_selector++;
}
diff --git a/drivers/net/wireless/ath/ath11k/hw.c b/drivers/net/wireless/ath/ath11k/hw.c
index caa6dc12a790b..d19c4b372a2aa 100644
--- a/drivers/net/wireless/ath/ath11k/hw.c
+++ b/drivers/net/wireless/ath/ath11k/hw.c
@@ -2707,6 +2707,14 @@ const struct ath11k_hw_regs wcn6750_regs = {
.hal_reo1_misc_ctl = 0x000005d8,
};
+static const struct ath11k_hw_tcl2wbm_rbm_map ath11k_hw_tcl2wbm_rbm_map_ipq5018[] = {
+ {
+ .tcl_ring_num = 0,
+ .wbm_ring_num = 0,
+ .rbm_id = HAL_RX_BUF_RBM_SW0_BM,
+ },
+};
+
static const struct ath11k_hw_tcl2wbm_rbm_map ath11k_hw_tcl2wbm_rbm_map_ipq8074[] = {
{
.tcl_ring_num = 0,
@@ -2822,19 +2830,28 @@ const struct ath11k_hw_regs ipq5018_regs = {
.hal_wbm1_release_ring_base_lsb = 0x0000097c,
};
+const struct ath11k_hw_hal_params ath11k_hw_hal_params_ipq5018 = {
+ .rx_buf_rbm = HAL_RX_BUF_RBM_SW3_BM,
+ .tcl2wbm_rbm_map = ath11k_hw_tcl2wbm_rbm_map_ipq5018,
+ .num_tx_rings = ARRAY_SIZE(ath11k_hw_tcl2wbm_rbm_map_ipq5018),
+};
+
const struct ath11k_hw_hal_params ath11k_hw_hal_params_ipq8074 = {
.rx_buf_rbm = HAL_RX_BUF_RBM_SW3_BM,
.tcl2wbm_rbm_map = ath11k_hw_tcl2wbm_rbm_map_ipq8074,
+ .num_tx_rings = ARRAY_SIZE(ath11k_hw_tcl2wbm_rbm_map_ipq8074),
};
const struct ath11k_hw_hal_params ath11k_hw_hal_params_qca6390 = {
.rx_buf_rbm = HAL_RX_BUF_RBM_SW1_BM,
- .tcl2wbm_rbm_map = ath11k_hw_tcl2wbm_rbm_map_ipq8074,
+ .tcl2wbm_rbm_map = ath11k_hw_tcl2wbm_rbm_map_ipq5018,
+ .num_tx_rings = ARRAY_SIZE(ath11k_hw_tcl2wbm_rbm_map_ipq5018),
};
const struct ath11k_hw_hal_params ath11k_hw_hal_params_wcn6750 = {
.rx_buf_rbm = HAL_RX_BUF_RBM_SW1_BM,
.tcl2wbm_rbm_map = ath11k_hw_tcl2wbm_rbm_map_wcn6750,
+ .num_tx_rings = ARRAY_SIZE(ath11k_hw_tcl2wbm_rbm_map_wcn6750),
};
static const struct cfg80211_sar_freq_ranges ath11k_hw_sar_freq_ranges_wcn6855[] = {
diff --git a/drivers/net/wireless/ath/ath11k/hw.h b/drivers/net/wireless/ath/ath11k/hw.h
index 52d9f4c13b136..9db984ac4321c 100644
--- a/drivers/net/wireless/ath/ath11k/hw.h
+++ b/drivers/net/wireless/ath/ath11k/hw.h
@@ -134,6 +134,7 @@ struct ath11k_hw_tcl2wbm_rbm_map {
struct ath11k_hw_hal_params {
enum hal_rx_buf_return_buf_manager rx_buf_rbm;
const struct ath11k_hw_tcl2wbm_rbm_map *tcl2wbm_rbm_map;
+ size_t num_tx_rings;
};
struct ath11k_hw_params {
@@ -198,7 +199,6 @@ struct ath11k_hw_params {
bool supports_regdb;
bool fix_l1ss;
bool credit_flow;
- u8 max_tx_ring;
const struct ath11k_hw_hal_params *hal_params;
bool supports_dynamic_smps_6ghz;
bool alloc_cacheable_memory;
@@ -291,6 +291,7 @@ extern const struct ce_ie_addr ath11k_ce_ie_addr_ipq5018;
extern const struct ce_remap ath11k_ce_remap_ipq5018;
+extern const struct ath11k_hw_hal_params ath11k_hw_hal_params_ipq5018;
extern const struct ath11k_hw_hal_params ath11k_hw_hal_params_ipq8074;
extern const struct ath11k_hw_hal_params ath11k_hw_hal_params_qca6390;
extern const struct ath11k_hw_hal_params ath11k_hw_hal_params_wcn6750;
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index 3276fe443502f..33ebe03380114 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -7392,7 +7392,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 < ab->hw_params.max_tx_ring; i++) {
+ for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; 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.45.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params
2025-12-28 15:14 [PATCH v3] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params Alexandru Gagniuc
@ 2025-12-29 3:14 ` Baochen Qiang
2026-01-13 7:00 ` Vasanthakumar Thiagarajan
2026-01-16 1:26 ` Jeff Johnson
2 siblings, 0 replies; 8+ messages in thread
From: Baochen Qiang @ 2025-12-29 3:14 UTC (permalink / raw)
To: Alexandru Gagniuc, jjohnson, ath11k; +Cc: linux-wireless, linux-kernel
On 12/28/2025 11:14 PM, Alexandru Gagniuc wrote:
> ".max_tx_ring" is an upper bounds to indexing ".tcl2wbm_rbm_map". It
> is initialized in, core.c, a different file than the array. This
> spaghetti-like relation is fragile and not obvious. Accidentally
> setting ".max_tx_ring" too high leads to a hard to track out-of-
> bounds access and memory corruption.
>
> There is a small ambiguity on the meaning of "max_tx_ring":
> - The highest ring, max=3 implies there are 4 rings (0, 1, 2, 3)
> - The highest number to use for array indexing (there are 3 rings)
>
> Clarify this dependency by moving ".max_tx_ring" adjacent to the array
> ".tcl2wbm_rbm_map", and name it "num_tx_rings". Use ARRAY_SIZE()
> instead of #defines to initialize the length field.
>
> The ath11k_hw_hal_params_qca6390 uses fewer num_tx_rings than its map,
> so use a constant to express the correct value. Add a static_assert()
> to fail compilation if the constant is accidentally set too high.
>
> The intent is to make the code easier to understand rather than fix
> an existing bug.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>
> I am trying to make ath11k work on IPQ9574. My device uses a IPQ9570
> with a QCN5024 as the 2.4 GHz wifi.
>
> I spent a few days tracking a memory corruption bug caused by
> erroneously setting ".max_tx_ring" too high. I think I would not have
> made this mistake if the initializations of .max_tx_ring and
> .tcl2wbm_rbm_map were right next to each other.
>
> Changes since v2:
> - Use ipq5018 rbm_map for ath11k_hw_hal_params_qca6390
> - Drop unecessarry static_assert
>
> Changes since v1:
> - use "num_tx_rings" name instead of "map_len"
> - make sure debugfs.c is correctly updated
> - add a static_assert for ath11k_hw_hal_params_qca6390 special case
> ---
> drivers/net/wireless/ath/ath11k/core.c | 12 +-----------
> drivers/net/wireless/ath/ath11k/debugfs.c | 2 +-
> drivers/net/wireless/ath/ath11k/dp.c | 12 ++++++------
> drivers/net/wireless/ath/ath11k/dp.h | 1 -
> drivers/net/wireless/ath/ath11k/dp_tx.c | 9 +++++----
> drivers/net/wireless/ath/ath11k/hw.c | 19 ++++++++++++++++++-
> drivers/net/wireless/ath/ath11k/hw.h | 3 ++-
> drivers/net/wireless/ath/ath11k/mac.c | 2 +-
> 8 files changed, 34 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
> index 812686173ac8a..07199ceecbeb4 100644
> --- a/drivers/net/wireless/ath/ath11k/core.c
> +++ b/drivers/net/wireless/ath/ath11k/core.c
> @@ -100,7 +100,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .supports_regdb = false,
> .fix_l1ss = true,
> .credit_flow = false,
> - .max_tx_ring = DP_TCL_NUM_RING_MAX,
> .hal_params = &ath11k_hw_hal_params_ipq8074,
> .supports_dynamic_smps_6ghz = false,
> .alloc_cacheable_memory = true,
> @@ -184,7 +183,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .supports_regdb = false,
> .fix_l1ss = true,
> .credit_flow = false,
> - .max_tx_ring = DP_TCL_NUM_RING_MAX,
> .hal_params = &ath11k_hw_hal_params_ipq8074,
> .supports_dynamic_smps_6ghz = false,
> .alloc_cacheable_memory = true,
> @@ -271,7 +269,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .supports_regdb = false,
> .fix_l1ss = true,
> .credit_flow = true,
> - .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
> .hal_params = &ath11k_hw_hal_params_qca6390,
> .supports_dynamic_smps_6ghz = false,
> .alloc_cacheable_memory = false,
> @@ -358,7 +355,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .supports_regdb = false,
> .fix_l1ss = true,
> .credit_flow = false,
> - .max_tx_ring = DP_TCL_NUM_RING_MAX,
> .hal_params = &ath11k_hw_hal_params_ipq8074,
> .supports_dynamic_smps_6ghz = true,
> .alloc_cacheable_memory = true,
> @@ -445,7 +441,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .supports_regdb = true,
> .fix_l1ss = false,
> .credit_flow = true,
> - .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
> .hal_params = &ath11k_hw_hal_params_qca6390,
> .supports_dynamic_smps_6ghz = false,
> .alloc_cacheable_memory = false,
> @@ -533,7 +528,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .supports_regdb = true,
> .fix_l1ss = false,
> .credit_flow = true,
> - .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
> .hal_params = &ath11k_hw_hal_params_qca6390,
> .supports_dynamic_smps_6ghz = false,
> .alloc_cacheable_memory = false,
> @@ -619,7 +613,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .supports_regdb = true,
> .fix_l1ss = false,
> .credit_flow = true,
> - .max_tx_ring = DP_TCL_NUM_RING_MAX,
> .hal_params = &ath11k_hw_hal_params_wcn6750,
> .supports_dynamic_smps_6ghz = false,
> .alloc_cacheable_memory = false,
> @@ -662,7 +655,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .qmi_service_ins_id = ATH11K_QMI_WLFW_SERVICE_INS_ID_V01_IPQ8074,
> .ring_mask = &ath11k_hw_ring_mask_ipq8074,
> .credit_flow = false,
> - .max_tx_ring = 1,
> .spectral = {
> .fft_sz = 2,
> .fft_pad_sz = 0,
> @@ -698,7 +690,7 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .supports_regdb = false,
> .idle_ps = false,
> .supports_suspend = false,
> - .hal_params = &ath11k_hw_hal_params_ipq8074,
> + .hal_params = &ath11k_hw_hal_params_ipq5018,
> .single_pdev_only = false,
> .coldboot_cal_mm = true,
> .coldboot_cal_ftm = true,
> @@ -789,7 +781,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .supports_regdb = true,
> .fix_l1ss = false,
> .credit_flow = true,
> - .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
> .hal_params = &ath11k_hw_hal_params_qca6390,
> .supports_dynamic_smps_6ghz = false,
> .alloc_cacheable_memory = false,
> @@ -876,7 +867,6 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .supports_regdb = true,
> .fix_l1ss = false,
> .credit_flow = true,
> - .max_tx_ring = DP_TCL_NUM_RING_MAX_QCA6390,
> .hal_params = &ath11k_hw_hal_params_qca6390,
> .supports_dynamic_smps_6ghz = false,
> .alloc_cacheable_memory = false,
> diff --git a/drivers/net/wireless/ath/ath11k/debugfs.c b/drivers/net/wireless/ath/ath11k/debugfs.c
> index 977f945b6e669..50f344803e8fd 100644
> --- a/drivers/net/wireless/ath/ath11k/debugfs.c
> +++ b/drivers/net/wireless/ath/ath11k/debugfs.c
> @@ -707,7 +707,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 < ab->hw_params.max_tx_ring; i++)
> + for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; 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 56b1a657e0b0f..c940de285276d 100644
> --- a/drivers/net/wireless/ath/ath11k/dp.c
> +++ b/drivers/net/wireless/ath/ath11k/dp.c
> @@ -344,7 +344,7 @@ void ath11k_dp_stop_shadow_timers(struct ath11k_base *ab)
> if (!ab->hw_params.supports_shadow_regs)
> return;
>
> - for (i = 0; i < ab->hw_params.max_tx_ring; i++)
> + for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; i++)
> ath11k_dp_shadow_stop_timer(ab, &ab->dp.tx_ring_timer[i]);
>
> ath11k_dp_shadow_stop_timer(ab, &ab->dp.reo_cmd_timer);
> @@ -359,7 +359,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 < ab->hw_params.max_tx_ring; i++) {
> + for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; 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);
> }
> @@ -400,7 +400,7 @@ static int ath11k_dp_srng_common_setup(struct ath11k_base *ab)
> goto err;
> }
>
> - for (i = 0; i < ab->hw_params.max_tx_ring; i++) {
> + for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; i++) {
> tcl_num = ab->hw_params.hal_params->tcl2wbm_rbm_map[i].tcl_ring_num;
> wbm_num = ab->hw_params.hal_params->tcl2wbm_rbm_map[i].wbm_ring_num;
>
> @@ -782,7 +782,7 @@ int ath11k_dp_service_srng(struct ath11k_base *ab,
> int i, j;
> int tot_work_done = 0;
>
> - for (i = 0; i < ab->hw_params.max_tx_ring; i++) {
> + for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; i++) {
> if (BIT(ab->hw_params.hal_params->tcl2wbm_rbm_map[i].wbm_ring_num) &
> ab->hw_params.ring_mask->tx[grp_id])
> ath11k_dp_tx_completion_handler(ab, i);
> @@ -1035,7 +1035,7 @@ void ath11k_dp_free(struct ath11k_base *ab)
>
> ath11k_dp_reo_cmd_list_cleanup(ab);
>
> - for (i = 0; i < ab->hw_params.max_tx_ring; i++) {
> + for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; 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);
> @@ -1086,7 +1086,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 < ab->hw_params.max_tx_ring; i++) {
> + for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; 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 7a55afd33be82..1bd513f68a3c3 100644
> --- a/drivers/net/wireless/ath/ath11k/dp.h
> +++ b/drivers/net/wireless/ath/ath11k/dp.h
> @@ -199,7 +199,6 @@ 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 562aba66582f3..86e1e6c27b36c 100644
> --- a/drivers/net/wireless/ath/ath11k/dp_tx.c
> +++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
> @@ -91,6 +91,7 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
> struct hal_srng *tcl_ring;
> struct ieee80211_hdr *hdr = (void *)skb->data;
> struct dp_tx_ring *tx_ring;
> + size_t num_tx_rings = ab->hw_params.hal_params->num_tx_rings;
> void *hal_tcl_desc;
> u8 pool_id;
> u8 hal_ring_id;
> @@ -113,7 +114,7 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
> tcl_ring_sel:
> tcl_ring_retry = false;
>
> - ti.ring_id = ring_selector % ab->hw_params.max_tx_ring;
> + ti.ring_id = ring_selector % num_tx_rings;
> ti.rbm_id = ab->hw_params.hal_params->tcl2wbm_rbm_map[ti.ring_id].rbm_id;
>
> ring_map |= BIT(ti.ring_id);
> @@ -126,7 +127,7 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
> spin_unlock_bh(&tx_ring->tx_idr_lock);
>
> if (unlikely(ret < 0)) {
> - if (ring_map == (BIT(ab->hw_params.max_tx_ring) - 1) ||
> + if (ring_map == (BIT(num_tx_rings) - 1) ||
> !ab->hw_params.tcl_ring_retry) {
> atomic_inc(&ab->soc_stats.tx_err.misc_fail);
> return -ENOSPC;
> @@ -244,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 (unlikely(ring_map != (BIT(ab->hw_params.max_tx_ring)) - 1) &&
> - ab->hw_params.tcl_ring_retry && ab->hw_params.max_tx_ring > 1) {
> + if (unlikely(ring_map != (BIT(num_tx_rings)) - 1) &&
> + ab->hw_params.tcl_ring_retry && num_tx_rings > 1) {
> tcl_ring_retry = true;
> ring_selector++;
> }
> diff --git a/drivers/net/wireless/ath/ath11k/hw.c b/drivers/net/wireless/ath/ath11k/hw.c
> index caa6dc12a790b..d19c4b372a2aa 100644
> --- a/drivers/net/wireless/ath/ath11k/hw.c
> +++ b/drivers/net/wireless/ath/ath11k/hw.c
> @@ -2707,6 +2707,14 @@ const struct ath11k_hw_regs wcn6750_regs = {
> .hal_reo1_misc_ctl = 0x000005d8,
> };
>
> +static const struct ath11k_hw_tcl2wbm_rbm_map ath11k_hw_tcl2wbm_rbm_map_ipq5018[] = {
> + {
> + .tcl_ring_num = 0,
> + .wbm_ring_num = 0,
> + .rbm_id = HAL_RX_BUF_RBM_SW0_BM,
> + },
> +};
> +
> static const struct ath11k_hw_tcl2wbm_rbm_map ath11k_hw_tcl2wbm_rbm_map_ipq8074[] = {
> {
> .tcl_ring_num = 0,
> @@ -2822,19 +2830,28 @@ const struct ath11k_hw_regs ipq5018_regs = {
> .hal_wbm1_release_ring_base_lsb = 0x0000097c,
> };
>
> +const struct ath11k_hw_hal_params ath11k_hw_hal_params_ipq5018 = {
> + .rx_buf_rbm = HAL_RX_BUF_RBM_SW3_BM,
> + .tcl2wbm_rbm_map = ath11k_hw_tcl2wbm_rbm_map_ipq5018,
> + .num_tx_rings = ARRAY_SIZE(ath11k_hw_tcl2wbm_rbm_map_ipq5018),
> +};
> +
> const struct ath11k_hw_hal_params ath11k_hw_hal_params_ipq8074 = {
> .rx_buf_rbm = HAL_RX_BUF_RBM_SW3_BM,
> .tcl2wbm_rbm_map = ath11k_hw_tcl2wbm_rbm_map_ipq8074,
> + .num_tx_rings = ARRAY_SIZE(ath11k_hw_tcl2wbm_rbm_map_ipq8074),
> };
>
> const struct ath11k_hw_hal_params ath11k_hw_hal_params_qca6390 = {
> .rx_buf_rbm = HAL_RX_BUF_RBM_SW1_BM,
> - .tcl2wbm_rbm_map = ath11k_hw_tcl2wbm_rbm_map_ipq8074,
> + .tcl2wbm_rbm_map = ath11k_hw_tcl2wbm_rbm_map_ipq5018,
> + .num_tx_rings = ARRAY_SIZE(ath11k_hw_tcl2wbm_rbm_map_ipq5018),
> };
>
> const struct ath11k_hw_hal_params ath11k_hw_hal_params_wcn6750 = {
> .rx_buf_rbm = HAL_RX_BUF_RBM_SW1_BM,
> .tcl2wbm_rbm_map = ath11k_hw_tcl2wbm_rbm_map_wcn6750,
> + .num_tx_rings = ARRAY_SIZE(ath11k_hw_tcl2wbm_rbm_map_wcn6750),
> };
>
> static const struct cfg80211_sar_freq_ranges ath11k_hw_sar_freq_ranges_wcn6855[] = {
> diff --git a/drivers/net/wireless/ath/ath11k/hw.h b/drivers/net/wireless/ath/ath11k/hw.h
> index 52d9f4c13b136..9db984ac4321c 100644
> --- a/drivers/net/wireless/ath/ath11k/hw.h
> +++ b/drivers/net/wireless/ath/ath11k/hw.h
> @@ -134,6 +134,7 @@ struct ath11k_hw_tcl2wbm_rbm_map {
> struct ath11k_hw_hal_params {
> enum hal_rx_buf_return_buf_manager rx_buf_rbm;
> const struct ath11k_hw_tcl2wbm_rbm_map *tcl2wbm_rbm_map;
> + size_t num_tx_rings;
> };
>
> struct ath11k_hw_params {
> @@ -198,7 +199,6 @@ struct ath11k_hw_params {
> bool supports_regdb;
> bool fix_l1ss;
> bool credit_flow;
> - u8 max_tx_ring;
> const struct ath11k_hw_hal_params *hal_params;
> bool supports_dynamic_smps_6ghz;
> bool alloc_cacheable_memory;
> @@ -291,6 +291,7 @@ extern const struct ce_ie_addr ath11k_ce_ie_addr_ipq5018;
>
> extern const struct ce_remap ath11k_ce_remap_ipq5018;
>
> +extern const struct ath11k_hw_hal_params ath11k_hw_hal_params_ipq5018;
> extern const struct ath11k_hw_hal_params ath11k_hw_hal_params_ipq8074;
> extern const struct ath11k_hw_hal_params ath11k_hw_hal_params_qca6390;
> extern const struct ath11k_hw_hal_params ath11k_hw_hal_params_wcn6750;
> diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
> index 3276fe443502f..33ebe03380114 100644
> --- a/drivers/net/wireless/ath/ath11k/mac.c
> +++ b/drivers/net/wireless/ath/ath11k/mac.c
> @@ -7392,7 +7392,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 < ab->hw_params.max_tx_ring; i++) {
> + for (i = 0; i < ab->hw_params.hal_params->num_tx_rings; 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);
Reviewed-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params
2025-12-28 15:14 [PATCH v3] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params Alexandru Gagniuc
2025-12-29 3:14 ` Baochen Qiang
@ 2026-01-13 7:00 ` Vasanthakumar Thiagarajan
2026-01-14 17:24 ` Jeff Johnson
2026-01-16 1:26 ` Jeff Johnson
2 siblings, 1 reply; 8+ messages in thread
From: Vasanthakumar Thiagarajan @ 2026-01-13 7:00 UTC (permalink / raw)
To: Alexandru Gagniuc, jjohnson, ath11k
Cc: baochen.qiang, linux-wireless, linux-kernel
On 12/28/2025 8:44 PM, Alexandru Gagniuc wrote:
> ".max_tx_ring" is an upper bounds to indexing ".tcl2wbm_rbm_map". It
> is initialized in, core.c, a different file than the array. This
> spaghetti-like relation is fragile and not obvious. Accidentally
> setting ".max_tx_ring" too high leads to a hard to track out-of-
> bounds access and memory corruption.
>
> There is a small ambiguity on the meaning of "max_tx_ring":
> - The highest ring, max=3 implies there are 4 rings (0, 1, 2, 3)
> - The highest number to use for array indexing (there are 3 rings)
>
> Clarify this dependency by moving ".max_tx_ring" adjacent to the array
> ".tcl2wbm_rbm_map", and name it "num_tx_rings". Use ARRAY_SIZE()
> instead of #defines to initialize the length field.
>
> The ath11k_hw_hal_params_qca6390 uses fewer num_tx_rings than its map,
> so use a constant to express the correct value. Add a static_assert()
> to fail compilation if the constant is accidentally set too high.
Text related to static_assert to be removed accordingly.
>
> The intent is to make the code easier to understand rather than fix
> an existing bug.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
With the above minor comment addressed.
Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params
2026-01-13 7:00 ` Vasanthakumar Thiagarajan
@ 2026-01-14 17:24 ` Jeff Johnson
2026-01-14 21:29 ` Alex G.
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Johnson @ 2026-01-14 17:24 UTC (permalink / raw)
To: Vasanthakumar Thiagarajan, Alexandru Gagniuc, jjohnson, ath11k
Cc: baochen.qiang, linux-wireless, linux-kernel
On 1/12/2026 11:00 PM, Vasanthakumar Thiagarajan wrote:
>
>
> On 12/28/2025 8:44 PM, Alexandru Gagniuc wrote:
>> ".max_tx_ring" is an upper bounds to indexing ".tcl2wbm_rbm_map". It
>> is initialized in, core.c, a different file than the array. This
>> spaghetti-like relation is fragile and not obvious. Accidentally
>> setting ".max_tx_ring" too high leads to a hard to track out-of-
>> bounds access and memory corruption.
>>
>> There is a small ambiguity on the meaning of "max_tx_ring":
>> - The highest ring, max=3 implies there are 4 rings (0, 1, 2, 3)
>> - The highest number to use for array indexing (there are 3 rings)
>>
>> Clarify this dependency by moving ".max_tx_ring" adjacent to the array
>> ".tcl2wbm_rbm_map", and name it "num_tx_rings". Use ARRAY_SIZE()
>> instead of #defines to initialize the length field.
>>
>> The ath11k_hw_hal_params_qca6390 uses fewer num_tx_rings than its map,
>> so use a constant to express the correct value. Add a static_assert()
>> to fail compilation if the constant is accidentally set too high.
>
> Text related to static_assert to be removed accordingly.
I removed the last sentence in 'pending', please check:
https://git.kernel.org/pub/scm/linux/kernel/git/ath/ath.git/commit/?h=pending&id=26bb149b5e011b0f73f7b74421589cbd38e3304b
>
>>
>> The intent is to make the code easier to understand rather than fix
>> an existing bug.
>>
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>
> With the above minor comment addressed.
>
> Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params
2026-01-14 17:24 ` Jeff Johnson
@ 2026-01-14 21:29 ` Alex G.
2026-01-14 21:59 ` Jeff Johnson
0 siblings, 1 reply; 8+ messages in thread
From: Alex G. @ 2026-01-14 21:29 UTC (permalink / raw)
To: Vasanthakumar Thiagarajan, jjohnson, ath11k, Jeff Johnson
Cc: baochen.qiang, linux-wireless, linux-kernel
On Wednesday, January 14, 2026 11:24:19 AM CST Jeff Johnson wrote:
> On 1/12/2026 11:00 PM, Vasanthakumar Thiagarajan wrote:
> > On 12/28/2025 8:44 PM, Alexandru Gagniuc wrote:
> >> ".max_tx_ring" is an upper bounds to indexing ".tcl2wbm_rbm_map". It
> >> is initialized in, core.c, a different file than the array. This
> >> spaghetti-like relation is fragile and not obvious. Accidentally
> >> setting ".max_tx_ring" too high leads to a hard to track out-of-
> >> bounds access and memory corruption.
> >>
> >> There is a small ambiguity on the meaning of "max_tx_ring":
> >> - The highest ring, max=3 implies there are 4 rings (0, 1, 2, 3)
> >> - The highest number to use for array indexing (there are 3 rings)
> >>
> >> Clarify this dependency by moving ".max_tx_ring" adjacent to the array
> >> ".tcl2wbm_rbm_map", and name it "num_tx_rings". Use ARRAY_SIZE()
> >> instead of #defines to initialize the length field.
> >>
> >> The ath11k_hw_hal_params_qca6390 uses fewer num_tx_rings than its map,
> >> so use a constant to express the correct value. Add a static_assert()
> >> to fail compilation if the constant is accidentally set too high.
> >
> > Text related to static_assert to be removed accordingly.
>
Hi Jeff,
> I removed the last sentence in 'pending', please check:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/ath/ath.git/commit/?h=pendin
> g&id=26bb149b5e011b0f73f7b74421589cbd38e3304b
Re-reading the commit message, I think it makes sense to also remove the
sentence "The ath11k_hw_hal_params_qca6390 uses fewer num_tx_rings than its
map, so use a constant to express the correct value.". Do you think it's worth
submitting a v4 with this minor change?
Alex
> >> The intent is to make the code easier to understand rather than fix
> >> an existing bug.
> >>
> >> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> >
> > With the above minor comment addressed.
> >
> > Reviewed-by: Vasanthakumar Thiagarajan
> > <vasanthakumar.thiagarajan@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params
2026-01-14 21:29 ` Alex G.
@ 2026-01-14 21:59 ` Jeff Johnson
2026-01-15 4:43 ` Alex G.
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Johnson @ 2026-01-14 21:59 UTC (permalink / raw)
To: Alex G., Vasanthakumar Thiagarajan, jjohnson, ath11k
Cc: baochen.qiang, linux-wireless, linux-kernel
On 1/14/2026 1:29 PM, Alex G. wrote:
> On Wednesday, January 14, 2026 11:24:19 AM CST Jeff Johnson wrote:
>> On 1/12/2026 11:00 PM, Vasanthakumar Thiagarajan wrote:
>>> On 12/28/2025 8:44 PM, Alexandru Gagniuc wrote:
>>>> ".max_tx_ring" is an upper bounds to indexing ".tcl2wbm_rbm_map". It
>>>> is initialized in, core.c, a different file than the array. This
>>>> spaghetti-like relation is fragile and not obvious. Accidentally
>>>> setting ".max_tx_ring" too high leads to a hard to track out-of-
>>>> bounds access and memory corruption.
>>>>
>>>> There is a small ambiguity on the meaning of "max_tx_ring":
>>>> - The highest ring, max=3 implies there are 4 rings (0, 1, 2, 3)
>>>> - The highest number to use for array indexing (there are 3 rings)
>>>>
>>>> Clarify this dependency by moving ".max_tx_ring" adjacent to the array
>>>> ".tcl2wbm_rbm_map", and name it "num_tx_rings". Use ARRAY_SIZE()
>>>> instead of #defines to initialize the length field.
>>>>
>>>> The ath11k_hw_hal_params_qca6390 uses fewer num_tx_rings than its map,
>>>> so use a constant to express the correct value. Add a static_assert()
>>>> to fail compilation if the constant is accidentally set too high.
>>>
>>> Text related to static_assert to be removed accordingly.
>>
> Hi Jeff,
>
>> I removed the last sentence in 'pending', please check:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/ath/ath.git/commit/?h=pendin
>> g&id=26bb149b5e011b0f73f7b74421589cbd38e3304b
>
> Re-reading the commit message, I think it makes sense to also remove the
> sentence "The ath11k_hw_hal_params_qca6390 uses fewer num_tx_rings than its
> map, so use a constant to express the correct value.". Do you think it's worth
> submitting a v4 with this minor change?
>
> Alex
No need to submit a v4. I can make that change in 'pending'
/jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params
2026-01-14 21:59 ` Jeff Johnson
@ 2026-01-15 4:43 ` Alex G.
0 siblings, 0 replies; 8+ messages in thread
From: Alex G. @ 2026-01-15 4:43 UTC (permalink / raw)
To: Vasanthakumar Thiagarajan, jjohnson, ath11k, Jeff Johnson
Cc: baochen.qiang, linux-wireless, linux-kernel
On Wednesday, January 14, 2026 3:59:09 PM CST Jeff Johnson wrote:
> On 1/14/2026 1:29 PM, Alex G. wrote:
> > On Wednesday, January 14, 2026 11:24:19 AM CST Jeff Johnson wrote:
> >> On 1/12/2026 11:00 PM, Vasanthakumar Thiagarajan wrote:
> >>> On 12/28/2025 8:44 PM, Alexandru Gagniuc wrote:
> >>>> ".max_tx_ring" is an upper bounds to indexing ".tcl2wbm_rbm_map". It
> >>>> is initialized in, core.c, a different file than the array. This
> >>>> spaghetti-like relation is fragile and not obvious. Accidentally
> >>>> setting ".max_tx_ring" too high leads to a hard to track out-of-
> >>>> bounds access and memory corruption.
> >>>>
> >>>> There is a small ambiguity on the meaning of "max_tx_ring":
> >>>> - The highest ring, max=3 implies there are 4 rings (0, 1, 2, 3)
> >>>> - The highest number to use for array indexing (there are 3 rings)
> >>>>
> >>>> Clarify this dependency by moving ".max_tx_ring" adjacent to the array
> >>>> ".tcl2wbm_rbm_map", and name it "num_tx_rings". Use ARRAY_SIZE()
> >>>> instead of #defines to initialize the length field.
> >>>>
> >>>> The ath11k_hw_hal_params_qca6390 uses fewer num_tx_rings than its map,
> >>>> so use a constant to express the correct value. Add a static_assert()
> >>>> to fail compilation if the constant is accidentally set too high.
> >>>
> >>> Text related to static_assert to be removed accordingly.
> >
> > Hi Jeff,
> >
> >> I removed the last sentence in 'pending', please check:
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/ath/ath.git/commit/?h=pen
> >> din g&id=26bb149b5e011b0f73f7b74421589cbd38e3304b
> >
> > Re-reading the commit message, I think it makes sense to also remove the
> > sentence "The ath11k_hw_hal_params_qca6390 uses fewer num_tx_rings than
> > its
> > map, so use a constant to express the correct value.". Do you think it's
> > worth submitting a v4 with this minor change?
> >
> > Alex
>
> No need to submit a v4. I can make that change in 'pending'
Thank you!
> /jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params
2025-12-28 15:14 [PATCH v3] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params Alexandru Gagniuc
2025-12-29 3:14 ` Baochen Qiang
2026-01-13 7:00 ` Vasanthakumar Thiagarajan
@ 2026-01-16 1:26 ` Jeff Johnson
2 siblings, 0 replies; 8+ messages in thread
From: Jeff Johnson @ 2026-01-16 1:26 UTC (permalink / raw)
To: jjohnson, ath11k, Alexandru Gagniuc
Cc: baochen.qiang, linux-wireless, linux-kernel
On Sun, 28 Dec 2025 09:14:05 -0600, Alexandru Gagniuc wrote:
> ".max_tx_ring" is an upper bounds to indexing ".tcl2wbm_rbm_map". It
> is initialized in, core.c, a different file than the array. This
> spaghetti-like relation is fragile and not obvious. Accidentally
> setting ".max_tx_ring" too high leads to a hard to track out-of-
> bounds access and memory corruption.
>
> There is a small ambiguity on the meaning of "max_tx_ring":
> - The highest ring, max=3 implies there are 4 rings (0, 1, 2, 3)
> - The highest number to use for array indexing (there are 3 rings)
>
> [...]
Applied, thanks!
[1/1] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params
commit: b515730ec3d231aa36b6177524532fc7d94f1750
Best regards,
--
Jeff Johnson <jeff.johnson@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-01-16 1:26 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-28 15:14 [PATCH v3] wifi: ath11k: move .max_tx_ring to struct ath11k_hw_hal_params Alexandru Gagniuc
2025-12-29 3:14 ` Baochen Qiang
2026-01-13 7:00 ` Vasanthakumar Thiagarajan
2026-01-14 17:24 ` Jeff Johnson
2026-01-14 21:29 ` Alex G.
2026-01-14 21:59 ` Jeff Johnson
2026-01-15 4:43 ` Alex G.
2026-01-16 1:26 ` Jeff Johnson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox