linux-kernel - Re: [PATCH 7/8] scsi: esas2r: Use FIELD

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: <20230912113817.000064db@Huawei.com>
Date:   Tue, 12 Sep 2023 11:38:17 +0100
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC:     <linux-pci@...r.kernel.org>, Bjorn Helgaas <helgaas@...nel.org>,
        "Bradley Grove" <linuxdrivers@...otech.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        <linux-scsi@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 7/8] scsi: esas2r: Use FIELD_GET() to extract Link Width

On Mon, 11 Sep 2023 15:15:00 +0300
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com> wrote:

> Use FIELD_GET() to extract PCIe Negotiated and Maximum Link Width fields
> instead of custom masking and shifting.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> ---
>  drivers/scsi/esas2r/esas2r_ioctl.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c b/drivers/scsi/esas2r/esas2r_ioctl.c
> index 055d2e87a2c8..3252780fd099 100644
> --- a/drivers/scsi/esas2r/esas2r_ioctl.c
> +++ b/drivers/scsi/esas2r/esas2r_ioctl.c
> @@ -41,6 +41,8 @@
>   * USA.
>   */
>  
> +#include <linux/bitfield.h>
> +
>  #include "esas2r.h"
>  
>  /*
> @@ -797,11 +799,9 @@ static int hba_ioctl_callback(struct esas2r_adapter *a,
>  			gai->pci.link_speed_max =
>  				(u8)(caps & PCI_EXP_LNKCAP_SLS);
Better to convert the other field gets as well.

I'm curious as to why the u8 casts are here. The masking should have
kept the compiler happy that it is fine to assign these without
the casts and no chance of overflow.

>  			gai->pci.link_width_curr =
> -				(u8)((stat & PCI_EXP_LNKSTA_NLW)
> -				     >> PCI_EXP_LNKSTA_NLW_SHIFT);  
> +				(u8)FIELD_GET(PCI_EXP_LNKSTA_NLW, stat);
>  			gai->pci.link_width_max =
> -				(u8)((caps & PCI_EXP_LNKCAP_MLW)
> -				     >> 4);  
> +				(u8)FIELD_GET(PCI_EXP_LNKCAP_MLW, caps);
>  		}
>  
>  		gai->pci.msi_vector_cnt = 1;