netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: phy: mscc: Add auto-negotiation feature to VSC8514
@ 2025-02-13 10:21 Chintan Vankar
  2025-02-13 10:51 ` Russell King (Oracle)
  0 siblings, 1 reply; 2+ messages in thread
From: Chintan Vankar @ 2025-02-13 10:21 UTC (permalink / raw)
  To: Rosen Penev, Christophe JAILLET, Chintan Vankar, Paolo Abeni,
	Jakub Kicinski, Eric Dumazet, David S. Miller, Russell King,
	Heiner Kallweit, Andrew Lunn
  Cc: s-vadapalli, linux-kernel, netdev

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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] net: phy: mscc: Add auto-negotiation feature to VSC8514
  2025-02-13 10:21 [PATCH] net: phy: mscc: Add auto-negotiation feature to VSC8514 Chintan Vankar
@ 2025-02-13 10:51 ` Russell King (Oracle)
  0 siblings, 0 replies; 2+ messages in thread
From: Russell King (Oracle) @ 2025-02-13 10:51 UTC (permalink / raw)
  To: Chintan Vankar
  Cc: Rosen Penev, Christophe JAILLET, Paolo Abeni, Jakub Kicinski,
	Eric Dumazet, David S. Miller, Heiner Kallweit, Andrew Lunn,
	s-vadapalli, linux-kernel, netdev

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!

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-02-13 10:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-13 10:21 [PATCH] net: phy: mscc: Add auto-negotiation feature to VSC8514 Chintan Vankar
2025-02-13 10:51 ` Russell King (Oracle)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).