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?

Makes sense.

> 
> >   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.

-- Steve



Reply via email to