RE: [PATCH 2/2] scsi: ufs: core: Add a trace function calling when uic command error occurs

["DooHyun Hwang" <dh0421.hwang@samsung.com>]

> On Thu, 2025-04-17 at 11:34 +0900, DooHyun Hwang wrote:
> >
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >
> >
> > When a uic command error occurs, there is no trace function calling.
> > Therefore, trace function calls are added when a uic command error
> > happens.
> >
> > Signed-off-by: DooHyun Hwang <dh0421.hwang@samsung.com>
> > ---
> >  drivers/ufs/core/ufshcd.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index ab98acd982b5..baac1ae94efc 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -2534,6 +2534,9 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba,
> > struct uic_command *uic_cmd)
> >         hba->active_uic_cmd = NULL;
> >         spin_unlock_irqrestore(hba->host->host_lock, flags);
> >
> > +       if (ret)
> > +               ufshcd_add_uic_command_trace(hba, uic_cmd,
> > UFS_CMD_ERR);
> > +
> >         return ret;
> >  }
> >
> > @@ -4306,6 +4309,8 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
> > *hba, struct uic_command *cmd)
> >         }
> >  out:
> >         if (ret) {
> > +               ufshcd_add_uic_command_trace(hba, hba-
> > >active_uic_cmd,
> > +                                            UFS_CMD_ERR);
> >                 ufshcd_print_host_state(hba);
> >                 ufshcd_print_pwr_info(hba);
> >                 ufshcd_print_evt_hist(hba);
> > --
> > 2.48.1
> >
> 
> 
> Hi DooHyun,
> 
> Is it possible to receive UFS_CMD_COMP and UFS_CMD_ERR at the same time?
> Wouldn't it be a bit strange for a command to receive two completions?
> Additionally, should we also add tracing for general SCSI commands that
> receive errors?
> 
> Thanks
> Peter
> 
Thank you for your review.

Yes, that’s correct. There is a case where both UFS_CMD_COMP and UFS_CMD_ERR are logged simultaneously.

In the typical case of a UIC command timeout, only UFS_CMD_ERR would be recorded.

However, the scenario you described earlier, where both UFS_CMD_COMP and UFS_CMD_ERR are received in the same UIC command, is distinct from a timeout situation.
In this case, the command completes in time, and UFS_CMD_COMP is received when handling the UIC command completion interrupt in ufshcd_uic_cmd_compl().
However, the UPMCRS status indicates an error because PWR_LOCAL is not in its expected state (when checking upmcrs in the "check_upmcrs:" section of the ufshcd_uic_pwr_ctrl() function).

UFS_CMD_COMP is generated when the system recognizes through interrupt status that the command has completed its execution, while UFS_CMD_ERR is recorded when an error associated with the command is detected.

While it may be cumbersome, it is possible to trace the timeout case and this case separately.

Regarding tracing general SCSI command errors directly through UFS_CMD_ERR, it might be challenging because SCSI command errors are typically handled through various mechanisms such as timeout detection, scsi_status value checks, sense error checks, and OCS (Overall Command Status) value verification.
Additionally, tracing SCSI command errors does not align with the purpose of this commit.

BR,
Thank you.
DooHyun Hwang