| Message ID | 20240427162644.2470886-1-quic_tamizhr@quicinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | wifi: ath12k: Remove unsupported and unused ring configurations | expand |
On 4/27/2024 9:56 PM, Tamizh Chelvam Raja wrote: > Currently in driver doing memory allocation for tx_monitor, > tcl_cmd_ring and tcl_status ring. Here driver support for > tx_monitor mode is not there and memory for tcl_cmd and tcl_status > rings are allocated by firmware and it uses that memory instead of > host allocated. So avoid these unused ring setup configuration. > > Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1 > > Tamizh Chelvam Raja (3): > wifi: ath12k: fix calling correct function for rx monitor mode > wifi: ath12k: Remove unsupported tx monitor handling > wifi: ath12k: Remove unused tcl_*_ring configuration > > v2: > * Rebased on top of ToT > > drivers/net/wireless/ath/ath12k/dp.c | 16 ------- > drivers/net/wireless/ath/ath12k/dp.h | 2 - > drivers/net/wireless/ath/ath12k/dp_mon.c | 40 +---------------- > drivers/net/wireless/ath/ath12k/dp_rx.c | 56 ------------------------ > drivers/net/wireless/ath/ath12k/dp_tx.c | 44 +------------------ > drivers/net/wireless/ath/ath12k/dp_tx.h | 1 - > 6 files changed, 2 insertions(+), 157 deletions(-) > > It nice to see code removal in general :) But I've also seen some concerns internally around code removal especially when the code will have to be re-added in future while properly supporting the feature. I guess the cover letter may need to clarify those concerns at least for internal review. Other than that, the series LGTM Vasanth
On 4/28/2024 8:59 AM, Vasanthakumar Thiagarajan wrote: > > > On 4/27/2024 9:56 PM, Tamizh Chelvam Raja wrote: >> Currently in driver doing memory allocation for tx_monitor, >> tcl_cmd_ring and tcl_status ring. Here driver support for >> tx_monitor mode is not there and memory for tcl_cmd and tcl_status >> rings are allocated by firmware and it uses that memory instead of >> host allocated. So avoid these unused ring setup configuration. >> >> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1 >> >> Tamizh Chelvam Raja (3): >> wifi: ath12k: fix calling correct function for rx monitor mode >> wifi: ath12k: Remove unsupported tx monitor handling >> wifi: ath12k: Remove unused tcl_*_ring configuration >> >> v2: >> * Rebased on top of ToT >> >> drivers/net/wireless/ath/ath12k/dp.c | 16 ------- >> drivers/net/wireless/ath/ath12k/dp.h | 2 - >> drivers/net/wireless/ath/ath12k/dp_mon.c | 40 +---------------- >> drivers/net/wireless/ath/ath12k/dp_rx.c | 56 ------------------------ >> drivers/net/wireless/ath/ath12k/dp_tx.c | 44 +------------------ >> drivers/net/wireless/ath/ath12k/dp_tx.h | 1 - >> 6 files changed, 2 insertions(+), 157 deletions(-) >> >> > > It nice to see code removal in general :) > But I've also seen some concerns internally around code removal > especially when the code will have to be re-added in future while > properly supporting the feature. I guess the cover letter > may need to clarify those concerns at least for internal review. > This is mainly to avoid unnecessary memory allocation for the unused rings. And this can be added in the future while enabling the feature. > Other than that, the series LGTM > > Vasanth Tamizh
On 4/29/2024 9:54 AM, Tamizh Chelvam Raja wrote: > On 4/28/2024 8:59 AM, Vasanthakumar Thiagarajan wrote: >> >> >> On 4/27/2024 9:56 PM, Tamizh Chelvam Raja wrote: >>> Currently in driver doing memory allocation for tx_monitor, >>> tcl_cmd_ring and tcl_status ring. Here driver support for >>> tx_monitor mode is not there and memory for tcl_cmd and tcl_status >>> rings are allocated by firmware and it uses that memory instead of >>> host allocated. So avoid these unused ring setup configuration. >>> >>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1 >>> >>> Tamizh Chelvam Raja (3): >>> wifi: ath12k: fix calling correct function for rx monitor mode >>> wifi: ath12k: Remove unsupported tx monitor handling >>> wifi: ath12k: Remove unused tcl_*_ring configuration >>> >>> v2: >>> * Rebased on top of ToT >>> >>> drivers/net/wireless/ath/ath12k/dp.c | 16 ------- >>> drivers/net/wireless/ath/ath12k/dp.h | 2 - >>> drivers/net/wireless/ath/ath12k/dp_mon.c | 40 +---------------- >>> drivers/net/wireless/ath/ath12k/dp_rx.c | 56 ------------------------ >>> drivers/net/wireless/ath/ath12k/dp_tx.c | 44 +------------------ >>> drivers/net/wireless/ath/ath12k/dp_tx.h | 1 - >>> 6 files changed, 2 insertions(+), 157 deletions(-) >>> >>> >> >> It nice to see code removal in general :) >> But I've also seen some concerns internally around code removal >> especially when the code will have to be re-added in future while >> properly supporting the feature. I guess the cover letter >> may need to clarify those concerns at least for internal review. >> > This is mainly to avoid unnecessary memory allocation for the unused rings. > And this can be added in the future while enabling the feature. I agree. I think I somehow overlooked the memory optimization part. Thanks for the clarification. Vasanth