Message-ID: <809038ef-dd0b-4b42-9357-f4f0825a4857@oss.qualcomm.com>
Date: Wed, 5 Nov 2025 10:40:32 +0800
From: Baochen Qiang <baochen.qiang@....qualcomm.com>
To: "Yu Zhang(Yuriy)" <yu.zhang@....qualcomm.com>, jjohnson@...nel.org
Cc: linux-kernel@...r.kernel.org, linux-wireless@...r.kernel.org,
        ath11k@...ts.infradead.org,
        Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@....qualcomm.com>
Subject: Re: [PATCH ath-next v2 1/6] wifi: ath11k: Add initialization and
 deinitialization sequence for CFR module



On 11/5/2025 12:26 AM, Yu Zhang(Yuriy) wrote:
> From: Venkateswara Naralasetty <quic_vnaralas@...cinc.com>
> 
> Channel Frequency Response (CFR) module will be initialized only when
> the following criteria passes:
>  * Enabled CFR support for the hardware through the hardware param
>    'cfr_support'
>  * WMI service enabled for the CFR support
>    'WMI_TLV_SERVICE_CFR_CAPTURE_SUPPORT'
> 
> Also, provide a configuration option CONFIG_ATH11K_CFR to enable CFR
> feature support during the compilation time.
> 
> CFR module initialization includes Direct Buffer(DB) ring initialization
> where hardware uses the DB ring buffers to copy CFR data to host.
> Number of buffers and buffer size of the ring is based on the DB ring
> capabilities advertised by the firmware through WMI service ready.
> Also ring configurations are sent to firmware through
> ath11k_dbring_wmi_cfg_setup().
> 
> Predefine ath11k_cfr_dma_hdr, ath11k_look_up_table, and ath11k_cfr
> structs and fields for subsequent patches.
> 
> Tested-on: IPQ8074 hw2.0 PCI IPQ8074 WLAN.HK.2.5.0.1-00991-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN6855 hw2.1 PCI WLAN.HSP.1.1-04685-QCAHSPSWPL_V1_V2_SILICONZ_IOE-1
> 
> Signed-off-by: Venkateswara Naralasetty <quic_vnaralas@...cinc.com>
> Co-developed-by: Yu Zhang (Yuriy) <yu.zhang@....qualcomm.com>
> Signed-off-by: Yu Zhang (Yuriy) <yu.zhang@....qualcomm.com>
> Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@....qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath11k/Kconfig  |  11 ++
>  drivers/net/wireless/ath/ath11k/Makefile |   1 +
>  drivers/net/wireless/ath/ath11k/cfr.c    | 160 +++++++++++++++++++++++
>  drivers/net/wireless/ath/ath11k/cfr.h    |  84 ++++++++++++
>  drivers/net/wireless/ath/ath11k/core.c   |  41 +++++-
>  drivers/net/wireless/ath/ath11k/core.h   |   8 +-
>  drivers/net/wireless/ath/ath11k/dbring.c |  40 ++++--
>  drivers/net/wireless/ath/ath11k/dbring.h |   6 +-
>  drivers/net/wireless/ath/ath11k/hal.c    |   3 +-
>  drivers/net/wireless/ath/ath11k/hw.h     |   5 +-
>  drivers/net/wireless/ath/ath11k/wmi.h    |   1 +
>  11 files changed, 343 insertions(+), 17 deletions(-)
>  create mode 100644 drivers/net/wireless/ath/ath11k/cfr.c
>  create mode 100644 drivers/net/wireless/ath/ath11k/cfr.h
> 
> diff --git a/drivers/net/wireless/ath/ath11k/Kconfig b/drivers/net/wireless/ath/ath11k/Kconfig
> index 659ef134ef16..47dfd39caa89 100644
> --- a/drivers/net/wireless/ath/ath11k/Kconfig
> +++ b/drivers/net/wireless/ath/ath11k/Kconfig
> @@ -58,3 +58,14 @@ config ATH11K_SPECTRAL
>  	  Enable ath11k spectral scan support
>  
>  	  Say Y to enable access to the FFT/spectral data via debugfs.
> +
> +config ATH11K_CFR
> +	bool "ath11k channel frequency response support"
> +	depends on ATH11K_DEBUGFS
> +	depends on RELAY
> +	help
> +	  Enable ath11k channel frequency response dump support.
> +	  This option exposes debugfs nodes that will allow the user
> +	  to enable, disable, and dump data.
> +
> +	  Say Y to enable CFR data dump collection via debugfs.
> diff --git a/drivers/net/wireless/ath/ath11k/Makefile b/drivers/net/wireless/ath/ath11k/Makefile
> index d9092414b362..b1435fcf3e1b 100644
> --- a/drivers/net/wireless/ath/ath11k/Makefile
> +++ b/drivers/net/wireless/ath/ath11k/Makefile
> @@ -28,6 +28,7 @@ ath11k-$(CONFIG_THERMAL) += thermal.o
>  ath11k-$(CONFIG_ATH11K_SPECTRAL) += spectral.o
>  ath11k-$(CONFIG_PM) += wow.o
>  ath11k-$(CONFIG_DEV_COREDUMP) += coredump.o
> +ath11k-$(CONFIG_ATH11K_CFR) += cfr.o
>  
>  obj-$(CONFIG_ATH11K_AHB) += ath11k_ahb.o
>  ath11k_ahb-y += ahb.o
> diff --git a/drivers/net/wireless/ath/ath11k/cfr.c b/drivers/net/wireless/ath/ath11k/cfr.c
> new file mode 100644
> index 000000000000..73ffe3510ffe
> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath11k/cfr.c
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: BSD-3-Clause-Clear
> +/*
> + * Copyright (c) 2020-2021 The Linux Foundation. All rights reserved.
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include <linux/relay.h>
> +#include "core.h"
> +#include "debug.h"
> +
> +static int ath11k_cfr_process_data(struct ath11k *ar,
> +				   struct ath11k_dbring_data *param)
> +{
> +	return 0;
> +}
> +
> +void ath11k_cfr_lut_update_paddr(struct ath11k *ar, dma_addr_t paddr,
> +				 u32 buf_id)
> +{
> +	struct ath11k_cfr *cfr = &ar->cfr;
> +
> +	if (cfr->lut)
> +		cfr->lut[buf_id].dbr_address = paddr;
> +}
> +
> +static void ath11k_cfr_ring_free(struct ath11k *ar)
> +{
> +	struct ath11k_cfr *cfr = &ar->cfr;
> +
> +	ath11k_dbring_buf_cleanup(ar, &cfr->rx_ring);
> +	ath11k_dbring_srng_cleanup(ar, &cfr->rx_ring);
> +}
> +
> +static int ath11k_cfr_ring_alloc(struct ath11k *ar,
> +				 struct ath11k_dbring_cap *db_cap)
> +{
> +	struct ath11k_cfr *cfr = &ar->cfr;
> +	int ret;
> +
> +	ret = ath11k_dbring_srng_setup(ar, &cfr->rx_ring,
> +				       ATH11K_CFR_NUM_RING_ENTRIES,
> +				       db_cap->min_elem);
> +	if (ret) {
> +		ath11k_warn(ar->ab, "failed to setup db ring: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ath11k_dbring_set_cfg(ar, &cfr->rx_ring,
> +			      ATH11K_CFR_NUM_RESP_PER_EVENT,
> +			      ATH11K_CFR_EVENT_TIMEOUT_MS,
> +			      ath11k_cfr_process_data);
> +
> +	ret = ath11k_dbring_buf_setup(ar, &cfr->rx_ring, db_cap);
> +	if (ret) {
> +		ath11k_warn(ar->ab, "failed to setup db ring buffer: %d\n", ret);
> +		goto srng_cleanup;
> +	}
> +
> +	ret = ath11k_dbring_wmi_cfg_setup(ar, &cfr->rx_ring, WMI_DIRECT_BUF_CFR);
> +	if (ret) {
> +		ath11k_warn(ar->ab, "failed to setup db ring cfg: %d\n", ret);
> +		goto buffer_cleanup;
> +	}
> +
> +	return 0;
> +
> +buffer_cleanup:
> +	ath11k_dbring_buf_cleanup(ar, &cfr->rx_ring);
> +srng_cleanup:
> +	ath11k_dbring_srng_cleanup(ar, &cfr->rx_ring);
> +	return ret;
> +}
> +
> +void ath11k_cfr_deinit(struct ath11k_base *ab)
> +{
> +	struct ath11k_cfr *cfr;
> +	struct ath11k *ar;
> +	int i;
> +
> +	if (!test_bit(WMI_TLV_SERVICE_CFR_CAPTURE_SUPPORT, ab->wmi_ab.svc_map) ||
> +	    !ab->hw_params.cfr_support)
> +		return;
> +
> +	for (i = 0; i <  ab->num_radios; i++) {
> +		ar = ab->pdevs[i].ar;
> +		cfr = &ar->cfr;
> +

don't you need to check whether the radio has WMI_DIRECT_BUF_CFR capability just as what
is done in the init path?

And for the other functions where CFR is involved, do we need to check it as well?
Checking spectral feature which seems to be similar to CFR, there is a 'enabled' flag
there indicating whether this capability is supported and initialized, maybe we need it as
well?

> +		ath11k_cfr_ring_free(ar);
> +
> +		spin_lock_bh(&cfr->lut_lock);
> +		kfree(cfr->lut);
> +		cfr->lut = NULL;
> +		spin_unlock_bh(&cfr->lut_lock);
> +	}
> +}
> +
> +int ath11k_cfr_init(struct ath11k_base *ab)
> +{
> +	struct ath11k_dbring_cap db_cap;
> +	struct ath11k_cfr *cfr;
> +	u32 num_lut_entries;
> +	struct ath11k *ar;
> +	int i, ret;
> +
> +	if (!test_bit(WMI_TLV_SERVICE_CFR_CAPTURE_SUPPORT, ab->wmi_ab.svc_map) ||
> +	    !ab->hw_params.cfr_support)
> +		return 0;
> +
> +	for (i = 0; i < ab->num_radios; i++) {
> +		ar = ab->pdevs[i].ar;
> +		cfr = &ar->cfr;
> +
> +		ret = ath11k_dbring_get_cap(ar->ab, ar->pdev_idx,
> +					    WMI_DIRECT_BUF_CFR, &db_cap);
> +		if (ret)
> +			continue;
> +
> +		idr_init(&cfr->rx_ring.bufs_idr);
> +		spin_lock_init(&cfr->rx_ring.idr_lock);
> +		spin_lock_init(&cfr->lock);
> +		spin_lock_init(&cfr->lut_lock);
> +
> +		num_lut_entries = min_t(u32, CFR_MAX_LUT_ENTRIES, db_cap.min_elem);
> +		cfr->lut = kcalloc(num_lut_entries, sizeof(*cfr->lut),
> +				   GFP_KERNEL);
> +		if (!cfr->lut) {
> +			ret = -ENOMEM;
> +			goto err;
> +		}
> +
> +		ret = ath11k_cfr_ring_alloc(ar, &db_cap);
> +		if (ret) {
> +			ath11k_warn(ab, "failed to init cfr ring for pdev %d: %d\n",
> +				    i, ret);
> +			spin_lock_bh(&cfr->lut_lock);
> +			kfree(cfr->lut);
> +			cfr->lut = NULL;
> +			spin_unlock_bh(&cfr->lut_lock);
> +			goto err;
> +		}
> +
> +		cfr->lut_num = num_lut_entries;
> +	}
> +
> +	return 0;
> +
> +err:
> +	for (i = i - 1; i >= 0; i--) {
> +		ar = ab->pdevs[i].ar;
> +		cfr = &ar->cfr;
> +

same here
> +		ath11k_cfr_ring_free(ar);
> +
> +		spin_lock_bh(&cfr->lut_lock);
> +		kfree(cfr->lut);
> +		cfr->lut = NULL;
> +		spin_unlock_bh(&cfr->lut_lock);
> +	}
> +	return ret;
> +}

Powered by blists - more mailing lists