Re: [PATCH] clk: mediatek: mt7629: Properly use CLK_IS_CRITICAL flag

Hi Ruoyu,

Just one more comment on top of the Sashiko review.

On Thu, Aug 13, 2026 at 11:31:13PM +0800, Ruoyu Wang wrote:
> The MT7629 clock driver calls clk_prepare_enable() for six clocks that
> must remain running. These calls create permanent references owned by
> the driver. In topckgen and apmixedsys, provider registration can fail
> after the references are acquired, leaving them unreleased.
> 
> Mark ARMPLL, main_core_en, the AXI, memory and DDRPHY selectors, and the
> UART0 gate with CLK_IS_CRITICAL, and remove the manual enable calls. The
> common clock framework then keeps these clocks enabled without
> driver-owned references. This preserves their always-on behavior and
> avoids leaking references when provider registration fails.
> 
> This issue was found by a static analysis checker and confirmed by manual
> source review.
> 
> Fixes: 3b5e748615e7 ("clk: mediatek: add clock support for MT7629 SoC")
> Signed-off-by: Ruoyu Wang <[email protected]>
> ---
>  drivers/clk/mediatek/clk-mt7629.c | 36 ++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt7629.c b/drivers/clk/mediatek/clk-mt7629.c
> index e154771b1b8bba..627e8017fa642f 100644
> --- a/drivers/clk/mediatek/clk-mt7629.c
> +++ b/drivers/clk/mediatek/clk-mt7629.c
> @@ -48,8 +48,9 @@
>  		_pd_reg, _pd_shift, _tuner_reg, _pcw_reg, _pcw_shift,	\
>  		NULL, "clk20m")
>  
> -#define GATE_APMIXED(_id, _name, _parent, _shift)			\
> -	GATE_MTK(_id, _name, _parent, &apmixed_cg_regs, _shift, &mtk_clk_gate_ops_no_setclr_inv)
> +#define GATE_APMIXED_AO(_id, _name, _parent, _shift)			\
> +	GATE_MTK_FLAGS(_id, _name, _parent, &apmixed_cg_regs, _shift,	\
> +		 &mtk_clk_gate_ops_no_setclr_inv, CLK_IS_CRITICAL)

[snip]

>  static const struct mtk_gate apmixed_clks[] = {
> -	GATE_APMIXED(CLK_APMIXED_MAIN_CORE_EN, "main_core_en", "mainpll", 5),
> +	GATE_APMIXED_AO(CLK_APMIXED_MAIN_CORE_EN, "main_core_en", "mainpll", 5),

We shouldn't put CLK_IS_CRITICAL in these generic macros for defining
clocks. Your commit message defines specific clocks where this needs to
happen. Put a flags argument in the macro, and pass the critical flag
in. 

There should also be a comment with the code describing why the critical
flag is needed.

Brian