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. > 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 -- மணிவண்ணன் சதாசிவம்