All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: Potential issue with f5e64032a799 "net: phy: fix resume handling"
Date: Sat, 3 Feb 2018 21:17:48 +0100	[thread overview]
Message-ID: <20180203201748.GN16818@lunn.ch> (raw)
In-Reply-To: <4886212b-0274-ffa0-f98e-da2e43a3b042@gmail.com>

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

  reply	other threads:[~2018-02-03 20:17 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-03 16:41 Potential issue with f5e64032a799 "net: phy: fix resume handling" Heiner Kallweit
2018-02-03 20:17 ` Andrew Lunn [this message]
2018-02-03 23:58   ` Heiner Kallweit
2018-02-04  2:48     ` Florian Fainelli
2018-02-05 21:48       ` Heiner Kallweit
2018-02-06 11:00         ` Russell King - ARM Linux
2018-02-06 12:55           ` Andrew Lunn
2018-02-07 20:56       ` handling of phy_stop() and phy_stop_machine() in phylib Heiner Kallweit
2018-02-07 21:13         ` Russell King - ARM Linux
2018-02-07 23:03           ` Florian Fainelli
2018-02-25 13:00   ` Potential issue with f5e64032a799 "net: phy: fix resume handling" Heiner Kallweit
2018-02-25 16:38     ` Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180203201748.GN16818@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=hkallweit1@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=rmk+kernel@armlinux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.