From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [PATCH] net: ethernet: fsl: don't en/disable refclk on open/close Date: Sun, 22 Oct 2017 11:31:55 -0700 Message-ID: <6656d8d0-874a-574e-9ea9-37f8238bd155@gmail.com> References: <20171022131130.8580-1-dev@g0hl1n.net> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Richard Leitner , stable@vger.kernel.org To: Richard Leitner , fugang.duan@nxp.com Return-path: In-Reply-To: <20171022131130.8580-1-dev@g0hl1n.net> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 10/22/2017 06:11 AM, Richard Leitner wrote: > From: Richard Leitner > > From: Richard Leitner > > Some PHYs (for example the LAN8710) doesn't allow turning the ethernet > ref clocks off and on again without reset (according to their datasheet). > Exactly this behaviour was introduced for power saving reasons by > commit e8fcfcd5684a ("net: fec: optimize the clock management to save...") Please don't truncate the commit subject, put it in full length. > Therefore remove those en/disables of the refclk during probe, open and > close of the fec. > > Generally speaking is this issue only be relevant if the ref clk for > the PHY is generated by the SoC. In our specific case (PCB) this problem > does occur at about every 10th to 50th POR of an LAN8710 connected to an > i.MX6 SoC. The typical symptom of this problem is a "swinging" ethernet > link. Similar issues were experienced by users of the NXP forum: > https://community.nxp.com/thread/389902 > https://community.nxp.com/message/309354 > With this patch applied the issue didn't occur for at least a few > thousand PORs of our board. FWIW, this is a common problem on most Ethernet systems attached to a PHY because of the clock dependencies between the MAC and PHY, in either direction really. We recently stumbled across a similar problem on the MAC side (using bcmgenet) where after shutting down the PHY in ndo_stop() we would not get enough clock cycles for the MAC to cleanly shutdown and upon subsequent ndo_open() we could get the MAC in an invalid state. > > Fixes: e8fcfcd5684a ("net: fec: optimize the clock management to sa...") Likewise here, not truncation of the commit subject necessary. > Cc: stable@vger.kernel.org netdev patches are handled directly by David Miller, see netdev-FAQ for details. > Signed-off-by: Richard Leitner > --- > drivers/net/ethernet/freescale/fec_main.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c > index 3dc2d771a222..8f696b53d8b8 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -2844,9 +2844,6 @@ fec_enet_open(struct net_device *ndev) > return ret; > > pinctrl_pm_select_default_state(&fep->pdev->dev); > - ret = fec_enet_clk_enable(ndev, true); > - if (ret) > - goto clk_enable; > > /* I should reset the ring buffers here, but I don't yet know > * a simple way to do that. > @@ -2879,8 +2876,6 @@ fec_enet_open(struct net_device *ndev) > err_enet_mii_probe: > fec_enet_free_buffers(ndev); > err_enet_alloc: > - fec_enet_clk_enable(ndev, false); > -clk_enable: > pm_runtime_mark_last_busy(&fep->pdev->dev); > pm_runtime_put_autosuspend(&fep->pdev->dev); > pinctrl_pm_select_sleep_state(&fep->pdev->dev); > @@ -2907,7 +2902,6 @@ fec_enet_close(struct net_device *ndev) > > fec_enet_update_ethtool_stats(ndev); > > - fec_enet_clk_enable(ndev, false); > pinctrl_pm_select_sleep_state(&fep->pdev->dev); > pm_runtime_mark_last_busy(&fep->pdev->dev); > pm_runtime_put_autosuspend(&fep->pdev->dev); > @@ -3495,7 +3489,6 @@ fec_probe(struct platform_device *pdev) > > /* Carrier starts down, phylib will bring it up */ > netif_carrier_off(ndev); > - fec_enet_clk_enable(ndev, false); > pinctrl_pm_select_sleep_state(&pdev->dev); > > ret = register_netdev(ndev); > -- Florian