[PATCH net-next v3 1/3] dt-bindings: ethernet: eswin: add clock sampling control
Bo Gan
ganboing at gmail.com
Wed Mar 4 00:22:00 PST 2026
Hi Krzysztof,
On 3/3/26 23:44, Krzysztof Kozlowski wrote:
> On Tue, Mar 03, 2026 at 02:16:37PM +0800, lizhi2 at eswincomputing.com wrote:
>> From: Zhi Li <lizhi2 at eswincomputing.com>
>>
>> The second Ethernet controller (eth1) on the EIC7700 SoC may experience
>> RX data sampling issues at high speed due to EIC7700-specific receive
>> clock to data skew at the MAC input.
>>
>> On the EIC7700 SoC, the second Ethernet controller (eth1) requires
>> inversion of the internal RGMII receive clock in order to meet RX data
>> sampling timing at high speed.
>>
>> Describe this SoC-specific difference by introducing a distinct compatible
>> string for MAC instances that require internal clock inversion, allowing the
>> driver to select the appropriate configuration without relying on per-board
>> vendor-specific properties.
>
> Pointless description/paragrapgh. Your explanation why adding a
> compatible is "because I need compatible". That's completely redundant.
>
> Explain what is special about this MAC instance, what's different in its
> programming model or other characteristics that you claim it is a
> different device.
>
I think ESWIN should improve the description/paragraph and properly doc
the timing issues discussed here:
https://lore.kernel.org/lkml/32a1f814.2c79.19bfe173225.Coremail.linmin@eswincomputing.com/
I do feel the need of using a different compatible, though. I think we
discussed in depth in that thread (link above), and advice from Andrew
https://lore.kernel.org/lkml/59cec617-0189-4dc3-bc3f-6346155a62ae@lunn.ch/
https://lore.kernel.org/lkml/bd202cfa-d6eb-4d0e-982d-b49795dd25f7@lunn.ch/
is to basically apply different parameters to MAC based on eth0/eth1. The
compatible string approach is a clean solution to achieve that. The reason
being that for eth1, there's no way to meet the standard without clock
inversion, given the vast internal clock skew. I don't think claiming it
as a different device than eth0 is that far-fetched. Hence, no need for an
additional property and the driver code to check for that.
>>
>> The rx-internal-delay-ps and tx-internal-delay-ps properties now use
>> minimum and maximum constraints to reflect the actual hardware delay
>> range (0-2540 ps) applied in 20 ps steps. This relaxes the binding
>> validation compared to the previous enum-based definition and avoids
>> regressions for existing DTBs while keeping the same hardware limits.
>>
>> Treat the RX/TX internal delay properties as optional, board-specific
>> tuning knobs and remove them from the example to avoid encouraging
>> their use.
>>
>> In addition, the binding now includes additional background information
>> about the HSP CSR registers accessed by the MAC. The TXD and RXD delay
>> control registers are included so the driver can explicitly clear any
>> residual configuration left by the bootloader. Background reference for
>> the High-Speed Subsystem and HSP CSR block is available in Chapter 10
>> ("High-Speed Interface") of the EIC7700X SoC Technical Reference Manual,
>> Part 4 (EIC7700X_SoC_Technical_Reference_Manual_Part4.pdf):
>> https://github.com/eswincomputing/EIC7700X-SoC-Technical-Reference-Manual/releases
>>
>> There are currently no in-tree users of the EIC7700 Ethernet driver, so
>> these changes are safe.
>>
>> Fixes: 888bd0eca93c ("dt-bindings: ethernet: eswin: Document for EIC7700 SoC")
>> Signed-off-by: Zhi Li <lizhi2 at eswincomputing.com>
>> ---
>> .../bindings/net/eswin,eic7700-eth.yaml | 75 +++++++++++++++----
>> 1 file changed, 59 insertions(+), 16 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
>> index 91e8cd1db67b..22d1cecea07e 100644
>> --- a/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
>> +++ b/Documentation/devicetree/bindings/net/eswin,eic7700-eth.yaml
>> @@ -20,6 +20,7 @@ select:
>> contains:
>> enum:
>> - eswin,eic7700-qos-eth
>> + - eswin,eic7700-qos-eth-clk-inversion
>> required:
>> - compatible
>>
>> @@ -28,9 +29,13 @@ allOf:
>>
>> properties:
>> compatible:
>> - items:
>> - - const: eswin,eic7700-qos-eth
>> - - const: snps,dwmac-5.20
>> + oneOf:
>> + - items:
>> + - const: eswin,eic7700-qos-eth
>> + - const: snps,dwmac-5.20
>> + - items:
>> + - const: eswin,eic7700-qos-eth-clk-inversion
>
> So just enum for both entries?
>
> Anyway, that's the same device, so you do not get two compatibles. This
> should be a property. Which property not sure, maybe all this was
> discussed already.
>
>
>> + - const: snps,dwmac-5.20
>>
>> reg:
>> maxItems: 1
>> @@ -63,16 +68,29 @@ properties:
>> - const: stmmaceth
>>
>> rx-internal-delay-ps:
>> - enum: [0, 200, 600, 1200, 1600, 1800, 2000, 2200, 2400]
>> + minimum: 0
>> + maximum: 2540
>> + multipleOf: 20
>>
>> tx-internal-delay-ps:
>> - enum: [0, 200, 600, 1200, 1600, 1800, 2000, 2200, 2400]
>> + minimum: 0
>> + maximum: 2540
>> + multipleOf: 20
>>
>> eswin,hsp-sp-csr:
>> description:
>> HSP CSR is to control and get status of different high-speed peripherals
>> (such as Ethernet, USB, SATA, etc.) via register, which can tune
>> board-level's parameters of PHY, etc.
>> +
>> + Additional background information about the High-Speed Subsystem
>> + and the HSP CSR block is available in Chapter 10 ("High-Speed Interface")
>> + of the EIC7700X SoC Technical Reference Manual, Part 4
>> + (EIC7700X_SoC_Technical_Reference_Manual_Part4.pdf). The manual is
>> + publicly available at
>> + https://github.com/eswincomputing/EIC7700X-SoC-Technical-Reference-Manual/releases
>> +
>> + This reference is provided for background information only.
>> $ref: /schemas/types.yaml#/definitions/phandle-array
>> items:
>> - items:
>> @@ -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
>
> As pointed out, you cannot change the order and there is no reason for
> doing this explained in commit msg.
>
> Best regards,
> Krzysztof
>
Bo
More information about the linux-riscv
mailing list