Re: [PATCH net 1/3] dt-bindings: net: thead,th1520-gmac: Describe APB interface clock

From: Drew Fustini <fustini@kernel.org>
To: Yao Zi <ziyao@disroot.org>
Cc: Guo Ren <guoren@kernel.org>, Fu Wei <wefu@redhat.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>,
	Emil Renner Berthing <emil.renner.berthing@canonical.com>,
	Jisheng Zhang <jszhang@kernel.org>,
	linux-riscv@lists.infradead.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net 1/3] dt-bindings: net: thead,th1520-gmac: Describe APB interface clock
Date: Tue, 29 Jul 2025 10:54:12 -0700	[thread overview]
Message-ID: <aIkKxM3zfVjaa1we@x1> (raw)
In-Reply-To: <20250729093734.40132-2-ziyao@disroot.org>

On Tue, Jul 29, 2025 at 09:37:32AM +0000, Yao Zi wrote:
> Besides ones for GMAC core and peripheral registers, the TH1520 GMAC
> requires one more clock for configuring APB glue registers. Describe
> it in the binding.
> 
> Though the clock is essential for operation, it's not marked as required
> for now to avoid introducing new dt-binding warnings to existing dts.
> 
> Fixes: f920ce04c399 ("dt-bindings: net: Add T-HEAD dwmac support")
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  .../devicetree/bindings/net/thead,th1520-gmac.yaml        | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/thead,th1520-gmac.yaml b/Documentation/devicetree/bindings/net/thead,th1520-gmac.yaml
> index 6d9de3303762..fea9fbc1d006 100644
> --- a/Documentation/devicetree/bindings/net/thead,th1520-gmac.yaml
> +++ b/Documentation/devicetree/bindings/net/thead,th1520-gmac.yaml
> @@ -59,14 +59,18 @@ properties:
>        - const: apb
>  
>    clocks:
> +    minItems: 2
>      items:
>        - description: GMAC main clock
>        - description: Peripheral registers interface clock
> +      - description: APB glue registers interface clock
>  
>    clock-names:
> +    minItems: 2
>      items:
>        - const: stmmaceth
>        - const: pclk
> +      - const: apb
>  
>    interrupts:
>      items:
> @@ -88,8 +92,8 @@ examples:
>          compatible = "thead,th1520-gmac", "snps,dwmac-3.70a";
>          reg = <0xe7070000 0x2000>, <0xec003000 0x1000>;
>          reg-names = "dwmac", "apb";
> -        clocks = <&clk 1>, <&clk 2>;
> -        clock-names = "stmmaceth", "pclk";
> +        clocks = <&clk 1>, <&clk 2>, <&clk 3>;
> +        clock-names = "stmmaceth", "pclk", "apb";
>          interrupts = <66>;
>          interrupt-names = "macirq";
>          phy-mode = "rgmii-id";
> -- 
> 2.50.1
> 

Thanks for figuring out that this clock is needed for the APB glue
registers. The schema passes W=1 dt_binding_check with no warnings.

Regarding minItems, I think it would be okay to change to 3 as the APB
clock should have been there from the start but I missed it. We can fix
the in-tree dts at the same time so that seems okay to me. But let's see
what the dt bindings maintainers think.

-Drew

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv