Re:[PATCH] soc: qmi: move tlv-micros to header file

From: çæè
Date: Thu May 14 2020 - 04:40:34 EST


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
>