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:0.046 / U:0.144 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site