From: 王文虎 <wenhu.wang@vivo.com>
To: Randy Dunlap <rdunlap@infradead.org>
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] driver: rpmon: qmi message version 01
Date: Sun, 12 Apr 2020 19:05:08 +0800 (GMT+08:00) [thread overview]
Message-ID: <AAcA7gD9CIeobJKAa2X9napS.3.1586689508686.Hmail.wenhu.wang@vivo.com> (raw)
In-Reply-To: <36d40370-f55a-2b71-68c0-1c777f35f85c@infradead.org>
Hi,
From: Randy Dunlap <rdunlap@infradead.org>
Date: 2020-04-12 03:07:11
To: Wang Wenhu <wenhu.wang@vivo.com>,gregkh@linuxfoundation.org,linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] driver: rpmon: qmi message version 01>Hi--
>
>On 4/11/20 2:53 AM, Wang Wenhu wrote:
>> Implements a RPMON_QMI message set for a certain rpmon 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.
>>
>> Signed-off-by: Wang Wenhu <wenhu.wang@vivo.com>
>> ---
>> drivers/rpmon/Kconfig | 13 ++
>> drivers/rpmon/Makefile | 1 +
>> drivers/rpmon/rpmon_qmi.h | 77 ++++++++++
>> drivers/rpmon/rpmon_qmi_msg_v1.c | 240 +++++++++++++++++++++++++++++++
>> 4 files changed, 331 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.h b/drivers/rpmon/rpmon_qmi.h
>> new file mode 100644
>> index 000000000000..f6e7cfc97a3f
>> --- /dev/null
>> +++ b/drivers/rpmon/rpmon_qmi.h
>> @@ -0,0 +1,77 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (C) 2020 Vivo Communication Technology Co. Ltd.
>> + * Copyright (C) 2020 Wang Wenhu <wenhu.wang@vivo.com>
>> + * All rights reserved.
>> + */
>> +
>> +#ifndef RPMON_QMI_INTERFACE_H
>> +#define RPMON_QMI_INTERFACE_H
>> +
>> +#define RP_NAME_LEN 255
>> +#define RPQMI_BUF_SIZE 4096
>> +#define QMI_TLV_TL_SIZE 3
>> +
>> +enum rpmon_exec_result {
>> + RPMON_EXEC_SUCCESS = 0,
>> + RPMON_EXEC_FAILURE = 1,
>> +};
>> +
>> +struct rpmon_register_req {
>> + uint8_t name_valid;
>> + char name[RP_NAME_LEN + 1];
>> + uint8_t timeout_valid;
>> + u32 timeout;
>> +};
>
>Is this struct packed or padded?
>Is it shared with/used by other drivers?
>
It is passed and repacked by qmi interfaces and then sent out as QRTR packet,
non-shared with others. So it is safe to be left without any locking protection here.
>> +
>> +struct rpmon_conn_indication {
>> + char placeholder;
>> +};
>> +
>> +struct rpmon_conn_check_resp {
>> + uint8_t result_valid;
>> + struct qmi_response_type_v01 result;
>> +};
>> +
>> +struct rpmon_response {
>> + struct qmi_response_type_v01 resp;
>> +};
>> +
>> +struct rpmon_qmi_device {
>> + struct list_head list;
>> + struct sockaddr_qrtr addr;
>> + u32 timeout;
>> + u32 flag;
>> + struct ratelimit_state ratelimit;
>> +
>> + atomic_t checks;
>> + atomic_t reports;
>> +
>> + struct rpmon_info *info;
>> + struct rpmon_qmi *rqmi;
>> +};
>> +
>> +struct rpmon_qmi {
>> + struct rpmon_info *info;
>> + struct qmi_handle qmi;
>> + struct qmi_service *svc;
>> + struct qmi_ops *ops;
>> + struct qmi_msg_handler *handlers;
>> + int (*sendmsg)(const struct rpmon_qmi_device *rdev,
>> + const void *data,
>> + u32 len);
>> +};
>> +
>> +/* rpqmi message types currently supported. */
>> +enum rpmon_qmi_msg_type {
>> + RPQMI_MSG_REGISTER = 0,
>> + RPQMI_MSG_CONNCHK_RESP,
>> + RPQMI_MSG_MAX,
>> +};
>> +
>> +int rpmon_qmi_handle_init(struct rpmon_qmi *rqmi,
>> + void (*cb)(enum rpmon_qmi_msg_type type,
>> + struct sockaddr_qrtr *sq,
>> + const void *msg));
>> +
>> +#endif /* RPMON_QMI_INTERFACE_H */
>> diff --git a/drivers/rpmon/rpmon_qmi_msg_v1.c b/drivers/rpmon/rpmon_qmi_msg_v1.c
>> new file mode 100644
>> index 000000000000..10d38d8133b0
>> --- /dev/null
>> +++ b/drivers/rpmon/rpmon_qmi_msg_v1.c
>> @@ -0,0 +1,240 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2020 Vivo Communication Technology Co. Ltd.
>> + * Copyright (C) 2020 Wang Wenhu <wenhu.wang@vivo.com>
>> + * All rights reserved.
>> + *
>> + * RPMON An implementation of remote processor monitor freamwork
>
>typo: framework {in all 3 source files}
>
>> + * for platforms that multi-processors exists. RPMON is implemented
>
>but that sentence is confusing at best.
>
Both addressed in v2
>> + * with chardev and sysfs class as interfaces to communicate with
>> + * the user level. It supports different communication interfaces
>> + * added modularly with remote processors. Currently QMI implementation
>> + * is available, and this file implements the message type v1.
>> + *
>> + * RPMON could be used to detect the stabilities of remote processors,
>> + * collect any kinds of information you are interested with, take
>
> interested in, take
>
Addressed in v2
>> + * actions like connection status check, and so on. Enhancements can
>> + * be made upon current implementation.
>> + */
>> +
>> +#include <linux/soc/qcom/qmi.h>
>> +#include <linux/rpmon.h>
>> +#include "rpmon_qmi.h"
>> +
>> +#define RPMON_SVC_ID_V01 0x3c
>> +#define RPMON_SVC_VER_V01 0x01
>> +#define RPMON_SVC_INS_V01 0x00
>> +
>> +#define RPMON_CONN_REQ_MSG_ID_VO1 0x20
>> +#define RPMON_CONN_RESP_MSG_ID_VO1 0x20
>> +#define RPMON_CONN_IND_MSG_ID_V01 0x21
>> +#define RPMON_EXEC_COMP_REQ_MSG_ID_V01 0x22
>> +#define RPMON_EXEC_COMP_RESP_MSG_ID_V01 0x22
>> +
>> +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,
>
>Can you use an enum or #define value for the tlv_type magic numbers?
>(multiple places)
Definitely. But actually the definitions belong to QMI module, and I will
figure this out with another patch of #include <linux/soc/qcom/qmi.h> or so.
I will Cc you if not mind?
>
>> + .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,
>> + },
>> +};
>> +
>> +static struct qmi_elem_info conn_indication_v01_ei[] = {
>> + {
>> + .data_type = QMI_EOTI,
>> + .array_type = NO_ARRAY,
>> + .tlv_type = QMI_COMMON_TLV_TYPE,
>> + },
>> +};
>> +
>> +static struct qmi_elem_info response_v01_ei[] = {
>> + {
>> + .data_type = QMI_STRUCT,
>> + .elem_len = 1,
>> + .elem_size = sizeof(struct qmi_response_type_v01),
>> + .array_type = NO_ARRAY,
>> + .tlv_type = 0x02,
>> + .offset = offsetof(
>> + struct rpmon_response,
>> + resp),
>
>multiple lines not needed above.
Addressed in v2
>
>> + .ei_array = qmi_response_type_v01_ei,
>> + },
>> + {
>> + .data_type = QMI_EOTI,
>> + .array_type = NO_ARRAY,
>> + .tlv_type = QMI_COMMON_TLV_TYPE,
>> + },
>> +};
>> +
>> +static void (*msg_callback)(enum rpmon_qmi_msg_type type,
>> + struct sockaddr_qrtr *sq,
>> + const void *msg);
>> +
>> +static int rpmon_qmi_sendmsg_v1(const struct rpmon_qmi_device *rdev,
>> + const void *data,
>> + u32 len)
>> +{
>> + int ret;
>> + struct sockaddr_qrtr sq;
>> +
>> + memcpy(&sq, &rdev->addr, sizeof(sq));
>> +
>> + ret = qmi_send_indication(&rdev->rqmi->qmi, &sq,
>> + RPMON_CONN_IND_MSG_ID_V01,
>> + QMI_TLV_TL_SIZE,
>> + conn_indication_v01_ei, NULL);
>> + if (ret < 0)
>> + pr_err("Error %d send indication failed", ret);
>> +
>> + return ret;
>> +}
>> +
>> +static void rpmon_qmi_recv_register_req_v1(struct qmi_handle *qmi,
>> + struct sockaddr_qrtr *sq,
>> + struct qmi_txn *txn,
>> + const void *msg)
>> +{
>> + struct rpmon_response resp;
>> + int ret;
>> +
>> + resp.resp.result = QMI_RESULT_SUCCESS_V01;
>> + resp.resp.error = QMI_ERR_NONE_V01;
>> + ret = qmi_send_response(qmi, sq, txn,
>> + RPMON_CONN_RESP_MSG_ID_VO1,
>> + sizeof(resp.resp) + QMI_TLV_TL_SIZE * 2,
>> + response_v01_ei,
>> + &resp.resp);
>> + if (ret < 0)
>> + pr_err("Error %d send respons failed", ret);
>> +
>> + if (msg_callback)
>> + msg_callback(RPQMI_MSG_REGISTER, sq, msg);
>> +}
>> +
>> +void rpmon_qmi_recv_conn_check_resp_v1(struct qmi_handle *qmi,
>> + struct sockaddr_qrtr *sq,
>> + struct qmi_txn *txn,
>> + const void *msg)
>> +{
>> + struct rpmon_response resp;
>> + int ret;
>> +
>> + resp.resp.result = QMI_RESULT_SUCCESS_V01;
>> + resp.resp.error = QMI_ERR_NONE_V01;
>> + ret = qmi_send_response(qmi, sq, txn,
>> + RPMON_EXEC_COMP_REQ_MSG_ID_V01,
>> + sizeof(resp.resp) + QMI_TLV_TL_SIZE * 2,
>> + response_v01_ei,
>> + &resp.resp);
>> + if (ret < 0)
>> + pr_err("Error %d send respons failed", ret);
>
> response
>
Addressed in v2
>> +
>> + if (msg_callback)
>> + msg_callback(RPQMI_MSG_CONNCHK_RESP, sq, msg);
>> +}
>> +
>> +static struct qmi_msg_handler rpmon_qmi_msg_handlers_v01[] = {
>> + {
>> + .type = QMI_REQUEST,
>> + .msg_id = RPMON_CONN_REQ_MSG_ID_VO1,
>> + .ei = register_req_v01_ei,
>> + .decoded_size = sizeof(struct rpmon_register_req),
>> + .fn = rpmon_qmi_recv_register_req_v1,
>> + },
>> + {
>> + .type = QMI_REQUEST,
>> + .msg_id = RPMON_EXEC_COMP_REQ_MSG_ID_V01,
>> + .ei = conn_check_resp_v01_ei,
>> + .decoded_size = sizeof(struct rpmon_conn_check_resp),
>> + .fn = rpmon_qmi_recv_conn_check_resp_v1,
>> + },
>> +};
>> +
>> +static struct qmi_service rpmon_qmi_svc = {
>> + .service = RPMON_SVC_ID_V01,
>> + .version = RPMON_SVC_VER_V01,
>> + .instance = RPMON_SVC_INS_V01,
>> +};
>> +
>> +int rpmon_qmi_handle_init(struct rpmon_qmi *rqmi,
>> + void (*cb)(enum rpmon_qmi_msg_type type,
>> + struct sockaddr_qrtr *sq,
>> + const void *msg))
>> +{
>> + rqmi->svc = &rpmon_qmi_svc;
>> + rqmi->handlers = rpmon_qmi_msg_handlers_v01;
>> + rqmi->sendmsg = rpmon_qmi_sendmsg_v1;
>> +
>> + if (cb)
>> + msg_callback = cb;
>> + else
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(rpmon_qmi_handle_init);
>> +
>> +MODULE_AUTHOR("Wang Wenhu");
>
>Please include email address in the string above.
>About 3/4 of all uses of MODULE_AUTHOR() do so.
>
Addressed in v2
>> +MODULE_LICENSE("GPL v2");
>>
>
>thanks.
>--
>~Randy
>
Thanks,
Wenhu
next prev parent reply other threads:[~2020-04-12 11:05 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-11 9:52 [PATCH 0/3] drivers: rpmon: new driver Remote Processor Monitor Wang Wenhu
2020-04-11 9:52 ` [PATCH 1/3] driver: " Wang Wenhu
2020-04-11 18:08 ` Randy Dunlap
2020-04-12 10:52 ` 王文虎
2020-04-11 9:53 ` [PATCH 2/3] driver: rpmon: qmi message version 01 Wang Wenhu
2020-04-11 19:07 ` Randy Dunlap
2020-04-12 11:05 ` 王文虎 [this message]
2020-04-11 9:53 ` [PATCH 3/3] driver: rpmon: add rpmon_qmi driver Wang Wenhu
2020-04-11 19:23 ` Randy Dunlap
2020-04-12 11:12 ` 王文虎
2020-04-12 11:24 ` [PATCH v2,0/3] drivers: rpmon: new driver Remote Processor Monitor Wang Wenhu
2020-04-12 11:24 ` [PATCH v2,1/3] driver: " Wang Wenhu
2020-04-13 17:04 ` Randy Dunlap
2020-04-12 11:24 ` [PATCH v2,2/3] driver: rpmon: qmi message version 01 Wang Wenhu
2020-04-13 17:12 ` Randy Dunlap
2020-04-12 11:24 ` [PATCH v2,3/3] driver: rpmon: add rpmon_qmi driver Wang Wenhu
2020-04-13 17:17 ` Randy Dunlap
2020-04-14 3:59 ` [PATCH v3,0/3] drivers: rpmon: new driver Remote Processor Monitor Wang Wenhu
2020-04-14 3:59 ` [PATCH v3,1/3] driver: " Wang Wenhu
2020-04-28 12:57 ` Greg KH
2020-04-14 3:59 ` [PATCH v3,2/3] driver: rpmon: qmi message version 01 Wang Wenhu
2020-04-14 3:59 ` [PATCH v3,3/3] driver: rpmon: add rpmon_qmi driver Wang Wenhu
2020-04-14 22:58 ` [PATCH v3,0/3] drivers: rpmon: new driver Remote Processor Monitor Bjorn Andersson
2020-04-14 22:58 ` Bjorn Andersson
2020-04-14 22:58 ` Bjorn Andersson
2020-04-15 15:16 ` Wang Wenhu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AAcA7gD9CIeobJKAa2X9napS.3.1586689508686.Hmail.wenhu.wang@vivo.com \
--to=wenhu.wang@vivo.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rdunlap@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.