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

[Sergey Shtylyov]

On 4/28/26 11:15 AM, Rand Deeb wrote:

[...]

>>> From: Sergey Shtylyov <s.shtylyov@xxxxxx>
>>>
>>> [ 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@xxxxxx>
>>> Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
>>> Signed-off-by: Rand Deeb <rand.sec96@xxxxxxxxx>
>>> ---
>>>  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.

   I figured. :-)

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

   Thank you for reminding me what my patch does... :-)

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

   What tooling, Svace?

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

   If I considered this a fix for something, I'd have added
the Fixes tag -- then it would be worthy of backporting. Now
it's not, I think...

> Thanks

MBR, Sergey