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

[Caleb Connolly]

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@xxxxxxxxxxxxxx/

https://lkml.org/lkml/2020/4/25/159


Patch 3 of this series removes this check entirely, as it is neither 
accurate or useful.

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.

Regards,

Caleb