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

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

Hello!

On 4/11/22 2:47 AM, Damien Le Moal 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@omp.ru>
>>
>> ---
>> 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.

   Yes, probably... but it's a matter of a separate patch, I think.
Note that both functions are inlined by gcc.

[...]

MBR, Sergey