Re: [PATCH 1/6] soc: qcom: rpmh-rsc: Skip TCS init when RSC is managed by firmware

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Returning early from the probe function bypasses the initialization of critical locks, waitqueues, and list heads, leading to kernel panics when RPMh APIs are invoked.
- [High] The early return completely skips `devm_of_platform_populate()`, preventing essential child devices from probing and permanently breaking the system boot process.
--

commit 42c9f85b1ee98ffd687b17f97edf46cb0e222ced
Author: Graham O'Connor <graham.oconnor@gmail.com>

soc: qcom: rpmh-rsc: Skip TCS init when RSC is managed by firmware

This commit skips TCS initialization in the RPMh RSC driver when the hardware
solver is already active. This prevents security violations and system resets
caused by reinitializing a controller that is actively managed by firmware.

> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c

[ ... ]

> @@ -1074,6 +1074,20 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
>  	else
>  		drv->regs = rpmh_rsc_reg_offset_ver_2_7;
>  
> +	/*
> +	 * On some platforms the RSC is already managed by the firmware
> +	 * when the kernel boots. Calling rpmh_probe_tcs_config() in this
> +	 * state would reinitialize the controller and cause a security
> +	 * violation. Skip TCS initialization if the hardware solver is
> +	 * already active.
> +	 */
> +	if (readl_relaxed(drv->base + drv->regs[DRV_SOLVER_CONFIG]) &
> +	    (DRV_HW_SOLVER_MASK << DRV_HW_SOLVER_SHIFT)) {
> +		dev_dbg(&pdev->dev, "RSC already managed by firmware, skipping TCS init\n");
> +		platform_set_drvdata(pdev, drv);
> +		return 0;

[Severity: Critical]
Since drv is allocated with devm_kzalloc(), bypassing the rest of the probe
function leaves essential software structures uninitialized.

Specifically, spin_lock_init(&drv->client.cache_lock),
init_waitqueue_head(&drv->tcs_wait), and INIT_LIST_HEAD(&drv->client.cache)
are skipped.

Because platform_set_drvdata(pdev, drv) exposes this controller to other
drivers, will standard RPMh APIs like rpmh_write() crash with a NULL pointer
dereference when attempting to acquire the uninitialized spinlock or add to
the zero-initialized list head?

[Severity: High]
Does this early return also skip the instantiation of child devices?

The rpmh_rsc_probe() function normally relies on devm_of_platform_populate()
at the end of the function to instantiate its children. By returning early,
crucial child devices such as the clock controller (rpmhcc), power domain
controller (rpmhpd), and interconnect voters (bcm-voter) are never created.

Will this leave downstream peripheral drivers that depend on these clocks
or regulators permanently stuck in an -EPROBE_DEFER state, breaking the
system boot process?

> +	}
> +
>  	ret = rpmh_probe_tcs_config(pdev, drv);
>  	if (ret)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522060645.4399-1-graham.oconnor@gmail.com?part=1