Re: [PATCH AUTOSEL 6.18] clk: qcom: clk-rpmh: Make all VRMs optional

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The clk-rpmh driver permanently mutates static global clock structures during probe, leading to severe resource address corruption on probe deferral or unbind/rebind.
--

commit db440aeaf96923e7980cc52bc879e1c6dfac5bf7
Author: Alexander Koskovich <akoskovich@pm.me>

clk: qcom: clk-rpmh: Make all VRMs optional

This commit marks all VRM clocks as optional to prevent probe failures on
boards where not all VRMs are present. It resolves issues on devices with
different PMIC configurations by checking the VRM offset directly.

Link: https://lore.kernel.org/r/20260414-clk-rpmh-vrm-opt-v3-1-8ca21469ffbc@pm.me

> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
> index 63c38cb47bc45..82f18e15cd77b 100644
> --- a/drivers/clk/qcom/clk-rpmh.c
> +++ b/drivers/clk/qcom/clk-rpmh.c

[ ... ]

> @@ -943,8 +938,7 @@ static int clk_rpmh_probe(struct platform_device *pdev)
>  		if (!res_addr) {
>  			hw_clks[i] = NULL;
>  
[Severity: High]
This is a pre-existing issue, but does this code permanently mutate static
global structures during probe?

Looking at clk_rpmh_probe(), it uses statically allocated arrays like
sm8550_rpmh_clocks and static structures. When the assignment above is
executed, it permanently clears missing clocks from the shared array.

Additionally, further down in this same function, the dynamic cmd-db address
is added to the static structure's initial offset:

    rpmh_clk->res_addr += res_addr;

If the driver is unbound and rebound via sysfs, or if probe defers, the
structure retains these mutated states. The probe path will erroneously add
the cmd-db address a second time for existing clocks, and missing clocks
will remain permanently NULL.

Could this lead to severe resource address corruption when issuing RPMh
commands to these accumulated addresses? Could a privileged user trigger
a sysfs unbind/bind cycle to corrupt the state and break SoC clock
management?

> -			if (desc->clka_optional &&
> -			    !strncmp(rpmh_clk->res_name, "clka", sizeof("clka") - 1))
> +			if (rpmh_clk->res_addr == CLK_RPMH_VRM_EN_OFFSET)
>  				continue;
>  
>  			dev_err(&pdev->dev, "missing RPMh resource address for %s\n",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-81-sashal@kernel.org?part=1