netdev - Re: [net-next,v2,21/22] net: stmmac: rk: use rk_encode

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: <20260127004101.3785871-1-kuba@kernel.org>
Date: Mon, 26 Jan 2026 16:41:01 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: rmk+kernel@...linux.org.uk
Cc: Jakub Kicinski <kuba@...nel.org>,
	linux-stm32@...md-mailman.stormreply.com,
	andrew+netdev@...n.ch,
	linux-arm-kernel@...ts.infradead.org,
	netdev@...r.kernel.org,
	pabeni@...hat.com,
	linux-rockchip@...ts.infradead.org,
	heiko@...ech.de,
	alexandre.torgue@...s.st.com,
	andrew@...n.ch,
	davem@...emloft.net,
	edumazet@...gle.com
Subject: Re: [net-next,v2,21/22] net: stmmac: rk: use rk_encode_wm16() for clock selection

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: use rk_encode_wm16() for clock selection

This commit refactors the per-SoC set_clock_selection() method into
unified helper functions using rk_encode_wm16(), with mask-based
configuration stored in struct rk_clock_fields.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index 26fc93854cb0..443d3bd62cae 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c

[ ... ]

> @@ -1000,25 +991,23 @@ static const struct rk_gmac_ops rk3576_ops = {
>  #define RK3588_GMAC_CLK_RMII_MODE(id)		GRF_BIT(5 * (id))
>  #define RK3588_GMAC_CLK_RGMII_MODE(id)		GRF_CLR_BIT(5 * (id))
>
> -#define RK3588_GMAC_CLK_SELECT_CRU(id)		GRF_BIT(5 * (id) + 4)
> -#define RK3588_GMAC_CLK_SELECT_IO(id)		GRF_CLR_BIT(5 * (id) + 4)
> -
> -#define RK3588_GMAC_CLK_RMII_GATE(id)		GRF_BIT(5 * (id) + 1)
> -#define RK3588_GMAC_CLK_RMII_NOGATE(id)		GRF_CLR_BIT(5 * (id) + 1)
> -
>  static int rk3588_init(struct rk_priv_data *bsp_priv)
>  {
>  	switch (bsp_priv->id) {
>  	case 0:
>  		bsp_priv->gmac_phy_intf_sel_mask = GENMASK_U16(5, 3);
> +		bsp_priv->clock.io_clksel_cru_mask = BIT_U16(4);
>  		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);
>  		return 0;
>
>  	case 1:
>  		bsp_priv->gmac_phy_intf_sel_mask = GENMASK_U16(11, 9);
> +		bsp_priv->clock.io_clksel_cru_mask = BIT_U16(10);
                                                        ^^

Should this be BIT_U16(9) instead of BIT_U16(10)?

The removed macro RK3588_GMAC_CLK_SELECT_CRU(id) used the formula
GRF_BIT(5 * (id) + 4), which for id=1 gives 5*1+4 = bit 9.

The other masks for id=1 follow the 5*(id) pattern correctly:
  - rmii_gate_en_mask = BIT_U16(6)  matches 5*1+1 = 6
  - gmii_clk_sel_mask = GENMASK_U16(8, 7)  matches 5*1+{2,3}
  - rmii_clk_sel_mask = BIT_U16(7)

But io_clksel_cru_mask = BIT_U16(10) does not match 5*1+4 = 9.

>  		bsp_priv->clock.gmii_clk_sel_mask = GENMASK_U16(8, 7);
>  		bsp_priv->clock.rmii_clk_sel_mask = BIT_U16(7);
> +		bsp_priv->clock.rmii_gate_en_mask = BIT_U16(6);
>  		return 0;

[ ... ]