[PATCH v4 RESEND 2/3] soc: qcom: fix QMI encoding/decoding for basic elements

Alexander Wilhelm posted 3 patches 1 week, 6 days ago
There is a newer version of this series
[PATCH v4 RESEND 2/3] soc: qcom: fix QMI encoding/decoding for basic elements
Posted by Alexander Wilhelm 1 week, 6 days ago
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.

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
Re: [PATCH v4 RESEND 2/3] soc: qcom: fix QMI encoding/decoding for basic elements
Posted by Dmitry Baryshkov 1 week, 5 days ago
On Tue, Nov 18, 2025 at 07:53:40AM +0100, Alexander Wilhelm wrote:
> 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.
> 
> 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;

default: scream?

> +		}
> +
>  		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;

same here

> +		}
> +
>  		rc += elem_size;
>  	}
>  
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry
Re: [PATCH v4 RESEND 2/3] soc: qcom: fix QMI encoding/decoding for basic elements
Posted by Alexander Wilhelm 1 week, 5 days ago
On Wed, Nov 19, 2025 at 10:03:41AM +0200, Dmitry Baryshkov wrote:
> On Tue, Nov 18, 2025 at 07:53:40AM +0100, Alexander Wilhelm wrote:
> > 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.
> > 
> > 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;
> 
> default: scream?

Okay, I'll fix that in the next version. What would be the recommended default
approach? The QMI protocol only uses the sizes mentioned above, and previously
the data was simple memcpy'ed.


Best regards
Alexander Wilhelm