Re: [PATCH net-next v2 09/10] net: stmmac: rk: remove need for ->set_speed() method

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Mon, Feb 02, 2026 at 10:04:36AM +0000, Russell King (Oracle) wrote:
> As we can detect whether the SoC provides the parameters necessary for
> rk_set_reg_speed(), we don't need to have explicit calls to this.
> Instead, we can move the contents of this function to
> rk_set_clk_tx_rate().
> 
> This remsoves all the .set_speed() implementations that merely go on to
> invoke rk_set_reg_speed().
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

I notice that the AI review claims there is a problem with this patch,
where rk_set_clk_tx_rate() may return -EINVAL for id=1 on rk3328.

It claims:

  The new code in rk_set_clk_tx_rate() requires either
  gmii_clk_sel_mask for RGMII or rmii_clk_sel_mask/mac_speed_mask
  for RMII. For rk3328 id=1, rk3328_init() sets only clock_grf_reg
  without any clock masks, so the function falls through and returns
  -EINVAL.

It is correct that rk3328_init() doesn't set gmii_clk_sel_mask
nor rmii_clk_sel_mask/mac_speed_mask. However:

static const struct rk_gmac_ops rk3328_ops = {
...
        .clock.rmii_clk_sel_mask = BIT_U16(7),
        .clock.mac_speed_mask = BIT_U16(2),

which sets the default for these masks via:

static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
                                          struct plat_stmmacenet_data *plat,
                                          const struct rk_gmac_ops *ops)
...
        /* Set the default phy_intf_sel and RMII mode register parameters. */
        bsp_priv->gmac_grf_reg = ops->gmac_grf_reg;
        bsp_priv->gmac_phy_intf_sel_mask = ops->gmac_phy_intf_sel_mask;
        bsp_priv->gmac_rmii_mode_mask = ops->gmac_rmii_mode_mask;

        /* Set the default clock control register related parameters */
        bsp_priv->clock_grf_reg = ops->clock_grf_reg;
        bsp_priv->clock = ops->clock;

Thus the AI review is incorrect.

Not setting clock.gmii_clk_sel_mask is fine, because gmac2phy (id=1)
only supports RMII. That was documented in a later patch that, because
of the problem with AI reviews failing, is not part of this smaller
patch set:

	net: stmmac: rk: rk3328: gmac2phy only supports RMII

Given that there are sixteen Rockchip SoCs in this file, describing
in detail every change just in case AI spontaneously decides there's
a problem that doesn't exist doesn't scale.

Note that previous AI review of this very same patch but part of the
full patch series passed without issue:

https://patchwork.kernel.org/project/netdevbpf/patch/E1vkL2s-00000005uta-3W8V@rmk-PC.armlinux.org.uk/

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!