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

[Rand Deeb]

On Mon, Apr 20, 2026 at 4:11 PM Sergey Shtylyov <s.shtylyov at omp.ru> wrote:
>
> On 4/20/26 1:23 AM, Rand Deeb wrote:
>
> > From: Sergey Shtylyov <s.shtylyov at 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 at omp.ru>
> > Signed-off-by: Damien Le Moal <damien.lemoal at opensource.wdc.com>
> > Signed-off-by: Rand Deeb <rand.sec96 at 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
>

Hi Sergey,

This is a direct backport of upstream commit dafbbf5c57dd.

It fixes a type mismatch between the helper functions and the
PCI config accessors (which expect int), as identified by
static analysis (SVACE).

The intent is to keep the code consistent and avoid issues
flagged by tooling.

If this is not considered appropriate for 5.10.y, I’m fine
with dropping it.

Thanks