From: Richard Leitner <richard.leitner@xxxxxxxxxxx> Sent: Wednesday, December 06, 2017 4:12 PM >To: Andy Duan <fugang.duan@xxxxxxx>; Richard Leitner <dev@xxxxxxxxxx>; >robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx; andrew@xxxxxxx; >f.fainelli@xxxxxxxxx; frowand.list@xxxxxxxxx >Cc: davem@xxxxxxxxxxxxx; geert+renesas@xxxxxxxxx; >sergei.shtylyov@xxxxxxxxxxxxxxxxxx; baruch@xxxxxxxxxx; david.wu@rock- >chips.com; lukma@xxxxxxx; netdev@xxxxxxxxxxxxxxx; >devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx >Subject: Re: [PATCH net-next v3 4/4] net: fec: add >phy_reset_after_clk_enable() support > >Hi Andy, > >On 12/06/2017 02:50 AM, Andy Duan wrote: >> From: Richard Leitner <dev@xxxxxxxxxx> Sent: Tuesday, December 05, >> 2017 9:26 PM >>> 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 add support for the phy_reset_after_clk_enable function >>> from phylib to mitigate this issue. > >... > >>> diff --git a/drivers/net/ethernet/freescale/fec_main.c >>> b/drivers/net/ethernet/freescale/fec_main.c >>> index 610573855213..8c3d0fb7db20 100644 >>> --- a/drivers/net/ethernet/freescale/fec_main.c >>> +++ b/drivers/net/ethernet/freescale/fec_main.c >>> @@ -1862,6 +1862,8 @@ 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; >>> + >>> + phy_reset_after_clk_enable(ndev->phydev); >>> } else { >>> clk_disable_unprepare(fep->clk_ahb); >>> clk_disable_unprepare(fep->clk_enet_out); >>> @@ -2860,6 +2862,11 @@ fec_enet_open(struct net_device *ndev) >>> if (ret) >>> goto err_enet_mii_probe; >>> >>> + /* reset phy if needed here, due to the fact this is the first time we >>> + * have the net_device to phy_driver link >>> + */ >>> + phy_reset_after_clk_enable(ndev->phydev); >>> + >> >> The patch series look better. >> But why does it need to reset phy here since phy already is hard reset after >clock enable. > >The problem here is that in fec_enet_open() the fec_enet_clk_enable() call is >done before the phy is probed. Therefore (as mentioned in the >comment) there's no link from the net_device (ndev) to the phy_driver >(which holds the flags). > >Any suggestions for a better solution are highly appreciated here! Thanks! > >regards;Richard.L Okay, I see. For the patch: Acked-by: Fugang Duan <fugang.duan@xxxxxxx> ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f