Re: [PATCH v2 08/14] clk: samsung: exynos8855: add block peris clock support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk: missing CLK_IS_CRITICAL on system-critical gate clocks
--

commit 717955d4667d1084ee3be06cb476ef2408019f3e
Author: Raghav Sharma <raghav.s@samsung.com>

clk: samsung: exynos8855: add block peris clock support

Register compatible and cmu_info data to support clocks.
CMU_PERIS, this provides clocks for PERIS.

> diff --git a/drivers/clk/samsung/clk-exynos8855.c b/drivers/clk/samsung/clk-exynos8855.c
> index 5394706d214c9..2bf889a478bc9 100644
> --- a/drivers/clk/samsung/clk-exynos8855.c
> +++ b/drivers/clk/samsung/clk-exynos8855.c
[ ... ]
> @@ -1094,6 +1096,99 @@ static const struct samsung_cmu_info peric_cmu_info __initconst = {
[ ... ]
> +static const struct samsung_gate_clock peris_gate_clks[] __initconst = {
> +	/* System will hang if this critical clock is gated */
> +	GATE(CLK_GOUT_CMU_PERIS, "gout_cmu_peris",
> +	     "dout_clkcmu_peris_nocp",
> +	     CMU_PERIS_CLK_CON_GAT_BLK_PERIS_UID_CMU_PERIS_IPCLKPORT_PCLK,
> +	     21, 0, 0),

[Severity: High]
Does this code expose the clock tree to fatal disablement by
clk_disable_unused()?

Even though .auto_clock_gate = true provides a no-op .disable callback for leaf
clocks, the Linux clock framework will still transition its software state to
DISABLED and decrement the reference counts of the entire parent chain (e.g.,
dout_clkcmu_peris_nocp -> mout_clkcmu_peris_noc_user -> ... ->
fout_shared0_pll).

Because root parents like PLLs use real hardware .disable operations, they will
be physically powered off when their reference count hits zero.

Should the CLK_IS_CRITICAL flag be explicitly passed in the flags parameter
here to artificially bump the reference count and keep the entire parent tree
active forever?

A parallel block introduced earlier in this patch series has the same issue:

drivers/clk/samsung/clk-exynos8855.c:
static const struct samsung_gate_clock peric_gate_clks[] __initconst = {
	/* System will hang if this critical clock is gated */
	GATE(CLK_GOUT_CMU_PERIC, "gout_cmu_peric",
	     "dout_clkcmu_peric_nocp",
	     CMU_PERIC_CLK_CON_GAT_BLK_PERIC_UID_CMU_PERIC_IPCLKPORT_PCLK,
	     21, 0, 0),
        ...
};

Should it also be updated with the CLK_IS_CRITICAL flag to prevent system
hangs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009120137.2801911-1-raghav.s@samsung.com?part=8