[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BN3PR0301MB1219C4D8E1B09F1AE27E18FAF5020@BN3PR0301MB1219.namprd03.prod.outlook.com>
Date: Sat, 28 Nov 2015 13:13:16 +0000
From: Duan Andy <fugang.duan@...escale.com>
To: Lothar Wa?mann <LW@...O-electronics.de>,
Andrew Lunn <andrew@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Fabio Estevam <Fabio.Estevam@...escale.com>,
Kevin Hao <haokexin@...il.com>,
Lucas Stach <l.stach@...gutronix.de>,
Philippe Reynes <tremyfr@...il.com>,
Russell King <rmk+kernel@....linux.org.uk>,
Uwe Kleine-K?nig <u.kleine-koenig@...gutronix.de>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Stefan Agner <stefan@...er.ch>
Subject: RE: [PATCH] net: fec: fix enet_out clock handling
From: Lothar Waßmann <LW@...O-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@...r.kernel.org; netdev@...r.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@...O-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
Powered by blists - more mailing lists