Re: [RFC PATCH 6/6] arm64: dts: amlogic: t7: khadas-vim4: enable the ethernet port

[Maxime Chevallier]

Hi Lucas,

On 10/4/26 16:40, Lucas Tanure wrote:
> The board carries a Realtek RTL8211F gigabit PHY on the external
> management bus, connected to the MAC over RGMII.
> 
> Enable the controller, hand it the ethernet pins and point it at the
> PHY. The clock delays RGMII needs are supplied by the MAC rather than
> the PHY: 2ns on transmit and 2000ps on receive, the values the vendor
> kernel programs.
> 
> The PHY interrupt output is wired to GPIOH_6, so describe it and let
> link changes be reported instead of polled. The PHY reset pin is driven
> by an RC network on the board, not by the SoC, so there is no reset line
> to describe.
> 
> Tested at 930Mbit/s sustained receive over a minute, with no MAC error
> counters moving in either direction.
> 
> Assisted-by: LLM
> Signed-off-by: Lucas Tanure <tanure@xxxxxxxxx>
> ---
>  .../amlogic/amlogic-t7-a311d2-khadas-vim4.dts | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
> index 0fa83362b417..5d0365ddde72 100644
> --- a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
> +++ b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
> @@ -14,6 +14,7 @@ / {
>  	compatible = "khadas,vim4", "amlogic,a311d2", "amlogic,t7";
>  
>  	aliases {
> +		ethernet0 = &ethmac;
>  		serial0 = &uart_a;
>  		mmc0 = &sd_emmc_c;
>  		mmc1 = &sd_emmc_b;
> @@ -256,6 +257,33 @@ xtal: xtal-clk {
>  	};
>  };
>  
> +&ethmac {
> +	status = "okay";
> +	pinctrl-0 = <&eth_pins>, <&eth_rgmii_pins>;
> +	pinctrl-names = "default";
> +
> +	/*
> +	 * The RGMII clock delays are added by the MAC, so the PHY is
> +	 * asked for the mode that adds none.
> +	 */
> +	phy-mode = "rgmii";

In devicetree, "rgmii" means that the delays are inserted at the hardware
level, i.e. with longer PCB traces for the clock. With the above comment,
this is therefore wrong.

As a rule of thumb, "phy-mode = rgmii" is almost always wrong.

The right behaviour here is to represent the HW as it is, that is to say
pass phy-mode = "rgmii-id"; delays are internal. In DT we don't say who
inserts the delays (MAC or PHY), that's for the drive to figure out.

If the MAC inserts the delays, then it needs to fixup the mode passed to the PHY so that
they follow this doc :

https://www.kernel.org/doc/html/v7.2/networking/phy.html#rg-mii-electrical-interface-considerations

Maxime