Re: [PATCH v2 3/3] clk: qcom: dprxcc: Add Nord DPRX clock controller support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk: qcom: missing runtime PM enablement in DPRX CC probe
--

commit 6de521e23dcd665ee6611adb907119066bae1783
Author: Taniya Das <taniya.das@oss.qualcomm.com>

clk: qcom: dprxcc: Add Nord DPRX clock controller support

This commit adds support for the DisplayPort Receiver (DPRX) clock controllers
on Nord devices. Two identical instances, DPRX0 and DPRX1, are introduced to
provide core, audio, AHB, and video memory clocks.

> diff --git a/drivers/clk/qcom/dprxcc0-nord.c b/drivers/clk/qcom/dprxcc0-nord.c
> new file mode 100644
> index 0000000000000..d17b9338f33bf
> --- /dev/null
> +++ b/drivers/clk/qcom/dprxcc0-nord.c

[ ... ]

> +static void dprx_cc_0_nord_regs_configure(struct device *dev, struct regmap *regmap)
> +{
> +	/*
> +	 * Configure cdiv 20 for core aud and 11 for core ahb clks of DPRX CC
> +	 */
> +	regmap_write(regmap, dprx_0_dprx_cc_core_aud_div_clk_src.reg, 0x13);
> +	regmap_write(regmap, dprx_0_dprx_cc_core_ahb_div_clk_src.reg, 0x0A);
> +}

[Severity: High]
Will this direct register access trigger a synchronous external abort? 

Since the probe function unconditionally calls qcom_cc_probe() which then
directly invokes this configuration callback, the registers are accessed
before the power domain is asserted. Accessing physical registers of a
peripheral clock controller without its power domain being ON can hang the
system at boot.

[ ... ]

> +static const struct qcom_cc_desc dprx_cc_0_nord_desc = {
> +	.config = &dprx_cc_0_nord_regmap_config,
> +	.clks = dprx_cc_0_nord_clocks,
> +	.num_clks = ARRAY_SIZE(dprx_cc_0_nord_clocks),
> +	.resets = dprx_cc_0_nord_resets,
> +	.num_resets = ARRAY_SIZE(dprx_cc_0_nord_resets),
> +	.driver_data = &dprx_cc_0_nord_driver_data,
> +};

[Severity: High]
Should this descriptor include .use_rpm = true?

Without it, the QCOM CC core does not automatically call
devm_pm_runtime_enable() and pm_runtime_resume_and_get() before invoking
the clk_regs_configure callback, which leads to the unpowered register
access above.

> diff --git a/drivers/clk/qcom/dprxcc1-nord.c b/drivers/clk/qcom/dprxcc1-nord.c
> new file mode 100644
> index 0000000000000..851a97c196c0a
> --- /dev/null
> +++ b/drivers/clk/qcom/dprxcc1-nord.c

[ ... ]

> +static const struct qcom_cc_desc dprx_cc_1_nord_desc = {
> +	.config = &dprx_cc_1_nord_regmap_config,
> +	.clks = dprx_cc_1_nord_clocks,
> +	.num_clks = ARRAY_SIZE(dprx_cc_1_nord_clocks),
> +	.resets = dprx_cc_1_nord_resets,
> +	.num_resets = ARRAY_SIZE(dprx_cc_1_nord_resets),
> +	.driver_data = &dprx_cc_1_nord_driver_data,
> +};

[Severity: High]
Does this second instance also need .use_rpm = true added here to prevent
the same unpowered register access issue during probe?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-nords_dprx-v2-0-fa4364e087ef@oss.qualcomm.com?part=3