Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754146AbdLFIXZ (ORCPT ); Wed, 6 Dec 2017 03:23:25 -0500 Received: from mail1.skidata.com ([91.230.2.99]:34333 "EHLO mail1.skidata.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750989AbdLFIXW (ORCPT ); Wed, 6 Dec 2017 03:23:22 -0500 X-Greylist: delayed 656 seconds by postgrey-1.27 at vger.kernel.org; Wed, 06 Dec 2017 03:23:21 EST X-IronPort-AV: E=Sophos;i="5.45,367,1508796000"; d="scan'208";a="7483669" Subject: Re: [PATCH net-next v3 4/4] net: fec: add phy_reset_after_clk_enable() support To: Andy Duan , Richard Leitner , "robh+dt@kernel.org" , "mark.rutland@arm.com" , "andrew@lunn.ch" , "f.fainelli@gmail.com" , "frowand.list@gmail.com" CC: "davem@davemloft.net" , "geert+renesas@glider.be" , "sergei.shtylyov@cogentembedded.com" , "baruch@tkos.co.il" , "david.wu@rock-chips.com" , "lukma@denx.de" , "netdev@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" References: <20171205132600.13796-1-dev@g0hl1n.net> <20171205132600.13796-5-dev@g0hl1n.net> From: Richard Leitner Message-ID: <1e7b3e7f-251d-4101-1441-702e891f2ca9@skidata.com> Date: Wed, 6 Dec 2017 09:12:14 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [172.16.60.30] X-ClientProxiedBy: sdex1srv.skidata.net (172.16.10.92) To sdex1srv.skidata.net (172.16.10.92) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1909 Lines: 50 Hi Andy, On 12/06/2017 02:50 AM, Andy Duan wrote: > From: Richard Leitner 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. ... >> 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. The problem here is that in fec_enet_open() the fec_enet_clk_enable() call is done before the phy is probed. Therefore (as mentioned in the comment) there's no link from the net_device (ndev) to the phy_driver (which holds the flags). Any suggestions for a better solution are highly appreciated here! Thanks! regards;Richard.L