> 

-----------------------------
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;
> >

Reply via email to