diff mbox

[dpdk-dev,v2,2/7] drivers/net/e1000: Fix missing brackets

Message ID 1458682638-28378-3-git-send-email-aconole@redhat.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Aaron Conole March 22, 2016, 9:37 p.m. UTC 
  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.

Fixes commit 38db3f7f50bd ("e1000: update base driver")

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
v2:
* Changed from "whitespace-only" fix to a functional change
  moving the phy writes into protection of the `if (locked)`
  code
* Added "Fixes" line.

 drivers/net/e1000/base/e1000_phy.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Thomas Monjalon March 23, 2016, 10:38 a.m. UTC | #1 
2016-03-22 17:37, Aaron Conole:
> 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.
> 
> Fixes commit 38db3f7f50bd ("e1000: update base driver")
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
> v2:
> * Changed from "whitespace-only" fix to a functional change
>   moving the phy writes into protection of the `if (locked)`
>   code
> * Added "Fixes" line.

Wenzhuo, do you agree to fix the base driver here?
Thomas Monjalon March 23, 2016, 10:41 a.m. UTC | #2 
fixing Wenzhuo email address...

2016-03-23 11:38, Thomas Monjalon:
> 2016-03-22 17:37, Aaron Conole:
> > 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.
> > 
> > Fixes commit 38db3f7f50bd ("e1000: update base driver")
> > 
> > Signed-off-by: Aaron Conole <aconole@redhat.com>
> > ---
> > v2:
> > * Changed from "whitespace-only" fix to a functional change
> >   moving the phy writes into protection of the `if (locked)`
> >   code
> > * Added "Fixes" line.
> 
> Wenzhuo, do you agree to fix the base driver here?
Wenzhuo Lu March 24, 2016, 12:35 a.m. UTC | #3 
Hi Thomas, Aaron,
> > Wenzhuo, do you agree to fix the base driver here?
Honestly I find the logic has some problem and maybe more changes needed. I'm talking with our kernel driver contactors to make sure they have no concern about  my idea.
And Aaron, do we really need to fix this code? For I find this function is not called. So it has no real impact to us. Could we just wait until kernel driver fixes it and leverage the new version? Thanks.
diff mbox

Patch

diff --git a/drivers/net/e1000/base/e1000_phy.c b/drivers/net/e1000/base/e1000_phy.c
index d43b7ce..ad3fd58 100644
--- a/drivers/net/e1000/base/e1000_phy.c
+++ b/drivers/net/e1000/base/e1000_phy.c
@@ -4153,12 +4153,12 @@  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,12 +4218,12 @@  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;
 }