On Thu, 30 Apr 2020, Badel, Laurent wrote:
-----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.
I have two additional things to take into account:
* phy_reset_after_clk_enable() shoulnd't be longer called that way since
it is now misleading -> the phy is no longer reset after clock enable but
during hw_init()
* how about fec_resume()? I don't think hw_init() is called then and so
phy_reset_after_clk_enable() will no longer be called.
+ phy_device_reset(phydev, 0);
if (!phydev->drv)
return 0;
