Re: Re: [PATCH net-next v3 1/3] dt-bindings: ethernet: eswin: add clock sampling control

On Thu, Mar 05, 2026 at 10:52:38AM +0800, 李志 wrote:

quoted

-----原始邮件-----
发件人: "Conor Dooley" [off-list ref]
发送时间:2026-03-04 17:30:57 (星期三)
收件人: "Bo Gan" [off-list ref]
抄送: "Jakub Kicinski" [off-list ref], lizhi2@eswincomputing.com, devicetree@vger.kernel.org, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, 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, wens@kernel.org, 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, ningyu@eswincomputing.com, linmin@eswincomputing.com, pinkesh.vaghela@einfochips.com, pritesh.patel@einfochips.com, weishangjuan@eswincomputing.com
主题: Re: [PATCH net-next v3 1/3] dt-bindings: ethernet: eswin: add clock sampling control

On Tue, Mar 03, 2026 at 05:23:18PM -0800, Bo Gan wrote:

quoted

Hi All,

On 3/3/26 16:47, Conor Dooley wrote:

quoted

On Tue, Mar 03, 2026 at 04:38:46PM -0800, Jakub Kicinski wrote:

quoted

On Tue,  3 Mar 2026 14:16:37 +0800 lizhi2@eswincomputing.com wrote:

quoted

There are currently no in-tree users of the EIC7700 Ethernet driver, so
these changes are safe.

What do you mean by this sentence? The commit under Fixes was part of
Linux v6.19 already.

The "funny" thing is that caring about users doesn't even really matter
on the devicetree patch, except for this hunk:
|@@ -81,7 +99,9 @@ properties:
|                          or external clock selection
|           - description: Offset of AXI clock controller Low-Power request
|                          register
|+          - description: Offset of register controlling TXD delay
|           - description: Offset of register controlling TX/RX clock delay
|+          - description: Offset of register controlling RXD delay
|
| required:
|   - compatible
And it only matters here because an item is injected mid-list. If this
was moved to the end with the RXD delay, the **dt-binding** changes
don't have issues with safety. I've not looked at whether there are
knock-on concerns about users in the driver or whatever yet, but from a
binding POV only that hunk can break something that currently works.

This was already discussed here in v1:
https://lore.kernel.org/lkml/e7183ae1-8b8b-4e77-9f4e-3bc1b4b63556@lunn.ch/ (local)

The device-tree is not checked in yet by ESWIN folks, so there's currently
no user of the dt-binding. No need to worry about backward compat.

The binding and driver exist, there doesn't need to be a dts in tree for
there to be potential users. If the break was important I might not
care, but this seems to be a gratuitous break, since the new items could
be added to the end of the list and compatibility maintained without
incurring any more difficulty for you.

Hi Conor and Krzysztof,

Thanks for the reviews.

- The next patch will fix the property order to avoid any breakage
  with existing DT bindings.

Good, thanks.

- Eth1 does have a timing issue in silicon, as discussed here:
  https://lore.kernel.org/lkml/32a1f814.2c79.19bfe173225.Coremail.linmin@eswincomputing.com/ (local)

  Based on this, and according to the advice from Andrew
  https://lore.kernel.org/lkml/59cec617-0189-4dc3-bc3f-6346155a62ae@lunn.ch/ (local)
  https://lore.kernel.org/lkml/bd202cfa-d6eb-4d0e-982d-b49795dd25f7@lunn.ch/ (local)
  adding a DT property is not a reasonable approach.
  
  In the next patch, I will improve the description/paragraph and properly
  document the timing issues.

I personally don't mind having two compatibles, but I might be more
clear about what device the new one refers to (so something like
s/clk-inversion/eth1/g). But Krzysztof was the one who objected to
having multiple compatibles, so it's worth waiting to see what he has to
say.