RE: [PATCH v5 2/3] scsi: ufs: Allow platform vendors to set rtt

[Avri Altman <Avri.Altman@wdc.com>]

> 
> On 5/23/24 23:06, Peter Wang (王信友) wrote:
> > diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-
> > mediatek.c
> > index c4f997196c57..f8725f3374f7 100644
> > --- a/drivers/ufs/host/ufs-mediatek.c
> > +++ b/drivers/ufs/host/ufs-mediatek.c
> > @@ -1777,6 +1777,32 @@ static int ufs_mtk_config_esi(struct ufs_hba
> > *hba)
> >          return ufs_mtk_config_mcq(hba, true);
> >   }
> >
> > +static void ufs_mtk_set_rtt(struct ufs_hba *hba)
> > +{
> > +       struct ufs_dev_info *dev_info = &hba->dev_info;
> > +       u32 rtt = 0;
> > +       u32 dev_rtt = 0;
> > +
> > +       /* RTT override makes sense only for UFS-4.0 and above */
> > +       if (dev_info->wspecversion < 0x400)
> > +               return;
> > +
> > +       if (ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> > +                                   QUERY_ATTR_IDN_MAX_NUM_OF_RTT, 0,
> > 0, &dev_rtt)) {
> > +               dev_err(hba->dev, "failed reading bMaxNumOfRTT\n");
> > +               return;
> > +       }
> > +
> > +       /* override if not mediatek support */
> > +       if (dev_rtt == MTK_MAX_NUM_RTT)
> > +               return;
> > +
> > +       rtt = MTK_MAX_NUM_RTT;
> > +       if (ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> > +                                   QUERY_ATTR_IDN_MAX_NUM_OF_RTT, 0,
> > 0, &rtt))
> > +               dev_err(hba->dev, "failed writing bMaxNumOfRTT\n");
> > +}
> > +
> >   /*
> >    * struct ufs_hba_mtk_vops - UFS MTK specific variant operations
> >    *
> > @@ -1805,6 +1831,7 @@ static const struct ufs_hba_variant_ops
> > ufs_hba_mtk_vops = {
> >          .op_runtime_config   = ufs_mtk_op_runtime_config,
> >          .mcq_config_resource = ufs_mtk_mcq_config_resource,
> >          .config_esi          = ufs_mtk_config_esi,
> > +       .set_rtt             = ufs_mtk_set_rtt,
> >   };
> >
> >   /**
> > diff --git a/drivers/ufs/host/ufs-mediatek.h b/drivers/ufs/host/ufs-
> > mediatek.h
> > index 3ff17e95afab..05d76a6bd772 100644
> > --- a/drivers/ufs/host/ufs-mediatek.h
> > +++ b/drivers/ufs/host/ufs-mediatek.h
> > @@ -189,4 +189,7 @@ struct ufs_mtk_host {
> >   /* MTK delay of autosuspend: 500 ms */
> >   #define MTK_RPM_AUTOSUSPEND_DELAY_MS 500
> >
> > +/* MTK RTT support number */
> > +#define MTK_MAX_NUM_RTT 2
> > +
> >   #endif /* !_UFS_MEDIATEK_H */
> 
> The above patch would result in duplication of code. We should avoid
> code duplication if that's reasonably possible. Instead of applying the
> above patch, I propose to add a callback function pointer in struct
> ufs_hba_variant_ops that returns the maximum RTT value supported by the
> host driver.
Yes.  That would simplify things.
Would replace that with the nortt capability.

Thanks,
Avri

> 
> Thanks,
> 
> Bart.