From: Andy Duan <fugang.duan@nxp.com>
To: Richard Leitner <dev@g0hl1n.net>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "andrew@lunn.ch" <andrew@lunn.ch>,
        "f.fainelli@gmail.com" <f.fainelli@gmail.com>,
        "frowand.list@gmail.com" <frowand.list@gmail.com>
CC: "davem@davemloft.net" <davem@davemloft.net>,
        "geert+renesas@glider.be" <geert+renesas@glider.be>,
        "sergei.shtylyov@cogentembedded.com" 
        <sergei.shtylyov@cogentembedded.com>,
        "baruch@tkos.co.il" <baruch@tkos.co.il>,
        "david.wu@rock-chips.com" <david.wu@rock-chips.com>,
        "lukma@denx.de" <lukma@denx.de>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "richard.leitner@skidata.com" <richard.leitner@skidata.com>
Subject: RE: [PATCH net-next v3 4/4] net: fec: add
 phy_reset_after_clk_enable() support
Thread-Topic: [PATCH net-next v3 4/4] net: fec: add
 phy_reset_after_clk_enable() support
Thread-Index: AQHTbcywCErLmvbit0yBnahadCjONaM1im7w
Date: Wed, 6 Dec 2017 01:50:14 +0000
Message-ID: <DB6PR0401MB2264B5060F745CEB5405FFBCFF320@DB6PR0401MB2264.eurprd04.prod.outlook.com>
References: <20171205132600.13796-1-dev@g0hl1n.net>
 <20171205132600.13796-5-dev@g0hl1n.net>
In-Reply-To: <20171205132600.13796-5-dev@g0hl1n.net>
Accept-Language: zh-CN, en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: bb5e2daa-047c-44e8-7b78-08d53c4bb1cc
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Dec 2017 01:50:14.4881
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0401MB2261
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id vB61oRhu002192
Content-Length: 2939
Lines: 69

From: Richard Leitner <dev@g0hl1n.net> Sent: Tuesday, December 05, 2017 9:26 PM
>Some PHYs (for example the SMSC LAN8710/LAN8720) doesn't allow turning
>the refclk on and off again during operation (according to their datasheet).
>Nonetheless exactly this behaviour was introduced for power saving reasons
>by commit e8fcfcd5684a ("net: fec: optimize the clock management to save
>power").
>Therefore add support for the phy_reset_after_clk_enable function from
>phylib to mitigate this issue.
>
>Generally speaking this issue is only relevant if the ref clk for the PHY is
>generated by the SoC and therefore the PHY is configured to "REF_CLK In
>Mode". In our specific case (PCB) this problem does occur at about every 10th
>to 50th POR of an LAN8710 connected to an i.MX6SOLO SoC. The typical
>symptom of this problem is a "swinging" ethernet link.
>Similar issues were reported by users of the NXP forum:
>	https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F
>%2Fcommunity.nxp.com%2Fthread%2F389902&data=02%7C01%7Cfugang.du
>an%40nxp.com%7C7f9fee272fc44662c2a108d53be3d1ee%7C686ea1d3bc2b4c6
>fa92cd99c5c301635%7C0%7C0%7C636480772022331090&sdata=7RdUsoWVWu
>o1nM5zKwLt7%2F6U3dxgDJtBDGlQCUWC6IM%3D&reserved=0
>	https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F
>%2Fcommunity.nxp.com%2Fmessage%2F309354&data=02%7C01%7Cfugang.d
>uan%40nxp.com%7C7f9fee272fc44662c2a108d53be3d1ee%7C686ea1d3bc2b4
>c6fa92cd99c5c301635%7C0%7C0%7C636480772022331090&sdata=D56KilGWD3
>kLABxc0yOI%2B44Y%2FhLfrGtdAvupCEyvI%2BI%3D&reserved=0
>With this patch applied the issue didn't occur for at least a few hundret PORs
>of our board.
>
>Fixes: e8fcfcd5684a ("net: fec: optimize the clock management to save
>power")
>Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
>---
> drivers/net/ethernet/freescale/fec_main.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/drivers/net/ethernet/freescale/fec_main.c
>b/drivers/net/ethernet/freescale/fec_main.c
>index 610573855213..8c3d0fb7db20 100644
>--- a/drivers/net/ethernet/freescale/fec_main.c
>+++ b/drivers/net/ethernet/freescale/fec_main.c
>@@ -1862,6 +1862,8 @@ static int fec_enet_clk_enable(struct net_device
>*ndev, bool enable)
> 		ret = clk_prepare_enable(fep->clk_ref);
> 		if (ret)
> 			goto failed_clk_ref;
>+
>+		phy_reset_after_clk_enable(ndev->phydev);
> 	} else {
> 		clk_disable_unprepare(fep->clk_ahb);
> 		clk_disable_unprepare(fep->clk_enet_out);
>@@ -2860,6 +2862,11 @@ fec_enet_open(struct net_device *ndev)
> 	if (ret)
> 		goto err_enet_mii_probe;
>
>+	/* reset phy if needed here, due to the fact this is the first time we
>+	 * have the net_device to phy_driver link
>+	 */
>+	phy_reset_after_clk_enable(ndev->phydev);
>+

The patch series look better.
But why does it need to reset phy here since phy already is hard reset after clock enable.


> 	if (fep->quirks & FEC_QUIRK_ERR006687)
> 		imx6q_cpuidle_fec_irqs_used();
>
>--
>2.11.0