Re: [PATCH v13 09/11] clk: realtek: Add RTD1625-CRT clock controller driver

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Sleeping in atomic context due to `fsleep()` in `regmap_multi_reg_write()` called under a spinlock.
--

--- Patch [9]: [PATCH v13 09/11] clk: realtek: Add RTD1625-CRT clock controller driver ---
commit 94dc67c9ada3ad3a78ad1cf0f8ae4674b5f69224
Author: Cheng-Yu Lee <[email protected]>

clk: realtek: Add RTD1625-CRT clock controller driver
    
Add support for the CRT (Clock, Reset, and Test) domain clock controller
on the Realtek RTD1625 SoC. This driver provides essential clock sources
(including PLLs), gating, and multiplexing functionalities.

> diff --git a/drivers/clk/realtek/clk-rtd1625-crt.c b/drivers/clk/realtek/clk-rtd1625-crt.c
> new file mode 100644
> index 0000000000000..3da88d3cf0dd2
> --- /dev/null
> +++ b/drivers/clk/realtek/clk-rtd1625-crt.c

[ ... ]

> +static const struct reg_sequence pll_acpu_seq_power_on[] = {
> +	{RTD1625_REG_PLL_ACPU2,         0x5},
> +	{RTD1625_REG_PLL_ACPU2,         0x7},
> +	{RTD1625_REG_PLL_ACPU1,         0x54000},
> +	{RTD1625_REG_PLL_SSC_DIG_ACPU2, 0x1e1f8e},
> +	{RTD1625_REG_PLL_SSC_DIG_ACPU0, 0x4},
> +	{RTD1625_REG_PLL_SSC_DIG_ACPU0, 0x5, 200},

[Severity: High]
Does this 200us delay cause a "scheduling while atomic" bug when enabling the
clock?

Looking at rtk_clk_regmap_pll_enable() in drivers/clk/realtek/clk-pll.c, it
acquires an irq-disabled spinlock before processing this power-on sequence:

	spin_lock_irqsave(&clkp->lock, flags);

	if (is_power_on(clkp))
		goto unlock;

	ret = regmap_multi_reg_write(clkp->clkr.regmap, clkp->seq_power_on,
				     clkp->num_seq_power_on);

When regmap_multi_reg_write() encounters the 200us delay in this sequence,
it calls fsleep(200), which delegates to usleep_range() and sleeps.

Since this is called while holding the spinlock, could it trigger a BUG or
panic? Could the power-on sequence be moved to a .prepare callback which is
allowed to sleep, or perhaps use a custom delay implementation that safely
spins using udelay()?

This also applies to the other power-on sequences introduced in this file
(pll_ve1_seq_power_on, pll_gpu_seq_power_on, pll_npu_seq_power_on,
pll_ve2_seq_power_on, and pll_hifi_seq_power_on).

> +	{RTD1625_REG_PLL_ACPU2,         0x3},
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9