From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753985AbdGCJ5n (ORCPT ); Mon, 3 Jul 2017 05:57:43 -0400 Received: from szxga01-in.huawei.com ([45.249.212.187]:9264 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753804AbdGCJ5h (ORCPT ); Mon, 3 Jul 2017 05:57:37 -0400 Subject: Re: [PATCH NET V5 2/2] net: hns: Use phy_driver to setup Phy loopback To: Andrew Lunn CC: davem , "f.fainelli" , huangdaode , "xuwei (O)" , "Liguozhu (Kenneth)" , "Zhuangyuzeng (Yisen)" , Gabriele Paoloni , John Garry , Linuxarm , "Salil Mehta" , "lipeng (Y)" , tremyfr , netdev , linux-kernel References: <1498443039-134503-3-git-send-email-linyunsheng@huawei.com> <20170626134235.GC2623@lunn.ch> <17132762-9b94-bc32-fee8-e90a6db5762a@huawei.com> <20170627132958.GA9921@lunn.ch> <3d382bc6-f7b6-9df3-8bb0-fee55b72ac74@huawei.com> <20170628202819.GA22815@lunn.ch> <1bff07ed-d423-dfa2-61e3-3f35c4536632@huawei.com> <20170629135648.GD9244@lunn.ch> <20170630133915.GD17212@lunn.ch> <20170701151707.GA5859@lunn.ch> From: Yunsheng Lin Message-ID: Date: Mon, 3 Jul 2017 17:57:05 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <20170701151707.GA5859@lunn.ch> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.74.190.125] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090205.595A14FF.0014,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 919f0c9548e18d869e458315ac66c172 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Andrew On 2017/7/1 23:17, Andrew Lunn wrote: > On Sat, Jul 01, 2017 at 11:57:32AM +0000, linyunsheng wrote: >> Hi, Andrew >> >> I am agreed wih you on this. >> But self test is also a feature of our product, and our >> customer way choose to diagnose a problem using >> self test, even if self test does not give a clear >> reason to the problem. >> we don't want to remove a feature that we don't >> know when our customer will be using. > > Far enough. So please take a close look at the code and try to fix > it. The corner cases are your problem, a down'ed interface, WOL, etc. > It is issues like this which can result in phy_resume() being called > without there first being a phy_suspend. > I looked into how the phy core deal with down'ed interface, WOL problem, here is what I found: 1.phydev->state is used to track the state of the phy. 2.phy_start/stop and phy_state_machine work together to make sure the phydev->state is consistent with phydev->suspended. And using phy_start/stop instead of phy_resume/suspend should take care of down'ed interface problem. Will using phy_start/stop cause other problems? As for WOL, phy_state_machine: if (needs_aneg) err = phy_start_aneg_priv(phydev, false); else if (do_suspend) phy_suspend(phydev); I think the phy core also have the same problem, because above code does not put the phy into suspending when it is WOL'ed, and it do not check the return value of phy_suspend. I hope I am not missing something obvious. Please let me know if you have any idea about WOL problem, thanks. Best Regards Yunsheng Lin