netdev - Re: [net-next,19/21] net: stmmac: rk: rk3528: gmac0 only supports RMII

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260125221632.3396270-1-kuba@kernel.org>
Date: Sun, 25 Jan 2026 14:16:32 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: rmk+kernel@...linux.org.uk
Cc: Jakub Kicinski <kuba@...nel.org>,
	linux-arm-kernel@...ts.infradead.org,
	andrew@...n.ch,
	davem@...emloft.net,
	alexandre.torgue@...s.st.com,
	netdev@...r.kernel.org,
	edumazet@...gle.com,
	pabeni@...hat.com,
	heiko@...ech.de,
	andrew+netdev@...n.ch,
	linux-rockchip@...ts.infradead.org,
	linux-stm32@...md-mailman.stormreply.com
Subject: Re: [net-next,19/21] net: stmmac: rk: rk3528: gmac0 only supports RMII

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: stmmac: rk: rk3506, rk3528 and kk3588 have rmii_mode in clock register

This commit moves the rmii_mode configuration for rk3506, rk3528, and
rk3588 from the per-SoC set_to_rmii() functions into the centralized
rk_gmac_powerup() function, using a new clock.rmii_mode_mask field.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index ebbc09ea572a..759deb66ef2a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c

[ ... ]

> @@ -987,6 +966,7 @@ static int rk3588_init(struct rk_priv_data *bsp_priv)
>  		bsp_priv->clock.gmii_clk_sel_mask = GENMASK_U16(3, 2);
>  		bsp_priv->clock.rmii_clk_sel_mask = BIT_U16(2);
>  		bsp_priv->clock.rmii_gate_en_mask = BIT_U16(1);
> +		bsp_priv->clock.rmii_mode_mask = BIT_U16(0);
>  		return 0;

[ ... ]

> @@ -1412,6 +1385,15 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
>  			return ret;
>  	}
>
> +	if (bsp_priv->clock.rmii_mode_mask) {
> +		val = rk_encode_wm16(ret == PHY_INTF_SEL_RMII,
> +				     bsp_priv->clock.rmii_mode_mask);

For rk3588, the variable ret has been overwritten by the preceding
regmap_write() in the gmac_phy_intf_sel_mask block. At this point ret
contains 0 (success) rather than PHY_INTF_SEL_RMII (4).

Looking at rk_gmac_powerup(), the flow for rk3588 is:

    ret = rk_get_phy_intf_sel(bsp_priv->phy_iface);  // ret = 4 for RMII
    ...
    if (bsp_priv->gmac_phy_intf_sel_mask || ...)     // true for rk3588
        ...
        ret = regmap_write(...);                     // ret = 0 on success
    ...
    if (bsp_priv->clock.rmii_mode_mask)              // true for rk3588
        val = rk_encode_wm16(ret == PHY_INTF_SEL_RMII, ...);  // 0 == 4 is false

Since rk3588_init() sets both gmac_phy_intf_sel_mask and
clock.rmii_mode_mask, does this mean RMII mode can never be correctly
configured for rk3588?

> +
> +		ret = rk_write_clock_grf_reg(bsp_priv, val);
> +		if (ret < 0)
> +			return ret;
> +	}

[ ... ]