[PATCH net 0/3] Fix broken link with TH1520 GMAC when linkspeed changes

[PATCH net 0/3] Fix broken link with TH1520 GMAC when linkspeed changes

[Yao Zi]

It's noted that on TH1520 SoC, the GMAC's link becomes broken after
the link speed is changed (for example, running ethtool -s eth0 speed
100 on the peer when negotiated to 1Gbps), but the GMAC could function
normally if the speed is brought back to the initial.

Just like many other SoCs utilizing STMMAC IP, we need to adjust the TX
clock supplying TH1520's GMAC through some SoC-specific glue registers
when linkspeed changes. But it's found that after the full kernel
startup, reading from them results in garbage and writing to them makes
no effect, which is the cause of broken link.

Further testing shows perisys-apb4-hclk must be ungated for normal
access to Th1520 GMAC APB glue registers, which is neither described in
dt-binding nor acquired by the driver.

This series expands the dt-binding of TH1520's GMAC to allow an extra
"APB glue registers interface clock", instructs the driver to acquire
and enable the clock, and finally supplies CLK_PERISYS_APB4_HCLK for
TH1520's GMACs in SoC devicetree.

Yao Zi (3):
  dt-bindings: net: thead,th1520-gmac: Describe APB interface clock
  net: stmmac: thead: Get and enable APB clock on initialization
  riscv: dts: thead: Add APB clocks for TH1520 GMACs

 .../devicetree/bindings/net/thead,th1520-gmac.yaml     |  8 ++++++--
 arch/riscv/boot/dts/thead/th1520.dtsi                  | 10 ++++++----
 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c      |  6 ++++++
 3 files changed, 18 insertions(+), 6 deletions(-)

-- 
2.50.1

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

[Yao Zi]

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

[PATCH net 2/3] net: stmmac: thead: Get and enable APB clock on initialization

[Yao Zi]

It's necessary to adjust the MAC TX clock when the linkspeed changes,
but it's noted such adjustment always fails on TH1520 SoC, and reading
back from APB glue registers that control clock generation results in
garbage, causing broken link.

With some testing, it's found a clock must be ungated for access to APB
glue registers. Without any consumer, the clock is automatically
disabled during late kernel startup. Let's get and enable it if it's
described in devicetree.

Fixes: 33a1a01e3afa ("net: stmmac: Add glue layer for T-HEAD TH1520 SoC")
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
index c72ee759aae5..95096244a846 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
@@ -211,6 +211,7 @@ static int thead_dwmac_probe(struct platform_device *pdev)
 	struct stmmac_resources stmmac_res;
 	struct plat_stmmacenet_data *plat;
 	struct thead_dwmac *dwmac;
+	struct clk *apb_clk;
 	void __iomem *apb;
 	int ret;
 
@@ -224,6 +225,11 @@ static int thead_dwmac_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, PTR_ERR(plat),
 				     "dt configuration failed\n");
 
