Commit 9b443e58 authored by Russell King (Oracle)'s avatar Russell King (Oracle) Committed by Jakub Kicinski
Browse files

net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification 



When operating in "SGMII" mode (Cisco SGMII or 2500BASE-X), qcom-ethqos
modifies the MAC control register in its ethqos_configure_sgmii()
function, which is only called from one path:

stmmac_mac_link_up()
+- reads MAC_CTRL_REG
+- masks out priv->hw->link.speed_mask
+- sets bits according to speed (2500, 1000, 100, 10) from priv->hw.link.speed*
+- ethqos_fix_mac_speed()
|  +- qcom_ethqos_set_sgmii_loopback(false)
|  +- ethqos_update_link_clk(speed)
|  `- ethqos_configure(speed)
|     `- ethqos_configure_sgmii(speed)
|        +- reads MAC_CTRL_REG,
|        +- configures PS/FES bits according to speed
|        `- writes MAC_CTRL_REG as the last operation
+- sets duplex bit(s)
+- stmmac_mac_flow_ctrl()
+- writes MAC_CTRL_REG if changed from original read
...

As can be seen, the modification of the control register that
stmmac_mac_link_up() overwrites the changes that ethqos_fix_mac_speed()
does to the register. This makes ethqos_configure_sgmii()'s
modification questionable at best.

Analysing the values written, GMAC4 sets the speed bits as:
speed_mask = GMAC_CONFIG_FES | GMAC_CONFIG_PS
speed2500 = GMAC_CONFIG_FES                     B14=1 B15=0
speed1000 = 0                                   B14=0 B15=0
speed100 = GMAC_CONFIG_FES | GMAC_CONFIG_PS     B14=1 B15=1
speed10 = GMAC_CONFIG_PS                        B14=0 B15=1

Whereas ethqos_configure_sgmii():
2500: clears ETHQOS_MAC_CTRL_PORT_SEL           B14=X B15=0
1000: clears ETHQOS_MAC_CTRL_PORT_SEL           B14=X B15=0
100: sets ETHQOS_MAC_CTRL_PORT_SEL |            B14=1 B15=1
          ETHQOS_MAC_CTRL_SPEED_MODE
10: sets ETHQOS_MAC_CTRL_PORT_SEL               B14=0 B15=1
    clears ETHQOS_MAC_CTRL_SPEED_MODE

Thus, they appear to be doing very similar, with the exception of the
FES bit (bit 14) for 1G and 2.5G speeds.

Given that stmmac_mac_link_up() will write the MAC_CTRL_REG after
ethqos_configure_sgmii(), remove the unnecessary update in the
glue driver's ethqos_configure_sgmii() method, simplifying the code.

Konrad states:

Without any additional knowledge, the register description says:

2500: B14=1 B15=0
1000: B14=0 B15=0
 100: B14=1 B15=1
  10: B14=0 B15=1

Tested-by: default avatarMohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>
Reviewed-by: default avatarKonrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
Link: https://patch.msgid.link/E1vEPlg-0000000CFHY-282A@rmk-PC.armlinux.org.uk


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent ab5b7680

drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c

+1 −15
Original line number Diff line number Diff line
@@ -76,10 +76,6 @@ 
#define RGMII_CONFIG2_DATA_DIVIDE_CLK_SEL	BIT(6)

#define RGMII_CONFIG2_TX_CLK_PHASE_SHIFT_EN	BIT(5)



/* MAC_CTRL_REG bits */

#define ETHQOS_MAC_CTRL_SPEED_MODE		BIT(14)

#define ETHQOS_MAC_CTRL_PORT_SEL		BIT(15)



/* EMAC_WRAPPER_SGMII_PHY_CNTRL1 bits */

#define SGMII_PHY_CNTRL1_SGMII_TX_TO_RX_LOOPBACK_EN	BIT(3)
@@ -632,13 +628,9 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos, int speed) 
{

	struct net_device *dev = platform_get_drvdata(ethqos->pdev);

	struct stmmac_priv *priv = netdev_priv(dev);

	int val;



	val = readl(ethqos->mac_base + MAC_CTRL_REG);



	switch (speed) {

	case SPEED_2500:

		val &= ~ETHQOS_MAC_CTRL_PORT_SEL;

		rgmii_updatel(ethqos, RGMII_CONFIG2_RGMII_CLK_SEL_CFG,

			      RGMII_CONFIG2_RGMII_CLK_SEL_CFG,

			      RGMII_IO_MACRO_CONFIG2);
@@ -646,7 +638,6 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos, int speed) 
		ethqos_pcs_set_inband(priv, false);

		break;

	case SPEED_1000:

		val &= ~ETHQOS_MAC_CTRL_PORT_SEL;

		rgmii_updatel(ethqos, RGMII_CONFIG2_RGMII_CLK_SEL_CFG,

			      RGMII_CONFIG2_RGMII_CLK_SEL_CFG,

			      RGMII_IO_MACRO_CONFIG2);
@@ -654,13 +645,10 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos, int speed) 
		ethqos_pcs_set_inband(priv, true);

		break;

	case SPEED_100:

		val |= ETHQOS_MAC_CTRL_PORT_SEL | ETHQOS_MAC_CTRL_SPEED_MODE;

		ethqos_set_serdes_speed(ethqos, SPEED_1000);

		ethqos_pcs_set_inband(priv, true);

		break;

	case SPEED_10:

		val |= ETHQOS_MAC_CTRL_PORT_SEL;

		val &= ~ETHQOS_MAC_CTRL_SPEED_MODE;

		rgmii_updatel(ethqos, RGMII_CONFIG_SGMII_CLK_DVDR,

			      FIELD_PREP(RGMII_CONFIG_SGMII_CLK_DVDR,

					 SGMII_10M_RX_CLK_DVDR),
@@ -670,9 +658,7 @@ static int ethqos_configure_sgmii(struct qcom_ethqos *ethqos, int speed) 
		break;

	}



	writel(val, ethqos->mac_base + MAC_CTRL_REG);



	return val;

	return 0;

}



static int ethqos_configure(struct qcom_ethqos *ethqos, int speed)