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

[Chuan Liu <chuan.liu@amlogic.com>]

Hi Krzysztof, Lucas,

On 11/5/2026 7:57 PM, Krzysztof Kozlowski wrote:
> [ EXTERNAL EMAIL ]
> 
> On Tue, Nov 03, 2026 at 04:37:10PM +0000, Lucas Tanure wrote:
>>
>> Assisted-by: LLM
> 
> Drop this one.
> 
>> Signed-off-by: Lucas Tanure <tanure@linux.com>
>> ---
>>   .../bindings/net/amlogic,meson-dwmac.yaml     | 59 +++++++++++++++++++
>>   1 file changed, 59 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
>> index e769c9bb70d8..6de013e3e8f6 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
>>
>> @@ -57,6 +58,59 @@ allOf:
>>               - const: clkin1
>>               - const: timing-adjustment
>>
>> +  - 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.
> 
> This sounds like you take clock from other block. That's wrong, that
> device should take its own clock.

I see that the clock in the dts patch corresponds to <&clkc_periphs 
CLKID_SYS_ETHPHY>. This clock provides the working clock for the APB Bus 
of the ETH PHY controller, and this clock will affect the register 
reading and writing of the ETH PHY (including the ETH_PLL). See section 
15.3.4.2 RegisterDescription in A311D2 Datasheet for details.

> 
> 
>> +
>> +        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,
>> +        # 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.
>> +        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
>> +
>> +  # Delay definitions for Meson8b and newer
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - amlogic,meson8b-dwmac
>> +              - amlogic,meson8m2-dwmac
>> +              - amlogic,meson-gxbb-dwmac
>> +              - amlogic,meson-axg-dwmac
>> +              - amlogic,meson-g12a-dwmac
>> +    then:
>> +      properties:
>>           amlogic,tx-delay-ns:
>>             enum: [0, 2, 4, 6]
>>             default: 2
>> @@ -106,6 +160,7 @@ allOf:
>>             contains:
>>               enum:
>>                 - amlogic,meson-g12a-dwmac
>> +              - amlogic,t7-dwmac
>>       then:
>>         properties:
>>           rx-internal-delay-ps:
>> @@ -142,6 +197,10 @@ properties:
>>         - items:
>>             - const: amlogic,meson6-dwmac
>>             - const: snps,dwmac
>> +      - items:
>> +          - const: amlogic,t7-dwmac
>> +          - const: snps,dwmac-5.10a
>> +          - const: snps,dwmac
> 
> I see no such hunk in the code, so not sure how to review this.
> 
> Best regards,
> Krzysztof
> 
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

-- 
Best regards,
Chuan