Netdev List
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Michael Walle <michael@walle.cc>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Andrew Lunn <andrew@lunn.ch>,
	Russell King <linux@armlinux.org.uk>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>
Subject: Re: [PATCH net-next] net: phy: at803x: remove at803x_aneg_done()
Date: Thu, 18 Mar 2021 17:17:12 +0200	[thread overview]
Message-ID: <20210318151712.7hmdaufxylyl33em@skbuf> (raw)
In-Reply-To: <411c3508-978e-4562-f1e9-33ca7e98a752@gmail.com>

On Thu, Mar 18, 2021 at 03:54:00PM +0100, Heiner Kallweit wrote:
> On 18.03.2021 15:23, Michael Walle wrote:
> > at803x_aneg_done() is pretty much dead code since the patch series
> > "net: phy: improve and simplify phylib state machine" [1]. Remove it.
> > 
> 
> Well, it's not dead, it's resting .. There are few places where
> phy_aneg_done() is used. So you would need to explain:
> - why these users can't be used with this PHY driver
> - or why the aneg_done callback isn't needed here and the
>   genphy_aneg_done() fallback is sufficient

The piece of code that Michael is removing keeps the aneg reporting as
"not done" even when the copper-side link was reported as up, but the
in-band autoneg has not finished.

That was the _intended_ behavior when that code was introduced, and you
have said about it:
https://www.spinics.net/lists/stable/msg389193.html

| That's not nice from the PHY:
| It signals "link up", and if the system asks the PHY for link details,
| then it sheepishly says "well, link is *almost* up".

If the specification of phy_aneg_done behavior does not include in-band
autoneg (and it doesn't), then this piece of code does not belong here.

The fact that we can no longer trigger this code from phylib is yet
another reason why it fails at its intended (and wrong) purpose and
should be removed.

  reply	other threads:[~2021-03-18 15:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-18 14:23 [PATCH net-next] net: phy: at803x: remove at803x_aneg_done() Michael Walle
2021-03-18 14:54 ` Heiner Kallweit
2021-03-18 15:17   ` Vladimir Oltean [this message]
2021-03-18 16:21     ` Heiner Kallweit
2021-03-18 16:38       ` Michael Walle
2021-03-18 17:04         ` Vladimir Oltean
2021-03-18 17:09           ` Heiner Kallweit

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=20210318151712.7hmdaufxylyl33em@skbuf \
    --to=olteanv@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=michael@walle.cc \
    --cc=netdev@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox