From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Lunn Subject: Re: Potential issue with f5e64032a799 "net: phy: fix resume handling" Date: Sat, 3 Feb 2018 21:17:48 +0100 Message-ID: <20180203201748.GN16818@lunn.ch> References: <4886212b-0274-ffa0-f98e-da2e43a3b042@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Russell King , "netdev@vger.kernel.org" To: Heiner Kallweit Return-path: Received: from vps0.lunn.ch ([185.16.172.187]:45237 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752704AbeBCURx (ORCPT ); Sat, 3 Feb 2018 15:17:53 -0500 Content-Disposition: inline In-Reply-To: <4886212b-0274-ffa0-f98e-da2e43a3b042@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Sat, Feb 03, 2018 at 05:41:54PM +0100, Heiner Kallweit wrote: > This commit forces callers of phy_resume() and phy_suspend() to hold > mutex phydev->lock. This was done for calls to phy_resume() and > phy_suspend() in phylib, however there are more callers in network > drivers. I'd assume that these other calls issue a warning now > because of the lock not being held. > So is there something I miss or would this have to be fixed? Hi Heiner This is a good point. Yes, it looks like some fixes are needed. But what exactly? The phy state machine will suspend and resume the phy is you call phy_stop() and phy_start() in the MAC suspend and resume functions. A few examples: tc35815_suspend(), ravb_suspend() via ravb_close(), sh_eth_suspend() via sh_eth_close(), fec_suspend(), mpc52xx_fec_of_suspend() via mpc52xx_fec_close(), ucc_geth_suspend(), etc... So i suspect those drivers which call phy_suspend()/phy_resume() should really be modified to call phy_stop()/phy_start(). hns_nic_config_phy_loopback() is just funky, and probably needs the help of the hns guys to fix. dsa_slave_suspend() already does a phy_stop(), so the phy_suspend() can be removed. The comments in lpc_eth_open() suggest the phy_resume() is needed, so locks should be added. socfpga_dwmac_resume() seems to be the same. Andrew