| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 20 Nov 2017 09:47:26 +0000
From: Andy Duan <fugang.duan@....com>
To: Richard Leitner <dev@...l1n.net>,
"f.fainelli@...il.com" <f.fainelli@...il.com>,
"andrew@...n.ch" <andrew@...n.ch>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"richard.leitner@...data.com" <richard.leitner@...data.com>
Subject: RE: [PATCH v2 3/3] net: ethernet: fec: fix refclk enable for SMSC
LAN8710/20
From: Richard Leitner <dev@...l1n.net> Sent: Monday, November 20, 2017 4:34 PM
>To: f.fainelli@...il.com; Andy Duan <fugang.duan@....com>;
>andrew@...n.ch
>Cc: netdev@...r.kernel.org; linux-kernel@...r.kernel.org;
>richard.leitner@...data.com
>Subject: [PATCH v2 3/3] net: ethernet: fec: fix refclk enable for SMSC
>LAN8710/20
>
>From: Richard Leitner <richard.leitner@...data.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.leitner@...data.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
Powered by blists - more mailing lists