On 4/10/22 05:29, Sergey Shtylyov wrote:
> sil680_sel{dev|reg}() return a PCI config space address but needlessly
> use the *unsigned long* type for that, whereas the PCI config space
> accessors take *int* for the address parameter. Switch these functions
> to returning *int*, updating the local variables at their call sites.
> Add the empty lines after some declarations, while at it...
>
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
>
> Signed-off-by: Sergey Shtylyov <s.shtylyov@xxxxxx>
>
> ---
> This patch is against the 'for-next' branch of Damien Le Moal's 'libata.git'
> repo.
>
> drivers/ata/pata_sil680.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> Index: libata/drivers/ata/pata_sil680.c
> ===================================================================
> --- libata.orig/drivers/ata/pata_sil680.c
> +++ libata/drivers/ata/pata_sil680.c
> @@ -47,9 +47,10 @@
> * criticial.
> */
>
> -static unsigned long sil680_selreg(struct ata_port *ap, int r)
> +static int sil680_selreg(struct ata_port *ap, int r)
> {
> - unsigned long base = 0xA0 + r;
> + int base = 0xA0 + r;
> +
> base += (ap->port_no << 4);
> return base;
The variable "base" is rather useless here... A simple:
return 0xA0 + r + (ap->port_no << 4);
would work too and is a lot cleaner.
> }
> @@ -65,9 +66,10 @@ static unsigned long sil680_selreg(struc
> * the unit shift.
> */
>
> -static unsigned long sil680_seldev(struct ata_port *ap, struct ata_device *adev, int r)
> +static int sil680_seldev(struct ata_port *ap, struct ata_device *adev, int r)
> {
> - unsigned long base = 0xA0 + r;
> + int base = 0xA0 + r;
> +
> base += (ap->port_no << 4);
> base |= adev->devno ? 2 : 0;
> return base;
> @@ -85,8 +87,9 @@ static unsigned long sil680_seldev(struc
> static int sil680_cable_detect(struct ata_port *ap)
> {
> struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> - unsigned long addr = sil680_selreg(ap, 0);
> + int addr = sil680_selreg(ap, 0);
> u8 ata66;
> +
> pci_read_config_byte(pdev, addr, &ata66);
> if (ata66 & 1)
> return ATA_CBL_PATA80;
> @@ -113,9 +116,9 @@ static void sil680_set_piomode(struct at
> 0x328A, 0x2283, 0x1281, 0x10C3, 0x10C1
> };
>
> - unsigned long tfaddr = sil680_selreg(ap, 0x02);
> - unsigned long addr = sil680_seldev(ap, adev, 0x04);
> - unsigned long addr_mask = 0x80 + 4 * ap->port_no;
> + int tfaddr = sil680_selreg(ap, 0x02);
> + int addr = sil680_seldev(ap, adev, 0x04);
> + int addr_mask = 0x80 + 4 * ap->port_no;
> struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> int pio = adev->pio_mode - XFER_PIO_0;
> int lowest_pio = pio;
> @@ -165,9 +168,9 @@ static void sil680_set_dmamode(struct at
> static const u16 dma_table[3] = { 0x2208, 0x10C2, 0x10C1 };
>
> struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> - unsigned long ma = sil680_seldev(ap, adev, 0x08);
> - unsigned long ua = sil680_seldev(ap, adev, 0x0C);
> - unsigned long addr_mask = 0x80 + 4 * ap->port_no;
> + int ma = sil680_seldev(ap, adev, 0x08);
> + int ua = sil680_seldev(ap, adev, 0x0C);
> + int addr_mask = 0x80 + 4 * ap->port_no;
> int port_shift = adev->devno * 4;
> u8 scsc, mode;
> u16 multi, ultra;
> @@ -219,7 +222,7 @@ static void sil680_sff_exec_command(stru
> static bool sil680_sff_irq_check(struct ata_port *ap)
> {
> struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> - unsigned long addr = sil680_selreg(ap, 1);
> + int addr = sil680_selreg(ap, 1);
> u8 val;
>
> pci_read_config_byte(pdev, addr, &val);
--
Damien Le Moal
Western Digital Research
