Re: [RFC PATCH 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The SGMII initialization path inadvertently skips enabling the MAC clocks, leading to hardware hangs or faults when MAC registers are accessed.
--

--- Patch [8]: [RFC PATCH 08/10] net: stmmac: dwmac-rk: add SGMII support for RK3568 ---
commit 7be50db2ac8c40c8451407f345aaaaf2d0538799
Author: Coia Prant <coiaprant@gmail.com>

net: stmmac: dwmac-rk: add SGMII support for RK3568

The RK3568 SoC integrates a Synopsys DesignWare XPCS that can be connected
to GMAC0 or GMAC1 in SGMII mode. Add the necessary glue logic to support
this configuration.

Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 386)

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index 8d7042e689261..eca482b4b6bfc 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
[ ... ]
> @@ -1379,6 +1441,9 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
>  	if (ret)
>  		return ret;
>  
> +	if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_SGMII)
> +		goto set_mode;
> +

[Severity: High]
Does this goto skip enabling the MAC clocks? By jumping directly to set_mode,
we bypass the call to gmac_clk_enable() further down in this function.

Can this cause a synchronous external abort when the MAC registers are accessed
subsequently while their clock domain remains disabled?

>  	ret = rk_get_phy_intf_sel(bsp_priv->phy_iface);
>  	if (ret < 0)
>  		return ret;
[ ... ]
> -	/*rmii or rgmii*/
> +set_mode:
> +	/* rmii, rgmii, sgmii */
>  	switch (bsp_priv->phy_iface) {
>  	case PHY_INTERFACE_MODE_RGMII:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714191341.690906-1-coiaprant@gmail.com?part=8