[PATCH] net: phy: mscc: Add auto-negotiation feature to VSC8514

Chintan Vankar posted 1 patch 10 months, 1 week ago
drivers/net/phy/mscc/mscc.h      |  2 ++
drivers/net/phy/mscc/mscc_main.c | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+)
[PATCH] net: phy: mscc: Add auto-negotiation feature to VSC8514
Posted by Chintan Vankar 10 months, 1 week ago
Add function vsc85xx_config_inband_aneg() in mscc_main.c to enable
auto-negotiation. The function enables auto-negotiation by configuring
the MAC SerDes PCS Control register of VSC8514.

Invoke the vsc85xx_config_inband_aneg() function from the
vsc8514_config_init() function present in mscc_main.c to start the
auto-negotiation process. This is required to get Ethernet working with
the Quad port Ethernet Add-On card connected to J7 common processor board.

Signed-off-by: Chintan Vankar <c-vankar@ti.com>
---

This patch is based on commit '7b7a883c7f4d' of linux-next branch of
Mainline Linux.

Regards,
Chintan.

 drivers/net/phy/mscc/mscc.h      |  2 ++
 drivers/net/phy/mscc/mscc_main.c | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index 6a3d8a754eb8..3baa0a418bae 100644
--- a/drivers/net/phy/mscc/mscc.h
+++ b/drivers/net/phy/mscc/mscc.h
@@ -196,6 +196,8 @@ enum rgmii_clock_delay {
 #define MSCC_PHY_EXTENDED_INT_MS_EGR	  BIT(9)
 
 /* Extended Page 3 Registers */
+#define MSCC_PHY_SERDES_PCS_CTRL          16
+#define MSCC_PHY_SERDES_ANEG              BIT(7)
 #define MSCC_PHY_SERDES_TX_VALID_CNT	  21
 #define MSCC_PHY_SERDES_TX_CRC_ERR_CNT	  22
 #define MSCC_PHY_SERDES_RX_VALID_CNT	  28
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 19cf12ee8990..f1f36ee1cc59 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1699,6 +1699,21 @@ static int vsc8574_config_host_serdes(struct phy_device *phydev)
 			   PROC_CMD_RST_CONF_PORT | PROC_CMD_FIBER_1000BASE_X);
 }
 
+static int vsc85xx_config_inband_aneg(struct phy_device *phydev, bool enabled)
+{
+	u16 reg_val = 0;
+	int rc;
+
+	if (enabled)
+		reg_val = MSCC_PHY_SERDES_ANEG;
+
+	rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_3,
+			      MSCC_PHY_SERDES_PCS_CTRL, MSCC_PHY_SERDES_ANEG,
+			      reg_val);
+
+	return rc;
+}
+
 static int vsc8584_config_init(struct phy_device *phydev)
 {
 	struct vsc8531_private *vsc8531 = phydev->priv;
@@ -2107,6 +2122,11 @@ static int vsc8514_config_init(struct phy_device *phydev)
 
 	ret = genphy_soft_reset(phydev);
 
+	if (ret)
+		return ret;
+
+	ret = vsc85xx_config_inband_aneg(phydev, true);
+
 	if (ret)
 		return ret;
 
-- 
2.34.1
Re: [PATCH] net: phy: mscc: Add auto-negotiation feature to VSC8514
Posted by Russell King (Oracle) 10 months, 1 week ago
On Thu, Feb 13, 2025 at 03:51:50PM +0530, Chintan Vankar wrote:
> Add function vsc85xx_config_inband_aneg() in mscc_main.c to enable
> auto-negotiation. The function enables auto-negotiation by configuring
> the MAC SerDes PCS Control register of VSC8514.
> 
> Invoke the vsc85xx_config_inband_aneg() function from the
> vsc8514_config_init() function present in mscc_main.c to start the
> auto-negotiation process. This is required to get Ethernet working with
> the Quad port Ethernet Add-On card connected to J7 common processor board.

A few points:

1. please read the netdev FAQ:
   https://www.kernel.org/doc/html/v5.6/networking/netdev-FAQ.html
   specifically the first question. Please also note the delay
   requirement for resending patches.

2. Is this a fix? Does something not work at the moment?

3. Will always forcing the inband signalling to be enabled result in
   another existing user that doesn't provide the inband signalling
   now failing? Do you know for sure that this won't disrupt any other
   users of this PHY?

4. Maybe consider using phylink in the ethernet driver and populating
   the .inband_caps() and .config_inband() methods which will allow
   the inband signalling properties to be negotiated between the MAC's
   PCS and the PHY.

Other points inline below:

> +static int vsc85xx_config_inband_aneg(struct phy_device *phydev, bool enabled)
> +{
> +	u16 reg_val = 0;
> +	int rc;
> +
> +	if (enabled)
> +		reg_val = MSCC_PHY_SERDES_ANEG;
> +
> +	rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_3,
> +			      MSCC_PHY_SERDES_PCS_CTRL, MSCC_PHY_SERDES_ANEG,
> +			      reg_val);
> +
> +	return rc;

Why not simply:

	return phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_3,
				MSCC_PHY_SERDES_PCS_CTRL,
				MSCC_PHY_SERDES_ANEG,
				reg_val);

?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!