linux-kernel - Re: Re: Re: Re: Re: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700 ethernet driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3261748c.629.198cfa3bc10.Coremail.lizhi2@eswincomputing.com>
Date: Fri, 22 Aug 2025 10:37:50 +0800 (GMT+08:00)
From: 李志 <lizhi2@...incomputing.com>
To: "Andrew Lunn" <andrew@...n.ch>
Cc: weishangjuan@...incomputing.com, andrew+netdev@...n.ch,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	netdev@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, mcoquelin.stm32@...il.com,
	alexandre.torgue@...s.st.com, rmk+kernel@...linux.org.uk,
	yong.liang.choong@...ux.intel.com, vladimir.oltean@....com,
	jszhang@...nel.org, jan.petrous@....nxp.com,
	prabhakar.mahadev-lad.rj@...renesas.com, inochiama@...il.com,
	boon.khai.ng@...era.com, dfustini@...storrent.com, 0x1207@...il.com,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org, ningyu@...incomputing.com,
	linmin@...incomputing.com, pinkesh.vaghela@...fochips.com
Subject: Re: Re: Re: Re: Re: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700
 ethernet driver

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" <andrew@...n.ch>
> 发送时间:2025-07-31 21:31:52 (星期四)
> 收件人: 李志 <lizhi2@...incomputing.com>
> 抄送: weishangjuan@...incomputing.com, andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, netdev@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, mcoquelin.stm32@...il.com, alexandre.torgue@...s.st.com, rmk+kernel@...linux.org.uk, yong.liang.choong@...ux.intel.com, vladimir.oltean@....com, jszhang@...nel.org, jan.petrous@....nxp.com, prabhakar.mahadev-lad.rj@...renesas.com, inochiama@...il.com, boon.khai.ng@...era.com, dfustini@...storrent.com, 0x1207@...il.com, linux-stm32@...md-mailman.stormreply.com, linux-arm-kernel@...ts.infradead.org, ningyu@...incomputing.com, linmin@...incomputing.com, pinkesh.vaghela@...fochips.com
> 主题: Re: Re: Re: Re: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700 ethernet driver
> 
> > > 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.
> 
> > 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?
> 
> > 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