Re: Re: [PATCH net-next v7 4/4] riscv: dts: eswin: eic7700-hifive-premier-p550: enable Ethernet controller

[李志]

> -----原始邮件-----
> 发件人: "Andrew Lunn" <andrew@xxxxxxx>
> 发送时间:2026-04-29 09:41:53 (星期三)
> 收件人: lizhi2@xxxxxxxxxxxxxxxxxx
> 抄送: devicetree@xxxxxxxxxxxxxxx, andrew+netdev@xxxxxxx, davem@xxxxxxxxxxxxx, edumazet@xxxxxxxxxx, kuba@xxxxxxxxxx, robh@xxxxxxxxxx, krzk+dt@xxxxxxxxxx, conor+dt@xxxxxxxxxx, netdev@xxxxxxxxxxxxxxx, pabeni@xxxxxxxxxx, mcoquelin.stm32@xxxxxxxxx, alexandre.torgue@xxxxxxxxxxx, rmk+kernel@xxxxxxxxxxxxxxx, pjw@xxxxxxxxxx, palmer@xxxxxxxxxxx, aou@xxxxxxxxxxxxxxxxx, alex@xxxxxxxx, linux-riscv@xxxxxxxxxxxxxxxxxxx, linux-stm32@xxxxxxxxxxxxxxxxxxxxxxxxxxxx, linux-arm-kernel@xxxxxxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, maxime.chevallier@xxxxxxxxxxx, ningyu@xxxxxxxxxxxxxxxxxx, linmin@xxxxxxxxxxxxxxxxxx, pinkesh.vaghela@xxxxxxxxxxxxxx, pritesh.patel@xxxxxxxxxxxxxx, weishangjuan@xxxxxxxxxxxxxxxxxx, horms@xxxxxxxxxx
> 主题: Re: [PATCH net-next v7 4/4] riscv: dts: eswin: eic7700-hifive-premier-p550: enable Ethernet controller
> 
> > +&gmac1 {
> > +	phy-handle = <&gmac1_phy0>;
> > +	/*
> > +	 * For the TX path of gmac1, there is a skew between the TX clock
> > +	 * and data on the MAC controller inside the silicon. This skew happens
> > +	 * to be approximately 2 ns. Therefore, it can be considered that the
> > +	 * 2 ns delay of TX is provided by the MAC.
> > +	 * No delay configuration for tx is needed in software via PHY driver.
> > +	 */
> > +	phy-mode = "rgmii-rxid";
> 
> This is wrong. Take a read of
> 
> https://elixir.bootlin.com/linux/v6.15/source/Documentation/devicetree/bindings/net/ethernet-controller.yaml#L287
> 
> phy-mode describes the board. If the board provides the 2ns delay, you
> use rgmii. If the MAC/PHY pair needs to provide the delay, you using
> rgmii-id.
> 
> If rgmii-id is used, it is up to the MAC/PHY to decide which will add
> the delay. If the MAC adds the delay, it needs to mask the value of
> phy-mode it passes to the PHY so it does not also add the delay.
> 
> Your broken hardware means you cannot support 'rgmii' or 'rgmii-rx',
> since you cannot turn off this 2ns delay, so you end up with double
> delays if anybody designs a board with 2ns TX delay on the board
> itself. So please validate the PHY modes and return -EINVAL if these
> modes are used.
> 

Thanks for the detailed explanation.

You are right that phy-mode should describe the board-level
configuration and should not be modified in the DT to reflect
MAC-internal behavior.

For this hardware, the MAC unconditionally introduces an ~2 ns
TX delay which cannot be disabled. This means the configuration
effectively matches rgmii-id at the board level, where TX delay
is provided by the MAC and RX delay by the PHY.

I will therefore:
- Keep the DT using phy-mode = "rgmii-id" to correctly describe
  the board-level timing.
- In the driver, mask the phy_interface passed to the PHY so that
  it does not add TX delay, avoiding double delay.
- Explicitly reject unsupported modes such as "rgmii" and
  "rgmii-rxid", since the MAC TX delay cannot be disabled and
  these configurations would lead to incorrect timing.

Regarding the tx-internal-delay-ps property, it represents the
total effective TX delay on the line, including the inherent
~2 ns skew introduced by the MAC silicon. The driver subtracts
this inherent skew and programs only the remaining delay.

As a result, for the clk-inversion variant, the valid range is
[2000, 4540], which matches the updated binding constraints.

I will clarify in the binding description that this property
represents the total TX delay (MAC + programmable), to avoid
ambiguity.

Thanks for pointing this out.