Re: [PATCH] arm64: dts: rockchip: use rgmii-id for LubanCat 4 gmac1

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] arm64: dts: rockchip: inaccurate MAC delay comment for rgmii-id
  in gmac1
--

commit 36ad97aa6620e705c8d483b7d079ee580a180c77
Author: Pufan Jin <cyan2254650260@gmail.com>

arm64: dts: rockchip: use rgmii-id for LubanCat 4 gmac1

This commit changes phy-mode for gmac1 on the LubanCat 4 board from
"rgmii" to "rgmii-id" to resolve packet loss on the RGMII link.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-lubancat-4.dts b/arch/arm64/boot/dts/rockchip/rk3588s-lubancat-4.dts
> index 0d28c616524fe..a042aa7021ffd 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-lubancat-4.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-lubancat-4.dts
> @@ -168,12 +168,13 @@ &gmac1 {
>  	clock_in_out = "output";
>  	phy-handle = <&rgmii_phy1>;
>  	/*
> -	 * The PCB adds ~2ns of trace delay on both TXC and RXC, so the
> -	 * required RGMII clock skew is provided by the board itself.
> -	 * Neither the MAC nor the PHY should add any internal delay,
> -	 * hence "rgmii" (not "rgmii-id") is correct here.
> +	 * The PCB adds ~2ns of trace delay on both TXC and RXC, but that
> +	 * alone is not sufficient for reliable RGMII operation: with
> +	 * "rgmii" and zero MAC delays the link shows ~90% packet loss.
> +	 * The PHY's internal TX/RX delay is required, so use "rgmii-id"
> +	 * and keep the MAC's default delays.
>  	 */
> -	phy-mode = "rgmii";
> +	phy-mode = "rgmii-id";

[Severity: Low]
Is this comment (and the commit message) accurate about how dwmac-rk
configures MAC delays in "rgmii" versus "rgmii-id" mode?

When tx_delay and rx_delay are omitted from the DT node, rk_gmac_setup()
initializes bsp_priv->tx_delay = 0x30 and bsp_priv->rx_delay = 0x10.
In rk_gmac_powerup(), "rgmii" programs those 0x30/0x10 fallback delays
into the MAC, whereas "rgmii-id" ignores them and forces the MAC delays to
(0, 0):

drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c:rk_gmac_powerup() {
    ...
	case PHY_INTERFACE_MODE_RGMII:
		dev_info(dev, "init for RGMII\n");
		if (bsp_priv->ops->set_to_rgmii)
			bsp_priv->ops->set_to_rgmii(bsp_priv,
						    bsp_priv->tx_delay,
						    bsp_priv->rx_delay);
		break;
	case PHY_INTERFACE_MODE_RGMII_ID:
		dev_info(dev, "init for RGMII_ID\n");
		if (bsp_priv->ops->set_to_rgmii)
			bsp_priv->ops->set_to_rgmii(bsp_priv, 0, 0);
		break;
    ...
}

Switching to "rgmii-id" zeroes the MAC delays while enabling the PHY's
internal TX/RX delays, rather than keeping the MAC's default delays.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109191515.252449-1-cyan2254650260@gmail.com?part=1