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

[Lucas Tanure]

On Tue, Nov 10, 2026 at 11:43 AM Chuan Liu <chuan.liu@xxxxxxxxxxx> wrote:
>
> Hi Lucas,
>
> On 11/9/2026 11:43 PM, Lucas Tanure wrote:
> > [ EXTERNAL EMAIL ]
> >
> > On Fri, Nov 6, 2026 at 3:16 AM Chuan Liu <chuan.liu@xxxxxxxxxxx> wrote:
> >>
> >> 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@xxxxxxxxx>
> >>>> ---
> >>>>    .../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.
> >
> > Thanks. That section is the PHY block at 0xfe028000, the region of the
> > mdio multiplexer node. The registers in this description are PRG_ETH0
> > and PRG_ETH1 in eth_top at 0xfe024000. On the VIM4 the same gate feeds
> > them too: with it off they read as zero and ignore writes. My datasheet
> > revisions do not say which clock feeds eth_top. Can you confirm it is
> > this one?
>
> The information I can confirm is:
>    - The base address of eth_phy is 0xfe028000, and its corresponding
> APB clock is <&clkc_periphs CLKID_SYS_ETHPHY>.
>    - The base address of eth_top is 0xfe024000, and the base address of
> eth_mac is 0xfdc00000; their corresponding APB clock is <&clkc_periphs
> CLKID_SYS_ETH>.

That is not what the VIM4 does. The stmmac core enables
CLKID_SYS_ETH before any glue code runs. With that clock on and
CLKID_SYS_ETHPHY off, PRG_ETH0 and PRG_ETH1 read as zero and the
driver's writes are lost: U-Boot's 0xa0000 was still in PRG_ETH1
after the driver had written 0. Enabling CLKID_SYS_ETHPHY right
before the write makes it land. That was the v3 bug, every frame a
CRC error. I repeated it today on the running kernel, toggling only
bit 4 of CLKCTRL_SYS_CLK_EN0_REG0 from user space, every other gate
left on:

  both gates on        EN0=ffffffff EN1=ffffffff PRG_ETH0=00001601
  ethphy off, eth on   EN0=ffffffef EN1=ffffffff PRG_ETH0=00000000
  ethphy restored      EN0=ffffffff EN1=ffffffff PRG_ETH0=00001601

So on this chip CLKID_SYS_ETH alone does not give access to eth_top.
Could the register interface of eth_top sit behind the eth_phy APB
slave? Either way the MAC node has to enable CLKID_SYS_ETHPHY, and
the binding will describe it as the clock of the glue registers.

>
> The CPU reads and writes the registers of these peripheral controllers
> (or Processing Units) via the APB. Consequently, the registers of these
> controllers cannot be read and written properly if the corresponding APB
> clock (sys_clk/clk81) is not enabled.
>
> >
> > Thanks,
> > Lucas
> >
> >>
> >>>
> >>>
> >>>> +
> >>>> +        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@xxxxxxxxxxxxxxxxxxx
> >>> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> >>
> >> --
> >> Best regards,
> >> Chuan
> >>
>
> --
> Best regards,
> Chuan
>
Thanks,
Lucas