From: Andrew Lunn <andrew@lunn.ch>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: netdev@vger.kernel.org, zefir.kurtisi@neratec.com,
Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
Wingman Kwok <w-kwok2@ti.com>,
Murali Karicheri <m-karicheri2@ti.com>,
Vivien Didelot <vivien.didelot@savoirfairelinux.com>,
"David S. Miller" <davem@davemloft.net>,
Philippe Reynes <tremyfr@gmail.com>,
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>,
Jisheng Zhang <jszhang@marvell.com>,
Jarod Wilson <jarod@redhat.com>,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next 4/4] net: dsa: Do not clobber PHY link outside of state machine
Date: Tue, 7 Feb 2017 01:11:32 +0100 [thread overview]
Message-ID: <20170207001132.GB30169@lunn.ch> (raw)
In-Reply-To: <20170206235523.23216-5-f.fainelli@gmail.com>
On Mon, Feb 06, 2017 at 03:55:23PM -0800, Florian Fainelli wrote:
> Calling phy_read_status() means that we may call into
> genphy_read_status() which in turn will use genphy_update_link() which
> can make changes to phydev->link outside of the state machine's state
> transitions. This is an invalid behavior that is now caught as of
> 811a919135b9 ("phy state machine: failsafe leave invalid RUNNING state")
>
> Reported-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> net/dsa/slave.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 09fc3e9462c1..4b6fb6b14de4 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -651,14 +651,10 @@ dsa_slave_get_link_ksettings(struct net_device *dev,
> struct ethtool_link_ksettings *cmd)
> {
> struct dsa_slave_priv *p = netdev_priv(dev);
> - int err;
> + int err = -EOPNOTSUPP;
>
> - err = -EOPNOTSUPP;
> - if (p->phy != NULL) {
> - err = phy_read_status(p->phy);
> - if (err == 0)
> - err = phy_ethtool_ksettings_get(p->phy, cmd);
> - }
> + if (p->phy != NULL)
> + err = phy_ethtool_ksettings_get(p->phy, cmd);
Hi Florian
So what we are effectively doing is returning the state from the last
poll/interrupt. The poll information could be up to 1 second out of
date, but those PHYs using interrupts should give more fresh
information.
Seems reasonable.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
next prev parent reply other threads:[~2017-02-07 0:11 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-06 23:55 [PATCH net-next 0/4] net: Incorrect use of phy_read_status() Florian Fainelli
2017-02-06 23:55 ` [PATCH net-next 1/4] net: mv643xx_eth: Do not clobber PHY link outside of state machine Florian Fainelli
2017-02-07 0:13 ` Andrew Lunn
2017-02-06 23:55 ` [PATCH net-next 2/4] net: pxa168_eth: " Florian Fainelli
2017-02-06 23:55 ` [PATCH net-next 3/4] net: netcp: " Florian Fainelli
2017-02-06 23:55 ` [PATCH net-next 4/4] net: dsa: " Florian Fainelli
2017-02-07 0:11 ` Andrew Lunn [this message]
2017-02-07 11:42 ` David Laight
2017-02-07 13:14 ` Andrew Lunn
2017-02-07 18:48 ` [PATCH net-next 0/4] net: Incorrect use of phy_read_status() David Miller
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=20170207001132.GB30169@lunn.ch \
--to=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=jarod@redhat.com \
--cc=jszhang@marvell.com \
--cc=linux-kernel@vger.kernel.org \
--cc=m-karicheri2@ti.com \
--cc=netdev@vger.kernel.org \
--cc=sebastian.hesselbarth@gmail.com \
--cc=sergei.shtylyov@cogentembedded.com \
--cc=tremyfr@gmail.com \
--cc=vivien.didelot@savoirfairelinux.com \
--cc=w-kwok2@ti.com \
--cc=zefir.kurtisi@neratec.com \
/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.