Re: [PATCH v3 13/47] clk: qcom: gcc-qcs8300: Tie the CX power domain to controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The GCC reset implementation does not take a pm_runtime reference before accessing reset registers, risking bus errors when consumers assert/deassert resets while the GCC controller is suspended.
--

--- Patch [13]: [PATCH v3 13/47] clk: qcom: gcc-qcs8300: Tie the CX power domain to controller ---
commit cad1ca2d5bc9c295249e6fa075b1dac72fab663e
Author: Abel Vesa <abel.vesa@oss.qualcomm.com>

clk: qcom: gcc-qcs8300: Tie the CX power domain to controller
    
Enable runtime PM for the controller, allowing CX to be powered down
once all GCC consumers have released their clocks and GDSCs.

> diff --git a/drivers/clk/qcom/gcc-qcs8300.c b/drivers/clk/qcom/gcc-qcs8300.c
> index 146b3b5dfead3..d04980e4a0bba 100644
> --- a/drivers/clk/qcom/gcc-qcs8300.c
> +++ b/drivers/clk/qcom/gcc-qcs8300.c
> @@ -3681,6 +3681,7 @@ static const struct qcom_cc_desc gcc_qcs8300_desc = {
>  	.num_resets = ARRAY_SIZE(gcc_qcs8300_resets),
>  	.gdscs = gcc_qcs8300_gdscs,
>  	.num_gdscs = ARRAY_SIZE(gcc_qcs8300_gdscs),
> +	.use_rpm = true,

[Severity: High]
This is a pre-existing issue, but does enabling use_rpm expose a risk of bus
errors during reset assertions?

By setting use_rpm to true, the GCC controller and its CX power domain are
allowed to runtime suspend when all clocks and GDSCs are released. However,
the generic QCOM reset operations do not ensure the device is runtime-active
before performing register writes.

For example, when qcom_reset_set_assert() in drivers/clk/qcom/reset.c is
called, it performs:

    regmap_update_bits(rst->regmap, map->reg, mask, assert ? mask : 0);

If a consumer asserts or deasserts a reset while the GCC controller is
suspended (such as from an error recovery interrupt, a teardown path, or by
a driver not utilizing PM runtime), could this register write fault and crash
the system?

Furthermore, because the reset operations use a fast_io regmap, they can be
invoked in an atomic context. If a consumer asserts a reset from an interrupt
handler while the GCC is suspended, it seems structurally impossible for the
consumer to wake the GCC controller first since doing so would sleep.

Is there a mechanism to prevent suspended register access during resets when
use_rpm is enabled?

>  };
>  
>  static const struct of_device_id gcc_qcs8300_match_table[] = {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-qcom-tie-cx-to-gcc-v3-0-f1205a92682b@oss.qualcomm.com?part=13