Patchwork phy: Replace genphy_update_link() call with phy_read_status()

login
register
mail settings
Submitter Alexander Sverdlin
Date Sept. 13, 2012, 7:20 a.m.
Message ID <5051894B.5060001@sysgo.com>
Download mbox | patch
Permalink /patch/183519/
State Rejected
Delegated to: David Miller
Headers show

Comments

Alexander Sverdlin - Sept. 13, 2012, 7:20 a.m.
From: Alexander Sverdlin <alexander.sverdlin@sysgo.com>

Replace genphy_update_link() call with phy_read_status() 

Code in phy.c should not call genphy_*() functions directly, this breaks PHY layer abstraction.
Some drivers may re-implement "read_status" callback and it's not being called in one place of
PHY state machine, where genphy_update_link() is called instead. So fix it.
For drivers that rely on genphy_* implementation nothing changed, as genphy_read_status() calls
genphy_update_link() anyway.

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@sysgo.com>
---
 drivers/net/phy/phy.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Sept. 17, 2012, 5:22 p.m.
From: Alexander Sverdlin <asv@sysgo.com>
Date: Thu, 13 Sep 2012 09:20:43 +0200

> From: Alexander Sverdlin <alexander.sverdlin@sysgo.com>
> 
> Replace genphy_update_link() call with phy_read_status() 
> 
> Code in phy.c should not call genphy_*() functions directly, this breaks PHY layer abstraction.
> Some drivers may re-implement "read_status" callback and it's not being called in one place of
> PHY state machine, where genphy_update_link() is called instead. So fix it.
> For drivers that rely on genphy_* implementation nothing changed, as genphy_read_status() calls
> genphy_update_link() anyway.
> 
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@sysgo.com>

This is a behavioral change, not just a change to make sure the right
interfaces are used.

genphy_update_link() does only a small subset of the operations
performed by the generic read_status callback.

I'm not applying this patch, because all of those extra operations
might be unexpected in some situations and break some configurations.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index dfca51d..cfed41c 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -839,7 +839,7 @@  void phy_state_machine(struct work_struct *work)
 			}
 			break;
 		case PHY_FORCING:
-			err = genphy_update_link(phydev);
+			err = phy_read_status(phydev);
 
 			if (err)
 				break;