netdev - Re: [PATCH net-next v3 4/4] net: fec: add phy_reset_after_clk

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1e7b3e7f-251d-4101-1441-702e891f2ca9@skidata.com>
Date:   Wed, 6 Dec 2017 09:12:14 +0100
From:   Richard Leitner <richard.leitner@...data.com>
To:     Andy Duan <fugang.duan@....com>, Richard Leitner <dev@...l1n.net>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "frowand.list@...il.com" <frowand.list@...il.com>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "geert+renesas@...der.be" <geert+renesas@...der.be>,
        "sergei.shtylyov@...entembedded.com" 
        <sergei.shtylyov@...entembedded.com>,
        "baruch@...s.co.il" <baruch@...s.co.il>,
        "david.wu@...k-chips.com" <david.wu@...k-chips.com>,
        "lukma@...x.de" <lukma@...x.de>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.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 <dev@...l1n.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