[Intel-wired-lan] [PATCH iwl-net v1 1/1] e1000e: fix non-deterministic behavior in e1000_shutdown

[Tony Nguyen]

On 1/2/2024 6:14 AM, Vitaly Lifshits wrote:
> A non-deterministic behavior was found in e1000_shutdown function.
> In it, the variable retval is being evaluated according to the
> configurations in the hardware. Sometimes, it might be validated
> for a previous assignment, where it is wrong.

retval looks to be initialized to 0 [1] and its only use, prior to this, 
is checked for error [2], so this value will only be 0 or the result of 
e1000_enable_ulp_lpt_lp(). The code looks to work correctly, however, 
this change does make it more readable. I'm ok with this going via 
iwl-next, with a different commit message, as I think it reads cleaner, 
but I don't believe there's a bug here.

Thanks,
Tony

> Therefore curly braces were added at that part of the code.
> 
> Fixes: 74f350ee08e2 ("e1000e: Feature Enable PHY Ultra Low Power Mode (ULP)")
> Signed-off-by: Vitaly Lifshits <vitaly.lifshits at intel.com>
> ---
>   drivers/net/ethernet/intel/e1000e/netdev.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index f536c856727c..e51d79d8a7d7 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -6691,14 +6691,14 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool runtime)
>   	if (adapter->hw.phy.type == e1000_phy_igp_3) {
>   		e1000e_igp3_phy_powerdown_workaround_ich8lan(&adapter->hw);
>   	} else if (hw->mac.type >= e1000_pch_lpt) {
> -		if (wufc && !(wufc & (E1000_WUFC_EX | E1000_WUFC_MC | E1000_WUFC_BC)))
> +		if (wufc && !(wufc & (E1000_WUFC_EX | E1000_WUFC_MC | E1000_WUFC_BC))) {
>   			/* ULP does not support wake from unicast, multicast
>   			 * or broadcast.
>   			 */
>   			retval = e1000_enable_ulp_lpt_lp(hw, !runtime);
> -
> -		if (retval)
> -			return retval;
> +			if (retval)
> +				return retval;
> +		}
>   	}
>   
>   	/* Ensure that the appropriate bits are set in LPI_CTRL

[1] 
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/intel/e1000e/netdev.c#L6628
[2] 
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/intel/e1000e/netdev.c#L6677