Re: [dpdk-dev] [PATCH 3/8] drivers/net/e1000: Fix missing brackets

["Lu, Wenzhuo" <wenzhuo.lu@intel.com>]

Hi Aaron,


> -----Original Message-----
> From: Aaron Conole [mailto:aconole@redhat.com]
> Sent: Friday, February 26, 2016 2:49 AM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo; Zhang, Helin; Ananyev, Konstantin; Richardson, Bruce
> Subject: [PATCH 3/8] drivers/net/e1000: Fix missing brackets
> 
> The register read/write mphy functions have misleading whitespace around the
> locked check. This cleanup merely preserves the existing functionality while
> improving the ready check.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  drivers/net/e1000/base/e1000_phy.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/e1000/base/e1000_phy.c
> b/drivers/net/e1000/base/e1000_phy.c
> index d43b7ce..8642d38 100644
> --- a/drivers/net/e1000/base/e1000_phy.c
> +++ b/drivers/net/e1000/base/e1000_phy.c
> @@ -4153,13 +4153,13 @@ s32 e1000_read_phy_reg_mphy(struct e1000_hw
> *hw, u32 address, u32 *data)
>  	*data = E1000_READ_REG(hw, E1000_MPHY_DATA);
> 
>  	/* Disable access to mPHY if it was originally disabled */
> -	if (locked)
> +	if (locked) {
>  		ready = e1000_is_mphy_ready(hw);
>  		if (!ready)
>  			return -E1000_ERR_PHY;
> -		E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL,
> -				E1000_MPHY_DIS_ACCESS);
> +	}
> 
> +	E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL,
> E1000_MPHY_DIS_ACCESS);
>  	return E1000_SUCCESS;
>  }
> 
> @@ -4218,13 +4218,13 @@ s32 e1000_write_phy_reg_mphy(struct e1000_hw
> *hw, u32 address, u32 data,
>  	E1000_WRITE_REG(hw, E1000_MPHY_DATA, data);
> 
>  	/* Disable access to mPHY if it was originally disabled */
> -	if (locked)
> +	if (locked) {
>  		ready = e1000_is_mphy_ready(hw);
>  		if (!ready)
>  			return -E1000_ERR_PHY;
> -		E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL,
> -				E1000_MPHY_DIS_ACCESS);
> +	}
> 
> +	E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL,
> E1000_MPHY_DIS_ACCESS);
>  	return E1000_SUCCESS;
>  }
> 
> --
> 2.5.0
Normally we will not maintain the base code. It's just taken from kernel driver.
Agree with you that the whitespace is misleading. But as it's no real impact. I'd like to say not a big deal, better not  change it. :)