Re: [PATCH net-next] net: stmmac: qcom-ethqos: remove MAC_CTRL_REG modification

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

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