[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZoW1fNqV3PxEobFx@shell.armlinux.org.uk>
Date: Wed, 3 Jul 2024 21:33:00 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Andrew Lunn <andrew@...n.ch>
Cc: Serge Semin <fancer.lancer@...il.com>, si.yanteng@...ux.dev,
Huacai Chen <chenhuacai@...nel.org>,
Yanteng Si <siyanteng@...ngson.cn>, hkallweit1@...il.com,
peppe.cavallaro@...com, alexandre.torgue@...s.st.com,
joabreu@...opsys.com, Jose.Abreu@...opsys.com,
guyinggang@...ngson.cn, netdev@...r.kernel.org,
chris.chenfeiyang@...il.com
Subject: Re: [PATCH net-next v13 12/15] net: stmmac: Fixed failure to set
network speed to 1000.
On Wed, Jul 03, 2024 at 09:09:53PM +0200, Andrew Lunn wrote:
> > Rather than erroring out, I think it may be better to just adopt
> > the Marvell solution to this problem to give consistent behaviour
> > across all PHYs.
>
> Yes, expand phy_config_aneg() to look for this condition and enable
> autoneg. But should we disable it when the condition is reverted? The
> user swaps to 100Mbps forced?
I think we should "lie" to userspace rather than report how the
hardware was actually programmed - again, because that's what would
happen with Marvell Alaska.
> What about other speeds? Is this limited to 1G? Since we have devices
> without auto-neg for 2500BaseX i assume it is not an issue there.
1000base-X can have AN disabled - that's not an issue. Yes, there's
the ongoing issues with 2500base-X. 10Gbase-T wording is similar to
1000base-T, so we probably need to do similar there. Likely also the
case for 2500base-T and 5000base-T as well.
So I'm thinking of something like this (untested):
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 6c6ec9475709..197c4d5ab55b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2094,22 +2094,20 @@ EXPORT_SYMBOL(phy_reset_after_clk_enable);
/**
* genphy_config_advert - sanitize and advertise auto-negotiation parameters
* @phydev: target phy_device struct
+ * @advert: auto-negotiation parameters to advertise
*
* Description: Writes MII_ADVERTISE with the appropriate values,
* after sanitizing the values to make sure we only advertise
* what is supported. Returns < 0 on error, 0 if the PHY's advertisement
* hasn't changed, and > 0 if it has changed.
*/
-static int genphy_config_advert(struct phy_device *phydev)
+static int genphy_config_advert(struct phy_device *phydev,
+ const unsigned long *advert)
{
int err, bmsr, changed = 0;
u32 adv;
- /* Only allow advertising what this PHY supports */
- linkmode_and(phydev->advertising, phydev->advertising,
- phydev->supported);
-
- adv = linkmode_adv_to_mii_adv_t(phydev->advertising);
+ adv = linkmode_adv_to_mii_adv_t(advert);
/* Setup standard advertisement */
err = phy_modify_changed(phydev, MII_ADVERTISE,
@@ -2132,7 +2130,7 @@ static int genphy_config_advert(struct phy_device *phydev)
if (!(bmsr & BMSR_ESTATEN))
return changed;
- adv = linkmode_adv_to_mii_ctrl1000_t(phydev->advertising);
+ adv = linkmode_adv_to_mii_ctrl1000_t(advert);
err = phy_modify_changed(phydev, MII_CTRL1000,
ADVERTISE_1000FULL | ADVERTISE_1000HALF,
@@ -2356,6 +2354,9 @@ EXPORT_SYMBOL(genphy_check_and_restart_aneg);
*/
int __genphy_config_aneg(struct phy_device *phydev, bool changed)
{
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(fixed_advert);
+ const struct phy_setting *set;
+ unsigned long *advert;
int err;
err = genphy_c45_an_config_eee_aneg(phydev);
@@ -2370,10 +2371,25 @@ int __genphy_config_aneg(struct phy_device *phydev, bool changed)
else if (err)
changed = true;
- if (AUTONEG_ENABLE != phydev->autoneg)
+ if (phydev->autoneg == AUTONEG_ENABLE) {
+ /* Only allow advertising what this PHY supports */
+ linkmode_and(phydev->advertising, phydev->advertising,
+ phydev->supported);
+ advert = phydev->advertising;
+ } else if (phydev->speed < SPEED_1000) {
return genphy_setup_forced(phydev);
+ } else {
+ linkmode_zero(fixed_advert);
+
+ set = phy_lookup_setting(phydev->speed, phydev->duplex,
+ phydev->supported, true);
+ if (set)
+ linkmode_set(set->bit, fixed_advert);
+
+ advert = fixed_advert;
+ }
- err = genphy_config_advert(phydev);
+ err = genphy_config_advert(phydev, advert);
if (err < 0) /* error */
return err;
else if (err)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists