[v1] net: stmmac: thead: Enable TX clock before MAC initialization

[PATCH net] net: stmmac: thead: Enable TX clock before MAC initialization

Posted by Yao Zi 2 months ago

The clk_tx_i clock must be supplied to the MAC for successful
initialization. On TH1520 SoC, the clock is provided by an internal
divider configured through GMAC_PLLCLK_DIV register when using RGMII
interface. However, currently we don't setup the divider before
initialization of the MAC, resulting in DMA reset failures if the
bootloader/firmware doesn't enable the divider,

[    7.839601] thead-dwmac ffe7060000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
[    7.938338] thead-dwmac ffe7060000.ethernet eth0: PHY [stmmac-0:02] driver [RTL8211F Gigabit Ethernet] (irq=POLL)
[    8.160746] thead-dwmac ffe7060000.ethernet eth0: Failed to reset the dma
[    8.170118] thead-dwmac ffe7060000.ethernet eth0: stmmac_hw_setup: DMA engine initialization failed
[    8.179384] thead-dwmac ffe7060000.ethernet eth0: __stmmac_open: Hw setup failed

Let's simply write GMAC_PLLCLK_DIV_EN to GMAC_PLLCLK_DIV to enable the
divider before MAC initialization. The rate doesn't matter, which we
could reclock properly according to the link speed later after link is
up.

Signed-off-by: Yao Zi <ziyao@disroot.org>
Fixes: 33a1a01e3afa ("net: stmmac: Add glue layer for T-HEAD TH1520 SoC")
---

Note that the DMA reset failures cannot be reproduced with the vendor
U-Boot, which always enables the divider, regardless whether the port is
used[1].

As this schema (enables the divider first and reclock it later) requires
access to the APB glue registers, the patch depends on v2 of series
"Fix broken link with TH1520 GMAC when linkspeed changes"[2] to ensure
the APB bus clock is ungated.

[1]: https://github.com/revyos/thead-u-boot/blob/93ff49d9f5bbe7942f727ab93311346173506d27/board/thead/light-c910/light.c#L581-L582
[2]: https://lore.kernel.org/netdev/20250801091240.46114-1-ziyao@disroot.org/

 drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
index 95096244a846..a65c2443bf42 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
@@ -164,6 +164,7 @@ static int thead_dwmac_enable_clk(struct plat_stmmacenet_data *plat)
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 		/* use pll */
+		writel(GMAC_PLLCLK_DIV_EN, dwmac->apb_base + GMAC_PLLCLK_DIV);
 		writel(GMAC_GTXCLK_SEL_PLL, dwmac->apb_base + GMAC_GTXCLK_SEL);
 		reg = GMAC_TX_CLK_EN | GMAC_TX_CLK_N_EN | GMAC_TX_CLK_OUT_EN |
 		      GMAC_RX_CLK_EN | GMAC_RX_CLK_N_EN;
-- 
2.50.1

Re: [PATCH net] net: stmmac: thead: Enable TX clock before MAC initialization

Posted by Drew Fustini 2 months ago

On Fri, Aug 01, 2025 at 09:45:07AM +0000, Yao Zi wrote:
> The clk_tx_i clock must be supplied to the MAC for successful
> initialization. On TH1520 SoC, the clock is provided by an internal
> divider configured through GMAC_PLLCLK_DIV register when using RGMII
> interface. However, currently we don't setup the divider before
> initialization of the MAC, resulting in DMA reset failures if the
> bootloader/firmware doesn't enable the divider,
> 
> [    7.839601] thead-dwmac ffe7060000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> [    7.938338] thead-dwmac ffe7060000.ethernet eth0: PHY [stmmac-0:02] driver [RTL8211F Gigabit Ethernet] (irq=POLL)
> [    8.160746] thead-dwmac ffe7060000.ethernet eth0: Failed to reset the dma
> [    8.170118] thead-dwmac ffe7060000.ethernet eth0: stmmac_hw_setup: DMA engine initialization failed
> [    8.179384] thead-dwmac ffe7060000.ethernet eth0: __stmmac_open: Hw setup failed
> 
> Let's simply write GMAC_PLLCLK_DIV_EN to GMAC_PLLCLK_DIV to enable the
> divider before MAC initialization. The rate doesn't matter, which we
> could reclock properly according to the link speed later after link is
> up.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> Fixes: 33a1a01e3afa ("net: stmmac: Add glue layer for T-HEAD TH1520 SoC")
> ---
> 
> Note that the DMA reset failures cannot be reproduced with the vendor
> U-Boot, which always enables the divider, regardless whether the port is
> used[1].

