On 10/25/2024 3:39 AM, Dmitry Baryshkov wrote:
On Thu, Oct 24, 2024 at 08:25:14AM +0800, Miaoqing Pan wrote:
QCA6698AQ IP core is the same as WCN6855 hw2.1, but it has different RF,
IPA, thermal, RAM size and etc, so new firmware files used. This change
allows board DT files to override the subdir of the firmware directory
used to lookup the amss.bin and m3.bin.
I have slight concerns regarding the _board_ DT files overriding the
subdir. This opens a can of worms, allowing per-board firmware sets,
which (as far as I understand) is far from being what driver maintainers
would like to see. This was required for ath10k-snoc devices, since
firmware for those platforms is signed by the vendor keys and it is
limited to a particular SoC or SoC family. For ath11k-pci there is no
such limitation.
Would it be possible to use PCI subvendor / subdev to identify affected
cards? PCI Revision? Any other way to identify the device? Please
provide lspci -nnvv for the affected device kind. Is there a way to
identify the RF part somehow?
It's rather difficult, for WCN685x, there are multiple evolved subseries
for customized products. e.g.
QCA6698AQ/hw2.1
QCA2066/hw2.1
WCN6855/hw2.0/hw2.1
WCN6856/hw2.1
They have the same PCIe ID (17cb:1103), the commit 5dc9d1a55e95 ("wifi:
ath11k: add support for QCA2066") reads TCSR_SOC_HW_SUB_VER to enumerate
all QCA2066 cards, it lacks of flexibility, as the list will become
longer and longer. But it's the only choice for QCA2066, as it's
customized for X86 platform which without DT files.
So for MSM those have DT file platforms, like SA8775P-RIDE/QCS8300-RIDE
both attached to QCA6698AQ, we can specify the correct firmware to
'ath11k/WCN6855/hw2.1/qca6698aq', so it's not per-board firmware, it
depends on the type of the products(x86 windows, IoT products or AUTO).
0000:01:00.0 Network controller [0280]: Qualcomm QCNFA765 Wireless
Network Adapter [17cb:1103] (rev 01)
Subsystem: Qualcomm QCNFA765 Wireless Network Adapter [17cb:0108]
Device tree node: /sys/firmware/devicetree/base/pci@1c00000/pcie@0/wifi@0
Could you possibly clarify, how this situation is handled in Windows
world?
X86 platforms use standard m.2 PCIe card, and it will only use the
default main firmware files, as they without DT files.
For example:
- ath11k/WCN6855/hw2.1/amss.bin,
ath11k/WCN6855/hw2.1/m3.bin: main firmware files, used by default
- ath11k/WCN6855/hw2.1/qca6698aq/amss.bin,
ath11k/WCN6855/hw2.1/qca6698aq/m3.bin
This approach looks good to me, thank you.
Tested-on: QCA6698AQ hw2.1 PCI WLAN.HSP.1.1-04402-QCAHSPSWPL_V1_V2_SILICONZ_IOE-1
Signed-off-by: Miaoqing Pan <quic_miaoqing@xxxxxxxxxxx>
---
drivers/net/wireless/ath/ath11k/core.c | 16 ++++++++++++++++
drivers/net/wireless/ath/ath11k/core.h | 11 +++--------
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index be67382c00f6..775e48551522 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -1178,6 +1178,22 @@ static int ath11k_core_create_chip_id_board_name(struct ath11k_base *ab, char *n
ATH11K_BDF_NAME_CHIP_ID);
}
+void ath11k_core_create_firmware_path(struct ath11k_base *ab,
+ const char *filename,
+ void *buf, size_t buf_len)
+{ const char *board_name = NULL;
+
+ of_property_read_string(ab->dev->of_node, "firmware-name", &board_name);
soc_name rather than board_name, please. Or just fw_name.
Will update to 'fw_name'.
+
+ if (board_name)
+ snprintf(buf, buf_len, "%s/%s/%s/%s", ATH11K_FW_DIR,
+ ab->hw_params.fw.dir, board_name, filename);
+ else
+ snprintf(buf, buf_len, "%s/%s/%s", ATH11K_FW_DIR,
+ ab->hw_params.fw.dir, filename);
+}
+EXPORT_SYMBOL(ath11k_core_create_firmware_path);
+
const struct firmware *ath11k_core_firmware_request(struct ath11k_base *ab,
const char *file)
{
diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index 09c37e19a168..ce4102cfed4d 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -1249,6 +1249,9 @@ bool ath11k_core_coldboot_cal_support(struct ath11k_base *ab);
const struct firmware *ath11k_core_firmware_request(struct ath11k_base *ab,
const char *filename);
+void ath11k_core_create_firmware_path(struct ath11k_base *ab,
+ const char *filename,
+ void *buf, size_t buf_len);
static inline const char *ath11k_scan_state_str(enum ath11k_scan_state state)
{
@@ -1295,14 +1298,6 @@ static inline struct ath11k *ath11k_ab_to_ar(struct ath11k_base *ab,
return ab->pdevs[ath11k_hw_mac_id_to_pdev_id(&ab->hw_params, mac_id)].ar;
}
-static inline void ath11k_core_create_firmware_path(struct ath11k_base *ab,
- const char *filename,
- void *buf, size_t buf_len)
-{
- snprintf(buf, buf_len, "%s/%s/%s", ATH11K_FW_DIR,
- ab->hw_params.fw.dir, filename);
It could have perfectly lived here. Is there any reason to move the
function?
Will update.
-}
-
static inline const char *ath11k_bus_str(enum ath11k_bus bus)
{
switch (bus) {
--
2.25.1
