Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v2,2/3] driver: rpmon: qmi message version 01
To:     Wang Wenhu <wenhu.wang@vivo.com>, gregkh@linuxfoundation.org,
        linux-kernel@vger.kernel.org
Cc:     kernel@vivo.com
References: <20200411095301.18780-1-wenhu.wang@vivo.com>
 <20200412112405.24116-1-wenhu.wang@vivo.com>
 <20200412112405.24116-3-wenhu.wang@vivo.com>
From:   Randy Dunlap <rdunlap@infradead.org>
Message-ID: <dbc89d13-a7ce-347f-0f7a-4b500b31f4fc@infradead.org>
Date:   Mon, 13 Apr 2020 10:12:49 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.6.0
MIME-Version: 1.0
In-Reply-To: <20200412112405.24116-3-wenhu.wang@vivo.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi--

On 4/12/20 4:24 AM, Wang Wenhu wrote:
> Implements a RPMON_QMI message set for connection checking service.
> RPMON_QMI defines its message types modularly. Each rpmon service
> binds to a message set and introduced as a module. This version 1.0
> message set could be used for connection checking of remote processors.
> 
> RPMON_QMI messages depend on QCOM_QMI_HELPERS and should be updated
> together with QMI related modules, and if so, RPMON_QMI_MAG_V2 would
> be introduced as a new module, in parallel with RPMON_QMI_MAG_V1.
> 
> Signed-off-by: Wang Wenhu <wenhu.wang@vivo.com>
> ---
> Changes since v1:
>  - Addressed review comments from Randy
> 
> ---
>  drivers/rpmon/Kconfig            |  13 ++
>  drivers/rpmon/Makefile           |   1 +
>  drivers/rpmon/rpmon_qmi.h        |  77 ++++++++++
>  drivers/rpmon/rpmon_qmi_msg_v1.c | 242 +++++++++++++++++++++++++++++++
>  4 files changed, 333 insertions(+)
>  create mode 100644 drivers/rpmon/rpmon_qmi.h
>  create mode 100644 drivers/rpmon/rpmon_qmi_msg_v1.c
> 

> diff --git a/drivers/rpmon/rpmon_qmi_msg_v1.c b/drivers/rpmon/rpmon_qmi_msg_v1.c
> new file mode 100644
> index 000000000000..3fe6d6a4cf31
> --- /dev/null
> +++ b/drivers/rpmon/rpmon_qmi_msg_v1.c
> @@ -0,0 +1,242 @@

I am hoping that your patch:
[PATCH] soc: qmi: move tlv-micros to header file

will eventually allow the .tlv_type constants below to become #define values.

Otherwise these changes (patch 2/3) look good.  Thanks.

> +static struct qmi_elem_info register_req_v01_ei[] = {
> +	{
> +		.data_type      = QMI_OPT_FLAG,
> +		.elem_len       = 1,
> +		.elem_size      = sizeof(uint8_t),
> +		.array_type     = NO_ARRAY,
> +		.tlv_type       = 0x10,
> +		.offset         = offsetof(struct rpmon_register_req,
> +					   name_valid),
> +	},
> +	{
> +		.data_type      = QMI_STRING,
> +		.elem_len       = RP_NAME_LEN,
> +		.elem_size      = sizeof(char),
> +		.array_type     = NO_ARRAY,
> +		.tlv_type       = 0x10,
> +		.offset         = offsetof(struct rpmon_register_req,
> +					   name),
> +	},
> +	{
> +		.data_type      = QMI_OPT_FLAG,
> +		.elem_len       = 1,
> +		.elem_size      = sizeof(uint8_t),
> +		.array_type     = NO_ARRAY,
> +		.tlv_type       = 0x11,
> +		.offset         = offsetof(struct rpmon_register_req,
> +					   timeout_valid),
> +	},
> +	{
> +		.data_type      = QMI_UNSIGNED_4_BYTE,
> +		.elem_len       = 1,
> +		.elem_size      = sizeof(u32),
> +		.array_type     = NO_ARRAY,
> +		.tlv_type       = 0x11,
> +		.offset         = offsetof(struct rpmon_register_req,
> +					   timeout),
> +	},
> +	{
> +		.data_type      = QMI_EOTI,
> +		.array_type     = NO_ARRAY,
> +		.tlv_type       = QMI_COMMON_TLV_TYPE,
> +	},
> +};
> +
> +static struct qmi_elem_info conn_check_resp_v01_ei[] = {
> +	{
> +		.data_type      = QMI_OPT_FLAG,
> +		.elem_len       = 1,
> +		.elem_size      = sizeof(uint8_t),
> +		.array_type     = NO_ARRAY,
> +		.tlv_type       = 0x10,
> +		.offset         = offsetof(struct rpmon_conn_check_resp,
> +					   result_valid),
> +	},
> +	{
> +		.data_type      = QMI_SIGNED_4_BYTE_ENUM,
> +		.elem_len       = 1,
> +		.elem_size      = sizeof(enum rpmon_exec_result),
> +		.array_type     = NO_ARRAY,
> +		.tlv_type       = 0x10,
> +		.offset         = offsetof(struct rpmon_conn_check_resp,
> +					   result),
> +	},
> +	{
> +		.data_type      = QMI_EOTI,
> +		.array_type     = NO_ARRAY,
> +		.tlv_type       = QMI_COMMON_TLV_TYPE,
> +	},
> +};


-- 
~Randy