lkml.org 
[lkml]   [2023]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH] bus: mhi: host: Add tracing support
    From

    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

    \
     
     \ /
      Last update: 2023-10-13 11:55    [W:5.696 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site