linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: lukasz.luba@arm.com
To: linux-kernel@vger.kernel.org, sudeep.holla@arm.com,
	linux-arm-kernel@lists.infradead.org
Cc: rostedt@goodmis.org, mingo@redhat.com, Lukasz Luba <lukasz.luba@arm.com>
Subject: [PATCH 1/2] include: trace: Add SCMI header with trace events
Date: Mon, 16 Dec 2019 16:16:49 +0000	[thread overview]
Message-ID: <20191216161650.21844-1-lukasz.luba@arm.com> (raw)

From: Lukasz Luba <lukasz.luba@arm.com>

Adding trace events would help to measure the speed of the communication
channel. It can be also potentially used helpful during investigation
of some issues platforms which use different transport layer.

Update also MAINTAINERS file with information that the new trace events
are maintained.

Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 MAINTAINERS                 |  1 +
 include/trace/events/scmi.h | 56 +++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)
 create mode 100644 include/trace/events/scmi.h

diff --git a/MAINTAINERS b/MAINTAINERS
index cc0a4a8ae06a..0182315226fc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15966,6 +15966,7 @@ F:	drivers/firmware/arm_scpi.c
 F:	drivers/firmware/arm_scmi/
 F:	drivers/reset/reset-scmi.c
 F:	include/linux/sc[mp]i_protocol.h
+F:	include/trace/events/scmi.h
 
 SYSTEM RESET/SHUTDOWN DRIVERS
 M:	Sebastian Reichel <sre@kernel.org>
diff --git a/include/trace/events/scmi.h b/include/trace/events/scmi.h
new file mode 100644
index 000000000000..a84016b02ffd
--- /dev/null
+++ b/include/trace/events/scmi.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM scmi
+
+#if !defined(_TRACE_SCMI_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_SCMI_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(scmi_xfer_begin,
+	TP_PROTO(u8 id, u8 protocol_id, u16 seq, bool poll),
+	TP_ARGS(id, protocol_id, seq, poll),
+
+	TP_STRUCT__entry(
+		__field(u8, id)
+		__field(u8, protocol_id)
+		__field(u16, seq)
+		__field(bool, poll)
+	),
+
+	TP_fast_assign(
+		__entry->id = id;
+		__entry->protocol_id = protocol_id;
+		__entry->seq = seq;
+		__entry->poll = poll;
+	),
+
+	TP_printk("id=%u protocol_id=%u seq=%u poll=%u", __entry->id,
+		__entry->protocol_id, __entry->seq, __entry->poll)
+);
+
+TRACE_EVENT(scmi_xfer_end,
+	TP_PROTO(u8 id, u8 protocol_id, u16 seq, u32 status),
+	TP_ARGS(id, protocol_id, seq, status),
+
+	TP_STRUCT__entry(
+		__field(u8, id)
+		__field(u8, protocol_id)
+		__field(u16, seq)
+		__field(u32, status)
+	),
+
+	TP_fast_assign(
+		__entry->id = id;
+		__entry->protocol_id = protocol_id;
+		__entry->seq = seq;
+		__entry->status = status;
+	),
+
+	TP_printk("id=%u protocol_id=%u seq=%u status=%u", __entry->id,
+		__entry->protocol_id, __entry->seq, __entry->status)
+);
+#endif /* _TRACE_SCMI_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
-- 
2.17.1


WARNING: multiple messages have this Message-ID (diff)
From: Jim Quinlan <james.quinlan@broadcom.com>
To: lukasz.luba@arm.com
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, mingo@redhat.com,
	rostedt@goodmis.org, sudeep.holla@arm.com
Subject: [PATCH 1/2] include: trace: Add SCMI header with trace events
Date: Mon, 16 Dec 2019 17:15:54 -0500	[thread overview]
Message-ID: <20191216161650.21844-1-lukasz.luba@arm.com> (raw)
Message-ID: <20191216221554.qHFQdzev0xmzO4sjCIiUnmTaGUOiKgDZtPVJtodb34I@z> (raw)
In-Reply-To: <20191216161650.21844-1-lukasz.luba@arm.com>

From: Lukasz Luba <lukasz.luba@arm.com>

+
+TRACE_EVENT(scmi_xfer_begin,
+	TP_PROTO(u8 id, u8 protocol_id, u16 seq, bool poll),
+	TP_ARGS(id, protocol_id, seq, poll),
+
+	TP_STRUCT__entry(
+		__field(u8, id)
+		__field(u8, protocol_id)
+		__field(u16, seq)
+		__field(bool, poll)
+	),
+
+	TP_fast_assign(
+		__entry->id = id;
+		__entry->protocol_id = protocol_id;
+		__entry->seq = seq;
+		__entry->poll = poll;
+	),
+
+	TP_printk("id=%u protocol_id=%u seq=%u poll=%u", __entry->id,
+		__entry->protocol_id, __entry->seq, __entry->poll)
+);
+
+TRACE_EVENT(scmi_xfer_end,
+	TP_PROTO(u8 id, u8 protocol_id, u16 seq, u32 status),
+	TP_ARGS(id, protocol_id, seq, status),
+
+	TP_STRUCT__entry(
+		__field(u8, id)
+		__field(u8, protocol_id)
+		__field(u16, seq)
+		__field(u32, status)
+	),
+
+	TP_fast_assign(
+		__entry->id = id;
+		__entry->protocol_id = protocol_id;
+		__entry->seq = seq;
+		__entry->status = status;
+	),
+
+	TP_printk("id=%u protocol_id=%u seq=%u status=%u", __entry->id,
+		__entry->protocol_id, __entry->seq, __entry->status)
+);

Hello,

When there are multiple messages in the mbox queue, I've found it
a chore matching up the 'begin' event with the 'end' event for each
SCMI msg.  The id (command) may not be unique, the proto_id may not be
unique, and the seq may not be unique.  The combination of the three
may not be unique.  Would it make sense to assign a monotonically
increasing ID to each msg so that one can easily match the two events
for each msg?  This id could be the result of an atomic increment and
could be stored in the xfer structure.  Of course, it would be one of
the values printed out in the events.

Also, would you consider a third event, right after the
scmi_fetch_response() invocation in scmi_rx_callback()?  I've found
this to be insightful in situations where we were debugging a timeout.

I'm fine if you elect not to do the above; I just wanted to post
this for your consideration.

Thanks,
Jim Quinlan
Broadcom

             reply	other threads:[~2019-12-16 16:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-16 16:16 lukasz.luba [this message]
2019-12-16 16:16 ` [PATCH 2/2] drivers: firmware: scmi: Extend SCMI transport layer by trace events lukasz.luba
2019-12-16 22:15 ` [PATCH 1/2] include: trace: Add SCMI header with " Jim Quinlan
2019-12-17 10:05 ` Lukasz Luba
2019-12-18 12:09 ` Sudeep Holla
2019-12-18 16:37   ` Jim Quinlan
2019-12-19 16:32     ` Jim Quinlan
2019-12-20  9:20       ` Lukasz Luba
2019-12-20 16:24         ` Jim Quinlan
2019-12-23 15:39           ` Lukasz Luba

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=20191216161650.21844-1-lukasz.luba@arm.com \
    --to=lukasz.luba@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=sudeep.holla@arm.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).