The DP83869 PHY transceiver supports converting from RGMII to 1000base-x. In this operation mode, autonegotiation can be performed, as described in IEEE802.3.
The DP83869 has a set of fiber-specific registers located at offset 0xc00. When the transceiver is configured in RGMII-to-1000base-x mode, these registers are mapped onto offset 0, which should, in theory, make reading the autonegotiation status transparent.
However, the fiber registers at offset 0xc04 and 0xc05 do not follow the bit layout of their standard counterparts. Thus, genphy_read_status() doesn't properly read the capabilities advertised by the link partner, resulting in incorrect link parameters.
Similarly, genphy_config_aneg() doesn't properly write advertised capabilities.
Fix the 1000base-x autonegotiation procedure by replacing genphy_read_status() and genphy_config_aneg() with driver-specific functions which take into account the nonstandard bit layout of the DP83869 registers in 1000base-x mode.
Fixes: a29de52ba2a1 ("net: dp83869: Add ability to advertise Fiber connection") Cc: stable@vger.kernel.org Signed-off-by: Romain Gantois romain.gantois@bootlin.com --- drivers/net/phy/dp83869.c | 130 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 127 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c index 5f056d7db83eed23f1cab42365fdc566a0d8e47f..7f89a4f963cab50d6954e8b8996d7bbe2c72a9ca 100644 --- a/drivers/net/phy/dp83869.c +++ b/drivers/net/phy/dp83869.c @@ -41,6 +41,8 @@ #define DP83869_IO_MUX_CFG 0x0170 #define DP83869_OP_MODE 0x01df #define DP83869_FX_CTRL 0x0c00 +#define DP83869_FX_ANADV 0x0c04 +#define DP83869_FX_LPABL 0x0c05
#define DP83869_SW_RESET BIT(15) #define DP83869_SW_RESTART BIT(14) @@ -135,6 +137,17 @@ #define DP83869_DOWNSHIFT_4_COUNT 4 #define DP83869_DOWNSHIFT_8_COUNT 8
+/* FX_ANADV bits */ +#define DP83869_BP_FULL_DUPLEX BIT(5) +#define DP83869_BP_PAUSE BIT(7) +#define DP83869_BP_ASYMMETRIC_PAUSE BIT(8) + +/* FX_LPABL bits */ +#define DP83869_LPA_1000FULL BIT(5) +#define DP83869_LPA_PAUSE_CAP BIT(7) +#define DP83869_LPA_PAUSE_ASYM BIT(8) +#define DP83869_LPA_LPACK BIT(14) + enum { DP83869_PORT_MIRRORING_KEEP, DP83869_PORT_MIRRORING_EN, @@ -153,19 +166,129 @@ struct dp83869_private { int mode; };
+static int dp83869_config_aneg(struct phy_device *phydev) +{ + struct dp83869_private *dp83869 = phydev->priv; + unsigned long *advertising; + int err, changed = false; + u32 adv; + + if (dp83869->mode != DP83869_RGMII_1000_BASE) + return genphy_config_aneg(phydev); + + /* Forcing speed or duplex isn't supported in 1000base-x mode */ + if (phydev->autoneg != AUTONEG_ENABLE) + return 0; + + /* In fiber modes, register locations 0xc0... get mapped to offset 0. + * Unfortunately, the fiber-specific autonegotiation advertisement + * register at address 0xc04 does not have the same bit layout as the + * corresponding standard MII_ADVERTISE register. Thus, functions such + * as genphy_config_advert() will write the advertisement register + * incorrectly. + */ + advertising = phydev->advertising; + + /* Only allow advertising what this PHY supports */ + linkmode_and(advertising, advertising, + phydev->supported); + + if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, advertising)) + adv |= DP83869_BP_FULL_DUPLEX; + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT, advertising)) + adv |= DP83869_BP_PAUSE; + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, advertising)) + adv |= DP83869_BP_ASYMMETRIC_PAUSE; + + err = phy_modify_changed(phydev, DP83869_FX_ANADV, + DP83869_BP_FULL_DUPLEX | DP83869_BP_PAUSE | + DP83869_BP_ASYMMETRIC_PAUSE, + adv); + + if (err < 0) + return err; + else if (err) + changed = true; + + return genphy_check_and_restart_aneg(phydev, changed); +} + +static int dp83869_read_status_fiber(struct phy_device *phydev) +{ + int err, lpa, old_link = phydev->link; + unsigned long *lp_advertising; + + err = genphy_update_link(phydev); + if (err) + return err; + + if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link) + return 0; + + phydev->speed = SPEED_UNKNOWN; + phydev->duplex = DUPLEX_UNKNOWN; + phydev->pause = 0; + phydev->asym_pause = 0; + + lp_advertising = phydev->lp_advertising; + + if (phydev->autoneg != AUTONEG_ENABLE) { + linkmode_zero(lp_advertising); + + phydev->duplex = DUPLEX_FULL; + phydev->speed = SPEED_1000; + + return 0; + } + + if (!phydev->autoneg_complete) { + linkmode_zero(lp_advertising); + return 0; + } + + /* In fiber modes, register locations 0xc0... get mapped to offset 0. + * Unfortunately, the fiber-specific link partner capabilities register + * at address 0xc05 does not have the same bit layout as the + * corresponding standard MII_LPA register. Thus, functions such as + * genphy_read_lpa() will read autonegotiation results incorrectly. + */ + + lpa = phy_read(phydev, DP83869_FX_LPABL); + if (lpa < 0) + return lpa; + + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, + lp_advertising, lpa & DP83869_LPA_1000FULL); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT, lp_advertising, + lpa & DP83869_LPA_PAUSE_CAP); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, lp_advertising, + lpa & DP83869_LPA_PAUSE_ASYM); + + linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, + lp_advertising, lpa & DP83869_LPA_LPACK); + + phy_resolve_aneg_linkmode(phydev); + + return 0; +} + static int dp83869_read_status(struct phy_device *phydev) { struct dp83869_private *dp83869 = phydev->priv; int ret;
+ if (dp83869->mode == DP83869_RGMII_1000_BASE) + return dp83869_read_status_fiber(phydev); + ret = genphy_read_status(phydev); if (ret) return ret;
- if (linkmode_test_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, phydev->supported)) { + if (dp83869->mode == DP83869_RGMII_100_BASE) { if (phydev->link) { - if (dp83869->mode == DP83869_RGMII_100_BASE) - phydev->speed = SPEED_100; + phydev->speed = SPEED_100; } else { phydev->speed = SPEED_UNKNOWN; phydev->duplex = DUPLEX_UNKNOWN; @@ -898,6 +1021,7 @@ static int dp83869_phy_reset(struct phy_device *phydev) .soft_reset = dp83869_phy_reset, \ .config_intr = dp83869_config_intr, \ .handle_interrupt = dp83869_handle_interrupt, \ + .config_aneg = dp83869_config_aneg, \ .read_status = dp83869_read_status, \ .get_tunable = dp83869_get_tunable, \ .set_tunable = dp83869_set_tunable, \
--- base-commit: 94c11e852955b2eef5c4f0b36cfeae7dcf11a759 change-id: 20241025-dp83869-1000base-x-0f0a61725784
Best regards,
Hi Romain,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 94c11e852955b2eef5c4f0b36cfeae7dcf11a759]
url: https://github.com/intel-lab-lkp/linux/commits/Romain-Gantois/net-phy-dp8386... base: 94c11e852955b2eef5c4f0b36cfeae7dcf11a759 patch link: https://lore.kernel.org/r/20241029-dp83869-1000base-x-v1-1-fcafe360bd98%40bo... patch subject: [PATCH net] net: phy: dp83869: fix status reporting for 1000base-x autonegotiation config: arm-randconfig-004-20241029 (https://download.01.org/0day-ci/archive/20241030/202410300125.K125vk3f-lkp@i...) compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241030/202410300125.K125vk3f-lkp@i...)
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/202410300125.K125vk3f-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/net/phy/dp83869.c:197:3: warning: variable 'adv' is uninitialized when used here [-Wuninitialized]
adv |= DP83869_BP_FULL_DUPLEX; ^~~ drivers/net/phy/dp83869.c:174:9: note: initialize the variable 'adv' to silence this warning u32 adv; ^ = 0 1 warning generated.
vim +/adv +197 drivers/net/phy/dp83869.c
168 169 static int dp83869_config_aneg(struct phy_device *phydev) 170 { 171 struct dp83869_private *dp83869 = phydev->priv; 172 unsigned long *advertising; 173 int err, changed = false; 174 u32 adv; 175 176 if (dp83869->mode != DP83869_RGMII_1000_BASE) 177 return genphy_config_aneg(phydev); 178 179 /* Forcing speed or duplex isn't supported in 1000base-x mode */ 180 if (phydev->autoneg != AUTONEG_ENABLE) 181 return 0; 182 183 /* In fiber modes, register locations 0xc0... get mapped to offset 0. 184 * Unfortunately, the fiber-specific autonegotiation advertisement 185 * register at address 0xc04 does not have the same bit layout as the 186 * corresponding standard MII_ADVERTISE register. Thus, functions such 187 * as genphy_config_advert() will write the advertisement register 188 * incorrectly. 189 */ 190 advertising = phydev->advertising; 191 192 /* Only allow advertising what this PHY supports */ 193 linkmode_and(advertising, advertising, 194 phydev->supported); 195 196 if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, advertising))
197 adv |= DP83869_BP_FULL_DUPLEX;
198 if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT, advertising)) 199 adv |= DP83869_BP_PAUSE; 200 if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, advertising)) 201 adv |= DP83869_BP_ASYMMETRIC_PAUSE; 202 203 err = phy_modify_changed(phydev, DP83869_FX_ANADV, 204 DP83869_BP_FULL_DUPLEX | DP83869_BP_PAUSE | 205 DP83869_BP_ASYMMETRIC_PAUSE, 206 adv); 207 208 if (err < 0) 209 return err; 210 else if (err) 211 changed = true; 212 213 return genphy_check_and_restart_aneg(phydev, changed); 214 } 215
linux-stable-mirror@lists.linaro.org