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

[Yao Zi]

On Fri, Aug 08, 2025 at 02:39:00PM +0100, Simon Horman wrote:
> On Fri, Aug 08, 2025 at 10:34:48AM +0000, Yao Zi wrote:
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
> > index f2946bea0bc2..50c1920bde6a 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-thead.c
> > @@ -152,7 +152,7 @@ static int thead_set_clk_tx_rate(void *bsp_priv, struct clk *clk_tx_i,
> >  static int thead_dwmac_enable_clk(struct plat_stmmacenet_data *plat)
> >  {
> >  	struct thead_dwmac *dwmac = plat->bsp_priv;
> > -	u32 reg;
> > +	u32 reg, div;
> >  
> >  	switch (plat->mac_interface) {
> >  	case PHY_INTERFACE_MODE_MII:
> > @@ -164,6 +164,13 @@ 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 */
> > +		div = clk_get_rate(plat->stmmac_clk) / rgmii_clock(SPEED_1000);
> > +		reg = FIELD_PREP(GMAC_PLLCLK_DIV_EN, 1) |
> > +		      FIELD_PREP(GMAC_PLLCLK_DIV_NUM, div),
> 
> Sorry for not noticing this before sending my previous email.
> 
> Although the code above is correct. I think it would be clearer
> to use ';' rather than ',' at the end of the line above. Perhaps ','
> is a typo.(',' is next to ';' on my keyboard at least).

This is actually a typo that occasionally passes the compilation... yeah
thanks for catching it.

I'll add a comment about the requirement that PLLCLK_DIV_EN must be set
to zero before changing the rate, and fix this typo in v3 as well.

> Flagged by Clang 20.1.8 with -Wcomma

Best regards,
Yao Zi

> > +
> > +		writel(0, dwmac->apb_base + GMAC_PLLCLK_DIV);
> > +		writel(reg, 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;
> 
> -- 
> pw-bot: changes-requested.