Re: [PATCH v3 2/2] clk: qcom: Add LPASS VA CSR heartbeat pulse clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing PM runtime enablement in lpass-va-csr causes a synchronous external abort when CCF reads the hardware state.
--

--- Patch [2]: [PATCH v3 2/2] clk: qcom: Add LPASS VA CSR heartbeat pulse clock ---
commit f6c6796cc5343493b52dddf79a9e09e549136b51
Author: Sarath Ganapathiraju <sarath.ganapathiraju@oss.qualcomm.com>

clk: qcom: Add LPASS VA CSR heartbeat pulse clock

Adds the LPASS VA CSR driver to model the rate generator as a clock provider.
This synchronizes the start of DMAs and Codec Interfaces and can serve as a
periodic wakeup source for the DSP.

> diff --git a/drivers/clk/qcom/lpass-va-csr.c b/drivers/clk/qcom/lpass-va-csr.c
> new file mode 100644
> index 0000000000000..1d0805789e25a
> --- /dev/null
> +++ b/drivers/clk/qcom/lpass-va-csr.c

[ ... ]

> +static int heartbeat_pulse_is_prepared(struct clk_hw *hw)
> +{
> +	struct lpass_va_csr *csr = to_lpass_va_csr(hw);
> +
> +	return regmap_test_bits(csr->regmap, LPASS_RATE_GEN_CTRL, LPASS_RG_CTRL_EN);

[Severity: High]
Could this MMIO read cause a synchronous external abort?

When CCF calls the .is_prepared callback (e.g., during late boot via
clk_disable_unused() or a debugfs read from clk_summary), this executes an
unprotected MMIO read. If the LPASS hardware is power-gated at this time, the
unpowered access can crash the system.

> +}

[ ... ]

> +static int lpass_va_csr_probe(struct platform_device *pdev)
> +{

[ ... ]

> +	csr->hb_hw.init = &init;
> +
> +	ret = devm_clk_hw_register(dev, &csr->hb_hw);

[Severity: High]
Is a call to devm_pm_runtime_enable() missing before clock registration?

Without PM runtime enabled on the device beforehand, CCF's
clk_pm_runtime_get() sees core->rpm_enabled as false and skips acquiring the
power domain. This leaves the hardware unpowered and vulnerable to crashes
during the .is_prepared checks.

> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register heartbeat clock\n");
> +
> +	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &csr->hb_hw);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to add clock provider\n");
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-master-v3-0-3d67b79b0515@oss.qualcomm.com?part=2