* [PATCH net-next] net: phylib: do not disable autoneg for fixed speeds >= 1G
@ 2024-08-07 10:31 Russell King (Oracle)
2024-08-07 20:36 ` kernel test robot
2024-08-07 20:46 ` kernel test robot
0 siblings, 2 replies; 3+ messages in thread
From: Russell King (Oracle) @ 2024-08-07 10:31 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
We have an increasing number of drivers that are forcing
auto-negotiation to be enabled for speeds of 1G or faster.
It would appear that auto-negotiation is mandatory for speeds above
100M. In 802.3, Annex 40C's state diagrams seems to imply that
mr_autoneg_enable (BMCR AN ENABLE) doesn't affect whether or not the
AN state machines work for 1000base-T, and some PHY datasheets (e.g.
Marvell Alaska) state that disabling mr_autoneg_enable leaves AN
enabled but forced to 1G full duplex.
Other PHY datasheets imply that BMCR AN ENABLE should not be cleared
for >= 1G.
Thus, this should be handled in phylib rather than in each driver.
Rather than erroring out, arrange to implement the Marvell Alaska
solution but in software for all PHYs: generate an appropriate
single-speed advertisement for the requested speed, and keep AN
enabled to the PHY driver. However, to avoid userspace API breakage,
continue to report to userspace that we have AN disabled.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phy_device.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 29a4225cb712..76312591dcb8 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2103,22 +2103,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,
@@ -2141,7 +2139,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,
@@ -2365,6 +2363,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);
@@ -2379,10 +2380,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)
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net-next] net: phylib: do not disable autoneg for fixed speeds >= 1G
2024-08-07 10:31 [PATCH net-next] net: phylib: do not disable autoneg for fixed speeds >= 1G Russell King (Oracle)
@ 2024-08-07 20:36 ` kernel test robot
2024-08-07 20:46 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2024-08-07 20:36 UTC (permalink / raw)
To: Russell King (Oracle), Andrew Lunn, Heiner Kallweit
Cc: llvm, oe-kbuild-all, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Hi Russell,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Russell-King-Oracle/net-phylib-do-not-disable-autoneg-for-fixed-speeds-1G/20240807-185909
base: net-next/main
patch link: https://lore.kernel.org/r/E1sbdxI-0024Eo-HE%40rmk-PC.armlinux.org.uk
patch subject: [PATCH net-next] net: phylib: do not disable autoneg for fixed speeds >= 1G
config: i386-buildonly-randconfig-004-20240808 (https://download.01.org/0day-ci/archive/20240808/202408080419.a2PXcqh8-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240808/202408080419.a2PXcqh8-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408080419.a2PXcqh8-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/net/phy/phy_device.c:2124:34: error: passing 'const unsigned long *' to parameter of type 'unsigned long *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
2124 | adv = linkmode_adv_to_mii_adv_t(advert);
| ^~~~~~
include/linux/mii.h:143:60: note: passing argument to parameter 'advertising' here
143 | static inline u32 linkmode_adv_to_mii_adv_t(unsigned long *advertising)
| ^
drivers/net/phy/phy_device.c:2147:39: error: passing 'const unsigned long *' to parameter of type 'unsigned long *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
2147 | adv = linkmode_adv_to_mii_ctrl1000_t(advert);
| ^~~~~~
include/linux/mii.h:218:65: note: passing argument to parameter 'advertising' here
218 | static inline u32 linkmode_adv_to_mii_ctrl1000_t(unsigned long *advertising)
| ^
>> drivers/net/phy/phy_device.c:2401:4: error: call to undeclared function 'linkmode_set'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
2401 | linkmode_set(set->bit, fixed_advert);
| ^
3 errors generated.
vim +2124 drivers/net/phy/phy_device.c
2107
2108 /**
2109 * genphy_config_advert - sanitize and advertise auto-negotiation parameters
2110 * @phydev: target phy_device struct
2111 * @advert: auto-negotiation parameters to advertise
2112 *
2113 * Description: Writes MII_ADVERTISE with the appropriate values,
2114 * after sanitizing the values to make sure we only advertise
2115 * what is supported. Returns < 0 on error, 0 if the PHY's advertisement
2116 * hasn't changed, and > 0 if it has changed.
2117 */
2118 static int genphy_config_advert(struct phy_device *phydev,
2119 const unsigned long *advert)
2120 {
2121 int err, bmsr, changed = 0;
2122 u32 adv;
2123
> 2124 adv = linkmode_adv_to_mii_adv_t(advert);
2125
2126 /* Setup standard advertisement */
2127 err = phy_modify_changed(phydev, MII_ADVERTISE,
2128 ADVERTISE_ALL | ADVERTISE_100BASE4 |
2129 ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM,
2130 adv);
2131 if (err < 0)
2132 return err;
2133 if (err > 0)
2134 changed = 1;
2135
2136 bmsr = phy_read(phydev, MII_BMSR);
2137 if (bmsr < 0)
2138 return bmsr;
2139
2140 /* Per 802.3-2008, Section 22.2.4.2.16 Extended status all
2141 * 1000Mbits/sec capable PHYs shall have the BMSR_ESTATEN bit set to a
2142 * logical 1.
2143 */
2144 if (!(bmsr & BMSR_ESTATEN))
2145 return changed;
2146
2147 adv = linkmode_adv_to_mii_ctrl1000_t(advert);
2148
2149 err = phy_modify_changed(phydev, MII_CTRL1000,
2150 ADVERTISE_1000FULL | ADVERTISE_1000HALF,
2151 adv);
2152 if (err < 0)
2153 return err;
2154 if (err > 0)
2155 changed = 1;
2156
2157 return changed;
2158 }
2159
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net-next] net: phylib: do not disable autoneg for fixed speeds >= 1G
2024-08-07 10:31 [PATCH net-next] net: phylib: do not disable autoneg for fixed speeds >= 1G Russell King (Oracle)
2024-08-07 20:36 ` kernel test robot
@ 2024-08-07 20:46 ` kernel test robot
1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2024-08-07 20:46 UTC (permalink / raw)
To: Russell King (Oracle), Andrew Lunn, Heiner Kallweit
Cc: oe-kbuild-all, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev
Hi Russell,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Russell-King-Oracle/net-phylib-do-not-disable-autoneg-for-fixed-speeds-1G/20240807-185909
base: net-next/main
patch link: https://lore.kernel.org/r/E1sbdxI-0024Eo-HE%40rmk-PC.armlinux.org.uk
patch subject: [PATCH net-next] net: phylib: do not disable autoneg for fixed speeds >= 1G
config: arc-randconfig-001-20240808 (https://download.01.org/0day-ci/archive/20240808/202408080457.vhF74DGf-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240808/202408080457.vhF74DGf-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408080457.vhF74DGf-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
drivers/net/phy/phy_device.c: In function 'genphy_config_advert':
>> drivers/net/phy/phy_device.c:2124:41: warning: passing argument 1 of 'linkmode_adv_to_mii_adv_t' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
2124 | adv = linkmode_adv_to_mii_adv_t(advert);
| ^~~~~~
In file included from include/uapi/linux/mdio.h:15,
from include/linux/mdio.h:9,
from drivers/net/phy/phy_device.c:23:
include/linux/mii.h:143:60: note: expected 'long unsigned int *' but argument is of type 'const long unsigned int *'
143 | static inline u32 linkmode_adv_to_mii_adv_t(unsigned long *advertising)
| ~~~~~~~~~~~~~~~^~~~~~~~~~~
>> drivers/net/phy/phy_device.c:2147:46: warning: passing argument 1 of 'linkmode_adv_to_mii_ctrl1000_t' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
2147 | adv = linkmode_adv_to_mii_ctrl1000_t(advert);
| ^~~~~~
include/linux/mii.h:218:65: note: expected 'long unsigned int *' but argument is of type 'const long unsigned int *'
218 | static inline u32 linkmode_adv_to_mii_ctrl1000_t(unsigned long *advertising)
| ~~~~~~~~~~~~~~~^~~~~~~~~~~
drivers/net/phy/phy_device.c: In function '__genphy_config_aneg':
>> drivers/net/phy/phy_device.c:2401:25: error: implicit declaration of function 'linkmode_set'; did you mean 'linkmode_subset'? [-Werror=implicit-function-declaration]
2401 | linkmode_set(set->bit, fixed_advert);
| ^~~~~~~~~~~~
| linkmode_subset
cc1: some warnings being treated as errors
vim +2401 drivers/net/phy/phy_device.c
2359
2360 /**
2361 * __genphy_config_aneg - restart auto-negotiation or write BMCR
2362 * @phydev: target phy_device struct
2363 * @changed: whether autoneg is requested
2364 *
2365 * Description: If auto-negotiation is enabled, we configure the
2366 * advertising, and then restart auto-negotiation. If it is not
2367 * enabled, then we write the BMCR.
2368 */
2369 int __genphy_config_aneg(struct phy_device *phydev, bool changed)
2370 {
2371 __ETHTOOL_DECLARE_LINK_MODE_MASK(fixed_advert);
2372 const struct phy_setting *set;
2373 unsigned long *advert;
2374 int err;
2375
2376 err = genphy_c45_an_config_eee_aneg(phydev);
2377 if (err < 0)
2378 return err;
2379 else if (err)
2380 changed = true;
2381
2382 err = genphy_setup_master_slave(phydev);
2383 if (err < 0)
2384 return err;
2385 else if (err)
2386 changed = true;
2387
2388 if (phydev->autoneg == AUTONEG_ENABLE) {
2389 /* Only allow advertising what this PHY supports */
2390 linkmode_and(phydev->advertising, phydev->advertising,
2391 phydev->supported);
2392 advert = phydev->advertising;
2393 } else if (phydev->speed < SPEED_1000) {
2394 return genphy_setup_forced(phydev);
2395 } else {
2396 linkmode_zero(fixed_advert);
2397
2398 set = phy_lookup_setting(phydev->speed, phydev->duplex,
2399 phydev->supported, true);
2400 if (set)
> 2401 linkmode_set(set->bit, fixed_advert);
2402
2403 advert = fixed_advert;
2404 }
2405
2406 err = genphy_config_advert(phydev, advert);
2407 if (err < 0) /* error */
2408 return err;
2409 else if (err)
2410 changed = true;
2411
2412 return genphy_check_and_restart_aneg(phydev, changed);
2413 }
2414 EXPORT_SYMBOL(__genphy_config_aneg);
2415
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-08-07 20:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-07 10:31 [PATCH net-next] net: phylib: do not disable autoneg for fixed speeds >= 1G Russell King (Oracle)
2024-08-07 20:36 ` kernel test robot
2024-08-07 20:46 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox