From: Bjorn Andersson <quic_bjorande@quicinc.com>
To: Chris Lew <quic_clew@quicinc.com>
Cc: Sudeepgoud Patil <quic_sudeepgo@quicinc.com>,
<andersson@kernel.org>, <mathieu.poirier@linaro.org>,
<linux-kernel@vger.kernel.org>, <quic_deesin@quicinc.com>,
<linux-arm-msm@vger.kernel.org>,
<linux-remoteproc@vger.kernel.org>,
Konrad Dybcio <konrad.dybcio@linaro.org>
Subject: Re: [PATCH V1] soc: qcom: smp2p: Introduce tracepoint support
Date: Tue, 30 Apr 2024 19:59:26 -0700 [thread overview]
Message-ID: <ZjGwDhqqe/MCKcEF@hu-bjorande-lv.qualcomm.com> (raw)
In-Reply-To: <172ab19b-4643-6ac0-53ab-d2d1e217171c@quicinc.com>
On Tue, Apr 30, 2024 at 04:18:27PM -0700, Chris Lew wrote:
> On 4/29/2024 12:55 AM, Sudeepgoud Patil wrote:
> > Introduce tracepoint support for smp2p providing useful logging
> > for communication between clients.
> >
>
> Let's add some more description as to why these tracepoint are useful. Do
> they help us track latency? debugging information for us? for clients?
+1
> > diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
[..]
> > diff --git a/drivers/soc/qcom/smp2p.c b/drivers/soc/qcom/smp2p.c
> > index a21241cbeec7..dde8513641ae 100644
> > --- a/drivers/soc/qcom/smp2p.c
> > +++ b/drivers/soc/qcom/smp2p.c
> > @@ -20,6 +20,9 @@
> > #include <linux/soc/qcom/smem_state.h>
> > #include <linux/spinlock.h>
> > +#define CREATE_TRACE_POINTS
> > +#include "trace-smp2p.h"
> > +
> > /*
> > * The Shared Memory Point to Point (SMP2P) protocol facilitates communication
> > * of a single 32-bit value between two processors. Each value has a single
> > @@ -191,6 +194,7 @@ static void qcom_smp2p_do_ssr_ack(struct qcom_smp2p *smp2p)
> > struct smp2p_smem_item *out = smp2p->out;
> > u32 val;
> > + trace_smp2p_ssr_ack(smp2p->remote_pid);
> > smp2p->ssr_ack = !smp2p->ssr_ack;
> > val = out->flags & ~BIT(SMP2P_FLAGS_RESTART_ACK_BIT);
> > @@ -213,6 +217,7 @@ static void qcom_smp2p_negotiate(struct qcom_smp2p *smp2p)
> > smp2p->ssr_ack_enabled = true;
> > smp2p->negotiation_done = true;
> > + trace_smp2p_negotiate(smp2p->remote_pid, smp2p->ssr_ack_enabled);
>
> since this tracepoint is for negotiating, maybe we should capture all of the
> features (out->features) instead of just the ssr_ack feature.
>
Perhaps we can use __print_flags() in TP_printk() for that, it will
attempt to resolve the bits and if it fails include the numeric value.
> > }
> > }
[..]
> > diff --git a/drivers/soc/qcom/trace-smp2p.h b/drivers/soc/qcom/trace-smp2p.h
[..]
> > +TRACE_EVENT(smp2p_ssr_ack,
> > + TP_PROTO(unsigned int remote_pid),
>
> Is there any way we can map the remote pid's to a string? I feel like the
> tracepoints would be more useful if they called out modem, adsp, etc instead
> of an integer value.
>
And __print_symbolic() for this one.
Regards,
Bjorn
prev parent reply other threads:[~2024-05-01 2:59 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 7:55 [PATCH V1] soc: qcom: smp2p: Introduce tracepoint support Sudeepgoud Patil
2024-04-30 23:18 ` Chris Lew
2024-05-01 2:59 ` Bjorn Andersson [this message]
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=ZjGwDhqqe/MCKcEF@hu-bjorande-lv.qualcomm.com \
--to=quic_bjorande@quicinc.com \
--cc=andersson@kernel.org \
--cc=konrad.dybcio@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=mathieu.poirier@linaro.org \
--cc=quic_clew@quicinc.com \
--cc=quic_deesin@quicinc.com \
--cc=quic_sudeepgo@quicinc.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 read-only IMAP folder(s) and NNTP newsgroup(s).