On 2.6.2016 07:42, Stefan Roese wrote: > 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 <[email protected]> >>> Cc: Joe Hershberger <[email protected]> >>> Cc: Michal Simek <[email protected]> >>> Cc: Stefan Roese <[email protected]> >>> --- >>> 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.
That was also the part of the reason that I have fixed just one particular phy which I have access to. I expect that some phy can require this reset and some of them not. But definitely this should be properly tested. Thanks, Michal _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
