[PATCH v3 0/4] wifi: ath12k: Fix endianness handling in QMI

[PATCH v3 0/4] wifi: ath12k: Fix endianness handling in QMI

[Alexander Wilhelm]

Fix endianness handling in QMI firmware transfer on big-endian platforms.
Without this fix, the firmware download fails due to misinterpreted data
structures exchanged between the host and the wireless module.

The issue occurs during early bring-up on big endian systems, where QMI
messages are not correctly parsed by the driver, leading to failed
initialization sequences. Ensure all relevant fields are properly converted
between CPU and little-endian format in request and response messages, as
expected by the firmware. Attached logs showing the failure before the fix:

    ath12k_pci 0001:01:00.0: BAR 0: assigned [mem 0xc00000000-0xc001fffff 64bit]
    ath12k_pci 0001:01:00.0: boot pci_mem 0xcd4148e9
    ath12k_pci 0001:01:00.0: pci probe 17cb:1109 17cb:1109
    ath12k_pci 0001:01:00.0: pci tcsr_soc_hw_version major 2 minor 0
    ath12k_pci 0001:01:00.0: request MSI one vector
    ath12k_pci 0001:01:00.0: MSI vectors: 1
    ath12k_pci 0001:01:00.0: msi base data is 0
    ath12k_pci 0001:01:00.0: Hardware name: qcn9274 hw2.0
    ath12k_pci 0001:01:00.0: boot firmware request ath12k/QCN9274/hw2.0/firmware-2.bin size 15134776
    ath12k_pci 0001:01:00.0: found fw timestamp 1722934582
    ath12k_pci 0001:01:00.0: found m3 image ie (421880 B)
    ath12k_pci 0001:01:00.0: found fw image ie (7229440 B)
    ath12k_pci 0001:01:00.0: found dualmac fw image ie (7483392 B)
    ath12k_pci 0001:01:00.0: found firmware features ie (1 B)
    ath12k_pci 0001:01:00.0: features
    ath12k_pci 0001:01:00.0: using fw api 2
    ath12k_pci 0001:01:00.0: dualmac fw selected for board id: 1005
    ath12k_pci 0001:01:00.0: Assign MSI to user: MHI, num_vectors: 3, user_base_data: 0, base_vector: 0
    ath12k_pci 0001:01:00.0: Number of assigned MSI for MHI is 3, base vector is 0
    ath12k_pci 0001:01:00.0: Assign MSI to user: CE, num_vectors: 1, user_base_data: 0, base_vector: 0
    ath12k_pci 0001:01:00.0: Assign MSI to user: DP, num_vectors: 1, user_base_data: 0, base_vector: 0
    ath12k_pci 0001:01:00.0: irq:18 group:0
    ath12k_pci 0001:01:00.0: irq:18 group:1
    ath12k_pci 0001:01:00.0: irq:18 group:2
    ath12k_pci 0001:01:00.0: irq:18 group:3
    ath12k_pci 0001:01:00.0: irq:18 group:4
    ath12k_pci 0001:01:00.0: irq:18 group:5
    ath12k_pci 0001:01:00.0: irq:18 group:6
    ath12k_pci 0001:01:00.0: irq:18 group:7
    ath12k_pci 0001:01:00.0: pci after request_irq msi_ep_base_data 0
    ath12k_pci 0001:01:00.0: cookie:0x0
    ath12k_pci 0001:01:00.0: WLAON_WARM_SW_ENTRY 0x2
    ath12k_pci 0001:01:00.0: WLAON_WARM_SW_ENTRY 0x2
    ath12k_pci 0001:01:00.0: soc reset cause:0
    ath12k_pci 0001:01:00.0: MHISTATUS 0xff04
    ath12k_pci 0001:01:00.0: pci link_ctl 0x0000 L0s 0 L1 0
    ath12k_pci 0001:01:00.0: pci reg 0x3164 instance 0x11 read val 0x11
    ath12k_pci 0001:01:00.0: setting mhi state: INIT(0)
    ath12k_pci 0001:01:00.0: setting mhi state: POWER_ON(2)
    ath12k_pci 0001:01:00.0: mhi notify status reason UNKNOWN
    ath12k_pci 0001:01:00.0: mhi notify status reason MHI_CB_EE_MISSION_MODE
    ath12k_pci 0001:01:00.0: qmi wifi fw qmi service connected
    ath12k_pci 0001:01:00.0: phy capability resp valid 1 num_phy 2 valid 1 board_id 84934656 valid 1 single_chip_mlo_support 0
    ath12k_pci 0001:01:00.0: intra device MLO is disabled hence skip QMI MLO cap
