On Mon 09 Nov 19:20 CST 2020, Rishabh Bhatnagar wrote: > Add tracepoints for q6v5_pas driver. These will help in > analyzing the time taken by each step in remoteproc > bootup/shutdown process and also serve as standard > checkpoints in code. > These tracepoints seems quite generic and useful to drivers other than the Qualcomm PAS driver. Please move them into the framework instead. > Signed-off-by: Rishabh Bhatnagar <rishabhb@xxxxxxxxxxxxxx> > --- > drivers/remoteproc/qcom_q6v5_pas.c | 11 +++++++++++ > include/trace/events/q6v5_pas.h | 34 ++++++++++++++++++++++++++++++++++ > 2 files changed, 45 insertions(+) > create mode 100644 include/trace/events/q6v5_pas.h > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > index 3837f23..b3c0a6a 100644 > --- a/drivers/remoteproc/qcom_q6v5_pas.c > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > @@ -29,6 +29,9 @@ > #include "qcom_q6v5.h" > #include "remoteproc_internal.h" > > +#define CREATE_TRACE_POINTS > +#include <trace/events/q6v5_pas.h> > + > struct adsp_data { > int crash_reason_smem; > const char *firmware_name; > @@ -121,12 +124,14 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw) > struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; > int ret; > > + trace_q6v5_pas("setting up memory and loading segments", rproc->name); > ret = qcom_mdt_load(adsp->dev, fw, rproc->firmware, adsp->pas_id, > adsp->mem_region, adsp->mem_phys, adsp->mem_size, > &adsp->mem_reloc); > if (ret) > return ret; > > + trace_q6v5_pas("done loading segments", rproc->name); > qcom_pil_info_store(adsp->info_name, adsp->mem_phys, adsp->mem_size); > > return 0; > @@ -137,6 +142,7 @@ static int adsp_start(struct rproc *rproc) > struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; > int ret; > > + trace_q6v5_pas("Voting for resources", rproc->name); > qcom_q6v5_prepare(&adsp->q6v5); > > ret = adsp_pds_enable(adsp, adsp->active_pds, adsp->active_pd_count); > @@ -163,12 +169,14 @@ static int adsp_start(struct rproc *rproc) > if (ret) > goto disable_cx_supply; > > + trace_q6v5_pas("Before authenticate and reset", rproc->name); > ret = qcom_scm_pas_auth_and_reset(adsp->pas_id); > if (ret) { > dev_err(adsp->dev, > "failed to authenticate image and release reset\n"); > goto disable_px_supply; > } > + trace_q6v5_pas("After authenticate and reset", rproc->name); > > ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000)); > if (ret == -ETIMEDOUT) { > @@ -177,6 +185,7 @@ static int adsp_start(struct rproc *rproc) > goto disable_px_supply; > } > > + trace_q6v5_pas("Remoteproc is up", rproc->name); > return 0; > > disable_px_supply: > @@ -214,6 +223,7 @@ static int adsp_stop(struct rproc *rproc) > int handover; > int ret; > > + trace_q6v5_pas("Request stop", rproc->name); > ret = qcom_q6v5_request_stop(&adsp->q6v5); > if (ret == -ETIMEDOUT) > dev_err(adsp->dev, "timed out on wait\n"); > @@ -227,6 +237,7 @@ static int adsp_stop(struct rproc *rproc) > if (handover) > qcom_pas_handover(&adsp->q6v5); > > + trace_q6v5_pas("Remoteproc is down", rproc->name); > return ret; > } > > diff --git a/include/trace/events/q6v5_pas.h b/include/trace/events/q6v5_pas.h > new file mode 100644 > index 0000000..38ee5e2 > --- /dev/null > +++ b/include/trace/events/q6v5_pas.h > @@ -0,0 +1,34 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2020, The Linux Foundation. All rights reserved. > + */ > + > +#undef TRACE_SYSTEM > +#define TRACE_SYSTEM q6v5_pas > + > +#if !defined(_TRACE_Q6V5_PAS_H) || defined(TRACE_HEADER_MULTI_READ) > +#define _TRACE_Q6V5_PAS_H > + > +#include <linux/tracepoint.h> > + > +TRACE_EVENT(q6v5_pas, > + > + TP_PROTO(const char *event, const char *rproc_name), Rather than distinguishing the trace events by the textual first parameter, split it into individual trace events for each event. Regards, Bjorn > + > + TP_ARGS(event, rproc_name), > + > + TP_STRUCT__entry( > + __string(event, event) > + __string(rproc_name, rproc_name) > + ), > + > + TP_fast_assign( > + __assign_str(event, event); > + __assign_str(rproc_name, rproc_name); > + ), > + > + TP_printk("event=%s remoteproc:%s", __get_str(event), __get_str(rproc_name)) > +); > + > +#endif > +#include <trace/define_trace.h> > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >