Hi Michal,
On 02.06.2016 07:31, Michal Simek wrote:
On 1.6.2016 18:22, Nathan Rossi wrote:
Commit a058052c "net: phy: do not read configuration register on reset", changes the behaviour of the phy_reset function such that the state of the BMCR register is not preserved during reset.
Reorder the phy_reset and genphy_config_aneg calls for some of the marvell phy drivers so that auto-negotiation occurs after reset.
Signed-off-by: Nathan Rossi nathan@nathanrossi.com Cc: Joe Hershberger joe.hershberger@ni.com Cc: Michal Simek michal.simek@xilinx.com Cc: Stefan Roese sr@denx.de
drivers/net/phy/marvell.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index d2e68d4..40284a5 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -414,10 +414,10 @@ static int m88e1145_config(struct phy_device *phydev) MIIM_M88E1145_RGMII_TX_DELAY; phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1145_PHY_EXT_CR, reg);
- genphy_config_aneg(phydev);
- phy_reset(phydev);
- genphy_config_aneg(phydev);
As you see from my patch 1b008fdb06848c7c84e7c1a4a9b2b76239550555 you should return value from genphy_config_aneg() which should be fixed everywhere.
Also as you see above you do some writes to the phy and the question is if you should run phy_reset here. Based on my patch above and investigating I found that phy_reset is called before this function is called and not sure if phy should be reset twice.
Some changes to the PHY registers need a soft-reset to occur before these changes to be effective. Not sure if this is the case here though.
But I share your thoughts about this phy_reset() being called now in some of the xxx_config() functions and not in others. Your patch mentions that phy_reset() is already called in phy_connect_dev(), which not all ethernet drivers do right now AFAICT.
We should perhaps take a look at the Linux Marvell PHY driver to check how it is done there.
Thanks, Stefan