QCA6698AQ uses different firmware/bdf/regdb with existing WCN6855
firmware, which is customized for IoE platforms. And the 'pci-device-id +
soc-hw-version + soc-hw-sub-version' may not be enough to identify the
correct firmware directory path.

The device tree allows "firmware-name" to define the firmware path,
    wifi@c000000 {
        firmware-name = "QCA6698AQ";
        status = "okay";
    };

Tested-on: QCA6698AQ hw2.1 PCI WLAN.HSP.1.1-04402-QCAHSPSWPL_V1_V2_SILICONZ_IOE-1

Signed-off-by: Miaoqing Pan <quic_miaoqing@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/core.c | 12 ++++++++++++
 drivers/net/wireless/ath/ath11k/core.h | 11 +++--------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index be67382c00f6..7720f467b11b 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -1178,6 +1178,18 @@ 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 *variant = NULL;
+
+	of_property_read_string(ab->dev->of_node, "firmware-name", &variant);
+
+	snprintf(buf, buf_len, "%s/%s/%s", ATH11K_FW_DIR,
+		 variant ? : 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);
-}
-
 static inline const char *ath11k_bus_str(enum ath11k_bus bus)
 {
 	switch (bus) {
-- 
2.25.1

On Tue, Oct 01, 2024 at 11:30:52AM +0800, Miaoqing Pan wrote:
> QCA6698AQ uses different firmware/bdf/regdb with existing WCN6855
> firmware, which is customized for IoE platforms. And the 'pci-device-id +
> soc-hw-version + soc-hw-sub-version' may not be enough to identify the
> correct firmware directory path.

Why is it so? What makes it so different from the existing platforms
that you can not use WCN6855 firmware?

> 
> The device tree allows "firmware-name" to define the firmware path,
>     wifi@c000000 {

You are describing platform node, while the commit message talks about
the PCIe devices. Could you please clarify, whether it is a PCIe device
or an AHB device?

>         firmware-name = "QCA6698AQ";

Could we please follow the approach that has been defined in the commit
5abf259772df ("wifi: ath10k: support board-specific firmware
overrides")? In other words, instead of creating another directory under
ath11k, create a subdir under the WCN6855/hwN.M/ which contains your
device-specific data.

>         status = "okay";
>     };
> 
> Tested-on: QCA6698AQ hw2.1 PCI WLAN.HSP.1.1-04402-QCAHSPSWPL_V1_V2_SILICONZ_IOE-1
> 
> Signed-off-by: Miaoqing Pan <quic_miaoqing@quicinc.com>

P.S. please CC linux-arm-msm for future respins of this series or for
all other submissions that concern board-specific DT data on MSM
platforms.

> ---
>  drivers/net/wireless/ath/ath11k/core.c | 12 ++++++++++++
>  drivers/net/wireless/ath/ath11k/core.h | 11 +++--------
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 

-- 
With best wishes
Dmitry

On 10/22/2024 5:57 PM, Dmitry Baryshkov wrote:
> On Tue, Oct 01, 2024 at 11:30:52AM +0800, Miaoqing Pan wrote:
>> QCA6698AQ uses different firmware/bdf/regdb with existing WCN6855
>> firmware, which is customized for IoE platforms. And the 'pci-device-id +
>> soc-hw-version + soc-hw-sub-version' may not be enough to identify the
>> correct firmware directory path.
> 
> Why is it so? What makes it so different from the existing platforms
> that you can not use WCN6855 firmware?

Just as I said, a new customized firmware for IoE devices.

> 
>>
>> The device tree allows "firmware-name" to define the firmware path,
>>      wifi@c000000 {
> 
> You are describing platform node, while the commit message talks about
> the PCIe devices. Could you please clarify, whether it is a PCIe device
> or an AHB device?

PCIe device. The change is for sa8775p/qcs8300 those non-AHB boards.

> 
>>          firmware-name = "QCA6698AQ";
> 
> Could we please follow the approach that has been defined in the commit
> 5abf259772df ("wifi: ath10k: support board-specific firmware
> overrides")? In other words, instead of creating another directory under
> ath11k, create a subdir under the WCN6855/hwN.M/ which contains your
> device-specific data.

Sure, thanks, will update.

> 
>>          status = "okay";
>>      };
>>
>> Tested-on: QCA6698AQ hw2.1 PCI WLAN.HSP.1.1-04402-QCAHSPSWPL_V1_V2_SILICONZ_IOE-1
>>
>> Signed-off-by: Miaoqing Pan <quic_miaoqing@quicinc.com>
> 
> P.S. please CC linux-arm-msm for future respins of this series or for
> all other submissions that concern board-specific DT data on MSM
> platforms.

ok.

> 
>> ---
>>   drivers/net/wireless/ath/ath11k/core.c | 12 ++++++++++++
>>   drivers/net/wireless/ath/ath11k/core.h | 11 +++--------
>>   2 files changed, 15 insertions(+), 8 deletions(-)
>>
>

Miaoqing Pan <quic_miaoqing@quicinc.com> writes:

> On 10/22/2024 5:57 PM, Dmitry Baryshkov wrote:
>> On Tue, Oct 01, 2024 at 11:30:52AM +0800, Miaoqing Pan wrote:
>>> QCA6698AQ uses different firmware/bdf/regdb with existing WCN6855
>>> firmware, which is customized for IoE platforms. And the 'pci-device-id +
>>> soc-hw-version + soc-hw-sub-version' may not be enough to identify the
>>> correct firmware directory path.
>> Why is it so? What makes it so different from the existing platforms
>> that you can not use WCN6855 firmware?
>
> Just as I said, a new customized firmware for IoE devices.

I know in Qualcomm it's common practise to fork the firmware multiple
times per project and what not, but in the community the preference is
to have one mainline branch. Having different firmware forks/branches is
a lot more difficult to maintain.

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

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

On Tue, 22 Oct 2024 at 13:20, Miaoqing Pan <quic_miaoqing@quicinc.com> wrote:
>
>
>
> On 10/22/2024 5:57 PM, Dmitry Baryshkov wrote:
> > On Tue, Oct 01, 2024 at 11:30:52AM +0800, Miaoqing Pan wrote:
> >> QCA6698AQ uses different firmware/bdf/regdb with existing WCN6855
> >> firmware, which is customized for IoE platforms. And the 'pci-device-id +
> >> soc-hw-version + soc-hw-sub-version' may not be enough to identify the
> >> correct firmware directory path.
> >
> > Why is it so? What makes it so different from the existing platforms
> > that you can not use WCN6855 firmware?
>
> Just as I said, a new customized firmware for IoE devices.

This answers the "what" question, not "why". Please provide a reason
and a description. Can the hardware work with the default firmware?
Does your custom firmware provide additional features? Is it just
signed for a different SoC? Is there anything else?

> >> The device tree allows "firmware-name" to define the firmware path,
> >>      wifi@c000000 {
> >
> > You are describing platform node, while the commit message talks about
> > the PCIe devices. Could you please clarify, whether it is a PCIe device
> > or an AHB device?
>
> PCIe device. The change is for sa8775p/qcs8300 those non-AHB boards.

Then why are you patching the AHB schema and why are you providing a
platform-based example? It makes it harder to follow your arguments.

>
> >
> >>          firmware-name = "QCA6698AQ";
> >
> > Could we please follow the approach that has been defined in the commit
> > 5abf259772df ("wifi: ath10k: support board-specific firmware
> > overrides")? In other words, instead of creating another directory under
> > ath11k, create a subdir under the WCN6855/hwN.M/ which contains your
> > device-specific data.
>
> Sure, thanks, will update.
>
> >
> >>          status = "okay";
> >>      };
> >>
> >> Tested-on: QCA6698AQ hw2.1 PCI WLAN.HSP.1.1-04402-QCAHSPSWPL_V1_V2_SILICONZ_IOE-1
> >>
> >> Signed-off-by: Miaoqing Pan <quic_miaoqing@quicinc.com>
> >
> > P.S. please CC linux-arm-msm for future respins of this series or for
> > all other submissions that concern board-specific DT data on MSM
> > platforms.
>
> ok.
>
> >
> >> ---
> >>   drivers/net/wireless/ath/ath11k/core.c | 12 ++++++++++++
> >>   drivers/net/wireless/ath/ath11k/core.h | 11 +++--------
> >>   2 files changed, 15 insertions(+), 8 deletions(-)
> >>
> >
>


-- 
With best wishes
Dmitry