[PATCH iwl-next v1] iavf: fix proper type for error code in iavf_resume()

[PATCH iwl-next v1] iavf: fix proper type for error code in iavf_resume()

[Aleksandr Loktionov]

The variable 'err' in iavf_resume() is used to store the return value
of different functions, which return an int. Currently, 'err' is
declared as u32, which is semantically incorrect and misleading.

In the Linux kernel, u32 is typically reserved for fixed-width data
used in hardware interfaces or protocol structures. Using it for a
generic error code may confuse reviewers or developers into thinking
the value is hardware-related or size-constrained.

Replace u32 with int to reflect the actual usage and improve code
clarity and semantic correctness.

No functional change.

Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
---
 drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 69054af..c2fbe44 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -5491,7 +5491,7 @@ static int iavf_resume(struct device *dev_d)
 {
 	struct pci_dev *pdev = to_pci_dev(dev_d);
 	struct iavf_adapter *adapter;
-	u32 err;
+	int err;
 
 	adapter = iavf_pdev_to_adapter(pdev);
 
-- 
2.49.0

Re: [PATCH iwl-next v1] iavf: fix proper type for error code in iavf_resume()

[Simon Horman]

On Fri, Sep 12, 2025 at 08:02:08AM +0000, Aleksandr Loktionov wrote:
> The variable 'err' in iavf_resume() is used to store the return value
> of different functions, which return an int. Currently, 'err' is
> declared as u32, which is semantically incorrect and misleading.
> 
> In the Linux kernel, u32 is typically reserved for fixed-width data
> used in hardware interfaces or protocol structures. Using it for a
> generic error code may confuse reviewers or developers into thinking
> the value is hardware-related or size-constrained.
> 
> Replace u32 with int to reflect the actual usage and improve code
> clarity and semantic correctness.
> 
> No functional change.
> 
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [Intel-wired-lan] [PATCH iwl-next v1] iavf: fix proper type for error code in iavf_resume()

[Paul Menzel]

Dear Aleksandr,


Thank you for your patch.

Am 12.09.25 um 10:02 schrieb Aleksandr Loktionov:
> The variable 'err' in iavf_resume() is used to store the return value
> of different functions, which return an int. Currently, 'err' is
> declared as u32, which is semantically incorrect and misleading.
> 
> In the Linux kernel, u32 is typically reserved for fixed-width data
> used in hardware interfaces or protocol structures. Using it for a
> generic error code may confuse reviewers or developers into thinking
> the value is hardware-related or size-constrained.
> 
> Replace u32 with int to reflect the actual usage and improve code
> clarity and semantic correctness.

Why not use `unsigned int`?

> 
> No functional change.
> 
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
>   drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 69054af..c2fbe44 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -5491,7 +5491,7 @@ static int iavf_resume(struct device *dev_d)
>   {
>   	struct pci_dev *pdev = to_pci_dev(dev_d);
>   	struct iavf_adapter *adapter;
> -	u32 err;
> +	int err;
>   
>   	adapter = iavf_pdev_to_adapter(pdev);
>   

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

Re: [Intel-wired-lan] [PATCH iwl-next v1] iavf: fix proper type for error code in iavf_resume()

[Przemek Kitszel]

On 9/15/25 11:12, Paul Menzel wrote:
> Dear Aleksandr,
> 
> 
> Thank you for your patch.
> 
> Am 12.09.25 um 10:02 schrieb Aleksandr Loktionov:
>> The variable 'err' in iavf_resume() is used to store the return value
>> of different functions, which return an int. Currently, 'err' is
>> declared as u32, which is semantically incorrect and misleading.
>>
>> In the Linux kernel, u32 is typically reserved for fixed-width data
>> used in hardware interfaces or protocol structures. Using it for a
>> generic error code may confuse reviewers or developers into thinking
>> the value is hardware-related or size-constrained.
>>
>> Replace u32 with int to reflect the actual usage and improve code
>> clarity and semantic correctness.
> 
> Why not use `unsigned int`?

I don't think we need to provide rationale for "kernel has adopted
a long standing practice of encoding errors as negative integer codes"
each time we change a type, IOW it's too basic thing to mention.

> 
>>
>> No functional change.
>>
>> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>> ---
>>   drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/ 
>> net/ethernet/intel/iavf/iavf_main.c
>> index 69054af..c2fbe44 100644
>> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
>> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
>> @@ -5491,7 +5491,7 @@ static int iavf_resume(struct device *dev_d)
>>   {
>>       struct pci_dev *pdev = to_pci_dev(dev_d);
>>       struct iavf_adapter *adapter;
>> -    u32 err;
>> +    int err;
>>       adapter = iavf_pdev_to_adapter(pdev);
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>

Thank you

> 
> 
> Kind regards,
> 
> Paul

Re: [Intel-wired-lan] [PATCH iwl-next v1] iavf: fix proper type for error code in iavf_resume()

[Paul Menzel]

Dear Przemek,


Thank you for your quick reply.

Am 15.09.25 um 11:58 schrieb Przemek Kitszel:
> On 9/15/25 11:12, Paul Menzel wrote:

>> Am 12.09.25 um 10:02 schrieb Aleksandr Loktionov:
>>> The variable 'err' in iavf_resume() is used to store the return value
>>> of different functions, which return an int. Currently, 'err' is
>>> declared as u32, which is semantically incorrect and misleading.
>>>
>>> In the Linux kernel, u32 is typically reserved for fixed-width data
>>> used in hardware interfaces or protocol structures. Using it for a
>>> generic error code may confuse reviewers or developers into thinking
>>> the value is hardware-related or size-constrained.
>>>
>>> Replace u32 with int to reflect the actual usage and improve code
>>> clarity and semantic correctness.
>>
>> Why not use `unsigned int`?
> 
> I don't think we need to provide rationale for "kernel has adopted
> a long standing practice of encoding errors as negative integer codes"
> each time we change a type, IOW it's too basic thing to mention.

Well, if it was unsigned before, than apparently no negative values were 
ever returned.

>>> No functional change.
>>>
>>> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>>> ---
>>>   drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/ 
>>> net/ethernet/intel/iavf/iavf_main.c
>>> index 69054af..c2fbe44 100644
>>> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
>>> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
>>> @@ -5491,7 +5491,7 @@ static int iavf_resume(struct device *dev_d)
>>>   {
>>>       struct pci_dev *pdev = to_pci_dev(dev_d);
>>>       struct iavf_adapter *adapter;
>>> -    u32 err;
>>> +    int err;
>>>       adapter = iavf_pdev_to_adapter(pdev);
>>
>> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> 
> Thank you

Actually looking at the involved functions

     err = iavf_set_interrupt_capability(adapter);
     […]
     err = iavf_request_misc_irq(adapter);

they return (signed) integer, so in my opinion, this is the actual 
motivation for the change, and it’d be great, if the commit message 
could be amended accordingly.


Kind regards,

Paul

Re: [Intel-wired-lan] [PATCH iwl-next v1] iavf: fix proper type for error code in iavf_resume()

[Przemek Kitszel]

On 9/15/25 12:12, Paul Menzel wrote:
> Dear Przemek,
> 
> 
> Thank you for your quick reply.
> 
> Am 15.09.25 um 11:58 schrieb Przemek Kitszel:
>> On 9/15/25 11:12, Paul Menzel wrote:
> 
>>> Am 12.09.25 um 10:02 schrieb Aleksandr Loktionov:
>>>> The variable 'err' in iavf_resume() is used to store the return value
>>>> of different functions, which return an int. Currently, 'err' is
>>>> declared as u32, which is semantically incorrect and misleading.

here we say "why"

>>>>
>>>> In the Linux kernel, u32 is typically reserved for fixed-width data
>>>> used in hardware interfaces or protocol structures. Using it for a
>>>> generic error code may confuse reviewers or developers into thinking
>>>> the value is hardware-related or size-constrained.
>>>>
>>>> Replace u32 with int to reflect the actual usage and improve code
>>>> clarity and semantic correctness.

here (and in the subject) we say what is the change

>>>
>>> Why not use `unsigned int`?
>>
>> I don't think we need to provide rationale for "kernel has adopted
>> a long standing practice of encoding errors as negative integer codes"
>> each time we change a type, IOW it's too basic thing to mention.
> 
> Well, if it was unsigned before, than apparently no negative values were 
> ever returned.
> 
>>>> No functional change.
>>>>
>>>> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>>>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>>>> ---
>>>>   drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/ 
>>>> net/ethernet/intel/iavf/iavf_main.c
>>>> index 69054af..c2fbe44 100644
>>>> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
>>>> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
>>>> @@ -5491,7 +5491,7 @@ static int iavf_resume(struct device *dev_d)

from "int" here compiler was forced to interpret returned 32bit value
as signed

>>>>   {
>>>>       struct pci_dev *pdev = to_pci_dev(dev_d);
>>>>       struct iavf_adapter *adapter;
>>>> -    u32 err;

and here it was just poorly typed storage for 32bit value

>>>> +    int err;
>>>>       adapter = iavf_pdev_to_adapter(pdev);
>>>
>>> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>>
>> Thank you
> 
> Actually looking at the involved functions
> 
>      err = iavf_set_interrupt_capability(adapter);
>      […]
>      err = iavf_request_misc_irq(adapter);
> 
> they return (signed) integer, so in my opinion, this is the actual 
> motivation for the change, and it’d be great, if the commit message 
> could be amended accordingly.

I could argue that current commit message expresses this rather ok.
An actual motivation was "coverity report"... :|

> 
> 
> Kind regards,
> 
> Paul

RE: [Intel-wired-lan] [PATCH iwl-next v1] iavf: fix proper type for error code in iavf_resume()

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Monday, September 15, 2025 11:12 AM
> To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; netdev@vger.kernel.org; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1] iavf: fix proper
> type for error code in iavf_resume()
> 
> Dear Aleksandr,
> 
> 
> Thank you for your patch.
> 
> Am 12.09.25 um 10:02 schrieb Aleksandr Loktionov:
> > The variable 'err' in iavf_resume() is used to store the return
> value
> > of different functions, which return an int. Currently, 'err' is
> > declared as u32, which is semantically incorrect and misleading.
> >
> > In the Linux kernel, u32 is typically reserved for fixed-width data
> > used in hardware interfaces or protocol structures. Using it for a
> > generic error code may confuse reviewers or developers into thinking
> > the value is hardware-related or size-constrained.
> >
> > Replace u32 with int to reflect the actual usage and improve code
> > clarity and semantic correctness.
> 
> Why not use `unsigned int`?
> 
The err variable stores values returned by int typed functions().

With the best regards
Alex

> >
> > No functional change.
> >
> > Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> > ---
> >   drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > index 69054af..c2fbe44 100644
> > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> > @@ -5491,7 +5491,7 @@ static int iavf_resume(struct device *dev_d)
> >   {
> >   	struct pci_dev *pdev = to_pci_dev(dev_d);
> >   	struct iavf_adapter *adapter;
> > -	u32 err;
> > +	int err;
> >
> >   	adapter = iavf_pdev_to_adapter(pdev);
> >
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> 
> 
> Kind regards,
> 
> Paul