From: Duan Andy <fugang.duan@freescale.com>
To: Lothar Wa?mann <LW@KARO-electronics.de>, Andrew Lunn <andrew@lunn.ch>,
        "David S. Miller" <davem@davemloft.net>,
        Fabio Estevam <Fabio.Estevam@freescale.com>,
        Kevin Hao <haokexin@gmail.com>, Lucas Stach <l.stach@pengutronix.de>,
        Philippe Reynes <tremyfr@gmail.com>,
        Russell King <rmk+kernel@arm.linux.org.uk>,
        Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        Stefan Agner <stefan@agner.ch>
Subject: RE: [PATCH] net: fec: fix enet_out clock handling
Thread-Topic: [PATCH] net: fec: fix enet_out clock handling
Thread-Index: AQHRKRk2yV8ik/j4gEuk1Xs5hLkIYJ6xaVug
Date: Sat, 28 Nov 2015 13:13:16 +0000
Message-ID: <BN3PR0301MB1219C4D8E1B09F1AE27E18FAF5020@BN3PR0301MB1219.namprd03.prod.outlook.com>
References: <1448631550-943-1-git-send-email-LW@KARO-electronics.de>
In-Reply-To: <1448631550-943-1-git-send-email-LW@KARO-electronics.de>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Nov 2015 13:13:16.5017
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 710a03f5-10f6-4d38-9ff4-a80b81da590d
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR03MB1437
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 base64 to 8bit by mail.home.local id tASDE9M7011870
Content-Length: 4318
Lines: 123

From: Lothar Waßmann <LW@KARO-electronics.de> Sent: Friday, November 27, 2015 9:39 PM
> To: Andrew Lunn; David S. Miller; Estevam Fabio-R49496; Kevin Hao; Lothar
> Waßmann; Lucas Stach; Duan Fugang-B38611; Philippe Reynes; Russell King;
> Uwe Kleine-König; linux-kernel@vger.kernel.org; netdev@vger.kernel.org;
> Stefan Agner
> Subject: [PATCH] net: fec: fix enet_out clock handling
> 
> When ENET_OUT is being used as reference clock for an external PHY, the
> clock must not be disabled while the PHY is active. Otherwise the PHY may
> lose its internal state and require a reset to become functional again.
> 
> A symptom for this bug is a network interface that constantly toggles
> between UP and DOWN state:
> fec 800f0000.ethernet eth0: Link is Up - 100Mbps/Full - flow control
> rx/tx fec 800f0000.ethernet eth0: Link is Down fec 800f0000.ethernet eth0:
> Link is Up - 100Mbps/Full - flow control rx/tx fec 800f0000.ethernet eth0:
> Link is Down [...]
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 34 +++++++++++++------------
> ------
>  1 file changed, 14 insertions(+), 20 deletions(-)
> 

When MAC is not ready with clocks disabled,  it is not necessary to supply clock for PHY. In fact, PHY also is not ready, why does it need clock ?
For your problem, you must add PHY reset in your dts file to resolve your problem.

I don't agree with this patch.

> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index d2328fc..d9df4c5 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1857,11 +1857,6 @@ static int fec_enet_clk_enable(struct net_device
> *ndev, bool enable)
>  		ret = clk_prepare_enable(fep->clk_ahb);
>  		if (ret)
>  			return ret;
> -		if (fep->clk_enet_out) {
> -			ret = clk_prepare_enable(fep->clk_enet_out);
> -			if (ret)
> -				goto failed_clk_enet_out;
> -		}
>  		if (fep->clk_ptp) {
>  			mutex_lock(&fep->ptp_clk_mutex);
>  			ret = clk_prepare_enable(fep->clk_ptp); @@ -1873,35
> +1868,26 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool
> enable)
>  			}
>  			mutex_unlock(&fep->ptp_clk_mutex);
>  		}
> -		if (fep->clk_ref) {
> -			ret = clk_prepare_enable(fep->clk_ref);
> -			if (ret)
> -				goto failed_clk_ref;
> -		}
> +		ret = clk_prepare_enable(fep->clk_ref);
> +		if (ret)
> +			goto failed_clk_ref;
>  	} else {
>  		clk_disable_unprepare(fep->clk_ahb);
> -		if (fep->clk_enet_out)
> -			clk_disable_unprepare(fep->clk_enet_out);
>  		if (fep->clk_ptp) {
>  			mutex_lock(&fep->ptp_clk_mutex);
>  			clk_disable_unprepare(fep->clk_ptp);
>  			fep->ptp_clk_on = false;
>  			mutex_unlock(&fep->ptp_clk_mutex);
>  		}
> -		if (fep->clk_ref)
> -			clk_disable_unprepare(fep->clk_ref);
> +		clk_disable_unprepare(fep->clk_ref);
>  	}
> 
>  	return 0;
> 
>  failed_clk_ref:
> -	if (fep->clk_ref)
> -		clk_disable_unprepare(fep->clk_ref);
> +	clk_disable_unprepare(fep->clk_ref);
>  failed_clk_ptp:
> -	if (fep->clk_enet_out)
> -		clk_disable_unprepare(fep->clk_enet_out);
> -failed_clk_enet_out:
> -		clk_disable_unprepare(fep->clk_ahb);
> +	clk_disable_unprepare(fep->clk_ahb);
> 
>  	return ret;
>  }
> @@ -3425,6 +3411,10 @@ fec_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto failed_clk;
> 
> +	ret = clk_prepare_enable(fep->clk_enet_out);
> +	if (ret)
> +		goto failed_clk_enet_out;
> +
>  	ret = clk_prepare_enable(fep->clk_ipg);
>  	if (ret)
>  		goto failed_clk_ipg;
> @@ -3509,6 +3499,8 @@ failed_init:
>  	if (fep->reg_phy)
>  		regulator_disable(fep->reg_phy);
>  failed_regulator:
> +	clk_disable_unprepare(fep->clk_enet_out);
> +failed_clk_enet_out:
>  	clk_disable_unprepare(fep->clk_ipg);
>  failed_clk_ipg:
>  	fec_enet_clk_enable(ndev, false);
> @@ -3531,6 +3523,8 @@ fec_drv_remove(struct platform_device *pdev)
>  	fec_ptp_stop(pdev);
>  	unregister_netdev(ndev);
>  	fec_enet_mii_remove(fep);
> +	fec_enet_clk_enable(ndev, false);
> +	clk_disable_unprepare(fep->clk_enet_out);
>  	if (fep->reg_phy)
>  		regulator_disable(fep->reg_phy);
>  	of_node_put(fep->phy_node);
> --
> 2.1.4
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?