Dear Andrew Lunn,
Thank you for your valuable and professional suggestions.
Please find our questions and explanations embedded below your comments
in the original email.

Best regards,

Li Zhi
Eswin Computing

-----原始邮件-----
发件人: "Andrew Lunn" [off-list ref]
发送时间:2025-07-31 21:31:52 (星期四)
收件人: 李志 [off-list ref]
抄送: weishangjuan@eswincomputing.com, 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, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, mcoquelin.stm32@gmail.com, alexandre.torgue@foss.st.com, rmk+kernel@armlinux.org.uk, yong.liang.choong@linux.intel.com, vladimir.oltean@nxp.com, jszhang@kernel.org, jan.petrous@oss.nxp.com, prabhakar.mahadev-lad.rj@bp.renesas.com, inochiama@gmail.com, boon.khai.ng@altera.com, dfustini@tenstorrent.com, 0x1207@gmail.com, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com
主题: Re: Re: Re: Re: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700 ethernet driver

quoted

quoted

You hardware has a lot of flexibility, but none of if should actually
be needed, if you follow the standard.

So phy-mode = "rgmii-id"; should be all you need for most boards.
Everything else should be optional, with sensible defaults.

On our platform, the vendor-specific attributes eswin,dly-param-* were
initially introduced to compensate for board-specific variations in RGMII
signal timing, primarily due to differences in PCB trace lengths.

So it seems like, because you have the flexibility in the hardware,
you designed your PCB poorly, breaking the standard, so now must have
these properties.  It would of been much better if you had stuck to
the standard...

Please ensure your default values, when nothing is specified in DT,
correspond to a board which actually fulfils the standard. The next
board which is made using this device can then avoid having anything
special in there DT blob.

quoted

These attributes allow fine-grained, per-signal delay control for RXD, TXD,
TXEN, RXDV, RXCLK, and TXCLK, based on empirically derived optimal phase
settings.
In our experience, setting phy-mode = "rgmii-id" alone, along with only
the standard properties rx-internal-delay-ps and tx-internal-delay-ps,
has proven insufficient to meet our hardware's timing requirements.

You don't need vendor properties for RXCLK and TXCLK, that is what
tx-internal-delay-ps and rx-internal-delay-ps do. They change the
clock signal relative to TX and RX data. So you only need properties
for TXEN and RXDV. You should probably call these
eswin,txen-internal-delay-ps and eswin,rxdv-internal-delay-ps.  In the
binding you need to clearly define what these mean, for your hardware,
i.e.  what is the delay relative to?

quoted

1. Setting all delay parameters (RXD, TXD, TXEN, RXDV, RXCLK, and TXCLK)
   using vendor-specific attributes eswin,dly-param-*.
   e.g.
   eswin,dly-param-1000m = <0x20202020 0x96205A20 0x20202020>;
2. Setting delay parameters (RXD, TXD, TXEN, RXDV) using vendor-specific
   attributes eswin,dly-param-* , RXCLK using rx-internal-delay-ps and
   TXCLK using tx-internal-delay-ps.
   e.g
   eswin,dly-param-1000m = <0x20202020 0x80200020 0x20202020>;
   rx-internal-delay-ps = <9000>;
   tx-internal-delay-ps = <2200>;

Neither. DT should not contain HW values you poke into registers. They
should be SI using, in this case, pico seconds. From these delays in
picoseconds, have the driver calculate what values should be written
into the registers.

And these delay values are unlikely to be correct. You are using
rgmii-id, so the PHY is adding 2ns. You want the MAC to make small
tuning adjustments, so 200 could be reasonable, but 9000ps is way too
big.

We re-tuned and verified that setting the TXD and RXD delays to 0 and
configuring TXEN and RXDV to 0 yielded the same hardware performance as
long as we only applied delays (e.g. 200ps) to TXCLK and RXCLK.
Therefore, in the next patch, we will drop the vendor-specific properties
(e.g. eswin,dly-param-*) and keep only the standard attributes, namely
rx-internal-delay-ps and tx-internal-delay-ps.
Is this correct?

	Andrew