On Fri, May 15, 2026 at 3:22 PM Bart Van Assche <[email protected]> wrote:
>
> On 5/15/26 11:50 AM, Steven Rostedt wrote:
> > On Fri, 15 May 2026 08:27:27 -0700
> > Bart Van Assche <[email protected]> wrote:
> >
> >> On 5/15/26 6:59 AM, Vineeth Pillai (Google) wrote:
> >>>    static void ufshcd_add_query_upiu_trace(struct ufs_hba *hba,
> >>> @@ -432,8 +432,8 @@ static void ufshcd_add_query_upiu_trace(struct 
> >>> ufs_hba *hba,
> >>>     if (!trace_ufshcd_upiu_enabled())
> >>>             return;
> >>>
> >>> -   trace_ufshcd_upiu(hba, str_t, &rq_rsp->header,
> >>> -                     &rq_rsp->qr, UFS_TSF_OSF);
> >>> +   trace_call__ufshcd_upiu(hba, str_t, &rq_rsp->header,
> >>> +                          &rq_rsp->qr, UFS_TSF_OSF);
> >>>    }
> >>
> >> Instead of making this change, please remove the
> >> trace_ufshcd_upiu_enabled() call because it is redundant.
> >
> > You mean to remove the ufshcd_add_query_upiu_trace() function and just use
> > a tracepoint where it is called?
>
> That would be even better.
>
Will do.

> >>>    static void ufshcd_add_tm_upiu_trace(struct ufs_hba *hba, unsigned int 
> >>> tag,
> >>> @@ -445,15 +445,15 @@ static void ufshcd_add_tm_upiu_trace(struct ufs_hba 
> >>> *hba, unsigned int tag,
> >>>             return;
> >>>
> >>>     if (str_t == UFS_TM_SEND)
> >>> -           trace_ufshcd_upiu(hba, str_t,
> >>> -                             &descp->upiu_req.req_header,
> >>> -                             &descp->upiu_req.input_param1,
> >>> -                             UFS_TSF_TM_INPUT);
> >>> +           trace_call__ufshcd_upiu(hba, str_t,
> >>> +                                   &descp->upiu_req.req_header,
> >>> +                                   &descp->upiu_req.input_param1,
> >>> +                                   UFS_TSF_TM_INPUT);
> >>>     else
> >>> -           trace_ufshcd_upiu(hba, str_t,
> >>> -                             &descp->upiu_rsp.rsp_header,
> >>> -                             &descp->upiu_rsp.output_param1,
> >>> -                             UFS_TSF_TM_OUTPUT);
> >>> +           trace_call__ufshcd_upiu(hba, str_t,
> >>> +                                   &descp->upiu_rsp.rsp_header,
> >>> +                                   &descp->upiu_rsp.output_param1,
> >>> +                                   UFS_TSF_TM_OUTPUT);
> >>>    }
> >>
> >> Same comment here: I think it would be better to remove the
> >> trace_ufshcd_upiu_enabled() call rather than
> >> changing trace_ufshcd_upiu() into trace_call__ufshcd_upiu().
> >
> > Well, removing it here would mean placing the if (str == UFS_TM_SEND) into
> > the code and processing it even when tracing is disabled. With the
> > trace_*_enabled() helper, it's all a nop.
>
> The ufshcd_add_tm_upiu_trace() function is only called from the UFS
> error handler and hence is not performance sensitive. The execution of
> an additional if-test in this function is not a concern at all.
>
Sure, I shall change this.

Thanks,
Vineeth

Reply via email to