* [PATCH net-next] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification
@ 2025-10-29 10:18 Russell King (Oracle)
2025-10-30 9:56 ` Mohd Ayaan Anwar
2025-10-30 10:01 ` Konrad Dybcio
0 siblings, 2 replies; 8+ messages in thread
From: Russell King (Oracle) @ 2025-10-29 10:18 UTC (permalink / raw)
To: Mohd Ayaan Anwar
Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-arm-kernel, linux-arm-msm, linux-stm32,
Maxime Coquelin, netdev, Paolo Abeni, Vinod Koul
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.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
Ayaan, please can you also test this patch? I believe that this
code is unnecessary as per the analysis in the commit message.
Thanks.
.../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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH net-next] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification
2025-10-29 10:18 [PATCH net-next] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification Russell King (Oracle)
@ 2025-10-30 9:56 ` Mohd Ayaan Anwar
2025-10-30 10:01 ` Konrad Dybcio
1 sibling, 0 replies; 8+ messages in thread
From: Mohd Ayaan Anwar @ 2025-10-30 9:56 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-arm-kernel, linux-arm-msm, linux-stm32,
Maxime Coquelin, netdev, Paolo Abeni, Vinod Koul
On Wed, Oct 29, 2025 at 10:18:36AM +0000, Russell King (Oracle) wrote:
> 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.
Makes sense.
>
> 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.
>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
> Ayaan, please can you also test this patch? I believe that this
> code is unnecessary as per the analysis in the commit message.
> Thanks.
>
> .../ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 16 +---------------
> 1 file changed, 1 insertion(+), 15 deletions(-)
>
Tested on top of net-next on the Qualcomm QCS9100 Ride R3 board and
found no issues, so:
Tested-by: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification
2025-10-29 10:18 [PATCH net-next] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification Russell King (Oracle)
2025-10-30 9:56 ` Mohd Ayaan Anwar
@ 2025-10-30 10:01 ` Konrad Dybcio
2025-10-30 10:16 ` Russell King (Oracle)
2025-10-30 12:17 ` Russell King (Oracle)
1 sibling, 2 replies; 8+ messages in thread
From: Konrad Dybcio @ 2025-10-30 10:01 UTC (permalink / raw)
To: Russell King (Oracle), Mohd Ayaan Anwar
Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-arm-kernel, linux-arm-msm, linux-stm32,
Maxime Coquelin, netdev, Paolo Abeni, Vinod Koul
On 10/29/25 11:18 AM, Russell King (Oracle) wrote:
> 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.
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
(so the current state of this driver)
and it indeed seems to match the values set in dwmac4_setup()
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification
2025-10-30 10:01 ` Konrad Dybcio
@ 2025-10-30 10:16 ` Russell King (Oracle)
2025-10-30 12:17 ` Russell King (Oracle)
1 sibling, 0 replies; 8+ messages in thread
From: Russell King (Oracle) @ 2025-10-30 10:16 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Mohd Ayaan Anwar, Alexandre Torgue, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, linux-arm-kernel, linux-arm-msm,
linux-stm32, Maxime Coquelin, netdev, Paolo Abeni, Vinod Koul
On Thu, Oct 30, 2025 at 11:01:48AM +0100, Konrad Dybcio wrote:
> On 10/29/25 11:18 AM, Russell King (Oracle) wrote:
> > 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.
>
> 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
>
> (so the current state of this driver)
>
> and it indeed seems to match the values set in dwmac4_setup()
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Thanks Konrad. I've included an extract from your reply in the commit
description as well, thanks for referring back to the documentation.
Note that B14 is not explicitly configured by ethqos_configure_sgmii()
for 1G and 2.5G speeds which is a harmless bug in this glue driver,
and an additional reason to get rid of this code!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification
2025-10-30 10:01 ` Konrad Dybcio
2025-10-30 10:16 ` Russell King (Oracle)
@ 2025-10-30 12:17 ` Russell King (Oracle)
2025-10-30 13:08 ` Konrad Dybcio
1 sibling, 1 reply; 8+ messages in thread
From: Russell King (Oracle) @ 2025-10-30 12:17 UTC (permalink / raw)
To: Konrad Dybcio, Mohd Ayaan Anwar
Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-arm-kernel, linux-arm-msm, linux-stm32,
Maxime Coquelin, netdev, Paolo Abeni, Vinod Koul
Konrad, Ayaan,
Can you shed any light on the manipulation of the RGMII_IO_MACRO_CONFIG
and RGMII_IO_MACRO_CONFIG2 registers in ethqos_configure_sgmii()?
Specifically:
- why would RGMII_CONFIG2_RGMII_CLK_SEL_CFG be set for 2.5G and 1G
speeds, but never be cleared for any other speed?
- why is RGMII_CONFIG_SGMII_CLK_DVDR set to SGMII_10M_RX_CLK_DVDR
for 10M, but never set to any other value for other speeds?
To me, this code looks very suspicious.
If you have time, please test with a connection capable of 1000BASE-T,
100BASE-TX and 10BASE-T, modifying the advertisement to make it
negotiate each of these, and checking that packet transfer is still
possible.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification
2025-10-30 12:17 ` Russell King (Oracle)
@ 2025-10-30 13:08 ` Konrad Dybcio
2025-10-30 14:05 ` Russell King (Oracle)
0 siblings, 1 reply; 8+ messages in thread
From: Konrad Dybcio @ 2025-10-30 13:08 UTC (permalink / raw)
To: Russell King (Oracle), Mohd Ayaan Anwar
Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-arm-kernel, linux-arm-msm, linux-stm32,
Maxime Coquelin, netdev, Paolo Abeni, Vinod Koul
On 10/30/25 1:17 PM, Russell King (Oracle) wrote:
> Konrad, Ayaan,
>
> Can you shed any light on the manipulation of the RGMII_IO_MACRO_CONFIG
> and RGMII_IO_MACRO_CONFIG2 registers in ethqos_configure_sgmii()?
>
> Specifically:
> - why would RGMII_CONFIG2_RGMII_CLK_SEL_CFG be set for 2.5G and 1G
> speeds, but never be cleared for any other speed?
BIT(16) - "enable to transmit delayed clock in RGMII 100/10 ID Mode"
> - why is RGMII_CONFIG_SGMII_CLK_DVDR set to SGMII_10M_RX_CLK_DVDR
> for 10M, but never set to any other value for other speeds?
[18:10] - In short, it configures a divider. The expected value is 0x13
for 10 Mbps / RMII mode
which seems to have been problematic given:
https://lore.kernel.org/all/20231212092208.22393-1-quic_snehshah@quicinc.com/
But it didn't say what hardware had this issue.. whether it concerns a
specific SoC or all of them..
A programming guide mentions the new 0x31 value for 10 Mbps in a
SoC-common paragraph so I suppose it's indeed better-er.. Perhaps issues
could arise if you switch back to a faster mode?
> To me, this code looks very suspicious.
>
> If you have time, please test with a connection capable of 1000BASE-T,
> 100BASE-TX and 10BASE-T, modifying the advertisement to make it
> negotiate each of these, and checking that packet transfer is still
> possible.
No HW with an ethernet port at hand, sorry
Konrad
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification
2025-10-30 13:08 ` Konrad Dybcio
@ 2025-10-30 14:05 ` Russell King (Oracle)
2025-11-07 11:34 ` Konrad Dybcio
0 siblings, 1 reply; 8+ messages in thread
From: Russell King (Oracle) @ 2025-10-30 14:05 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Mohd Ayaan Anwar, Alexandre Torgue, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, linux-arm-kernel, linux-arm-msm,
linux-stm32, Maxime Coquelin, netdev, Paolo Abeni, Vinod Koul
On Thu, Oct 30, 2025 at 02:08:41PM +0100, Konrad Dybcio wrote:
> On 10/30/25 1:17 PM, Russell King (Oracle) wrote:
> > Konrad, Ayaan,
> >
> > Can you shed any light on the manipulation of the RGMII_IO_MACRO_CONFIG
> > and RGMII_IO_MACRO_CONFIG2 registers in ethqos_configure_sgmii()?
> >
> > Specifically:
> > - why would RGMII_CONFIG2_RGMII_CLK_SEL_CFG be set for 2.5G and 1G
> > speeds, but never be cleared for any other speed?
>
> BIT(16) - "enable to transmit delayed clock in RGMII 100/10 ID Mode"
I guess that means that changing this bit is not relevant for the SGMII
path, and thus can be removed:
switch (speed) {
case SPEED_2500:
- rgmii_updatel(ethqos, RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
- RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
- RGMII_IO_MACRO_CONFIG2);
ethqos_set_serdes_speed(ethqos, SPEED_2500);
ethqos_pcs_set_inband(priv, false);
break;
case SPEED_1000:
- rgmii_updatel(ethqos, RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
- RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
- RGMII_IO_MACRO_CONFIG2);
ethqos_set_serdes_speed(ethqos, SPEED_1000);
ethqos_pcs_set_inband(priv, true);
> > - why is RGMII_CONFIG_SGMII_CLK_DVDR set to SGMII_10M_RX_CLK_DVDR
> > for 10M, but never set to any other value for other speeds?
>
> [18:10] - In short, it configures a divider. The expected value is 0x13
> for 10 Mbps / RMII mode
This gets confusing. Is the "/" meaning "10Mbps in RMII mode" or "10Mbps
or RMII mode".
> which seems to have been problematic given:
>
> https://lore.kernel.org/all/20231212092208.22393-1-quic_snehshah@quicinc.com/
>
> But it didn't say what hardware had this issue.. whether it concerns a
> specific SoC or all of them..
>
> A programming guide mentions the new 0x31 value for 10 Mbps in a
> SoC-common paragraph so I suppose it's indeed better-er.. Perhaps issues
> could arise if you switch back to a faster mode?
Could the 0x13 be a typo? Its suspicious that the two values are 0x13
vs 0x31. 0x13 = 19 vs 0x31 = 49. 0x31 makes more sense than 19.
The platform glue is required to supply clk_rx_i to the dwmac's MAC
receive path, deriving it from the 125MHz SGMII rx link clock divided
by 1, 5 or 50. Normally, this would be done by hardware signals output
from the dwmac.
This suggests that the value programmed is one less than the actual
divisor.
There's two possibilities why this value needs to be programmed:
1. the hardware doesn't divide the SGMII rx link clock according to
the hardware signals output from the dwmac, and needs the divisor to
be manually programmed. This would require the divisor to also be
programmed to 4 for 100M (but the driver doesn't do this.)
2. the hardware selects the clk_rx_i depending on the hardware
signals, and while 1G and 100M use a fixed divisor of 1 and 5, the
10M divisor needs to be manually programmed.
Any ideas what's really going on here?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net-next] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification
2025-10-30 14:05 ` Russell King (Oracle)
@ 2025-11-07 11:34 ` Konrad Dybcio
0 siblings, 0 replies; 8+ messages in thread
From: Konrad Dybcio @ 2025-11-07 11:34 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Mohd Ayaan Anwar, Alexandre Torgue, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, linux-arm-kernel, linux-arm-msm,
linux-stm32, Maxime Coquelin, netdev, Paolo Abeni, Vinod Koul
On 10/30/25 3:05 PM, Russell King (Oracle) wrote:
> On Thu, Oct 30, 2025 at 02:08:41PM +0100, Konrad Dybcio wrote:
>> On 10/30/25 1:17 PM, Russell King (Oracle) wrote:
>>> Konrad, Ayaan,
>>>
>>> Can you shed any light on the manipulation of the RGMII_IO_MACRO_CONFIG
>>> and RGMII_IO_MACRO_CONFIG2 registers in ethqos_configure_sgmii()?
>>>
>>> Specifically:
>>> - why would RGMII_CONFIG2_RGMII_CLK_SEL_CFG be set for 2.5G and 1G
>>> speeds, but never be cleared for any other speed?
>>
>> BIT(16) - "enable to transmit delayed clock in RGMII 100/10 ID Mode"
>
> I guess that means that changing this bit is not relevant for the SGMII
> path, and thus can be removed:
>
> switch (speed) {
> case SPEED_2500:
> - rgmii_updatel(ethqos, RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
> - RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
> - RGMII_IO_MACRO_CONFIG2);
> ethqos_set_serdes_speed(ethqos, SPEED_2500);
> ethqos_pcs_set_inband(priv, false);
> break;
> case SPEED_1000:
> - rgmii_updatel(ethqos, RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
> - RGMII_CONFIG2_RGMII_CLK_SEL_CFG,
> - RGMII_IO_MACRO_CONFIG2);
> ethqos_set_serdes_speed(ethqos, SPEED_1000);
> ethqos_pcs_set_inband(priv, true);
>
>>> - why is RGMII_CONFIG_SGMII_CLK_DVDR set to SGMII_10M_RX_CLK_DVDR
>>> for 10M, but never set to any other value for other speeds?
>>
>> [18:10] - In short, it configures a divider. The expected value is 0x13
>> for 10 Mbps / RMII mode
>
> This gets confusing. Is the "/" meaning "10Mbps in RMII mode" or "10Mbps
> or RMII mode".
>
>> which seems to have been problematic given:
>>
>> https://lore.kernel.org/all/20231212092208.22393-1-quic_snehshah@quicinc.com/
>>
>> But it didn't say what hardware had this issue.. whether it concerns a
>> specific SoC or all of them..
>>
>> A programming guide mentions the new 0x31 value for 10 Mbps in a
>> SoC-common paragraph so I suppose it's indeed better-er.. Perhaps issues
>> could arise if you switch back to a faster mode?
>
> Could the 0x13 be a typo? Its suspicious that the two values are 0x13
> vs 0x31. 0x13 = 19 vs 0x31 = 49. 0x31 makes more sense than 19.
>
> The platform glue is required to supply clk_rx_i to the dwmac's MAC
> receive path, deriving it from the 125MHz SGMII rx link clock divided
> by 1, 5 or 50. Normally, this would be done by hardware signals output
> from the dwmac.
>
> This suggests that the value programmed is one less than the actual
> divisor.
>
> There's two possibilities why this value needs to be programmed:
>
> 1. the hardware doesn't divide the SGMII rx link clock according to
> the hardware signals output from the dwmac, and needs the divisor to
> be manually programmed. This would require the divisor to also be
> programmed to 4 for 100M (but the driver doesn't do this.)
>
> 2. the hardware selects the clk_rx_i depending on the hardware
> signals, and while 1G and 100M use a fixed divisor of 1 and 5, the
> 10M divisor needs to be manually programmed.
>
> Any ideas what's really going on here?
The computer says:
RGMII ID mode - speed == 10 ? 0x13 : dontcare?
RMII Bypass ID mode - - speed == (10 || 100) ? 0x13 : dontcare?
(*the 100 above says "default" but that's again 0x13)
RMII mode (100 Mbps) - default (0x13)
RMII mode (10 MBps) - 0x13
SGMII mode - speed == 10 ? 0x31 : 0x13
Make of that what you will, I would *guess* there may be something like
2. going on
Konrad
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-11-07 11:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-29 10:18 [PATCH net-next] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification Russell King (Oracle)
2025-10-30 9:56 ` Mohd Ayaan Anwar
2025-10-30 10:01 ` Konrad Dybcio
2025-10-30 10:16 ` Russell King (Oracle)
2025-10-30 12:17 ` Russell King (Oracle)
2025-10-30 13:08 ` Konrad Dybcio
2025-10-30 14:05 ` Russell King (Oracle)
2025-11-07 11:34 ` Konrad Dybcio
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).