[Intel-wired-lan] [PATH iwl-next v1 1/1] e1000e: Minor flow correction in e1000_shutdown function

Paul Menzel pmenzel at molgen.mpg.de
Wed Jan 3 15:27:48 UTC 2024


Dear Vitaly,


Thank you for the patch.

In the commit message summary, it’d be great if you used a statement by 
adding a verb in imperative mood. Maybe:

Correct flow in e1000_shutdown()

Am 03.01.24 um 09:50 schrieb Vitaly Lifshits:
> Added a missing curly braces to avoid entering to an if statement

s/Added/Add/
s/entering to/entering/

The curly braces are not missing though.

> where it is not always required in e1000_shutdown function.
> This improves code readability and might prevent a non-deterministic
> behaviour in the future.

Looking at the diff, it’s not clear to me, how `retval` is used/set 
before. Could you please elaborate, what the problem is?

Missing Fixes tag?

> 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 af5d9d97a0d6..cc8c531ec3df 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -6688,14 +6688,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


Kind regards,

Paul


More information about the Intel-wired-lan mailing list