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

[Krzysztof Kozlowski]

On Tue, Mar 03, 2026 at 02:16:37PM +0800, lizhi2@xxxxxxxxxxxxxxxxxx wrote:
> From: Zhi Li <lizhi2@xxxxxxxxxxxxxxxxxx>
> 
> 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.

> 
> 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@xxxxxxxxxxxxxxxxxx>
> ---
>  .../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