From: Richard Leitner <d...@g0hl1n.net> Sent: Monday, November 20, 2017 4:34 PM
>To: f.faine...@gmail.com; Andy Duan <fugang.d...@nxp.com>;
>and...@lunn.ch
>Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
>richard.leit...@skidata.com
>Subject: [PATCH v2 3/3] net: ethernet: fec: fix refclk enable for SMSC
>LAN8710/20
>
>From: Richard Leitner <richard.leit...@skidata.com>
>
>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 after enabling the refclk we detect if an affected PHY is attached.
>If
>so reset and initialize it again.
>
>For a better understanding here's a outline of the time response of the clock
>and reset lines before and after this patch:
>
> v--fec_probe() v--fec_enet_open()
> v v
> w/o patch eCLK:
>___||||||||____________________|||||||||||||||||
> w/o patch nRST: ----__------------------------------------------
> w/o patch CONF:
>_______XX_______________________________________
>
> w/ patch eCLK:
>___||||||||____________________|||||||||||||||||
> w/ patch nRST: ----__-----------------------------__-----------
> w/ patch CONF:
>_______XX_____________________________XX________
> ^ ^
> ^--fec_probe() ^--fec_enet_open()
>
>Generally speaking this issue is only 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.MX6DL 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%7C507c7aafbd4744f81ebf08d52ff1acff%7C686ea1d3bc2b4c6f
>a92cd99c5c301635%7C0%7C0%7C636467637399145483&sdata=RNXVGpPrlrcyL
>0SoQl8%2BI0k8Oc8BM0Iwykd1O%2Bjmvcc%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%7C507c7aafbd4744f81ebf08d52ff1acff%7C686ea1d3bc2b4c6
>fa92cd99c5c301635%7C0%7C0%7C636467637399145483&sdata=pjeJEZGuBpb9
>uCMKGr70qa%2FmsNoak6v3nCID2vbNAeg%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.leit...@skidata.com>
>---
> drivers/net/ethernet/freescale/fec_main.c | 37
>+++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
>diff --git a/drivers/net/ethernet/freescale/fec_main.c
>b/drivers/net/ethernet/freescale/fec_main.c
>index 06a7caca0cee..52ec9b29a70e 100644
>--- a/drivers/net/ethernet/freescale/fec_main.c
>+++ b/drivers/net/ethernet/freescale/fec_main.c
>@@ -68,6 +68,7 @@
>
> static void set_multicast_list(struct net_device *ndev); static void
>fec_enet_itr_coal_init(struct net_device *ndev);
>+static int fec_reset_phy(struct net_device *ndev);
>
> #define DRIVER_NAME "fec"
>
>@@ -1833,6 +1834,32 @@ static int fec_enet_mdio_write(struct mii_bus *bus,
>int mii_id, int regnum,
> return ret;
> }
>
>+static int fec_enet_clk_ref_enable_reset_phy_quirk(struct net_device
>+*ndev) {
>+ struct phy_device *phy_dev = ndev->phydev;
>+ u32 real_phy_id;
>+ int ret;
>+
>+ /* some PHYs need a reset after the refclk was enabled, so we
>+ * reset them here
>+ */
>+ if (!phy_dev)
>+ return 0;
>+ if (!phy_dev->drv)
>+ return 0;
>+ real_phy_id = phy_dev->drv->phy_id & phy_dev->drv->phy_id_mask;
>+ switch (real_phy_id) {
>+ case 0x0007c0f0: /* SMSC LAN8710/LAN8720 */
Don't hard code here...
I believe there have many other phys also do such operation, hardcode is
unacceptable...
And these code can be put into phy_device.c as common interface.
>+ ret = fec_reset_phy(ndev);
>+ if (ret)
>+ return ret;
>+ ret = phy_init_hw(phy_dev);
>+ if (ret)
>+ return ret;
>+ }
>+ return 0;
>+}
>+
> static int fec_enet_clk_enable(struct net_device *ndev, bool enable) {
> struct fec_enet_private *fep = netdev_priv(ndev); @@ -1862,6
>+1889,10 @@ 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;
>+
>+ ret = fec_enet_clk_ref_enable_reset_phy_quirk(ndev);
>+ if (ret)
>+ netdev_warn(ndev, "Resetting PHY failed, connection
>may be
>+unstable\n");
> } else {
> clk_disable_unprepare(fep->clk_ahb);
> clk_disable_unprepare(fep->clk_enet_out);
>@@ -2860,11 +2891,17 @@ fec_enet_open(struct net_device *ndev)
> if (ret)
> goto err_enet_mii_probe;
>
>+ /* as the PHY is connected now, trigger the reset quirk again */
>+ ret = fec_enet_clk_ref_enable_reset_phy_quirk(ndev);
>+ if (ret)
>+ netdev_warn(ndev, "Resetting PHY failed, connection may be
>+unstable\n");
>+
> if (fep->quirks & FEC_QUIRK_ERR006687)
> imx6q_cpuidle_fec_irqs_used();
>
> napi_enable(&fep->napi);
> phy_start(ndev->phydev);
>+
No need blank line here...
> netif_tx_start_all_queues(ndev);
>
> device_set_wakeup_enable(&ndev->dev, fep->wol_flag &
>--
>2.11.0