Thanks for sending this fix. I do now have a 16GB RAM LPi4a so I'll try
to get the mainline u-boot on there in order to test.

Thanks,
Drew

Re: [PATCH net] net: stmmac: thead: Enable TX clock before MAC initialization

Posted by Jakub Kicinski 2 months ago

On Fri,  1 Aug 2025 09:45:07 +0000 Yao Zi wrote:
> The clk_tx_i clock must be supplied to the MAC for successful
> initialization. On TH1520 SoC, the clock is provided by an internal
> divider configured through GMAC_PLLCLK_DIV register when using RGMII
> interface. However, currently we don't setup the divider before
> initialization of the MAC, resulting in DMA reset failures if the
> bootloader/firmware doesn't enable the divider,
> 
> [    7.839601] thead-dwmac ffe7060000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> [    7.938338] thead-dwmac ffe7060000.ethernet eth0: PHY [stmmac-0:02] driver [RTL8211F Gigabit Ethernet] (irq=POLL)
> [    8.160746] thead-dwmac ffe7060000.ethernet eth0: Failed to reset the dma
> [    8.170118] thead-dwmac ffe7060000.ethernet eth0: stmmac_hw_setup: DMA engine initialization failed
> [    8.179384] thead-dwmac ffe7060000.ethernet eth0: __stmmac_open: Hw setup failed
> 
> Let's simply write GMAC_PLLCLK_DIV_EN to GMAC_PLLCLK_DIV to enable the
> divider before MAC initialization. The rate doesn't matter, which we
> could reclock properly according to the link speed later after link is
> up.

All the possible DIV values are valid?
I think it's safer to set the DIV to some well known constant, 
just to be on the safe side.

Re: [PATCH net] net: stmmac: thead: Enable TX clock before MAC initialization

Posted by Yao Zi 2 months ago

On Tue, Aug 05, 2025 at 05:38:25PM -0700, Jakub Kicinski wrote:
> On Fri,  1 Aug 2025 09:45:07 +0000 Yao Zi wrote:
> > The clk_tx_i clock must be supplied to the MAC for successful
> > initialization. On TH1520 SoC, the clock is provided by an internal
> > divider configured through GMAC_PLLCLK_DIV register when using RGMII
> > interface. However, currently we don't setup the divider before
> > initialization of the MAC, resulting in DMA reset failures if the
> > bootloader/firmware doesn't enable the divider,
> > 
> > [    7.839601] thead-dwmac ffe7060000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
> > [    7.938338] thead-dwmac ffe7060000.ethernet eth0: PHY [stmmac-0:02] driver [RTL8211F Gigabit Ethernet] (irq=POLL)
> > [    8.160746] thead-dwmac ffe7060000.ethernet eth0: Failed to reset the dma
> > [    8.170118] thead-dwmac ffe7060000.ethernet eth0: stmmac_hw_setup: DMA engine initialization failed
> > [    8.179384] thead-dwmac ffe7060000.ethernet eth0: __stmmac_open: Hw setup failed
> > 
> > Let's simply write GMAC_PLLCLK_DIV_EN to GMAC_PLLCLK_DIV to enable the
> > divider before MAC initialization. The rate doesn't matter, which we
> > could reclock properly according to the link speed later after link is
> > up.
> 
> All the possible DIV values are valid?
> I think it's safer to set the DIV to some well known constant, 
> just to be on the safe side.

Oops, this statement seems misleading. I was going to say "the exact
rate isn't critical for MAC initialization". The patch actually sets
the divider to zero, which works according to my test.

And I realized the divider's behavior isn't well-documented when DIV is
set to zero. Will set the clock to RGMII speed in v2, and adjust the
commit message like

	Let's simply write GMAC_PLLCLK_DIV_EN to GMAC_PLLCLK_DIV to
	enable the divider before MAC initialization. The exact rate
	doesn't affect MAC's initialization according to my test. It's
	set to the speed required by RGMII and could be reclocked
	later after link is up if necessary.

Thanks,
Yao Zi