diff mbox series

[net-next,07/13] net: dsa: sja1105: always keep RGMII ports in the MAC role

Message ID 20210524232214.1378937-8-olteanv@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add NXP SJA1110 support to the sja1105 DSA driver | expand

Checks

Context Check Description
netdev/apply fail Patch does not apply to net-next
netdev/tree_selection success Clearly marked for net-next

Commit Message

Vladimir Oltean May 24, 2021, 11:22 p.m. UTC 
From: Vladimir Oltean <vladimir.oltean@nxp.com>

In SJA1105, the xMII Mode Parameters Table field called PHY_MAC denotes
the 'role' of the port, be it a PHY or a MAC. This makes a difference in
the MII and RMII protocols, but RGMII is symmetric, so either PHY or MAC
settings result in the same hardware behavior.

The SJA1110 is different, and the RGMII ports only work when configured
in MAC mode, so keep the port roles in MAC mode unconditionally.

Why we had an RGMII port in the PHY role in the first place was because
we wanted to have a way in the driver to denote whether RGMII delays
should be applied based on the phy-mode property or not. This is already
done in sja1105_parse_rgmii_delays() based on an intermediary
struct sja1105_dt_port (which contains the port role). So it is a
logical fallacy to use the hardware configuration as a scratchpad for
driver data, it isn't necessary.

We can also remove the gating condition for applying RGMII delays only
for ports in the PHY role. The .setup_rgmii_delay() method looks at
the priv->rgmii_rx_delay[port] and priv->rgmii_tx_delay[port] properties
which are already populated properly (in the case of a port in the MAC
role they are false). Removing this condition generates a few more SPI
writes for these ports (clearing the RGMII delays) which are perhaps
useless for SJA1105P/Q/R/S, where we know that the delays are disabled
by default. But for SJA1110, the firmware on the embedded microcontroller
might have done something funny, so it's always a good idea to clear the
RGMII delays if that's what Linux expects.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/sja1105/sja1105_clocking.c | 7 +------
 drivers/net/dsa/sja1105/sja1105_main.c     | 8 +++++++-
 2 files changed, 8 insertions(+), 7 deletions(-)

Comments

Florian Fainelli May 25, 2021, 2:24 a.m. UTC | #1 
On 5/24/2021 4:22 PM, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> In SJA1105, the xMII Mode Parameters Table field called PHY_MAC denotes
> the 'role' of the port, be it a PHY or a MAC. This makes a difference in
> the MII and RMII protocols, but RGMII is symmetric, so either PHY or MAC
> settings result in the same hardware behavior.
> 
> The SJA1110 is different, and the RGMII ports only work when configured
> in MAC mode, so keep the port roles in MAC mode unconditionally.
> 
> Why we had an RGMII port in the PHY role in the first place was because
> we wanted to have a way in the driver to denote whether RGMII delays
> should be applied based on the phy-mode property or not. This is already
> done in sja1105_parse_rgmii_delays() based on an intermediary
> struct sja1105_dt_port (which contains the port role). So it is a
> logical fallacy to use the hardware configuration as a scratchpad for
> driver data, it isn't necessary.
> 
> We can also remove the gating condition for applying RGMII delays only
> for ports in the PHY role. The .setup_rgmii_delay() method looks at
> the priv->rgmii_rx_delay[port] and priv->rgmii_tx_delay[port] properties
> which are already populated properly (in the case of a port in the MAC
> role they are false). Removing this condition generates a few more SPI
> writes for these ports (clearing the RGMII delays) which are perhaps
> useless for SJA1105P/Q/R/S, where we know that the delays are disabled
> by default. But for SJA1110, the firmware on the embedded microcontroller
> might have done something funny, so it's always a good idea to clear the
> RGMII delays if that's what Linux expects.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
diff mbox series

Patch

diff --git a/drivers/net/dsa/sja1105/sja1105_clocking.c b/drivers/net/dsa/sja1105/sja1105_clocking.c
index 03173397d950..ae297648611f 100644
--- a/drivers/net/dsa/sja1105/sja1105_clocking.c
+++ b/drivers/net/dsa/sja1105/sja1105_clocking.c
@@ -566,14 +566,9 @@  static int sja1105_rgmii_clocking_setup(struct sja1105_private *priv, int port,
 		dev_err(dev, "Failed to configure Tx pad registers\n");
 		return rc;
 	}
+
 	if (!priv->info->setup_rgmii_delay)
 		return 0;
-	/* The role has no hardware effect for RGMII. However we use it as
-	 * a proxy for this interface being a MAC-to-MAC connection, with
-	 * the RGMII internal delays needing to be applied by us.
-	 */
-	if (role == XMII_MAC)
-		return 0;
 
 	return priv->info->setup_rgmii_delay(priv, port);
 }
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index d3aa14d3a5c6..04af644bf656 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -218,8 +218,14 @@  static int sja1105_init_mii_settings(struct sja1105_private *priv,
 		/* Even though the SerDes port is able to drive SGMII autoneg
 		 * like a PHY would, from the perspective of the XMII tables,
 		 * the SGMII port should always be put in MAC mode.
+		 * Similarly, RGMII is a symmetric protocol electrically
+		 * speaking, and the 'RGMII PHY' role does not mean anything to
+		 * hardware. Just keep the 'PHY role' notation relevant to the
+		 * driver to mean 'the switch port should apply RGMII delays',
+		 * but unconditionally put the port in the MAC role.
 		 */
-		if (ports[i].phy_mode == PHY_INTERFACE_MODE_SGMII)
+		if (ports[i].phy_mode == PHY_INTERFACE_MODE_SGMII ||
+		    phy_interface_mode_is_rgmii(ports[i].phy_mode))
 			mii->phy_mac[i] = XMII_MAC;
 		else
 			mii->phy_mac[i] = ports[i].role;