Hi all, Any comments here? Regards, Wenhu >It's highly helpful to move the definitions of TLV related micros >into header file for user reference. The OPTIONAL_TLV_TYPE_START >should be known to any user that might define messages containing >optional fields. SIZE fields are the same, for users to calculate >message buffer length. > >While encoding messages, Type always occurs together with Length. >So the better way is to use TL_SIZE, rather than (T_SIZE + L_SIZE). > >Signed-off-by: Wang Wenhu <wenhu.wang@xxxxxxxx> >--- > drivers/soc/qcom/qmi_encdec.c | 28 ++++++++++++---------------- > include/linux/soc/qcom/qmi.h | 5 +++++ > 2 files changed, 17 insertions(+), 16 deletions(-) > >diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c >index 3aaab71d1b2c..a674c68efab2 100644 >--- a/drivers/soc/qcom/qmi_encdec.c >+++ b/drivers/soc/qcom/qmi_encdec.c >@@ -53,10 +53,6 @@ do { \ > decoded_bytes += rc; \ > } while (0) > >-#define TLV_LEN_SIZE sizeof(u16) >-#define TLV_TYPE_SIZE sizeof(u8) >-#define OPTIONAL_TLV_TYPE_START 0x10 >- > static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf, > const void *in_c_struct, u32 out_buf_len, > int enc_level); >@@ -142,7 +138,7 @@ static int qmi_calc_min_msg_len(struct qmi_elem_info *ei_array, > * nested structure. > */ > if (level == 1) >- min_msg_len += (TLV_TYPE_SIZE + TLV_LEN_SIZE); >+ min_msg_len += QMI_TLV_TL_SIZE; > } > > return min_msg_len; >@@ -253,8 +249,7 @@ static int qmi_encode_string_elem(struct qmi_elem_info *ei_array, > } > > if (enc_level == 1) { >- if (string_len + TLV_LEN_SIZE + TLV_TYPE_SIZE > >- out_buf_len) { >+ if (string_len + QMI_TLV_TL_SIZE > out_buf_len) { > pr_err("%s: Output len %d > Out Buf len %d\n", > __func__, string_len, out_buf_len); > return -ETOOSMALL; >@@ -311,7 +306,7 @@ static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf, > tlv_pointer = buf_dst; > tlv_len = 0; > if (enc_level == 1) >- buf_dst = buf_dst + (TLV_LEN_SIZE + TLV_TYPE_SIZE); >+ buf_dst = buf_dst + QMI_TLV_TL_SIZE; > > while (temp_ei->data_type != QMI_EOTI) { > buf_src = in_c_struct + temp_ei->offset; >@@ -342,8 +337,8 @@ static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf, > data_len_sz = temp_ei->elem_size == sizeof(u8) ? > sizeof(u8) : sizeof(u16); > /* Check to avoid out of range buffer access */ >- if ((data_len_sz + encoded_bytes + TLV_LEN_SIZE + >- TLV_TYPE_SIZE) > out_buf_len) { >+ if ((data_len_sz + encoded_bytes + QMI_TLV_TL_SIZE) > >+ out_buf_len) { > pr_err("%s: Too Small Buffer @DATA_LEN\n", > __func__); > return -ETOOSMALL; >@@ -367,7 +362,7 @@ static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf, > case QMI_SIGNED_4_BYTE_ENUM: > /* Check to avoid out of range buffer access */ > if (((data_len_value * temp_ei->elem_size) + >- encoded_bytes + TLV_LEN_SIZE + TLV_TYPE_SIZE) > >+ encoded_bytes + QMI_TLV_TL_SIZE) > > out_buf_len) { > pr_err("%s: Too Small Buffer @data_type:%d\n", > __func__, temp_ei->data_type); >@@ -410,10 +405,10 @@ static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf, > > if (encode_tlv && enc_level == 1) { > QMI_ENCDEC_ENCODE_TLV(tlv_type, tlv_len, tlv_pointer); >- encoded_bytes += (TLV_TYPE_SIZE + TLV_LEN_SIZE); >+ encoded_bytes += QMI_TLV_TL_SIZE; > tlv_pointer = buf_dst; > tlv_len = 0; >- buf_dst = buf_dst + TLV_LEN_SIZE + TLV_TYPE_SIZE; >+ buf_dst = buf_dst + QMI_TLV_TL_SIZE; > encode_tlv = 0; > } > } >@@ -613,10 +608,11 @@ static int qmi_decode(struct qmi_elem_info *ei_array, void *out_c_struct, > tlv_pointer = buf_src; > QMI_ENCDEC_DECODE_TLV(&tlv_type, > &tlv_len, tlv_pointer); >- buf_src += (TLV_TYPE_SIZE + TLV_LEN_SIZE); >- decoded_bytes += (TLV_TYPE_SIZE + TLV_LEN_SIZE); >+ buf_src += QMI_TLV_TL_SIZE; >+ decoded_bytes += QMI_TLV_TL_SIZE; > temp_ei = find_ei(ei_array, tlv_type); >- if (!temp_ei && tlv_type < OPTIONAL_TLV_TYPE_START) { >+ if (!temp_ei && tlv_type < >+ QMI_OPTIONAL_TLV_TYPE_START) { > pr_err("%s: Inval element info\n", __func__); > return -EINVAL; > } else if (!temp_ei) { >diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h >index e712f94b89fc..eb53aebdf45e 100644 >--- a/include/linux/soc/qcom/qmi.h >+++ b/include/linux/soc/qcom/qmi.h >@@ -34,6 +34,11 @@ struct qmi_header { > #define QMI_INDICATION 4 > > #define QMI_COMMON_TLV_TYPE 0 >+#define QMI_OPTIONAL_TLV_TYPE_START 0x10 >+ >+#define QMI_TLV_LEN_SIZE sizeof(u16) >+#define QMI_TLV_TYPE_SIZE sizeof(u8) >+#define QMI_TLV_TL_SIZE (QMI_TLV_LEN_SIZE + QMI_TLV_TYPE_SIZE) > > enum qmi_elem_type { > QMI_EOTI, >-- >2.17.1 >