All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH net] net: phy: dp83869: fix status reporting for 1000base-x autonegotiation
@ 2024-10-31  1:37 kernel test robot
  0 siblings, 0 replies; 3+ messages in thread
From: kernel test robot @ 2024-10-31  1:37 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Dan Carpenter

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20241029-dp83869-1000base-x-v1-1-fcafe360bd98@bootlin.com>
References: <20241029-dp83869-1000base-x-v1-1-fcafe360bd98@bootlin.com>
TO: Romain Gantois <romain.gantois@bootlin.com>
TO: Andrew Lunn <andrew@lunn.ch>
TO: Heiner Kallweit <hkallweit1@gmail.com>
TO: Russell King <linux@armlinux.org.uk>
TO: "David S. Miller" <davem@davemloft.net>
CC: netdev@vger.kernel.org
TO: Eric Dumazet <edumazet@google.com>
TO: Jakub Kicinski <kuba@kernel.org>
TO: Paolo Abeni <pabeni@redhat.com>
TO: Dan Murphy <dmurphy@ti.com>
TO: Florian Fainelli <f.fainelli@gmail.com>
CC: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
CC: Maxime Chevallier <maxime.chevallier@bootlin.com>
CC: linux-kernel@vger.kernel.org
CC: stable@vger.kernel.org
CC: Romain Gantois <romain.gantois@bootlin.com>

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-dp83869-fix-status-reporting-for-1000base-x-autonegotiation/20241029-173146
base:   94c11e852955b2eef5c4f0b36cfeae7dcf11a759
patch link:    https://lore.kernel.org/r/20241029-dp83869-1000base-x-v1-1-fcafe360bd98%40bootlin.com
patch subject: [PATCH net] net: phy: dp83869: fix status reporting for 1000base-x autonegotiation
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
config: x86_64-randconfig-161-20241031 (https://download.01.org/0day-ci/archive/20241031/202410310949.CLirAHxH-lkp@intel.com/config)
compiler: clang version 19.1.2 (https://github.com/llvm/llvm-project 7ba7d8e2f7b6445b60679da826210cdde29eaf8b)

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>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202410310949.CLirAHxH-lkp@intel.com/

smatch warnings:
drivers/net/phy/dp83869.c:206 dp83869_config_aneg() error: uninitialized symbol 'adv'.

vim +/adv +206 drivers/net/phy/dp83869.c

01db923e83779b Dan Murphy     2019-11-13  168  
99f7e4c674a2fd Romain Gantois 2024-10-29  169  static int dp83869_config_aneg(struct phy_device *phydev)
99f7e4c674a2fd Romain Gantois 2024-10-29  170  {
99f7e4c674a2fd Romain Gantois 2024-10-29  171  	struct dp83869_private *dp83869 = phydev->priv;
99f7e4c674a2fd Romain Gantois 2024-10-29  172  	unsigned long *advertising;
99f7e4c674a2fd Romain Gantois 2024-10-29  173  	int err, changed = false;
99f7e4c674a2fd Romain Gantois 2024-10-29  174  	u32 adv;
99f7e4c674a2fd Romain Gantois 2024-10-29  175  
99f7e4c674a2fd Romain Gantois 2024-10-29  176  	if (dp83869->mode != DP83869_RGMII_1000_BASE)
99f7e4c674a2fd Romain Gantois 2024-10-29  177  		return genphy_config_aneg(phydev);
99f7e4c674a2fd Romain Gantois 2024-10-29  178  
99f7e4c674a2fd Romain Gantois 2024-10-29  179  	/* Forcing speed or duplex isn't supported in 1000base-x mode */
99f7e4c674a2fd Romain Gantois 2024-10-29  180  	if (phydev->autoneg != AUTONEG_ENABLE)
99f7e4c674a2fd Romain Gantois 2024-10-29  181  		return 0;
99f7e4c674a2fd Romain Gantois 2024-10-29  182  
99f7e4c674a2fd Romain Gantois 2024-10-29  183  	/* In fiber modes, register locations 0xc0... get mapped to offset 0.
99f7e4c674a2fd Romain Gantois 2024-10-29  184  	 * Unfortunately, the fiber-specific autonegotiation advertisement
99f7e4c674a2fd Romain Gantois 2024-10-29  185  	 * register at address 0xc04 does not have the same bit layout as the
99f7e4c674a2fd Romain Gantois 2024-10-29  186  	 * corresponding standard MII_ADVERTISE register. Thus, functions such
99f7e4c674a2fd Romain Gantois 2024-10-29  187  	 * as genphy_config_advert() will write the advertisement register
99f7e4c674a2fd Romain Gantois 2024-10-29  188  	 * incorrectly.
99f7e4c674a2fd Romain Gantois 2024-10-29  189  	 */
99f7e4c674a2fd Romain Gantois 2024-10-29  190  	advertising = phydev->advertising;
99f7e4c674a2fd Romain Gantois 2024-10-29  191  
99f7e4c674a2fd Romain Gantois 2024-10-29  192  	/* Only allow advertising what this PHY supports */
99f7e4c674a2fd Romain Gantois 2024-10-29  193  	linkmode_and(advertising, advertising,
99f7e4c674a2fd Romain Gantois 2024-10-29  194  		     phydev->supported);
99f7e4c674a2fd Romain Gantois 2024-10-29  195  
99f7e4c674a2fd Romain Gantois 2024-10-29  196  	if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT, advertising))
99f7e4c674a2fd Romain Gantois 2024-10-29  197  		adv |= DP83869_BP_FULL_DUPLEX;
99f7e4c674a2fd Romain Gantois 2024-10-29  198  	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT, advertising))
99f7e4c674a2fd Romain Gantois 2024-10-29  199  		adv |= DP83869_BP_PAUSE;
99f7e4c674a2fd Romain Gantois 2024-10-29  200  	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, advertising))
99f7e4c674a2fd Romain Gantois 2024-10-29  201  		adv |= DP83869_BP_ASYMMETRIC_PAUSE;
99f7e4c674a2fd Romain Gantois 2024-10-29  202  
99f7e4c674a2fd Romain Gantois 2024-10-29  203  	err = phy_modify_changed(phydev, DP83869_FX_ANADV,
99f7e4c674a2fd Romain Gantois 2024-10-29  204  				 DP83869_BP_FULL_DUPLEX | DP83869_BP_PAUSE |
99f7e4c674a2fd Romain Gantois 2024-10-29  205  				 DP83869_BP_ASYMMETRIC_PAUSE,
99f7e4c674a2fd Romain Gantois 2024-10-29 @206  				 adv);
99f7e4c674a2fd Romain Gantois 2024-10-29  207  
99f7e4c674a2fd Romain Gantois 2024-10-29  208  	if (err < 0)
99f7e4c674a2fd Romain Gantois 2024-10-29  209  		return err;
99f7e4c674a2fd Romain Gantois 2024-10-29  210  	else if (err)
99f7e4c674a2fd Romain Gantois 2024-10-29  211  		changed = true;
99f7e4c674a2fd Romain Gantois 2024-10-29  212  
99f7e4c674a2fd Romain Gantois 2024-10-29  213  	return genphy_check_and_restart_aneg(phydev, changed);
99f7e4c674a2fd Romain Gantois 2024-10-29  214  }
99f7e4c674a2fd Romain Gantois 2024-10-29  215  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 3+ messages in thread
* [PATCH net] net: phy: dp83869: fix status reporting for 1000base-x autonegotiation
@ 2024-10-29  9:30 Romain Gantois
  2024-10-29 17:57 ` kernel test robot
  0 siblings, 1 reply; 3+ messages in thread
From: Romain Gantois @ 2024-10-29  9:30 UTC (permalink / raw)
  To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Dan Murphy,
	Florian Fainelli
  Cc: Thomas Petazzoni, Maxime Chevallier, netdev, linux-kernel, stable,
	Romain Gantois

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,
-- 
Romain Gantois <romain.gantois@bootlin.com>


^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-10-31  1:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-31  1:37 [PATCH net] net: phy: dp83869: fix status reporting for 1000base-x autonegotiation kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2024-10-29  9:30 Romain Gantois
2024-10-29 17:57 ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.