Re: [PATCH v6 09/12] clk: zte: Introduce a driver for zx297520v3 matrix clocks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Required GMAC clock gates lack the CLK_IS_CRITICAL flag and will be disabled by the clock framework.
--

commit 1d4a13593592a02eeff53731204ae6b0376ef509
Author: Stefan Dösinger <stefandoesinger@gmail.com>
Subject: clk: zte: Introduce a driver for zx297520v3 matrix clocks

This commit introduces a new clock driver for the ZTE zx297520v3 matrix
clocks. It defines the matrix muxes, inputs, and gates, mapping them to
the hardware registers. The driver is then registered alongside the
existing topclk driver.

> diff --git a/drivers/clk/zte/clk-zx297520v3.c b/drivers/clk/zte/clk-zx297520v3.c
> index 6fc98f98cf9c8..501f333b84fee 100644
> --- a/drivers/clk/zte/clk-zx297520v3.c
> +++ b/drivers/clk/zte/clk-zx297520v3.c

[ ... ]

> +	/* ZTE's driver has a statemt to the effect of *(top->base+0x11c) = 5, with a comment
> +	 * suggesting that this sets a 50 mhz clock. The clock code itself lists gmac clocks in
> +	 * matrix+110 and lists the parents of these clock as 50mhz gpll output, but the GMAC
> +	 * driver never enables the clocks. It turns out ZTE's code is highly misleading.
> +	 *
> +	 * The GMAC's work clock is definitly not any gpll output because it keeps working fine with
> +	 * gpll disabled. The MDIO speed is mostly unaffected by mpll speed changes, so it is most
> +	 * likely not fed by mpll either. All other PLLs can be disabled without breaking GMAC, so
> +	 * osc26m is the only possible remaining parent.
> +	 *
> +	 * The GMAC Gates are left enabled by the boot loader and are required for the GMAC to work.
> +	 *
> +	 * As for the 50 MHz comment: See rmiiphy_wclk.
> +	 */
> +	GATE(ZX297520V3_GMAC_WCLK,         "gmac_wclk",      clk_main[0],     0x110,  0, 0),
> +	GATE(ZX297520V3_GMAC_PCLK,         "gmac_pclk",      clk_main[0],     0x110,  1, 0),
> +	GATE(ZX297520V3_GMAC_AHB,          "gmac_ahb",       "ahb_wclk",      0x110,  2, 0),

[Severity: High]
Since the comment indicates the GMAC driver never enables these clocks but
they are required for the GMAC to work, will the clock framework's
clk_disable_unused() late initcall turn them off?

If they are left with 0 flags, the unused clock sweep will disable any
unclaimed clocks. Should these gates be marked with CLK_IS_CRITICAL (or
CLK_IGNORE_UNUSED) so they remain enabled and the Ethernet interface
keeps functioning?

> +
> +	GATE(ZX297520V3_MBOX_PCLK,         "mbox_pclk",      clk_main[0],      0x88,  2, 0),

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-zx29clk-v6-0-377b704f80c4@gmail.com?part=9