netdev - Re: [PATCH net-next v2 09/10] net: stmmac: rk: remove need for ->set

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: <aYH7nTGx474azyk-@shell.armlinux.org.uk>
Date: Tue, 3 Feb 2026 13:43:57 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <kuba@...nel.org>
Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Heiko Stuebner <heiko@...ech.de>,
	linux-arm-kernel@...ts.infradead.org,
	linux-rockchip@...ts.infradead.org,
	linux-stm32@...md-mailman.stormreply.com, netdev@...r.kernel.org,
	Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next v2 09/10] net: stmmac: rk: remove need for
 ->set_speed() method

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@...linux.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!