[PATCH net-next] net: phy: at803x: remove at803x_aneg_done()

at803x_aneg_done() is pretty much dead code since the patch series
"net: phy: improve and simplify phylib state machine" [1]. Remove it.

[1] https://lore.kernel.org/netdev/922c223b-7bc0-e0ec-345d-2034b796af91@gmail.com/

Suggested-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Michael Walle <redacted>
---
 drivers/net/phy/at803x.c | 31 -------------------------------
 1 file changed, 31 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c2aa4c92edde..d7799beb811c 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c

@@ -751,36 +751,6 @@ static void at803x_link_change_notify(struct phy_device *phydev)
 	}
 }
 
-static int at803x_aneg_done(struct phy_device *phydev)
-{
-	int ccr;
-
-	int aneg_done = genphy_aneg_done(phydev);
-	if (aneg_done != BMSR_ANEGCOMPLETE)
-		return aneg_done;
-
-	/*
-	 * in SGMII mode, if copper side autoneg is successful,
-	 * also check SGMII side autoneg result
-	 */
-	ccr = phy_read(phydev, AT803X_REG_CHIP_CONFIG);
-	if ((ccr & AT803X_MODE_CFG_MASK) != AT803X_MODE_CFG_SGMII)
-		return aneg_done;
-
-	/* switch to SGMII/fiber page */
-	phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr & ~AT803X_BT_BX_REG_SEL);
-
-	/* check if the SGMII link is OK. */
-	if (!(phy_read(phydev, AT803X_PSSR) & AT803X_PSSR_MR_AN_COMPLETE)) {
-		phydev_warn(phydev, "803x_aneg_done: SGMII link is not ok\n");
-		aneg_done = 0;
-	}
-	/* switch back to copper page */
-	phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr | AT803X_BT_BX_REG_SEL);
-
-	return aneg_done;
-}
-
 static int at803x_read_status(struct phy_device *phydev)
 {
 	int ss, err, old_link = phydev->link;

@@ -1198,7 +1168,6 @@ static struct phy_driver at803x_driver[] = {
 	.resume			= at803x_resume,
 	/* PHY_GBIT_FEATURES */
 	.read_status		= at803x_read_status,
-	.aneg_done		= at803x_aneg_done,
 	.config_intr		= &at803x_config_intr,
 	.handle_interrupt	= at803x_handle_interrupt,
 	.get_tunable		= at803x_get_tunable,

-- 
2.20.1

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

On Thu, Mar 18, 2021 at 03:54:00PM +0100, Heiner Kallweit wrote:

On 18.03.2021 15:23, Michael Walle wrote:

quoted

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.

On 18.03.2021 16:17, Vladimir Oltean wrote:

On Thu, Mar 18, 2021 at 03:54:00PM +0100, Heiner Kallweit wrote:

quoted

On 18.03.2021 15:23, Michael Walle wrote:

quoted

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.

I don't argue against the change, I just think that the current commit
description isn't sufficient. What you just said I would have expected
in the commit description.

Am 2021-03-18 17:21, schrieb Heiner Kallweit:

On 18.03.2021 16:17, Vladimir Oltean wrote:

quoted

On Thu, Mar 18, 2021 at 03:54:00PM +0100, Heiner Kallweit wrote:

quoted

On 18.03.2021 15:23, Michael Walle wrote:

quoted

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.

I don't argue against the change, I just think that the current commit
description isn't sufficient. What you just said I would have expected
in the commit description.

I'll come up with a better one, Vladimir, may I use parts of the text
above?

-michael

On Thu, Mar 18, 2021 at 05:38:13PM +0100, Michael Walle wrote:

Am 2021-03-18 17:21, schrieb Heiner Kallweit:

quoted

On 18.03.2021 16:17, Vladimir Oltean wrote:

quoted

On Thu, Mar 18, 2021 at 03:54:00PM +0100, Heiner Kallweit wrote:

quoted

On 18.03.2021 15:23, Michael Walle wrote:

quoted

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.

I don't argue against the change, I just think that the current commit
description isn't sufficient. What you just said I would have expected
in the commit description.

I'll come up with a better one, Vladimir, may I use parts of the text
above?

My words aren't copyrighted, so feel free, however you might want to
check with Heiner too for his part, you never know.

On 18.03.2021 18:04, Vladimir Oltean wrote:

On Thu, Mar 18, 2021 at 05:38:13PM +0100, Michael Walle wrote:

quoted

Am 2021-03-18 17:21, schrieb Heiner Kallweit:

quoted

On 18.03.2021 16:17, Vladimir Oltean wrote:

quoted

On Thu, Mar 18, 2021 at 03:54:00PM +0100, Heiner Kallweit wrote:

quoted

On 18.03.2021 15:23, Michael Walle wrote:

quoted

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.

I don't argue against the change, I just think that the current commit
description isn't sufficient. What you just said I would have expected
in the commit description.

I'll come up with a better one, Vladimir, may I use parts of the text
above?

My words aren't copyrighted, so feel free, however you might want to
check with Heiner too for his part, you never know.

I'm not paid for the content of my mails, so feel free to quote.