---
Changes in v3:
- Rebase on latest ath master
- Using a generic encoding/decoding macro causes sparse to complain about
  type violations. Use separate macros for each basic element type instead.

Changes in v2:
- Handle QMI conversion within the QMI subsystem instead of the driver.

Alexander Wilhelm (4):
  soc: qcom: introduce new QMI encode/decode macros
  soc: qcom: fix QMI encoding/decoding for basic elements
  soc: qcom: preserve CPU endianness for QMI_DATA_LEN
  wifi: ath12k: enforce CPU-endian format for all QMI

 drivers/net/wireless/ath/ath12k/qmi.c |  24 ++++--
 drivers/net/wireless/ath/ath12k/qmi.h |  16 ++--
 drivers/soc/qcom/qmi_encdec.c         | 101 +++++++++++++++++++++-----
 3 files changed, 108 insertions(+), 33 deletions(-)


base-commit: d9d70d689fb6b06a59d95831981258729cbfc209
-- 
2.43.0

[PATCH v3 1/4] soc: qcom: introduce new QMI encode/decode macros

[Alexander Wilhelm]

Introduce new QMI encode/decode macros for each basic element type. It
allows us to further distribute and handle endianness conversion of basic
element types of different size.

Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
---
 drivers/soc/qcom/qmi_encdec.c | 56 +++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
index 7660a960fb45..1f9091458d72 100644
--- a/drivers/soc/qcom/qmi_encdec.c
+++ b/drivers/soc/qcom/qmi_encdec.c
@@ -30,6 +30,34 @@ do { \
 	p_src = (u8 *)p_src + size; \
 } while (0)
 
+#define QMI_ENCDEC_ENCODE_U8(p_dst, p_src) \
+do { \
+	memcpy(p_dst, p_src, sizeof(u8)); \
+	p_dst = (u8 *)p_dst + sizeof(u8); \
+	p_src = (u8 *)p_src + sizeof(u8); \
+} while (0)
+
+#define QMI_ENCDEC_ENCODE_U16(p_dst, p_src) \
+do { \
+	*(__le16 *)p_dst = __cpu_to_le16(*(u16 *)p_src); \
+	p_dst = (u8 *)p_dst + sizeof(u16); \
+	p_src = (u8 *)p_src + sizeof(u16); \
+} while (0)
+
+#define QMI_ENCDEC_ENCODE_U32(p_dst, p_src) \
+do { \
+	*(__le32 *)p_dst = __cpu_to_le32(*(u32 *)p_src); \
+	p_dst = (u8 *)p_dst + sizeof(u32); \
+	p_src = (u8 *)p_src + sizeof(u32); \
+} while (0)
+
+#define QMI_ENCDEC_ENCODE_U64(p_dst, p_src) \
+do { \
+	*(__le64 *)p_dst = __cpu_to_le64(*(u64 *)p_src); \
+	p_dst = (u8 *)p_dst + sizeof(u64); \
+	p_src = (u8 *)p_src + sizeof(u64); \
+} while (0)
+
 #define QMI_ENCDEC_DECODE_N_BYTES(p_dst, p_src, size) \
 do { \
 	memcpy(p_dst, p_src, size); \
@@ -37,6 +65,34 @@ do { \
 	p_src = (u8 *)p_src + size; \
 } while (0)
 
+#define QMI_ENCDEC_DECODE_U8(p_dst, p_src) \
+do { \
+	memcpy(p_dst, p_src, sizeof(u8)); \
+	p_dst = (u8 *)p_dst + sizeof(u8); \
+	p_src = (u8 *)p_src + sizeof(u8); \
+} while (0)
+
+#define QMI_ENCDEC_DECODE_U16(p_dst, p_src) \
+do { \
+	*(u16 *)p_dst = __le16_to_cpu(*(__le16 *)p_src); \
+	p_dst = (u8 *)p_dst + sizeof(u16); \
+	p_src = (u8 *)p_src + sizeof(u16); \
+} while (0)
+
+#define QMI_ENCDEC_DECODE_U32(p_dst, p_src) \
+do { \
+	*(u32 *)p_dst = __le32_to_cpu(*(__le32 *)p_src); \
+	p_dst = (u8 *)p_dst + sizeof(u32); \
+	p_src = (u8 *)p_src + sizeof(u32); \
+} while (0)
+
+#define QMI_ENCDEC_DECODE_U64(p_dst, p_src) \
+do { \
+	*(u64 *)p_dst = __le64_to_cpu(*(__le64 *)p_src); \
+	p_dst = (u8 *)p_dst + sizeof(u64); \
+	p_src = (u8 *)p_src + sizeof(u64); \
+} while (0)
+
 #define UPDATE_ENCODE_VARIABLES(temp_si, buf_dst, \
 				encoded_bytes, tlv_len, encode_tlv, rc) \
 do { \
-- 
2.43.0

[PATCH v3 2/4] soc: qcom: fix QMI encoding/decoding for basic elements

[Alexander Wilhelm]

Extend the QMI byte encoding and decoding logic to support multiple basic
data type sizes (u8, u16, u32, u64) using existing macros. Ensure correct
handling of data sizes and proper byte order conversion on big-endian
platforms by consistently applying these macros during encoding and
decoding of basic elements.

Fixes: 9b8a11e82615 ("soc: qcom: Introduce QMI encoder/decoder")
Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
---
 drivers/soc/qcom/qmi_encdec.c | 46 +++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
index 1f9091458d72..90a48fa7ecf4 100644
--- a/drivers/soc/qcom/qmi_encdec.c
+++ b/drivers/soc/qcom/qmi_encdec.c
@@ -23,13 +23,6 @@
 	*p_length |= ((u8)*p_src) << 8; \
 } while (0)
 
