Re: [PATCH v9] bus: mhi: host: Add tracing support
From: Krishna Chaitanya Chundru
Date: Tue Jan 30 2024 - 20:50:28 EST
On 1/30/2024 1:41 PM, Manivannan Sadhasivam wrote:
On Fri, Jan 05, 2024 at 05:53:03PM +0530, Krishna chaitanya chundru wrote:
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
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
Change the implementation of the arrays which has enum to strings mapping
to make it consistent in both trace header file and other files.
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru <quic_krichai@xxxxxxxxxxx>
Few nitpicks below.
I will make suggested changes in next patch.
- Krishna Chaitanya.
Reviewed-by: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
---
Changes in v9:
- Change the implementations of some array so that the strings to enum mapping
- is same in both trace header and other files as suggested by steve.
- Link to v8: https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558555@xxxxxxxxxxx
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign as suggested by steve
- Link to v7: https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a04268b@xxxxxxxxxxx
Changes in v7:
- change log format as pointed by mani.
- Link to v6: https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546dac2@xxxxxxxxxxx
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5: https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead4f1@xxxxxxxxxxx
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address to avoid
- warnings in some platforms.
- Link to v4: https://lore.kernel.org/r/20231111-ftrace_support-v4-1-c83602399461@xxxxxxxxxxx
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation issue is not
- seen in my workspace.
- Link to v3: https://lore.kernel.org/r/20231111-ftrace_support-v3-1-f358d2911a74@xxxxxxxxxxx
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2: https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce010b5@xxxxxxxxxxx
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1: https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394fa49@xxxxxxxxxxx
---
drivers/bus/mhi/common.h | 38 +++---
drivers/bus/mhi/host/init.c | 63 +++++----
drivers/bus/mhi/host/internal.h | 40 ++++++
drivers/bus/mhi/host/main.c | 19 ++-
drivers/bus/mhi/host/pm.c | 7 +-
drivers/bus/mhi/host/trace.h | 275 ++++++++++++++++++++++++++++++++++++++++
6 files changed, 378 insertions(+), 64 deletions(-)
[...]
+TRACE_EVENT(mhi_gen_tre,
+
+ TP_PROTO(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan,
+ struct mhi_ring_element *mhi_tre),
+
+ TP_ARGS(mhi_cntrl, mhi_chan, mhi_tre),
+
+ TP_STRUCT__entry(
+ __string(name, mhi_cntrl->mhi_dev->name)
+ __field(int, ch_num)
+ __field(void *, wp)
+ __field(__le64, tre_ptr)
+ __field(__le32, dword0)
+ __field(__le32, dword1)
+ ),
+
+ TP_fast_assign(
+ __assign_str(name, mhi_cntrl->mhi_dev->name);
+ __entry->ch_num = mhi_chan->chan;
+ __entry->wp = mhi_tre;
+ __entry->tre_ptr = mhi_tre->ptr;
+ __entry->dword0 = mhi_tre->dword[0];
+ __entry->dword1 = mhi_tre->dword[1];
+ ),
+
+ TP_printk("%s: Chan: %d Tre: 0x%p Tre buf: 0x%llx dword0: 0x%08x dword1: 0x%08x\n",
Use caps for printing the acronyms everywhere. Like TRE, DWORD etc...
+ __get_str(name), __entry->ch_num, __entry->wp, __entry->tre_ptr,
+ __entry->dword0, __entry->dword1)
+);
+
+TRACE_EVENT(mhi_intvec_states,
+
+ TP_PROTO(struct mhi_controller *mhi_cntrl, int dev_ee, int dev_state),
+
+ TP_ARGS(mhi_cntrl, dev_ee, dev_state),
+
+ TP_STRUCT__entry(
+ __string(name, mhi_cntrl->mhi_dev->name)
+ __field(int, local_ee)
+ __field(int, state)
+ __field(int, dev_ee)
+ __field(int, dev_state)
+ ),
+
+ TP_fast_assign(
+ __assign_str(name, mhi_cntrl->mhi_dev->name);
+ __entry->local_ee = mhi_cntrl->ee;
+ __entry->state = mhi_cntrl->dev_state;
+ __entry->dev_ee = dev_ee;
+ __entry->dev_state = dev_state;
+ ),
+
+ TP_printk("%s: local ee: %s state: %s device ee: %s state: %s\n",
"Local EE... State:...Device EE.."
+ __get_str(name),
+ __print_symbolic(__entry->local_ee, MHI_EE_LIST),
+ __print_symbolic(__entry->state, MHI_STATE_LIST),
+ __print_symbolic(__entry->dev_ee, MHI_EE_LIST),
+ __print_symbolic(__entry->state, MHI_STATE_LIST))
+);
+
[...]
+DECLARE_EVENT_CLASS(mhi_update_channel_state,
+
+ TP_PROTO(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan, int state),
+
+ TP_ARGS(mhi_cntrl, mhi_chan, state),
+
+ TP_STRUCT__entry(
+ __string(name, mhi_cntrl->mhi_dev->name)
+ __field(int, ch_num)
+ __field(int, state)
+ ),
+
+ TP_fast_assign(
+ __assign_str(name, mhi_cntrl->mhi_dev->name);
+ __entry->ch_num = mhi_chan->chan;
+ __entry->state = state;
+ ),
+
+ TP_printk("%s: chan%d: Updating state to: %s\n",
+ __get_str(name), __entry->ch_num,
+ __print_symbolic(__entry->state, MHI_CH_STATE_TYPE_LIST))
So same trace will get printed for both mhi_channel_command_start() and
mhi_channel_command_end()?
- Mani