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

[李志 <lizhi2@eswincomputing.com>]

> -----原始邮件-----
> 发件人: "Andrew Lunn" <andrew@lunn.ch>
> 发送时间:2026-04-29 09:41:53 (星期三)
> 收件人: lizhi2@eswincomputing.com
> 抄送: devicetree@vger.kernel.org, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, netdev@vger.kernel.org, pabeni@redhat.com, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, rmk+kernel@armlinux.org.uk, pjw@kernel.org, palmer@dabbelt.com, aou@eecs.berkeley.edu, alex@ghiti.fr, linux-riscv@lists.infradead.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, maxime.chevallier@bootlin.com, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, pritesh.patel@einfochips.com, weishangjuan@eswincomputing.com, horms@kernel.org
> 主题: 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.