RE: [RFC PATCH v3 1/2] ufs: support various values per device

["Grant Jung" <grant.jung@samsung.com>]

> > Respective UFS devices have their own characteristics and many of them
> > could be a form of numbers, such as timeout and a number of retires.
> > This introduces the way to set those things per specific device vendor
> > or specific device.
> >
> > I wrote this like the style of ufs_fixups stuffs.
> >
> > Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com>
> This patch legitimize quirks of all kinds and shapes.
> I am not sure that we should allow it.
> 
> 
> > ---
> >  drivers/scsi/ufs/ufs_quirks.h | 13 +++++++++++++
> >  drivers/scsi/ufs/ufshcd.c     | 39
> > +++++++++++++++++++++++++++++++++++++++
> >  drivers/scsi/ufs/ufshcd.h     |  1 +
> >  3 files changed, 53 insertions(+)
> >
> > diff --git a/drivers/scsi/ufs/ufs_quirks.h
> > b/drivers/scsi/ufs/ufs_quirks.h index 2a00414..f074093 100644
> > --- a/drivers/scsi/ufs/ufs_quirks.h
> > +++ b/drivers/scsi/ufs/ufs_quirks.h
> > @@ -29,6 +29,19 @@ struct ufs_dev_fix {
> >         unsigned int quirk;
> >  };
> >
> > +enum dev_val_type {
> > +       DEV_VAL_FDEVICEINIT     = 0x0,
> 
>             /* keep last */
> > +       DEV_VAL_NUM,
> > +};
> > +
> > +struct ufs_dev_value {
> > +       u16 wmanufacturerid;
> > +       u8 *model;
> > +       u32 key;
> > +       u32 val;
> > +       bool enable;
> > +};
> > +
> >  #define END_FIX { }
> >
> >  /* add specific device quirk */
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 52abe82..b26f182 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -207,6 +207,21 @@ static struct ufs_dev_fix ufs_fixups[] = {
> >         END_FIX
> >  };
> >
> > +static const struct ufs_dev_value ufs_dev_values[] = {
> > +       {0, 0, 0, 0, false},
> > +};
> > +
> > +static inline bool
> > +ufs_get_dev_specific_value(struct ufs_hba *hba,
> > +                          enum dev_val_type type, u32 *val) {
> If (ARRAY_SIZE(ufs_dev_values) <= type)
>     return false;
> 
> 
> Thanks,
> Avri

There is no specification for fdeviceinit timeout value like eMMC CMD1 which is 1s.
Usually this value is small but can be increased under some abnormal situation like SPO(fdeviceinit after Sudden Power Off).
I think that 1000 retries take less than 1 second but it is inaccurate and not enough. Some UFS vendor wants 1.5s.
Moreover, the latency of resuming ufs driver can be dependent on this value when vcc and vccq is off during suspend.
So it's bad to set with big value to apply all UFS devices.
 
I wonder quirk is needed for that.

BR
Grant