From: Andy Duan <fugang.duan@nxp.com>
To: Richard Leitner <richard.leitner@skidata.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>
CC: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "dev@g0hl1n.net" <dev@g0hl1n.net>, Andrew Lunn <andrew@lunn.ch>
Subject: RE: [PATCH 2/2] net: ethernet: fsl: add phy reset after clk enable
 option
Thread-Topic: [PATCH 2/2] net: ethernet: fsl: add phy reset after clk enable
 option
Thread-Index: AQHS9liVhz49Jf19SE+3rJ5oc5evWaJH0raAgAAKjoCAAASU8IAAPw0AgAATqGA=
Date: Fri, 7 Jul 2017 11:08:13 +0000
Message-ID: <AM4PR0401MB22609FFACD2BB51247709936FFAA0@AM4PR0401MB2260.eurprd04.prod.outlook.com>
References: <1499346330-12166-1-git-send-email-richard.leitner@skidata.com>
 <1499346330-12166-2-git-send-email-richard.leitner@skidata.com>
 <AM4PR0401MB2260ECD4508E26FF16670004FFAA0@AM4PR0401MB2260.eurprd04.prod.outlook.com>
 <81105c77-d48f-271b-2de1-c877b9413184@skidata.com>
 <AM4PR0401MB226089839458F5F92C3B1F39FFAA0@AM4PR0401MB2260.eurprd04.prod.outlook.com>
 <6de114cb-4521-4bb2-d0a3-4aea32936bd3@skidata.com>
In-Reply-To: <6de114cb-4521-4bb2-d0a3-4aea32936bd3@skidata.com>
Accept-Language: zh-CN, en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Jul 2017 11:08:13.0643
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR0401MB2259
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v67B8SBn018230
Content-Length: 2362
Lines: 60

From: Richard Leitner <richard.leitner@skidata.com> Sent: Friday, July 07, 2017 5:53 PM
>To: Andy Duan <fugang.duan@nxp.com>; robh+dt@kernel.org;
>mark.rutland@arm.com
>Cc: netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
>kernel@vger.kernel.org; dev@g0hl1n.net; Andrew Lunn <andrew@lunn.ch>
>Subject: Re: [PATCH 2/2] net: ethernet: fsl: add phy reset after clk enable
>option
>
>
>On 07/07/2017 09:03 AM, Andy Duan wrote:
>> From: Richard Leitner <richard.leitner@skidata.com> Sent: Friday, July
>> 07, 2017 1:51 PM
>>>> 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.
>>>
>>> Some more points that come into my mind:
>>>  - The smsc_phy_reset function is registered as "soft_reset". Would
>>> it be OK to use nRST in it?
>>
>> It is not reasonable.
>>
>>>  - Would it be OK to call the phy_init_hw function from within the
>>> smsc_phy_reset?
>>
>> No, phy_init_hw() already call .drv->soft_reset().
>>
>>>  - IMHO I'd have to move the reset gpio binding inside the phy node
>>> then. Isn't that a pretty big change doing that for all PHYs/FECs? Would it be
>"worth" it?
>>>
>> To make the change to be common, there have big change for phy driver.
>> Maybe somebody can give one good suggestion/solution for it.
>
>Sorry, I don't think I understood everything correctly:
>
>1. The "phy-reset-gpios" binding should go inside the phy node. This will cause
>to *change ALL FEC and PHY drivers*. Correct?
>
The "phy-reset-gpios" binding should go inside the phy node that is more reasonable.
It is better PHY core driver handle phy hw reset.

>2. Add an additonal "hard reset" function to the PHY driver which handles the
>"phy-reset-gpios". Correct?
>
Correct.

>3. Who should then trigger the "hard reset" of the PHY? phy_init_hw? The FEC?
>
>The point is that the LAN8710 is currently not always working correctly,
>therefore this small change was proposed. Should we really change all
>PHY/FECs only because of this?
>Furthermore one problem still remains: The enet_refclk is controlled by the
>FEC. How does the PHY recognize when it was disabled/enabled?
>
Your patch is workaround for the issue. As you pointed out these is a common issue.
So we hope to get a better solution to handle these in common code.

Andy