From: Richard Leitner <richard.leit...@skidata.com> Sent: Wednesday, December
06, 2017 4:12 PM
>To: Andy Duan <fugang.d...@nxp.com>; Richard Leitner <d...@g0hl1n.net>;
>robh...@kernel.org; mark.rutl...@arm.com; and...@lunn.ch;
>f.faine...@gmail.com; frowand.l...@gmail.com
>Cc: da...@davemloft.net; geert+rene...@glider.be;
>sergei.shtyl...@cogentembedded.com; bar...@tkos.co.il; david.wu@rock-
>chips.com; lu...@denx.de; netdev@vger.kernel.org;
>devicet...@vger.kernel.org; linux-ker...@vger.kernel.org
>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 <d...@g0hl1n.net> 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.d...@nxp.com>
