> ----------------------------- Eaton Industries Manufacturing GmbH ~ Registered place of business: Route de la Longeraie 7, 1110, Morges, Switzerland
----------------------------- -----Original Message----- > From: Heiner Kallweit <hkallwe...@gmail.com> > Sent: Wednesday, April 29, 2020 7:06 PM > To: Badel, Laurent <laurentba...@eaton.com>; fugang.d...@nxp.com; > netdev@vger.kernel.org; and...@lunn.ch; f.faine...@gmail.com; > li...@armlinux.org.uk; richard.leit...@skidata.com; > da...@davemloft.net; alexander.le...@microsoft.com; > gre...@linuxfoundation.org > Cc: Quette, Arnaud <arnaudque...@eaton.com> > Subject: [EXTERNAL] Re: [PATCH 2/2] Reset PHY in phy_init_hw() before > interrupt configuration > > On 29.04.2020 11:03, Badel, Laurent wrote: > > Description: this patch adds a reset of the PHY in phy_init_hw() > > for PHY drivers bearing the PHY_RST_AFTER_CLK_EN flag. > > > > Rationale: due to the PHY reset reverting the interrupt mask to default, > > it is necessary to either perform the reset before PHY configuration, > > or re-configure the PHY after reset. This patch implements the former > > as it is simpler and more generic. > > > > Fixes: 1b0a83ac04e383e3bed21332962b90710fcf2828 ("net: fec: add > phy_reset_after_clk_enable() support") > > Signed-off-by: Laurent Badel <laurentba...@eaton.com> > > > > --- > > drivers/net/phy/phy_device.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > > index 28e3c5c0e..2cc511364 100644 > > --- a/drivers/net/phy/phy_device.c > > +++ b/drivers/net/phy/phy_device.c > > @@ -1082,8 +1082,11 @@ int phy_init_hw(struct phy_device *phydev) > > { > > int ret = 0; > > > > - /* Deassert the reset signal */ > > - phy_device_reset(phydev, 0); > > + /* Deassert the reset signal > > + * If the PHY needs a reset, do it now > > + */ > > + if (!phy_reset_after_clk_enable(phydev)) > > If reset is asserted when entering phy_init_hw(), then > phy_reset_after_clk_enable() basically becomes a no-op. > Still it should work as expected due to the reset signal being > deasserted. It would be worth describing in the comment > why the code still works in this case. > Thank you for the comment, this is a very good point. I will make sure to include some description when resubmitting. I had previously tested this and what I saw was that the first time you bring up the interface, the reset is not asserted so that phy_reset_after_clk_enable() is effective. The subsequent times the interface is brought up, the reset is already asserted when entering phy_init_hw(), so that it becomes a no-op as you correctly pointed out. However, that didn't cause any problem on my board, presumably because in that case the clock is already running when the PHY comes out of reset. I will re-test this carefully against the 'net' tree, though, before coming to conclusions. > > + phy_device_reset(phydev, 0); > > > > if (!phydev->drv) > > return 0; > >