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

[Simon Horman <horms@kernel.org>]

On Fri, Aug 08, 2025 at 10:34:48AM +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 exact rate doesn't affect MAC's
> initialization according to my test. It's set to the speed required by
> RGMII when the linkspeed is 1Gbps and could be reclocked later after
> link is up if necessary.
> 
> Fixes: 33a1a01e3afa ("net: stmmac: Add glue layer for T-HEAD TH1520 SoC")
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
> 
> 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 scheme (enables the divider first and reclock it later) requires
> access to the APB glue registers, the patch depends on v3 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/20250808093655.48074-2-ziyao@disroot.org/
> 
> Changed from v1
> - Initialize the divisor to a well-known value (producing the clock rate
>   required by RGMII link at 1Gbps)
> - Write zero to GMAC_PLLCLK_DIV before writing the configuration, as
>   required by the TRM

FWIIW, I think it would be worth adding something about writing zero
to the commit message.

> - Link to v1: https://lore.kernel.org/netdev/20250801094507.54011-1-ziyao@disroot.org/
> 
>  drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

...