| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <E1vEPlg-0000000CFHY-282A@rmk-PC.armlinux.org.uk>
Date: Thu, 30 Oct 2025 10:20:32 +0000
From: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>
To: Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>
Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>,
Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
linux-arm-kernel@...ts.infradead.org,
linux-arm-msm@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
netdev@...r.kernel.org,
Paolo Abeni <pabeni@...hat.com>,
Vinod Koul <vkoul@...nel.org>
Subject: [PATCH net-next v2] 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: Mohd Ayaan Anwar <mohd.anwar@....qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
---
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
Powered by blists - more mailing lists