Re: [Intel-wired-lan] [PATCH iwl-next 2/8] ixgbe: use int instead of u32 for error code variables

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 5/8/26 05:12, Aleksandr Loktionov wrote:
> The variables used to store return values of kernel and driver functions
> throughout the ixgbe driver are declared as u32 in several places.  Such
> functions return negative errno values on error (e.g. -EIO, -EFAULT),
> which are sign-extended negative integers.  Storing them in an unsigned
> u32 silently wraps the value: -EIO (0xFFFFFFF7) stored in u32 becomes a
> large positive number, so any "if (status)" truthiness check still works
> by accident, but comparisons against specific negative error codes or
> propagation up the call stack produce wrong results.
> 
> In the Linux kernel, u32 is reserved for fixed-width quantities used in
> hardware interfaces or protocol structures.  Using it for generic error
> codes misleads reviewers into thinking the value is hardware-constrained.
> 
> Change all such local variables from u32 to int driver-wide: one in
> ixgbe_main.c (ixgbe_resume), three in ixgbe_phy.c
> (ixgbe_identify_phy_generic, ixgbe_tn_check_overtemp,
> ixgbe_set_copper_phy_power), and six in ixgbe_x550.c
> (ixgbe_check_link_t_X550em, ixgbe_get_lasi_ext_t_x550em,
> ixgbe_enable_lasi_ext_t_x550em, ixgbe_handle_lasi_ext_t_x550em,
> ixgbe_ext_phy_t_x550em_get_link, ixgbe_setup_internal_phy_t_x550em).
> 
> No functional change.

unless there is a check "status < 0" somewhere
anyway, that's why the preferred style is to write "!status" or "status"

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

> 
> Reviewed-by: Simon Horman <horms@kernel.org>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  2 +-
>   drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c  |  6 +++---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 12 ++++++------
>   3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 65426a1..b6308c1 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -7528,7 +7528,7 @@ static int ixgbe_resume(struct device *dev_d)
>   	struct pci_dev *pdev = to_pci_dev(dev_d);
>   	struct ixgbe_adapter *adapter = pci_get_drvdata(pdev);
>   	struct net_device *netdev = adapter->netdev;
> -	u32 err;
> +	int err;
>   
>   	adapter->hw.hw_addr = adapter->io_addr;
>   
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
> index ab733e7..de8f6c6 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c
> @@ -262,7 +262,7 @@ static bool ixgbe_probe_phy(struct ixgbe_hw *hw, u16 phy_addr)
>    **/
>   int ixgbe_identify_phy_generic(struct ixgbe_hw *hw)
>   {
> -	u32 status = -EFAULT;
> +	int status = -EFAULT;
>   	u32 phy_addr;
>   
>   	if (!hw->phy.phy_semaphore_mask) {
> @@ -2811,7 +2811,7 @@ static void ixgbe_i2c_bus_clear(struct ixgbe_hw *hw)
>   bool ixgbe_tn_check_overtemp(struct ixgbe_hw *hw)
>   {
>   	u16 phy_data = 0;
> -	u32 status;
> +	int status;
>   
>   	if (hw->device_id != IXGBE_DEV_ID_82599_T3_LOM)
>   		return false;
> @@ -2831,7 +2831,7 @@ bool ixgbe_tn_check_overtemp(struct ixgbe_hw *hw)
>    **/
>   int ixgbe_set_copper_phy_power(struct ixgbe_hw *hw, bool on)
>   {
> -	u32 status;
> +	int status;
>   	u16 reg;
>   
>   	/* Bail if we don't have copper phy */
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
> index 76d2fa3..9b14f3b 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c
> @@ -1911,7 +1911,7 @@ static int ixgbe_check_link_t_X550em(struct ixgbe_hw *hw,
>   				     bool *link_up,
>   				     bool link_up_wait_to_complete)
>   {
> -	u32 status;
> +	int status;
>   	u16 i, autoneg_status;
>   
>   	if (hw->mac.ops.get_media_type(hw) != ixgbe_media_type_copper)
> @@ -2330,7 +2330,7 @@ static int ixgbe_get_link_capabilities_X550em(struct ixgbe_hw *hw,
>   static int ixgbe_get_lasi_ext_t_x550em(struct ixgbe_hw *hw, bool *lsc,
>   				       bool *is_overtemp)
>   {
> -	u32 status;
> +	int status;
>   	u16 reg;
>   
>   	*is_overtemp = false;
> @@ -2418,7 +2418,7 @@ static int ixgbe_get_lasi_ext_t_x550em(struct ixgbe_hw *hw, bool *lsc,
>   static int ixgbe_enable_lasi_ext_t_x550em(struct ixgbe_hw *hw)
>   {
>   	bool lsc, overtemp;
> -	u32 status;
> +	int status;
>   	u16 reg;
>   
>   	/* Clear interrupt flags */
> @@ -2512,7 +2512,7 @@ static int ixgbe_handle_lasi_ext_t_x550em(struct ixgbe_hw *hw,
>   {
>   	struct ixgbe_phy_info *phy = &hw->phy;
>   	bool lsc;
> -	u32 status;
> +	int status;
>   
>   	status = ixgbe_get_lasi_ext_t_x550em(hw, &lsc, is_overtemp);
>   	if (status)
> @@ -2606,7 +2606,7 @@ static int ixgbe_setup_kr_x550em(struct ixgbe_hw *hw)
>    **/
>   static int ixgbe_ext_phy_t_x550em_get_link(struct ixgbe_hw *hw, bool *link_up)
>   {
> -	u32 ret;
> +	int ret;
>   	u16 autoneg_status;
>   
>   	*link_up = false;
> @@ -2642,7 +2642,7 @@ static int ixgbe_setup_internal_phy_t_x550em(struct ixgbe_hw *hw)
>   {
>   	ixgbe_link_speed force_speed;
>   	bool link_up;
> -	u32 status;
> +	int status;
>   	u16 speed;
>   
>   	if (hw->mac.ops.get_media_type(hw) != ixgbe_media_type_copper)