[net-next,17/21] net: stmmac: rk: replace empty set_to_rmii() with supports_rmii

[Jakub Kicinski]

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 patch consolidates RMII mode configuration for rk3506, rk3528, and
rk3588 by moving the rmii_mode bit writes from per-SoC set_to_rmii()
functions into a centralized block in rk_gmac_powerup().

> 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

[ ... ]

> @@ -1412,6 +1385,15 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
> 			return ret;
> 	}

The context above this hunk is relevant. Earlier in rk_gmac_powerup():

    ret = rk_get_phy_intf_sel(bsp_priv->phy_iface);
    ...
    if (bsp_priv->gmac_phy_intf_sel_mask ||
        bsp_priv->gmac_rmii_mode_mask) {
        ...
        ret = regmap_write(bsp_priv->grf, bsp_priv->gmac_grf_reg, val);
        ...
    }

At this point, if the first if-block executed, ret has been overwritten by
regmap_write() and will be 0 on success.

> +	if (bsp_priv->clock.rmii_mode_mask) {
> +		val = rk_encode_wm16(ret == PHY_INTF_SEL_RMII,
> +				     bsp_priv->clock.rmii_mode_mask);

For RK3588, both gmac_phy_intf_sel_mask and clock.rmii_mode_mask are set.
The first if-block will execute and overwrite ret with the regmap_write()
return value (0 on success).

When this second if-block runs, ret will be 0 rather than the original
PHY_INTF_SEL_RMII value (which is 4). This means ret == PHY_INTF_SEL_RMII
will always evaluate to false for RK3588, causing the rmii_mode bit to be
cleared instead of set when RMII mode is configured.

Should ret be saved before the first if-block and restored here, or should
a separate variable hold the phy interface selection?

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

[ ... ]