Re: [PATCH v6 2/7] dt-bindings: net: amlogic,meson-dwmac: add amlogic,t7-dwmac

[Rob Herring]

On Thu, Nov 05, 2026 at 10:03:17AM +0000, Lucas Tanure wrote:
> The T7 has the same ethernet glue as the G12A but needs two clocks of
> its own. The controller reaches memory through a pipeline stage with
> its own gate; nothing else claims it, so it is switched off as unused
> and transfers never finish. The glue registers sit in a block clocked
> together with the MDIO multiplexer, which probes after the controller
> has programmed them: the writes are lost and the port keeps the
> bootloader's delays.
> 
> So the T7 gets its own clock list, without the timing adjustment clock
> it never uses. Its delays follow ethernet-controller.yaml: the PHY adds
> the 2ns, and only the receive delay line offers fine tuning, so only
> rx-internal-delay-ps is valid here.
> 
> Assisted-by: LLM
> Signed-off-by: Lucas Tanure <tanure@xxxxxxxxx>
> ---
>  .../bindings/net/amlogic,meson-dwmac.yaml     | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)

I tend to agree with the sashiko comment, but am fine either way. Once 
the if/then schemas are longer than the main schema, it's a sign to 
split.

> 
> diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> index e769c9bb70d8..ff5e50f65114 100644
> --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> @@ -23,6 +23,7 @@ select:
>            - amlogic,meson-gxbb-dwmac
>            - amlogic,meson-axg-dwmac
>            - amlogic,meson-g12a-dwmac
> +          - amlogic,t7-dwmac
>    required:
>      - compatible
>  
> @@ -84,6 +85,49 @@ allOf:
>          rx-internal-delay-ps:
>            default: 0
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: amlogic,t7-dwmac
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: GMAC main clock
> +            - description: First parent clock of the internal mux
> +            - description: Second parent clock of the internal mux
> +            - description:
> +                clock of the pipeline stage inserted in the bus path between
> +                the controller and the DRAM. Without it, the controller cannot
> +                complete DMA transfers.
> +            - description:
> +                clock of the register block holding PRG_ETH0 and PRG_ETH1.
> +                Without it, the block reads as zero and ignores writes.
> +
> +        clock-names:
> +          items:
> +            - const: stmmaceth
> +            - const: clkin0
> +            - const: clkin1
> +            - const: pipeline
> +            - const: pclk
> +
> +        # The PHY adds the 2ns RGMII delays, see ethernet-controller.yaml.
> +        # The RX delay line of this glue is fine tuning in 200ps steps,

Sounds like constraints:

maximum: 2000
multipleOf: 200

> +        # added on top of what phy-mode describes. The TX side has no fine
> +        # tuning: its only step is a quarter of the clock period, the full
> +        # 2ns at 1Gbit/s, so the TX property is not accepted.

Move this to tx-internal-delay-ps. You can (kind of ackwardly) do:

tx-internal-delay-ps:
  description: ...
  not: {}

(or "not: true")

> +        rx-internal-delay-ps:
> +          default: 0
> +
> +        tx-internal-delay-ps: false
> +
> +        # Deprecated: new SoCs use the standard *-internal-delay-ps
> +        # properties instead.
> +        amlogic,tx-delay-ns: false
> +        amlogic,rx-delay-ns: false
> +
>    - if:
>        properties:
>          compatible:
> @@ -106,6 +150,7 @@ allOf:
>            contains:
>              enum:
>                - amlogic,meson-g12a-dwmac
> +              - amlogic,t7-dwmac
>      then:
>        properties:
>          rx-internal-delay-ps:
> @@ -142,6 +187,10 @@ properties:
>        - items:
>            - const: amlogic,meson6-dwmac
>            - const: snps,dwmac
> +      - items:
> +          - const: amlogic,t7-dwmac
> +          - const: snps,dwmac-5.10a
> +          - const: snps,dwmac

I would drop either both or at least 'snps,dwmac'. They provide no value 
because there is nothing to use them. IOW, there's not s/w that works 
which only understands the last 2 and not amlogic,t7-dwmac. Correct me 
if I'm wrong. Maybe a bootloader with simple needs.

Rob