2019-11-25 18:53:37

by Raul Rangel

[permalink] [raw]
Subject: [PATCH v2] platform/chrome: cros_ec_proto: Add response tracing

Add the ability to view response codes as well.

I dropped the EVENT_CLASS since there is only one event per class.

cros_ec_cmd has now been renamed to cros_ec_request_start.

Example:
$ echo 1 > /sys/kernel/debug/tracing/events/cros_ec/enable
$ cat /sys/kernel/debug/tracing/trace

369.416372: cros_ec_request_start: version: 0, command: EC_CMD_USB_PD_POWER_INFO
369.420528: cros_ec_request_done: version: 0, command: EC_CMD_USB_PD_POWER_INFO, ec result: EC_RES_SUCCESS, retval: 16
369.420529: cros_ec_request_start: version: 0, command: EC_CMD_USB_PD_DISCOVERY
369.421383: cros_ec_request_done: version: 0, command: EC_CMD_USB_PD_DISCOVERY, ec result: EC_RES_SUCCESS, retval: 5

Signed-off-by: Raul E Rangel <[email protected]>
---

Changes in v2:
* Renamed events to cros_ec_request_start and cros_ec_request_done.
* Minor printf changes.
* Moved trace_cros_ec_request_start right above xfer_fxn.
* Fixed comment style.
END

