Hi Avri,

On Wed, 2019-07-10 at 08:04 +0000, Avri Altman wrote:
> Hi Stanley,
> 
> > +                                          (u32)ret);
> >                     goto out;
> > +           }
> >     } while (ret && retries--);
> > 
> >     if (ret)
> Here also link startup fails...

Thanks! Will track this place as well in next version.

> >   * ufshcd_update_uic_error - check and set fatal UIC error flags.
> >   * @hba: per-adapter instance
> > @@ -6043,6 +6056,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
> >      */
> >     scsi_print_command(hba->lrb[tag].cmd);
> >     if (!hba->req_abort_count) {
> > +           ufshcd_update_reg_hist(&hba->ufs_stats.task_abort_err,
> > 0);
> Here you are collecting abort events statistics, not abort errors.
> If this is what you meant, then it's not task_abort_err, but task_abort.
> And if indeed you are tracking task aborts, maybe add lun resets as well?

Good suggestion! I would add history of lun reset and host reset as well
in next version.

> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index c6ec5c749ceb..f9f109da7f18 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -438,6 +438,10 @@ struct ufs_err_reg_hist {
> >   * @dme_err: tracks dme errors
> >   * @fatal_err: tracks fatal errors
> >   * @auto_hibern8_err: tracks auto-hibernate errors
> > + * @tsk_abort_err: tracks task abort events
> > + * @linkup_err: tracks link-startup fail events
> > + * @suspend_err: tracks suspend fail events
> > + * @resume_err: tracks resume fail events
> >   */
> >  struct ufs_stats {
> >     u32 hibern8_exit_cnt;
> > @@ -453,6 +457,12 @@ struct ufs_stats {
> >     /* fatal errors */
> >     struct ufs_err_reg_hist fatal_err;
> >     struct ufs_err_reg_hist auto_hibern8_err;
> > +
> > +   /* fatal events */
> Maybe move here fatal_err as well?

OK! these could be classified as fatal errors as well.
Will fix them in next version.

> 
> > +   struct ufs_err_reg_hist task_abort_err;
> > +   struct ufs_err_reg_hist link_startup_err;
> > +   struct ufs_err_reg_hist suspend_err;
> > +   struct ufs_err_reg_hist resume_err;
> >  };
> > 
> >  /**
> > --
> > 2.18.0
> 
> 
> Thanks,
> Avri
> 

Thanks,
Stanley



Reply via email to