| Message ID | 20210318194431.14811-1-michael@walle.cc (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5b6b827413e8a86b1f83c3a199fe639c0e292295 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,net-next] net: phy: at803x: remove at803x_aneg_done() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 43 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
Am 2021-03-18 20:44, schrieb Michael Walle: > Here is what Vladimir says about it: > > at803x_aneg_done() 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 > Heiner have said about it [1]: > > | 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. > > Removing the SGMII link check, would just keep the call to > genphy_aneg_done(), which is also the fallback. Thus we can just remove > at803x_aneg_done() altogether. > > [1] > https://lore.kernel.org/netdev/fdf0074a-2572-5914-6f3e-77202cbf96de@gmail.com/ > > Suggested-by: Vladimir Oltean <olteanv@gmail.com> > Signed-off-by: Michael Walle <michael@walle.cc> > --- Sorry forgot the version history: Changes since v1: - more detailed commit message -michael
On 18.03.2021 20:44, Michael Walle wrote: > Here is what Vladimir says about it: > > at803x_aneg_done() 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 > Heiner have said about it [1]: > > | 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. > > Removing the SGMII link check, would just keep the call to > genphy_aneg_done(), which is also the fallback. Thus we can just remove > at803x_aneg_done() altogether. > > [1] https://lore.kernel.org/netdev/fdf0074a-2572-5914-6f3e-77202cbf96de@gmail.com/ > > Suggested-by: Vladimir Oltean <olteanv@gmail.com> > Signed-off-by: Michael Walle <michael@walle.cc> > --- Reviewed-by: Heiner Kallweit <hkallweit1@gmail.com>
Hello: This patch was applied to netdev/net-next.git (refs/heads/master): On Thu, 18 Mar 2021 20:44:31 +0100 you wrote: > Here is what Vladimir says about it: > > at803x_aneg_done() 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 > Heiner have said about it [1]: > > [...] Here is the summary with links: - [v2,net-next] net: phy: at803x: remove at803x_aneg_done() https://git.kernel.org/netdev/net-next/c/5b6b827413e8 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
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,
Here is what Vladimir says about it: at803x_aneg_done() 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 Heiner have said about it [1]: | 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. Removing the SGMII link check, would just keep the call to genphy_aneg_done(), which is also the fallback. Thus we can just remove at803x_aneg_done() altogether. [1] https://lore.kernel.org/netdev/fdf0074a-2572-5914-6f3e-77202cbf96de@gmail.com/ Suggested-by: Vladimir Oltean <olteanv@gmail.com> Signed-off-by: Michael Walle <michael@walle.cc> --- drivers/net/phy/at803x.c | 31 ------------------------------- 1 file changed, 31 deletions(-)