drivers/platform/chrome/cros_ec_proto.c | 8 ++++++--
drivers/platform/chrome/cros_ec_trace.c | 24 ++++++++++++++++++++++
drivers/platform/chrome/cros_ec_trace.h | 27 +++++++++++++++++++------
3 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index bd485ce98a42..1b98193a9fc1 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -54,8 +54,6 @@ static int send_command(struct cros_ec_device *ec_dev,
int ret;
int (*xfer_fxn)(struct cros_ec_device *ec, struct cros_ec_command *msg);

- trace_cros_ec_cmd(msg);
-
if (ec_dev->proto_version > 2)
xfer_fxn = ec_dev->pkt_xfer;
else
@@ -72,7 +70,10 @@ static int send_command(struct cros_ec_device *ec_dev,
return -EIO;
}

+ trace_cros_ec_request_start(msg);
ret = (*xfer_fxn)(ec_dev, msg);
+ trace_cros_ec_request_done(msg, ret);
+
if (msg->result == EC_RES_IN_PROGRESS) {
int i;
struct cros_ec_command *status_msg;
@@ -95,7 +96,10 @@ static int send_command(struct cros_ec_device *ec_dev,
for (i = 0; i < EC_COMMAND_RETRIES; i++) {
usleep_range(10000, 11000);

+ trace_cros_ec_request_start(status_msg);
ret = (*xfer_fxn)(ec_dev, status_msg);
+ trace_cros_ec_request_done(status_msg, ret);
+
if (ret == -EAGAIN)
continue;
if (ret < 0)
diff --git a/drivers/platform/chrome/cros_ec_trace.c b/drivers/platform/chrome/cros_ec_trace.c
index 6f80ff4532ae..ef423522bedc 100644
--- a/drivers/platform/chrome/cros_ec_trace.c
+++ b/drivers/platform/chrome/cros_ec_trace.c
@@ -120,5 +120,29 @@
TRACE_SYMBOL(EC_CMD_PD_GET_LOG_ENTRY), \
TRACE_SYMBOL(EC_CMD_USB_PD_MUX_INFO)

+/* See enum ec_status */
+#define EC_RESULT \
+ TRACE_SYMBOL(EC_RES_SUCCESS), \
+ TRACE_SYMBOL(EC_RES_INVALID_COMMAND), \
+ TRACE_SYMBOL(EC_RES_ERROR), \
+ TRACE_SYMBOL(EC_RES_INVALID_PARAM), \
+ TRACE_SYMBOL(EC_RES_ACCESS_DENIED), \
+ TRACE_SYMBOL(EC_RES_INVALID_RESPONSE), \
+ TRACE_SYMBOL(EC_RES_INVALID_VERSION), \
+ TRACE_SYMBOL(EC_RES_INVALID_CHECKSUM), \
+ TRACE_SYMBOL(EC_RES_IN_PROGRESS), \
+ TRACE_SYMBOL(EC_RES_UNAVAILABLE), \
+ TRACE_SYMBOL(EC_RES_TIMEOUT), \
+ TRACE_SYMBOL(EC_RES_OVERFLOW), \
+ TRACE_SYMBOL(EC_RES_INVALID_HEADER), \
+ TRACE_SYMBOL(EC_RES_REQUEST_TRUNCATED), \
+ TRACE_SYMBOL(EC_RES_RESPONSE_TOO_BIG), \
+ TRACE_SYMBOL(EC_RES_BUS_ERROR), \
+ TRACE_SYMBOL(EC_RES_BUSY), \
+ TRACE_SYMBOL(EC_RES_INVALID_HEADER_VERSION), \
+ TRACE_SYMBOL(EC_RES_INVALID_HEADER_CRC), \
+ TRACE_SYMBOL(EC_RES_INVALID_DATA_CRC), \
+ TRACE_SYMBOL(EC_RES_DUP_UNAVAILABLE)
+
#define CREATE_TRACE_POINTS
#include "cros_ec_trace.h"
diff --git a/drivers/platform/chrome/cros_ec_trace.h b/drivers/platform/chrome/cros_ec_trace.h
index 0dd4df30fa89..ee20d8571796 100644
--- a/drivers/platform/chrome/cros_ec_trace.h
+++ b/drivers/platform/chrome/cros_ec_trace.h
@@ -18,7 +18,7 @@

#include <linux/tracepoint.h>

-DECLARE_EVENT_CLASS(cros_ec_cmd_class,
+TRACE_EVENT(cros_ec_request_start,
TP_PROTO(struct cros_ec_command *cmd),
TP_ARGS(cmd),
TP_STRUCT__entry(
@@ -33,13 +33,28 @@ DECLARE_EVENT_CLASS(cros_ec_cmd_class,
__print_symbolic(__entry->command, EC_CMDS))
);

-
-DEFINE_EVENT(cros_ec_cmd_class, cros_ec_cmd,
- TP_PROTO(struct cros_ec_command *cmd),
- TP_ARGS(cmd)
+TRACE_EVENT(cros_ec_request_done,
+ TP_PROTO(struct cros_ec_command *cmd, int retval),
+ TP_ARGS(cmd, retval),
+ TP_STRUCT__entry(
+ __field(uint32_t, version)
+ __field(uint32_t, command)
+ __field(uint32_t, result)
+ __field(int, retval)
+ ),
+ TP_fast_assign(
+ __entry->version = cmd->version;
+ __entry->command = cmd->command;
+ __entry->result = cmd->result;
+ __entry->retval = retval;
+ ),
+ TP_printk("version: %u, command: %s, ec result: %s, retval: %d",
+ __entry->version,
+ __print_symbolic(__entry->command, EC_CMDS),
+ __print_symbolic(__entry->result, EC_RESULT),
+ __entry->retval)
);

-
#endif /* _CROS_EC_TRACE_H_ */

/* this part must be outside header guard */
--
2.24.0.432.g9d3f5f5b63-goog


2019-12-14 11:56:28

by Enric Balletbo i Serra

[permalink] [raw]
Subject: Re: [PATCH v2] platform/chrome: cros_ec_proto: Add response tracing

Hi,

On 25/11/19 18:45, Raul E Rangel wrote:
> Add the ability to view response codes as well.
>
> I dropped the EVENT_CLASS since there is only one event per class.
>
> cros_ec_cmd has now been renamed to cros_ec_request_start.
>
> Example:
> $ echo 1 > /sys/kernel/debug/tracing/events/cros_ec/enable
> $ cat /sys/kernel/debug/tracing/trace
>
> 369.416372: cros_ec_request_start: version: 0, command: EC_CMD_USB_PD_POWER_INFO
> 369.420528: cros_ec_request_done: version: 0, command: EC_CMD_USB_PD_POWER_INFO, ec result: EC_RES_SUCCESS, retval: 16
> 369.420529: cros_ec_request_start: version: 0, command: EC_CMD_USB_PD_DISCOVERY
> 369.421383: cros_ec_request_done: version: 0, command: EC_CMD_USB_PD_DISCOVERY, ec result: EC_RES_SUCCESS, retval: 5
>
> Signed-off-by: Raul E Rangel <[email protected]>

Merge window is open now! Queued for 5.6.

Thanks,
Enric

> ---
>
> Changes in v2:
> * Renamed events to cros_ec_request_start and cros_ec_request_done.
> * Minor printf changes.
> * Moved trace_cros_ec_request_start right above xfer_fxn.
> * Fixed comment style.
> END
>
> drivers/platform/chrome/cros_ec_proto.c | 8 ++++++--
> drivers/platform/chrome/cros_ec_trace.c | 24 ++++++++++++++++++++++
> drivers/platform/chrome/cros_ec_trace.h | 27 +++++++++++++++++++------
> 3 files changed, 51 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index bd485ce98a42..1b98193a9fc1 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -54,8 +54,6 @@ static int send_command(struct cros_ec_device *ec_dev,
> int ret;
> int (*xfer_fxn)(struct cros_ec_device *ec, struct cros_ec_command *msg);
>
> - trace_cros_ec_cmd(msg);
> -
> if (ec_dev->proto_version > 2)
> xfer_fxn = ec_dev->pkt_xfer;
> else
> @@ -72,7 +70,10 @@ static int send_command(struct cros_ec_device *ec_dev,
> return -EIO;
> }
>
> + trace_cros_ec_request_start(msg);
> ret = (*xfer_fxn)(ec_dev, msg);
> + trace_cros_ec_request_done(msg, ret);
> +
> if (msg->result == EC_RES_IN_PROGRESS) {
> int i;
> struct cros_ec_command *status_msg;
> @@ -95,7 +96,10 @@ static int send_command(struct cros_ec_device *ec_dev,
> for (i = 0; i < EC_COMMAND_RETRIES; i++) {
> usleep_range(10000, 11000);
>
> + trace_cros_ec_request_start(status_msg);
> ret = (*xfer_fxn)(ec_dev, status_msg);
> + trace_cros_ec_request_done(status_msg, ret);
> +
> if (ret == -EAGAIN)
> continue;
> if (ret < 0)
> diff --git a/drivers/platform/chrome/cros_ec_trace.c b/drivers/platform/chrome/cros_ec_trace.c
> index 6f80ff4532ae..ef423522bedc 100644
> --- a/drivers/platform/chrome/cros_ec_trace.c
> +++ b/drivers/platform/chrome/cros_ec_trace.c
> @@ -120,5 +120,29 @@
> TRACE_SYMBOL(EC_CMD_PD_GET_LOG_ENTRY), \
> TRACE_SYMBOL(EC_CMD_USB_PD_MUX_INFO)
>
> +/* See enum ec_status */
> +#define EC_RESULT \
> + TRACE_SYMBOL(EC_RES_SUCCESS), \
> + TRACE_SYMBOL(EC_RES_INVALID_COMMAND), \
> + TRACE_SYMBOL(EC_RES_ERROR), \
> + TRACE_SYMBOL(EC_RES_INVALID_PARAM), \
> + TRACE_SYMBOL(EC_RES_ACCESS_DENIED), \
> + TRACE_SYMBOL(EC_RES_INVALID_RESPONSE), \
> + TRACE_SYMBOL(EC_RES_INVALID_VERSION), \
> + TRACE_SYMBOL(EC_RES_INVALID_CHECKSUM), \
> + TRACE_SYMBOL(EC_RES_IN_PROGRESS), \
> + TRACE_SYMBOL(EC_RES_UNAVAILABLE), \
> + TRACE_SYMBOL(EC_RES_TIMEOUT), \
> + TRACE_SYMBOL(EC_RES_OVERFLOW), \
> + TRACE_SYMBOL(EC_RES_INVALID_HEADER), \
> + TRACE_SYMBOL(EC_RES_REQUEST_TRUNCATED), \
> + TRACE_SYMBOL(EC_RES_RESPONSE_TOO_BIG), \
> + TRACE_SYMBOL(EC_RES_BUS_ERROR), \
> + TRACE_SYMBOL(EC_RES_BUSY), \
> + TRACE_SYMBOL(EC_RES_INVALID_HEADER_VERSION), \
> + TRACE_SYMBOL(EC_RES_INVALID_HEADER_CRC), \
> + TRACE_SYMBOL(EC_RES_INVALID_DATA_CRC), \
> + TRACE_SYMBOL(EC_RES_DUP_UNAVAILABLE)
> +
> #define CREATE_TRACE_POINTS
> #include "cros_ec_trace.h"
> diff --git a/drivers/platform/chrome/cros_ec_trace.h b/drivers/platform/chrome/cros_ec_trace.h
> index 0dd4df30fa89..ee20d8571796 100644
> --- a/drivers/platform/chrome/cros_ec_trace.h
> +++ b/drivers/platform/chrome/cros_ec_trace.h
> @@ -18,7 +18,7 @@
>
> #include <linux/tracepoint.h>
>
> -DECLARE_EVENT_CLASS(cros_ec_cmd_class,
> +TRACE_EVENT(cros_ec_request_start,
> TP_PROTO(struct cros_ec_command *cmd),
> TP_ARGS(cmd),
> TP_STRUCT__entry(
> @@ -33,13 +33,28 @@ DECLARE_EVENT_CLASS(cros_ec_cmd_class,
> __print_symbolic(__entry->command, EC_CMDS))
> );
>
> -
> -DEFINE_EVENT(cros_ec_cmd_class, cros_ec_cmd,
> - TP_PROTO(struct cros_ec_command *cmd),
> - TP_ARGS(cmd)
> +TRACE_EVENT(cros_ec_request_done,
> + TP_PROTO(struct cros_ec_command *cmd, int retval),
> + TP_ARGS(cmd, retval),
> + TP_STRUCT__entry(
> + __field(uint32_t, version)
> + __field(uint32_t, command)
> + __field(uint32_t, result)
> + __field(int, retval)
> + ),
> + TP_fast_assign(
> + __entry->version = cmd->version;
> + __entry->command = cmd->command;
> + __entry->result = cmd->result;
> + __entry->retval = retval;
> + ),
> + TP_printk("version: %u, command: %s, ec result: %s, retval: %d",
> + __entry->version,
> + __print_symbolic(__entry->command, EC_CMDS),
> + __print_symbolic(__entry->result, EC_RESULT),
> + __entry->retval)
> );
>
> -
> #endif /* _CROS_EC_TRACE_H_ */
>
> /* this part must be outside header guard */
>