Re: [PATCH 5.10.y] ata: pata_sil680: fix result type of sil680_sel{dev|reg}()

[Sergey Shtylyov <s.shtylyov@omp.ru>]

On 4/20/26 1:23 AM, Rand Deeb wrote:

> From: Sergey Shtylyov <s.shtylyov@omp.ru>
> 
> [ Upstream commit dafbbf5c57dd6ae01d20b894bc2200e9d9834c4e ]
> 
> 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.
> Get rid of the 'base' local variables in these functions, while at it...
> 
> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Signed-off-by: Rand Deeb <rand.sec96@gmail.com>
> ---
>  drivers/ata/pata_sil680.c | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/ata/pata_sil680.c b/drivers/ata/pata_sil680.c
> index 7ab9aea3b..fe60f884b 100644
> --- a/drivers/ata/pata_sil680.c
> +++ b/drivers/ata/pata_sil680.c
> @@ -47,11 +47,9 @@
>   *     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;
> -       base += (ap->port_no << 4);
> -       return base;
> +       return 0xA0 + (ap->port_no << 4) + r;
>  }
> 
>  /**
> @@ -64,12 +62,9 @@ static unsigned long sil680_selreg(struct ata_port *ap, int r)
>   *     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;
> -       base += (ap->port_no << 4);
> -       base |= adev->devno ? 2 : 0;
> -       return base;
> +       return 0xA0 + (ap->port_no << 4) + r + (adev->devno << 1);
>  }

   And why exactly is this needed in 5.10.y?

[...]
MBR, Sergey