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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f8284c73-34d1-e1a7-6c47-563a0057a9c0@connolly.tech>
Date:   Wed, 10 Mar 2021 17:06:22 +0000
From:   Caleb Connolly <caleb@...nolly.tech>
To:     Avri Altman <Avri.Altman@....com>,
        Alim Akhtar <alim.akhtar@...sung.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>
Cc:     "ejb@...ux.ibm.com" <ejb@...ux.ibm.com>,
        "stanley.chu@...iatek.com" <stanley.chu@...iatek.com>,
        "cang@...eaurora.org" <cang@...eaurora.org>,
        "beanhuo@...ron.com" <beanhuo@...ron.com>,
        "jaegeuk@...nel.org" <jaegeuk@...nel.org>,
        "asutoshd@...eaurora.org" <asutoshd@...eaurora.org>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH v3 1/3] scsi: ufshcd: use a function to calculate versions

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.

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