Re: [PATCH net-next v2 5/9] net: stmmac: qcom-ethqos: fix RGMII_ID mode to use DLL bypass

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver mishandles PHY_INTERFACE_MODE_RGMII_RXID, incorrectly adding a MAC-side RX delay via the DLL, causing double RX delay when combined with the PHY's RX delay.
--

commit 62a16405ee0e741087a5282875b7de0ba607d81f
Author: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>

net: stmmac: qcom-ethqos: fix RGMII_ID mode to use DLL bypass

This commit fixes RGMII_ID mode by powering down the MAC's DLL and setting
DDR bypass mode, ensuring the MAC does not add delay when the PHY already
provides it. It introduces a new ethqos_rgmii_id_macro_init() helper and
adjusts clock rate logic accordingly.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c

[ ... ]

> @@ -405,8 +416,7 @@ static void ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos, int speed)
>  		      RGMII_IO_MACRO_CONFIG2);
>  
>  	/* Determine if the PHY adds a 2 ns TX delay or the MAC handles it */
> -	if (ethqos->phy_mode == PHY_INTERFACE_MODE_RGMII_ID ||
> -	    ethqos->phy_mode == PHY_INTERFACE_MODE_RGMII_TXID)
> +	if (ethqos->phy_mode == PHY_INTERFACE_MODE_RGMII_TXID)
>  		phase_shift = 0;
>  	else
>  		phase_shift = RGMII_CONFIG2_TX_CLK_PHASE_SHIFT_EN;

[Severity: High]
This is a pre-existing issue, but does this function mishandle
PHY_INTERFACE_MODE_RGMII_RXID?

By keeping RGMII_RXID in this generic initialization path, it appears
the MAC-side RX delay via the DLL will still be enabled later in the
function when negotiating 1Gbps speeds:

	rgmii_setmask(ethqos, SDCC_DDR_CONFIG_PRG_DLY_EN,
		      SDCC_HC_REG_DDR_CONFIG);

Since the PHY already provides an RX delay in RGMII_RXID mode, does this
result in a double RX delay?

A double RX delay could shift the receive clock too far relative to the
data eye at Gigabit speeds, potentially causing data corruption or packet
loss.

While this patch successfully fixes RGMII_ID by moving it to bypass mode,
could RGMII_RXID be broken on platforms that specify it in their device tree,
and would it need similar bypass handling?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-shikra_ethernet-v2-0-bbe3389d0652@oss.qualcomm.com?part=5