-#define QMI_ENCDEC_ENCODE_N_BYTES(p_dst, p_src, size) \
-do { \
-	memcpy(p_dst, p_src, size); \
-	p_dst = (u8 *)p_dst + size; \
-	p_src = (u8 *)p_src + size; \
-} while (0)
-
 #define QMI_ENCDEC_ENCODE_U8(p_dst, p_src) \
 do { \
 	memcpy(p_dst, p_src, sizeof(u8)); \
@@ -58,13 +51,6 @@ do { \
 	p_src = (u8 *)p_src + sizeof(u64); \
 } while (0)
 
-#define QMI_ENCDEC_DECODE_N_BYTES(p_dst, p_src, size) \
-do { \
-	memcpy(p_dst, p_src, size); \
-	p_dst = (u8 *)p_dst + size; \
-	p_src = (u8 *)p_src + size; \
-} while (0)
-
 #define QMI_ENCDEC_DECODE_U8(p_dst, p_src) \
 do { \
 	memcpy(p_dst, p_src, sizeof(u8)); \
@@ -225,7 +211,21 @@ static int qmi_encode_basic_elem(void *buf_dst, const void *buf_src,
 	u32 i, rc = 0;
 
 	for (i = 0; i < elem_len; i++) {
-		QMI_ENCDEC_ENCODE_N_BYTES(buf_dst, buf_src, elem_size);
+		switch (elem_size) {
+		case sizeof(u8):
+			QMI_ENCDEC_ENCODE_U8(buf_dst, buf_src);
+			break;
+		case sizeof(u16):
+			QMI_ENCDEC_ENCODE_U16(buf_dst, buf_src);
+			break;
+		case sizeof(u32):
+			QMI_ENCDEC_ENCODE_U32(buf_dst, buf_src);
+			break;
+		case sizeof(u64):
+			QMI_ENCDEC_ENCODE_U64(buf_dst, buf_src);
+			break;
+		}
+
 		rc += elem_size;
 	}
 
@@ -508,7 +508,21 @@ static int qmi_decode_basic_elem(void *buf_dst, const void *buf_src,
 	u32 i, rc = 0;
 
 	for (i = 0; i < elem_len; i++) {
-		QMI_ENCDEC_DECODE_N_BYTES(buf_dst, buf_src, elem_size);
+		switch (elem_size) {
+		case sizeof(u8):
+			QMI_ENCDEC_DECODE_U8(buf_dst, buf_src);
+			break;
+		case sizeof(u16):
+			QMI_ENCDEC_DECODE_U16(buf_dst, buf_src);
+			break;
+		case sizeof(u32):
+			QMI_ENCDEC_DECODE_U32(buf_dst, buf_src);
+			break;
+		case sizeof(u64):
+			QMI_ENCDEC_DECODE_U64(buf_dst, buf_src);
+			break;
+		}
+
 		rc += elem_size;
 	}
 
-- 
2.43.0

[PATCH v3 3/4] soc: qcom: preserve CPU endianness for QMI_DATA_LEN

[Alexander Wilhelm]

To ensure correct handling of endianness in the QMI subsystem, the
QMI_DATA_LEN field used in host-side drivers remains in CPU-native byte
order. Remove unnecessary endianness conversions, considering that
QMI_DATA_LEN is always of type `u32` on the host. On the QMI wire
interface, however, its representation is variable and may use either 1 or
2 bytes.

Fixes: 3ced38da5f7d ("soc: qcom: QMI encoding/decoding for big endian")
Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
---
 drivers/soc/qcom/qmi_encdec.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
index 90a48fa7ecf4..b92af573e715 100644
--- a/drivers/soc/qcom/qmi_encdec.c
+++ b/drivers/soc/qcom/qmi_encdec.c
@@ -396,6 +396,7 @@ static int qmi_encode(const struct qmi_elem_info *ei_array, void *out_buf,
 			break;
 
 		case QMI_DATA_LEN:
+			memcpy(&data_len_value, buf_src, sizeof(u32));
 			data_len_sz = temp_ei->elem_size == sizeof(u8) ?
 					sizeof(u8) : sizeof(u16);
 			/* Check to avoid out of range buffer access */
@@ -406,13 +407,11 @@ static int qmi_encode(const struct qmi_elem_info *ei_array, void *out_buf,
 				return -ETOOSMALL;
 			}
 			if (data_len_sz == sizeof(u8)) {
-				val8 = *(u8 *)buf_src;
-				data_len_value = (u32)val8;
+				val8 = data_len_value;
 				rc = qmi_encode_basic_elem(buf_dst, &val8,
 							   1, data_len_sz);
 			} else {
-				val16 = *(u16 *)buf_src;
-				data_len_value = (u32)le16_to_cpu(val16);
+				val16 = data_len_value;
 				rc = qmi_encode_basic_elem(buf_dst, &val16,
 							   1, data_len_sz);
 			}
@@ -695,7 +694,6 @@ static int qmi_decode(const struct qmi_elem_info *ei_array, void *out_c_struct,
 	int rc;
 	u8 val8;
 	u16 val16;
-	u32 val32;
 
 	while (decoded_bytes < in_buf_len) {
 		if (dec_level >= 2 && temp_ei->data_type == QMI_EOTI)
@@ -743,8 +741,7 @@ static int qmi_decode(const struct qmi_elem_info *ei_array, void *out_c_struct,
 							   1, data_len_sz);
 				data_len_value = (u32)val16;
 			}
-			val32 = cpu_to_le32(data_len_value);
-			memcpy(buf_dst, &val32, sizeof(u32));
+			memcpy(buf_dst, &data_len_value, sizeof(u32));
 			temp_ei = temp_ei + 1;
 			buf_dst = out_c_struct + temp_ei->offset;
 			tlv_len -= data_len_sz;
-- 
2.43.0

[PATCH v3 4/4] wifi: ath12k: enforce CPU-endian format for all QMI

[Alexander Wilhelm]

Due to internal endianness handling within the QMI subsystem, all QMI
requests and responses must now be provided in CPU byte order. Replace all
QMI-related data types with CPU-endian types and add the necessary
conversions to ensure correct interpretation across architectures.

Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
Signed-off-by: Alexander Wilhelm <alexander.wilhelm@westermo.com>
---
 drivers/net/wireless/ath/ath12k/qmi.c | 24 ++++++++++++++++--------
 drivers/net/wireless/ath/ath12k/qmi.h | 16 ++++++++--------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index 7c611a1fd6d0..36325e62aa24 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -3307,20 +3307,28 @@ static int ath12k_qmi_wlanfw_wlan_cfg_send(struct ath12k_base *ab)
 	/* This is number of CE configs */
 	req->tgt_cfg_len = ab->qmi.ce_cfg.tgt_ce_len;
 	for (pipe_num = 0; pipe_num < req->tgt_cfg_len ; pipe_num++) {
-		req->tgt_cfg[pipe_num].pipe_num = ce_cfg[pipe_num].pipenum;
-		req->tgt_cfg[pipe_num].pipe_dir = ce_cfg[pipe_num].pipedir;
-		req->tgt_cfg[pipe_num].nentries = ce_cfg[pipe_num].nentries;
-		req->tgt_cfg[pipe_num].nbytes_max = ce_cfg[pipe_num].nbytes_max;
-		req->tgt_cfg[pipe_num].flags = ce_cfg[pipe_num].flags;
+		req->tgt_cfg[pipe_num].pipe_num =
+			__le32_to_cpu(ce_cfg[pipe_num].pipenum);
+		req->tgt_cfg[pipe_num].pipe_dir =
+			__le32_to_cpu(ce_cfg[pipe_num].pipedir);
+		req->tgt_cfg[pipe_num].nentries =
+			__le32_to_cpu(ce_cfg[pipe_num].nentries);
+		req->tgt_cfg[pipe_num].nbytes_max =
+			__le32_to_cpu(ce_cfg[pipe_num].nbytes_max);
+		req->tgt_cfg[pipe_num].flags =
+			__le32_to_cpu(ce_cfg[pipe_num].flags);
 	}
 
 	req->svc_cfg_valid = 1;
 	/* This is number of Service/CE configs */
 	req->svc_cfg_len = ab->qmi.ce_cfg.svc_to_ce_map_len;
 	for (pipe_num = 0; pipe_num < req->svc_cfg_len; pipe_num++) {
-		req->svc_cfg[pipe_num].service_id = svc_cfg[pipe_num].service_id;
-		req->svc_cfg[pipe_num].pipe_dir = svc_cfg[pipe_num].pipedir;
-		req->svc_cfg[pipe_num].pipe_num = svc_cfg[pipe_num].pipenum;
+		req->svc_cfg[pipe_num].service_id =
+			__le32_to_cpu(svc_cfg[pipe_num].service_id);
+		req->svc_cfg[pipe_num].pipe_dir =
+			__le32_to_cpu(svc_cfg[pipe_num].pipedir);
+		req->svc_cfg[pipe_num].pipe_num =
+			__le32_to_cpu(svc_cfg[pipe_num].pipenum);
 	}
 
 	/* set shadow v3 configuration */
diff --git a/drivers/net/wireless/ath/ath12k/qmi.h b/drivers/net/wireless/ath/ath12k/qmi.h
index abdaade3b542..4767d9a2e309 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.h
+++ b/drivers/net/wireless/ath/ath12k/qmi.h
@@ -392,17 +392,17 @@ enum qmi_wlanfw_pipedir_enum_v01 {
 };
 
 struct qmi_wlanfw_ce_tgt_pipe_cfg_s_v01 {
-	__le32 pipe_num;
-	__le32 pipe_dir;
-	__le32 nentries;
-	__le32 nbytes_max;
-	__le32 flags;
+	u32 pipe_num;
+	u32 pipe_dir;
+	u32 nentries;
+	u32 nbytes_max;
+	u32 flags;
 };
 
 struct qmi_wlanfw_ce_svc_pipe_cfg_s_v01 {
-	__le32 service_id;
-	__le32 pipe_dir;
-	__le32 pipe_num;
+	u32 service_id;
+	u32 pipe_dir;
+	u32 pipe_num;
 };
 
 struct qmi_wlanfw_shadow_reg_cfg_s_v01 {
-- 
2.43.0

Re: [PATCH v3 4/4] wifi: ath12k: enforce CPU-endian format for all QMI

[Jeff Johnson]

On 9/18/2025 1:53 AM, Alexander Wilhelm wrote:
> Due to internal endianness handling within the QMI subsystem, all QMI
> requests and responses must now be provided in CPU byte order. Replace all
> QMI-related data types with CPU-endian types and add the necessary
> conversions to ensure correct interpretation across architectures.

I think you can break this out into a separate patch, but reword in a manner
that doesn't indicate any dependency upon your series (it can be a predecessor)

You can do that by noting:
1) currently (before your series) the QMI interface only works on LE systems
because of how it encodes and decodes the data
2) however almost all QMI data structures did NOT use endianess-specific types
for the data structures
3) the ath12k structs being modified are exceptions
4) the QMI interfaces are being modified to support BE, and that modification
requires the QMI data structures to actually be in CPU order
5) So change the two ath12k QMI struct which currently use endianess-specific
types
6) This change will work correctly with existing kernels (which only support
LE) and will also work on future kernels where BE support is added.

Then I can take this patch separately from the QMI changes.

/jeff

Re: [PATCH v3 4/4] wifi: ath12k: enforce CPU-endian format for all QMI

[Alexander Wilhelm]

On Thu, Sep 18, 2025 at 09:31:18AM -0700, Jeff Johnson wrote:
> On 9/18/2025 1:53 AM, Alexander Wilhelm wrote:
> > Due to internal endianness handling within the QMI subsystem, all QMI
> > requests and responses must now be provided in CPU byte order. Replace all
> > QMI-related data types with CPU-endian types and add the necessary
> > conversions to ensure correct interpretation across architectures.
> 
> I think you can break this out into a separate patch, but reword in a manner
> that doesn't indicate any dependency upon your series (it can be a predecessor)

Sure, I will do that. I have only two questions:
* If I split this patch series, should I start on each one with v1 again, right?
* When I mention that BE support will only work on future kernel version, should
  I better avoid the "Fixed:" tag or not?


Best regards
Alexander Wilhelm

Re: [PATCH v3 4/4] wifi: ath12k: enforce CPU-endian format for all QMI

[Jeff Johnson]

On 9/19/2025 12:07 AM, Alexander Wilhelm wrote:
> On Thu, Sep 18, 2025 at 09:31:18AM -0700, Jeff Johnson wrote:
>> On 9/18/2025 1:53 AM, Alexander Wilhelm wrote:
>>> Due to internal endianness handling within the QMI subsystem, all QMI
>>> requests and responses must now be provided in CPU byte order. Replace all
>>> QMI-related data types with CPU-endian types and add the necessary
>>> conversions to ensure correct interpretation across architectures.
>>
>> I think you can break this out into a separate patch, but reword in a manner
>> that doesn't indicate any dependency upon your series (it can be a predecessor)
> 
> Sure, I will do that. I have only two questions:
> * If I split this patch series, should I start on each one with v1 again, right?

for the qmi series use v2 and in the rev history note you dropped the ath12k
patch. for the ath12k singleton use v1

> * When I mention that BE support will only work on future kernel version, should
>   I better avoid the "Fixed:" tag or not?

i'd avoid the Fixes tag.
If you eventually need this series backported, you can do it yourself rather
than have the stable team unnecessarily backport it.

/jeff