Re: [PATCH] bus: mhi: host: Add tracing support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/6/2023 4:10 AM, Bjorn Andersson wrote:
On Thu, Oct 05, 2023 at 03:55:20PM +0530, Krishna chaitanya chundru wrote:
This change adds ftrace support for following:
1. mhi_intvec_threaded_handler
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
This is not the best "problem description".

Usage:
	echo 1 > /sys/kernel/debug/tracing/events/mhi_host/enable
	cat /sys/kernel/debug/tracing/trace
This does not need to be included in the commit message, how to use the
tracing framework is documented elsewhere.
Removed this now.
[..]
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..499590437e9b 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -491,11 +491,10 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number, void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
  	ee = mhi_get_exec_env(mhi_cntrl);
-	dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
-		TO_MHI_EXEC_STR(mhi_cntrl->ee),
-		mhi_state_str(mhi_cntrl->dev_state),
-		TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_threaded_handler(mhi_cntrl->mhi_dev->name, TO_MHI_EXEC_STR(mhi_cntrl->ee),
+					  mhi_state_str(mhi_cntrl->dev_state),
+					  TO_MHI_EXEC_STR(ee), mhi_state_str(state));
All these helper functions that translates a state to a string, pass the
raw state into the trace event and use __print_symbolic() in your
TP_printk() instead.

This will allow you to read the state, but you can have tools act of the
numerical value.


(This comment applies to all the trace events)
changed the events as you suggested.
  	if (state == MHI_STATE_SYS_ERR) {
  		dev_dbg(dev, "System error detected\n");
  		pm_state = mhi_tryset_pm_state(mhi_cntrl,
[..]
diff --git a/include/trace/events/mhi_host.h b/include/trace/events/mhi_host.h
[..]
+
+TRACE_EVENT(mhi_pm_st_worker,
Why is this trace event called "worker", isn't the event a
"mhi_pm_state_transition"?

Don't just name your trace event based on the function that triggers
them, but what they represent and make sure they carry useful
information to understand the system.

If you want to trace the flow through your functions, you can use e.g.
ftrace.

Regards,
Bjorn

Changed as you suggested.

- KC




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux