RE: [PATCH v3 1/3] scsi: ufshcd: use a function to calculate versions

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

> Hi Avri,
> 
> On 10/03/2021 4:34 pm, Avri Altman wrote:
> >> @@ -9298,10 +9291,7 @@ int ufshcd_init(struct ufs_hba *hba, void
> __iomem
> >> *mmio_base, unsigned int irq)
> >>          /* Get UFS version supported by the controller */
> >>          hba->ufs_version = ufshcd_get_ufs_version(hba);
> >>
> >> -       if ((hba->ufs_version != UFSHCI_VERSION_10) &&
> >> -           (hba->ufs_version != UFSHCI_VERSION_11) &&
> >> -           (hba->ufs_version != UFSHCI_VERSION_20) &&
> >> -           (hba->ufs_version != UFSHCI_VERSION_21))
> >> +       if (hba->ufs_version < ufshci_version(1, 0))
> >>                  dev_err(hba->dev, "invalid UFS version 0x%x\n",
> >>                          hba->ufs_version);
> > Here you replaces the specific allowable values, with an expression
> > That doesn't really reflects those values.
> 
> I took this approach based on feedback from previous patches:
> 
> https://lore.kernel.org/linux-
> scsi/d1b23943b6b3ae6c1f6af041cc592932@codeaurora.org/
> 
> https://lkml.org/lkml/2020/4/25/159
> 
> 
> Patch 3 of this series removes this check entirely, as it is neither
> accurate or useful.
I noticed that.

> 
> The driver does not fail when printing this error, nor is the list of
> "valid" UFS versions here kept up to date, I struggle to see a situation
> in which that error message would actually be helpful. Responses to
> previous patches (above) that added UFS 3.0 to the list have all
> suggested that removing this check is a more sensible approach.
OK.

Thanks,
Avri