+	apb_clk = devm_clk_get_optional_enabled(&pdev->dev, "apb");
+	if (IS_ERR(apb_clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(apb_clk),
+				     "failed to get apb clock\n");
+
 	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
 	if (!dwmac)
 		return -ENOMEM;
-- 
2.50.1

[PATCH net 3/3] riscv: dts: thead: Add APB clocks for TH1520 GMACs

[Yao Zi]

Describe perisys-apb4-hclk as the APB clock for TH1520 SoC, which is
essential for accessing GMAC glue registers.

Fixes: 7e756671a664 ("riscv: dts: thead: Add TH1520 ethernet nodes")
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index 42724bf7e90e..03f1d7319049 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -297,8 +297,9 @@ gmac1: ethernet@ffe7060000 {
 			reg-names = "dwmac", "apb";
 			interrupts = <67 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "macirq";
-			clocks = <&clk CLK_GMAC_AXI>, <&clk CLK_GMAC1>;
-			clock-names = "stmmaceth", "pclk";
+			clocks = <&clk CLK_GMAC_AXI>, <&clk CLK_GMAC1>,
+				 <&clk CLK_PERISYS_APB4_HCLK>;
+			clock-names = "stmmaceth", "pclk", "apb";
 			snps,pbl = <32>;
 			snps,fixed-burst;
 			snps,multicast-filter-bins = <64>;
@@ -319,8 +320,9 @@ gmac0: ethernet@ffe7070000 {
 			reg-names = "dwmac", "apb";
 			interrupts = <66 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "macirq";
-			clocks = <&clk CLK_GMAC_AXI>, <&clk CLK_GMAC0>;
-			clock-names = "stmmaceth", "pclk";
+			clocks = <&clk CLK_GMAC_AXI>, <&clk CLK_GMAC0>,
+				 <&clk CLK_PERISYS_APB4_HCLK>;
+			clock-names = "stmmaceth", "pclk", "apb";
 			snps,pbl = <32>;
 			snps,fixed-burst;
 			snps,multicast-filter-bins = <64>;
-- 
2.50.1

Re: [PATCH net 0/3] Fix broken link with TH1520 GMAC when linkspeed changes

[Drew Fustini]

On Tue, Jul 29, 2025 at 09:37:31AM +0000, Yao Zi wrote:
> It's noted that on TH1520 SoC, the GMAC's link becomes broken after
> the link speed is changed (for example, running ethtool -s eth0 speed
> 100 on the peer when negotiated to 1Gbps), but the GMAC could function
> normally if the speed is brought back to the initial.
> 
> Just like many other SoCs utilizing STMMAC IP, we need to adjust the TX
> clock supplying TH1520's GMAC through some SoC-specific glue registers
> when linkspeed changes. But it's found that after the full kernel
> startup, reading from them results in garbage and writing to them makes
> no effect, which is the cause of broken link.
> 
> Further testing shows perisys-apb4-hclk must be ungated for normal
> access to Th1520 GMAC APB glue registers, which is neither described in
> dt-binding nor acquired by the driver.
> 
> This series expands the dt-binding of TH1520's GMAC to allow an extra
> "APB glue registers interface clock", instructs the driver to acquire
> and enable the clock, and finally supplies CLK_PERISYS_APB4_HCLK for
> TH1520's GMACs in SoC devicetree.
> 
> Yao Zi (3):
>   dt-bindings: net: thead,th1520-gmac: Describe APB interface clock
>   net: stmmac: thead: Get and enable APB clock on initialization
>   riscv: dts: thead: Add APB clocks for TH1520 GMACs
> 
>  .../devicetree/bindings/net/thead,th1520-gmac.yaml     |  8 ++++++--
>  arch/riscv/boot/dts/thead/th1520.dtsi                  | 10 ++++++----
>  drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c      |  6 ++++++
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 
> -- 
> 2.50.1
> 

Thanks for fixing this issue. I've tested this series on next-20250729
with my LPi4a. I'm able to change the speed from 1000 to 100 and back to
1000. The network continues to work without any problems through those
transistions.

Tested-by: Drew Fustini <fustini@kernel.org>

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2161 bytes --]

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.

Nah, introduce the warnings. If the clock is required for operation, it
should be marked as such. You've made it optional in the driver, which
is the important part (backwards compatible) and you've got the dts
patch in the series.

> 
> 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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net 3/3] riscv: dts: thead: Add APB clocks for TH1520 GMACs

[Drew Fustini]

On Tue, Jul 29, 2025 at 09:37:34AM +0000, Yao Zi wrote:
> Describe perisys-apb4-hclk as the APB clock for TH1520 SoC, which is
> essential for accessing GMAC glue registers.
> 
> Fixes: 7e756671a664 ("riscv: dts: thead: Add TH1520 ethernet nodes")
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  arch/riscv/boot/dts/thead/th1520.dtsi | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> index 42724bf7e90e..03f1d7319049 100644
> --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> @@ -297,8 +297,9 @@ gmac1: ethernet@ffe7060000 {
>  			reg-names = "dwmac", "apb";
>  			interrupts = <67 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "macirq";
> -			clocks = <&clk CLK_GMAC_AXI>, <&clk CLK_GMAC1>;
> -			clock-names = "stmmaceth", "pclk";
> +			clocks = <&clk CLK_GMAC_AXI>, <&clk CLK_GMAC1>,
> +				 <&clk CLK_PERISYS_APB4_HCLK>;
> +			clock-names = "stmmaceth", "pclk", "apb";
>  			snps,pbl = <32>;
>  			snps,fixed-burst;
>  			snps,multicast-filter-bins = <64>;
> @@ -319,8 +320,9 @@ gmac0: ethernet@ffe7070000 {
>  			reg-names = "dwmac", "apb";
>  			interrupts = <66 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "macirq";
> -			clocks = <&clk CLK_GMAC_AXI>, <&clk CLK_GMAC0>;
> -			clock-names = "stmmaceth", "pclk";
> +			clocks = <&clk CLK_GMAC_AXI>, <&clk CLK_GMAC0>,
> +				 <&clk CLK_PERISYS_APB4_HCLK>;
> +			clock-names = "stmmaceth", "pclk", "apb";
>  			snps,pbl = <32>;
>  			snps,fixed-burst;
>  			snps,multicast-filter-bins = <64>;
> -- 
> 2.50.1
> 

Thank you for determining that this clock is needed for the GMAC.

Reviewed-by: Drew Fustini <fustini@kernel.org>

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

[Drew Fustini]

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

Re: [PATCH net 2/3] net: stmmac: thead: Get and enable APB clock on initialization

[Drew Fustini]

On Tue, Jul 29, 2025 at 09:37:33AM +0000, Yao Zi wrote:
> It's necessary to adjust the MAC TX clock when the linkspeed changes,
> but it's noted such adjustment always fails on TH1520 SoC, and reading
> back from APB glue registers that control clock generation results in
> garbage, causing broken link.
> 
> With some testing, it's found a clock must be ungated for access to APB
> glue registers. Without any consumer, the clock is automatically
> disabled during late kernel startup. Let's get and enable it if it's
> described in devicetree.
> 
> Fixes: 33a1a01e3afa ("net: stmmac: Add glue layer for T-HEAD TH1520 SoC")
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
> index c72ee759aae5..95096244a846 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
> @@ -211,6 +211,7 @@ static int thead_dwmac_probe(struct platform_device *pdev)
>  	struct stmmac_resources stmmac_res;
>  	struct plat_stmmacenet_data *plat;
>  	struct thead_dwmac *dwmac;
> +	struct clk *apb_clk;
>  	void __iomem *apb;
>  	int ret;
>  
> @@ -224,6 +225,11 @@ static int thead_dwmac_probe(struct platform_device *pdev)
>  		return dev_err_probe(&pdev->dev, PTR_ERR(plat),
>  				     "dt configuration failed\n");
>  
> +	apb_clk = devm_clk_get_optional_enabled(&pdev->dev, "apb");
> +	if (IS_ERR(apb_clk))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(apb_clk),
> +				     "failed to get apb clock\n");
> +
>  	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
>  	if (!dwmac)
>  		return -ENOMEM;
> -- 
> 2.50.1
> 

Thank you for tracking down root cause and fixing it.

Reviewed-by: Drew Fustini <fustini@kernel.org>

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

[Yao Zi]

On Tue, Jul 29, 2025 at 06:43:42PM +0100, Conor Dooley wrote:
> 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.
> 
> Nah, introduce the warnings. If the clock is required for operation, it
> should be marked as such. You've made it optional in the driver, which
> is the important part (backwards compatible) and you've got the dts
> patch in the series.

Thanks for the confirmation, will remove minItems in v2.

Regards,
Yao Zi

> > 
> > 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
> >