All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: Joe Hershberger <joe.hershberger@ni.com>,
	Ramon Fried <rfried.dev@gmail.com>,
	u-boot@lists.denx.de
Cc: Claudiu Manoil <claudiu.manoil@nxp.com>,
	Michael Walle <michael@walle.cc>,
	Priyanka Jain <priyanka.jain@nxp.com>
Subject: [PATCH v2 5/6] net: dsa: felix: call phy_config at .port_probe() time
Date: Tue, 24 Aug 2021 15:00:42 +0300	[thread overview]
Message-ID: <20210824120043.3823931-6-vladimir.oltean@nxp.com> (raw)
In-Reply-To: <20210824120043.3823931-1-vladimir.oltean@nxp.com>

It is an unfortunate reality that some PHY settings done by U-Boot
persist even after the PHY is reset and taken over by Linux, and even
more unfortunate that Linux has come to depend on things being set in a
certain way.

For example, on the NXP LS1028A-RDB, the felix switch ports are
connected to a VSC8514 QSGMII PHY. Between the switch port PCS and the
PHY, the U-Boot drivers enable in-band auto-negotiation which makes the
copper-side negotiated speed and duplex be transmitted from the PHY to
the MAC automatically.

The PHY driver portion that does this is in vsc8514_config():

	/* Enable Serdes Auto-negotiation */
	phy_write(phydev, MDIO_DEVAD_NONE, PHY_EXT_PAGE_ACCESS,
		  PHY_EXT_PAGE_ACCESS_EXTENDED3);
	val = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_VSC8514_MAC_SERDES_CON);
	val = val | MIIM_VSC8574_MAC_SERDES_ANEG;
	phy_write(phydev, MDIO_DEVAD_NONE, MIIM_VSC8514_MAC_SERDES_CON, val);

The point is that in-band autoneg should be turned on in both the PHY
and the MAC, or off in both the PHY and the MAC, otherwise the QSGMII
link will be broken.

And because phy_config() is currently called at .port_enable() time, the
result is that ports on which traffic has been sent in U-Boot will have
in-band autoneg enabled, and the rest won't.

It can be argued that the Linux kernel should not assume one way or
another and just reinitialize everything according to what it expects,
and that is completely fair. In fact, I've already started an attempt to
remove this dependency, although admittedly I am making slow progress at
it:
https://patchwork.kernel.org/project/netdevbpf/cover/20210212172341.3489046-1-olteanv@gmail.com/

Nonetheless, the sad reality is that NXP also has, apart from kernel
drivers, some user space networking (DPDK), and for some reason, the
expectation there is that somebody else initializes the PHYs. The kernel
can't do it because the device ownership doesn't belong to the kernel,
so what remains is for the bootloader to do it (especially since other
drivers generally call phy_config() at probe time). This is a really
weak guarantee that might break at any time, but apparently that is
enough for some.

Since initializing the ports and PHYs at probe time does not break
anything, we can just do that.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
Tested-by: Michael Walle <michael@walle.cc>
---
 drivers/net/mscc_eswitch/felix_switch.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/mscc_eswitch/felix_switch.c b/drivers/net/mscc_eswitch/felix_switch.c
index 75073880cf87..c8ecf4f19442 100644
--- a/drivers/net/mscc_eswitch/felix_switch.c
+++ b/drivers/net/mscc_eswitch/felix_switch.c
@@ -317,10 +317,23 @@ static int felix_probe(struct udevice *dev)
 	return 0;
 }
 
+static int felix_port_probe(struct udevice *dev, int port,
+			    struct phy_device *phy)
+{
+	int supported = PHY_GBIT_FEATURES | SUPPORTED_2500baseX_Full;
+	struct felix_priv *priv = dev_get_priv(dev);
+
+	phy->supported &= supported;
+	phy->advertising &= supported;
+
+	felix_start_pcs(dev, port, phy, &priv->imdio);
+
+	return phy_config(phy);
+}
+
 static int felix_port_enable(struct udevice *dev, int port,
 			     struct phy_device *phy)
 {
-	int supported = PHY_GBIT_FEATURES | SUPPORTED_2500baseX_Full;
 	struct felix_priv *priv = dev_get_priv(dev);
 	void *base = priv->regs_base;
 
@@ -339,12 +352,6 @@ static int felix_port_enable(struct udevice *dev, int port,
 		 FELIX_QSYS_SYSTEM_SW_PORT_LOSSY |
 		 FELIX_QSYS_SYSTEM_SW_PORT_SCH(1));
 
-	felix_start_pcs(dev, port, phy, &priv->imdio);
-
-	phy->supported &= supported;
-	phy->advertising &= supported;
-	phy_config(phy);
-
 	phy_startup(phy);
 
 	return 0;
@@ -392,6 +399,7 @@ static int felix_rcv(struct udevice *dev, int *pidx, void *packet, int length)
 }
 
 static const struct dsa_ops felix_dsa_ops = {
+	.port_probe	= felix_port_probe,
 	.port_enable	= felix_port_enable,
 	.port_disable	= felix_port_disable,
 	.xmit		= felix_xmit,
-- 
2.25.1


  parent reply	other threads:[~2021-08-24 12:02 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 12:00 [PATCH v2 0/6] Call phy_config at port probe time for the Felix DSA driver Vladimir Oltean
2021-08-24 12:00 ` [PATCH v2 1/6] net: dsa: felix: felix_init() can be static Vladimir Oltean
2021-09-14  9:23   ` Ramon Fried
2021-08-24 12:00 ` [PATCH v2 2/6] net: dsa: use "err" instead of "ret" in dsa_port_probe Vladimir Oltean
2021-09-14  9:23   ` Ramon Fried
2021-08-24 12:00 ` [PATCH v2 3/6] net: dsa: refactor the code to set the port MAC address into a dedicated function Vladimir Oltean
2021-09-14  9:23   ` Ramon Fried
2021-08-24 12:00 ` [PATCH v2 4/6] net: dsa: introduce a .port_probe() method in struct dsa_ops Vladimir Oltean
2021-09-14  9:24   ` Ramon Fried
2021-08-24 12:00 ` Vladimir Oltean [this message]
2021-08-24 12:00 ` [PATCH v2 6/6] net: dsa: felix: propagate the error code from phy_startup() Vladimir Oltean
2021-09-14  9:24   ` Ramon Fried
2021-08-25 11:08 ` [PATCH v2 0/6] Call phy_config at port probe time for the Felix DSA driver Vladimir Oltean

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210824120043.3823931-6-vladimir.oltean@nxp.com \
    --to=vladimir.oltean@nxp.com \
    --cc=claudiu.manoil@nxp.com \
    --cc=joe.hershberger@ni.com \
    --cc=michael@walle.cc \
    --cc=priyanka.jain@nxp.com \
    --cc=rfried.dev@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.