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: Mohd Ayaan Anwar Reviewed-by: Konrad Dybcio Signed-off-by: Russell King (Oracle) --- v2: added tested-by/reviewed-by, added additional comments from Konrad w.r.t. the register description to commit description (which is the reason for resending.) .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c index d1e48b524d7a..1a616a71c36a 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c @@ -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) -- 2.47.3