Re: [PATCH v2,2/3] driver: rpmon: qmi message version 01

[Randy Dunlap]

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@xxxxxxxx>
> ---
> 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