Re: [PATCH net-next 5/6] net: stmmac: rk: use rk_encode_wm16() for clock selection

["Heiko Stübner" <heiko@sntech.de>]

Am Mittwoch, 4. Februar 2026, 10:14:38 Mitteleuropäische Normalzeit schrieb Russell King (Oracle):
> Use rk_encode_wm16() for RMII clock gating control, and also for the
> io_clksel bit used to select the transmit clock between CRU-derived
> and IO-derived clock sources.
> 
> Both of these were configured via the "set_clock_selection" method in
> the SoC specific operations, but there is no requirement to change the
> io_clksel except when enabling clocks.
> 
> It is also possible that we don't need to ungate the RMII clock if we
> are operating in RGMII mode, but this commit makes no change there.
> 
> Split up the configuration of these as separate functions, and remove
> the set_clock_selection() method. Since these clocking bits are in the
> same register that we call the "speed" register, move the logic for
> writing that register into rk_write_speed_grf_reg().
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 174 ++++++++----------
>  1 file changed, 75 insertions(+), 99 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index 4099cbc5d0de..ed9adac70f0a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -27,8 +27,17 @@
>  struct rk_priv_data;
>  
>  struct rk_clock_fields {
> +	/* io_clksel_cru_mask - io_clksel bit in clock GRF register which,
> +	 * when set, selects the tx clock from CRU.
> +	 */
> +	u16 io_clksel_cru_mask;
> +	/* io_clksel_io_mask - io_clksel bit in clock GRF register which,
> +	 * when set, selects the tx clock from IO.
> +	 */

nit: comment stile does not seem to follow the kernel coding style
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n622

> +	u16 io_clksel_io_mask;
>  	u16 gmii_clk_sel_mask;
>  	u16 rmii_clk_sel_mask;
> +	u16 rmii_gate_en_mask;
>  	u16 mac_speed_mask;
>  };

[...]

> +	/* The io_clksel configuration can be either:
> +	 *  0=CRU, 1=IO (rk3506, rk3520, rk3576) or
> +	 *  0=IO, 1=CRU (rk3588)
> +	 * where CRU means the transmit clock comes from the CRU and IO
> +	 * means the transmit clock comes from IO.
> +	 *
> +	 * Handle this by having two masks.
> +	 */

nit: same as above

Other than these 2 nits, this looks good to me.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Heiko Stuebner <heiko@sntech.de> #px30,rk3328,rk3568,rk3588