Received-SPF: pass (google.com: domain of linux-kernel+bounces-36119-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 23 Jan 2024 14:10:14 -0800
From: Bjorn Andersson <quic_bjorande@quicinc.com>
To: Chris Lew <quic_clew@quicinc.com>
CC: Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio
	<konrad.dybcio@linaro.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] soc: qcom: aoss: Add tracepoints in qmp_send()
Message-ID: <20240123221014.GG2936378@hu-bjorande-lv.qualcomm.com>
References: <20240117-qcom-aoss-tracepoints-v1-1-4f935920cf4b@quicinc.com>
 <02dc2748-e73d-f565-9879-6a05e84cbd8b@quicinc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <02dc2748-e73d-f565-9879-6a05e84cbd8b@quicinc.com>

On Tue, Jan 23, 2024 at 10:54:16AM -0800, Chris Lew wrote:
> 
> 
> On 1/17/2024 7:52 PM, Bjorn Andersson wrote:
> > @@ -235,6 +238,8 @@ int qmp_send(struct qmp *qmp, const char *fmt, ...)
> >   	mutex_lock(&qmp->tx_lock);
> > +	trace_aoss_send(buf);
> > +
> >   	/* The message RAM only implements 32-bit accesses */
> >   	__iowrite32_copy(qmp->msgram + qmp->offset + sizeof(u32),
> >   			 buf, sizeof(buf) / sizeof(u32));
> > @@ -256,6 +261,8 @@ int qmp_send(struct qmp *qmp, const char *fmt, ...)
> >   		ret = 0;
> >   	}
> > +	trace_aoss_send_done(buf, ret);
> > +
> 
> As a side note, another place where we've traced before is on the receiving
> irq to get the full timing of how long AOSS takes to process a command. I
> think we've discussed this in the past and decided that we can use kprobes
> if that need occurs.
> 

Right, these two tracepoints would not provide an accurate measurement
for the that time. But as we discussed, we can use this in combination
with other events to acquire such measurements.

> >   	mutex_unlock(&qmp->tx_lock);
> >   	return ret;
> > diff --git a/drivers/soc/qcom/trace-aoss.h b/drivers/soc/qcom/trace-aoss.h
> > new file mode 100644
> > index 000000000000..48cd3f0f4cb8
> > --- /dev/null
> > +++ b/drivers/soc/qcom/trace-aoss.h
> > @@ -0,0 +1,48 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> > + */
> > +
> > +#undef TRACE_SYSTEM
> > +#define TRACE_SYSTEM qcom_aoss
> > +
> > +#if !defined(_TRACE_RPMH_H) || defined(TRACE_HEADER_MULTI_READ)
> > +#define _TRACE_RPMH_H
> 
> Any Reason for this to be _TRACE_RPMH_H instead of _TRACE_AOSS_H?
> 

That's what copy-paste gave me... Thanks for spotting that!

Regards,
Bjorn