From: Richard Leitner <richard.leit...@skidata.com> Sent: Thursday, July 06,
2017 9:06 PM
>To: Andy Duan <fugang.d...@nxp.com>; robh...@kernel.org;
>mark.rutl...@arm.com
>Cc: netdev@vger.kernel.org; devicet...@vger.kernel.org; linux-
>ker...@vger.kernel.org; d...@g0hl1n.net; Richard Leitner
><richard.leit...@skidata.com>
>Subject: [PATCH 2/2] net: ethernet: fsl: add phy reset after clk enable option
>
>Some PHYs (for example the LAN8710) doesn't allow turning the clocks off and
>on again without reset (according to their datasheet). Exactly this behaviour
>was introduced for power saving reasons by commit e8fcfcd5684a
>("net: fec: optimize the clock management to save power") Therefore add a
>devictree option to perform a PHY reset and configuration after every clock
>enable.
>
>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()
>
>In our case this problem does occur at about every 10th to 50th POR of an
>LAN8710 connected to an i.MX6 SoC. The typical symptom of this problem is a
>"swinging" ethernet link. Similar issues were experienced by users of the NXP
>forum:
> https://community.nxp.com/thread/389902
> https://community.nxp.com/message/309354
>With this patch applied the issue didn't occur for at least a few hundred PORs
>of our board.
>
>Fixes: e8fcfcd5684a ("net: fec: optimize the clock management to sa...")
>Signed-off-by: Richard Leitner <richard.leit...@skidata.com>
>---
> Documentation/devicetree/bindings/net/fsl-fec.txt | 3 +++
> drivers/net/ethernet/freescale/fec.h | 1 +
> drivers/net/ethernet/freescale/fec_main.c | 16 ++++++++++++++++
> 3 files changed, 20 insertions(+)
>
>diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt
>b/Documentation/devicetree/bindings/net/fsl-fec.txt
>index 6f55bdd..1766579 100644
>--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
>+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
>@@ -23,6 +23,9 @@ Optional properties:
> - phy-handle : phandle to the PHY device connected to this device.
> - fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
> Use instead of phy-handle.
>+- phy-reset-after-clk-enable : If present then a phy reset and
>+configuration
>+ will be performed everytime after the clocks are enabled. This is
>+required
>+ for some PHYs to work properly.
> - fsl,num-tx-queues : The property is valid for enet-avb IP, which supports
> hw multi queues. Should specify the tx queue number, otherwise set tx
>queue
> number to 1.
>diff --git a/drivers/net/ethernet/freescale/fec.h
>b/drivers/net/ethernet/freescale/fec.h
>index 3da8084..d4f658a 100644
>--- a/drivers/net/ethernet/freescale/fec.h
>+++ b/drivers/net/ethernet/freescale/fec.h
>@@ -538,6 +538,7 @@ struct fec_enet_private {
> int phy_reset_duration;
> int phy_reset_post_delay;
> bool phy_reset_active_high;
>+ bool phy_reset_after_clk_enable;
>
> struct napi_struct napi;
> int csum_flags;
>diff --git a/drivers/net/ethernet/freescale/fec_main.c
>b/drivers/net/ethernet/freescale/fec_main.c
>index cbbf7b8..4e744f3 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"
>
>@@ -1862,6 +1863,18 @@ 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;
>+
>+ /* reset the phy after clk is enabled if it's configured */
>+ if (fep->phy_reset_after_clk_enable) {
>+ ret = fec_reset_phy(ndev);
>+ if (ret)
>+ goto failed_reset;
>+ if (ndev->phydev) {
>+ ret = phy_init_hw(ndev->phydev);
>+ if (ret)
>+ goto failed_reset;
>+ }
>+ }
Since it is common issue so long as using the PHY, can you move it into smsc
phy driver like in .smsc_phy_reset() function ?
And get the reset pin from phy dts node.
Andy
> } else {
> clk_disable_unprepare(fep->clk_ahb);
> clk_disable_unprepare(fep->clk_enet_out);
>@@ -1876,6 +1889,7 @@ static int fec_enet_clk_enable(struct net_device
>*ndev, bool enable)
>
> return 0;
>
>+failed_reset:
> failed_clk_ref:
> if (fep->clk_ref)
> clk_disable_unprepare(fep->clk_ref);
>@@ -3350,6 +3364,7 @@ fec_probe(struct platform_device *pdev)
> fep->phy_reset_post_delay = 1;
>
> fep->phy_reset_active_high = of_property_read_bool(np,
>"phy-reset-active-high");
>+ fep->phy_reset_after_clk_enable =
>of_property_read_bool(np,
>+"phy-reset-after-clk-enable");
>
> ret = devm_gpio_request_one(&pdev->dev, fep->phy_reset,
> fep->phy_reset_active_high ?
>GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW, @@ -3360,6 +3375,7 @@
>fec_probe(struct platform_device *pdev)
> }
> } else {
> fep->phy_reset = 0;
>+ fep->phy_reset_after_clk_enable = false;
> }
>
> ret = of_get_phy_mode(pdev->dev.of_node);
>--
>2.1.4
